Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add interactive migration selection for rollback command #54250

Closed
wants to merge 9 commits into from

Conversation

rez1Pro
Copy link

@rez1Pro rez1Pro commented Jan 18, 2025

Interactive Migration Selection for Rollback Command

Description

This PR enhances the migrate:rollback command by adding an interactive multi-select prompt when no step or batch options are provided. Users can now selectively choose which migrations they want to rollback, providing more fine-grained control over the database migration process.

Features

  • Interactive multi-select prompt for migration rollback
  • Users can select multiple migrations using spacebar
  • Scrollable interface when there are many migrations (limited to 10 visible at a time)
  • Maintains existing step/batch functionality when those options are provided

Usage

When running php artisan migrate:rollback --select add --select options:

image

Why

Previously, users could only rollback migrations by batch or step numbers, which sometimes led to rolling back more migrations than intended. This feature provides more precise control over which migrations to rollback, especially useful in development environments where you might want to rollback specific migrations without affecting others.

Testing

  • Tested with multiple migrations
  • Verified behavior when no migrations exist
  • Confirmed compatibility with existing step/batch options
  • Tested scrolling behavior with >10 migrations

Notes

  • This feature is particularly useful during development and testing
  • The interactive prompt only appears when neither step nor batch options are provided
  • Maintains backward compatibility with all existing rollback functionality

@shaedrich
Copy link
Contributor

Usually, migrations should be executed one after another, so that they represent a cumulative state of the database. Rollbacking should work the same way, just in the opposite direction. So, users shouldn't be encouraged to roll back migrations in arbitrary order and not rolling back every migration up until a certain point.

Sure, there might be totally unrelated migrations that would cause no harm, but that is usually the exception and not the norm, so it should be handled as a special case, not the standard one accordingly.

@rez1Pro
Copy link
Author

rez1Pro commented Jan 18, 2025

But I think there should be an option.. what do you thing @shaedrich ?

@rez1Pro rez1Pro closed this Jan 18, 2025
Comment on lines +61 to +62
$this->createBaseMigration($table),
$table
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if unrelated lines should be changed in another PR, which intention is to add a new feature, not to refactor

@@ -104,7 +108,7 @@ protected function replaceMigrationPlaceholders($path, $table)
protected function migrationExists($table)
{
return count($this->files->glob(
join_paths($this->laravel->databasePath('migrations'), '*_*_*_*_create_'.$table.'_table.php')
join_paths($this->laravel->databasePath('migrations'), '*_*_*_*_create_' . $table . '_table.php')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

StyleCI will fail with this one

@@ -230,7 +231,7 @@ public function rollback($paths = [], array $options = [])
// We want to pull in the last batch of migrations that ran on the previous
// migration operation. We'll then reverse those migrations and run each
// of them "down" to reverse the last migration "operation" which ran.
$migrations = $this->getMigrationsForRollback($options);
$migrations = isset($options['selected']) && count($options['selected']) > 0 ? $options['selected'] : $this->getMigrationsForRollback($options);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Function call can be avoided like this:

Suggested change
$migrations = isset($options['selected']) && count($options['selected']) > 0 ? $options['selected'] : $this->getMigrationsForRollback($options);
$migrations = isset($options['selected']) && $options['selected'] !== [] ? $options['selected'] : $this->getMigrationsForRollback($options);

$options = $this->resolveOptions($migrations);
$selected = $options['selected'] ?? [];

if (count($selected) > 0) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here:

Suggested change
if (count($selected) > 0) {
if ($selected !== []) {

->filter()
->values()
->keyBy(fn ($file) => $this->getMigrationName($file))
->sortBy(fn ($file, $key) => $key)
->keyBy(fn($file) => $this->getMigrationName($file))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You might be able to replace it with this:

Suggested change
->keyBy(fn($file) => $this->getMigrationName($file))
->keyBy($this->getMigrationName(...))

Comment on lines +116 to +126
if ($migrationsInstance->count() > 0) {
$options = multiselect(
label: 'Which migrations would you like to rollback (leave blank to default behaviour)',
options: $migrationsInstance->pluck('migration')->toArray(),
hint: 'Use the space bar to select options.',
scroll: 10,
required: true,
);
return $migrationsInstance->whereIn('migration', $options)->get()->toArray();
}
return [];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if you want to swap this:

Suggested change
if ($migrationsInstance->count() > 0) {
$options = multiselect(
label: 'Which migrations would you like to rollback (leave blank to default behaviour)',
options: $migrationsInstance->pluck('migration')->toArray(),
hint: 'Use the space bar to select options.',
scroll: 10,
required: true,
);
return $migrationsInstance->whereIn('migration', $options)->get()->toArray();
}
return [];
if ($migrationsInstance->count() === 0) {
return [];
}
$options = multiselect(
label: 'Which migrations would you like to rollback (leave blank to default behaviour)',
options: $migrationsInstance->pluck('migration')->toArray(),
hint: 'Use the space bar to select options.',
scroll: 10,
required: true,
);
return $migrationsInstance->whereIn('migration', $options)->get()->toArray();

@shaedrich
Copy link
Contributor

But I think there should be an option.. what do you thing @shaedrich ?

I like your general approach. But instead of selecting multiple arbitrary migrations, you could offer to select one migrations and take this as well as every later migration. This would only be circumventable by using an additional parameter (let's call it --force for now).

@rez1Pro rez1Pro reopened this Jan 18, 2025
@rez1Pro
Copy link
Author

rez1Pro commented Jan 18, 2025

I don't know why my editor changed the indentation.. can you suggest to me what I should do about it?

@shaedrich
Copy link
Contributor

I don't know why my editor changed the indentation.. can you suggest to me what I should do about it?

This might have been a setting for auto formatting. Depending on your IDE, you can either turn it off or make it adhere to Laravel's StyleCI configuration.

@taylorotwell
Copy link
Member

Thanks for your pull request to Laravel!

Unfortunately, I'm going to delay merging this code for now. To preserve our ability to adequately maintain the framework, we need to be very careful regarding the amount of code we include.

If applicable, please consider releasing your code as a package so that the community can still take advantage of your contributions!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants