From 7bb22e4c41120a94d884190edc854bf678fb90a2 Mon Sep 17 00:00:00 2001 From: Eileen McNaughton Date: Wed, 24 Aug 2022 21:53:15 +1200 Subject: [PATCH] [Ref] Import cleanup code that looks up contribution in import --- CRM/Contribute/Import/Parser/Contribution.php | 159 ++++++++++-------- CRM/Import/Parser.php | 3 + .../Import/Parser/ContributionTest.php | 40 +++-- 3 files changed, 118 insertions(+), 84 deletions(-) diff --git a/CRM/Contribute/Import/Parser/Contribution.php b/CRM/Contribute/Import/Parser/Contribution.php index f5d6200f5b..627f4ec7e0 100644 --- a/CRM/Contribute/Import/Parser/Contribution.php +++ b/CRM/Contribute/Import/Parser/Contribution.php @@ -16,6 +16,7 @@ */ use Civi\Api4\Contact; +use Civi\Api4\Contribution; use Civi\Api4\Email; /** @@ -23,8 +24,6 @@ use Civi\Api4\Email; */ class CRM_Contribute_Import_Parser_Contribution extends CRM_Import_Parser { - protected $_mapperKeys; - /** * Array of successfully imported contribution id's * @@ -32,16 +31,6 @@ class CRM_Contribute_Import_Parser_Contribution extends CRM_Import_Parser { */ protected $_newContributions; - /** - * Class constructor. - * - * @param $mapperKeys - */ - public function __construct($mapperKeys = []) { - parent::__construct(); - $this->_mapperKeys = $mapperKeys; - } - /** * Get information about the provided job. * - name @@ -188,12 +177,38 @@ class CRM_Contribute_Import_Parser_Contribution extends CRM_Import_Parser { $params['soft_credit'][$i] = ['soft_credit_type_id' => $mappedField['soft_credit_type_id'], $mappedField['soft_credit_match_field'] => $values[$i]]; } else { - $params[$this->getFieldMetadata($mappedField['name'])['name']] = $this->getTransformedFieldValue($mappedField['name'], $values[$i]); + $entity = $this->getFieldMetadata($mappedField['name'])['entity'] ?? 'Contribution'; + $params[$entity][$this->getFieldMetadata($mappedField['name'])['name']] = $this->getTransformedFieldValue($mappedField['name'], $values[$i]); } } return $params; } + /** + * Override parent to cope with params being separated by entity already. + * + * @todo - make this the parent method... + * + * @param array $params + * + * @throws \CRM_Core_Exception + */ + protected function validateParams(array $params): void { + + if (empty($params['Contribution']['id'])) { + $this->validateRequiredFields($this->getRequiredFields(), $params['Contribution']); + } + $errors = []; + foreach ($params as $entity => $values) { + foreach ($values as $key => $value) { + $errors = array_merge($this->getInvalidValues($value, $key), $errors); + } + } + if ($errors) { + throw new CRM_Core_Exception('Invalid value for field(s) : ' . implode(',', $errors)); + } + } + /** * The initializer code, called before the processing */ @@ -229,7 +244,9 @@ class CRM_Contribute_Import_Parser_Contribution extends CRM_Import_Parser { 'title' => ts('Pledge ID'), 'headerPattern' => '/Pledge ID/i', 'name' => 'pledge_id', - 'entity' => 'Pledge', + // This is handled as a contribution field & the goal is + // to make it pseudofield on the contribution. + 'entity' => 'Contribution', 'type' => CRM_Utils_Type::T_INT, 'options' => FALSE, ], @@ -240,7 +257,6 @@ class CRM_Contribute_Import_Parser_Contribution extends CRM_Import_Parser { foreach ($fields as $name => $field) { $fields[$name] = array_merge([ 'type' => CRM_Utils_Type::T_INT, - 'dataPattern' => '//', 'headerPattern' => '//', ], $field); } @@ -282,10 +298,24 @@ class CRM_Contribute_Import_Parser_Contribution extends CRM_Import_Parser { public function import($values): void { $rowNumber = (int) ($values[array_key_last($values)]); try { - $params = $this->getMappedRow($values); - if (!empty($params['contact_id'])) { - $this->validateContactID($params['contact_id'], $this->getContactType()); + $entityKeyedParams = $this->getMappedRow($values); + $entityKeyedParams['Contribution']['id'] = $this->lookupContributionID($entityKeyedParams['Contribution']); + if (empty($entityKeyedParams['Contribution']['id']) && $this->isUpdateExisting()) { + throw new CRM_Core_Exception('Empty Contribution and Invoice and Transaction ID. Row was skipped.', CRM_Import_Parser::ERROR); + } + if (!empty($params['Contact']['contact_id'])) { + $this->validateContactID($params['Contact']['contact_id'], $this->getContactType()); + } + // @todo - here we flatten the entities back into a single array. + // The entity format is better but the code below needs to be migrated. + $params = []; + foreach (['Contact', 'Contribution', 'Note'] as $entity) { + $params = array_merge($params, ($entityKeyedParams[$entity] ?? [])); + } + if (isset($entityKeyedParams['soft_credit'])) { + $params['soft_credit'] = $entityKeyedParams['soft_credit']; } + $formatted = array_merge(['version' => 3, 'skipRecentView' => TRUE, 'skipCleanMoney' => TRUE, 'contribution_id' => $params['id'] ?? NULL], $params); //CRM-10994 if (isset($params['total_amount']) && $params['total_amount'] == 0) { @@ -307,7 +337,7 @@ class CRM_Contribute_Import_Parser_Contribution extends CRM_Import_Parser { $paramValues['contact_type'] = $this->getContactType(); } elseif ($this->isUpdateExisting() && - (!empty($paramValues['id']) || !empty($values['trxn_id']) || !empty($paramValues['invoice_id'])) + (!empty($paramValues['id'])) ) { $paramValues['contact_type'] = $this->getContactType(); } @@ -320,23 +350,17 @@ class CRM_Contribute_Import_Parser_Contribution extends CRM_Import_Parser { if ($this->isUpdateExisting()) { //fix for CRM-2219 - Update Contribution // onDuplicate == CRM_Import_Parser::DUPLICATE_UPDATE - if (!empty($paramValues['invoice_id']) || !empty($paramValues['trxn_id']) || !empty($paramValues['id'])) { - $dupeIds = [ - 'id' => $paramValues['id'] ?? NULL, - 'trxn_id' => $paramValues['trxn_id'] ?? NULL, - 'invoice_id' => $paramValues['invoice_id'] ?? NULL, - ]; - $ids['contribution'] = CRM_Contribute_BAO_Contribution::checkDuplicateIds($dupeIds); - - if ($ids['contribution']) { - $formatted['id'] = $ids['contribution']; + if (!empty($paramValues['id'])) { + // todo Remove if in separate PR + if (TRUE) { + $formatted['id'] = $paramValues['id']; //process note if (!empty($paramValues['note'])) { $noteID = []; - $contactID = CRM_Core_DAO::getFieldValue('CRM_Contribute_DAO_Contribution', $ids['contribution'], 'contact_id'); + $contactID = CRM_Core_DAO::getFieldValue('CRM_Contribute_DAO_Contribution', $paramValues['id'], 'contact_id'); $daoNote = new CRM_Core_BAO_Note(); $daoNote->entity_table = 'civicrm_contribution'; - $daoNote->entity_id = $ids['contribution']; + $daoNote->entity_id = $paramValues['id']; if ($daoNote->find(TRUE)) { $noteID['id'] = $daoNote->id; } @@ -344,7 +368,7 @@ class CRM_Contribute_Import_Parser_Contribution extends CRM_Import_Parser { $noteParams = [ 'entity_table' => 'civicrm_contribution', 'note' => $paramValues['note'], - 'entity_id' => $ids['contribution'], + 'entity_id' => $paramValues['id'], 'contact_id' => $contactID, ]; CRM_Core_BAO_Note::add($noteParams, $noteID); @@ -355,7 +379,7 @@ class CRM_Contribute_Import_Parser_Contribution extends CRM_Import_Parser { if (!empty($formatted['soft_credit'])) { $dupeSoftCredit = [ 'contact_id' => $formatted['soft_credit'], - 'contribution_id' => $ids['contribution'], + 'contribution_id' => $paramValues['id'], ]; //Delete all existing soft Contribution from contribution_soft table for pcp_id is_null @@ -372,7 +396,7 @@ class CRM_Contribute_Import_Parser_Contribution extends CRM_Import_Parser { } } - $formatted['id'] = $ids['contribution']; + $formatted['id'] = $paramValues['id']; $newContribution = civicrm_api3('contribution', 'create', $formatted); $this->_newContributions[] = $newContribution['id']; @@ -386,18 +410,6 @@ class CRM_Contribute_Import_Parser_Contribution extends CRM_Import_Parser { $this->setImportStatus($rowNumber, $this->processPledgePayments($formatted) ? $this->getStatus(self::PLEDGE_PAYMENT) : $this->getStatus(self::VALID), '', $newContribution['id']); return; } - $labels = [ - 'id' => 'Contribution ID', - 'trxn_id' => 'Transaction ID', - 'invoice_id' => 'Invoice ID', - ]; - foreach ($dupeIds as $k => $v) { - if ($v) { - $errorMsg[] = "$labels[$k] $v"; - } - } - $errorMsg = implode(' AND ', $errorMsg); - throw new CRM_Core_Exception('Matching Contribution record not found for ' . $errorMsg . '. Row was skipped.', CRM_Import_Parser::ERROR); } } @@ -509,6 +521,34 @@ class CRM_Contribute_Import_Parser_Contribution extends CRM_Import_Parser { } } + /** + * Lookup pre-existing contribution ID. + * + * @param array $params + * + * @throws \CRM_Core_Exception + * + * @return int|null + */ + private function lookupContributionID(array $params): ?int { + $where = []; + $labels = []; + foreach (['id' => 'Contribution ID', 'trxn_id' => 'Transaction ID', 'invoice_id' => 'Invoice ID'] as $field => $label) { + if (!empty($params[$field])) { + $where[] = [$field, '=', $params[$field]]; + $labels[] = $label . ' ' . $params[$field]; + } + } + if (empty($where)) { + return NULL; + } + $contribution = Contribution::get(FALSE)->setWhere($where)->addSelect('id')->execute()->first(); + if ($contribution['id'] ?? NULL) { + return $contribution['id']; + } + throw new CRM_Core_Exception('Matching Contribution record not found for ' . implode(' AND ', $labels) . '. Row was skipped.', CRM_Import_Parser::ERROR); + } + /** * Get the status to record. * @@ -641,7 +681,7 @@ class CRM_Contribute_Import_Parser_Contribution extends CRM_Import_Parser { $params['contribution_contact_id'] = $matchingContactIds[0]; } } - elseif (!empty($params['id']) || !empty($params['trxn_id']) || !empty($params['invoice_id'])) { + elseif (!empty($params['id'])) { // when update mode check contribution id or trxn id or // invoice id // @todo - this check is obsolete. It survives for now @@ -650,12 +690,6 @@ class CRM_Contribute_Import_Parser_Contribution extends CRM_Import_Parser { if (!empty($params['id'])) { $contactId->id = $params['id']; } - elseif (!empty($params['trxn_id'])) { - $contactId->trxn_id = $params['trxn_id']; - } - elseif (!empty($params['invoice_id'])) { - $contactId->invoice_id = $params['invoice_id']; - } if ($contactId->find(TRUE)) { $contactType->id = $contactId->contact_id; if ($contactType->find(TRUE)) { @@ -665,11 +699,6 @@ class CRM_Contribute_Import_Parser_Contribution extends CRM_Import_Parser { } } } - else { - if ($this->isUpdateExisting()) { - throw new CRM_Core_Exception('Empty Contribution and Invoice and Transaction ID. Row was skipped.', CRM_Import_Parser::ERROR); - } - } break; case 'soft_credit': @@ -690,18 +719,10 @@ class CRM_Contribute_Import_Parser_Contribution extends CRM_Import_Parser { // retrieve pledge details as well as to validate pledge ID // first need to check for update mode - if ($this->isUpdateExisting() && - ($params['id'] || $params['trxn_id'] || $params['invoice_id']) - ) { + if (!empty($params['id'])) { $contribution = new CRM_Contribute_DAO_Contribution(); - if ($params['contribution_id']) { - $contribution->id = $params['contribution_id']; - } - elseif ($params['trxn_id']) { - $contribution->trxn_id = $params['trxn_id']; - } - elseif ($params['invoice_id']) { - $contribution->invoice_id = $params['invoice_id']; + if ($params['id']) { + $contribution->id = $params['id']; } if ($contribution->find(TRUE)) { diff --git a/CRM/Import/Parser.php b/CRM/Import/Parser.php index a0a65b7875..f6f71be5ae 100644 --- a/CRM/Import/Parser.php +++ b/CRM/Import/Parser.php @@ -335,6 +335,9 @@ abstract class CRM_Import_Parser implements UserJobInterface { $tmpContactField[$value] = $contactFields[$value]; $title = $tmpContactField[$value]['title'] . ' ' . ts('(match to contact)'); $tmpContactField[$value]['title'] = $title; + // When we switch to apiv4 getfields this will already be set for + // all fields (including custom which it isn't yet) + $tmpContactField[$value]['entity'] = 'Contact'; } } diff --git a/tests/phpunit/CRM/Contribute/Import/Parser/ContributionTest.php b/tests/phpunit/CRM/Contribute/Import/Parser/ContributionTest.php index 1a821648c7..831adf6ce5 100644 --- a/tests/phpunit/CRM/Contribute/Import/Parser/ContributionTest.php +++ b/tests/phpunit/CRM/Contribute/Import/Parser/ContributionTest.php @@ -75,7 +75,7 @@ class CRM_Contribute_Import_Parser_ContributionTest extends CiviUnitTestCase { ['name' => 'external_identifier'], ['name' => 'soft_credit', 'soft_credit_type_id' => 1, 'soft_credit_match_field' => 'external_identifier'], ]; - $this->importCSV('contributions_amount_validate.csv', $mapping); + $this->importCSV('contributions_amount_validate.csv', $mapping, ['onDuplicate' => CRM_Import_Parser::DUPLICATE_SKIP]); $contributionsOfMainContact = Contribution::get()->addWhere('contact_id', '=', $contact1Id)->execute(); // Although there are 2 rows in the csv, 1 should fail each time due to conflicting money formats. @@ -95,47 +95,52 @@ class CRM_Contribute_Import_Parser_ContributionTest extends CiviUnitTestCase { /** * Test payment types are passed. * - * Note that the expected result should logically be CRM_Import_Parser::valid but writing test to reflect not fix here + * Note that the expected result should logically be CRM_Import_Parser::valid + * but writing test to reflect not fix here + * + * @throws \CRM_Core_Exception */ public function testPaymentTypeLabel(): void { $this->addRandomOption(); $contactID = $this->individualCreate(); $values = ['contribution_contact_id' => $contactID, 'total_amount' => 10, 'financial_type_id' => 'Donation', 'payment_instrument_id' => 'Check']; - $this->runImport($values, CRM_Import_Parser::DUPLICATE_UPDATE, NULL); + $this->runImport($values, CRM_Import_Parser::DUPLICATE_SKIP); $contribution = $this->callAPISuccessGetSingle('Contribution', ['contact_id' => $contactID]); $this->assertEquals('Check', $contribution['payment_instrument']); $values = ['contribution_contact_id' => $contactID, 'total_amount' => 10, 'financial_type_id' => 'Donation', 'payment_instrument_id' => 'not at all random']; - $this->runImport($values, CRM_Import_Parser::DUPLICATE_UPDATE, NULL); + $this->runImport($values, CRM_Import_Parser::DUPLICATE_SKIP); $contribution = $this->callAPISuccessGetSingle('Contribution', ['contact_id' => $contactID, 'payment_instrument_id' => 'random']); $this->assertEquals('not at all random', $contribution['payment_instrument']); } /** * Test handling of contribution statuses. + * + * @throws \CRM_Core_Exception */ public function testContributionStatusLabel(): void { $contactID = $this->individualCreate(); $values = ['contribution_contact_id' => $contactID, 'total_amount' => 10, 'financial_type_id' => 'Donation', 'payment_instrument_id' => 'Check', 'contribution_status_id' => 'Pending']; // Note that the expected result should logically be CRM_Import_Parser::valid but writing test to reflect not fix here - $this->runImport($values, CRM_Import_Parser::DUPLICATE_UPDATE, NULL); + $this->runImport($values, CRM_Import_Parser::DUPLICATE_SKIP, NULL); $contribution = $this->callAPISuccessGetSingle('Contribution', ['contact_id' => $contactID]); $this->assertEquals('Pending Label**', $contribution['contribution_status']); $this->addRandomOption('contribution_status'); $values['contribution_status_id'] = 'not at all random'; - $this->runImport($values, CRM_Import_Parser::DUPLICATE_UPDATE); + $this->runImport($values, CRM_Import_Parser::DUPLICATE_SKIP); $contribution = $this->callAPISuccessGetSingle('Contribution', ['contact_id' => $contactID, 'contribution_status_id' => 'random']); $this->assertEquals('not at all random', $contribution['contribution_status']); $values['contribution_status_id'] = 'just say no'; - $this->runImport($values, CRM_Import_Parser::DUPLICATE_UPDATE); + $this->runImport($values, CRM_Import_Parser::DUPLICATE_SKIP); $this->callAPISuccessGetCount('Contribution', ['contact_id' => $contactID], 2); // Per https://lab.civicrm.org/dev/core/issues/1285 it's a bit arguable but Ok we can support id... $values['contribution_status_id'] = 3; - $this->runImport($values, CRM_Import_Parser::DUPLICATE_UPDATE, NULL); + $this->runImport($values, CRM_Import_Parser::DUPLICATE_SKIP); $this->callAPISuccessGetCount('Contribution', ['contact_id' => $contactID, 'contribution_status_id' => 3], 1); } @@ -159,11 +164,14 @@ class CRM_Contribute_Import_Parser_ContributionTest extends CiviUnitTestCase { $this->assertEquals('2019-10-20 00:00:00', $contribution[$this->getCustomFieldName('date')]); } + /** + * @throws \CRM_Core_Exception + */ public function testParsedCustomOption(): void { $contactID = $this->individualCreate(); $values = ['contribution_contact_id' => $contactID, 'total_amount' => 10, 'financial_type_id' => 'Donation', 'payment_instrument_id' => 'Check', 'contribution_status_id' => 'Pending']; // Note that the expected result should logically be CRM_Import_Parser::valid but writing test to reflect not fix here - $this->runImport($values, CRM_Import_Parser::DUPLICATE_UPDATE, NULL); + $this->runImport($values, CRM_Import_Parser::DUPLICATE_SKIP, NULL); $contribution = $this->callAPISuccess('Contribution', 'getsingle', ['contact_id' => $contactID]); $this->createCustomGroupWithFieldOfType([], 'radio'); $values['contribution_id'] = $contribution['id']; @@ -241,6 +249,8 @@ class CRM_Contribute_Import_Parser_ContributionTest extends CiviUnitTestCase { /** * Test custom multi-value checkbox field is imported properly. + * + * @throws \CRM_Core_Exception */ public function testCustomSerializedCheckBox(): void { $this->createCustomGroupWithFieldOfType([], 'checkbox'); @@ -311,8 +321,8 @@ class CRM_Contribute_Import_Parser_ContributionTest extends CiviUnitTestCase { $contactSapphireId = $this->individualCreate($contactSapphireParams); // make sure we're testing dev/core#3784 - self::assertEquals(1, substr($contactRubyId, 0, 1)); - self::assertEquals(1, substr($contactSapphireId, 0, 1)); + $this->assertEquals(1, substr($contactRubyId, 0, 1)); + $this->assertEquals(1, substr($contactSapphireId, 0, 1)); $mapping = [ ['name' => 'external_identifier'], @@ -347,6 +357,8 @@ class CRM_Contribute_Import_Parser_ContributionTest extends CiviUnitTestCase { * @param array|null $mappings * @param array|null $fields * Array of field names. Will be calculated from $originalValues if not passed in. + * + * @throws \CRM_Core_Exception */ protected function runImport(array $originalValues, int $onDuplicateAction, ?int $expectedResult = NULL, array $mappings = [], array $fields = NULL): void { if (!$fields) { @@ -432,12 +444,10 @@ class CRM_Contribute_Import_Parser_ContributionTest extends CiviUnitTestCase { $rows = $this->getDataSource()->getRows(); foreach ($rows as $row) { if ($row[8] === 'valid') { - // Note that as the valid value is 'IMPORTED' - but the fix to make that correct - // is not in 5.53 so temporary support for pledge_payment_imported - $this->assertContains($row[10], ['pledge_payment_imported', 'IMPORTED'], $row[11]); + $this->assertEquals('IMPORTED', $row[10], $row[11]); } else { - $this->assertEquals('ERROR', $row[10], $row[11]); + $this->assertEquals('ERROR', $row[10], $row[11] . print_r($rows, TRUE)); } } } -- 2.25.1