Skip to content

Commit b58bf2e

Browse files
SOHELAHMED7SOHELAHMED7
committed
Resolve TODOs + add tests
1 parent 832c102 commit b58bf2e

File tree

5 files changed

+124
-34
lines changed

5 files changed

+124
-34
lines changed

‎src/lib/migrations/MigrationRecordBuilder.php‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,7 @@ public function addDbColumn(string $tableAlias, ColumnSchema $column, ?string $p
112112
/**
113113
* @throws \yii\base\InvalidConfigException
114114
*/
115-
public function alterColumn(string $tableAlias, ColumnSchema $column, ?string $position = null):string # TODO
115+
public function alterColumn(string $tableAlias, ColumnSchema $column, ?string $position = null):string
116116
{
117117
if (property_exists($column, 'xDbType') && is_string($column->xDbType) && !empty($column->xDbType)) {
118118
$converter = $this->columnToCode($tableAlias, $column, true, false, true, true, $position);
@@ -135,18 +135,18 @@ public function alterColumnType(
135135
ColumnSchema $column,
136136
bool $addUsing = false,
137137
?string $position = null
138-
):string # TODO
138+
):string
139139
{
140140
if (property_exists($column, 'xDbType') && is_string($column->xDbType) && !empty($column->xDbType)) {
141-
$converter = $this->columnToCode($tableAlias, $column, false, false, true, true);
141+
$converter = $this->columnToCode($tableAlias, $column, false, false, true, true, $position);
142142
return sprintf(
143143
ApiGenerator::isPostgres() ? self::ALTER_COLUMN_RAW_PGSQL : self::ALTER_COLUMN_RAW,
144144
$tableAlias,
145145
$column->name,
146146
rtrim(ltrim($converter->getAlterExpression($addUsing), "'"), "'")
147147
);
148148
}
149-
$converter = $this->columnToCode($tableAlias, $column, false);
149+
$converter = $this->columnToCode($tableAlias, $column, false, false, false, false, $position);
150150
return sprintf(self::ALTER_COLUMN, $tableAlias, $column->name, $converter->getAlterExpression($addUsing));
151151
}
152152

@@ -162,15 +162,15 @@ public function alterColumnTypeFromDb(
162162
) :string # TODO
163163
{
164164
if (property_exists($column, 'xDbType') && is_string($column->xDbType) && !empty($column->xDbType)) {
165-
$converter = $this->columnToCode($tableAlias, $column, true, false, true, true);
165+
$converter = $this->columnToCode($tableAlias, $column, true, false, true, true, $position);
166166
return sprintf(
167167
ApiGenerator::isPostgres() ? self::ALTER_COLUMN_RAW_PGSQL : self::ALTER_COLUMN_RAW,
168168
$tableAlias,
169169
$column->name,
170170
rtrim(ltrim($converter->getAlterExpression($addUsing), "'"), "'")
171171
);
172172
}
173-
$converter = $this->columnToCode($tableAlias, $column, true);
173+
$converter = $this->columnToCode($tableAlias, $column, true, false, false, false, $position);
174174
return sprintf(self::ALTER_COLUMN, $tableAlias, $column->name, $converter->getAlterExpression($addUsing));
175175
}
176176

‎src/lib/migrations/MysqlMigrationBuilder.php‎

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -294,18 +294,10 @@ public function setPositions()
294294
if ($column->fromPosition['before'] === $column->toPosition['before']) {
295295
continue;
296296
}
297-
// if (!in_array($column->fromPosition['after'], $onlyColumnNames)) {
298-
// continue;
299-
// }
300-
// if (!in_array($column->fromPosition['before'], $onlyColumnNames)) {
301-
// continue;
302-
// }
303297

304298
if (!in_array($column->fromPosition['after'], $onlyColumnNames) && !in_array($column->fromPosition['before'], $onlyColumnNames)) {
305299
continue;
306300
}
307-
308-
309301
$column->isPositionReallyChanged = true;
310302
$takenIndices[] = [$column->fromPosition['index'], $column->toPosition['index']];
311303
}

‎tests/specs/issue_fix/58_create_migration_for_column_position_change_if_a_field_position_is_changed_in_spec/index.php‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,11 +3,11 @@
33
return [
44
'openApiPath' => '@specs/issue_fix/58_create_migration_for_column_position_change_if_a_field_position_is_changed_in_spec/index.yml',
55
'generateUrls' => false,
6-
'generateModels' => true,
6+
'generateModels' => false,
77
'excludeModels' => [
88
'Error',
99
],
1010
'generateControllers' => false,
1111
'generateMigrations' => true,
12-
'generateModelFaker' => true, // `generateModels` must be `true` in order to use `generateModelFaker` as `true`
12+
'generateModelFaker' => false, // `generateModels` must be `true` in order to use `generateModelFaker` as `true`
1313
];

‎tests/specs/issue_fix/58_create_migration_for_column_position_change_if_a_field_position_is_changed_in_spec/mysql/migrations_mysql_db/m200000_000000_change_table_fruits.php‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
<?php
2+
3+
/**
4+
* Table for Fruit
5+
*/
6+
class m200000_000000_change_table_fruits extends \yii\db\Migration
7+
{
8+
public function up()
9+
{
10+
$this->alterColumn('{{%fruits}}', 'name', $this->text()->null()->after('id'));
11+
}
12+
13+
public function down()
14+
{
15+
$this->alterColumn('{{%fruits}}', 'name', $this->text()->null()->after('description'));
16+
}
17+
}

‎tests/unit/IssueFixTest.php‎

Lines changed: 99 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -369,13 +369,14 @@ public function test58CreateMigrationForColumnPositionChange()
369369

370370
$testFile = Yii::getAlias("@specs/issue_fix/58_create_migration_for_column_position_change_if_a_field_position_is_changed_in_spec/index.php");
371371
$this->runGenerator($testFile);
372-
// $actualFiles = FileHelper::findFiles(Yii::getAlias('@app'), [
373-
// 'recursive' => true,
374-
// ]);
375-
// $expectedFiles = FileHelper::findFiles(Yii::getAlias("@specs/issue_fix/58_create_migration_for_column_position_change_if_a_field_position_is_changed_in_spec/mysql"), [
376-
// 'recursive' => true,
377-
// ]);
378-
// $this->checkFiles($actualFiles, $expectedFiles);
372+
$actualFiles = FileHelper::findFiles(Yii::getAlias('@app'), [
373+
'recursive' => true,
374+
]);
375+
$expectedFiles = FileHelper::findFiles(Yii::getAlias("@specs/issue_fix/58_create_migration_for_column_position_change_if_a_field_position_is_changed_in_spec/mysql"), [
376+
'recursive' => true,
377+
]);
378+
$this->checkFiles($actualFiles, $expectedFiles);
379+
$this->runActualMigrations('mysql', 1);
379380
$this->deleteTableFor58CreateMigrationForColumnPositionChange();
380381
}
381382

@@ -393,19 +394,19 @@ private function deleteTableFor58CreateMigrationForColumnPositionChange()
393394
Yii::$app->db->createCommand('DROP TABLE IF EXISTS {{%fruits}}')->execute();
394395
}
395396

396-
private function for58($schema, $expected)
397+
private function for58($schema, $expected, $columns = [
398+
'id' => 'pk',
399+
'name' => 'text not null',
400+
'description' => 'text not null',
401+
'colour' => 'text not null',
402+
'size' => 'text not null',
403+
])
397404
{
398405
$deleteTable = function () {
399406
Yii::$app->db->createCommand('DROP TABLE IF EXISTS {{%fruits}}')->execute();
400407
};
401-
$createTable = function () {
402-
Yii::$app->db->createCommand()->createTable('{{%fruits}}', [
403-
'id' => 'pk',
404-
'name' => 'text not null',
405-
'description' => 'text not null',
406-
'colour' => 'text not null',
407-
'size' => 'text not null',
408-
])->execute();
408+
$createTable = function () use ($columns) {
409+
Yii::$app->db->createCommand()->createTable('{{%fruits}}', $columns)->execute();
409410
};
410411

411412
$config = [
@@ -426,15 +427,16 @@ private function for58($schema, $expected)
426427

427428
$dbStr = str_replace('db', '', strtolower($db));
428429
$this->runGenerator($tmpConfigFile, $dbStr);
429-
$this->runActualMigrations($dbStr, 1);
430430
$actual = file_get_contents(Yii::getAlias('@app') . '/migrations_' . $dbStr . '_db/m200000_000000_change_table_fruits.php');
431431
$this->assertSame($expected, $actual);
432+
$this->runActualMigrations($dbStr, 1);
432433

433434
$deleteTable();
434435
}
435436
FileHelper::unlink($tmpConfigFile);
436437
}
437438

439+
// ------------ Delete
438440
public function test58DeleteLastCol()
439441
{
440442
$schema = <<<YAML
@@ -707,7 +709,7 @@ public function down()
707709
$this->for58($schema, $expected);
708710
}
709711

710-
// ------------
712+
// ------------ Add
711713
public function test58AddAColAtLastPos()
712714
{
713715
// default position is last so no `AFTER` needed
@@ -1028,4 +1030,83 @@ public function down()
10281030

10291031
$this->for58($schema, $expected);
10301032
}
1033+
1034+
// ------------ Just move columns
1035+
public function test58MoveColumns()
1036+
{
1037+
$columns = [
1038+
'id' => 'pk',
1039+
'name' => 'text null',
1040+
'description' => 'text null',
1041+
'colour' => 'text null',
1042+
'size' => 'text null',
1043+
'col_6' => 'text null',
1044+
'col_7' => 'text null',
1045+
'col_8' => 'text null',
1046+
'col_9' => 'text null',
1047+
1048+
];
1049+
1050+
$schema = <<<YAML
1051+
openapi: 3.0.3
1052+
info:
1053+
title: 'test58MoveColumns'
1054+
version: 1.0.0
1055+
components:
1056+
schemas:
1057+
Fruit:
1058+
type: object
1059+
properties:
1060+
id:
1061+
type: integer
1062+
colour:
1063+
type: string
1064+
size:
1065+
type: string
1066+
name:
1067+
type: string
1068+
description:
1069+
type: string
1070+
col_6:
1071+
type: string
1072+
col_7:
1073+
type: string
1074+
col_8:
1075+
type: string
1076+
col_9:
1077+
type: string
1078+
1079+
paths:
1080+
'/':
1081+
get:
1082+
responses:
1083+
'200':
1084+
description: OK
1085+
YAML;
1086+
1087+
$expected = <<<'PHP'
1088+
<?php
1089+
1090+
/**
1091+
* Table for Fruit
1092+
*/
1093+
class m200000_000000_change_table_fruits extends \yii\db\Migration
1094+
{
1095+
public function up()
1096+
{
1097+
$this->alterColumn('{{%fruits}}', 'colour', $this->text()->notNull()->after('id'));
1098+
$this->alterColumn('{{%fruits}}', 'size', $this->text()->notNull()->after('colour'));
1099+
}
1100+
1101+
public function down()
1102+
{
1103+
$this->alterColumn('{{%fruits}}', 'size', $this->text()->notNull()->after('colour'));
1104+
$this->alterColumn('{{%fruits}}', 'colour', $this->text()->notNull()->after('description'));
1105+
}
1106+
}
1107+
1108+
PHP;
1109+
1110+
$this->for58($schema, $expected, $columns);
1111+
}
10311112
}

0 commit comments

Comments
 (0)