From 13943a5010f60893d4e3df15c784f518d44a7995 Mon Sep 17 00:00:00 2001 From: Jamie McClelland Date: Tue, 8 Nov 2016 14:45:45 -0500 Subject: [PATCH] CRM-19620 - Restore matching contacts in trash prior to import along with Unit Test. This avoids duplicate key errors. If we don't restore and return the deleted contact when we have a match on external_identifier, then we end up with a key error. --- CRM/Contact/Import/Parser/Contact.php | 52 +++++++++++++++---- .../CRM/Contact/Import/Parser/ContactTest.php | 26 ++++++++-- 2 files changed, 65 insertions(+), 13 deletions(-) diff --git a/CRM/Contact/Import/Parser/Contact.php b/CRM/Contact/Import/Parser/Contact.php index 2d4cf7cfe7..a6925e439a 100644 --- a/CRM/Contact/Import/Parser/Contact.php +++ b/CRM/Contact/Import/Parser/Contact.php @@ -508,16 +508,36 @@ class CRM_Contact_Import_Parser_Contact extends CRM_Contact_Import_Parser { ))) ) { - if ($internalCid = CRM_Core_DAO::getFieldValue('CRM_Contact_DAO_Contact', $params['external_identifier'], 'id', 'external_identifier', TRUE)) { + $extIDResult = civicrm_api3('Contact', 'get', array( + 'external_identifier' => $params['external_identifier'], + 'showAll' => 'all', + 'return' => array('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)) { - $errorMessage = ts('External ID already exists in Database.'); - array_unshift($values, $errorMessage); - $importRecordParams = array( - $statusFieldName => 'ERROR', - "${statusFieldName}Msg" => $errorMessage, - ); - $this->updateImportRecord($values[count($values) - 1], $importRecordParams); - return CRM_Import_Parser::DUPLICATE; + 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 = array('id' => $internalCid, 'external_identifier' => ''); + civicrm_api3('Contact', 'create', $update_params); + } + else { + $errorMessage = ts('External ID already exists in Database.'); + array_unshift($values, $errorMessage); + $importRecordParams = array( + $statusFieldName => 'ERROR', + "${statusFieldName}Msg" => $errorMessage, + ); + $this->updateImportRecord($values[count($values) - 1], $importRecordParams); + return CRM_Import_Parser::DUPLICATE; + } } } } @@ -1997,12 +2017,24 @@ class CRM_Contact_Import_Parser_Contact extends CRM_Contact_Import_Parser { $extIDMatch = NULL; if (!empty($params['external_identifier'])) { + // Check for any match on external id, deleted or otherwise. $extIDContact = civicrm_api3('Contact', 'get', array( 'external_identifier' => $params['external_identifier'], - 'return' => 'id', + 'showAll' => 'all', + 'return' => array('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 = array('id' => $extIDMatch, 'external_identifier' => ''); + civicrm_api3('Contact', 'create', $params); + + // And now it is no longer a match. + $extIDMatch = NULL; + } } } $checkParams = array('check_permissions' => FALSE, 'match' => $params); diff --git a/tests/phpunit/CRM/Contact/Import/Parser/ContactTest.php b/tests/phpunit/CRM/Contact/Import/Parser/ContactTest.php index 1e72128ca0..2890a7be35 100644 --- a/tests/phpunit/CRM/Contact/Import/Parser/ContactTest.php +++ b/tests/phpunit/CRM/Contact/Import/Parser/ContactTest.php @@ -46,6 +46,26 @@ class CRM_Contact_Imports_Parser_ContactTest extends CiviUnitTestCase { parent::setUp(); } + /** + * Test that import parser will not fail when same external_identifier found of deleted contact. + * + * @throws \Exception + */ + public function testImportParserWtihDeletedContactExternalIdentifier() { + $contactId = $this->individualCreate(array( + "external_identifier" => "ext-1", + )); + CRM_Contact_BAO_Contact::deleteContact($contactId); + list($originalValues, $result) = $this->setUpBaseContact(array( + 'external_identifier' => 'ext-1', + )); + $originalValues['nick_name'] = 'Old Bill'; + $this->runImport($originalValues, CRM_Import_Parser::DUPLICATE_UPDATE, CRM_Import_Parser::VALID); + $originalValues['id'] = $result['id']; + $this->assertEquals('ext-1', $this->callAPISuccessGetValue('Contact', array('id' => $result['id'], 'return' => 'external_identifier'))); + $this->callAPISuccessGetSingle('Contact', $originalValues); + } + /** * Test import parser will update based on a rule match. * @@ -302,9 +322,9 @@ class CRM_Contact_Imports_Parser_ContactTest extends CiviUnitTestCase { $this->callAPISuccessGetSingle('Contact', $contactValues); } - /** - * Test the determination of whether a custom field is valid. - */ + /** + * Test the determination of whether a custom field is valid. + */ public function testCustomFieldValidation() { $errorMessage = array(); $customGroup = $this->customGroupCreate(array( -- 2.25.1