Fix Contribution.create to not attempt to set contacts on activity update

In udpate cases the original allocations were more correct

diff --git a/CRM/Contribute/BAO/Contribution.php b/CRM/Contribute/BAO/Contribution.php
index 7f825adfa016215bbd39444fc7f1a56ba56ad303..9c58190677b38eb0d378710f7ff98f68bbddc0de 100644 (file)
--- 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 de46b7eeebf1377f2aebdda1ed5c09e141ac0388..2c7d4faa3224807f5da36548d0b3710ae8f57aa5 100644 (file)
--- 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 5bc0ccb8fddb19c9184ffb6469378eb0eed36df4..46e2a09ca212fd910e1c31d67f0c9698568434af 100644 (file)
--- 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]);