From 365f37f03fcf8316c4baee0642dc8fd1029389b0 Mon Sep 17 00:00:00 2001
From: Coleman Watts <coleman@civicrm.org>
Date: Wed, 19 Aug 2020 16:04:34 -0400
Subject: [PATCH] APIv4 - Fix output of CustomValue create/save/update

Before: output contained no useful data
After: output contains values and id
---
 CRM/Core/BAO/CustomValueTable.php             |  8 ++---
 .../Generic/Traits/CustomValueActionTrait.php | 14 +++++---
 .../phpunit/api/v4/Action/CustomValueTest.php | 35 +++++++++++--------
 3 files changed, 34 insertions(+), 23 deletions(-)

diff --git a/CRM/Core/BAO/CustomValueTable.php b/CRM/Core/BAO/CustomValueTable.php
index 8bfc4fb8add9..fb3bff28f973 100644
--- a/CRM/Core/BAO/CustomValueTable.php
+++ b/CRM/Core/BAO/CustomValueTable.php
@@ -540,7 +540,7 @@ public static function setValues(&$params) {
     }
 
     if (!isset($params['entityID']) || !CRM_Utils_Type::validate($params['entityID'], 'Integer', FALSE)) {
-      return CRM_Core_Error::createAPIError(ts('entity_id needs to be set and of type Integer'));
+      throw new Exception(ts('entity_id needs to be set and of type Integer'));
     }
 
     // first collect all the id/value pairs. The format is:
@@ -550,7 +550,7 @@ public static function setValues(&$params) {
       if ($customFieldInfo = CRM_Core_BAO_CustomField::getKeyID($n, TRUE)) {
         $fieldID = (int ) $customFieldInfo[0];
         if (CRM_Utils_Type::escape($fieldID, 'Integer', FALSE) === NULL) {
-          return CRM_Core_Error::createAPIError(ts('field ID needs to be of type Integer for index %1',
+          throw new Exception(ts('field ID needs to be of type Integer for index %1',
             [1 => $fieldID]
           ));
         }
@@ -620,7 +620,7 @@ public static function setValues(&$params) {
         }
         // Ensure that value is of the right data type
         elseif (CRM_Utils_Type::escape($fieldValue['value'], $dataType, FALSE) === NULL) {
-          return CRM_Core_Error::createAPIError(ts('value: %1 is not of the right field data type: %2',
+          throw new Exception(ts('value: %1 is not of the right field data type: %2',
             [
               1 => $fieldValue['value'],
               2 => $dao->data_type,
@@ -668,7 +668,7 @@ public static function setValues(&$params) {
       return ['is_error' => 0, 'result' => 1];
     }
 
-    return CRM_Core_Error::createAPIError(ts('Unknown error'));
+    throw new Exception(ts('Unknown error'));
   }
 
   /**
diff --git a/Civi/Api4/Generic/Traits/CustomValueActionTrait.php b/Civi/Api4/Generic/Traits/CustomValueActionTrait.php
index acf2bd70a408..2fed2b7f467c 100644
--- a/Civi/Api4/Generic/Traits/CustomValueActionTrait.php
+++ b/Civi/Api4/Generic/Traits/CustomValueActionTrait.php
@@ -52,9 +52,8 @@ public function getEntityName() {
    * @inheritDoc
    */
   protected function writeObjects($items) {
-    $result = [];
     $fields = $this->entityFields();
-    foreach ($items as $item) {
+    foreach ($items as $idx => $item) {
       FormattingUtil::formatWriteParams($item, $fields);
 
       // Convert field names to custom_xx format
@@ -65,9 +64,16 @@ protected function writeObjects($items) {
         }
       }
 
-      $result[] = \CRM_Core_BAO_CustomValueTable::setValues($item);
+      \CRM_Core_BAO_CustomValueTable::setValues($item);
+
+      // Darn setValues function doesn't return an id.
+      if (empty($item['id'])) {
+        $tableName = CoreUtil::getTableName($this->getEntityName());
+        $items[$idx]['id'] = (int) \CRM_Core_DAO::singleValueQuery('SELECT MAX(id) FROM ' . $tableName);
+      }
     }
-    return $result;
+    FormattingUtil::formatOutputValues($items, $this->entityFields(), $this->getEntityName(), 'create');
+    return $items;
   }
 
   /**
diff --git a/tests/phpunit/api/v4/Action/CustomValueTest.php b/tests/phpunit/api/v4/Action/CustomValueTest.php
index b8a369658060..81edd46e2a7f 100644
--- a/tests/phpunit/api/v4/Action/CustomValueTest.php
+++ b/tests/phpunit/api/v4/Action/CustomValueTest.php
@@ -135,40 +135,45 @@ public function testCRUD() {
 
     // CASE 1: Test CustomValue::create
     // Create two records for a single contact and using CustomValue::get ensure that two records are created
-    CustomValue::create($group)
-      ->addValue($colorField, 'g')
-      ->addValue("entity_id", $this->contactID)
-      ->execute();
-    CustomValue::create($group)
-      ->addValue($colorField, 'r')
-      ->addValue("entity_id", $this->contactID)
-      ->execute();
+    $created = [
+      CustomValue::create($group)
+        ->addValue($colorField, 'g')
+        ->addValue("entity_id", $this->contactID)
+        ->execute()->first(),
+      CustomValue::create($group)
+        ->addValue($colorField . ':label', 'Red')
+        ->addValue("entity_id", $this->contactID)
+        ->execute()->first(),
+    ];
     // fetch custom values using API4 CustomValue::get
     $result = CustomValue::get($group)
       ->addSelect('id', 'entity_id', $colorField, $colorField . ':label')
-      ->addOrderBy($colorField, 'DESC')
+      ->addOrderBy($colorField, 'ASC')
       ->execute();
 
     // check if two custom values are created
     $this->assertEquals(2, count($result));
     $expectedResult = [
-      [
-        'id' => 2,
-        $colorField => 'r',
-        $colorField . ':label' => 'Red',
-        'entity_id' => $this->contactID,
-      ],
       [
         'id' => 1,
         $colorField => 'g',
         $colorField . ':label' => 'Green',
         'entity_id' => $this->contactID,
       ],
+      [
+        'id' => 2,
+        $colorField => 'r',
+        $colorField . ':label' => 'Red',
+        'entity_id' => $this->contactID,
+      ],
     ];
     // match the data
     foreach ($expectedResult as $key => $field) {
       foreach ($field as $attr => $value) {
         $this->assertEquals($expectedResult[$key][$attr], $result[$key][$attr]);
+        if (!strpos($attr, ':')) {
+          $this->assertEquals($expectedResult[$key][$attr], $created[$key][$attr]);
+        }
       }
     }