dev/core#755 - CRM/Core - Fix missing group name in custom field cache

CRM_Core_BAO_CustomField::getCustomFieldID caches custom fields based
on the field name, meaning if a custom field of the same name exists
in more than one group, the method will always return the first field
that was cached. This change adds the $groupName parameter to the
cache key.

This also adds a missing test for the $fullString parameter and
renames the $groupTitle parameter to $groupName in preparation
for future changes.

diff --git a/CRM/Core/BAO/CustomField.php b/CRM/Core/BAO/CustomField.php
index 24b0bd5864ce7a801f5b38d03bebd2941508e237..70cfe416931ee122b8e3225b5a64cda35dc13253 100644 (file)
--- a/CRM/Core/BAO/CustomField.php
+++ b/CRM/Core/BAO/CustomField.php
@@ -2249,42 +2249,43 @@ ORDER BY html_type";
    * Get custom field ID from field/group name/title.
    *
    * @param string $fieldName Field name or label
-   * @param string|null $groupTitle (Optional) Group name or label
+   * @param string|null $groupName (Optional) Group name or label
    * @param bool $fullString Whether to return "custom_123" or "123"
    *
    * @return string|int|null
    * @throws \CiviCRM_API3_Exception
    */
-  public static function getCustomFieldID($fieldName, $groupTitle = NULL, $fullString = FALSE) {
-    if (!isset(Civi::$statics['CRM_Core_BAO_CustomField'][$fieldName])) {
+  public static function getCustomFieldID($fieldName, $groupName = NULL, $fullString = FALSE) {
+    $cacheKey = $groupName . '.' . $fieldName;
+    if (!isset(Civi::$statics['CRM_Core_BAO_CustomField'][$cacheKey])) {
       $customFieldParams = [
         'name' => $fieldName,
         'label' => $fieldName,
         'options' => ['or' => [["name", "label"]]],
       ];
 
-      if ($groupTitle) {
-        $customFieldParams['custom_group_id.name'] = $groupTitle;
-        $customFieldParams['custom_group_id.title'] = $groupTitle;
+      if ($groupName) {
+        $customFieldParams['custom_group_id.name'] = $groupName;
+        $customFieldParams['custom_group_id.title'] = $groupName;
         $customFieldParams['options'] = ['or' => [["name", "label"], ["custom_group_id.name", "custom_group_id.title"]]];
       }
 
       $field = civicrm_api3('CustomField', 'get', $customFieldParams);
 
       if (empty($field['id'])) {
-        Civi::$statics['CRM_Core_BAO_CustomField'][$fieldName]['id'] = NULL;
-        Civi::$statics['CRM_Core_BAO_CustomField'][$fieldName]['string'] = NULL;
+        Civi::$statics['CRM_Core_BAO_CustomField'][$cacheKey]['id'] = NULL;
+        Civi::$statics['CRM_Core_BAO_CustomField'][$cacheKey]['string'] = NULL;
       }
       else {
-        Civi::$statics['CRM_Core_BAO_CustomField'][$fieldName]['id'] = $field['id'];
-        Civi::$statics['CRM_Core_BAO_CustomField'][$fieldName]['string'] = 'custom_' . $field['id'];
+        Civi::$statics['CRM_Core_BAO_CustomField'][$cacheKey]['id'] = $field['id'];
+        Civi::$statics['CRM_Core_BAO_CustomField'][$cacheKey]['string'] = 'custom_' . $field['id'];
       }
     }
 
     if ($fullString) {
-      return Civi::$statics['CRM_Core_BAO_CustomField'][$fieldName]['string'];
+      return Civi::$statics['CRM_Core_BAO_CustomField'][$cacheKey]['string'];
     }
-    return Civi::$statics['CRM_Core_BAO_CustomField'][$fieldName]['id'];
+    return Civi::$statics['CRM_Core_BAO_CustomField'][$cacheKey]['id'];
   }
 
   /**

diff --git a/tests/phpunit/CRM/Core/BAO/CustomFieldTest.php b/tests/phpunit/CRM/Core/BAO/CustomFieldTest.php
index 58b056cf05fa9325e83234fccf97030f4e6842da..625fded6cad83835827347d0e0a19df0b8928496 100644 (file)
--- a/tests/phpunit/CRM/Core/BAO/CustomFieldTest.php
+++ b/tests/phpunit/CRM/Core/BAO/CustomFieldTest.php
@@ -361,13 +361,29 @@ class CRM_Core_BAO_CustomFieldTest extends CiviUnitTestCase {
 
     $fieldID = CRM_Core_BAO_CustomField::getCustomFieldID('testFld', 'new custom group');
     $this->assertEquals($this->customFieldID, $fieldID);
+
+    $fieldID = CRM_Core_BAO_CustomField::getCustomFieldID('testFld', 'new custom group', TRUE);
+    $this->assertEquals('custom_' . $this->customFieldID, $fieldID);
+
+    // create field with same name in a different group
+    $this->createCustomField('other custom group');
+    $otherFieldID = CRM_Core_BAO_CustomField::getCustomFieldID('testFld', 'other custom group');
+    // make sure it does not return the field ID of the first field
+    $this->assertNotEquals($fieldID, $otherFieldID);
   }
 
   /**
+   * Create a custom field
+   *
+   * @param string $groupTitle
+   *
    * @return array
    */
-  protected function createCustomField() {
-    $customGroup = $this->customGroupCreate(array('extends' => 'Individual'));
+  protected function createCustomField($groupTitle = 'new custom group') {
+    $customGroup = $this->customGroupCreate([
+      'extends' => 'Individual',
+      'title'   => $groupTitle,
+    ]);
     $fields = array(
       'label' => 'testFld',
       'data_type' => 'String',