From 6fd75357518454c744701bccd5d28e96cfcc084e Mon Sep 17 00:00:00 2001
From: eileen <emcnaughton@wikimedia.org>
Date: Mon, 14 Dec 2020 14:26:16 +1300
Subject: [PATCH] Fix Contribution.create to not attempt to set contacts on
 activity update

In udpate cases the original allocations were more correct
---
 CRM/Contribute/BAO/Contribution.php            | 14 ++++++++++----
 tests/phpunit/CRM/Activity/Form/SearchTest.php |  7 +++++--
 tests/phpunit/api/v3/JobTest.php               |  3 ++-
 3 files changed, 17 insertions(+), 7 deletions(-)

diff --git a/CRM/Contribute/BAO/Contribution.php b/CRM/Contribute/BAO/Contribution.php
index 7f825adfa0..9c58190677 100644
--- a/CRM/Contribute/BAO/Contribution.php
+++ b/CRM/Contribute/BAO/Contribution.php
@@ -517,18 +517,24 @@ class CRM_Contribute_BAO_Contribution extends CRM_Contribute_DAO_Contribution {
       ])->execute()->first();
 
       $campaignParams = isset($params['campaign_id']) ? ['campaign_id' => ($params['campaign_id'] ?? NULL)] : [];
-      Activity::save(FALSE)->addRecord(array_merge([
+      $activityParams = array_merge([
         'activity_type_id:name' => 'Contribution',
         'source_record_id' => $contribution->id,
-        'source_contact_id' => CRM_Core_Session::getLoggedInContactID() ?: $contribution->contact_id,
-        'target_contact_id' => CRM_Core_Session::getLoggedInContactID() ? [$contribution->contact_id] : [],
         'activity_date_time' => $contribution->receive_date,
         'is_test' => (bool) $contribution->is_test,
         'status_id:name' => $isCompleted ? 'Completed' : 'Scheduled',
         'skipRecentView' => TRUE,
         'subject' => CRM_Activity_BAO_Activity::getActivitySubject($contribution),
         'id' => $existingActivity['id'] ?? NULL,
-      ], $campaignParams))->execute();
+      ], $campaignParams);
+      if (!$activityParams['id']) {
+        // Don't set target contacts on update as these will have been
+        // correctly created and we risk overwriting them with
+        // 'best guess' params.
+        $activityParams['source_contact_id'] = (int) ($params['source_contact_id'] ?? (CRM_Core_Session::getLoggedInContactID() ?: $contribution->contact_id));
+        $activityParams['target_contact_id'] = ($activityParams['source_contact_id'] === (int) $contribution->contact_id) ? [] : [$contribution->contact_id];
+      }
+      Activity::save(FALSE)->addRecord($activityParams)->execute();
     }
 
     // do not add to recent items for import, CRM-4399
diff --git a/tests/phpunit/CRM/Activity/Form/SearchTest.php b/tests/phpunit/CRM/Activity/Form/SearchTest.php
index de46b7eeeb..2c7d4faa32 100644
--- a/tests/phpunit/CRM/Activity/Form/SearchTest.php
+++ b/tests/phpunit/CRM/Activity/Form/SearchTest.php
@@ -24,9 +24,12 @@ class CRM_Activity_Form_SearchTest extends CiviUnitTestCase {
   }
 
   /**
-   *  Test submitted the search form.
+   * Test submitted the search form.
+   *
+   * @throws \CRM_Core_Exception
+   * @throws \CiviCRM_API3_Exception
    */
-  public function testSearch() {
+  public function testSearch(): void {
 
     $form = new CRM_Activity_Form_Search();
     $_SERVER['REQUEST_METHOD'] = 'GET';
diff --git a/tests/phpunit/api/v3/JobTest.php b/tests/phpunit/api/v3/JobTest.php
index 5bc0ccb8fd..46e2a09ca2 100644
--- a/tests/phpunit/api/v3/JobTest.php
+++ b/tests/phpunit/api/v3/JobTest.php
@@ -462,8 +462,9 @@ class api_v3_JobTest extends CiviUnitTestCase {
    * Note the group combinations & expected results:
    *
    * @throws \CRM_Core_Exception
+   * @throws \CiviCRM_API3_Exception
    */
-  public function testBatchMergeWithAssets() {
+  public function testBatchMergeWithAssets(): void {
     $contactID = $this->individualCreate();
     $contact2ID = $this->individualCreate();
     $this->contributionCreate(['contact_id' => $contactID]);
-- 
2.25.1