From 65d27ad8593afe910ae8aa6c6795c64a7ba97eb8 Mon Sep 17 00:00:00 2001 From: eileen Date: Fri, 4 Dec 2020 17:22:29 +1300 Subject: [PATCH] dev/core#866, dev/core#1318 Fix failure to import checkboxes for activities This seeks to address a long-standing issue whereby checkboxes cannot be imported onto activities if the name and value match. Since this code was written the create was switched from a BAO create to the activity api create. Simply removing the handling to prepare for the BAO create allows the api to do it correctly --- CRM/Activity/Import/Parser/Activity.php | 29 ----- CRM/Core/Session.php | 4 +- api/v3/CustomField.php | 3 + .../Activity/Import/Parser/ActivityTest.php | 108 ++++++++++++++++++ .../phpunit/CRM/Core/BAO/CustomFieldTest.php | 40 ++++++- .../CRMTraits/Custom/CustomDataTrait.php | 70 ++++++++++-- 6 files changed, 212 insertions(+), 42 deletions(-) create mode 100644 tests/phpunit/CRM/Activity/Import/Parser/ActivityTest.php diff --git a/CRM/Activity/Import/Parser/Activity.php b/CRM/Activity/Import/Parser/Activity.php index e38432cff8..86186d256c 100644 --- a/CRM/Activity/Import/Parser/Activity.php +++ b/CRM/Activity/Import/Parser/Activity.php @@ -278,11 +278,6 @@ class CRM_Activity_Import_Parser_Activity extends CRM_Activity_Import_Parser { return CRM_Import_Parser::ERROR; } - $params['custom'] = CRM_Core_BAO_CustomField::postProcess($params, - NULL, - 'Activity' - ); - if ($this->_contactIdIndex < 0) { // Retrieve contact id using contact dedupe rule. @@ -393,36 +388,12 @@ class CRM_Activity_Import_Parser_Activity extends CRM_Activity_Import_Parser { $fields = CRM_Activity_DAO_Activity::fields(); _civicrm_api3_store_values($fields, $params, $values); - require_once 'CRM/Core/OptionGroup.php'; - $customFields = CRM_Core_BAO_CustomField::getFields('Activity'); - foreach ($params as $key => $value) { // ignore empty values or empty arrays etc if (CRM_Utils_System::isNull($value)) { continue; } - //Handling Custom Data - if ($customFieldID = CRM_Core_BAO_CustomField::getKeyID($key)) { - $values[$key] = $value; - $type = $customFields[$customFieldID]['html_type']; - if (CRM_Core_BAO_CustomField::isSerialized($customFields[$customFieldID])) { - $values[$key] = CRM_Import_Parser::unserializeCustomValue($customFieldID, $value, $type); - } - elseif ($type == 'Select' || $type == 'Radio') { - $customOption = CRM_Core_BAO_CustomOption::getCustomOption($customFieldID, TRUE); - foreach ($customOption as $customFldID => $customValue) { - $val = $customValue['value'] ?? NULL; - $label = $customValue['label'] ?? NULL; - $label = strtolower($label); - $value = strtolower(trim($value)); - if (($value == $label) || ($value == strtolower($val))) { - $values[$key] = $val; - } - } - } - } - if ($key == 'target_contact_id') { if (!CRM_Utils_Rule::integer($value)) { return civicrm_api3_create_error("contact_id not valid: $value"); diff --git a/CRM/Core/Session.php b/CRM/Core/Session.php index 037efb1115..7ca6a46ead 100644 --- a/CRM/Core/Session.php +++ b/CRM/Core/Session.php @@ -52,9 +52,9 @@ class CRM_Core_Session { * We only need one instance of this object. So we use the singleton * pattern and cache the instance in this variable * - * @var object + * @var \CRM_Core_Session */ - static private $_singleton = NULL; + static private $_singleton; /** * Constructor. diff --git a/api/v3/CustomField.php b/api/v3/CustomField.php index 346a1ec2be..f8652419be 100644 --- a/api/v3/CustomField.php +++ b/api/v3/CustomField.php @@ -69,6 +69,9 @@ function civicrm_api3_custom_field_create($params) { * Flush static caches in functions that might have stored available custom fields. */ function _civicrm_api3_custom_field_flush_static_caches() { + if (isset(\Civi::$statics['CRM_Core_BAO_OptionGroup']['titles_by_name'])) { + unset(\Civi::$statics['CRM_Core_BAO_OptionGroup']['titles_by_name']); + } civicrm_api('CustomField', 'getfields', ['version' => 3, 'cache_clear' => 1]); CRM_Core_BAO_UFField::getAvailableFieldsFlat(TRUE); } diff --git a/tests/phpunit/CRM/Activity/Import/Parser/ActivityTest.php b/tests/phpunit/CRM/Activity/Import/Parser/ActivityTest.php new file mode 100644 index 0000000000..99df179f30 --- /dev/null +++ b/tests/phpunit/CRM/Activity/Import/Parser/ActivityTest.php @@ -0,0 +1,108 @@ +. + */ + +/** + * Test CRM/Member/BAO Membership Log add , delete functions + * + * @package CiviCRM + * @group headless + */ +class CRM_Activity_Import_Parser_ActivityTest extends CiviUnitTestCase { + use CRMTraits_Custom_CustomDataTrait; + + /** + * Prepare for tests. + */ + public function setUp():void { + parent::setUp(); + $this->createLoggedInUser(); + } + + /** + * Clean up after test. + * + * @throws \CRM_Core_Exception + */ + public function tearDown():void { + $this->quickCleanup(['civicrm_contact', 'civicrm_activity'], TRUE); + parent::tearDown(); + } + + /** + * Test Import. + * + * So far this is just testing the class constructor & preparing for more + * tests. + * + * @throws \API_Exception + * @throws \CRM_Core_Exception + * @throws \CiviCRM_API3_Exception + */ + public function testImport(): void { + $this->createCustomGroupWithFieldOfType(['extends' => 'Activity'], 'checkbox'); + $values = [ + 'activity_detail' => 'fascinating', + 'activity_type_id' => 1, + 'activity_date_time' => '2010-01-06', + 'target_contact_id' => $this->individualCreate(), + 'subject' => 'riveting stuff', + $this->getCustomFieldName('checkbox') => 'L', + ]; + $this->importValues($values); + $this->callAPISuccessGetSingle('Activity', [$this->getCustomFieldName('checkbox') => 'L']); + } + + /** + * Create an import object. + * + * @param array $fields + * + * @return \CRM_Activity_Import_Parser_Activity + */ + protected function createImportObject(array $fields): \CRM_Activity_Import_Parser_Activity { + $fieldMapper = []; + foreach ($fields as $index => $field) { + $fieldMapper[] = $field; + } + $importer = new CRM_Activity_Import_Parser_Activity($fieldMapper); + $importer->init(); + return $importer; + } + + /** + * Run the importer. + * + * @param array $values + * @param int $expectedOutcome + */ + protected function importValues(array $values, $expectedOutcome = 1): void { + $importer = $this->createImportObject(array_keys($values)); + $params = array_values($values); + CRM_Core_Session::singleton()->set('dateTypes', 1); + $this->assertEquals($expectedOutcome, $importer->import(NULL, $params)); + } + +} diff --git a/tests/phpunit/CRM/Core/BAO/CustomFieldTest.php b/tests/phpunit/CRM/Core/BAO/CustomFieldTest.php index 8ea4dad0cd..8f543c95d0 100644 --- a/tests/phpunit/CRM/Core/BAO/CustomFieldTest.php +++ b/tests/phpunit/CRM/Core/BAO/CustomFieldTest.php @@ -618,7 +618,7 @@ class CRM_Core_BAO_CustomFieldTest extends CiviUnitTestCase { 'extends_entity_column_id' => NULL, 'is_view' => '0', 'is_multiple' => '0', - 'option_group_id' => $this->callAPISuccessGetValue('CustomField', ['id' => $this->getCustomFieldID('select_string'), 'return' => 'option_group_id']), + 'option_group_id' => $this->getOptionGroupID('select_string'), 'date_format' => NULL, 'time_format' => NULL, 'is_required' => 0, @@ -877,6 +877,44 @@ class CRM_Core_BAO_CustomFieldTest extends CiviUnitTestCase { 'callback' => 'CRM_Core_SelectValues::boolean', ], ], + $this->getCustomFieldName('checkbox') => [ + 'name' => $this->getCustomFieldName('checkbox'), + 'custom_field_id' => $this->getCustomFieldID('checkbox'), + 'id' => $this->getCustomFieldID('checkbox'), + 'groupTitle' => 'Custom Group', + 'default_value' => NULL, + 'option_group_id' => $this->getOptionGroupID('checkbox'), + 'custom_group_id' => $customGroupID, + 'extends' => 'Contact', + 'extends_entity_column_value' => NULL, + 'extends_entity_column_id' => NULL, + 'is_view' => '0', + 'is_multiple' => '0', + 'date_format' => NULL, + 'time_format' => NULL, + 'is_required' => 0, + 'table_name' => 'civicrm_value_custom_group_' . $customGroupID, + 'column_name' => $this->getCustomFieldColumnName('checkbox'), + 'where' => 'civicrm_value_custom_group_' . $customGroupID . '.' . $this->getCustomFieldColumnName('checkbox'), + 'extends_table' => 'civicrm_contact', + 'search_table' => 'contact_a', + 'import' => 1, + 'label' => 'Pick Shade', + 'headerPattern' => '//', + 'title' => 'Pick Shade', + 'data_type' => 'String', + 'type' => 2, + 'html_type' => 'CheckBox', + 'text_length' => NULL, + 'options_per_line' => NULL, + 'is_search_range' => '0', + 'serialize' => '1', + 'pseudoconstant' => [ + 'optionGroupName' => $this->getOptionGroupName('checkbox'), + 'optionEditPath' => 'civicrm/admin/options/' . $this->getOptionGroupName('checkbox'), + + ], + ], ]; $this->assertEquals($expected, CRM_Core_BAO_CustomField::getFieldsForImport()); } diff --git a/tests/phpunit/CRMTraits/Custom/CustomDataTrait.php b/tests/phpunit/CRMTraits/Custom/CustomDataTrait.php index 1e9ef5e4bf..95cbb922f5 100644 --- a/tests/phpunit/CRMTraits/Custom/CustomDataTrait.php +++ b/tests/phpunit/CRMTraits/Custom/CustomDataTrait.php @@ -92,10 +92,9 @@ trait CRMTraits_Custom_CustomDataTrait { * * @throws \API_Exception * @throws \CRM_Core_Exception - * @throws \Civi\API\Exception\UnauthorizedException */ - public function createCustomGroupWithFieldOfType($groupParams = [], $customFieldType = 'text', $identifier = NULL, $fieldParams = []) { - $supported = ['text', 'select', 'date', 'int', 'contact_reference', 'radio', 'multi_country']; + public function createCustomGroupWithFieldOfType($groupParams = [], $customFieldType = 'text', $identifier = NULL, $fieldParams = []): void { + $supported = ['text', 'select', 'date', 'checkbox', 'int', 'contact_reference', 'radio', 'multi_country']; if (!in_array($customFieldType, $supported, TRUE)) { throw new CRM_Core_Exception('we have not yet extracted other custom field types from createCustomFieldsOfAllTypes, Use consistent syntax when you do'); } @@ -113,6 +112,10 @@ trait CRMTraits_Custom_CustomDataTrait { $reference = $this->createSelectCustomField($fieldParams)['id']; return; + case 'checkbox': + $reference = $this->createStringCheckboxCustomField($fieldParams)['id']; + return; + case 'int': $reference = $this->createIntCustomField($fieldParams)['id']; return; @@ -154,6 +157,7 @@ trait CRMTraits_Custom_CustomDataTrait { $ids['state'] = (int) $this->createStateCustomField(['custom_group_id' => $customGroupID])['id']; $ids['multi_state'] = (int) $this->createMultiStateCustomField(['custom_group_id' => $customGroupID])['id']; $ids['boolean'] = (int) $this->createBooleanCustomField(['custom_group_id' => $customGroupID])['id']; + $ids['checkbox'] = (int) $this->createStringCheckboxCustomField(['custom_group_id' => $customGroupID])['id']; return $ids; } @@ -206,6 +210,34 @@ trait CRMTraits_Custom_CustomDataTrait { return $this->ids['CustomField'][$key]; } + /** + * Get the option group id of the created field. + * + * @param string $key + * + * @return string + */ + protected function getOptionGroupID(string $key): string { + return (string) $this->callAPISuccessGetValue('CustomField', [ + 'id' => $this->getCustomFieldID($key), + 'return' => 'option_group_id', + ]); + } + + /** + * Get the option group id of the created field. + * + * @param string $key + * + * @return string + */ + protected function getOptionGroupName(string $key): string { + return (string) $this->callAPISuccessGetValue('CustomField', [ + 'id' => $this->getCustomFieldID($key), + 'return' => 'option_group_id.name', + ]); + } + /** * Create a custom text fields. * @@ -374,6 +406,18 @@ trait CRMTraits_Custom_CustomDataTrait { return $this->callAPISuccess('custom_field', 'create', $params)['values'][0]; } + /** + * Create a custom field of type radio with integer values. + * + * @param array $params + * + * @return array + */ + protected function createStringCheckboxCustomField(array $params): array { + $params = array_merge($this->getFieldsValuesByType('String', 'CheckBox'), $params); + return $this->callAPISuccess('custom_field', 'create', $params)['values'][0]; + } + /** * Create a custom field of type radio with integer values. * @@ -480,30 +524,36 @@ trait CRMTraits_Custom_CustomDataTrait { ], ], 'CheckBox' => [ - 'label' => 'Pick Color', + 'label' => 'Pick Shade', 'html_type' => 'CheckBox', 'data_type' => 'String', 'text_length' => '', 'default_value' => '', 'option_values' => [ [ - 'label' => 'Red', - 'value' => 'R', + 'label' => 'Lilac', + 'value' => 'L', 'weight' => 1, 'is_active' => 1, ], [ - 'label' => 'Yellow', - 'value' => 'Y', + 'label' => 'Purple', + 'value' => 'P', 'weight' => 2, 'is_active' => 1, ], [ - 'label' => 'Green', - 'value' => 'G', + 'label' => 'Mauve', + 'value' => 'M', 'weight' => 3, 'is_active' => 1, ], + [ + 'label' => 'Violet', + 'value' => 'V', + 'weight' => 4, + 'is_active' => 1, + ], ], ], 'Multi-Select' => [ -- 2.25.1