From 9ccfced437e90c3383a3ad70791e63aeb4ecb530 Mon Sep 17 00:00:00 2001
From: Eileen McNaughton <emcnaughton@wikimedia.org>
Date: Thu, 1 Jul 2021 17:50:57 +1200
Subject: [PATCH] Improve api consistency on custom field creation

Api supports a key 'option_values' by always setting the 'magic' param
option_type to 1. v4 doesn't - but creating option values when creating
a custom field is desirable. This fixes so that the form still
'opts out' if it passes in '2' but otherwise we create the option values
if passed in.

Just a teensy bit less magic in the api layer & bao
---
 CRM/Core/BAO/CustomField.php             | 10 ++++++++--
 Civi/Test/Api3TestTrait.php              |  2 --
 api/v3/CustomField.php                   |  7 -------
 tests/phpunit/api/v3/CustomFieldTest.php | 10 +++++-----
 4 files changed, 13 insertions(+), 16 deletions(-)

diff --git a/CRM/Core/BAO/CustomField.php b/CRM/Core/BAO/CustomField.php
index f62d22773c6f..ba06f7197373 100644
--- a/CRM/Core/BAO/CustomField.php
+++ b/CRM/Core/BAO/CustomField.php
@@ -2003,9 +2003,15 @@ protected static function prepareCreate($params) {
 
     // create any option group & values if required
     $allowedOptionTypes = ['String', 'Int', 'Float', 'Money'];
-    if ($htmlType != 'Text' && in_array($dataType, $allowedOptionTypes)) {
+    if ($htmlType !== 'Text' && in_array($dataType, $allowedOptionTypes, TRUE)) {
       //CRM-16659: if option_value then create an option group for this custom field.
-      if ($params['option_type'] == 1 && (empty($params['option_group_id']) || !empty($params['option_value']))) {
+      // An option_type of 2 would be a 'message' from the form layer not to handle
+      // the option_values key. If not set then it is not ignored.
+      $optionsType = (int) ($params['option_type'] ?? 0);
+      if (($optionsType !== 2 && empty($params['id']))
+        && (empty($params['option_group_id']) || !empty($params['option_value'])
+        )
+      ) {
         // first create an option group for this custom group
         $optionGroup = new CRM_Core_DAO_OptionGroup();
         $optionGroup->name = "{$params['column_name']}_" . date('YmdHis');
diff --git a/Civi/Test/Api3TestTrait.php b/Civi/Test/Api3TestTrait.php
index ca7ba747ce07..65bd2ee2162e 100644
--- a/Civi/Test/Api3TestTrait.php
+++ b/Civi/Test/Api3TestTrait.php
@@ -152,8 +152,6 @@ public function callAPIFailure($entity, $action, $params, $expectedErrorMessage
    *   better or worse )
    *
    * @return array|int
-   *
-   * @throws \CRM_Core_Exception
    */
   public function callAPISuccess($entity, $action, $params = [], $checkAgainst = NULL) {
     $params = array_merge([
diff --git a/api/v3/CustomField.php b/api/v3/CustomField.php
index f8652419bee9..1d0ee3d0033b 100644
--- a/api/v3/CustomField.php
+++ b/api/v3/CustomField.php
@@ -90,13 +90,6 @@ function _civicrm_api3_custom_field_create_spec(&$params) {
     'title' => 'Option Values',
     'description' => "Pass an array of options (value => label) to create this field's option values",
   ];
-  // TODO: Why expose this to the api at all?
-  $params['option_type'] = [
-    'title' => 'Option Type',
-    'description' => 'This (boolean) field tells the BAO to create an option group for the field if the field type is appropriate',
-    'api.default' => 1,
-    'type' => CRM_Utils_Type::T_BOOLEAN,
-  ];
   $params['data_type']['api.default'] = 'String';
   $params['is_active']['api.default'] = 1;
 }
diff --git a/tests/phpunit/api/v3/CustomFieldTest.php b/tests/phpunit/api/v3/CustomFieldTest.php
index 87acb78cba0b..d1278d09f9e9 100644
--- a/tests/phpunit/api/v3/CustomFieldTest.php
+++ b/tests/phpunit/api/v3/CustomFieldTest.php
@@ -192,7 +192,7 @@ public function _buildBasicParams($gid, $htype, $dtype) {
   /**
    * Check with data type - Options with option_values
    */
-  public function testCustomFieldCreateWithEmptyOptionGroup() {
+  public function testCustomFieldCreateWithEmptyOptionGroup(): void {
     $customGroup = $this->customGroupCreate(['extends' => 'Contact', 'title' => 'select_test_group']);
     $params = [
       'custom_group_id' => $customGroup['id'],
@@ -205,9 +205,9 @@ public function testCustomFieldCreateWithEmptyOptionGroup() {
       'is_active' => 1,
     ];
 
-    $customField = $this->callAPISuccess('custom_field', 'create', $params);
+    $customField = $this->callAPISuccess('CustomField', 'create', $params);
     $this->assertNotNull($customField['id']);
-    $optionGroupID = $this->callAPISuccess('custom_field', 'getvalue', [
+    $optionGroupID = $this->callAPISuccess('CustomField', 'getvalue', [
       'id' => $customField['id'],
       'return' => 'option_group_id',
     ]);
@@ -238,10 +238,10 @@ public function testCustomFieldCreateWithNonAsciiLabel() {
       'is_searchable' => 0,
       'is_active' => 1,
     ];
-    $customField = $this->callAPISuccess('custom_field', 'create', $params);
+    $customField = $this->callAPISuccess('CustomField', 'create', $params);
     $this->assertNotNull($customField['id']);
     $params['label'] = 'ààà';
-    $customField = $this->callAPISuccess('custom_field', 'create', $params);
+    $customField = $this->callAPISuccess('CustomField', 'create', $params);
     $this->assertNotNull($customField['id']);
   }