Skip to content

Commit 9ef528c

Browse files
committed
Fix bugs + fix failing tests + add more tests + refactor
1 parent 3dcc87e commit 9ef528c

File tree

3 files changed

+187
-23
lines changed

3 files changed

+187
-23
lines changed

src/lib/migrations/BaseMigrationBuilder.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -185,7 +185,7 @@ function (string $missingColumn) {
185185
function (string $unknownColumn) {
186186
return $this->tableSchema->columns[$unknownColumn];
187187
},
188-
array_diff($haveNames, $wantNames)
188+
array_reverse(array_diff($haveNames, $wantNames), true)
189189
);
190190

191191
$columnsForChange = array_intersect($wantNames, $haveNames);

src/lib/migrations/MysqlMigrationBuilder.php

Lines changed: 53 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -197,6 +197,42 @@ public function checkOrder()
197197
*/
198198
public function findPosition(ColumnSchema $column, bool $forDrop = false): ?string
199199
{
200+
// if (!$forDrop) {
201+
// $columnNames = array_keys($this->newColumns);
202+
// $key = array_search($column->name, $columnNames);
203+
// if (is_int($key)) {
204+
// if ($key > 0) {
205+
// $prevColName = $columnNames[$key - 1];
206+
//
207+
// // if the perv col is last then no need for `'AFTER <column-name>'` because last is the default position
208+
// if (array_search($prevColName, $columnNames) === (count($columnNames) - 1)) {
209+
// return null;
210+
// }
211+
//
212+
// return self::POS_AFTER . ' ' . $prevColName;
213+
// } elseif ($key === 0) {
214+
// return self::POS_FIRST;
215+
// }
216+
// }
217+
//
218+
// } else {
219+
// $columnNames = array_keys($this->tableSchema->columns);
220+
// $key = array_search($column->name, $columnNames);
221+
// if (is_int($key)) {
222+
// if ($key > 0) {
223+
// $prevColName = $columnNames[$key - 1];
224+
//
225+
// if (array_search($prevColName, $columnNames) === count($columnNames) - 1) {
226+
// return null;
227+
// }
228+
//
229+
// return self::POS_AFTER . ' ' . $prevColName;
230+
// } elseif ($key === 0) {
231+
// return self::POS_FIRST;
232+
// }
233+
// }
234+
// }
235+
// return null;
200236
$columnNames = array_keys($forDrop ? $this->tableSchema->columns : $this->newColumns);
201237

202238
$key = array_search($column->name, $columnNames);
@@ -250,51 +286,47 @@ public function setPositions()
250286
{
251287
$i = 0;
252288
$haveColumns = $this->tableSchema->columns;
253-
$onlyColumnNames = array_keys($this->newColumns);
254-
$haveNamesOnlyColNames = array_keys($haveColumns);
255-
foreach ($this->newColumns as $columnName => $column) {
289+
$wantNames = array_keys($this->newColumns);
290+
$haveNames = array_keys($haveColumns);
291+
foreach ($this->newColumns as $name => $column) {
256292
/** @var \cebe\yii2openapi\db\ColumnSchema $column */
257293
$column->toPosition = [
258294
'index' => $i + 1,
259-
'after' => $i === 0 ? null : $onlyColumnNames[$i - 1],
260-
'before' => $i === (count($onlyColumnNames) - 1) ? null : $onlyColumnNames[$i + 1],
295+
'after' => $i === 0 ? null : $wantNames[$i - 1],
296+
'before' => $i === (count($wantNames) - 1) ? null : $wantNames[$i + 1],
261297
];
262298

263-
if (isset($haveColumns[$columnName])) {
264-
$index = array_search($columnName, $haveNamesOnlyColNames) + 1;
299+
if (isset($haveColumns[$name])) {
300+
$index = array_search($name, $haveNames) + 1;
265301
$column->fromPosition = [
266302
'index' => $index,
267-
'after' => $haveNamesOnlyColNames[$index - 2] ?? null,
268-
'before' => $haveNamesOnlyColNames[$index] ?? null,
303+
'after' => $haveNames[$index - 2] ?? null,
304+
'before' => $haveNames[$index] ?? null,
269305
];
270306
}
271307

272308
$i++;
273309
}
274310

275311
$takenIndices = [];
276-
foreach ($this->newColumns as $columnName => $column) {
312+
foreach ($this->newColumns as $column) {
277313
/** @var \cebe\yii2openapi\db\ColumnSchema $column */
278314

279315
if (!$column->fromPosition || !$column->toPosition) {
280316
continue;
281317
}
282318

283-
if (count($onlyColumnNames) !== count($haveNamesOnlyColNames)) {
319+
if (count($wantNames) !== count($haveNames)) {
284320
// check if only new columns are added without any explicit position change
285-
$columnsForCreate = array_diff($onlyColumnNames, $haveNamesOnlyColNames);
286-
if ($columnsForCreate) {
287-
if ($haveNamesOnlyColNames === array_values(array_diff($onlyColumnNames, $columnsForCreate))) {
288-
continue;
289-
}
321+
$namesForCreate = array_diff($wantNames, $haveNames);
322+
if ($namesForCreate && $haveNames === array_values(array_diff($wantNames, $namesForCreate))) {
323+
continue;
290324
}
291325

292326
// check if only existing columns are deleted without any explicit position change
293-
$columnsForDrop = array_diff($haveNamesOnlyColNames, $onlyColumnNames);
294-
if ($columnsForDrop) {
295-
if ($onlyColumnNames === array_values(array_diff($haveNamesOnlyColNames, $columnsForDrop))) {
296-
continue;
297-
}
327+
$namesForDrop = array_diff($haveNames, $wantNames);
328+
if ($namesForDrop && $wantNames === array_values(array_diff($haveNames, $namesForDrop))) {
329+
continue;
298330
}
299331
}
300332

tests/unit/IssueFixTest.php

Lines changed: 133 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -709,6 +709,65 @@ public function down()
709709
$this->for58($schema, $expected);
710710
}
711711

712+
public function test58DeleteLast4Col()
713+
{
714+
$columns = [
715+
'id' => 'pk',
716+
'name' => 'text null',
717+
'description' => 'text null',
718+
'colour' => 'text null',
719+
'size' => 'text null',
720+
'col_6' => 'text null',
721+
];
722+
723+
$schema = <<<YAML
724+
openapi: 3.0.3
725+
info:
726+
title: 'test58MoveColumns'
727+
version: 1.0.0
728+
components:
729+
schemas:
730+
Fruit:
731+
type: object
732+
properties:
733+
id:
734+
type: integer
735+
name:
736+
type: string
737+
paths:
738+
'/':
739+
get:
740+
responses:
741+
'200':
742+
description: OK
743+
YAML;
744+
745+
$expected = <<<'PHP'
746+
<?php
747+
748+
/**
749+
* Table for Fruit
750+
*/
751+
class m200000_000000_change_table_fruits extends \yii\db\Migration
752+
{
753+
public function up()
754+
{
755+
$this->alterColumn('{{%fruits}}', 'colour', $this->text()->null()->after('id'));
756+
$this->alterColumn('{{%fruits}}', 'size', $this->text()->null()->after('colour'));
757+
}
758+
759+
public function down()
760+
{
761+
$this->alterColumn('{{%fruits}}', 'size', $this->text()->null()->after('colour'));
762+
$this->alterColumn('{{%fruits}}', 'colour', $this->text()->null()->after('description'));
763+
}
764+
}
765+
766+
PHP;
767+
768+
$this->for58($schema, $expected, $columns, ['Mysql', 'Mariadb']);
769+
}
770+
712771
// ------------ Add
713772
public function test58AddAColAtLastPos()
714773
{
@@ -1032,7 +1091,7 @@ public function down()
10321091
}
10331092

10341093
// ------------ Just move columns
1035-
public function test58MoveColumns()
1094+
public function test58MoveLast2Col2PosUp()
10361095
{
10371096
$columns = [
10381097
'id' => 'pk',
@@ -1100,4 +1159,77 @@ public function down()
11001159

11011160
$this->for58($schema, $expected, $columns, ['Mysql']);
11021161
}
1162+
1163+
// -----------
1164+
public function test58Move1Add1Del1Col()
1165+
{
1166+
$columns = [
1167+
'id' => 'pk',
1168+
'name' => 'text null',
1169+
'description' => 'text null',
1170+
'colour' => 'text null',
1171+
'size' => 'text null',
1172+
// 'col_6' => 'text null',
1173+
// 'col_7' => 'text null',
1174+
// 'col_8' => 'text null',
1175+
// 'col_9' => 'text null',
1176+
1177+
];
1178+
1179+
$schema = <<<YAML
1180+
openapi: 3.0.3
1181+
info:
1182+
title: 'test58MoveColumns'
1183+
version: 1.0.0
1184+
components:
1185+
schemas:
1186+
Fruit:
1187+
type: object
1188+
properties:
1189+
id:
1190+
type: integer
1191+
colour:
1192+
type: string
1193+
name:
1194+
type: string
1195+
description:
1196+
type: string
1197+
col_6:
1198+
type: string
1199+
paths:
1200+
'/':
1201+
get:
1202+
responses:
1203+
'200':
1204+
description: OK
1205+
YAML;
1206+
1207+
$expected = <<<'PHP'
1208+
<?php
1209+
1210+
/**
1211+
* Table for Fruit
1212+
*/
1213+
class m200000_000000_change_table_fruits extends \yii\db\Migration
1214+
{
1215+
public function up()
1216+
{
1217+
$this->alterColumn('{{%fruits}}', 'colour', $this->text()->null()->after('id'));
1218+
$this->alterColumn('{{%fruits}}', 'size', $this->text()->null()->after('colour'));
1219+
}
1220+
1221+
public function down()
1222+
{
1223+
$this->alterColumn('{{%fruits}}', 'size', $this->text()->null()->after('colour'));
1224+
$this->alterColumn('{{%fruits}}', 'colour', $this->text()->null()->after('description'));
1225+
}
1226+
}
1227+
1228+
PHP;
1229+
1230+
$this->for58($schema, $expected, $columns, ['Mysql']);
1231+
}
1232+
1233+
// add 1 and del 1 col at same position
1234+
// add 1 and del 1 col at different position
11031235
}

0 commit comments

Comments
 (0)