From a0d25688fbd2224ea0f36332e51f87ab132bc0ab Mon Sep 17 00:00:00 2001
From: Eileen McNaughton <emcnaughton@wikimedia.org>
Date: Wed, 24 Aug 2022 15:52:20 +1200
Subject: [PATCH] Import - fix regression on matching by contribution ID

---
 CRM/Contribute/Import/Parser/Contribution.php | 14 ++++---
 .../Import/Parser/ContributionTest.php        | 41 +++++++++++++++++++
 .../Parser/data/contributions_update.csv      |  7 ++++
 3 files changed, 56 insertions(+), 6 deletions(-)
 create mode 100644 tests/phpunit/CRM/Contribute/Import/Parser/data/contributions_update.csv

diff --git a/CRM/Contribute/Import/Parser/Contribution.php b/CRM/Contribute/Import/Parser/Contribution.php
index 9c4a4bbd24..eed146f4cb 100644
--- a/CRM/Contribute/Import/Parser/Contribution.php
+++ b/CRM/Contribute/Import/Parser/Contribution.php
@@ -306,7 +306,7 @@ class CRM_Contribute_Import_Parser_Contribution extends CRM_Import_Parser {
         $paramValues['contact_type'] = $this->getContactType();
       }
       elseif ($this->isUpdateExisting() &&
-        (!empty($paramValues['contribution_id']) || !empty($values['trxn_id']) || !empty($paramValues['invoice_id']))
+        (!empty($paramValues['id']) || !empty($values['trxn_id']) || !empty($paramValues['invoice_id']))
       ) {
         $paramValues['contact_type'] = $this->getContactType();
       }
@@ -329,9 +329,9 @@ 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['contribution_id'])) {
+        if (!empty($paramValues['invoice_id']) || !empty($paramValues['trxn_id']) || !empty($paramValues['id'])) {
           $dupeIds = [
-            'id' => $paramValues['contribution_id'] ?? NULL,
+            'id' => $paramValues['id'] ?? NULL,
             'trxn_id' => $paramValues['trxn_id'] ?? NULL,
             'invoice_id' => $paramValues['invoice_id'] ?? NULL,
           ];
@@ -684,12 +684,14 @@ class CRM_Contribute_Import_Parser_Contribution extends CRM_Import_Parser {
               $params['contribution_contact_id'] = $matchingContactIds[0];
             }
           }
-          elseif (!empty($params['contribution_id']) || !empty($params['trxn_id']) || !empty($params['invoice_id'])) {
+          elseif (!empty($params['id']) || !empty($params['trxn_id']) || !empty($params['invoice_id'])) {
             // when update mode check contribution id or trxn id or
             // invoice id
+            // @todo - this check is obsolete. It survives for now
+            // in order to keep the rc patch small & non-conflicty.
             $contactId = new CRM_Contribute_DAO_Contribution();
-            if (!empty($params['contribution_id'])) {
-              $contactId->id = $params['contribution_id'];
+            if (!empty($params['id'])) {
+              $contactId->id = $params['id'];
             }
             elseif (!empty($params['trxn_id'])) {
               $contactId->trxn_id = $params['trxn_id'];
diff --git a/tests/phpunit/CRM/Contribute/Import/Parser/ContributionTest.php b/tests/phpunit/CRM/Contribute/Import/Parser/ContributionTest.php
index e1463439fa..b5d934781b 100644
--- a/tests/phpunit/CRM/Contribute/Import/Parser/ContributionTest.php
+++ b/tests/phpunit/CRM/Contribute/Import/Parser/ContributionTest.php
@@ -277,6 +277,9 @@ class CRM_Contribute_Import_Parser_ContributionTest extends CiviUnitTestCase {
     $this->assertEquals('No matching Contact found for (mum@example.com )', $row['_status_message']);
   }
 
+  /**
+   * @throws \CRM_Core_Exception
+   */
   public function testImportWithMatchByExternalIdentifier() :void {
     CRM_Core_DAO::executeQuery("ALTER TABLE civicrm_contact AUTO_INCREMENT = 1000000");
 
@@ -387,6 +390,44 @@ class CRM_Contribute_Import_Parser_ContributionTest extends CiviUnitTestCase {
     return $userJobID;
   }
 
+  /**
+   * Test that existing contributions are found and updated.
+   *
+   * @throws \CRM_Core_Exception
+   */
+  public function testImportUpdateExisting(): void {
+    $this->contributionCreate([
+      'contact_id' => $this->individualCreate(),
+      'trxn_id' => 'abc',
+      'invoice_id' => '65',
+      'total_amount' => 8,
+      'financial_type_id:name' => 'Event Fee',
+    ]);
+    $mapping = [
+      ['name' => 'contribution_id'],
+      ['name' => 'invoice_id'],
+      ['name' => 'trxn_id'],
+      ['name' => ''],
+      ['name' => 'total_amount'],
+      ['name' => 'receive_date'],
+      ['name' => 'financial_type_id'],
+      ['name' => 'contribution_source'],
+      ['name' => ''],
+    ];
+    $this->importCSV('contributions_update.csv', $mapping, ['onDuplicate' => CRM_Import_Parser::DUPLICATE_UPDATE]);
+    $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]);
+      }
+      else {
+        $this->assertEquals('ERROR', $row[10], $row[11]);
+      }
+    }
+  }
+
   /**
    * Add a random extra option value
    *
diff --git a/tests/phpunit/CRM/Contribute/Import/Parser/data/contributions_update.csv b/tests/phpunit/CRM/Contribute/Import/Parser/data/contributions_update.csv
new file mode 100644
index 0000000000..9e3ee80e5d
--- /dev/null
+++ b/tests/phpunit/CRM/Contribute/Import/Parser/data/contributions_update.csv
@@ -0,0 +1,7 @@
+Contribution ID,Invoice ID,Trxn ID,Contact ID,Total Amount,Receive Date,Financial Type,Reason,Row Status
+1,67,abc,8,999,2008-09-20,Donation,Invoice ID mismatch,invalid
+1,67,abcd,8,999,2008-09-20,Donation,Trxn ID mismatch,invalid
+1,65,abc,3,999,2008-09-20,Donation,All fields OK,valid
+,65,abc,3,999,2008-09-20,Donation,Only Invoice ID OK,valid
+1,,"",3,99,2008-09-20,Donation,Only Contribution ID OK,valid
+
-- 
2.25.1