From 17c402fa7f30a75ceea2510f9ddb6d374baaf060 Mon Sep 17 00:00:00 2001 From: mscherer Date: Mon, 3 Nov 2025 01:21:29 +0100 Subject: [PATCH 1/7] Fix up decimal migration_diff --- src/Command/BakeMigrationDiffCommand.php | 31 ++++++++- .../Command/BakeMigrationDiffCommandTest.php | 58 +++++++++++++++- .../initial_decimal_change_mysql.php | 40 +++++++++++ .../mysql/the_diff_decimal_change_mysql.php | 62 ++++++++++++++++++ .../schema-dump-test_comparisons_mysql.lock | Bin 0 -> 1252 bytes 5 files changed, 186 insertions(+), 5 deletions(-) create mode 100644 tests/comparisons/Diff/decimalChange/initial_decimal_change_mysql.php create mode 100644 tests/comparisons/Diff/decimalChange/mysql/the_diff_decimal_change_mysql.php create mode 100644 tests/comparisons/Diff/decimalChange/schema-dump-test_comparisons_mysql.lock diff --git a/src/Command/BakeMigrationDiffCommand.php b/src/Command/BakeMigrationDiffCommand.php index e24d6d3b0..e835dd063 100644 --- a/src/Command/BakeMigrationDiffCommand.php +++ b/src/Command/BakeMigrationDiffCommand.php @@ -300,11 +300,38 @@ protected function getColumns(): void } } - if (isset($changedAttributes['length'])) { + // For decimal columns, handle CakePHP schema -> migration attribute mapping + if ($column['type'] === 'decimal') { + // In CakePHP schema: 'length' = precision, 'precision' = scale + // In migrations: 'precision' = precision, 'scale' = scale + + // Convert CakePHP schema's 'precision' (which is scale) to migration's 'scale' + if (isset($changedAttributes['precision'])) { + $changedAttributes['scale'] = $changedAttributes['precision']; + unset($changedAttributes['precision']); + } + + // Convert CakePHP schema's 'length' (which is precision) to migration's 'precision' + if (isset($changedAttributes['length'])) { + $changedAttributes['precision'] = $changedAttributes['length']; + unset($changedAttributes['length']); + } + + // Ensure both precision and scale are always set for decimal columns + if (!isset($changedAttributes['precision']) && isset($column['length'])) { + $changedAttributes['precision'] = $column['length']; + } + if (!isset($changedAttributes['scale']) && isset($column['precision'])) { + $changedAttributes['scale'] = $column['precision']; + } + + // Remove 'limit' for decimal columns as they use precision/scale instead + unset($changedAttributes['limit']); + } elseif (isset($changedAttributes['length'])) { + // For non-decimal columns, convert 'length' to 'limit' if (!isset($changedAttributes['limit'])) { $changedAttributes['limit'] = $changedAttributes['length']; } - unset($changedAttributes['length']); } diff --git a/tests/TestCase/Command/BakeMigrationDiffCommandTest.php b/tests/TestCase/Command/BakeMigrationDiffCommandTest.php index 070c65ec7..f9bc22d73 100644 --- a/tests/TestCase/Command/BakeMigrationDiffCommandTest.php +++ b/tests/TestCase/Command/BakeMigrationDiffCommandTest.php @@ -21,6 +21,7 @@ use Cake\Datasource\ConnectionManager; use Cake\TestSuite\StringCompareTrait; use Cake\Utility\Inflector; +use Exception; use Migrations\Migrations; use Migrations\Test\TestCase\TestCase; use function Cake\Core\env; @@ -47,6 +48,28 @@ public function setUp(): void parent::setUp(); $this->generatedFiles = []; + + // Clean up any TheDiff migration files from all directories before test starts + $configPath = ROOT . DS . 'config' . DS; + $directories = glob($configPath . '*', GLOB_ONLYDIR) ?: []; + foreach ($directories as $dir) { + $migrationFiles = glob($dir . DS . '*TheDiff*.php') ?: []; + foreach ($migrationFiles as $file) { + if (file_exists($file)) { + unlink($file); + } + } + } + + // Clean up test_decimal_types table if it exists + if (env('DB_URL_COMPARE')) { + try { + $connection = ConnectionManager::get('test_comparisons'); + $connection->execute('DROP TABLE IF EXISTS test_decimal_types'); + } catch (Exception $e) { + // Ignore errors if connection doesn't exist yet + } + } } public function tearDown(): void @@ -57,10 +80,23 @@ public function tearDown(): void unlink($file); } } + + // Clean up any TheDiff migration files from all directories + $configPath = ROOT . DS . 'config' . DS; + $directories = glob($configPath . '*', GLOB_ONLYDIR) ?: []; + foreach ($directories as $dir) { + $migrationFiles = glob($dir . DS . '*TheDiff*.php') ?: []; + foreach ($migrationFiles as $file) { + if (file_exists($file)) { + unlink($file); + } + } + } + if (env('DB_URL_COMPARE')) { // Clean up the comparison database each time. Table order is important. $connection = ConnectionManager::get('test_comparisons'); - $tables = ['articles', 'categories', 'comments', 'users', 'orphan_table', 'phinxlog', 'tags', 'test_blog_phinxlog']; + $tables = ['articles', 'categories', 'comments', 'users', 'orphan_table', 'phinxlog', 'tags', 'test_blog_phinxlog', 'test_decimal_types']; foreach ($tables as $table) { $connection->execute("DROP TABLE IF EXISTS $table"); } @@ -240,6 +276,17 @@ public function testBakingDiffWithAutoIdIncompatibleUnsignedPrimaryKeys(): void $this->runDiffBakingTest('WithAutoIdIncompatibleUnsignedPrimaryKeys'); } + /** + * Tests baking a diff with decimal column changes + * Regression test for issue #659 + */ + public function testBakingDiffDecimalChange(): void + { + $this->skipIf(!env('DB_URL_COMPARE')); + + $this->runDiffBakingTest('DecimalChange'); + } + /** * Tests that baking a diff with --plugin option only includes tables with Table classes */ @@ -331,11 +378,16 @@ protected function runDiffBakingTest(string $scenario): void $this->skipIf(!env('DB_URL_COMPARE')); $diffConfigFolder = Plugin::path('Migrations') . 'tests' . DS . 'comparisons' . DS . 'Diff' . DS . lcfirst($scenario) . DS; - $diffMigrationsPath = $diffConfigFolder . 'the_diff_' . Inflector::underscore($scenario) . '_' . env('DB') . '.php'; + + // DecimalChange uses 'initial_' prefix to avoid class name conflicts + $prefix = $scenario === 'DecimalChange' ? 'initial_' : 'the_diff_'; + $classPrefix = $scenario === 'DecimalChange' ? 'Initial' : 'TheDiff'; + + $diffMigrationsPath = $diffConfigFolder . $prefix . Inflector::underscore($scenario) . '_' . env('DB') . '.php'; $diffDumpPath = $diffConfigFolder . 'schema-dump-test_comparisons_' . env('DB') . '.lock'; $destinationConfigDir = ROOT . DS . 'config' . DS . "MigrationsDiff{$scenario}" . DS; - $destination = $destinationConfigDir . "20160415220805_TheDiff{$scenario}" . ucfirst(env('DB')) . '.php'; + $destination = $destinationConfigDir . "20160415220805_{$classPrefix}{$scenario}" . ucfirst(env('DB')) . '.php'; $destinationDumpPath = $destinationConfigDir . 'schema-dump-test_comparisons_' . env('DB') . '.lock'; copy($diffMigrationsPath, $destination); diff --git a/tests/comparisons/Diff/decimalChange/initial_decimal_change_mysql.php b/tests/comparisons/Diff/decimalChange/initial_decimal_change_mysql.php new file mode 100644 index 000000000..984829c9c --- /dev/null +++ b/tests/comparisons/Diff/decimalChange/initial_decimal_change_mysql.php @@ -0,0 +1,40 @@ +table('test_decimal_types') + ->addColumn('amount', 'decimal', [ + 'default' => null, + 'null' => false, + 'precision' => 5, + 'scale' => 2, + ]) + ->create(); + } + + /** + * Down Method. + * + * More information on this method is available here: + * https://book.cakephp.org/migrations/5/en/migrations.html#the-down-method + * + * @return void + */ + public function down(): void + { + $this->table('test_decimal_types')->drop()->save(); + } +} diff --git a/tests/comparisons/Diff/decimalChange/mysql/the_diff_decimal_change_mysql.php b/tests/comparisons/Diff/decimalChange/mysql/the_diff_decimal_change_mysql.php new file mode 100644 index 000000000..168a568da --- /dev/null +++ b/tests/comparisons/Diff/decimalChange/mysql/the_diff_decimal_change_mysql.php @@ -0,0 +1,62 @@ +table('test_decimal_types') + ->changeColumn('id', 'integer', [ + 'default' => null, + 'length' => null, + 'limit' => null, + 'null' => false, + 'signed' => false, + ]) + ->changeColumn('amount', 'decimal', [ + 'default' => null, + 'null' => false, + 'precision' => 5, + 'scale' => 5, + ]) + ->update(); + } + + /** + * Down Method. + * + * More information on this method is available here: + * https://book.cakephp.org/migrations/5/en/migrations.html#the-down-method + * + * @return void + */ + public function down(): void + { + + $this->table('test_decimal_types') + ->changeColumn('id', 'integer', [ + 'autoIncrement' => true, + 'default' => null, + 'length' => 11, + 'null' => false, + ]) + ->changeColumn('amount', 'decimal', [ + 'default' => null, + 'null' => false, + 'precision' => 10, + 'scale' => 2, + ]) + ->update(); + } +} diff --git a/tests/comparisons/Diff/decimalChange/schema-dump-test_comparisons_mysql.lock b/tests/comparisons/Diff/decimalChange/schema-dump-test_comparisons_mysql.lock new file mode 100644 index 0000000000000000000000000000000000000000..9c497e7b29c23bb40bbb619249db999f94b0bd89 GIT binary patch literal 1252 zcmcIk!A`?44CPyL#H@V8B+PrpnQ_0)S@D%UJn5|e6Ok_|fSr9Ot4mm#-g z$zdSYF9=zRHjW$Wb!|UIwJjV~43nbVd1Ogf9Y`l#$Awbtvc z73xmXDb<+1JoU;uY4CtUM!JlhTI5F$Q&3pdtth*23Il|1CeiKJA0`n;_H`05{L4vb z7=KLY|7H=peBmT`6ep9&9zS4nZj$rGKJRe3sCB2ES5k_#3gPXW|>!P+kIAov1 z!}uof!ZeN_1IL@r?*O;_+jn5zmnxWiHRTY6`TBO8e9u%m+OTski%Oe1ux~HpIJisQ W1h&=iE|YYZKvh$_C|eAEJiY^=;BBY? literal 0 HcmV?d00001 From 4555456d3ed680f1f94b45f70e61f90a8ffa6326 Mon Sep 17 00:00:00 2001 From: mscherer Date: Tue, 4 Nov 2025 10:24:14 +0100 Subject: [PATCH 2/7] Add .gitkeep for MigrationsDiffDecimalChange directory --- tests/test_app/config/MigrationsDiffDecimalChange/.gitkeep | 0 1 file changed, 0 insertions(+), 0 deletions(-) create mode 100644 tests/test_app/config/MigrationsDiffDecimalChange/.gitkeep diff --git a/tests/test_app/config/MigrationsDiffDecimalChange/.gitkeep b/tests/test_app/config/MigrationsDiffDecimalChange/.gitkeep new file mode 100644 index 000000000..e69de29bb From 0dc4faded2618337777176c536a5bdccc8db97b3 Mon Sep 17 00:00:00 2001 From: Mark Scherer Date: Tue, 4 Nov 2025 18:07:51 +0100 Subject: [PATCH 3/7] Update tests/TestCase/Command/BakeMigrationDiffCommandTest.php Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- tests/TestCase/Command/BakeMigrationDiffCommandTest.php | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/tests/TestCase/Command/BakeMigrationDiffCommandTest.php b/tests/TestCase/Command/BakeMigrationDiffCommandTest.php index f9bc22d73..e1b6b81cb 100644 --- a/tests/TestCase/Command/BakeMigrationDiffCommandTest.php +++ b/tests/TestCase/Command/BakeMigrationDiffCommandTest.php @@ -53,12 +53,20 @@ public function setUp(): void $configPath = ROOT . DS . 'config' . DS; $directories = glob($configPath . '*', GLOB_ONLYDIR) ?: []; foreach ($directories as $dir) { + // Clean up TheDiff migration files $migrationFiles = glob($dir . DS . '*TheDiff*.php') ?: []; foreach ($migrationFiles as $file) { if (file_exists($file)) { unlink($file); } } + // Clean up Initial migration files + $initialMigrationFiles = glob($dir . DS . '*Initial*.php') ?: []; + foreach ($initialMigrationFiles as $file) { + if (file_exists($file)) { + unlink($file); + } + } } // Clean up test_decimal_types table if it exists From 22061a32118bb544b814299018533b118d206393 Mon Sep 17 00:00:00 2001 From: Mark Scherer Date: Wed, 5 Nov 2025 07:17:43 +0100 Subject: [PATCH 4/7] Update tests/TestCase/Command/BakeMigrationDiffCommandTest.php Co-authored-by: Mark Story --- tests/TestCase/Command/BakeMigrationDiffCommandTest.php | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tests/TestCase/Command/BakeMigrationDiffCommandTest.php b/tests/TestCase/Command/BakeMigrationDiffCommandTest.php index e1b6b81cb..b1eb10c19 100644 --- a/tests/TestCase/Command/BakeMigrationDiffCommandTest.php +++ b/tests/TestCase/Command/BakeMigrationDiffCommandTest.php @@ -99,6 +99,12 @@ public function tearDown(): void unlink($file); } } + $initialMigrationFiles = glob($dir . DS . '*Initial*.php') ?: []; + foreach ($initialMigrationFiles as $file) { + if (file_exists($file)) { + unlink($file); + } + } } if (env('DB_URL_COMPARE')) { From 9c943ffbf74d94dc4edd9f1d400b70b5c8b3be88 Mon Sep 17 00:00:00 2001 From: mscherer Date: Tue, 11 Nov 2025 23:06:16 +0100 Subject: [PATCH 5/7] Fixes. --- src/View/Helper/MigrationHelper.php | 6 +++++- .../decimalChange/mysql/the_diff_decimal_change_mysql.php | 2 +- tests/test_app/config/MigrationsDiffDecimalChange/.gitkeep | 0 3 files changed, 6 insertions(+), 2 deletions(-) delete mode 100644 tests/test_app/config/MigrationsDiffDecimalChange/.gitkeep diff --git a/src/View/Helper/MigrationHelper.php b/src/View/Helper/MigrationHelper.php index 42b3dbfac..f6dd991c7 100644 --- a/src/View/Helper/MigrationHelper.php +++ b/src/View/Helper/MigrationHelper.php @@ -387,6 +387,7 @@ public function getColumnOption(array $options): array 'comment', 'autoIncrement', 'precision', + 'scale', 'after', 'collate', ]); @@ -420,7 +421,10 @@ public function getColumnOption(array $options): array unset($columnOptions['precision']); } else { // due to Phinx using different naming for the precision and scale to CakePHP - $columnOptions['scale'] = $columnOptions['precision']; + // Only convert precision to scale if scale is not already set (for decimal columns from diff) + if (!isset($columnOptions['scale'])) { + $columnOptions['scale'] = $columnOptions['precision']; + } if (isset($columnOptions['limit'])) { $columnOptions['precision'] = $columnOptions['limit']; diff --git a/tests/comparisons/Diff/decimalChange/mysql/the_diff_decimal_change_mysql.php b/tests/comparisons/Diff/decimalChange/mysql/the_diff_decimal_change_mysql.php index 168a568da..a64755d93 100644 --- a/tests/comparisons/Diff/decimalChange/mysql/the_diff_decimal_change_mysql.php +++ b/tests/comparisons/Diff/decimalChange/mysql/the_diff_decimal_change_mysql.php @@ -28,7 +28,7 @@ public function up(): void 'default' => null, 'null' => false, 'precision' => 5, - 'scale' => 5, + 'scale' => 2, ]) ->update(); } diff --git a/tests/test_app/config/MigrationsDiffDecimalChange/.gitkeep b/tests/test_app/config/MigrationsDiffDecimalChange/.gitkeep deleted file mode 100644 index e69de29bb..000000000 From 59e9a69a4b4a91f1ea9e0cfa82d279badb0c8925 Mon Sep 17 00:00:00 2001 From: mscherer Date: Tue, 11 Nov 2025 23:20:41 +0100 Subject: [PATCH 6/7] Fixes. --- .../Command/BakeMigrationDiffCommandTest.php | 46 +++++++++++++++++-- 1 file changed, 41 insertions(+), 5 deletions(-) diff --git a/tests/TestCase/Command/BakeMigrationDiffCommandTest.php b/tests/TestCase/Command/BakeMigrationDiffCommandTest.php index b1eb10c19..576d0756a 100644 --- a/tests/TestCase/Command/BakeMigrationDiffCommandTest.php +++ b/tests/TestCase/Command/BakeMigrationDiffCommandTest.php @@ -18,6 +18,9 @@ use Cake\Core\Configure; use Cake\Core\Plugin; use Cake\Database\Driver\Mysql; +use Cake\Database\Driver\Postgres; +use Cake\Database\Driver\Sqlite; +use Cake\Database\Driver\Sqlserver; use Cake\Datasource\ConnectionManager; use Cake\TestSuite\StringCompareTrait; use Cake\Utility\Inflector; @@ -391,18 +394,21 @@ protected function runDiffBakingTest(string $scenario): void { $this->skipIf(!env('DB_URL_COMPARE')); + // Detect database type from connection if DB env is not set + $db = env('DB') ?: $this->getDbType(); + $diffConfigFolder = Plugin::path('Migrations') . 'tests' . DS . 'comparisons' . DS . 'Diff' . DS . lcfirst($scenario) . DS; // DecimalChange uses 'initial_' prefix to avoid class name conflicts $prefix = $scenario === 'DecimalChange' ? 'initial_' : 'the_diff_'; $classPrefix = $scenario === 'DecimalChange' ? 'Initial' : 'TheDiff'; - $diffMigrationsPath = $diffConfigFolder . $prefix . Inflector::underscore($scenario) . '_' . env('DB') . '.php'; - $diffDumpPath = $diffConfigFolder . 'schema-dump-test_comparisons_' . env('DB') . '.lock'; + $diffMigrationsPath = $diffConfigFolder . $prefix . Inflector::underscore($scenario) . '_' . $db . '.php'; + $diffDumpPath = $diffConfigFolder . 'schema-dump-test_comparisons_' . $db . '.lock'; $destinationConfigDir = ROOT . DS . 'config' . DS . "MigrationsDiff{$scenario}" . DS; - $destination = $destinationConfigDir . "20160415220805_{$classPrefix}{$scenario}" . ucfirst(env('DB')) . '.php'; - $destinationDumpPath = $destinationConfigDir . 'schema-dump-test_comparisons_' . env('DB') . '.lock'; + $destination = $destinationConfigDir . "20160415220805_{$classPrefix}{$scenario}" . ucfirst($db) . '.php'; + $destinationDumpPath = $destinationConfigDir . 'schema-dump-test_comparisons_' . $db . '.lock'; copy($diffMigrationsPath, $destination); $this->generatedFiles = [ @@ -453,6 +459,29 @@ protected function runDiffBakingTest(string $scenario): void $this->getMigrations("MigrationsDiff{$scenario}")->rollback(['target' => 'all']); } + /** + * Detect database type from connection + * + * @return string Database type (mysql, pgsql, sqlite, sqlserver) + */ + protected function getDbType(): string + { + $connection = ConnectionManager::get('test_comparisons'); + $driver = $connection->getDriver(); + + if ($driver instanceof Mysql) { + return 'mysql'; + } elseif ($driver instanceof Postgres) { + return 'pgsql'; + } elseif ($driver instanceof Sqlite) { + return 'sqlite'; + } elseif ($driver instanceof Sqlserver) { + return 'sqlserver'; + } + + return 'mysql'; // Default fallback + } + /** * Get the baked filename based on the current db environment * @@ -461,7 +490,11 @@ protected function runDiffBakingTest(string $scenario): void */ public function getBakeName($name) { - $name .= ucfirst(getenv('DB')); + $db = getenv('DB'); + if (!$db) { + $db = $this->getDbType(); + } + $name .= ucfirst($db); return $name; } @@ -494,6 +527,9 @@ protected function getMigrations($source = 'MigrationsDiff') public function assertCorrectSnapshot($bakeName, $result) { $dbenv = getenv('DB'); + if (!$dbenv) { + $dbenv = $this->getDbType(); + } $bakeName = Inflector::underscore($bakeName); if (file_exists($this->_compareBasePath . $dbenv . DS . $bakeName . '.php')) { $this->assertSameAsFile($dbenv . DS . $bakeName . '.php', $result); From a2e524185c2403a453ee13e3cf0590c1c116d7b3 Mon Sep 17 00:00:00 2001 From: mscherer Date: Wed, 12 Nov 2025 00:16:15 +0100 Subject: [PATCH 7/7] Fixes. --- tests/test_app/config/MigrationsDiffDecimalChange/.gitkeep | 0 1 file changed, 0 insertions(+), 0 deletions(-) create mode 100644 tests/test_app/config/MigrationsDiffDecimalChange/.gitkeep diff --git a/tests/test_app/config/MigrationsDiffDecimalChange/.gitkeep b/tests/test_app/config/MigrationsDiffDecimalChange/.gitkeep new file mode 100644 index 000000000..e69de29bb