Fix issue: case: drop 2 column: its position in add column in down code in migration

Author: SOHELAHMED7

src/db/ColumnSchema.php

@@ -25,4 +25,11 @@ class ColumnSchema extends \yii\db\ColumnSchema
      * ```
      */
     public $xDbType;
+
+    /**
+     * TODO
+     * Used only for MySQL/MariaDB
+     * @var string|null
+     */
+    public ?string $position = null;
 }

src/lib/migrations/BaseMigrationBuilder.php

@@ -220,6 +220,9 @@ function (string $unknownColumn) {
         } else {
             $this->buildRelations();
         }
+
+//        $this->handleColumnsPositionsChanges($haveNames, $wantNames);
+
         return $this->migration;
     }
 
@@ -249,12 +252,12 @@ protected function buildColumnsDrop(array $columns):void
     {
         foreach ($columns as $column) {
             $tableName = $this->model->getTableAlias();
+            $position = $this->findPosition($column, true);
             if ($column->isPrimaryKey && !$column->autoIncrement) {
                 $pkName = 'pk_' . $this->model->tableName . '_' . $column->name;
                 $this->migration->addDownCode($this->recordBuilder->addPrimaryKey($tableName, [$column->name], $pkName))
                                 ->addUpCode($this->recordBuilder->dropPrimaryKey($tableName, [$column->name], $pkName));
             }
-            $position = $this->findPosition($column, true);
             $this->migration->addDownCode($this->recordBuilder->addDbColumn($tableName, $column, $position))
                             ->addUpCode($this->recordBuilder->dropColumn($tableName, $column->name));
         }
@@ -514,6 +517,8 @@ public function isDefaultValueChanged(
     }
 
     /**
+     * TODO move this method to MysqlMigrationBuilder
+     * Only for MySQL and MariaDB
      * Given a column, compute its previous column name present in OpenAPI schema
      * @return ?string
      * `null` if column is added at last
@@ -528,24 +533,26 @@ public function findPosition(ColumnSchema $column, bool $forDrop = false): ?stri
         if ($key > 0) {
             $prevColName = $columnNames[$key-1];
 
-            if (!isset($columnNames[$key+1])) { // if new col is added at last then no need to add 'AFTER' SQL part. This is checked as if next column is present or not
+            if (!$forDrop && !isset($columnNames[$key+1])) { // if new col is added at last then no need to add 'AFTER' SQL part. This is checked as if next column is present or not
                 return null;
             }
 
-            // in case of `down()` code of migration, putting 'after <colName>' in add column statmenet is erroneous because <colName> may not exist.
+            // in case of `down()` code of migration, putting 'after <colName>' in add column statement is erroneous because <colName> may not exist.
             // Example: From col a, b, c, d, if I drop c and d then their migration code will be generated like:
             // `up()` code
             // drop c
             // drop d
             // `down()` code
-            // add d after c (c does not exist! Error!)
-            // add c
+            // add d after c (c does not exist! Error!) (TODO check if c is present in newColumn)
+            // add c after b (can fix this issue) TODO
             if ($forDrop) {
-                return null;
+//                return null; // TODO this case can be fixed
             }
 
-
-            return self::POS_AFTER . ' ' . $prevColName;
+            if (array_key_exists($prevColName, $this->newColumns)) {
+                return self::POS_AFTER . ' ' . $prevColName;
+            }
+            return null;
 
         // if no `$columnSchema` is found, previous column does not exist. This happens when 'after column' is not yet added in migration or added after currently undertaken column
         } elseif ($key === 0) {
@@ -574,4 +581,20 @@ public function modifyDesiredInContextOfDesiredFromDb(ColumnSchema $desired, Col
         }
         $desired->dbType = $desiredFromDb->dbType;
     }
+
+    // TODO
+    public function handleColumnsPositionsChanges(array $haveNames, array $wantNames)
+    {
+        $indices = [];
+        if ($haveNames !== $wantNames) {
+            foreach ($wantNames as $key => $name) {
+                if ($name !== $haveNames[$key]) {
+                    $indices[] = $key;
+                }
+            }
+        }
+        for ($i = 0; $i < count($indices)/2; $i++) {
+            $this->migration->addUpCode($this->recordBuilder->alterColumn());
+        }
+    }
 }

src/lib/migrations/MigrationRecordBuilder.php

@@ -112,7 +112,7 @@ public function addDbColumn(string $tableAlias, ColumnSchema $column, ?string $p
     /**
      * @throws \yii\base\InvalidConfigException
      */
-    public function alterColumn(string $tableAlias, ColumnSchema $column):string
+    public function alterColumn(string $tableAlias, ColumnSchema $column, ?string $position = null):string # TODO
     {
         if (property_exists($column, 'xDbType') && is_string($column->xDbType) && !empty($column->xDbType)) {
             $converter = $this->columnToCode($tableAlias, $column, true, false, true, true);
@@ -130,7 +130,12 @@ public function alterColumn(string $tableAlias, ColumnSchema $column):string
     /**
      * @throws \yii\base\InvalidConfigException
      */
-    public function alterColumnType(string $tableAlias, ColumnSchema $column, bool $addUsing = false):string
+    public function alterColumnType(
+        string $tableAlias,
+        ColumnSchema $column,
+        bool $addUsing = false,
+        ?string $position = null
+    ):string # TODO
     {
         if (property_exists($column, 'xDbType') && is_string($column->xDbType) && !empty($column->xDbType)) {
             $converter = $this->columnToCode($tableAlias, $column, false, false, true, true);
@@ -149,7 +154,12 @@ public function alterColumnType(string $tableAlias, ColumnSchema $column, bool $
      * This method is only used in Pgsql
      * @throws \yii\base\InvalidConfigException
      */
-    public function alterColumnTypeFromDb(string $tableAlias, ColumnSchema $column, bool $addUsing = false) :string
+    public function alterColumnTypeFromDb(
+        string $tableAlias,
+        ColumnSchema $column,
+        bool $addUsing = false,
+        ?string $position = null
+    ) :string # TODO
     {
         if (property_exists($column, 'xDbType') && is_string($column->xDbType) && !empty($column->xDbType)) {
             $converter = $this->columnToCode($tableAlias, $column, true, false, true, true);

src/lib/migrations/MysqlMigrationBuilder.php

@@ -26,14 +26,15 @@ final class MysqlMigrationBuilder extends BaseMigrationBuilder
     protected function buildColumnChanges(ColumnSchema $current, ColumnSchema $desired, array $changed):void
     {
         $newColumn = clone $current;
+        $position = $this->findPosition($desired);
         foreach ($changed as $attr) {
             $newColumn->$attr = $desired->$attr;
         }
         if (static::isEnum($newColumn)) {
             $newColumn->dbType = 'enum'; // TODO this is concretely not correct
         }
-        $this->migration->addUpCode($this->recordBuilder->alterColumn($this->model->getTableAlias(), $newColumn))
-                        ->addDownCode($this->recordBuilder->alterColumn($this->model->getTableAlias(), $current));
+        $this->migration->addUpCode($this->recordBuilder->alterColumn($this->model->getTableAlias(), $newColumn, $position))
+                        ->addDownCode($this->recordBuilder->alterColumn($this->model->getTableAlias(), $current, $position));
     }
 
     protected function compareColumns(ColumnSchema $current, ColumnSchema $desired):array
@@ -159,4 +160,14 @@ public function modifyDesiredInContextOfCurrent(ColumnSchema $current, ColumnSch
             $desired->size = $current->size;
         }
     }
+
+    /**
+     * TODO
+     * Check if order/position of column is changed
+     * @return void
+     */
+    public function checkOrder()
+    {
+
+    }
 }

tests/specs/issue_fix/58_create_migration_for_column_position_change_if_a_field_position_is_changed_in_spec/index.yml

@@ -11,13 +11,13 @@ components:
       properties:
         id:
           type: integer
-        name:
-          type: string
-          description: desc
-        # colour:
-        #   type: string
-        description:
-          type: string
+#        name: # TODO
+#          type: string
+#          description: desc
+#        colour:
+#          type: string
+#        description:
+#          type: string
 
 paths:
   '/':

tests/unit/IssueFixTest.php

@@ -384,7 +384,7 @@ private function createTableFor58CreateMigrationForColumnPositionChangeIfAFieldP
         Yii::$app->db->createCommand()->createTable('{{%fruits}}', [
             'id' => 'pk',
             'description' => 'text',
-            'name' => 'varchar(255) COMMENT "some-comment"',
+            'name' => 'text',
         ])->execute();
     }