migrations
migrations copied to clipboard
Incorrect behavior when using "migration_diff" with "--plugin" option.
When using migration_diff with --plugin option is creates migrations for all tables in the db instead of just the tables for which Table classes are present in the plugin. It would except it to only consider the tables of the plugin similar to how migration_snapshot does.
The main problem is that without some table prefixing for plugins it will be rather difficult to detect "new tables" vs "existing but not plugin related". There will either be too many or too less shown then..
Can't migration_diff simply ignore tables for which it doesn't find a class file inside the plugin?
For normal purposes I agree. But what if you want to add this new SQL table into your plugin? Then you would want this one to be not excluded. I agree, however, that this case is more the edge case then yours.
I remember doing that in the first version of the command : only including tables from the db which have a Table file in the plugin.
But for some reason, they were comments that asked me to remove this. I don't remember why exactly though.
Can't see why people wouldn't want that behavior since that's how snapshot works too. Anyway the behavior could have been made configurable.
It still can.
One would need to modify the MigrationDiffTask::getCurrentSchema() method to only filter the tables with a Table class existing in the plugin directory, if there is a specific parameter (some other methods have a require-table bool flag that enforce the need for Table classes to exist), using the TableFinderTrait for instance (which might need a bit of refactor to avoid code duplication ?).
Just to add some information, I tested including use TableFinderTrait; in MigrationDiffTask.php
If I add a few testing lines to the templateData() similar to what's in MigrationSnapshot.php:
https://github.com/cakephp/migrations/blob/e5a92d961acb3371f6b7649cf9017647d2cb6d9a/src/Shell/Task/MigrationDiffTask.php#L171
/**
* Process and prepare the data needed for the bake template to be generated.
*
* @return array
*/
public function templateData()
{
$connection = ConnectionManager::get($this->connection);
$options = [
'require-table' => true,
// Left out of this GitHub post for simplicity
//'require-table' => $this->params['require-table'],
'plugin' => $this->plugin
];
$tables = $this->getTablesToBake($connection->getSchemaCollection(), $options);
dd($tables);
$this->dumpSchema = $this->getDumpSchema();
$this->currentSchema = $this->getCurrentSchema();
$this->commonTables = array_intersect_key($this->currentSchema, $this->dumpSchema);
$this->calculateDiff();
return [
'data' => $this->templateData,
'dumpSchema' => $this->dumpSchema,
'currentSchema' => $this->currentSchema,
];
}
Then run bin/cake bake migration_diff --plugin=PluginName TestAddColumn
It does indeed dump an array of just the table names contained within the plugin
PR welcome then.
@ADmad Are you still interested in following up on this?
My only follow up for now is I would like to get it addressed/fixed :)