Skip to content

Commit

Permalink
Call apiv4 from Contribution create rather than fugly addActivity fun…
Browse files Browse the repository at this point in the history 
…ction

I took a look at civicrm#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
  • Loading branch information
@eileenmcnaughton
eileenmcnaughton committed Jul 20, 2020
1 parent a461078 commit 6e90264
Show file tree
Hide file tree
Showing 2 changed files with 41 additions and 33 deletions.
46 changes: 28 additions & 18 deletions CRM/Contribute/BAO/Contribution.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@
+--------------------------------------------------------------------+
*/

use Civi\Api4\Activity;

/**
*
* @package CRM
Expand Down Expand Up @@ -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
*/
Expand Down Expand Up @@ -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'])) {
Expand Down Expand Up @@ -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,
Expand Down
28 changes: 13 additions & 15 deletions tests/phpunit/CRM/Contribute/BAO/ContributionTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
| and copyright information, see https://civicrm.org/licensing |
+--------------------------------------------------------------------+
*/
use Civi\Api4\Activity;

/**
* Class CRM_Contribute_BAO_ContributionTest
Expand Down Expand Up @@ -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,
Expand All @@ -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,
Expand All @@ -913,31 +912,30 @@ 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];

$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.');

// 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']);
}

/**
Expand Down

0 comments on commit 6e90264

Please sign in to comment.