From a1c800771d88c97d16b2ad0df9bf89e7b9b4f96c Mon Sep 17 00:00:00 2001 From: Coleman Watts Date: Thu, 25 Aug 2022 12:21:12 -0400 Subject: [PATCH] Add unique index to DedupeRuleGroup.name field --- CRM/Contact/Form/DedupeRules.php | 26 ++++++------------- CRM/Core/DAO.php | 6 ++--- CRM/Dedupe/DAO/DedupeRuleGroup.php | 24 ++++++++++++----- CRM/Upgrade/Incremental/php/FiveFiftyFour.php | 13 ++++++++++ api/v3/RuleGroup.php | 2 +- .../CRM/Contact/Import/Parser/ContactTest.php | 1 - tests/phpunit/CiviTest/CiviUnitTestCase.php | 1 - tests/phpunit/api/v3/JobTest.php | 2 +- xml/schema/Dedupe/DedupeRuleGroup.xml | 10 +++++-- 9 files changed, 50 insertions(+), 35 deletions(-) diff --git a/CRM/Contact/Form/DedupeRules.php b/CRM/Contact/Form/DedupeRules.php index c63935739c..7c2040631f 100644 --- a/CRM/Contact/Form/DedupeRules.php +++ b/CRM/Contact/Form/DedupeRules.php @@ -213,24 +213,14 @@ UPDATE civicrm_dedupe_rule_group CRM_Core_DAO::executeQuery($query, $queryParams); } - $rgDao = new CRM_Dedupe_DAO_DedupeRuleGroup(); - if ($this->_action & CRM_Core_Action::UPDATE) { - $rgDao->id = $this->_rgid; - } - - $rgDao->title = $values['title']; - $rgDao->is_reserved = CRM_Utils_Array::value('is_reserved', $values, FALSE); - $rgDao->used = $values['used']; - $rgDao->contact_type = $this->_contactType; - $rgDao->threshold = $values['threshold']; - $rgDao->save(); - - // make sure name is set only during insert - if ($this->_action & CRM_Core_Action::ADD) { - // generate name based on title - $rgDao->name = CRM_Utils_String::titleToVar($values['title']) . "_{$rgDao->id}"; - $rgDao->save(); - } + $rgDao = CRM_Dedupe_BAO_DedupeRuleGroup::writeRecord([ + 'id' => $this->_rgid, + 'contact_type' => $this->_contactType, + 'title' => $values['title'], + 'is_reserved' => $values['is_reserved'] ?? FALSE, + 'used' => $values['used'], + 'threshold' => $values['threshold'], + ]); // lets skip updating of fields for reserved dedupe group if (!empty($this->_defaults['is_reserved'])) { diff --git a/CRM/Core/DAO.php b/CRM/Core/DAO.php index c83bba9438..8106c36a6c 100644 --- a/CRM/Core/DAO.php +++ b/CRM/Core/DAO.php @@ -941,7 +941,7 @@ class CRM_Core_DAO extends DB_DataObject { $values = array_intersect_key($record, $fields); $instance->copyValues($values); if (empty($values[$idField]) && array_key_exists('name', $fields) && empty($values['name'])) { - $instance->makeNameFromLabel(!empty($fields['name']['required'])); + $instance->makeNameFromLabel(); } $instance->save(); @@ -3326,10 +3326,8 @@ SELECT contact_id * create a unique, clean name derived from the label. * * Note: this function does nothing unless a unique index exists for "name" column. - * - * @var bool $isRequired */ - private function makeNameFromLabel(bool $isRequired): void { + private function makeNameFromLabel(): void { $indexNameWith = NULL; // Look for a unique index which includes the "name" field if (method_exists($this, 'indices')) { diff --git a/CRM/Dedupe/DAO/DedupeRuleGroup.php b/CRM/Dedupe/DAO/DedupeRuleGroup.php index 95ac2b5393..be90c79b54 100644 --- a/CRM/Dedupe/DAO/DedupeRuleGroup.php +++ b/CRM/Dedupe/DAO/DedupeRuleGroup.php @@ -6,7 +6,7 @@ * * Generated from xml/schema/CRM/Dedupe/DedupeRuleGroup.xml * DO NOT EDIT. Generated by CRM_Core_CodeGen - * (GenCodeChecksum:7bcb6791c8bff662caa873128c101da8) + * (GenCodeChecksum:dc8957103d3accc4eb60eb94a3ee2282) */ /** @@ -67,10 +67,10 @@ class CRM_Dedupe_DAO_DedupeRuleGroup extends CRM_Core_DAO { public $used; /** - * Name of the rule group + * Unique name of rule group * * @var string|null - * (SQL type: varchar(64)) + * (SQL type: varchar(255)) * Note that values will be retrieved from the database as a string. */ public $name; @@ -200,9 +200,9 @@ class CRM_Dedupe_DAO_DedupeRuleGroup extends CRM_Core_DAO { 'name' => 'name', 'type' => CRM_Utils_Type::T_STRING, 'title' => ts('Name'), - 'description' => ts('Name of the rule group'), - 'maxlength' => 64, - 'size' => CRM_Utils_Type::BIG, + 'description' => ts('Unique name of rule group'), + 'maxlength' => 255, + 'size' => CRM_Utils_Type::HUGE, 'where' => 'civicrm_dedupe_rule_group.name', 'table_name' => 'civicrm_dedupe_rule_group', 'entity' => 'DedupeRuleGroup', @@ -313,7 +313,17 @@ class CRM_Dedupe_DAO_DedupeRuleGroup extends CRM_Core_DAO { * @return array */ public static function indices($localize = TRUE) { - $indices = []; + $indices = [ + 'UI_name' => [ + 'name' => 'UI_name', + 'field' => [ + 0 => 'name', + ], + 'localizable' => FALSE, + 'unique' => TRUE, + 'sig' => 'civicrm_dedupe_rule_group::1::name', + ], + ]; return ($localize && !empty($indices)) ? CRM_Core_DAO_AllCoreTables::multilingualize(__CLASS__, $indices) : $indices; } diff --git a/CRM/Upgrade/Incremental/php/FiveFiftyFour.php b/CRM/Upgrade/Incremental/php/FiveFiftyFour.php index d3c0d3a449..3ad5ca414f 100644 --- a/CRM/Upgrade/Incremental/php/FiveFiftyFour.php +++ b/CRM/Upgrade/Incremental/php/FiveFiftyFour.php @@ -39,6 +39,8 @@ class CRM_Upgrade_Incremental_php_FiveFiftyFour extends CRM_Upgrade_Incremental_ public function upgrade_5_54_alpha1($rev): void { $this->addTask(ts('Upgrade DB to %1: SQL', [1 => $rev]), 'runSql', $rev); $this->addTask('Add "created_id" column to "civicrm_participant"', 'addCreatedIDColumnToParticipant'); + $this->addTask('Increase field length of civicrm_dedupe_rule_group.name', 'alterDedupeRuleGroupName'); + $this->addTask('Add index civicrm_dedupe_rule_group.UI_name', 'addIndex', 'civicrm_dedupe_rule_group', 'name', 'UI'); $this->addTask('Install Elavon Payment Processor Extension as needed', 'installElavonPaymentProcessorExtension'); } @@ -50,6 +52,17 @@ class CRM_Upgrade_Incremental_php_FiveFiftyFour extends CRM_Upgrade_Incremental_ return TRUE; } + /** + * @param \CRM_Queue_TaskContext $ctx + * + * @return bool + */ + public static function alterDedupeRuleGroupName(CRM_Queue_TaskContext $ctx) { + CRM_Core_DAO::executeQuery("ALTER TABLE `civicrm_dedupe_rule_group` CHANGE COLUMN `name` `name` varchar(255) COMMENT 'Unique name of rule group'", [], TRUE, NULL, FALSE, FALSE); + CRM_Core_DAO::executeQuery("UPDATE `civicrm_dedupe_rule_group` g1, `civicrm_dedupe_rule_group` g2 SET g1.name = CONCAT(g1.name, '_', g1.id) WHERE g1.name = g2.name AND g1.id > g2.id", [], TRUE, NULL, FALSE, FALSE); + return TRUE; + } + /** * @param CRM_Queue_TaskContext $ctx * @return bool diff --git a/api/v3/RuleGroup.php b/api/v3/RuleGroup.php index fe40d28ae0..7edbcd33ce 100644 --- a/api/v3/RuleGroup.php +++ b/api/v3/RuleGroup.php @@ -27,6 +27,7 @@ * API result array */ function civicrm_api3_rule_group_create($params) { + civicrm_api3_verify_one_mandatory($params, NULL, ['title', 'name']); return _civicrm_api3_basic_create(_civicrm_api3_get_BAO(__FUNCTION__), $params, 'DedupeRuleGroup'); } @@ -42,7 +43,6 @@ function _civicrm_api3_rule_group_create_spec(&$params) { $params['contact_type']['api.required'] = TRUE; $params['threshold']['api.required'] = TRUE; $params['used']['api.required'] = TRUE; - $params['name']['api.required'] = TRUE; } /** diff --git a/tests/phpunit/CRM/Contact/Import/Parser/ContactTest.php b/tests/phpunit/CRM/Contact/Import/Parser/ContactTest.php index 7644bc77a3..bbc4340d55 100644 --- a/tests/phpunit/CRM/Contact/Import/Parser/ContactTest.php +++ b/tests/phpunit/CRM/Contact/Import/Parser/ContactTest.php @@ -207,7 +207,6 @@ class CRM_Contact_Import_Parser_ContactTest extends CiviUnitTestCase { 'contact_type' => 'Individual', 'threshold' => 10, 'used' => 'General', - 'name' => 'TestRule', 'title' => 'TestRule', 'is_reserved' => 0, ]); diff --git a/tests/phpunit/CiviTest/CiviUnitTestCase.php b/tests/phpunit/CiviTest/CiviUnitTestCase.php index bab7810251..8ae4b7c627 100644 --- a/tests/phpunit/CiviTest/CiviUnitTestCase.php +++ b/tests/phpunit/CiviTest/CiviUnitTestCase.php @@ -3653,7 +3653,6 @@ class CiviUnitTestCase extends PHPUnit\Framework\TestCase { 'contact_type' => 'Individual', 'threshold' => 8, 'used' => 'General', - 'name' => 'TestRule', 'title' => 'TestRule', 'is_reserved' => 0, ]); diff --git a/tests/phpunit/api/v3/JobTest.php b/tests/phpunit/api/v3/JobTest.php index 967dec4737..6d5319a4f7 100644 --- a/tests/phpunit/api/v3/JobTest.php +++ b/tests/phpunit/api/v3/JobTest.php @@ -1019,7 +1019,7 @@ class api_v3_JobTest extends CiviUnitTestCase { 'contact_type' => $contactType, 'threshold' => $threshold, 'used' => $used, - 'name' => $name, + 'title' => $name, 'is_reserved' => $isReserved, ]); $this->callAPISuccess('Job', 'process_batch_merge', ['rule_group_id' => $ruleGroup['id']]); diff --git a/xml/schema/Dedupe/DedupeRuleGroup.xml b/xml/schema/Dedupe/DedupeRuleGroup.xml index ff77740d4c..a8b8c78981 100644 --- a/xml/schema/Dedupe/DedupeRuleGroup.xml +++ b/xml/schema/Dedupe/DedupeRuleGroup.xml @@ -67,10 +67,16 @@ name Name varchar - 64 - Name of the rule group + 255 + Unique name of rule group 2.1 + + UI_name + name + true + 5.54 + title Title -- 2.25.1