Merge pull request #20931 from civicrm/5.40

5.40
civicrm · Jul 21, 2021 · 0c60049 · 0c60049
2 parents 7c97bc4 + 735dc4c
commit 0c60049
Show file tree

Hide file tree

Showing 5 changed files with 55 additions and 20 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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());
+  }
+
 }