diff --git a/CRM/Core/BAO/CustomField.php b/CRM/Core/BAO/CustomField.php index ba06f7197373..b7c2386fba3b 100644 --- a/CRM/Core/BAO/CustomField.php +++ b/CRM/Core/BAO/CustomField.php @@ -2009,7 +2009,7 @@ protected static function prepareCreate($params) { // 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']) + && (empty($params['option_group_id']) && !empty($params['option_value']) ) ) { // first create an option group for this custom group diff --git a/api/v3/CustomField.php b/api/v3/CustomField.php index 1d0ee3d0033b..7a66f733f50c 100644 --- a/api/v3/CustomField.php +++ b/api/v3/CustomField.php @@ -19,20 +19,21 @@ * Create a 'custom field' within a custom field group. * * We also empty the static var in the getfields - * function after deletion so that the field is available for us (getfields manages date conversion - * among other things + * function after deletion so that the field is available for us (getfields + * manages date conversion among other things * * @param array $params * Array per getfields metadata. * * @return array * API success array + * @throws \CiviCRM_API3_Exception */ -function civicrm_api3_custom_field_create($params) { +function civicrm_api3_custom_field_create(array $params): array { // Legacy handling for old way of naming serialized fields if (!empty($params['html_type'])) { - if ($params['html_type'] == 'CheckBox' || strpos($params['html_type'], 'Multi-') === 0) { + if ($params['html_type'] === 'CheckBox' || strpos($params['html_type'], 'Multi-') === 0) { $params['serialize'] = 1; } $params['html_type'] = str_replace(['Multi-Select', 'Select Country', 'Select State/Province'], 'Select', $params['html_type']); @@ -58,6 +59,18 @@ function civicrm_api3_custom_field_create($params) { $params['option_weight'][$key] = $value['weight']; } } + elseif ( + // Legacy handling for historical apiv3 behaviour. + empty($params['id']) + && !empty($params['html_type']) + && $params['html_type'] !== 'Text' + && empty($params['option_group_id']) + && empty($params['option_value']) + && in_array($params['data_type'] ?? '', ['String', 'Int', 'Float', 'Money'])) { + // Trick the BAO into creating an option group even though no option values exist + // because that odd behaviour is locked in via a test. + $params['option_value'] = 1; + } $values = []; $customField = CRM_Core_BAO_CustomField::create($params); _civicrm_api3_object_to_array_unique_fields($customField, $values[$customField->id]); diff --git a/tests/phpunit/CRM/Contact/Import/Parser/ContactTest.php b/tests/phpunit/CRM/Contact/Import/Parser/ContactTest.php index 75c68bb0ccea..2e7d06a73d00 100644 --- a/tests/phpunit/CRM/Contact/Import/Parser/ContactTest.php +++ b/tests/phpunit/CRM/Contact/Import/Parser/ContactTest.php @@ -689,7 +689,7 @@ public function testImportPrimaryAddressUpdate() { /** * Test the determination of whether a custom field is valid. */ - public function testCustomFieldValidation() { + public function testCustomFieldValidation(): void { $errorMessage = []; $customGroup = $this->customGroupCreate([ 'extends' => 'Contact', diff --git a/tests/phpunit/api/v3/CustomFieldTest.php b/tests/phpunit/api/v3/CustomFieldTest.php index d1278d09f9e9..0fc13cea2b03 100644 --- a/tests/phpunit/api/v3/CustomFieldTest.php +++ b/tests/phpunit/api/v3/CustomFieldTest.php @@ -176,19 +176,6 @@ public function _buildBasicParams($gid, $htype, $dtype) { ]; } - /** - * Test using example code. - */ - /*function testCustomFieldCreateExample( ) - { - - $customGroup = $this->customGroupCreate('Individual','date_test_group',3); - require_once 'api/v3/examples/CustomField/Create.ex.php'; - $result = custom_field_create_example(); - $expectedResult = custom_field_create_expectedresult(); - $this->assertEquals($result,$expectedResult); - }*/ - /** * Check with data type - Options with option_values */ @@ -216,7 +203,7 @@ public function testCustomFieldCreateWithEmptyOptionGroup(): void { $optionGroup = $this->callAPISuccess('option_group', 'getsingle', [ 'id' => $optionGroupID, ]); - $this->assertEquals($optionGroup['title'], 'Country'); + $this->assertEquals('Country', $optionGroup['title']); $optionValueCount = $this->callAPISuccess('option_value', 'getcount', [ 'option_group_id' => $optionGroupID, ]); diff --git a/tests/phpunit/api/v4/Action/BasicCustomFieldTest.php b/tests/phpunit/api/v4/Action/BasicCustomFieldTest.php index d9d1c2e31189..d22b3e041cb9 100644 --- a/tests/phpunit/api/v4/Action/BasicCustomFieldTest.php +++ b/tests/phpunit/api/v4/Action/BasicCustomFieldTest.php @@ -277,4 +277,39 @@ public function testRelationshipCacheCustomFields() { $this->assertEquals('Buddy', $results[0]["$cgName.PetName"]); } + /** + * Some types are creating a dummy option group even if we don't have + * any option values. + * @throws \API_Exception + */ + public function testUndesiredOptionGroupCreation(): void { + $optionGroupCount = OptionGroup::get(FALSE)->selectRowCount()->execute()->count(); + + $customGroup = CustomGroup::create(FALSE) + ->addValue('name', 'MyIndividualFields') + ->addValue('extends', 'Individual') + ->execute() + ->first(); + + // This one doesn't make sense to have an option group. + CustomField::create(FALSE) + ->addValue('label', 'FavColor') + ->addValue('custom_group_id', $customGroup['id']) + ->addValue('html_type', 'Number') + ->addValue('data_type', 'Money') + ->execute(); + + // This one might be ok if we planned to then use the autocreated option + // group, but if we go on to create our own after then we have an extra + // unused group. + CustomField::create(FALSE) + ->addValue('label', 'FavMovie') + ->addValue('custom_group_id', $customGroup['id']) + ->addValue('html_type', 'Select') + ->addValue('data_type', 'String') + ->execute(); + + $this->assertEquals($optionGroupCount, OptionGroup::get(FALSE)->selectRowCount()->execute()->count()); + } + }