From f8df3008fcd3b24a332f6c875ed1f0bc1bb5d515 Mon Sep 17 00:00:00 2001
From: eileen <emcnaughton@wikimedia.org>
Date: Tue, 2 Jul 2019 13:38:07 +1200
Subject: [PATCH] [REF] restructure create params.

This is a bigger logic change but it brings back the parts that really are used only by CustomField::create
to that BAO & separates the generating of the sql from the running of it
---
 CRM/Core/BAO/CustomField.php                  | 38 ++++++++++++++++++-
 CRM/Core/BAO/SchemaHandler.php                | 35 -----------------
 .../CRM/Core/BAO/SchemaHandlerTest.php        |  4 +-
 3 files changed, 38 insertions(+), 39 deletions(-)

diff --git a/CRM/Core/BAO/CustomField.php b/CRM/Core/BAO/CustomField.php
index 6971b578fd5a..0fbb5b0242bd 100644
--- a/CRM/Core/BAO/CustomField.php
+++ b/CRM/Core/BAO/CustomField.php
@@ -199,7 +199,7 @@ protected static function doCreate($params) {
    *
    * @return array
    */
-  protected static function prepareCreateParams($field, $operation): array {
+  protected static function prepareCreateParams($field, $operation) {
     $tableName = CRM_Core_DAO::getFieldValue(
       'CRM_Core_DAO_CustomGroup',
       $field->custom_group_id,
@@ -1762,9 +1762,43 @@ public static function defaultCustomTableSchema($params) {
    * @param bool $triggerRebuild
    */
   public static function createField($field, $operation, $indexExist = FALSE, $triggerRebuild = TRUE) {
+    $params = [];
+    $sql = self::getAlterFieldSQL($field, $operation, $params, $indexExist);
+
+    // CRM-7007: do not i18n-rewrite this query
+    CRM_Core_DAO::executeQuery($sql, [], TRUE, NULL, FALSE, FALSE);
+
+    $config = CRM_Core_Config::singleton();
+    if ($config->logging) {
+      // CRM-16717 not sure why this was originally limited to add.
+      // For example custom tables can have field length changes - which need to flow through to logging.
+      // Are there any modifies we DON'T was to call this function for (& shouldn't it be clever enough to cope?)
+      if ($operation == 'add' || $operation == 'modify') {
+        $logging = new CRM_Logging_Schema();
+        $logging->fixSchemaDifferencesFor($params['table_name'], [trim(strtoupper($operation)) => [$params['name']]]);
+      }
+    }
+
+    if ($triggerRebuild) {
+      Civi::service('sql_triggers')->rebuild($params['table_name'], TRUE);
+    }
+
+  }
+
+  /**
+   * @param CRM_Core_DAO_CustomField $field
+   * @param string $operation
+   * @param array $params
+   * @param bool $indexExist
+   *
+   * @return bool
+   */
+  public static function getAlterFieldSQL($field, $operation, &$params, $indexExist = FALSE) {
     $params = self::prepareCreateParams($field, $operation);
+    // lets suppress the required flag, since that can cause sql issue
+    $params['required'] = FALSE;
 
-    CRM_Core_BAO_SchemaHandler::alterFieldSQL($params, $indexExist, $triggerRebuild);
+    return CRM_Core_BAO_SchemaHandler::buildFieldChangeSql($params, $indexExist);
   }
 
   /**
diff --git a/CRM/Core/BAO/SchemaHandler.php b/CRM/Core/BAO/SchemaHandler.php
index 8d78e337fd7f..72ab0408a6b6 100644
--- a/CRM/Core/BAO/SchemaHandler.php
+++ b/CRM/Core/BAO/SchemaHandler.php
@@ -287,41 +287,6 @@ public static function buildForeignKeySQL($params, $separator, $prefix, $tableNa
     return $sql;
   }
 
-  /**
-   * @param array $params
-   * @param bool $indexExist
-   * @param bool $triggerRebuild
-   *
-   * @return bool
-   */
-  public static function alterFieldSQL($params, $indexExist = FALSE, $triggerRebuild = TRUE) {
-
-    // lets suppress the required flag, since that can cause sql issue
-    $params['required'] = FALSE;
-
-    $sql = self::buildFieldChangeSql($params, $indexExist);
-
-    // CRM-7007: do not i18n-rewrite this query
-    CRM_Core_DAO::executeQuery($sql, [], TRUE, NULL, FALSE, FALSE);
-
-    $config = CRM_Core_Config::singleton();
-    if ($config->logging) {
-      // CRM-16717 not sure why this was originally limited to add.
-      // For example custom tables can have field length changes - which need to flow through to logging.
-      // 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(strtoupper($params['operation'])) => [$params['name']]]);
-      }
-    }
-
-    if ($triggerRebuild) {
-      Civi::service('sql_triggers')->rebuild($params['table_name'], TRUE);
-    }
-
-    return TRUE;
-  }
-
   /**
    * Delete a CiviCRM-table.
    *
diff --git a/tests/phpunit/CRM/Core/BAO/SchemaHandlerTest.php b/tests/phpunit/CRM/Core/BAO/SchemaHandlerTest.php
index f65fc6543088..a5b2d6634a2c 100644
--- a/tests/phpunit/CRM/Core/BAO/SchemaHandlerTest.php
+++ b/tests/phpunit/CRM/Core/BAO/SchemaHandlerTest.php
@@ -318,7 +318,7 @@ public function testDropColumn() {
     ];
 
     // drop col1
-    CRM_Core_BAO_SchemaHandler::alterFieldSQL($alterParams, FALSE, TRUE);
+    CRM_Core_DAO::executeQuery(CRM_Core_BAO_SchemaHandler::buildFieldChangeSql($alterParams, FALSE));
 
     $create_table = CRM_Core_DAO::executeQuery("SHOW CREATE TABLE civicrm_test_drop_column");
     while ($create_table->fetch()) {
@@ -328,7 +328,7 @@ public function testDropColumn() {
 
     // drop col2
     $alterParams['name'] = 'col2';
-    CRM_Core_BAO_SchemaHandler::alterFieldSQL($alterParams, FALSE, TRUE);
+    CRM_Core_DAO::executeQuery(CRM_Core_BAO_SchemaHandler::buildFieldChangeSql($alterParams, FALSE));
 
     $create_table = CRM_Core_DAO::executeQuery("SHOW CREATE TABLE civicrm_test_drop_column");
     while ($create_table->fetch()) {