From: eileen <emcnaughton@wikimedia.org>
Date: Thu, 27 Jun 2019 06:22:49 +0000 (+1200)
Subject: [REF + test] extract buildFieldChangeSql and add unit test
X-Git-Url: https://vcs.fsf.org/?a=commitdiff_plain;h=d992b2f580614ad554119331461ad45208448b61;p=civicrm-core.git

[REF + test] extract buildFieldChangeSql and add unit test
---

diff --git a/CRM/Core/BAO/SchemaHandler.php b/CRM/Core/BAO/SchemaHandler.php
index 7458e6b0b1..936b27c0aa 100644
--- a/CRM/Core/BAO/SchemaHandler.php
+++ b/CRM/Core/BAO/SchemaHandler.php
@@ -296,41 +296,11 @@ ALTER TABLE {$tableName}
    * @return bool
    */
   public static function alterFieldSQL(&$params, $indexExist = FALSE, $triggerRebuild = TRUE) {
-    $sql = str_repeat(' ', 8);
-    $sql .= "ALTER TABLE {$params['table_name']}";
 
     // lets suppress the required flag, since that can cause sql issue
     $params['required'] = FALSE;
 
-    switch ($params['operation']) {
-      case 'add':
-        $separator = "\n";
-        $prefix = "ADD ";
-        $sql .= self::buildFieldSQL($params, $separator, "ADD COLUMN ");
-        $separator = ",\n";
-        $sql .= self::buildPrimaryKeySQL($params, $separator, "ADD PRIMARY KEY ");
-        $sql .= self::buildSearchIndexSQL($params, $separator, "ADD INDEX ");
-        $sql .= self::buildForeignKeySQL($params, $separator, "ADD ", $params['table_name']);
-        break;
-
-      case 'modify':
-        $separator = "\n";
-        $prefix = "MODIFY ";
-        $sql .= self::buildFieldSQL($params, $separator, $prefix);
-        $separator = ",\n";
-        $sql .= self::buildSearchIndexSQL($params, $separator, "ADD INDEX ", $indexExist);
-        break;
-
-      case 'delete':
-        $sql .= " DROP COLUMN `{$params['name']}`";
-        if (!empty($params['primary'])) {
-          $sql .= ", DROP PRIMARY KEY";
-        }
-        if (!empty($params['fk_table_name'])) {
-          $sql .= ", DROP FOREIGN KEY FK_{$params['fkName']}";
-        }
-        break;
-    }
+    $sql = self::buildFieldChangeSql($params, $indexExist);
 
     // CRM-7007: do not i18n-rewrite this query
     CRM_Core_DAO::executeQuery($sql, [], TRUE, NULL, FALSE, FALSE);
@@ -342,7 +312,7 @@ ALTER TABLE {$tableName}
       // Are there any modifies we DON'T was to call this function for (& shouldn't it be clever enough to cope?)
       if ($params['operation'] == 'add' || $params['operation'] == 'modify') {
         $logging = new CRM_Logging_Schema();
-        $logging->fixSchemaDifferencesFor($params['table_name'], [trim($prefix) => [$params['name']]], FALSE);
+        $logging->fixSchemaDifferencesFor($params['table_name'], [trim(strtoupper($params['operation'])) => [$params['name']]], FALSE);
       }
     }
 
@@ -779,4 +749,45 @@ MODIFY      {$columnName} varchar( $length )
     }
   }
 
+  /**
+   * Build the sql to alter the field.
+   *
+   * @param array $params
+   * @param bool $indexExist
+   *
+   * @return string
+   */
+  public static function buildFieldChangeSql($params, $indexExist) {
+    $sql = str_repeat(' ', 8);
+    $sql .= "ALTER TABLE {$params['table_name']}";
+    switch ($params['operation']) {
+      case 'add':
+        $separator = "\n";
+        $sql .= self::buildFieldSQL($params, $separator, "ADD COLUMN ");
+        $separator = ",\n";
+        $sql .= self::buildPrimaryKeySQL($params, $separator, "ADD PRIMARY KEY ");
+        $sql .= self::buildSearchIndexSQL($params, $separator, "ADD INDEX ");
+        $sql .= self::buildForeignKeySQL($params, $separator, "ADD ", $params['table_name']);
+        break;
+
+      case 'modify':
+        $separator = "\n";
+        $sql .= self::buildFieldSQL($params, $separator, "MODIFY ");
+        $separator = ",\n";
+        $sql .= self::buildSearchIndexSQL($params, $separator, "ADD INDEX ", $indexExist);
+        break;
+
+      case 'delete':
+        $sql .= " DROP COLUMN `{$params['name']}`";
+        if (!empty($params['primary'])) {
+          $sql .= ", DROP PRIMARY KEY";
+        }
+        if (!empty($params['fk_table_name'])) {
+          $sql .= ", DROP FOREIGN KEY FK_{$params['fkName']}";
+        }
+        break;
+    }
+    return $sql;
+  }
+
 }
diff --git a/tests/phpunit/CRM/Core/BAO/SchemaHandlerTest.php b/tests/phpunit/CRM/Core/BAO/SchemaHandlerTest.php
index a504f8f437..f65fc65430 100644
--- a/tests/phpunit/CRM/Core/BAO/SchemaHandlerTest.php
+++ b/tests/phpunit/CRM/Core/BAO/SchemaHandlerTest.php
@@ -336,4 +336,29 @@ class CRM_Core_BAO_SchemaHandlerTest extends CiviUnitTestCase {
     }
   }
 
+  /**
+   * Tests the function that generates sql to modify fields.
+   */
+  public function testBuildFieldChangeSql() {
+    $params = [
+      'table_name' => 'big_table',
+      'operation' => 'add',
+      'name' => 'big_bob',
+      'type' => 'text',
+    ];
+    $sql = CRM_Core_BAO_SchemaHandler::buildFieldChangeSql($params, FALSE);
+    $this->assertEquals("ALTER TABLE big_table
+        ADD COLUMN `big_bob` text", trim($sql));
+
+    $params['operation'] = 'modify';
+    $params['comment'] = 'super big';
+    $sql = CRM_Core_BAO_SchemaHandler::buildFieldChangeSql($params, FALSE);
+    $this->assertEquals("ALTER TABLE big_table
+        MODIFY `big_bob` text COMMENT 'super big'", trim($sql));
+
+    $params['operation'] = 'delete';
+    $sql = CRM_Core_BAO_SchemaHandler::buildFieldChangeSql($params, FALSE);
+    $this->assertEquals('ALTER TABLE big_table DROP COLUMN `big_bob`', trim($sql));
+  }
+
 }