From a4c7afc01652db4675b4da69f6e8d3465ed19358 Mon Sep 17 00:00:00 2001
From: Coleman Watts <coleman@civicrm.org>
Date: Thu, 27 Feb 2020 19:45:22 -0500
Subject: [PATCH] APIv4 - Correctly return null values from DAO save actions

Make sure the value "null" is not returned as a string
Don't return null values from saved DAO objects unless it is in params or reload is set to true
---
 Civi/Api4/Generic/Traits/DAOActionTrait.php   | 41 +++++++++++--------
 tests/phpunit/api/v4/Action/NullValueTest.php | 40 ++++++++++++++++++
 2 files changed, 64 insertions(+), 17 deletions(-)

diff --git a/Civi/Api4/Generic/Traits/DAOActionTrait.php b/Civi/Api4/Generic/Traits/DAOActionTrait.php
index 39bc4388f5ee..fc4fe9d262b3 100644
--- a/Civi/Api4/Generic/Traits/DAOActionTrait.php
+++ b/Civi/Api4/Generic/Traits/DAOActionTrait.php
@@ -47,19 +47,33 @@ protected function getBaoName() {
   }
 
   /**
-   * Extract the true fields from a BAO
+   * Convert saved object to array
    *
-   * (Used by create and update actions)
-   * @param object $bao
+   * Used by create, update & save actions
+   *
+   * @param \CRM_Core_DAO $bao
+   * @param array $input
    * @return array
    */
-  public static function baoToArray($bao) {
-    $fields = $bao->fields();
+  public function baoToArray($bao, $input) {
+    $allFields = array_column($bao->fields(), 'name');
+    if (!empty($this->reload)) {
+      $inputFields = $allFields;
+      $bao->find(TRUE);
+    }
+    else {
+      $inputFields = array_keys($input);
+      // Convert 'null' input to true null
+      foreach ($input as $key => $val) {
+        if ($val === 'null') {
+          $bao->$key = NULL;
+        }
+      }
+    }
     $values = [];
-    foreach ($fields as $key => $field) {
-      $name = $field['name'];
-      if (property_exists($bao, $name)) {
-        $values[$name] = isset($bao->$name) ? $bao->$name : NULL;
+    foreach ($allFields as $field) {
+      if (isset($bao->$field) || in_array($field, $inputFields)) {
+        $values[$field] = $bao->$field ?? NULL;
       }
     }
     return $values;
@@ -160,14 +174,7 @@ protected function writeObjects($items) {
         throw new \API_Exception($errMessage);
       }
 
-      if (!empty($this->reload) && is_a($createResult, 'CRM_Core_DAO')) {
-        $createResult->find(TRUE);
-      }
-
-      // trim back the junk and just get the array:
-      $resultArray = $this->baoToArray($createResult);
-
-      $result[] = $resultArray;
+      $result[] = $this->baoToArray($createResult, $item);
     }
     FormattingUtil::formatOutputValues($result, $this->entityFields(), $this->getEntityName());
     return $result;
diff --git a/tests/phpunit/api/v4/Action/NullValueTest.php b/tests/phpunit/api/v4/Action/NullValueTest.php
index cc3030c91c74..4fedd9eda966 100644
--- a/tests/phpunit/api/v4/Action/NullValueTest.php
+++ b/tests/phpunit/api/v4/Action/NullValueTest.php
@@ -21,6 +21,7 @@
 
 namespace api\v4\Action;
 
+use Civi\Api4\Activity;
 use Civi\Api4\Contact;
 use api\v4\UnitTestCase;
 
@@ -71,4 +72,43 @@ public function testSettingToNull() {
     $this->assertSame('ILoveMy', $contact['display_name']);
   }
 
+  public function testSaveWithReload() {
+    $contact = Contact::create()
+      ->setCheckPermissions(FALSE)
+      ->addValue('first_name', 'Firsty')
+      ->addValue('last_name', 'Lasty')
+      ->execute()
+      ->first();
+
+    $activity = Activity::create()
+      ->setCheckPermissions(FALSE)
+      ->addValue('source_contact_id', $contact['id'])
+      ->addValue('activity_type_id', 1)
+      ->addValue('subject', 'hello')
+      ->execute()
+      ->first();
+
+    $this->assertEquals('hello', $activity['subject']);
+
+    $saved = Activity::save()
+      ->setCheckPermissions(FALSE)
+      ->addRecord(['id' => $activity['id'], 'subject' => NULL])
+      ->execute()
+      ->first();
+
+    $this->assertNull($saved['subject']);
+    $this->assertArrayNotHasKey('activity_date_time', $saved);
+
+    $saved = Activity::save()
+      ->setCheckPermissions(FALSE)
+      ->addRecord(['id' => $activity['id'], 'subject' => NULL])
+      ->setReload(TRUE)
+      ->execute()
+      ->first();
+
+    $this->assertNull($saved['subject']);
+    $this->assertArrayHasKey('activity_date_time', $saved);
+
+  }
+
 }