From 67ab8757d7c39abbfc978d2f19205ec16239aed2 Mon Sep 17 00:00:00 2001 From: Darrick Servis Date: Fri, 13 May 2022 15:13:29 -0700 Subject: [PATCH] Fix Can not use Custom Fields defined on a contact_sub_type in dedupe rule --- CRM/Dedupe/BAO/DedupeRuleGroup.php | 16 +- CRM/Dedupe/Finder.php | 6 +- .../phpunit/CRM/Dedupe/BAO/RuleGroupTest.php | 77 ++++++-- tests/phpunit/CRM/Dedupe/DedupeFinderTest.php | 180 ++++++++++++++++++ 4 files changed, 259 insertions(+), 20 deletions(-) diff --git a/CRM/Dedupe/BAO/DedupeRuleGroup.php b/CRM/Dedupe/BAO/DedupeRuleGroup.php index e40118c783..8fa7e35033 100644 --- a/CRM/Dedupe/BAO/DedupeRuleGroup.php +++ b/CRM/Dedupe/BAO/DedupeRuleGroup.php @@ -60,8 +60,7 @@ class CRM_Dedupe_BAO_DedupeRuleGroup extends CRM_Dedupe_DAO_DedupeRuleGroup { * a table-keyed array of field-keyed arrays holding supported fields' titles */ public static function supportedFields($requestedType) { - static $fields = NULL; - if (!$fields) { + if (!isset(Civi::$statics[__CLASS__]['supportedFields'])) { // this is needed, as we're piggy-backing importableFields() below $replacements = [ 'civicrm_country.name' => 'civicrm_address.country_id', @@ -109,8 +108,9 @@ class CRM_Dedupe_BAO_DedupeRuleGroup extends CRM_Dedupe_DAO_DedupeRuleGroup { // Justice League vs The Justice League but these could have the same sort_name if 'the the' // exension is installed (https://github.com/eileenmcnaughton/org.wikimedia.thethe) $fields[$ctype]['civicrm_contact']['sort_name'] = ts('Sort Name'); - // add custom data fields - foreach (CRM_Core_BAO_CustomGroup::getTree($ctype, NULL, NULL, -1) as $key => $cg) { + + // add all custom data fields including those only for sub_types. + foreach (CRM_Core_BAO_CustomGroup::getTree($ctype, NULL, NULL, -1, [], NULL, TRUE, NULL, TRUE) as $key => $cg) { if (!is_int($key)) { continue; } @@ -119,9 +119,13 @@ class CRM_Dedupe_BAO_DedupeRuleGroup extends CRM_Dedupe_DAO_DedupeRuleGroup { } } } + //Does this have to run outside of cache? + CRM_Utils_Hook::dupeQuery(CRM_Core_DAO::$_nullObject, 'supportedFields', $fields); + Civi::$statics[__CLASS__]['supportedFields'] = $fields; } - CRM_Utils_Hook::dupeQuery(CRM_Core_DAO::$_nullObject, 'supportedFields', $fields); - return !empty($fields[$requestedType]) ? $fields[$requestedType] : []; + + return Civi::$statics[__CLASS__]['supportedFields'][$requestedType] ?? []; + } /** diff --git a/CRM/Dedupe/Finder.php b/CRM/Dedupe/Finder.php index a04648e07c..e4ee36ff30 100644 --- a/CRM/Dedupe/Finder.php +++ b/CRM/Dedupe/Finder.php @@ -220,7 +220,11 @@ class CRM_Dedupe_Finder { } // handle custom data - $tree = CRM_Core_BAO_CustomGroup::getTree($ctype, NULL, NULL, -1); + + $subTypes = $fields['contact_sub_type'] ?? []; + // Only return custom for subType + unrestricted or return all custom + // fields. + $tree = CRM_Core_BAO_CustomGroup::getTree($ctype, NULL, NULL, -1, $subTypes, NULL, TRUE, NULL, TRUE); CRM_Core_BAO_CustomGroup::postProcess($tree, $fields, TRUE); foreach ($tree as $key => $cg) { if (!is_int($key)) { diff --git a/tests/phpunit/CRM/Dedupe/BAO/RuleGroupTest.php b/tests/phpunit/CRM/Dedupe/BAO/RuleGroupTest.php index dd5cb947e2..ea36a16eb5 100644 --- a/tests/phpunit/CRM/Dedupe/BAO/RuleGroupTest.php +++ b/tests/phpunit/CRM/Dedupe/BAO/RuleGroupTest.php @@ -62,6 +62,8 @@ class CRM_Dedupe_DAO_TestEntity extends CRM_Core_DAO { */ class CRM_Dedupe_BAO_RuleGroupTest extends CiviUnitTestCase { + use CRMTraits_Custom_CustomDataTrait; + /** * IDs of created contacts. * @@ -107,21 +109,13 @@ class CRM_Dedupe_BAO_RuleGroupTest extends CiviUnitTestCase { } /** - * Test that sort_name is included in supported fields. + * Get the list of supportedFields to test against. * - * This feels like kind of a brittle test but since I debated actually making it - * importable in the schema & bottled out at least some degree of test support - * to ensure the field remains 'hacked in' seems important. - * - * This will at least surface any changes that affect this function. + * This is a statically maintained (in this test list). * - * In general we do have a bit of a problem with having overloaded the meaning of - * importable & exportable fields. */ - public function testSupportedFields() { - $fields = CRM_Dedupe_BAO_DedupeRuleGroup::supportedFields('Organization'); - - $this->assertEquals([ + public function getSupportedFields() { + return [ 'civicrm_address' => [ 'name' => 'Address Name', @@ -197,7 +191,64 @@ class CRM_Dedupe_BAO_RuleGroupTest extends CiviUnitTestCase { [ 'url' => 'Website', ], - ], $fields); + ]; + } + + /** + * Test that sort_name is included in supported fields. + * + * This feels like kind of a brittle test but since I debated actually making it + * importable in the schema & bottled out at least some degree of test support + * to ensure the field remains 'hacked in' seems important. + * + * This will at least surface any changes that affect this function. + * + * In general we do have a bit of a problem with having overloaded the meaning of + * importable & exportable fields. + */ + public function testSupportedFields() { + $fields = CRM_Dedupe_BAO_DedupeRuleGroup::supportedFields('Organization'); + + $this->assertEquals($this->getSupportedFields(), $fields); + } + + /** + * Test that custom_fields are included in supported fields. + * + */ + public function testSupportedCustomFields() { + //Create custom group with fields of all types to test. + $customGroup = $this->createCustomGroup(['extends' => 'Organization']); + + $customGroupID = $this->ids['CustomGroup']['Custom Group']; + $cf = $this->createTextCustomField(['custom_group_id' => $customGroupID]); + + $fields = $this->getSupportedFields(); + $fields[$this->getCustomGroupTable()][$cf['column_name']] = 'Custom Group' . ' : ' . $cf['label']; + + $this->assertEquals($fields, CRM_Dedupe_BAO_DedupeRuleGroup::supportedFields('Organization')); + } + + /** + * Test that custom_fields for a sub_type are included in supported fields. + * + * dev/core#2300 Can not use Custom Fields defined on a contact_sub_type in + * dedupe rule. + * + */ + public function testSupportedCustomFieldsSubtype() { + + //Create custom group with fields of all types to test. + $contactType = $this->callAPISuccess('ContactType', 'create', ['name' => 'Big Bank', 'label' => 'biggee', 'parent_id' => 'Organization']); + $customGroup = $this->createCustomGroup(['extends' => 'Organization', 'extends_entity_column_value' => ['Big_Bank']]); + + $customGroupID = $this->ids['CustomGroup']['Custom Group']; + $cf = $this->createTextCustomField(['custom_group_id' => $customGroupID]); + + $fields = $this->getSupportedFields(); + $fields[$this->getCustomGroupTable()][$cf['column_name']] = 'Custom Group' . ' : ' . $cf['label']; + + $this->assertEquals($fields, CRM_Dedupe_BAO_DedupeRuleGroup::supportedFields('Organization')); } /** diff --git a/tests/phpunit/CRM/Dedupe/DedupeFinderTest.php b/tests/phpunit/CRM/Dedupe/DedupeFinderTest.php index a7756380f4..c1d1676750 100644 --- a/tests/phpunit/CRM/Dedupe/DedupeFinderTest.php +++ b/tests/phpunit/CRM/Dedupe/DedupeFinderTest.php @@ -114,6 +114,186 @@ class CRM_Dedupe_DedupeFinderTest extends CiviUnitTestCase { } + /** + * Test our rule group with a custom group. + * + * @throws \CRM_Core_Exception + */ + public function testCustomRuleCustomFields() { + + $this->setupForGroupDedupe(); + + //Create custom group with fields of all types to test. + $customGroup = $this->createCustomGroup(); + + $customGroupID = $this->ids['CustomGroup']['Custom Group']; + $ids = &$this->ids['CustomField']; + $ids['string'] = (int) $this->createTextCustomField(['custom_group_id' => $customGroupID])['id']; + $ids['date'] = (int) $this->createDateCustomField(['custom_group_id' => $customGroupID])['id']; + $ids['int'] = (int) $this->createIntCustomField(['custom_group_id' => $customGroupID])['id']; + + $params = []; + foreach ($this->ids['CustomField'] as $key => $field_id) { + switch ($key) { + case 'string': + $params["custom_{$field_id}"] = "text"; + break; + + case 'date': + $params["custom_{$field_id}"] = "20220511"; + break; + + case 'int': + $params["custom_{$field_id}"] = 5; + break; + } + } + + $count = 0; + foreach ($this->contactIDs as $contact_id) { + // Update the text custom fields for duplicate contact + foreach ($this->ids['CustomField'] as $key => $field_id) { + $this->callAPISuccess('Contact', 'create', array_merge([ + 'id' => $contact_id, + ], $params)); + } + $count++; + if ($count == 2) { + break; + } + } + + $ruleGroup = $this->callAPISuccess('RuleGroup', 'create', [ + 'contact_type' => 'Individual', + 'threshold' => 4 * count($this->ids['CustomField']), + 'used' => 'General', + 'name' => 'TestRule', + 'title' => 'TestRule', + 'is_reserved' => 0, + ]); + + foreach ($this->ids['CustomField'] as $key => $field_id) { + $this->callAPISuccess('Rule', 'create', [ + 'dedupe_rule_group_id' => $ruleGroup['id'], + 'rule_table' => $this->getCustomGroupTable(), + 'rule_weight' => 4, + 'rule_field' => $this->getCustomFieldColumnName($key), + ]); + } + + $foundDupes = CRM_Dedupe_Finder::dupesInGroup($ruleGroup['id'], $this->groupID); + $this->assertCount(1, $foundDupes); + CRM_Dedupe_Finder::dupes($ruleGroup['id']); + + $fields = [ + 'first_name' => 'robin', + 'last_name' => 'hood', + 'email' => 'hood@example.com', + 'street_address' => 'Ambachtstraat 23', + ]; + + $ids = CRM_Contact_BAO_Contact::getDuplicateContacts($fields, 'Individual', 'General', [], TRUE, $ruleGroup['id'], ['event_id' => 1]); + $this->assertCount(0, $ids); + + $fields = array_merge($fields, $params); + $ids = CRM_Contact_BAO_Contact::getDuplicateContacts($fields, 'Individual', 'General', [], TRUE, $ruleGroup['id'], ['event_id' => 1]); + $this->assertCount(2, $ids); + } + + /** + * Test our rule group with a custom group for a SubType. + * + * @throws \CRM_Core_Exception + */ + public function testCustomRuleCustomFieldsSubtypes() { + + $this->setupForGroupDedupe(); + + //Create custom group with fields of all types to test. + $contactType = $this->callAPISuccess('ContactType', 'create', ['name' => 'Big Bank', 'label' => 'biggee', 'parent_id' => 'Individual']); + $customGroup = $this->createCustomGroup(['extends' => 'Individual', 'extends_entity_column_value' => ['Big_Bank']]); + + $customGroupID = $this->ids['CustomGroup']['Custom Group']; + $ids = &$this->ids['CustomField']; + $ids['string'] = (int) $this->createTextCustomField(['custom_group_id' => $customGroupID])['id']; + $ids['date'] = (int) $this->createDateCustomField(['custom_group_id' => $customGroupID])['id']; + $ids['int'] = (int) $this->createIntCustomField(['custom_group_id' => $customGroupID])['id']; + + $params = []; + foreach ($this->ids['CustomField'] as $key => $field_id) { + switch ($key) { + case 'string': + $params["custom_{$field_id}"] = "text"; + break; + + case 'date': + $params["custom_{$field_id}"] = "20220511"; + break; + + case 'int': + $params["custom_{$field_id}"] = 5; + break; + } + } + + foreach ($this->contactIDs as $contact_id) { + $this->callAPISuccess('Contact', 'create', array_merge([ + 'id' => $contact_id, + 'contact_sub_type' => 'Big_Bank', + ])); + } + + $count = 0; + foreach ($this->contactIDs as $contact_id) { + // Update the text custom fields for duplicate contact + foreach ($this->ids['CustomField'] as $key => $field_id) { + $this->callAPISuccess('Contact', 'create', array_merge([ + 'id' => $contact_id, + ], $params)); + } + $count++; + if ($count == 2) { + break; + } + } + + $ruleGroup = $this->callAPISuccess('RuleGroup', 'create', [ + 'contact_type' => 'Individual', + 'threshold' => 4 * count($this->ids['CustomField']), + 'used' => 'General', + 'name' => 'TestRule', + 'title' => 'TestRule', + 'is_reserved' => 0, + ]); + + foreach ($this->ids['CustomField'] as $key => $field_id) { + $this->callAPISuccess('Rule', 'create', [ + 'dedupe_rule_group_id' => $ruleGroup['id'], + 'rule_table' => $this->getCustomGroupTable(), + 'rule_weight' => 4, + 'rule_field' => $this->getCustomFieldColumnName($key), + ]); + } + + $foundDupes = CRM_Dedupe_Finder::dupesInGroup($ruleGroup['id'], $this->groupID); + $this->assertCount(1, $foundDupes); + CRM_Dedupe_Finder::dupes($ruleGroup['id']); + + $fields = [ + 'first_name' => 'robin', + 'last_name' => 'hood', + 'email' => 'hood@example.com', + 'street_address' => 'Ambachtstraat 23', + ]; + + $ids = CRM_Contact_BAO_Contact::getDuplicateContacts($fields, 'Individual', 'General', [], TRUE, $ruleGroup['id'], ['event_id' => 1]); + $this->assertCount(0, $ids); + + $fields = array_merge($fields, $params); + $ids = CRM_Contact_BAO_Contact::getDuplicateContacts($fields, 'Individual', 'General', [], TRUE, $ruleGroup['id'], ['event_id' => 1]); + $this->assertCount(2, $ids); + } + /** * Test that we do not get a fatal error when our rule group is a custom date field. * -- 2.25.1