From 540c0ec9988c9bb1ec1d4b29e72966e63814e6be Mon Sep 17 00:00:00 2001
From: Coleman Watts <coleman@civicrm.org>
Date: Mon, 27 Dec 2021 09:15:36 -0500
Subject: [PATCH] APIv4 - Fix saving NULL as custom field value

Fixes dev/report#89
---
 Civi/Api4/Generic/Traits/DAOActionTrait.php   | 65 ++++++++++---------
 .../api/v4/Action/BasicCustomFieldTest.php    | 12 ++++
 2 files changed, 45 insertions(+), 32 deletions(-)

diff --git a/Civi/Api4/Generic/Traits/DAOActionTrait.php b/Civi/Api4/Generic/Traits/DAOActionTrait.php
index 46120cd40b..8f00ba7ee0 100644
--- a/Civi/Api4/Generic/Traits/DAOActionTrait.php
+++ b/Civi/Api4/Generic/Traits/DAOActionTrait.php
@@ -257,43 +257,44 @@ trait DAOActionTrait {
         continue;
       }
 
-      // todo are we sure we don't want to allow setting to NULL? need to test
-      if (NULL !== $value) {
+      // Null and empty string are interchangeable as far as the custom bao understands
+      if (NULL === $value) {
+        $value = '';
+      }
 
-        if ($field['suffix']) {
-          $options = FormattingUtil::getPseudoconstantList($field, $name, $params, $this->getActionName());
-          $value = FormattingUtil::replacePseudoconstant($options, $value, TRUE);
-        }
+      if ($field['suffix']) {
+        $options = FormattingUtil::getPseudoconstantList($field, $name, $params, $this->getActionName());
+        $value = FormattingUtil::replacePseudoconstant($options, $value, TRUE);
+      }
 
-        if ($field['html_type'] === 'CheckBox') {
-          // this function should be part of a class
-          formatCheckBoxField($value, 'custom_' . $field['id'], $this->getEntityName());
-        }
+      if ($field['html_type'] === 'CheckBox') {
+        // this function should be part of a class
+        formatCheckBoxField($value, 'custom_' . $field['id'], $this->getEntityName());
+      }
 
-        // Match contact id to strings like "user_contact_id"
-        // FIXME handle arrays for multi-value contact reference fields, etc.
-        if ($field['data_type'] === 'ContactReference' && is_string($value) && !is_numeric($value)) {
-          // FIXME decouple from v3 API
-          require_once 'api/v3/utils.php';
-          $value = \_civicrm_api3_resolve_contactID($value);
-          if ('unknown-user' === $value) {
-            throw new \API_Exception("\"{$field['name']}\" \"{$value}\" cannot be resolved to a contact ID", 2002, ['error_field' => $field['name'], "type" => "integer"]);
-          }
+      // Match contact id to strings like "user_contact_id"
+      // FIXME handle arrays for multi-value contact reference fields, etc.
+      if ($field['data_type'] === 'ContactReference' && is_string($value) && !is_numeric($value)) {
+        // FIXME decouple from v3 API
+        require_once 'api/v3/utils.php';
+        $value = \_civicrm_api3_resolve_contactID($value);
+        if ('unknown-user' === $value) {
+          throw new \API_Exception("\"{$field['name']}\" \"{$value}\" cannot be resolved to a contact ID", 2002, ['error_field' => $field['name'], "type" => "integer"]);
         }
-
-        \CRM_Core_BAO_CustomField::formatCustomField(
-          $field['id'],
-          $customParams,
-          $value,
-          $field['custom_group_id.extends'],
-          // todo check when this is needed
-          NULL,
-          $entityId,
-          FALSE,
-          $this->getCheckPermissions(),
-          TRUE
-        );
       }
+
+      \CRM_Core_BAO_CustomField::formatCustomField(
+        $field['id'],
+        $customParams,
+        $value,
+        $field['custom_group_id.extends'],
+        // todo check when this is needed
+        NULL,
+        $entityId,
+        FALSE,
+        $this->getCheckPermissions(),
+        TRUE
+      );
     }
 
     $params['custom'] = $customParams ?: NULL;
diff --git a/tests/phpunit/api/v4/Action/BasicCustomFieldTest.php b/tests/phpunit/api/v4/Action/BasicCustomFieldTest.php
index de2edf923c..240ce3a943 100644
--- a/tests/phpunit/api/v4/Action/BasicCustomFieldTest.php
+++ b/tests/phpunit/api/v4/Action/BasicCustomFieldTest.php
@@ -84,6 +84,18 @@ class BasicCustomFieldTest extends BaseCustomValueTest {
       ->first();
 
     $this->assertEquals('Blue', $contact['MyIndividualFields.FavColor']);
+
+    // Try setting to null
+    Contact::update()
+      ->addWhere('id', '=', $contactId)
+      ->addValue('MyIndividualFields.FavColor', NULL)
+      ->execute();
+    $contact = Contact::get(FALSE)
+      ->addSelect('MyIndividualFields.FavColor')
+      ->addWhere('id', '=', $contactId)
+      ->execute()
+      ->first();
+    $this->assertEquals(NULL, $contact['MyIndividualFields.FavColor']);
   }
 
   public function testWithTwoFields() {
-- 
2.25.1