From 697f7b54cbf0023cdbcc627a3eac7a8869e22b2b Mon Sep 17 00:00:00 2001 From: eileen Date: Fri, 4 Dec 2020 10:59:47 +1300 Subject: [PATCH] [REF] Extract duplicate handling code This winds up being a very strange process - however the goal seems to be to unravel error handling to appropriate exception throwing & catching & a bit of back & forth helps us isolate the code places --- CRM/Contact/Import/Parser/Contact.php | 208 ++++++++++-------- .../CRM/Contact/Import/Parser/ContactTest.php | 12 +- 2 files changed, 120 insertions(+), 100 deletions(-) diff --git a/CRM/Contact/Import/Parser/Contact.php b/CRM/Contact/Import/Parser/Contact.php index b16c1e60d5..740fe56582 100644 --- a/CRM/Contact/Import/Parser/Contact.php +++ b/CRM/Contact/Import/Parser/Contact.php @@ -739,7 +739,7 @@ class CRM_Contact_Import_Parser_Contact extends CRM_Contact_Import_Parser { //relationship contact insert foreach ($params as $key => $field) { - list($id, $first, $second) = CRM_Utils_System::explode('_', $key, 3); + [$id, $first, $second] = CRM_Utils_System::explode('_', $key, 3); if (!($first == 'a' && $second == 'b') && !($first == 'b' && $second == 'a')) { continue; } @@ -972,100 +972,7 @@ class CRM_Contact_Import_Parser_Contact extends CRM_Contact_Import_Parser { $code = NULL; if (($code = CRM_Utils_Array::value('code', $newContact['error_message'])) && ($code == CRM_Core_Error::DUPLICATE_CONTACT)) { - $urls = []; - // need to fix at some stage and decide if the error will return an - // array or string, crude hack for now - if (is_array($newContact['error_message']['params'][0])) { - $cids = $newContact['error_message']['params'][0]; - } - else { - $cids = explode(',', $newContact['error_message']['params'][0]); - } - - foreach ($cids as $cid) { - $urls[] = CRM_Utils_System::url('civicrm/contact/view', 'reset=1&cid=' . $cid, TRUE); - } - - $url_string = implode("\n", $urls); - - // If we duplicate more than one record, skip no matter what - if (count($cids) > 1) { - $errorMessage = ts('Record duplicates multiple contacts'); - $importRecordParams = [ - $statusFieldName => 'ERROR', - "${statusFieldName}Msg" => $errorMessage, - ]; - - //combine error msg to avoid mismatch between error file columns. - $errorMessage .= "\n" . $url_string; - array_unshift($values, $errorMessage); - $this->updateImportRecord($values[count($values) - 1], $importRecordParams); - return CRM_Import_Parser::ERROR; - } - - // Params only had one id, so shift it out - $contactId = array_shift($cids); - $cid = NULL; - - $vals = ['contact_id' => $contactId]; - - if ($onDuplicate == CRM_Import_Parser::DUPLICATE_REPLACE) { - civicrm_api('contact', 'delete', $vals); - $cid = CRM_Contact_BAO_Contact::createProfileContact($formatted, $contactFields, $contactId, NULL, NULL, $formatted['contact_type']); - } - elseif ($onDuplicate == CRM_Import_Parser::DUPLICATE_UPDATE) { - $newContact = $this->createContact($formatted, $contactFields, $onDuplicate, $contactId); - } - elseif ($onDuplicate == CRM_Import_Parser::DUPLICATE_FILL) { - $newContact = $this->createContact($formatted, $contactFields, $onDuplicate, $contactId); - } - // else skip does nothing and just returns an error code. - if ($cid) { - $contact = [ - 'contact_id' => $cid, - ]; - $defaults = []; - $newContact = CRM_Contact_BAO_Contact::retrieve($contact, $defaults); - } - - if (civicrm_error($newContact)) { - if (empty($newContact['error_message']['params'])) { - // different kind of error other than DUPLICATE - $errorMessage = $newContact['error_message']; - array_unshift($values, $errorMessage); - $importRecordParams = [ - $statusFieldName => 'ERROR', - "${statusFieldName}Msg" => $errorMessage, - ]; - $this->updateImportRecord($values[count($values) - 1], $importRecordParams); - return CRM_Import_Parser::ERROR; - } - - $contactID = $newContact['error_message']['params'][0]; - if (is_array($contactID)) { - $contactID = array_pop($contactID); - } - if (!in_array($contactID, $this->_newContacts)) { - $this->_newContacts[] = $contactID; - } - } - //CRM-262 No Duplicate Checking - if ($onDuplicate == CRM_Import_Parser::DUPLICATE_SKIP) { - array_unshift($values, $url_string); - $importRecordParams = [ - $statusFieldName => 'DUPLICATE', - "${statusFieldName}Msg" => "Skipping duplicate record", - ]; - $this->updateImportRecord($values[count($values) - 1], $importRecordParams); - return CRM_Import_Parser::DUPLICATE; - } - - $importRecordParams = [ - $statusFieldName => 'IMPORTED', - ]; - $this->updateImportRecord($values[count($values) - 1], $importRecordParams); - //return warning if street address is not parsed, CRM-5886 - return $this->processMessage($values, $statusFieldName, CRM_Import_Parser::VALID); + return $this->handleDuplicateError($newContact, $statusFieldName, $values, $onDuplicate, $formatted, $contactFields); } else { // Not a dupe, so we had an error @@ -2091,4 +1998,115 @@ class CRM_Contact_Import_Parser_Contact extends CRM_Contact_Import_Parser { $this->_relationships = $this->getRelationships(); } + /** + * @param array $newContact + * @param $statusFieldName + * @param array $values + * @param int $onDuplicate + * @param array $formatted + * @param array $contactFields + * + * @return int + * + * @throws \CRM_Core_Exception + * @throws \CiviCRM_API3_Exception + * @throws \Civi\API\Exception\UnauthorizedException + */ + protected function handleDuplicateError(array $newContact, $statusFieldName, array $values, int $onDuplicate, array $formatted, array $contactFields): int { + $urls = []; + // need to fix at some stage and decide if the error will return an + // array or string, crude hack for now + if (is_array($newContact['error_message']['params'][0])) { + $cids = $newContact['error_message']['params'][0]; + } + else { + $cids = explode(',', $newContact['error_message']['params'][0]); + } + + foreach ($cids as $cid) { + $urls[] = CRM_Utils_System::url('civicrm/contact/view', 'reset=1&cid=' . $cid, TRUE); + } + + $url_string = implode("\n", $urls); + + // If we duplicate more than one record, skip no matter what + if (count($cids) > 1) { + $errorMessage = ts('Record duplicates multiple contacts'); + $importRecordParams = [ + $statusFieldName => 'ERROR', + "${statusFieldName}Msg" => $errorMessage, + ]; + + //combine error msg to avoid mismatch between error file columns. + $errorMessage .= "\n" . $url_string; + array_unshift($values, $errorMessage); + $this->updateImportRecord($values[count($values) - 1], $importRecordParams); + return CRM_Import_Parser::ERROR; + } + + // Params only had one id, so shift it out + $contactId = array_shift($cids); + $cid = NULL; + + $vals = ['contact_id' => $contactId]; + + if ($onDuplicate == CRM_Import_Parser::DUPLICATE_REPLACE) { + civicrm_api('contact', 'delete', $vals); + $cid = CRM_Contact_BAO_Contact::createProfileContact($formatted, $contactFields, $contactId, NULL, NULL, $formatted['contact_type']); + } + elseif ($onDuplicate == CRM_Import_Parser::DUPLICATE_UPDATE) { + $newContact = $this->createContact($formatted, $contactFields, $onDuplicate, $contactId); + } + elseif ($onDuplicate == CRM_Import_Parser::DUPLICATE_FILL) { + $newContact = $this->createContact($formatted, $contactFields, $onDuplicate, $contactId); + } + // else skip does nothing and just returns an error code. + if ($cid) { + $contact = [ + 'contact_id' => $cid, + ]; + $defaults = []; + $newContact = CRM_Contact_BAO_Contact::retrieve($contact, $defaults); + } + + if (civicrm_error($newContact)) { + if (empty($newContact['error_message']['params'])) { + // different kind of error other than DUPLICATE + $errorMessage = $newContact['error_message']; + array_unshift($values, $errorMessage); + $importRecordParams = [ + $statusFieldName => 'ERROR', + "${statusFieldName}Msg" => $errorMessage, + ]; + $this->updateImportRecord($values[count($values) - 1], $importRecordParams); + return CRM_Import_Parser::ERROR; + } + + $contactID = $newContact['error_message']['params'][0]; + if (is_array($contactID)) { + $contactID = array_pop($contactID); + } + if (!in_array($contactID, $this->_newContacts)) { + $this->_newContacts[] = $contactID; + } + } + //CRM-262 No Duplicate Checking + if ($onDuplicate == CRM_Import_Parser::DUPLICATE_SKIP) { + array_unshift($values, $url_string); + $importRecordParams = [ + $statusFieldName => 'DUPLICATE', + "${statusFieldName}Msg" => "Skipping duplicate record", + ]; + $this->updateImportRecord($values[count($values) - 1], $importRecordParams); + return CRM_Import_Parser::DUPLICATE; + } + + $importRecordParams = [ + $statusFieldName => 'IMPORTED', + ]; + $this->updateImportRecord($values[count($values) - 1], $importRecordParams); + //return warning if street address is not parsed, CRM-5886 + return $this->processMessage($values, $statusFieldName, CRM_Import_Parser::VALID); + } + } diff --git a/tests/phpunit/CRM/Contact/Import/Parser/ContactTest.php b/tests/phpunit/CRM/Contact/Import/Parser/ContactTest.php index fbfa8f9990..1a154a2671 100644 --- a/tests/phpunit/CRM/Contact/Import/Parser/ContactTest.php +++ b/tests/phpunit/CRM/Contact/Import/Parser/ContactTest.php @@ -121,10 +121,10 @@ class CRM_Contact_Import_Parser_ContactTest extends CiviUnitTestCase { * * In this case the contact has no external identifier. * - * @throws \Exception + * @throws \CRM_Core_Exception */ - public function testImportParserWithUpdateWithoutExternalIdentifier() { - list($originalValues, $result) = $this->setUpBaseContact(); + public function testImportParserWithUpdateWithoutExternalIdentifier(): void { + [$originalValues, $result] = $this->setUpBaseContact(); $originalValues['nick_name'] = 'Old Bill'; $this->runImport($originalValues, CRM_Import_Parser::DUPLICATE_UPDATE, CRM_Import_Parser::VALID); $originalValues['id'] = $result['id']; @@ -802,14 +802,16 @@ class CRM_Contact_Import_Parser_ContactTest extends CiviUnitTestCase { /** * CRM-19888 default country should be used if ambigous. + * + * @throws \CRM_Core_Exception */ - public function testImportAmbiguousStateCountry() { + public function testImportAmbiguousStateCountry(): void { $this->callAPISuccess('Setting', 'create', ['defaultContactCountry' => 1228]); $countries = CRM_Core_PseudoConstant::country(FALSE, FALSE); $this->callAPISuccess('Setting', 'create', ['countryLimit' => [array_search('United States', $countries), array_search('Guyana', $countries), array_search('Netherlands', $countries)]]); $this->callAPISuccess('Setting', 'create', ['provinceLimit' => [array_search('United States', $countries), array_search('Guyana', $countries), array_search('Netherlands', $countries)]]); $mapper = [0 => NULL, 1 => NULL, 2 => 'Primary', 3 => NULL]; - list($contactValues) = $this->setUpBaseContact(); + [$contactValues] = $this->setUpBaseContact(); $fields = array_keys($contactValues); $addressValues = [ 'street_address' => 'PO Box 2716', -- 2.25.1