Skip to content

Commit 7a7f9a9

Browse files
authored
Fix phinx backend using incorrect RollbackCommand class (#991)
When using the phinx backend with Bake installed, discoverPlugin() was finding both builtin and phinx commands. Since both RollbackCommand and MigrationsRollbackCommand have the same defaultName(), the builtin command was overwriting the phinx one due to alphabetical ordering. This fix explicitly registers only the phinx-specific commands instead of using discoverPlugin(), matching the approach used for builtin backend. Fixes #990
1 parent f9d7d96 commit 7a7f9a9

2 files changed

Lines changed: 59 additions & 5 deletions

File tree

src/MigrationsPlugin.php

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -131,15 +131,16 @@ public function console(CommandCollection $commands): CommandCollection
131131
return $commands;
132132
}
133133

134+
$classes = $this->migrationCommandsList;
134135
if (class_exists(SimpleBakeCommand::class)) {
135-
$found = $commands->discoverPlugin($this->getName());
136-
137-
return $commands->addMany($found);
136+
$classes[] = BakeMigrationCommand::class;
137+
$classes[] = BakeMigrationDiffCommand::class;
138+
$classes[] = BakeMigrationSnapshotCommand::class;
139+
$classes[] = BakeSeedCommand::class;
138140
}
139141

140142
$found = [];
141-
// Convert to a method and use config to toggle command names.
142-
foreach ($this->migrationCommandsList as $class) {
143+
foreach ($classes as $class) {
143144
$name = $class::defaultName();
144145
// If the short name has been used, use the full name.
145146
// This allows app commands to have name preference.
Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
<?php
2+
declare(strict_types=1);
3+
4+
namespace Migrations\Test\TestCase;
5+
6+
use Cake\Console\CommandCollection;
7+
use Cake\Core\Configure;
8+
use Cake\TestSuite\TestCase;
9+
use Migrations\Command\MigrationsRollbackCommand;
10+
use Migrations\Command\RollbackCommand;
11+
use Migrations\MigrationsPlugin;
12+
13+
class MigrationsPluginTest extends TestCase
14+
{
15+
protected function tearDown(): void
16+
{
17+
parent::tearDown();
18+
Configure::delete('Migrations.backend');
19+
}
20+
21+
/**
22+
* Test that builtin backend uses RollbackCommand
23+
*/
24+
public function testConsoleBuiltinBackendUsesCorrectRollbackCommand(): void
25+
{
26+
Configure::write('Migrations.backend', 'builtin');
27+
28+
$plugin = new MigrationsPlugin();
29+
$commands = new CommandCollection();
30+
$commands = $plugin->console($commands);
31+
32+
$this->assertTrue($commands->has('migrations rollback'));
33+
$this->assertSame(RollbackCommand::class, $commands->get('migrations rollback'));
34+
}
35+
36+
/**
37+
* Test that phinx backend uses MigrationsRollbackCommand
38+
*
39+
* This is the reported bug in https://github.com/cakephp/migrations/issues/990
40+
*/
41+
public function testConsolePhinxBackendUsesCorrectRollbackCommand(): void
42+
{
43+
Configure::write('Migrations.backend', 'phinx');
44+
45+
$plugin = new MigrationsPlugin();
46+
$commands = new CommandCollection();
47+
$commands = $plugin->console($commands);
48+
49+
$this->assertTrue($commands->has('migrations rollback'));
50+
// Bug: RollbackCommand is loaded instead of MigrationsRollbackCommand
51+
$this->assertSame(MigrationsRollbackCommand::class, $commands->get('migrations rollback'));
52+
}
53+
}

0 commit comments

Comments
 (0)