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

@@ -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');

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([

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;
 }

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']);
   }