From: eileen <emcnaughton@wikimedia.org>
Date: Thu, 4 Jul 2019 00:08:34 +0000 (+1200)
Subject: dev/core#1093 add a bulkSave action for many customFields in one go
X-Git-Url: https://vcs.fsf.org/?a=commitdiff_plain;h=e5ce15c371ec38817f7faca00b5ea6314f4adea3;p=civicrm-core.git

dev/core#1093 add a bulkSave action for many customFields in one go

Add bulkCreate function for CustomField with a view towards this being a new protocol for how bulk create actions would look from a code POV which we could expose via apiv4
---

diff --git a/CRM/Core/BAO/CustomField.php b/CRM/Core/BAO/CustomField.php
index b893873e96..4af2b20991 100644
--- a/CRM/Core/BAO/CustomField.php
+++ b/CRM/Core/BAO/CustomField.php
@@ -154,8 +154,7 @@ class CRM_Core_BAO_CustomField extends CRM_Core_DAO_CustomField {
   public static function create($params) {
     $customField = self::createCustomFieldRecord($params);
     $op = empty($params['id']) ? 'add' : 'modify';
-    $indexExist = empty($params['id']) ? FALSE : CRM_Core_DAO::getFieldValue('CRM_Core_DAO_CustomField', $params['id'], 'is_searchable');
-    self::createField($customField, $op, $indexExist, CRM_Utils_Array::value('triggerRebuild', $params, TRUE));
+    self::createField($customField, $op, CRM_Utils_Array::value('triggerRebuild', $params, TRUE));
 
     CRM_Utils_Hook::post(($op === 'add' ? 'create' : 'edit'), 'CustomField', $customField->id, $customField);
 
@@ -164,6 +163,53 @@ class CRM_Core_BAO_CustomField extends CRM_Core_DAO_CustomField {
     return $customField;
   }
 
+  /**
+   * Create/ update several fields at once in a mysql efficient way.
+   *
+   * https://lab.civicrm.org/dev/core/issues/1093
+   *
+   * The intention is that apiv4 would expose any BAO with bulkSave as a new action.
+   *
+   * @param array $bulkParams
+   *   Array of arrays as would be passed into create
+   * @param array $defaults
+   *  Default parameters to be be merged into each of the params.
+   */
+  public static function bulkSave($bulkParams, $defaults) {
+    $sql = [];
+    $tables = [];
+    $customFields = [];
+    foreach ($bulkParams as $fieldParams) {
+      $params = array_merge($defaults, $fieldParams);
+      $operation = empty($params['id']) ? 'add' : 'modify';
+      $customField = self::createCustomFieldRecord($params);
+      $fieldSQL = self::getAlterFieldSQL($customField, $operation);
+      if (!isset($params['custom_group_id'])) {
+        $params['custom_group_id'] = civicrm_api3('CustomField', 'getvalue', ['id' => $customField->id, 'return' => 'custom_group_id']);
+      }
+      if (!isset($params['table_name'])) {
+        if (!isset($tables[$params['custom_group_id']])) {
+          $tables[$params['custom_group_id']] = civicrm_api3('CustomGroup', 'getvalue', [
+            'id' => $params['custom_group_id'],
+            'return' => 'table_name',
+          ]);
+        }
+        $params['table_name'] = $tables[$params['custom_group_id']];
+      }
+      $sql[$params['table_name']][] = $fieldSQL;
+      $customFields[] = $customField;
+    }
+    foreach ($sql as $tableName => $statements) {
+      // CRM-7007: do not i18n-rewrite this query
+      CRM_Core_DAO::executeQuery("ALTER TABLE $tableName " . implode(', ', $statements), [], TRUE, NULL, FALSE, FALSE);
+      Civi::service('sql_triggers')->rebuild($params['table_name'], TRUE);
+    }
+    CRM_Utils_System::flushCache();
+    foreach ($customFields as $customField) {
+      CRM_Utils_Hook::post($operation === 'add' ? 'create' : 'edit', 'CustomField', $customField->id, $customField);
+    }
+  }
+
   /**
    * Fetch object based on array of properties.
    *
@@ -1646,16 +1692,13 @@ SELECT $columnName
    *
    * @param CRM_Core_DAO_CustomField $field
    * @param string $operation
-   * @param bool $indexExist
    * @param bool $triggerRebuild
    */
-  public static function createField($field, $operation, $indexExist = FALSE, $triggerRebuild = TRUE) {
-    $params = [
-      'table_name' => CRM_Core_DAO::getFieldValue('CRM_Core_DAO_CustomGroup', $field->custom_group_id, 'table_name'),
-    ];
+  public static function createField($field, $operation, $triggerRebuild = TRUE) {
     $sql = str_repeat(' ', 8);
-    $sql .= "ALTER TABLE {$params['table_name']}";
-    $sql .= self::getAlterFieldSQL($field, $operation, $params, $indexExist);
+    $tableName = CRM_Core_DAO::getFieldValue('CRM_Core_DAO_CustomGroup', $field->custom_group_id, 'table_name');
+    $sql .= "ALTER TABLE " . $tableName;
+    $sql .= self::getAlterFieldSQL($field, $operation);
 
     // CRM-7007: do not i18n-rewrite this query
     CRM_Core_DAO::executeQuery($sql, [], TRUE, NULL, FALSE, FALSE);
@@ -1665,14 +1708,14 @@ SELECT $columnName
       // 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') {
+      if ($operation === 'add' || $operation === 'modify') {
         $logging = new CRM_Logging_Schema();
-        $logging->fixSchemaDifferencesFor($params['table_name'], [trim(strtoupper($operation)) => [$params['name']]]);
+        $logging->fixSchemaDifferencesFor($tableName, [trim(strtoupper($operation)) => [$field->column_name]]);
       }
     }
 
     if ($triggerRebuild) {
-      Civi::service('sql_triggers')->rebuild($params['table_name'], TRUE);
+      Civi::service('sql_triggers')->rebuild($tableName, TRUE);
     }
 
   }
@@ -1680,16 +1723,15 @@ SELECT $columnName
   /**
    * @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) {
+  public static function getAlterFieldSQL($field, $operation) {
+    $indexExist = $operation === 'add' ? FALSE : CRM_Core_DAO::getFieldValue('CRM_Core_DAO_CustomField', $field->id, 'is_searchable');
     $params = self::prepareCreateParams($field, $operation);
-    // lets suppress the required flag, since that can cause sql issue
+    // Let's suppress the required flag, since that can cause an sql issue... for unknown reasons since we are calling
+    // a function only used by Custom Field creation...
     $params['required'] = FALSE;
-
     return CRM_Core_BAO_SchemaHandler::getFieldAlterSQL($params, $indexExist);
   }
 
diff --git a/CRM/Utils/Migrate/Import.php b/CRM/Utils/Migrate/Import.php
index 340e10d51a..590a924c70 100644
--- a/CRM/Utils/Migrate/Import.php
+++ b/CRM/Utils/Migrate/Import.php
@@ -386,6 +386,7 @@ AND        v.name = %1
 
         // Only rebuild the table's trigger on the last field added to avoid un-necessary
         // and slow rebuilds when adding many fields at the same time.
+        // @todo - call bulkSave instead.
         $triggerRebuild = FALSE;
         if ($count == $total) {
           $triggerRebuild = TRUE;
diff --git a/tests/phpunit/CRM/Core/BAO/CustomFieldTest.php b/tests/phpunit/CRM/Core/BAO/CustomFieldTest.php
index 70449f8d6c..ffb3e19a91 100644
--- a/tests/phpunit/CRM/Core/BAO/CustomFieldTest.php
+++ b/tests/phpunit/CRM/Core/BAO/CustomFieldTest.php
@@ -636,4 +636,37 @@ class CRM_Core_BAO_CustomFieldTest extends CiviUnitTestCase {
     $this->assertEquals($expected, CRM_Core_BAO_CustomField::getFieldsForImport());
   }
 
+  /**
+   * Test the bulk create function works.
+   */
+  public function testBulkCreate() {
+    $customGroup = $this->customGroupCreate([
+      'extends' => 'Individual',
+      'title' => 'my bulk group',
+    ]);
+    CRM_Core_BAO_CustomField::bulkSave([
+      [
+        'label' => 'Test',
+        'data_type' => 'String',
+        'html_type' => 'Text',
+        'column_name' => 'my_text',
+      ],
+      [
+        'label' => 'test_link',
+        'data_type' => 'Link',
+        'html_type' => 'Link',
+        'is_search_range' => '0',
+      ],
+    ],
+    [
+      'custom_group_id' => $customGroup['id'],
+      'is_active' => 1,
+      'is_searchable' => 1,
+    ]);
+    $dao = CRM_Core_DAO::executeQuery(('SHOW CREATE TABLE ' . $customGroup['values'][$customGroup['id']]['table_name']));
+    $dao->fetch();
+    $this->assertContains('`test_link_2` varchar(255) COLLATE utf8_unicode_ci DEFAULT NULL', $dao->Create_Table);
+    $this->assertContains('KEY `INDEX_my_text` (`my_text`)', $dao->Create_Table);
+  }
+
 }
diff --git a/tests/phpunit/api/v3/CustomFieldTest.php b/tests/phpunit/api/v3/CustomFieldTest.php
index b05a439e11..1e7ad9d728 100644
--- a/tests/phpunit/api/v3/CustomFieldTest.php
+++ b/tests/phpunit/api/v3/CustomFieldTest.php
@@ -628,6 +628,9 @@ class api_v3_CustomFieldTest extends CiviUnitTestCase {
     $result = $this->callAPISuccess('CustomField', 'create', $params);
   }
 
+  /**
+   * Test disabling a searchable contact reference field.
+   */
   public function testDisableSearchableContactReferenceField() {
     $customGroup = $this->customGroupCreate([
       'name' => 'testCustomGroup',
@@ -647,7 +650,7 @@ class api_v3_CustomFieldTest extends CiviUnitTestCase {
       'id' => $result['id'],
       'is_active' => 0,
     ];
-    $result = $this->callAPISuccess('CustomField', 'create', $params);
+    $this->callAPISuccess('CustomField', 'create', $params);
   }
 
 }