From 6e902643f72c1e215dbdd04e15e99148c2f9706e Mon Sep 17 00:00:00 2001 From: eileen Date: Sat, 18 Jul 2020 14:02:47 +1200 Subject: [PATCH] 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 --- CRM/Contribute/BAO/Contribution.php | 46 +++++++++++-------- .../CRM/Contribute/BAO/ContributionTest.php | 28 ++++++----- 2 files changed, 41 insertions(+), 33 deletions(-) diff --git a/CRM/Contribute/BAO/Contribution.php b/CRM/Contribute/BAO/Contribution.php index 10501932192f..97632dc2a3d4 100644 --- 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 @@ public function getNumTermsByContributionAndMembershipType($membershipTypeID, $c * * @return CRM_Contribute_BAO_Contribution * + * @throws \API_Exception * @throws \CRM_Core_Exception * @throws \CiviCRM_API3_Exception */ @@ -512,26 +515,33 @@ public static function create(&$params, $ids = []) { $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 @@ public static function completeOrder($input, &$ids, $objects, $transaction = NUL } $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 66244fbf982c..a9cbb3718444 100644 --- 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 @@ public function testCheckLineItemsWithFloatingPointValues() { } /** - * 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 @@ public function testActivityCreate() { '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 @@ public function testActivityCreate() { $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 @@ public function testActivityCreate() { $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']); } /**