Fix for cust field

diff --git a/CRM/Core/BAO/CustomField.php b/CRM/Core/BAO/CustomField.php
index ba06f7197373dc245056d2bf08a92271d999fe7f..b7c2386fba3b3b722b16ad6a7b2b527d10b3e340 100644 (file)
--- a/CRM/Core/BAO/CustomField.php
+++ b/CRM/Core/BAO/CustomField.php
@@ -2009,7 +2009,7 @@ WHERE  id IN ( %1, %2 )
       // 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 1d0ee3d0033b64b9e8280f8eae64fae4acfe48fe..7a66f733f50c45d53d57ddd0519ad381a2b984c0 100644 (file)
--- 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 75c68bb0cceae51565393163fb3fe5596bbda636..2e7d06a73d00964d638c2dc3b7312b830efc5974 100644 (file)
--- a/tests/phpunit/CRM/Contact/Import/Parser/ContactTest.php
+++ b/tests/phpunit/CRM/Contact/Import/Parser/ContactTest.php
@@ -689,7 +689,7 @@ class CRM_Contact_Import_Parser_ContactTest extends CiviUnitTestCase {
   /**
    * 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 d1278d09f9e9a047a8c235e09043ea71514490dd..0fc13cea2b03218cde632b3d126efe971c4c6fdb 100644 (file)
--- a/tests/phpunit/api/v3/CustomFieldTest.php
+++ b/tests/phpunit/api/v3/CustomFieldTest.php
@@ -176,19 +176,6 @@ class api_v3_CustomFieldTest extends CiviUnitTestCase {
     ];
   }
 
-  /**
-   * 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 @@ class api_v3_CustomFieldTest extends CiviUnitTestCase {
     $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 e85f816758df2204500f53237908818b882bce86..e0623d1f36ef6b4ba56c50d1946c2e1321e347aa 100644 (file)
--- a/tests/phpunit/api/v4/Action/BasicCustomFieldTest.php
+++ b/tests/phpunit/api/v4/Action/BasicCustomFieldTest.php
@@ -22,6 +22,7 @@ namespace api\v4\Action;
 use Civi\Api4\Contact;
 use Civi\Api4\CustomField;
 use Civi\Api4\CustomGroup;
+use Civi\Api4\OptionGroup;
 use Civi\Api4\Relationship;
 use Civi\Api4\RelationshipCache;
 
@@ -270,4 +271,39 @@ class BasicCustomFieldTest extends BaseCustomValueTest {
     $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());
+  }
+
 }