From 77b23b82dbb76928f569f72c7267cca448ffd5b6 Mon Sep 17 00:00:00 2001 From: Eileen McNaughton Date: Tue, 24 May 2022 17:10:24 +1200 Subject: [PATCH] [Import] Test + fix for failure to reject invalid ex identifier --- CRM/Contact/Import/Parser/Contact.php | 137 +++++++++--------- .../CRM/Contact/Import/Parser/ContactTest.php | 55 ++++++- 2 files changed, 118 insertions(+), 74 deletions(-) diff --git a/CRM/Contact/Import/Parser/Contact.php b/CRM/Contact/Import/Parser/Contact.php index 2b65e44aea..13e2787823 100644 --- a/CRM/Contact/Import/Parser/Contact.php +++ b/CRM/Contact/Import/Parser/Contact.php @@ -298,17 +298,7 @@ class CRM_Contact_Import_Parser_Contact extends CRM_Import_Parser { $params['contact_sub_type'] = $this->getContactSubType() ?: ($params['contact_sub_type'] ?? NULL); try { - $params['id'] = $formatted['id'] = $this->lookupContactID($params, ($this->isSkipDuplicates() || $this->isIgnoreDuplicates())); - if ($params['id'] && $params['contact_sub_type']) { - $contactSubType = Contact::get(FALSE) - ->addWhere('id', '=', $params['id']) - ->addSelect('contact_sub_type') - ->execute() - ->first()['contact_sub_type']; - if (!empty($contactSubType) && $contactSubType[0] !== $params['contact_sub_type'] && !CRM_Contact_BAO_ContactType::isAllowEdit($params['id'], $contactSubType[0])) { - throw new CRM_Core_Exception('Mismatched contact SubTypes :', CRM_Import_Parser::NO_MATCH); - } - } + [$formatted, $params] = $this->processContact($params, $formatted); } catch (CRM_Core_Exception $e) { $statuses = [CRM_Import_Parser::DUPLICATE => 'DUPLICATE', CRM_Import_Parser::ERROR => 'ERROR', CRM_Import_Parser::NO_MATCH => 'invalid_no_match']; @@ -397,70 +387,19 @@ class CRM_Contact_Import_Parser_Contact extends CRM_Import_Parser { if ((CRM_Core_Error::isAPIError($newContact, CRM_Core_ERROR::DUPLICATE_CONTACT) || is_a($newContact, 'CRM_Contact_BAO_Contact')) && $primaryContactId) { //relationship contact insert - foreach ($params as $key => $field) { - [$id, $first, $second] = CRM_Utils_System::explode('_', $key, 3); - if (!($first == 'a' && $second == 'b') && !($first == 'b' && $second == 'a')) { - continue; + foreach ($this->getRelatedContactsParams($params) as $key => $field) { + $formatting = $field; + try { + [$formatting, $field] = $this->processContact($field, $formatting); } - - $relationType = new CRM_Contact_DAO_RelationshipType(); - $relationType->id = $id; - $relationType->find(TRUE); - $direction = "contact_sub_type_$second"; - - $formatting = array_filter(array_intersect_key($field, array_fill_keys($this->metadataHandledFields, 1))); - - //set subtype for related contact CRM-5125 - if (isset($relationType->$direction)) { - //validation of related contact subtype for update mode - if ($relCsType = CRM_Utils_Array::value('contact_sub_type', $params[$key]) && $relCsType != $relationType->$direction) { - $errorMessage = ts("Mismatched or Invalid contact subtype found for this related contact."); - array_unshift($values, $errorMessage); - return CRM_Import_Parser::NO_MATCH; - } - else { - $formatting['contact_sub_type'] = $relationType->$direction; - } + 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; } - $contactFields = NULL; $contactFields = CRM_Contact_DAO_Contact::import(); - //Relation on the basis of External Identifier. - if (empty($params[$key]['id']) && !empty($params[$key]['external_identifier'])) { - $params[$key]['id'] = CRM_Core_DAO::getFieldValue('CRM_Contact_DAO_Contact', $params[$key]['external_identifier'], 'id', 'external_identifier'); - } - // check for valid related contact id in update/fill mode, CRM-4424 - if (in_array($onDuplicate, [ - CRM_Import_Parser::DUPLICATE_UPDATE, - CRM_Import_Parser::DUPLICATE_FILL, - ]) && !empty($params[$key]['id'])) { - $relatedContactType = CRM_Core_DAO::getFieldValue('CRM_Contact_DAO_Contact', $params[$key]['id'], 'contact_type'); - if (!$relatedContactType) { - $errorMessage = ts("No contact found for this related contact ID: %1", [1 => $params[$key]['id']]); - array_unshift($values, $errorMessage); - return CRM_Import_Parser::NO_MATCH; - } - - //validation of related contact subtype for update mode - //CRM-5125 - $relatedCsType = NULL; - if (!empty($formatting['contact_sub_type'])) { - $relatedCsType = CRM_Core_DAO::getFieldValue('CRM_Contact_DAO_Contact', $params[$key]['id'], 'contact_sub_type'); - } - - if (!empty($relatedCsType) && (!CRM_Contact_BAO_ContactType::isAllowEdit($params[$key]['id'], $relatedCsType) && - $relatedCsType != CRM_Utils_Array::value('contact_sub_type', $formatting)) - ) { - $errorMessage = ts("Mismatched or Invalid contact subtype found for this related contact.") . ' ' . ts("ID: %1", [1 => $params[$key]['id']]); - array_unshift($values, $errorMessage); - return CRM_Import_Parser::NO_MATCH; - } - // get related contact id to format data in update/fill mode, - //if external identifier is present, CRM-4423 - $formatting['id'] = $params[$key]['id']; - } - //format common data, CRM-4062 $this->formatCommonData($field, $formatting, $contactFields); @@ -2039,7 +1978,11 @@ class CRM_Contact_Import_Parser_Contact extends CRM_Import_Parser { if ($relatedContactKey) { if (!isset($params[$relatedContactKey])) { - $params[$relatedContactKey] = ['contact_type' => $this->getRelatedContactType($mappedField['relationship_type_id'], $mappedField['relationship_direction'])]; + $params[$relatedContactKey] = [ + // These will be over-written by any the importer has chosen but defaults are based on the relationship. + 'contact_type' => $this->getRelatedContactType($mappedField['relationship_type_id'], $mappedField['relationship_direction']), + 'contact_sub_type' => $this->getRelatedContactSubType($mappedField['relationship_type_id'], $mappedField['relationship_direction']), + ]; } $this->addFieldToParams($params[$relatedContactKey], $locationValues, $fieldName, $importedValue); } @@ -2586,7 +2529,14 @@ class CRM_Contact_Import_Parser_Contact extends CRM_Import_Parser { $useExistingMatchFields = !empty($value['relationship_type_id']) || $this->isUpdateExistingContacts(); $prefixString = !empty($value['relationship_label']) ? '(' . $value['relationship_label'] . ') ' : ''; $this->validateRequiredContactFields($value['contact_type'], $value, $useExistingMatchFields, $prefixString); + $errors = array_merge($errors, $this->getInvalidValuesForContact($value, $prefixString)); + if (!empty($value['relationship_type_id'])) { + $requiredSubType = $this->getRelatedContactSubType($value['relationship_type_id'], $value['relationship_direction']); + if ($requiredSubType && $value['contact_sub_type'] && $requiredSubType !== $value['contact_sub_type']) { + throw new CRM_Core_Exception($prefixString . ts('Mismatched or Invalid contact subtype found for this related contact.')); + } + } } //check for duplicate external Identifier @@ -2685,6 +2635,24 @@ class CRM_Contact_Import_Parser_Contact extends CRM_Import_Parser { return $this->getRelationshipType($relationshipTypeID)[$relationshipField]; } + /** + * Get the related contact sub type. + * + * @param int|null $relationshipTypeID + * @param int|string $relationshipDirection + * + * @return null|string + * + * @throws \API_Exception + */ + protected function getRelatedContactSubType(int $relationshipTypeID, $relationshipDirection): ?string { + if (!$relationshipTypeID) { + return NULL; + } + $relationshipField = 'contact_sub_type_' . substr($relationshipDirection, -1); + return $this->getRelationshipType($relationshipTypeID)[$relationshipField]; + } + /** * Get the related contact type. * @@ -2863,6 +2831,9 @@ class CRM_Contact_Import_Parser_Contact extends CRM_Import_Parser { */ protected function lookupContactID(array $params, bool $isDuplicateIfExternalIdentifierExists): ?int { $extIDMatch = $this->lookupExternalIdentifier($params['external_identifier'] ?? NULL, $params['contact_type']); + if (!empty($params['external_identifier']) && !$extIDMatch && $isDuplicateIfExternalIdentifierExists) { + throw new CRM_Core_Exception(ts('Existing external ID lookup failed.'), CRM_Import_Parser::ERROR); + } $contactID = !empty($params['id']) ? (int) $params['id'] : NULL; //check if external identifier exists in database if ($extIDMatch && $contactID && $extIDMatch !== $contactID) { @@ -2893,4 +2864,28 @@ class CRM_Contact_Import_Parser_Contact extends CRM_Import_Parser { return NULL; } + /** + * @param array $params + * @param array $formatted + * @return array[] + * @throws \API_Exception + * @throws \CRM_Core_Exception + * @throws \CiviCRM_API3_Exception + * @throws \Civi\API\Exception\UnauthorizedException + */ + protected function processContact(array $params, array $formatted): array { + $params['id'] = $formatted['id'] = $this->lookupContactID($params, ($this->isSkipDuplicates() || $this->isIgnoreDuplicates())); + if ($params['id'] && $params['contact_sub_type']) { + $contactSubType = Contact::get(FALSE) + ->addWhere('id', '=', $params['id']) + ->addSelect('contact_sub_type') + ->execute() + ->first()['contact_sub_type']; + if (!empty($contactSubType) && $contactSubType[0] !== $params['contact_sub_type'] && !CRM_Contact_BAO_ContactType::isAllowEdit($params['id'], $contactSubType[0])) { + throw new CRM_Core_Exception('Mismatched contact SubTypes :', CRM_Import_Parser::NO_MATCH); + } + } + return array($formatted, $params); + } + } diff --git a/tests/phpunit/CRM/Contact/Import/Parser/ContactTest.php b/tests/phpunit/CRM/Contact/Import/Parser/ContactTest.php index ba8e4dbe19..7442194405 100644 --- a/tests/phpunit/CRM/Contact/Import/Parser/ContactTest.php +++ b/tests/phpunit/CRM/Contact/Import/Parser/ContactTest.php @@ -300,7 +300,7 @@ class CRM_Contact_Import_Parser_ContactTest extends CiviUnitTestCase { 'external_identifier' => 'billy', 'nick_name' => 'Old Bill', 'contact_sub_type' => 'Staff', - ], CRM_Import_Parser::DUPLICATE_UPDATE, NULL); + ], CRM_Import_Parser::DUPLICATE_UPDATE, FALSE); $contact = $this->callAPISuccessGetSingle('Contact', ['id' => $contactID]); $this->assertEquals('', $contact['nick_name']); $this->assertEquals(['Parent'], $contact['contact_sub_type']); @@ -1541,7 +1541,14 @@ class CRM_Contact_Import_Parser_ContactTest extends CiviUnitTestCase { $parser->setUserJobID($userJobID); $parser->_dedupeRuleGroupID = $ruleGroupId; $parser->init(); - $this->assertEquals($expectedResult, $parser->import($onDuplicateAction, $values), 'Return code from parser import was not as expected'); + $result = $parser->import($onDuplicateAction, $values); + $dataSource = new CRM_Import_DataSource_CSV($userJobID); + if ($result === FALSE && $expectedResult !== FALSE) { + // Import is moving away from returning a status - this is a better way to check + $this->assertGreaterThan(0, $dataSource->getRowCount([$expectedResult])); + return; + } + $this->assertEquals($expectedResult, $result, 'Return code from parser import was not as expected'); } /** @@ -1680,6 +1687,7 @@ class CRM_Contact_Import_Parser_ContactTest extends CiviUnitTestCase { ], '5_a_b' => [ 'contact_type' => 'Organization', + 'contact_sub_type' => NULL, 'website' => [ 'https://example.org' => [ 'url' => 'https://example.org', @@ -1911,7 +1919,12 @@ class CRM_Contact_Import_Parser_ContactTest extends CiviUnitTestCase { */ public function getImportedContacts(): array { return (array) Contact::get() - ->addWhere('display_name', 'IN', ['Susie Jones', 'Mum Jones', 'sis@example.com', 'Soccer Superstars']) + ->addWhere('display_name', 'IN', [ + 'Susie Jones', + 'Mum Jones', + 'sis@example.com', + 'Soccer Superstars', + ]) ->addChain('phone', Phone::get()->addWhere('contact_id', '=', '$id')) ->addChain('address', Address::get()->addWhere('contact_id', '=', '$id')) ->addChain('website', Website::get()->addWhere('contact_id', '=', '$id')) @@ -1921,4 +1934,40 @@ class CRM_Contact_Import_Parser_ContactTest extends CiviUnitTestCase { ->execute()->indexBy('display_name'); } + /** + * Test that import parser will not throw error if Related Contact is not found via passed in External ID. + * + * Currently fails because validation assumes the Related contact will be found. + * When it is later not found creating the contact via the API throws an + * error for missing required fields. + * + * @throws \API_Exception + * @throws \CRM_Core_Exception + * @throws \CiviCRM_API3_Exception + */ + public function testImportParserWithExternalIdForRelationship(): void { + $contactImportValues = [ + 'first_name' => 'Alok', + 'last_name' => 'Patel', + 'Employee of' => 'related external identifier', + ]; + + $mapper = [ + ['first_name'], + ['last_name'], + ['5_a_b', 'external_identifier'], + ]; + $fields = array_keys($contactImportValues); + $values = array_values($contactImportValues); + $userJobID = $this->getUserJobID([ + 'mapper' => $mapper, + ]); + + $parser = new CRM_Contact_Import_Parser_Contact($fields); + $parser->setUserJobID($userJobID); + $parser->init(); + + $parser->import(CRM_Import_Parser::DUPLICATE_UPDATE, $values); + } + } -- 2.25.1