From 02f1858b6556bd6fdc03000cf48b930002bdd27d Mon Sep 17 00:00:00 2001 From: Eileen McNaughton Date: Wed, 18 May 2022 11:37:33 +1200 Subject: [PATCH 1/5] [REF] [Import] Extraction of get related contacts function --- CRM/Contact/Import/Parser/Contact.php | 51 +++++++++++++++++++++------ 1 file changed, 40 insertions(+), 11 deletions(-) diff --git a/CRM/Contact/Import/Parser/Contact.php b/CRM/Contact/Import/Parser/Contact.php index a78ab046539c..5b72b261cbcc 100644 --- a/CRM/Contact/Import/Parser/Contact.php +++ b/CRM/Contact/Import/Parser/Contact.php @@ -2872,15 +2872,12 @@ public function getMappedRow(array $values): array { */ public function validateValues(array $values): void { $params = $this->getMappedRow($values); - $this->validateRequiredContactFields($params['contact_type'], $params, $this->isUpdateExistingContacts()); - foreach ($params as $key => $value) { - // If the key is a relationship key - eg. 5_a_b or 10_b_a - // then the value is an array that describes an existing contact. - // We need to check the fields are present to identify or create this - // contact. - if (preg_match('/^\d+_[a|b]_[a|b]$/', $key)) { - $this->validateRequiredContactFields($value['contact_type'], $value, TRUE, '(' . $this->getRelatedContactLabel(substr($key, 0, -4), substr($key, -3)) . ')'); - } + $contacts = array_merge(['0' => $params], $this->getRelatedContactsParams($params)); + foreach ($contacts as $value) { + // If we are referencing a related contact, or are in update mode then we + // don't need all the required fields if we have enough to find an existing contact. + $useExistingMatchFields = !empty($value['relationship_type_id']) || $this->isUpdateExistingContacts(); + $this->validateRequiredContactFields($value['contact_type'], $value, $useExistingMatchFields, !empty($value['relationship_label']) ? '(' . $value['relationship_label'] . ')' : ''); } //check for duplicate external Identifier @@ -2961,7 +2958,7 @@ protected function getRelatedContactType($relationshipTypeID, $relationshipDirec return NULL; } $relationshipField = 'contact_type_' . substr($relationshipDirection, -1); - return $this->getRelationshipType($relationshipTypeID, $relationshipDirection)[$relationshipField]; + return $this->getRelationshipType($relationshipTypeID)[$relationshipField]; } /** @@ -2976,7 +2973,7 @@ protected function getRelatedContactType($relationshipTypeID, $relationshipDirec */ protected function getRelatedContactLabel($relationshipTypeID, $relationshipDirection): ?string { $relationshipField = 'label_' . $relationshipDirection; - return $this->getRelationshipType($relationshipTypeID, $relationshipDirection)[$relationshipField]; + return $this->getRelationshipType($relationshipTypeID)[$relationshipField]; } /** @@ -3019,4 +3016,36 @@ private function addFieldToParams(array &$contactArray, array $locationValues, s } } + /** + * Get any related contacts designated for update. + * + * This extracts the parts that relate to separate related + * contacts from the 'params' array. + * + * It is probably a bit silly not to nest them more clearly in + * `getParams` in the first place & maybe in future we can do that. + * + * @param array $params + * + * @return array + * e.g ['5_a_b' => ['contact_type' => 'Organization', 'organization_name' => 'The Firm']] + * @throws \API_Exception + */ + protected function getRelatedContactsParams(array $params): array { + $relatedContacts = []; + foreach ($params as $key => $value) { + // If the key is a relationship key - eg. 5_a_b or 10_b_a + // then the value is an array that describes an existing contact. + // We need to check the fields are present to identify or create this + // contact. + if (preg_match('/^\d+_[a|b]_[a|b]$/', $key)) { + $value['relationship_type_id'] = substr($key, 0, -4); + $value['relationship_direction'] = substr($key, -3); + $value['relationship_label'] = $this->getRelationshipLabel($value['relationship_type_id'], $value['relationship_direction']); + $relatedContacts[$key] = $value; + } + } + return $relatedContacts; + } + } From 3050997a3fab4cfdf2fb14926d18e307df9c9d89 Mon Sep 17 00:00:00 2001 From: Darrick Servis Date: Sat, 14 May 2022 15:07:35 -0700 Subject: [PATCH 2/5] Text user supplied dedupe rule is used during import. --- CRM/Contact/Import/Parser/Contact.php | 1 + .../CRM/Contact/Import/Parser/ContactTest.php | 106 ++++++++++++++++++ 2 files changed, 107 insertions(+) diff --git a/CRM/Contact/Import/Parser/Contact.php b/CRM/Contact/Import/Parser/Contact.php index 5b72b261cbcc..e57df876cc9a 100644 --- a/CRM/Contact/Import/Parser/Contact.php +++ b/CRM/Contact/Import/Parser/Contact.php @@ -1925,6 +1925,7 @@ protected function getPossibleContactMatches($params) { } $checkParams = ['check_permissions' => FALSE, 'match' => $params]; $checkParams['match']['contact_type'] = $this->_contactType; + $checkParams['dedupe_rule_id'] = $this->_dedupeRuleGroupID ?? NULL; $possibleMatches = civicrm_api3('Contact', 'duplicatecheck', $checkParams); if (!$extIDMatch) { diff --git a/tests/phpunit/CRM/Contact/Import/Parser/ContactTest.php b/tests/phpunit/CRM/Contact/Import/Parser/ContactTest.php index 4a4ddfbe1258..028f3d759822 100644 --- a/tests/phpunit/CRM/Contact/Import/Parser/ContactTest.php +++ b/tests/phpunit/CRM/Contact/Import/Parser/ContactTest.php @@ -120,6 +120,112 @@ public function testImportParserWithUpdateWithoutExternalIdentifier(): void { $this->callAPISuccessGetSingle('Contact', $originalValues); } + /** + * Test import parser will update based on a custom rule match. + * + * In this case the contact has no external identifier. + * + * @throws \API_Exception + * @throws \CRM_Core_Exception + * @throws \CiviCRM_API3_Exception + */ + public function testImportParserWithUpdateWithCustomRule(): void { + $this->createCustomGroupWithFieldsOfAllTypes(); + + $ruleGroup = $this->callAPISuccess('RuleGroup', 'create', [ + 'contact_type' => 'Individual', + 'threshold' => 10, + 'used' => 'General', + 'name' => 'TestRule', + 'title' => 'TestRule', + 'is_reserved' => 0, + ]); + $this->callAPISuccess('Rule', 'create', [ + 'dedupe_rule_group_id' => $ruleGroup['id'], + 'rule_table' => $this->getCustomGroupTable(), + 'rule_weight' => 10, + 'rule_field' => $this->getCustomFieldColumnName('text'), + ]); + + $extra = [ + $this->getCustomFieldName('select_string') => 'Yellow', + $this->getCustomFieldName('text') => 'Duplicate', + ]; + + [$originalValues, $result] = $this->setUpBaseContact($extra); + + $contactValues = [ + 'first_name' => 'Tim', + 'last_name' => 'Cook', + 'email' => 'tim.cook@apple.com', + 'nick_name' => 'Steve', + $this->getCustomFieldName('select_string') => 'Red', + $this->getCustomFieldName('text') => 'Duplicate', + ]; + + $this->runImport($contactValues, CRM_Import_Parser::DUPLICATE_UPDATE, CRM_Import_Parser::VALID, [], NULL, $ruleGroup['id']); + $contactValues['id'] = $result['id']; + $this->assertEquals('R', $this->callAPISuccessGetValue('Contact', ['id' => $result['id'], 'return' => $this->getCustomFieldName('select_string')])); + $this->callAPISuccessGetSingle('Contact', $contactValues); + + $foundDupes = CRM_Dedupe_Finder::dupes($ruleGroup['id']); + $this->assertCount(0, $foundDupes); + } + + /** + * Test import parser will update based on a custom rule match. + * + * In this case the contact has no external identifier. + * + * @throws \API_Exception + * @throws \CRM_Core_Exception + * @throws \CiviCRM_API3_Exception + */ + public function testImportParserWithUpdateWithCustomRuleNoExternalIDMatch(): void { + $this->createCustomGroupWithFieldsOfAllTypes(); + + $ruleGroup = $this->callAPISuccess('RuleGroup', 'create', [ + 'contact_type' => 'Individual', + 'threshold' => 10, + 'used' => 'General', + 'name' => 'TestRule', + 'title' => 'TestRule', + 'is_reserved' => 0, + ]); + $this->callAPISuccess('Rule', 'create', [ + 'dedupe_rule_group_id' => $ruleGroup['id'], + 'rule_table' => $this->getCustomGroupTable(), + 'rule_weight' => 10, + 'rule_field' => $this->getCustomFieldColumnName('text'), + ]); + + $extra = [ + $this->getCustomFieldName('select_string') => 'Yellow', + $this->getCustomFieldName('text') => 'Duplicate', + 'external_identifier' => 'ext-2', + ]; + + [$originalValues, $result] = $this->setUpBaseContact($extra); + + $contactValues = [ + 'first_name' => 'Tim', + 'last_name' => 'Cook', + 'email' => 'tim.cook@apple.com', + 'nick_name' => 'Steve', + 'external_identifier' => 'ext-1', + $this->getCustomFieldName('select_string') => 'Red', + $this->getCustomFieldName('text') => 'Duplicate', + ]; + + $this->runImport($contactValues, CRM_Import_Parser::DUPLICATE_UPDATE, CRM_Import_Parser::VALID, [], NULL, $ruleGroup['id']); + $contactValues['id'] = $result['id']; + $this->assertEquals('R', $this->callAPISuccessGetValue('Contact', ['id' => $result['id'], 'return' => $this->getCustomFieldName('select_string')])); + $this->callAPISuccessGetSingle('Contact', $contactValues); + + $foundDupes = CRM_Dedupe_Finder::dupes($ruleGroup['id']); + $this->assertCount(0, $foundDupes); + } + /** * Test import parser will update contacts with an external identifier. * From 64623d6c7c0963a40146c571761c4b7392d238c5 Mon Sep 17 00:00:00 2001 From: Eileen McNaughton Date: Tue, 17 May 2022 15:22:25 +1200 Subject: [PATCH 3/5] [Import] Duplicate finding cleanup --- CRM/Contact/Import/Parser/Contact.php | 186 ++++++++++-------- .../CRM/Contact/Import/Parser/ContactTest.php | 5 +- 2 files changed, 106 insertions(+), 85 deletions(-) diff --git a/CRM/Contact/Import/Parser/Contact.php b/CRM/Contact/Import/Parser/Contact.php index e57df876cc9a..ef4945a5129e 100644 --- a/CRM/Contact/Import/Parser/Contact.php +++ b/CRM/Contact/Import/Parser/Contact.php @@ -199,6 +199,20 @@ private function isSkipDuplicates(): bool { return ((int) $this->getSubmittedValue('onDuplicate')) === CRM_Import_Parser::DUPLICATE_SKIP; } + /** + * Did the user specify duplicates checking should be skipped, resulting in possible duplicate contacts. + * + * Note we still need to check for external_identifier as it will hard-fail + * if we duplicate. + * + * @return bool + * + * @throws \API_Exception + */ + private function isIgnoreDuplicates(): bool { + return ((int) $this->getSubmittedValue('onDuplicate')) === CRM_Import_Parser::DUPLICATE_NOCHECK; + } + /** * Handle the values in preview mode. * @@ -291,42 +305,6 @@ public function import($onDuplicate, &$values) { $contactFields = CRM_Contact_DAO_Contact::import(); - //check if external identifier exists in database - if (!empty($params['external_identifier']) && (!empty($params['id']) || in_array($onDuplicate, [ - CRM_Import_Parser::DUPLICATE_SKIP, - CRM_Import_Parser::DUPLICATE_NOCHECK, - ]))) { - - $extIDResult = civicrm_api3('Contact', 'get', [ - 'external_identifier' => $params['external_identifier'], - 'showAll' => 'all', - 'return' => ['id', 'contact_is_deleted'], - ]); - if (isset($extIDResult['id'])) { - // record with matching external identifier does exist. - $internalCid = $extIDResult['id']; - if ($internalCid != CRM_Utils_Array::value('id', $params)) { - if ($extIDResult['values'][$internalCid]['contact_is_deleted'] == 1) { - // And it is deleted. What to do? If we skip it, they user - // will be under the impression that the record exists in - // the database, yet they won't be able to find it. If we - // don't skip it, the database will try to insert a new record - // with an external_identifier that is non-unique. So... - // we will update this contact to remove the external_identifier - // and let a new record be created. - $update_params = ['id' => $internalCid, 'external_identifier' => '']; - civicrm_api3('Contact', 'create', $update_params); - } - else { - $errorMessage = ts('External ID already exists in Database.'); - array_unshift($values, $errorMessage); - $this->setImportStatus((int) $values[count($values) - 1], 'ERROR', $errorMessage); - return CRM_Import_Parser::DUPLICATE; - } - } - } - } - if (!empty($this->_contactSubType)) { $params['contact_sub_type'] = $this->_contactSubType; } @@ -343,23 +321,17 @@ public function import($onDuplicate, &$values) { } } + try { + $params['id'] = $formatted['id'] = $this->lookupContactID($params, ($this->isSkipDuplicates() || $this->isIgnoreDuplicates())); + } + catch (CRM_Core_Exception $e) { + $statuses = [CRM_Import_Parser::DUPLICATE => 'DUPLICATE', CRM_Import_Parser::ERROR => 'ERROR', CRM_Import_Parser::NO_MATCH => 'invalid_no_match']; + $this->setImportStatus((int) $values[count($values) - 1], $statuses[$e->getErrorCode()], $e->getMessage()); + return FALSE; + } // Get contact id to format common data in update/fill mode, // prioritising a dedupe rule check over an external_identifier check, but falling back on ext id. - if ($this->_updateWithId && empty($params['id'])) { - try { - $possibleMatches = $this->getPossibleContactMatches($params); - } - catch (CRM_Core_Exception $e) { - $errorMessage = $e->getMessage(); - array_unshift($values, $errorMessage); - $this->setImportStatus((int) $values[count($values) - 1], 'ERROR', $errorMessage); - return CRM_Import_Parser::ERROR; - } - foreach ($possibleMatches as $possibleID) { - $params['id'] = $formatted['id'] = $possibleID; - } - } //format common data, CRM-4062 $this->formatCommonData($params, $formatted, $contactFields); @@ -1892,53 +1864,31 @@ public static function getSubtypes($contactType) { * @see https://issues.civicrm.org/jira/browse/CRM-17275 * * @param array $params + * @param int|null $extIDMatch + * @param int|null $dedupeRuleID * - * @return array - * IDs of possible matches. + * @return int|null + * IDs of a possible. * * @throws \CRM_Core_Exception * @throws \CiviCRM_API3_Exception */ - protected function getPossibleContactMatches($params) { - $extIDMatch = NULL; - - if (!empty($params['external_identifier'])) { - // Check for any match on external id, deleted or otherwise. - $extIDContact = civicrm_api3('Contact', 'get', [ - 'external_identifier' => $params['external_identifier'], - 'showAll' => 'all', - 'return' => ['id', 'contact_is_deleted'], - ]); - if (isset($extIDContact['id'])) { - $extIDMatch = $extIDContact['id']; - - if ($extIDContact['values'][$extIDMatch]['contact_is_deleted'] == 1) { - // If the contact is deleted, update external identifier to be blank - // to avoid key error from MySQL. - $params = ['id' => $extIDMatch, 'external_identifier' => '']; - civicrm_api3('Contact', 'create', $params); - - // And now it is no longer a match. - $extIDMatch = NULL; - } - } - } - $checkParams = ['check_permissions' => FALSE, 'match' => $params]; - $checkParams['match']['contact_type'] = $this->_contactType; - $checkParams['dedupe_rule_id'] = $this->_dedupeRuleGroupID ?? NULL; - + protected function getPossibleContactMatch(array $params, ?int $extIDMatch, ?int $dedupeRuleID): ?int { + $checkParams = ['check_permissions' => FALSE, 'match' => $params, 'dedupe_rule_id' => $dedupeRuleID]; $possibleMatches = civicrm_api3('Contact', 'duplicatecheck', $checkParams); if (!$extIDMatch) { - return array_keys($possibleMatches['values']); + // Historically we have used the last ID - it is not clear if this was + // deliberate. + return array_key_last($possibleMatches['values']); } if ($possibleMatches['count']) { if (array_key_exists($extIDMatch, $possibleMatches['values'])) { - return [$extIDMatch]; + return $extIDMatch; } throw new CRM_Core_Exception(ts( 'Matching this contact based on the de-dupe rule would cause an external ID conflict')); } - return [$extIDMatch]; + return $extIDMatch; } /** @@ -3049,4 +2999,74 @@ protected function getRelatedContactsParams(array $params): array { return $relatedContacts; } + /** + * Look up for an existing contact with the given external_identifier. + * + * If the identifier is found on a deleted contact then it is not a match + * but it must be removed from that contact to allow the new contact to + * have that external_identifier. + * + * @param string|null $externalIdentifier + * + * @return int|null + * + * @throws \CiviCRM_API3_Exception + */ + protected function lookupExternalIdentifier(?string $externalIdentifier): ?int { + if (!$externalIdentifier) { + return NULL; + } + // Check for any match on external id, deleted or otherwise. + $foundContact = civicrm_api3('Contact', 'get', [ + 'external_identifier' => $externalIdentifier, + 'showAll' => 'all', + 'sequential' => TRUE, + 'return' => ['id', 'contact_is_deleted'], + ]); + if (empty($foundContact['id'])) { + return NULL; + } + if (!empty($foundContact['values'][0]['contact_is_deleted'])) { + // If the contact is deleted, update external identifier to be blank + // to avoid key error from MySQL. + $params = ['id' => $foundContact['id'], 'external_identifier' => '']; + civicrm_api3('Contact', 'create', $params); + return NULL; + } + return (int) $foundContact['id']; + } + + /** + * Lookup the contact's contact ID. + * + * @param array $params + * @param bool $isDuplicateIfExternalIdentifierExists + * + * @return int|null + * + * @throws \API_Exception + * @throws \CRM_Core_Exception + * @throws \CiviCRM_API3_Exception + */ + protected function lookupContactID(array $params, bool $isDuplicateIfExternalIdentifierExists): ?int { + $extIDMatch = $this->lookupExternalIdentifier($params['external_identifier'] ?? NULL); + $contactID = !empty($params['id']) ? (int) $params['id'] : NULL; + //check if external identifier exists in database + if ($extIDMatch && $contactID && $extIDMatch !== $contactID) { + throw new CRM_Core_Exception(ts('Existing external ID does not match the imported contact ID.'), CRM_Import_Parser::ERROR); + } + if ($extIDMatch && $isDuplicateIfExternalIdentifierExists) { + throw new CRM_Core_Exception(ts('External ID already exists in Database.'), CRM_Import_Parser::DUPLICATE); + } + if ($contactID) { + return $contactID; + } + // Time to see if we can find an existing contact ID to make this an update + // not a create. + if ($extIDMatch || $this->isUpdateExistingContacts()) { + return $this->getPossibleContactMatch($params, $extIDMatch, $this->getSubmittedValue('dedupe_rule_id')); + } + return NULL; + } + } diff --git a/tests/phpunit/CRM/Contact/Import/Parser/ContactTest.php b/tests/phpunit/CRM/Contact/Import/Parser/ContactTest.php index 028f3d759822..86c72764697e 100644 --- a/tests/phpunit/CRM/Contact/Import/Parser/ContactTest.php +++ b/tests/phpunit/CRM/Contact/Import/Parser/ContactTest.php @@ -1241,8 +1241,8 @@ protected function runImport(array $originalValues, $onDuplicateAction, $expecte foreach ($fields as $index => $field) { $mapper[] = [$field, $mapperLocType[$index] ?? NULL, $field === 'phone' ? 1 : NULL]; } - $userJobID = $this->getUserJobID(['mapper' => $mapper, 'onDuplicate' => $onDuplicateAction]); - $parser = new CRM_Contact_Import_Parser_Contact($fields, $mapperLocType); + $userJobID = $this->getUserJobID(['mapper' => $mapper, 'onDuplicate' => $onDuplicateAction, 'dedupe_rule_id' => $ruleGroupId]); + $parser = new CRM_Contact_Import_Parser_Contact($fields); $parser->setUserJobID($userJobID); $parser->_dedupeRuleGroupID = $ruleGroupId; $parser->init(); @@ -1412,6 +1412,7 @@ protected function getUserJobID($submittedValues = []) { 'dataSource' => 'CRM_Import_DataSource_SQL', 'sqlQuery' => 'SELECT first_name FROM civicrm_contact', 'onDuplicate' => CRM_Import_Parser::DUPLICATE_SKIP, + 'dedupe_rule_id' => NULL, ], $submittedValues), ], 'status_id:name' => 'draft', From ed75aff292a637685931b320f08512a2aed77290 Mon Sep 17 00:00:00 2001 From: Eileen McNaughton Date: Tue, 17 May 2022 18:28:41 +1200 Subject: [PATCH 4/5] [Import] Simplify checking contact type is valid --- CRM/Contact/Import/Parser/Contact.php | 51 ++++++++----------- .../CRM/Contact/Import/Parser/ContactTest.php | 14 ++++- 2 files changed, 34 insertions(+), 31 deletions(-) diff --git a/CRM/Contact/Import/Parser/Contact.php b/CRM/Contact/Import/Parser/Contact.php index ef4945a5129e..7420b956490b 100644 --- a/CRM/Contact/Import/Parser/Contact.php +++ b/CRM/Contact/Import/Parser/Contact.php @@ -348,8 +348,7 @@ public function import($onDuplicate, &$values) { $updateflag = TRUE; foreach ($matchedIDs as $contactId) { if ($params['id'] == $contactId) { - $contactType = CRM_Core_DAO::getFieldValue('CRM_Contact_DAO_Contact', $params['id'], 'contact_type'); - if ($formatted['contact_type'] == $contactType) { + if (1) { //validation of subtype for update mode //CRM-5125 $contactSubType = NULL; @@ -369,12 +368,6 @@ public function import($onDuplicate, &$values) { $this->_retCode = CRM_Import_Parser::VALID; } } - else { - $message = "Mismatched contact Types :"; - array_unshift($values, $message); - $updateflag = FALSE; - $this->_retCode = CRM_Import_Parser::NO_MATCH; - } } } if ($updateflag) { @@ -385,11 +378,9 @@ public function import($onDuplicate, &$values) { } } else { - $contactType = NULL; if (!empty($params['id'])) { - $contactType = CRM_Core_DAO::getFieldValue('CRM_Contact_DAO_Contact', $params['id'], 'contact_type'); - if ($contactType) { - if ($formatted['contact_type'] == $contactType) { + if (1) { + if (1) { //validation of subtype for update mode //CRM-5125 $contactSubType = NULL; @@ -408,20 +399,6 @@ public function import($onDuplicate, &$values) { $this->_retCode = CRM_Import_Parser::VALID; } } - else { - $message = "Mismatched contact Types :"; - array_unshift($values, $message); - $this->_retCode = CRM_Import_Parser::NO_MATCH; - } - } - else { - // we should avoid multiple errors for single record - // since we have already retCode and we trying to force again. - if ($this->_retCode != CRM_Import_Parser::NO_MATCH) { - $message = "No contact found for this contact ID:" . $params['id']; - array_unshift($values, $message); - $this->_retCode = CRM_Import_Parser::NO_MATCH; - } } } else { @@ -3007,12 +2984,14 @@ protected function getRelatedContactsParams(array $params): array { * have that external_identifier. * * @param string|null $externalIdentifier + * @param string $contactType * * @return int|null * + * @throws \CRM_Core_Exception * @throws \CiviCRM_API3_Exception */ - protected function lookupExternalIdentifier(?string $externalIdentifier): ?int { + protected function lookupExternalIdentifier(?string $externalIdentifier, string $contactType): ?int { if (!$externalIdentifier) { return NULL; } @@ -3021,7 +3000,7 @@ protected function lookupExternalIdentifier(?string $externalIdentifier): ?int { 'external_identifier' => $externalIdentifier, 'showAll' => 'all', 'sequential' => TRUE, - 'return' => ['id', 'contact_is_deleted'], + 'return' => ['id', 'contact_is_deleted', 'contact_type'], ]); if (empty($foundContact['id'])) { return NULL; @@ -3033,6 +3012,9 @@ protected function lookupExternalIdentifier(?string $externalIdentifier): ?int { civicrm_api3('Contact', 'create', $params); return NULL; } + if ($foundContact['values'][0]['contact_type'] !== $contactType) { + throw new CRM_Core_Exception('Mismatched contact Types', CRM_Import_Parser::NO_MATCH); + } return (int) $foundContact['id']; } @@ -3049,7 +3031,7 @@ protected function lookupExternalIdentifier(?string $externalIdentifier): ?int { * @throws \CiviCRM_API3_Exception */ protected function lookupContactID(array $params, bool $isDuplicateIfExternalIdentifierExists): ?int { - $extIDMatch = $this->lookupExternalIdentifier($params['external_identifier'] ?? NULL); + $extIDMatch = $this->lookupExternalIdentifier($params['external_identifier'] ?? NULL, $params['contact_type']); $contactID = !empty($params['id']) ? (int) $params['id'] : NULL; //check if external identifier exists in database if ($extIDMatch && $contactID && $extIDMatch !== $contactID) { @@ -3059,6 +3041,17 @@ protected function lookupContactID(array $params, bool $isDuplicateIfExternalIde throw new CRM_Core_Exception(ts('External ID already exists in Database.'), CRM_Import_Parser::DUPLICATE); } if ($contactID) { + $existingContact = Contact::get(FALSE) + ->addWhere('id', '=', $contactID) + // Don't auto-filter deleted - people use import to undelete. + ->addWhere('is_deleted', 'IN', [0, 1]) + ->addSelect('contact_type')->execute()->first(); + if (empty($existingContact['id'])) { + throw new CRM_Core_Exception('No contact found for this contact ID:' . $params['id'], CRM_Import_Parser::NO_MATCH); + } + if ($existingContact['contact_type'] !== $params['contact_type']) { + throw new CRM_Core_Exception('Mismatched contact Types', CRM_Import_Parser::NO_MATCH); + } return $contactID; } // Time to see if we can find an existing contact ID to make this an update diff --git a/tests/phpunit/CRM/Contact/Import/Parser/ContactTest.php b/tests/phpunit/CRM/Contact/Import/Parser/ContactTest.php index 86c72764697e..347cf2c47f4f 100644 --- a/tests/phpunit/CRM/Contact/Import/Parser/ContactTest.php +++ b/tests/phpunit/CRM/Contact/Import/Parser/ContactTest.php @@ -271,16 +271,26 @@ public function testImportParserWithUpdateWithExternalIdentifierSubtypeMismatch( * * @throws \Exception */ - public function testImportParserWithUpdateWithExternalIdentifierTypeMismatch(): void { + public function testImportParserWithUpdateWithTypeMismatch(): void { $contactID = $this->organizationCreate(['external_identifier' => 'billy']); $this->runImport([ 'external_identifier' => 'billy', 'nick_name' => 'Old Bill', - ], CRM_Import_Parser::DUPLICATE_UPDATE, CRM_Import_Parser::NO_MATCH); + ], CRM_Import_Parser::DUPLICATE_UPDATE, FALSE); $contact = $this->callAPISuccessGetSingle('Contact', ['id' => $contactID]); $this->assertEquals('', $contact['nick_name']); $this->assertEquals('billy', $contact['external_identifier']); $this->assertEquals('Organization', $contact['contact_type']); + + $this->runImport([ + 'id' => $contactID, + 'nick_name' => 'Old Bill', + ], CRM_Import_Parser::DUPLICATE_UPDATE, FALSE); + $contact = $this->callAPISuccessGetSingle('Contact', ['id' => $contactID]); + $this->assertEquals('', $contact['nick_name']); + $this->assertEquals('billy', $contact['external_identifier']); + $this->assertEquals('Organization', $contact['contact_type']); + } /** From b6c2df03782ac454ef133025e1970e9cea2d766c Mon Sep 17 00:00:00 2001 From: Eileen McNaughton Date: Wed, 18 May 2022 11:09:33 +1200 Subject: [PATCH 5/5] Validate subtype change --- CRM/Contact/Import/Parser/Contact.php | 24 +++++++-------- .../CRM/Contact/Import/Parser/ContactTest.php | 30 +++++++++++++++++-- 2 files changed, 39 insertions(+), 15 deletions(-) diff --git a/CRM/Contact/Import/Parser/Contact.php b/CRM/Contact/Import/Parser/Contact.php index 7420b956490b..00ae15f24f62 100644 --- a/CRM/Contact/Import/Parser/Contact.php +++ b/CRM/Contact/Import/Parser/Contact.php @@ -300,28 +300,26 @@ public function import($onDuplicate, &$values) { $params = $this->getMappedRow($values); $formatted = [ - 'contact_type' => $this->_contactType, + 'contact_type' => $this->getContactType(), ]; $contactFields = CRM_Contact_DAO_Contact::import(); - if (!empty($this->_contactSubType)) { - $params['contact_sub_type'] = $this->_contactSubType; - } + $params['contact_sub_type'] = $this->getContactSubType() ?: ($params['contact_sub_type'] ?? NULL); - if ($subType = CRM_Utils_Array::value('contact_sub_type', $params)) { - if (CRM_Contact_BAO_ContactType::isExtendsContactType($subType, $this->_contactType, FALSE, 'label')) { - $subTypes = CRM_Contact_BAO_ContactType::subTypePairs($this->_contactType, FALSE, NULL); - $params['contact_sub_type'] = array_search($subType, $subTypes); - } - elseif (!CRM_Contact_BAO_ContactType::isExtendsContactType($subType, $this->_contactType)) { - $message = "Mismatched or Invalid Contact Subtype."; - array_unshift($values, $message); - return CRM_Import_Parser::NO_MATCH; + if ($params['contact_sub_type']) { + if (CRM_Contact_BAO_ContactType::isExtendsContactType($params['contact_sub_type'], $this->getContactType(), FALSE, 'label')) { + // I think this bit is switching a passed in label to + // a name. + $subTypes = CRM_Contact_BAO_ContactType::subTypePairs($this->getContactType(), FALSE, NULL); + $params['contact_sub_type'] = array_search($params['contact_sub_type'], $subTypes); } } try { + if ($params['contact_sub_type'] && !CRM_Contact_BAO_ContactType::isExtendsContactType($params['contact_sub_type'], $this->getContactType())) { + throw new CRM_Core_Exception('Mismatched or Invalid Contact Subtype.', CRM_Import_Parser::NO_MATCH); + } $params['id'] = $formatted['id'] = $this->lookupContactID($params, ($this->isSkipDuplicates() || $this->isIgnoreDuplicates())); } catch (CRM_Core_Exception $e) { diff --git a/tests/phpunit/CRM/Contact/Import/Parser/ContactTest.php b/tests/phpunit/CRM/Contact/Import/Parser/ContactTest.php index 347cf2c47f4f..b4d263bb6e72 100644 --- a/tests/phpunit/CRM/Contact/Import/Parser/ContactTest.php +++ b/tests/phpunit/CRM/Contact/Import/Parser/ContactTest.php @@ -15,6 +15,7 @@ */ use Civi\Api4\Contact; +use Civi\Api4\RelationshipType; use Civi\Api4\UserJob; /** @@ -37,7 +38,8 @@ class CRM_Contact_Import_Parser_ContactTest extends CiviUnitTestCase { * Tear down after test. */ public function tearDown(): void { - $this->quickCleanup(['civicrm_address', 'civicrm_phone', 'civicrm_email', 'civicrm_user_job'], TRUE); + $this->quickCleanup(['civicrm_address', 'civicrm_phone', 'civicrm_email', 'civicrm_user_job', 'civicrm_relationship'], TRUE); + RelationshipType::delete()->addWhere('name_a_b', '=', 'Dad to')->execute(); parent::tearDown(); } @@ -250,9 +252,11 @@ public function testImportParserWithUpdateWithExternalIdentifier(): void { /** * Test updating an existing contact with external_identifier match but subtype mismatch. * + * The subtype is updated, as there is no conflicting contact data. + * * @throws \Exception */ - public function testImportParserWithUpdateWithExternalIdentifierSubtypeMismatch(): void { + public function testImportParserWithUpdateWithExternalIdentifierSubtypeChange(): void { $contactID = $this->individualCreate(['external_identifier' => 'billy', 'first_name' => 'William', 'contact_sub_type' => 'Parent']); $this->runImport([ 'external_identifier' => 'billy', @@ -1286,6 +1290,28 @@ protected function getDataSourceAndParser(string $csv, array $mapper, array $sub return [$dataSource, $parser]; } + /** + * @param int $contactID + * + * @throws \API_Exception + * @throws \Civi\API\Exception\UnauthorizedException + */ + protected function addChild(int $contactID): void { + $relatedContactID = $this->individualCreate(); + $relationshipTypeID = RelationshipType::create()->setValues([ + 'name_a_b' => 'Dad to', + 'name_b_a' => 'Sleep destroyer of', + 'contact_type_a' => 'Individual', + 'contact_type_b' => 'Individual', + 'contact_sub_type_a' => 'Parent', + ])->execute()->first()['id']; + \Civi\Api4\Relationship::create()->setValues([ + 'relationship_type_id' => $relationshipTypeID, + 'contact_id_a' => $contactID, + 'contact_id_b' => $relatedContactID, + ])->execute(); + } + /** * @param array $fields Array of fields to be imported * @param array $allfields Array of all fields which can be part of import