Call apiv4 from Contribution create rather than fugly addActivity function

I took a look at https://github.com/civicrm/civicrm-core/pull/17274 which has blocking test failures - but felt that
the shared function was adding nothing and simply using the api to create the activity made more sense. The
shared function does a lot of silly wrangling for very little shared functionality and is
hard to read. In this call only 2 params are passed in - so most of the wranglingg
doesn't apply anyway. I ensured the 2 JIRA issues referenced in the removed code have test cover (one already
had a test written by Monish & I added in the campaign check

diff --git a/CRM/Contribute/BAO/Contribution.php b/CRM/Contribute/BAO/Contribution.php
index 10501932192fd8a196c2cbcd2a3fcae5fa5ab200..97632dc2a3d457b9434a9d239568fe93b5966318 100644 (file)
--- a/CRM/Contribute/BAO/Contribution.php
+++ b/CRM/Contribute/BAO/Contribution.php
@@ -9,6 +9,8 @@
  +--------------------------------------------------------------------+
  */
 
+use Civi\Api4\Activity;
+
 /**
  *
  * @package CRM
@@ -461,6 +463,7 @@ class CRM_Contribute_BAO_Contribution extends CRM_Contribute_DAO_Contribution {
    *
    * @return CRM_Contribute_BAO_Contribution
    *
+   * @throws \API_Exception
    * @throws \CRM_Core_Exception
    * @throws \CiviCRM_API3_Exception
    */
@@ -512,26 +515,33 @@ class CRM_Contribute_BAO_Contribution extends CRM_Contribute_DAO_Contribution {
 
     $transaction->commit();
 
-    $activity = civicrm_api3('Activity', 'get', [
-      'source_record_id' => $contribution->id,
-      'options' => ['limit' => 1],
-      'sequential' => 1,
-      'activity_type_id' => 'Contribution',
-      'return' => ['id', 'campaign'],
-    ]);
-
-    //CRM-18406: Update activity when edit contribution.
-    if ($activity['count']) {
-      // CRM-13237 : if activity record found, update it with campaign id of contribution
-      // @todo compare campaign ids first.
-      CRM_Core_DAO::setFieldValue('CRM_Activity_BAO_Activity', $activity['id'], 'campaign_id', $contribution->campaign_id);
-      $contribution->activity_id = $activity['id'];
-    }
-
     if (empty($contribution->contact_id)) {
       $contribution->find(TRUE);
     }
-    CRM_Activity_BAO_Activity::addActivity($contribution, 'Contribution');
+
+    $isCompleted = ('Completed' === CRM_Core_PseudoConstant::getName('CRM_Contribute_BAO_Contribution', 'contribution_status_id', $contribution->contribution_status_id));
+    if (!empty($params['on_behalf'])
+      ||  $isCompleted
+    ) {
+      $existingActivity = Activity::get(FALSE)->setWhere([
+        ['source_record_id', '=', $contribution->id],
+        ['activity_type_id:name', '=', 'Contribution'],
+      ])->execute()->first();
+
+      $campaignParams = isset($params['campaign_id']) ? ['campaign_id' => ($params['campaign_id'] ?? NULL)] : [];
+      Activity::save(FALSE)->addRecord(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();
+    }
 
     // do not add to recent items for import, CRM-4399
     if (empty($params['skipRecentView'])) {
@@ -4549,7 +4559,7 @@ INNER JOIN civicrm_activity ON civicrm_activity_contact.activity_id = civicrm_ac
     }
     $contribution->contribution_status_id = $contributionParams['contribution_status_id'];
 
-    CRM_Core_Error::debug_log_message("Contribution record updated successfully");
+    CRM_Core_Error::debug_log_message('Contribution record updated successfully');
     $transaction->commit();
 
     CRM_Contribute_BAO_ContributionRecur::updateRecurLinkedPledge($contribution->id, $recurringContributionID,

diff --git a/tests/phpunit/CRM/Contribute/BAO/ContributionTest.php b/tests/phpunit/CRM/Contribute/BAO/ContributionTest.php
index 66244fbf982cb11159e6a4a8f0317234f9296a50..a9cbb37184448d9c9856175cdee38fdde8c49b0c 100644 (file)
--- a/tests/phpunit/CRM/Contribute/BAO/ContributionTest.php
+++ b/tests/phpunit/CRM/Contribute/BAO/ContributionTest.php
@@ -8,6 +8,7 @@
  | and copyright information, see https://civicrm.org/licensing       |
  +--------------------------------------------------------------------+
  */
+use Civi\Api4\Activity;
 
 /**
  * Class CRM_Contribute_BAO_ContributionTest
@@ -886,11 +887,10 @@ WHERE eft.entity_id = %1 AND ft.to_financial_account_id <> %2";
   }
 
   /**
-   * Test activity amount updation.
+   * Test activity amount updates activity subject.
    */
   public function testActivityCreate() {
     $contactId = $this->individualCreate();
-    $defaults = [];
 
     $params = [
       'contact_id' => $contactId,
@@ -903,7 +903,6 @@ WHERE eft.entity_id = %1 AND ft.to_financial_account_id <> %2";
       'receipt_date' => '20080522000000',
       'non_deductible_amount' => 0.00,
       'total_amount' => 100.00,
-      'trxn_id' => '22ereerwww444444',
       'invoice_id' => '86ed39c9e9ee6ef6031621ce0eafe7eb81',
       'thankyou_date' => '20160519',
       'sequential' => 1,
@@ -913,20 +912,18 @@ WHERE eft.entity_id = %1 AND ft.to_financial_account_id <> %2";
 
     $this->assertEquals($params['total_amount'], $contribution['total_amount'], 'Check for total amount in contribution.');
     $this->assertEquals($contactId, $contribution['contact_id'], 'Check for contact id  creation.');
-
-    // Check amount in activity.
-    $activityParams = [
-      'source_record_id' => $contribution['id'],
-      'activity_type_id' => CRM_Core_PseudoConstant::getKey('CRM_Activity_BAO_Activity', 'activity_type_id', 'Contribution'),
+    $activityWhere = [
+      ['source_record_id', '=', $contribution['id']],
+      ['activity_type_id:name', '=', 'Contribution'],
     ];
-    // @todo use api instead.
-    $activity = CRM_Activity_BAO_Activity::retrieve($activityParams, $defaults);
+    $activity = Activity::get()->setWhere($activityWhere)->setSelect(['source_record_id', 'subject'])->execute()->first();
 
-    $this->assertEquals($contribution['id'], $activity->source_record_id, 'Check for activity associated with contribution.');
-    $this->assertEquals("$ 100.00 - STUDENT", $activity->subject, 'Check for total amount in activity.');
+    $this->assertEquals($contribution['id'], $activity['source_record_id'], 'Check for activity associated with contribution.');
+    $this->assertEquals('$ 100.00 - STUDENT', $activity['subject'], 'Check for total amount in activity.');
 
     $params['id'] = $contribution['id'];
     $params['total_amount'] = 200;
+    $params['campaign_id'] = $this->campaignCreate();
 
     $contribution = $this->callAPISuccess('Contribution', 'create', $params)['values'][0];
 
@@ -934,10 +931,11 @@ WHERE eft.entity_id = %1 AND ft.to_financial_account_id <> %2";
     $this->assertEquals($contactId, $contribution['contact_id'], 'Check for contact id  creation.');
 
     // Retrieve activity again.
-    $activity = CRM_Activity_BAO_Activity::retrieve($activityParams, $defaults);
+    $activity = Activity::get()->setWhere($activityWhere)->setSelect(['source_record_id', 'subject', 'campaign_id'])->execute()->first();
 
-    $this->assertEquals($contribution['id'], $activity->source_record_id, 'Check for activity associated with contribution.');
-    $this->assertEquals("$ 200.00 - STUDENT", $activity->subject, 'Check for total amount in activity.');
+    $this->assertEquals($contribution['id'], $activity['source_record_id'], 'Check for activity associated with contribution.');
+    $this->assertEquals('$ 200.00 - STUDENT', $activity['subject'], 'Check for total amount in activity.');
+    $this->assertEquals($params['campaign_id'], $activity['campaign_id']);
   }
 
   /**