Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 29 additions & 2 deletions src/Command/BakeMigrationDiffCommand.php
Original file line number Diff line number Diff line change
Expand Up @@ -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']);
}

Expand Down
6 changes: 5 additions & 1 deletion src/View/Helper/MigrationHelper.php
Original file line number Diff line number Diff line change
Expand Up @@ -387,6 +387,7 @@ public function getColumnOption(array $options): array
'comment',
'autoIncrement',
'precision',
'scale',
'after',
'collate',
]);
Expand Down Expand Up @@ -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'];
Expand Down
114 changes: 108 additions & 6 deletions tests/TestCase/Command/BakeMigrationDiffCommandTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,13 @@
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;
use Exception;
use Migrations\Migrations;
use Migrations\Test\TestCase\TestCase;
use function Cake\Core\env;
Expand All @@ -47,6 +51,36 @@ 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) {
// 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
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
Expand All @@ -57,10 +91,29 @@ 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);
}
}
$initialMigrationFiles = glob($dir . DS . '*Initial*.php') ?: [];
foreach ($initialMigrationFiles 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");
}
Expand Down Expand Up @@ -240,6 +293,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
*/
Expand Down Expand Up @@ -330,13 +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;
$diffMigrationsPath = $diffConfigFolder . 'the_diff_' . Inflector::underscore($scenario) . '_' . env('DB') . '.php';
$diffDumpPath = $diffConfigFolder . 'schema-dump-test_comparisons_' . env('DB') . '.lock';

// 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) . '_' . $db . '.php';
$diffDumpPath = $diffConfigFolder . 'schema-dump-test_comparisons_' . $db . '.lock';

$destinationConfigDir = ROOT . DS . 'config' . DS . "MigrationsDiff{$scenario}" . DS;
$destination = $destinationConfigDir . "20160415220805_TheDiff{$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 = [
Expand Down Expand Up @@ -387,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
*
Expand All @@ -395,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;
}
Expand Down Expand Up @@ -428,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);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
<?php
declare(strict_types=1);

use Migrations\BaseMigration;

class InitialDecimalChangeMysql extends BaseMigration
{
/**
* Up Method.
*
* More information on this method is available here:
* https://book.cakephp.org/migrations/5/en/migrations.html#the-up-method
*
* @return void
*/
public function up(): void
{
$this->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();
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
<?php
declare(strict_types=1);

use Migrations\BaseMigration;

class TheDiffDecimalChangeMysql extends BaseMigration
{
/**
* Up Method.
*
* More information on this method is available here:
* https://book.cakephp.org/migrations/5/en/migrations.html#the-up-method
*
* @return void
*/
public function up(): void
{

$this->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' => 2,
])
->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();
}
}
Binary file not shown.
Empty file.
Loading