Fix saving custom fields with same name

diff --git a/Civi/Api4/Generic/Traits/DAOActionTrait.php b/Civi/Api4/Generic/Traits/DAOActionTrait.php
index 85ac173f5e6ce96dfdb8fe98a9204958bacac5c2..8db750d13e1c4546a9653dc41efe7b8b3f5ec7ef 100644 (file)
--- a/Civi/Api4/Generic/Traits/DAOActionTrait.php
+++ b/Civi/Api4/Generic/Traits/DAOActionTrait.php
@@ -12,6 +12,7 @@
 
 namespace Civi\Api4\Generic\Traits;
 
+use Civi\Api4\CustomField;
 use Civi\Api4\Utils\FormattingUtil;
 
 /**
@@ -168,50 +169,29 @@ trait DAOActionTrait {
     // $customValueID is the ID of the custom value in the custom table for this
     // entity (i guess this assumes it's not a multi value entity)
     foreach ($params as $name => $value) {
-      if (strpos($name, '.') === FALSE) {
+      $field = $this->getCustomFieldInfo($name);
+      if (!$field) {
         continue;
       }
 
-      list($customGroup, $customField) = explode('.', $name);
-      list($customField, $option) = array_pad(explode(':', $customField), 2, NULL);
-
-      $customFieldId = \CRM_Core_BAO_CustomField::getFieldValue(
-        \CRM_Core_DAO_CustomField::class,
-        $customField,
-        'id',
-        'name'
-      );
-      $customFieldType = \CRM_Core_BAO_CustomField::getFieldValue(
-        \CRM_Core_DAO_CustomField::class,
-        $customField,
-        'html_type',
-        'name'
-      );
-      $customFieldExtends = \CRM_Core_BAO_CustomGroup::getFieldValue(
-        \CRM_Core_DAO_CustomGroup::class,
-        $customGroup,
-        'extends',
-        'name'
-      );
-
       // todo are we sure we don't want to allow setting to NULL? need to test
-      if ($customFieldId && NULL !== $value) {
+      if (NULL !== $value) {
 
-        if ($option) {
-          $options = FormattingUtil::getPseudoconstantList($this->getEntityName(), 'custom_' . $customFieldId, $option, $params, $this->getActionName());
+        if ($field['suffix']) {
+          $options = FormattingUtil::getPseudoconstantList($this->getEntityName(), 'custom_' . $field['id'], $field['suffix'], $params, $this->getActionName());
           $value = FormattingUtil::replacePseudoconstant($options, $value, TRUE);
         }
 
-        if ($customFieldType === 'CheckBox') {
+        if ($field['html_type'] === 'CheckBox') {
           // this function should be part of a class
-          formatCheckBoxField($value, 'custom_' . $customFieldId, $this->getEntityName());
+          formatCheckBoxField($value, 'custom_' . $field['id'], $this->getEntityName());
         }
 
         \CRM_Core_BAO_CustomField::formatCustomField(
-          $customFieldId,
+          $field['id'],
           $customParams,
           $value,
-          $customFieldExtends,
+          $field['custom_group.extends'],
           // todo check when this is needed
           NULL,
           $entityId,
@@ -227,6 +207,30 @@ trait DAOActionTrait {
     }
   }
 
+  /**
+   * Gets field info needed to save custom data
+   *
+   * @param string $name
+   *   Field identifier with possible suffix, e.g. MyCustomGroup.MyField1:label
+   * @return array|NULL
+   */
+  protected function getCustomFieldInfo($name) {
+    if (strpos($name, '.') === FALSE) {
+      return NULL;
+    }
+    list($groupName, $fieldName) = explode('.', $name);
+    list($fieldName, $suffix) = array_pad(explode(':', $fieldName), 2, NULL);
+    if (empty(\Civi::$statics['APIv4_Custom_Fields'][$groupName])) {
+      \Civi::$statics['APIv4_Custom_Fields'][$groupName] = (array) CustomField::get()
+        ->setCheckPermissions(FALSE)
+        ->addSelect('id', 'name', 'html_type', 'custom_group.extends')
+        ->addWhere('custom_group.name', '=', $groupName)
+        ->execute()->indexBy('name');
+    }
+    $info = \Civi::$statics['APIv4_Custom_Fields'][$groupName][$fieldName] ?? NULL;
+    return $info ? ['suffix' => $suffix] + $info : NULL;
+  }
+
   /**
    * Check edit/delete permissions for contacts and related entities.
    *

diff --git a/tests/phpunit/api/v4/Action/BasicCustomFieldTest.php b/tests/phpunit/api/v4/Action/BasicCustomFieldTest.php
index 0c6162cb615b1b540f8fd52e4d236391c62ae2b2..1d5b6e42477aa65aa668aa409edd49246c7d101e 100644 (file)
--- a/tests/phpunit/api/v4/Action/BasicCustomFieldTest.php
+++ b/tests/phpunit/api/v4/Action/BasicCustomFieldTest.php
@@ -90,27 +90,38 @@ class BasicCustomFieldTest extends BaseCustomValueTest {
 
   public function testWithTwoFields() {
 
-    $customGroup = CustomGroup::create()
+    // First custom set
+    CustomGroup::create()
       ->setCheckPermissions(FALSE)
       ->addValue('name', 'MyContactFields')
       ->addValue('extends', 'Contact')
-      ->execute()
-      ->first();
-
-    CustomField::create()
-      ->setCheckPermissions(FALSE)
-      ->addValue('label', 'FavColor')
-      ->addValue('custom_group_id', $customGroup['id'])
-      ->addValue('html_type', 'Text')
-      ->addValue('data_type', 'String')
+      ->addChain('field1', CustomField::create()
+        ->addValue('label', 'FavColor')
+        ->addValue('custom_group_id', '$id')
+        ->addValue('html_type', 'Text')
+        ->addValue('data_type', 'String'))
+      ->addChain('field2', CustomField::create()
+        ->addValue('label', 'FavFood')
+        ->addValue('custom_group_id', '$id')
+        ->addValue('html_type', 'Text')
+        ->addValue('data_type', 'String'))
       ->execute();
 
-    CustomField::create()
+    // Second custom set
+    CustomGroup::create()
       ->setCheckPermissions(FALSE)
-      ->addValue('label', 'FavFood')
-      ->addValue('custom_group_id', $customGroup['id'])
-      ->addValue('html_type', 'Text')
-      ->addValue('data_type', 'String')
+      ->addValue('name', 'MyContactFields2')
+      ->addValue('extends', 'Contact')
+      ->addChain('field1', CustomField::create()
+        ->addValue('label', 'FavColor')
+        ->addValue('custom_group_id', '$id')
+        ->addValue('html_type', 'Text')
+        ->addValue('data_type', 'String'))
+      ->addChain('field2', CustomField::create()
+        ->addValue('label', 'FavFood')
+        ->addValue('custom_group_id', '$id')
+        ->addValue('html_type', 'Text')
+        ->addValue('data_type', 'String'))
       ->execute();
 
     $contactId1 = Contact::create()
@@ -145,19 +156,38 @@ class BasicCustomFieldTest extends BaseCustomValueTest {
     $this->assertArrayHasKey('MyContactFields.FavColor', $contact);
     $this->assertEquals('Red', $contact['MyContactFields.FavColor']);
 
+    // Update 2nd set and ensure 1st hasn't changed
     Contact::update()
       ->addWhere('id', '=', $contactId1)
-      ->addValue('MyContactFields.FavColor', 'Blue')
+      ->addValue('MyContactFields2.FavColor', 'Orange')
+      ->addValue('MyContactFields2.FavFood', 'Tangerine')
       ->execute();
-
     $contact = Contact::get()
       ->setCheckPermissions(FALSE)
-      ->addSelect('MyContactFields.FavColor')
+      ->addSelect('MyContactFields.FavColor', 'MyContactFields2.FavColor', 'MyContactFields.FavFood', 'MyContactFields2.FavFood')
       ->addWhere('id', '=', $contactId1)
       ->execute()
       ->first();
+    $this->assertEquals('Red', $contact['MyContactFields.FavColor']);
+    $this->assertEquals('Orange', $contact['MyContactFields2.FavColor']);
+    $this->assertEquals('Cherry', $contact['MyContactFields.FavFood']);
+    $this->assertEquals('Tangerine', $contact['MyContactFields2.FavFood']);
 
+    // Update 1st set and ensure 2st hasn't changed
+    Contact::update()
+      ->addWhere('id', '=', $contactId1)
+      ->addValue('MyContactFields.FavColor', 'Blue')
+      ->execute();
+    $contact = Contact::get()
+      ->setCheckPermissions(FALSE)
+      ->addSelect('MyContactFields.FavColor', 'MyContactFields2.FavColor', 'MyContactFields.FavFood', 'MyContactFields2.FavFood')
+      ->addWhere('id', '=', $contactId1)
+      ->execute()
+      ->first();
     $this->assertEquals('Blue', $contact['MyContactFields.FavColor']);
+    $this->assertEquals('Orange', $contact['MyContactFields2.FavColor']);
+    $this->assertEquals('Cherry', $contact['MyContactFields.FavFood']);
+    $this->assertEquals('Tangerine', $contact['MyContactFields2.FavFood']);
 
     $search = Contact::get()
       ->setCheckPermissions(FALSE)