Remove redundant custom field types
authorColeman Watts <coleman@civicrm.org>
Sat, 5 Sep 2020 19:22:32 +0000 (15:22 -0400)
committerColeman Watts <coleman@civicrm.org>
Sun, 6 Sep 2020 15:49:51 +0000 (11:49 -0400)
The html_type 'Select Country' and 'Select State/Province' serve no purpose. They are the same as Select, just with a different data_type.

This adds an APIv3 shim to display the old types.

CRM/Core/BAO/CustomField.php
CRM/Core/SelectValues.php
CRM/Custom/Form/Field.php
CRM/Profile/Page/MultipleRecordFieldsListing.php
CRM/Upgrade/Incremental/sql/5.31.alpha1.mysql.tpl
Civi/Api4/Service/Spec/SpecFormatter.php
api/v3/CustomField.php
tests/phpunit/CRM/Core/BAO/CustomFieldTest.php
tests/phpunit/CRM/Core/BAO/CustomValueTableTest.php
tests/phpunit/api/v3/CustomFieldTest.php

index 66871a7d5d8ea88bad7a04fdaaa1186d653607f7..107242a5155c9690ef5d97a3f41eeb7bc6632521 100644 (file)
@@ -250,7 +250,9 @@ class CRM_Core_BAO_CustomField extends CRM_Core_DAO_CustomField {
     // This provides legacy support for APIv3, allowing no-longer-existent html types
     if ($fieldName == 'html_type' && isset($props['version']) && $props['version'] == 3) {
       $options['Multi-Select'] = 'Multi-Select';
+      $options['Select Country'] = 'Select Country';
       $options['Multi-Select Country'] = 'Multi-Select Country';
+      $options['Select State/Province'] = 'Select State/Province';
       $options['Multi-Select State/Province'] = 'Multi-Select State/Province';
     }
     return $options;
@@ -678,11 +680,8 @@ class CRM_Core_BAO_CustomField extends CRM_Core_DAO_CustomField {
 
     $placeholder = $search ? ts('- any -') : ($useRequired ? ts('- select -') : ts('- none -'));
 
-    // FIXME: Why are select state/country separate widget types?
     $isSelect = (in_array($widget, [
       'Select',
-      'Select State/Province',
-      'Select Country',
       'CheckBox',
       'Radio',
     ]));
@@ -1057,8 +1056,6 @@ class CRM_Core_BAO_CustomField extends CRM_Core_DAO_CustomField {
       case 'Select':
       case 'Autocomplete-Select':
       case 'Radio':
-      case 'Select Country':
-      case 'Select State/Province':
       case 'CheckBox':
         if ($field['data_type'] == 'ContactReference' && $value) {
           if (is_numeric($value)) {
index f7d22c826a1d9182fd7a596f98ed7a847640554e..7adcd178c5284f02922ad99e9be3e0acf3c8e8c8 100644 (file)
@@ -174,8 +174,6 @@ class CRM_Core_SelectValues {
       'CheckBox' => ts('Checkbox(es)'),
       'Select Date' => ts('Select Date'),
       'File' => ts('File'),
-      'Select State/Province' => ts('Select State/Province'),
-      'Select Country' => ts('Select Country'),
       'RichTextEditor' => ts('Rich Text Editor'),
       'Autocomplete-Select' => ts('Autocomplete-Select'),
       'Link' => ts('Link'),
index f5e6096c0e6c687804103b6178a78448dd71c5f4..840650e7d9f754ea778b79b4614323529a8cd3fe 100644 (file)
@@ -82,8 +82,8 @@ class CRM_Custom_Form_Field extends CRM_Core_Form {
     ['TextArea' => 'TextArea', 'RichTextEditor' => 'RichTextEditor'],
     ['Date' => 'Select Date'],
     ['Radio' => 'Radio'],
-    ['StateProvince' => 'Select State/Province'],
-    ['Country' => 'Select Country'],
+    ['StateProvince' => 'Select'],
+    ['Country' => 'Select'],
     ['File' => 'File'],
     ['Link' => 'Link'],
     ['ContactReference' => 'Autocomplete-Select'],
@@ -158,8 +158,8 @@ class CRM_Custom_Form_Field extends CRM_Core_Form {
         ['TextArea' => ts('TextArea'), 'RichTextEditor' => ts('Rich Text Editor')],
         ['Date' => ts('Select Date')],
         ['Radio' => ts('Radio')],
-        ['StateProvince' => ts('Select State/Province')],
-        ['Country' => ts('Select Country')],
+        ['StateProvince' => ts('Select')],
+        ['Country' => ts('Select')],
         ['File' => ts('Select File')],
         ['Link' => ts('Link')],
         ['ContactReference' => ts('Autocomplete-Select')],
index 1311d85c3ee6a33a67358cbe73c878af05862d57..a6c92afc62a30a140afef0b4b25c14919080bb05 100644 (file)
@@ -341,8 +341,6 @@ class CRM_Profile_Page_MultipleRecordFieldsListing extends CRM_Core_Page_Basic {
 
                   case 'Radio':
                   case 'Select':
-                  case 'Select Country':
-                  case 'Select State/Province':
                     $editable = TRUE;
                     $fieldAttributes['data-type'] = $spec['data_type'] == 'Boolean' ? 'boolean' : 'select';
                     if (!$spec['is_required']) {
@@ -408,7 +406,7 @@ class CRM_Profile_Page_MultipleRecordFieldsListing extends CRM_Core_Page_Basic {
 
     $headers = [];
     if (!empty($fieldIDs)) {
-      $fields = ['Radio', 'Select', 'Select Country', 'Select State/Province'];
+      $fields = ['Radio', 'Select'];
       foreach ($fieldIDs as $fieldID) {
         if ($this->_pageViewType == 'profileDataView') {
           $headers[$fieldID] = $customGroupInfo[$fieldID]['fieldLabel'];
index aaf47e53be3f88dec628a581a4e4ec8100c684b7..663ae49a267804eb8f38fbc9a24ce949d4faf97d 100644 (file)
@@ -1 +1,5 @@
 {* file to handle db changes in 5.31.alpha1 during upgrade *}
+
+{* Remove Country & State special select fields *}
+UPDATE civicrm_custom_field SET html_type = 'Select'
+WHERE html_type IN ('Select Country', 'Select State/Province');
index 23209a4e242cb5b063f8196c6f0f9aa9146614dc..9e00ba09f59cadadeab394d66f1ea5a12f6bb1dc 100644 (file)
@@ -143,8 +143,6 @@ class SpecFormatter {
     unset($inputAttrs['type']);
 
     $map = [
-      'Select State/Province' => 'Select',
-      'Select Country' => 'Select',
       'Select Date' => 'Date',
       'Link' => 'Url',
     ];
index a9b664b0b59ac2a90952c4614aee5edabd361733..ecfe665fddaeed5624b8a80065ced93ead91b4ca 100644 (file)
 function civicrm_api3_custom_field_create($params) {
 
   // Legacy handling for old way of naming serialized fields
-  if (!empty($params['html_type']) && ($params['html_type'] == 'CheckBox' || strpos($params['html_type'], 'Multi-') === 0)) {
-    $params['serialize'] = 1;
-    $params['html_type'] = str_replace('Multi-', '', $params['html_type']);
+  if (!empty($params['html_type'])) {
+    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']);
   }
 
   // Array created for passing options in params.
@@ -131,7 +133,14 @@ function civicrm_api3_custom_field_get($params) {
     if (!in_array('serialize', $params['return'])) {
       $params['return'][] = 'serialize';
     }
+    if (!in_array('data_type', $params['return'])) {
+      $params['return'][] = 'data_type';
+    }
   }
+  $legacyDataTypes = [
+    'Select State/Province' => 'StateProvince',
+    'Select Country' => 'Country',
+  ];
   if ($handleLegacy && !empty($params['html_type'])) {
     $serializedTypes = ['CheckBox', 'Multi-Select', 'Multi-Select Country', 'Multi-Select State/Province'];
     if (is_string($params['html_type'])) {
@@ -142,12 +151,16 @@ function civicrm_api3_custom_field_get($params) {
       elseif (!in_array($params['html_type'], $serializedTypes)) {
         $params['serialize'] = 0;
       }
+      if (isset($legacyDataTypes[$params['html_type']])) {
+        $params['data_type'] = $legacyDataTypes[$params['html_type']];
+        unset($params['html_type']);
+      }
     }
     elseif (is_array($params['html_type']) && !empty($params['html_type']['IN'])) {
       $excludeNonSerialized = !array_diff($params['html_type']['IN'], $serializedTypes);
       $onlyNonSerialized = !array_intersect($params['html_type']['IN'], $serializedTypes);
       $params['html_type']['IN'] = array_map(function($val) {
-        return str_replace('Multi-Select', 'Select', $val);
+        return str_replace(['Multi-Select', 'Select Country', 'Select State/Province'], 'Select', $val);
       }, $params['html_type']['IN']);
       if ($excludeNonSerialized) {
         $params['serialize'] = 1;
@@ -160,10 +173,15 @@ function civicrm_api3_custom_field_get($params) {
 
   $results = _civicrm_api3_basic_get(_civicrm_api3_get_BAO(__FUNCTION__), $params);
 
-  if ($handleLegacy && !empty($results['values']) && is_array($results['values']) && !isset($params['serialize'])) {
-    foreach ($results['values'] as $id => $result) {
-      if (!empty($result['serialize']) && !empty($result['html_type'])) {
-        $results['values'][$id]['html_type'] = str_replace('Select', 'Multi-Select', $result['html_type']);
+  if ($handleLegacy && !empty($results['values']) && is_array($results['values'])) {
+    foreach ($results['values'] as $id => &$result) {
+      if (!empty($result['html_type'])) {
+        if (in_array($result['data_type'], $legacyDataTypes)) {
+          $result['html_type'] = array_search($result['data_type'], $legacyDataTypes);
+        }
+        if (!empty($result['serialize'])) {
+          $result['html_type'] = str_replace('Select', 'Multi-Select', $result['html_type']);
+        }
       }
     }
   }
index bf3cdf38c8f29bd71e42b332e3becbc3d6dc9b01..6642d75c778218fca7255163ca0d18a4d6b83ad1 100644 (file)
@@ -135,7 +135,7 @@ class CRM_Core_BAO_CustomFieldTest extends CiviUnitTestCase {
     $fieldsToCreate = [
       [
         'data_type' => 'Country',
-        'html_type' => 'Select Country',
+        'html_type' => 'Select',
         'tests' => [
           'United States' => 1228,
           '' => NULL,
@@ -143,7 +143,8 @@ class CRM_Core_BAO_CustomFieldTest extends CiviUnitTestCase {
       ],
       [
         'data_type' => 'StateProvince',
-        'html_type' => 'Multi-Select State/Province',
+        'html_type' => 'Select',
+        'serialize' => 1,
         'tests' => [
           '' => 0,
           'Alabama' => 1000,
@@ -296,21 +297,21 @@ class CRM_Core_BAO_CustomFieldTest extends CiviUnitTestCase {
       'custom_group_id' => $groups['A']['id'],
       'label' => 'Country A',
       'dataType' => 'Country',
-      'htmlType' => 'Select Country',
+      'htmlType' => 'Select',
       'default_value' => NULL,
     ]);
     $countryB = $this->customFieldCreate([
       'custom_group_id' => $groups['A']['id'],
       'label' => 'Country B',
       'dataType' => 'Country',
-      'htmlType' => 'Select Country',
+      'htmlType' => 'Select',
       'default_value' => NULL,
     ]);
     $countryC = $this->customFieldCreate([
       'custom_group_id' => $groups['B']['id'],
       'label' => 'Country C',
       'dataType' => 'Country',
-      'htmlType' => 'Select Country',
+      'htmlType' => 'Select',
       'default_value' => NULL,
     ]);
 
@@ -460,7 +461,7 @@ class CRM_Core_BAO_CustomFieldTest extends CiviUnitTestCase {
         'options_per_line' => NULL,
         'text_length' => NULL,
         'data_type' => 'Country',
-        'html_type' => 'Select Country',
+        'html_type' => 'Select',
         'is_search_range' => '0',
         'id' => $this->getCustomFieldID('country'),
         'label' => 'Country',
@@ -499,7 +500,7 @@ class CRM_Core_BAO_CustomFieldTest extends CiviUnitTestCase {
         'options_per_line' => NULL,
         'text_length' => NULL,
         'data_type' => 'Country',
-        'html_type' => 'Select Country',
+        'html_type' => 'Select',
         'is_search_range' => '0',
         'id' => $this->getCustomFieldID('multi_country'),
         'label' => 'Country-multi',
@@ -797,7 +798,7 @@ class CRM_Core_BAO_CustomFieldTest extends CiviUnitTestCase {
         'import' => 1,
         'data_type' => 'StateProvince',
         'type' => 1,
-        'html_type' => 'Select State/Province',
+        'html_type' => 'Select',
         'text_length' => NULL,
         'options_per_line' => NULL,
         'is_search_range' => '0',
@@ -835,7 +836,7 @@ class CRM_Core_BAO_CustomFieldTest extends CiviUnitTestCase {
         'data_type' => 'StateProvince',
         'name' => $this->getCustomFieldName('multi_state'),
         'type' => 1,
-        'html_type' => 'Select State/Province',
+        'html_type' => 'Select',
         'text_length' => NULL,
         'options_per_line' => NULL,
         'is_search_range' => '0',
index 580df3134dfe958a899be8d0725e2a87bf90f5e1..a4d51bdc0feedd270cf232fba0e67d50e96382b6 100644 (file)
@@ -89,7 +89,7 @@ class CRM_Core_BAO_CustomValueTableTest extends CiviUnitTestCase {
     $fields = [
       'custom_group_id' => $customGroup['id'],
       'data_type' => 'StateProvince',
-      'html_type' => 'Select State/Province',
+      'html_type' => 'Select',
       'default_value' => '',
     ];
 
index 8fc16468a1df084a3be4a4ac776da6cf1df8199a..e5c900dd245f515bb0a1d53914c4c696a463ce39 100644 (file)
@@ -107,6 +107,16 @@ class api_v3_CustomFieldTest extends CiviUnitTestCase {
     $dtype = CRM_Core_BAO_CustomField::dataType();
     $htype = CRM_Custom_Form_Field::$_dataToHTML;
 
+    // Legacy html types returned by v3
+    foreach ($htype as &$item) {
+      if (isset($item['StateProvince'])) {
+        $item['StateProvince'] = 'Select State/Province';
+      }
+      if (isset($item['Country'])) {
+        $item['Country'] = 'Select Country';
+      }
+    }
+
     $n = 0;
     foreach ($dtype as $dkey => $dvalue) {
       foreach ($htype[$n] as $hkey => $hvalue) {
@@ -678,4 +688,77 @@ class api_v3_CustomFieldTest extends CiviUnitTestCase {
     $this->assertEquals('SingleSelect', $result['values'][0]['label']);
   }
 
+  public function testLegacyStateCountryTypes() {
+    $customGroup = $this->customGroupCreate([
+      'name' => 'testCustomGroup',
+      'title' => 'testCustomGroup',
+      'extends' => 'Individual',
+    ]);
+    $f1 = $this->callAPISuccess('CustomField', 'create', [
+      'label' => 'CountrySelect',
+      'custom_group_id' => 'testCustomGroup',
+      'data_type' => 'Country',
+      'html_type' => 'Select Country',
+    ]);
+    $f2 = $this->callAPISuccess('CustomField', 'create', [
+      'label' => 'StateSelect',
+      'custom_group_id' => 'testCustomGroup',
+      'data_type' => 'StateProvince',
+      'html_type' => 'Select State/Province',
+    ]);
+    $f3 = $this->callAPISuccess('CustomField', 'create', [
+      'label' => 'MultiSelectSP',
+      'custom_group_id' => 'testCustomGroup',
+      'data_type' => 'StateProvince',
+      'html_type' => 'Multi-Select State/Province',
+    ]);
+    $f4 = $this->callAPISuccess('CustomField', 'create', [
+      'label' => 'MultiSelectCountry',
+      'custom_group_id' => 'testCustomGroup',
+      'data_type' => 'Country',
+      'html_type' => 'Select Country',
+      'serialize' => 1,
+    ]);
+
+    $result = $this->callAPISuccess('CustomField', 'get', [
+      'custom_group_id' => 'testCustomGroup',
+      'html_type' => 'Multi-Select State/Province',
+      'sequential' => 1,
+    ]);
+    $this->assertCount(1, $result['values']);
+    $this->assertEquals('MultiSelectSP', $result['values'][0]['label']);
+    $this->assertEquals('Multi-Select State/Province', $result['values'][0]['html_type']);
+    $this->assertEquals('1', $result['values'][0]['serialize']);
+
+    $result = $this->callAPISuccess('CustomField', 'get', [
+      'custom_group_id' => 'testCustomGroup',
+      'html_type' => 'Multi-Select Country',
+      'sequential' => 1,
+    ]);
+    $this->assertCount(1, $result['values']);
+    $this->assertEquals('MultiSelectCountry', $result['values'][0]['label']);
+    $this->assertEquals('Multi-Select Country', $result['values'][0]['html_type']);
+    $this->assertEquals('1', $result['values'][0]['serialize']);
+
+    $result = $this->callAPISuccess('CustomField', 'get', [
+      'custom_group_id' => 'testCustomGroup',
+      'html_type' => 'Select Country',
+      'sequential' => 1,
+    ]);
+    $this->assertCount(1, $result['values']);
+    $this->assertEquals('CountrySelect', $result['values'][0]['label']);
+    $this->assertEquals('Select Country', $result['values'][0]['html_type']);
+    $this->assertEquals('0', $result['values'][0]['serialize']);
+
+    $result = $this->callAPISuccess('CustomField', 'get', [
+      'custom_group_id' => 'testCustomGroup',
+      'html_type' => 'Select State/Province',
+      'sequential' => 1,
+    ]);
+    $this->assertCount(1, $result['values']);
+    $this->assertEquals('StateSelect', $result['values'][0]['label']);
+    $this->assertEquals('Select State/Province', $result['values'][0]['html_type']);
+    $this->assertEquals('0', $result['values'][0]['serialize']);
+  }
+
 }