From 726695357cb3f524d395431eb12b3bc025f82cd3 Mon Sep 17 00:00:00 2001 From: Eileen McNaughton Date: Tue, 13 Sep 2022 13:11:48 +1200 Subject: [PATCH] Fix unreleased regression on soft credit imports Fixes an unreleased regression whereby it was only permitting an email match with the same contact type as the main contact for a soft credit import --- CRM/Contribute/Import/Parser/Contribution.php | 27 +++++----- CRM/Import/Parser.php | 51 +++++++++++++----- .../Import/Parser/ContributionTest.php | 53 ++++++++++++++++--- .../data/contributions_amount_validate.csv | 6 +-- 4 files changed, 101 insertions(+), 36 deletions(-) diff --git a/CRM/Contribute/Import/Parser/Contribution.php b/CRM/Contribute/Import/Parser/Contribution.php index 46940b7343..1a9cda70b5 100644 --- a/CRM/Contribute/Import/Parser/Contribution.php +++ b/CRM/Contribute/Import/Parser/Contribution.php @@ -194,22 +194,20 @@ class CRM_Contribute_Import_Parser_Contribution extends CRM_Import_Parser { if ($mappedField['name'] === 'do_not_import' || !$mappedField['name']) { continue; } - if (!empty($mappedField['soft_credit_match_field'])) { - $params['soft_credit'][$i] = ['soft_credit_type_id' => $mappedField['soft_credit_type_id'], $mappedField['soft_credit_match_field'] => $values[$i]]; + $fieldSpec = $this->getFieldMetadata($mappedField['name']); + $entity = $fieldSpec['entity_instance'] ?? ($fieldSpec['entity'] ?? 'Contribution'); + // If we move this to the parent we can check if the entity config 'supports_multiple' + if ($entity === 'SoftCreditContact') { + $entityKey = json_encode($mappedField['entity_data']); + $entityInstance = $params[$entity][$entityKey] ?? $mappedField['entity_data']['soft_credit']; + $entityInstance['Contact'] = array_merge($entityInstance['Contact'] ?? [], [$this->getFieldMetadata($mappedField['name'])['name'] => $this->getTransformedFieldValue($mappedField['name'], $values[$i])]); + $params[$entity][$entityKey] = $entityInstance; } else { - $fieldSpec = $this->getFieldMetadata($mappedField['name']); - $entity = $fieldSpec['entity_instance'] ?? ($fieldSpec['entity'] ?? 'Contribution'); - // If we move this to the parent we can check if the entity config 'supports_multiple' - if ($entity === 'SoftCreditContact') { - $entityKey = json_encode($mappedField['entity_data']); - $entityInstance = $params[$entity][$entityKey] ?? $mappedField['entity_data']['soft_credit']; - $entityInstance['Contact'] = array_merge($entityInstance['Contact'] ?? [], [$this->getFieldMetadata($mappedField['name'])['name'] => $this->getTransformedFieldValue($mappedField['name'], $values[$i])]); - $params[$entity][$entityKey] = $entityInstance; - } - else { - $params[$entity][$this->getFieldMetadata($mappedField['name'])['name']] = $this->getTransformedFieldValue($mappedField['name'], $values[$i]); + if ($entity === 'Contact' && !isset($params[$entity])) { + $params[$entity] = $this->getContactType() ? ['contact_type' => $this->getContactType()] : []; } + $params[$entity][$this->getFieldMetadata($mappedField['name'])['name']] = $this->getTransformedFieldValue($mappedField['name'], $values[$i]); } } return $params; @@ -413,6 +411,9 @@ class CRM_Contribute_Import_Parser_Contribution extends CRM_Import_Parser { foreach ($params['SoftCreditContact'] ?? [] as $index => $softCreditContact) { $softCreditParams[$index]['soft_credit_type_id'] = $softCreditContact['soft_credit_type_id']; $softCreditParams[$index]['contact_id'] = $this->getContactID($softCreditContact['Contact'], $softCreditContact['id'] ?? NULL, 'SoftCreditContact'); + if (empty($softCreditParams[$index]['contact_id'])) { + throw new CRM_Core_Exception(ts('Soft Credit Contact not found')); + } } $this->deprecatedFormatParams($contributionParams, $contributionParams); diff --git a/CRM/Import/Parser.php b/CRM/Import/Parser.php index a463aff955..c76c845211 100644 --- a/CRM/Import/Parser.php +++ b/CRM/Import/Parser.php @@ -2162,7 +2162,8 @@ abstract class CRM_Import_Parser implements UserJobInterface { * have that external_identifier. * * @param string|null $externalIdentifier - * @param string $contactType + * @param string|null $contactType + * If supplied the contact will be validated against this type. * @param int|null $contactID * * @return int|null @@ -2170,7 +2171,7 @@ abstract class CRM_Import_Parser implements UserJobInterface { * @throws \CRM_Core_Exception * @throws \CiviCRM_API3_Exception */ - protected function lookupExternalIdentifier(?string $externalIdentifier, string $contactType, ?int $contactID): ?int { + protected function lookupExternalIdentifier(?string $externalIdentifier, ?string $contactType, ?int $contactID): ?int { if (!$externalIdentifier) { return NULL; } @@ -2242,16 +2243,19 @@ abstract class CRM_Import_Parser implements UserJobInterface { } } } - $dedupeRule = $dedupeRuleID ? $this->getDedupeRuleName($dedupeRuleID) : $this->getDefaultRuleForContactType($params['contact_type']); - $possibleMatches = Contact::getDuplicates(FALSE) - ->setValues($params) - ->setDedupeRule($dedupeRule) - ->execute(); - $matchIDs = []; - foreach ($possibleMatches as $possibleMatch) { - $matchIDs[(int) $possibleMatch['id']] = (int) $possibleMatch['id']; + $dedupeRules = $this->getDedupeRules((array) $dedupeRuleID, $params['contact_type'] ?? NULL); + foreach ($dedupeRules as $dedupeRule) { + $possibleMatches = Contact::getDuplicates(FALSE) + ->setValues($params) + ->setDedupeRule($dedupeRule) + ->execute(); + + foreach ($possibleMatches as $possibleMatch) { + $matchIDs[(int) $possibleMatch['id']] = (int) $possibleMatch['id']; + } } + return $matchIDs; } @@ -2292,7 +2296,7 @@ abstract class CRM_Import_Parser implements UserJobInterface { * @throws \CRM_Core_Exception */ protected function getContactID(array $contactParams, ?int $contactID, string $entity): ?int { - $contactType = $contactParams['contact_type'] ?? $this->getContactType(); + $contactType = $contactParams['contact_type'] ?? NULL; if ($contactID) { $this->validateContactID($contactID, $contactType); } @@ -2301,7 +2305,6 @@ abstract class CRM_Import_Parser implements UserJobInterface { } if (!$contactID) { $action = $this->getActionForEntity($entity); - $contactParams['contact_type'] = $contactType; $possibleMatches = $this->getPossibleMatchesByDedupeRule($contactParams); if (count($possibleMatches) === 1) { $contactID = array_key_first($possibleMatches); @@ -2496,4 +2499,28 @@ abstract class CRM_Import_Parser implements UserJobInterface { } } + /** + * Get the dedupe rules to use to lookup a contact. + * + * @param array $dedupeRuleIDs + * @param string|array|null $contact_type + * + * @return array + * @throws \CRM_Core_Exception + */ + protected function getDedupeRules(array $dedupeRuleIDs, $contact_type) { + $dedupeRules = []; + if (!empty($dedupeRuleIDs)) { + foreach ($dedupeRuleIDs as $dedupeRuleID) { + $dedupeRules[] = $this->getDedupeRuleName($dedupeRuleID); + } + return $dedupeRules; + } + $contactTypes = $contact_type ? (array) $contact_type : CRM_Contact_BAO_ContactType::basicTypes(); + foreach ($contactTypes as $contactType) { + $dedupeRules[] = $this->getDefaultRuleForContactType($contactType); + } + return $dedupeRules; + } + } diff --git a/tests/phpunit/CRM/Contribute/Import/Parser/ContributionTest.php b/tests/phpunit/CRM/Contribute/Import/Parser/ContributionTest.php index 31cf95bd6d..860486b257 100644 --- a/tests/phpunit/CRM/Contribute/Import/Parser/ContributionTest.php +++ b/tests/phpunit/CRM/Contribute/Import/Parser/ContributionTest.php @@ -59,20 +59,18 @@ class CRM_Contribute_Import_Parser_ContributionTest extends CiviUnitTestCase { */ public function testImportParserWithSoftCreditsByExternalIdentifier(string $thousandSeparator): void { $this->setCurrencySeparators($thousandSeparator); - $contact1Params = [ + $mainContactID = $this->individualCreate([ 'first_name' => 'Contact', 'last_name' => 'One', 'external_identifier' => 'ext-1', 'contact_type' => 'Individual', - ]; - $contact2Params = [ + ]); + $softCreditContactID = $this->individualCreate([ 'first_name' => 'Contact', 'last_name' => 'Two', 'external_identifier' => 'ext-2', 'contact_type' => 'Individual', - ]; - $contact1Id = $this->individualCreate($contact1Params); - $contact2Id = $this->individualCreate($contact2Params); + ]); $mapping = [ ['name' => 'total_amount'], @@ -83,14 +81,14 @@ class CRM_Contribute_Import_Parser_ContributionTest extends CiviUnitTestCase { ]; $this->importCSV('contributions_amount_validate.csv', $mapping, ['onDuplicate' => CRM_Import_Parser::DUPLICATE_SKIP]); - $contributionsOfMainContact = Contribution::get()->addWhere('contact_id', '=', $contact1Id)->execute(); + $contributionsOfMainContact = Contribution::get()->addWhere('contact_id', '=', $mainContactID)->execute(); // Although there are 2 rows in the csv, 1 should fail each time due to conflicting money formats. $this->assertCount(1, $contributionsOfMainContact, 'Wrong number of contributions imported'); $this->assertEquals(1230.99, $contributionsOfMainContact->first()['total_amount']); $this->assertEquals(1230.99, $contributionsOfMainContact->first()['net_amount']); $this->assertEquals(0, $contributionsOfMainContact->first()['fee_amount']); - $contributionsOfSoftContact = ContributionSoft::get()->addWhere('contact_id', '=', $contact2Id)->execute(); + $contributionsOfSoftContact = ContributionSoft::get()->addWhere('contact_id', '=', $softCreditContactID)->execute(); $this->assertCount(1, $contributionsOfSoftContact, 'Contribution Soft not added for primary contact'); $dataSource = new CRM_Import_DataSource_CSV($this->userJobID); $this->assertEquals(1, $dataSource->getRowCount([CRM_Import_Parser::ERROR])); @@ -98,6 +96,45 @@ class CRM_Contribute_Import_Parser_ContributionTest extends CiviUnitTestCase { $this->assertEquals(1, $dataSource->getRowCount([CRM_Import_Parser::VALID])); } + /** + * Test import parser can add to a soft credit contact of a different type. + * + * @throws \CRM_Core_Exception + */ + public function testImportParserWithSoftCreditDifferentContactType(): void { + $mainContactID = $this->individualCreate([ + 'first_name' => 'Contact', + 'last_name' => 'One', + 'email' => 'harry@example.com', + 'external_identifier' => 'ext-1', + 'contact_type' => 'Individual', + ]); + $softCreditContactID = $this->individualCreate([ + 'organization_name' => 'The firm', + 'external_identifier' => 'ext-2', + 'email' => 'the-firm@example.com', + 'contact_type' => 'Organization', + ]); + + $mapping = [ + ['name' => 'total_amount'], + ['name' => 'receive_date'], + ['name' => 'financial_type_id'], + ['name' => ''], + ['name' => ''], + ['name' => 'email_primary.email'], + ['name' => 'soft_credit.contact.email_primary.email', 'soft_credit_type_id' => 1], + ]; + $this->importCSV('contributions_amount_validate.csv', $mapping, ['onDuplicate' => CRM_Import_Parser::DUPLICATE_SKIP]); + + $contributionsOfMainContact = Contribution::get()->addWhere('contact_id', '=', $mainContactID)->execute(); + // Although there are 2 rows in the csv, 1 should fail each time due to conflicting money formats. + $this->assertCount(1, $contributionsOfMainContact, 'Wrong number of contributions imported'); + + $contributionsOfSoftContact = ContributionSoft::get()->addWhere('contact_id', '=', $softCreditContactID)->execute(); + $this->assertCount(1, $contributionsOfSoftContact, 'Contribution Soft not added for primary contact'); + } + /** * Test payment types are passed. * diff --git a/tests/phpunit/CRM/Contribute/Import/Parser/data/contributions_amount_validate.csv b/tests/phpunit/CRM/Contribute/Import/Parser/data/contributions_amount_validate.csv index a940169109..bc933de80f 100644 --- a/tests/phpunit/CRM/Contribute/Import/Parser/data/contributions_amount_validate.csv +++ b/tests/phpunit/CRM/Contribute/Import/Parser/data/contributions_amount_validate.csv @@ -1,3 +1,3 @@ -Total Amount,Receive Date,Financial Type,External identifier,Soft Credit Ext ID -"1,230.99",2008-09-20,Donation,ext-1,ext-2 -"1.230,99",2008-09-20,Donation,ext-1,ext-2 +Total Amount,Receive Date,Financial Type,External identifier,Soft Credit Ext ID,Email,Email - soft credit +"1,230.99",2008-09-20,Donation,ext-1,ext-2,harry@example.com,the-firm@example.com +"1.230,99",2008-09-20,Donation,ext-1,ext-2,, -- 2.25.1