From b362f4864edc846878aa7c99ba9bf91d550185d6 Mon Sep 17 00:00:00 2001 From: Coleman Watts Date: Tue, 11 Apr 2023 22:14:36 -0400 Subject: [PATCH] Make civicrm_mapping.name required and fix index to be unique --- CRM/Admin/Form/Mapping.php | 2 +- CRM/Contact/BAO/Group.php | 5 ++- CRM/Contact/Form/Task/SaveSearch.php | 4 +- CRM/Core/BAO/Mapping.php | 25 +++++------ CRM/Core/DAO/Mapping.php | 12 +++--- CRM/Upgrade/Incremental/php/FiveSixtyTwo.php | 3 ++ .../Incremental/sql/5.62.alpha1.mysql.tpl | 6 +++ .../Provider/MappingCreationSpecProvider.php | 42 ------------------- .../SavedSearchDataSets/242_email_on_hold.sql | 2 +- .../801_pre44_billing_email_smartgroup.sql | 2 +- .../94_legacy_state_province.sql | 2 +- .../primary_multiple_state_province.sql | 2 +- .../Contact/Import/Form/DataSourceTest.php | 2 +- xml/schema/Core/Mapping.xml | 4 +- 14 files changed, 41 insertions(+), 72 deletions(-) delete mode 100644 Civi/Api4/Service/Spec/Provider/MappingCreationSpecProvider.php diff --git a/CRM/Admin/Form/Mapping.php b/CRM/Admin/Form/Mapping.php index bf72f47c1d..3eaf5acce8 100644 --- a/CRM/Admin/Form/Mapping.php +++ b/CRM/Admin/Form/Mapping.php @@ -92,7 +92,7 @@ class CRM_Admin_Form_Mapping extends CRM_Admin_Form { $params['id'] = $this->_id; } - CRM_Core_BAO_Mapping::add($params); + CRM_Core_BAO_Mapping::writeRecord($params); } } diff --git a/CRM/Contact/BAO/Group.php b/CRM/Contact/BAO/Group.php index 245f183f2f..89ec6bd5fc 100644 --- a/CRM/Contact/BAO/Group.php +++ b/CRM/Contact/BAO/Group.php @@ -656,9 +656,10 @@ class CRM_Contact_BAO_Group extends CRM_Contact_DAO_Group { if (!$ssId) { //save record in mapping table $mappingParams = [ + 'name' => 'search_builder_' . $ssId, 'mapping_type_id' => CRM_Core_PseudoConstant::getKey('CRM_Core_BAO_Mapping', 'mapping_type_id', 'Search Builder'), ]; - $mapping = CRM_Core_BAO_Mapping::add($mappingParams); + $mapping = CRM_Core_BAO_Mapping::writeRecord($mappingParams); $mappingId = $mapping->id; } else { @@ -714,7 +715,7 @@ class CRM_Contact_BAO_Group extends CRM_Contact_DAO_Group { 'description' => CRM_Core_DAO::getFieldValue('CRM_Contact_DAO_Group', $smartGroupId, 'description', 'id'), 'mapping_type_id' => CRM_Core_PseudoConstant::getKey('CRM_Core_BAO_Mapping', 'mapping_type_id', 'Search Builder'), ]; - CRM_Core_BAO_Mapping::add($mappingParams); + CRM_Core_BAO_Mapping::writeRecord($mappingParams); } return [$smartGroupId, $ssId]; diff --git a/CRM/Contact/Form/Task/SaveSearch.php b/CRM/Contact/Form/Task/SaveSearch.php index f843f69ecb..8901ee49a4 100644 --- a/CRM/Contact/Form/Task/SaveSearch.php +++ b/CRM/Contact/Form/Task/SaveSearch.php @@ -145,7 +145,7 @@ class CRM_Contact_Form_Task_SaveSearch extends CRM_Contact_Form_Task { $mappingParams = [ 'mapping_type_id' => CRM_Core_PseudoConstant::getKey('CRM_Core_BAO_Mapping', 'mapping_type_id', 'Search Builder'), ]; - $mapping = CRM_Core_BAO_Mapping::add($mappingParams); + $mapping = CRM_Core_BAO_Mapping::writeRecord($mappingParams); $mappingId = $mapping->id; } else { @@ -220,7 +220,7 @@ class CRM_Contact_Form_Task_SaveSearch extends CRM_Contact_Form_Task { 'description' => CRM_Core_DAO::getFieldValue('CRM_Contact_DAO_Group', $group->id, 'description', 'id'), 'mapping_type_id' => CRM_Core_PseudoConstant::getKey('CRM_Core_BAO_Mapping', 'mapping_type_id', 'Search Builder'), ]; - CRM_Core_BAO_Mapping::add($mappingParams); + CRM_Core_BAO_Mapping::writeRecord($mappingParams); } // CRM-9464 diff --git a/CRM/Core/BAO/Mapping.php b/CRM/Core/BAO/Mapping.php index 07abc10379..62bde83761 100644 --- a/CRM/Core/BAO/Mapping.php +++ b/CRM/Core/BAO/Mapping.php @@ -41,24 +41,14 @@ class CRM_Core_BAO_Mapping extends CRM_Core_DAO_Mapping implements \Civi\Core\Ho } /** - * Takes an associative array and creates a contact object. - * - * The function extract all the params it needs to initialize the create a - * contact object. the params array could contain additional unused name/value - * pairs + * @deprecated * * @param array $params - * An array of name/value pairs. - * - * @return object - * CRM_Core_DAO_Mapper object on success, otherwise NULL + * @return CRM_Core_DAO_Mapping */ public static function add($params) { - $mapping = new CRM_Core_DAO_Mapping(); - $mapping->copyValues($params); - $mapping->save(); - - return $mapping; + CRM_Core_Error::deprecatedFunctionWarning('writeRecord'); + return self::writeRecord($params); } /** @@ -795,6 +785,13 @@ class CRM_Core_BAO_Mapping extends CRM_Core_DAO_Mapping implements \Civi\Core\Ho ->addWhere('relationship_type_id', '=', $event->id) ->execute(); } + if ($event->action === 'create' && $event->entity === 'Mapping') { + if (CRM_Utils_System::isNull($event->params['name'] ?? NULL)) { + CRM_Core_Error::deprecatedWarning('Creating a mapping with no name is deprecated.'); + $id = 1 + CRM_Core_DAO::singleValueQuery('SELECT MAX(id) FROM civicrm_mapping'); + $event->params['name'] = "mapping_$id"; + } + } } } diff --git a/CRM/Core/DAO/Mapping.php b/CRM/Core/DAO/Mapping.php index 2f60c6d5a1..af4aede449 100644 --- a/CRM/Core/DAO/Mapping.php +++ b/CRM/Core/DAO/Mapping.php @@ -6,7 +6,7 @@ * * Generated from xml/schema/CRM/Core/Mapping.xml * DO NOT EDIT. Generated by CRM_Core_CodeGen - * (GenCodeChecksum:89c538397697d0633134470f9e8ac880) + * (GenCodeChecksum:aed252d8a2aaf3cc0cf287903d8197e8) */ /** @@ -47,9 +47,9 @@ class CRM_Core_DAO_Mapping extends CRM_Core_DAO { public $id; /** - * Name of Mapping + * Unique name of Mapping * - * @var string|null + * @var string * (SQL type: varchar(64)) * Note that values will be retrieved from the database as a string. */ @@ -126,7 +126,8 @@ class CRM_Core_DAO_Mapping extends CRM_Core_DAO { 'name' => 'name', 'type' => CRM_Utils_Type::T_STRING, 'title' => ts('Mapping Name'), - 'description' => ts('Name of Mapping'), + 'description' => ts('Unique name of Mapping'), + 'required' => TRUE, 'maxlength' => 64, 'size' => CRM_Utils_Type::BIG, 'usage' => [ @@ -263,7 +264,8 @@ class CRM_Core_DAO_Mapping extends CRM_Core_DAO { 0 => 'name', ], 'localizable' => FALSE, - 'sig' => 'civicrm_mapping::0::name', + 'unique' => TRUE, + 'sig' => 'civicrm_mapping::1::name', ], ]; return ($localize && !empty($indices)) ? CRM_Core_DAO_AllCoreTables::multilingualize(__CLASS__, $indices) : $indices; diff --git a/CRM/Upgrade/Incremental/php/FiveSixtyTwo.php b/CRM/Upgrade/Incremental/php/FiveSixtyTwo.php index cd2b513cdd..2678ee99de 100644 --- a/CRM/Upgrade/Incremental/php/FiveSixtyTwo.php +++ b/CRM/Upgrade/Incremental/php/FiveSixtyTwo.php @@ -44,6 +44,9 @@ class CRM_Upgrade_Incremental_php_FiveSixtyTwo extends CRM_Upgrade_Incremental_B $this->addTask('Make civicrm_setting.domain_id optional', 'alterColumn', 'civicrm_setting', 'domain_id', "int unsigned DEFAULT NULL COMMENT 'Which Domain does this setting belong to'"); $this->addTask('Consolidate the list of components', 'consolidateComponents'); $this->addTask(ts('Upgrade DB to %1: SQL', [1 => $rev]), 'runSql', $rev); + $this->addTask('Make civicrm_mapping.name required', 'alterColumn', 'civicrm_mapping', 'name', "varchar(64) NOT NULL COMMENT 'Unique name of Mapping'"); + $this->addTask(ts('Drop index %1', [1 => 'civicrm_mapping.UI_name']), 'dropIndex', 'civicrm_mapping', 'UI_name'); + $this->addTask(ts('Create index %1', [1 => 'civicrm_mapping.UI_name']), 'addIndex', 'civicrm_mapping', [['name']], 'UI'); $this->addTask( 'Add option group for file_type_id in file table', diff --git a/CRM/Upgrade/Incremental/sql/5.62.alpha1.mysql.tpl b/CRM/Upgrade/Incremental/sql/5.62.alpha1.mysql.tpl index c07509a60c..97d48e8774 100644 --- a/CRM/Upgrade/Incremental/sql/5.62.alpha1.mysql.tpl +++ b/CRM/Upgrade/Incremental/sql/5.62.alpha1.mysql.tpl @@ -1 +1,7 @@ {* file to handle db changes in 5.62.alpha1 during upgrade *} + +{* https://github.com/civicrm/civicrm-core/pull/26055 *} +UPDATE civicrm_mapping SET name = CONCAT('mapping_', id) WHERE name IS NULL; +UPDATE civicrm_mapping m1, civicrm_mapping m2 +SET m2.name = CONCAT(m2.name, '_', m2.id) +WHERE m2.name = m1.name AND m2.id > m1.id; diff --git a/Civi/Api4/Service/Spec/Provider/MappingCreationSpecProvider.php b/Civi/Api4/Service/Spec/Provider/MappingCreationSpecProvider.php deleted file mode 100644 index 97cbd89b3b..0000000000 --- a/Civi/Api4/Service/Spec/Provider/MappingCreationSpecProvider.php +++ /dev/null @@ -1,42 +0,0 @@ -getFieldByName('name')->setRequired(TRUE); - } - - /** - * @param string $entity - * @param string $action - * - * @return bool - */ - public function applies($entity, $action) { - return strpos($entity, 'Mapping') === 0 && $action === 'create'; - } - -} diff --git a/tests/phpunit/CRM/Contact/BAO/SavedSearchDataSets/242_email_on_hold.sql b/tests/phpunit/CRM/Contact/BAO/SavedSearchDataSets/242_email_on_hold.sql index ab73e404d0..ec65882e38 100644 --- a/tests/phpunit/CRM/Contact/BAO/SavedSearchDataSets/242_email_on_hold.sql +++ b/tests/phpunit/CRM/Contact/BAO/SavedSearchDataSets/242_email_on_hold.sql @@ -1,4 +1,4 @@ -INSERT INTO `civicrm_mapping` (`id`, `name`, `description`, `mapping_type_id`) VALUES (149, NULL, NULL, NULL); +INSERT INTO `civicrm_mapping` (`id`, `name`, `description`, `mapping_type_id`) VALUES (149, 'test_149', NULL, NULL); INSERT INTO `civicrm_mapping_field` (`id`, `mapping_id`, `name`, `contact_type`, `column_number`, `location_type_id`, `phone_type_id`, `im_provider_id`, `relationship_type_id`, `relationship_direction`, `grouping`, `operator`, `value`, `website_type_id`) VALUES (3538, 149, 'email', 'Individual', 0, NULL, NULL, NULL, NULL, NULL, 1, '', '', NULL); INSERT INTO `civicrm_mapping_field` (`id`, `mapping_id`, `name`, `contact_type`, `column_number`, `location_type_id`, `phone_type_id`, `im_provider_id`, `relationship_type_id`, `relationship_direction`, `grouping`, `operator`, `value`, `website_type_id`) VALUES (3539, 149, 'on_hold', 'Individual', 1, NULL, NULL, NULL, NULL, NULL, 1, '=', '1', NULL); diff --git a/tests/phpunit/CRM/Contact/BAO/SavedSearchDataSets/801_pre44_billing_email_smartgroup.sql b/tests/phpunit/CRM/Contact/BAO/SavedSearchDataSets/801_pre44_billing_email_smartgroup.sql index 0a5b83a600..48171d8d9b 100644 --- a/tests/phpunit/CRM/Contact/BAO/SavedSearchDataSets/801_pre44_billing_email_smartgroup.sql +++ b/tests/phpunit/CRM/Contact/BAO/SavedSearchDataSets/801_pre44_billing_email_smartgroup.sql @@ -1,4 +1,4 @@ -INSERT IGNORE INTO `civicrm_mapping` (`id`, `name`, `description`, `mapping_type_id`) VALUES (315, NULL, NULL, NULL); +INSERT IGNORE INTO `civicrm_mapping` (`id`, `name`, `description`, `mapping_type_id`) VALUES (315, 'test_315', NULL, NULL); INSERT IGNORE INTO `civicrm_mapping_field` (`id`, `mapping_id`, `name`, `contact_type`, `column_number`, `location_type_id`, `phone_type_id`, `im_provider_id`, `relationship_type_id`, `relationship_direction`, `grouping`, `operator`, `value`, `website_type_id`) VALUES (7455, 315, 'email', 'Individual', 0, 5, NULL, NULL, NULL, NULL, 1, 'IS NOT EMPTY', '', NULL); INSERT IGNORE INTO `civicrm_mapping_field` (`id`, `mapping_id`, `name`, `contact_type`, `column_number`, `location_type_id`, `phone_type_id`, `im_provider_id`, `relationship_type_id`, `relationship_direction`, `grouping`, `operator`, `value`, `website_type_id`) VALUES (7456, 315, 'do_not_email', 'Individual', 1, NULL, NULL, NULL, NULL, NULL, 1, '=', '0', NULL); diff --git a/tests/phpunit/CRM/Contact/BAO/SavedSearchDataSets/94_legacy_state_province.sql b/tests/phpunit/CRM/Contact/BAO/SavedSearchDataSets/94_legacy_state_province.sql index b311a84eda..107518f404 100644 --- a/tests/phpunit/CRM/Contact/BAO/SavedSearchDataSets/94_legacy_state_province.sql +++ b/tests/phpunit/CRM/Contact/BAO/SavedSearchDataSets/94_legacy_state_province.sql @@ -1,5 +1,5 @@ INSERT INTO civicrm_mapping (id,name,description,mapping_type_id) Values -(113, NULL, NULL, NULL); +(113, 'test_113', NULL, NULL); INSERT INTO `civicrm_mapping_field` (`id`, `mapping_id`, `name`, `contact_type`, `column_number`, `location_type_id`, `phone_type_id`, `im_provider_id`, `relationship_type_id`, `relationship_direction`, `grouping`, `operator`, `value`, `website_type_id`) VALUES (2841, 113, 'email', 'Individual', 0, NULL, NULL, NULL, NULL, NULL, 1, '', '', NULL), (2842, 113, 'do_not_email', 'Individual', 1, NULL, NULL, NULL, NULL, NULL, 1, '!=', '1', NULL), diff --git a/tests/phpunit/CRM/Contact/BAO/SavedSearchDataSets/primary_multiple_state_province.sql b/tests/phpunit/CRM/Contact/BAO/SavedSearchDataSets/primary_multiple_state_province.sql index cce1f48916..195e773488 100644 --- a/tests/phpunit/CRM/Contact/BAO/SavedSearchDataSets/primary_multiple_state_province.sql +++ b/tests/phpunit/CRM/Contact/BAO/SavedSearchDataSets/primary_multiple_state_province.sql @@ -1,5 +1,5 @@ INSERT INTO civicrm_mapping (id,name,description,mapping_type_id) Values -(114, NULL, NULL, NULL); +(114, 'test_114', NULL, NULL); INSERT INTO `civicrm_mapping_field` (`id`, `mapping_id`, `name`, `contact_type`, `column_number`, `location_type_id`, `phone_type_id`, `im_provider_id`, `relationship_type_id`, `relationship_direction`, `grouping`, `operator`, `value`, `website_type_id`) VALUES (2846, 114, 'state_province', 'Contact', 0, NULL, NULL, NULL, NULL, NULL, 1, 'IN', '1501,2704', NULL); INSERT INTO `civicrm_saved_search` (`id`, `form_values`, `mapping_id`, `search_custom_id`) VALUES diff --git a/tests/phpunit/CRM/Contact/Import/Form/DataSourceTest.php b/tests/phpunit/CRM/Contact/Import/Form/DataSourceTest.php index 58460dd5da..5471c1c712 100644 --- a/tests/phpunit/CRM/Contact/Import/Form/DataSourceTest.php +++ b/tests/phpunit/CRM/Contact/Import/Form/DataSourceTest.php @@ -29,7 +29,7 @@ class CRM_Contact_Import_Form_DataSourceTest extends CiviUnitTestCase { * Post test cleanup. */ public function tearDown(): void { - $this->quickCleanup(['civicrm_user_job']); + $this->quickCleanup(['civicrm_user_job', 'civicrm_mapping']); parent::tearDown(); } diff --git a/xml/schema/Core/Mapping.xml b/xml/schema/Core/Mapping.xml index 281a0bcd24..470bad7526 100644 --- a/xml/schema/Core/Mapping.xml +++ b/xml/schema/Core/Mapping.xml @@ -26,9 +26,10 @@ name Mapping Name + true varchar 64 - Name of Mapping + Unique name of Mapping 1.2 @@ -55,6 +56,7 @@ UI_name name + true 1.2 -- 2.25.1