Skip to content

Commit

Permalink
Stop passing / using object when all we need is the id
Browse files Browse the repository at this point in the history
Rather than set id on the contribution object just to be able to access it via contribution->id
let's name the param we keep using & use that. Note I'm still getting contribution->id from contribution
here but I think this makes it clear that the object is mostly only used in addActivity now.

The sligtly larger change is in updateMembershipBasedOnCompletionOfContribution where there is
an instantiation of 'self()' since we no longer have the object
  • Loading branch information
eileenmcnaughton committed Sep 3, 2020
1 parent d42a0ae commit 4711a5d
Showing 1 changed file with 12 additions and 10 deletions.
22 changes: 12 additions & 10 deletions CRM/Contribute/BAO/Contribution.php
Original file line number Diff line number Diff line change
Expand Up @@ -4509,11 +4509,12 @@ public static function completeOrder($input, $ids, $objects, $isPostPaymentCreat
$changeDate = CRM_Utils_Array::value('trxn_date', $input, date('YmdHis'));

$contributionResult = self::repeatTransaction($contribution, $input, $contributionParams);
$contributionID = (int) $contribution->id;

if ($input['component'] == 'contribute') {
if ($contributionParams['contribution_status_id'] === $completedContributionStatusID) {
self::updateMembershipBasedOnCompletionOfContribution(
$contribution,
$contributionID,
$changeDate
);
}
Expand All @@ -4526,7 +4527,7 @@ public static function completeOrder($input, $ids, $objects, $isPostPaymentCreat
}
}

$contributionParams['id'] = $contribution->id;
$contributionParams['id'] = $contributionID;
$contributionParams['is_post_payment_create'] = $isPostPaymentCreate;

if (!$contributionResult) {
Expand All @@ -4535,7 +4536,7 @@ public static function completeOrder($input, $ids, $objects, $isPostPaymentCreat

// Add new soft credit against current $contribution.
if ($recurringContributionID) {
CRM_Contribute_BAO_ContributionRecur::addrecurSoftCredit($recurringContributionID, $contribution->id);
CRM_Contribute_BAO_ContributionRecur::addrecurSoftCredit($recurringContributionID, $contributionID);
}

$contribution->contribution_status_id = $contributionParams['contribution_status_id'];
Expand All @@ -4544,7 +4545,7 @@ public static function completeOrder($input, $ids, $objects, $isPostPaymentCreat
$transaction->commit();

// @todo - check if Contribution::create does this, test, remove.
CRM_Contribute_BAO_ContributionRecur::updateRecurLinkedPledge($contribution->id, $recurringContributionID,
CRM_Contribute_BAO_ContributionRecur::updateRecurLinkedPledge($contributionID, $recurringContributionID,
$contributionParams['contribution_status_id'], $input['amount']);

// create an activity record
Expand All @@ -4561,7 +4562,7 @@ public static function completeOrder($input, $ids, $objects, $isPostPaymentCreat

if (self::isEmailReceipt($input, $contribution->contribution_page_id, $recurringContributionID)) {
civicrm_api3('Contribution', 'sendconfirmation', [
'id' => $contribution->id,
'id' => $contributionID,
'payment_processor_id' => $paymentProcessorId,
]);
CRM_Core_Error::debug_log_message("Receipt sent");
Expand Down Expand Up @@ -5157,14 +5158,14 @@ protected static function isPaymentInstrumentChange(&$params, $pendingStatuses)
* Note that the way in which $memberships are loaded as objects is pretty messy & I think we could just
* load them in this function. Code clean up would compensate for any minor performance implication.
*
* @param \CRM_Contribute_BAO_Contribution $contribution
* @param int $contribution
* @param string $changeDate
*
* @throws \CRM_Core_Exception
* @throws \CiviCRM_API3_Exception
*/
public static function updateMembershipBasedOnCompletionOfContribution($contribution, $changeDate) {
$memberships = self::getRelatedMemberships($contribution->id);
public static function updateMembershipBasedOnCompletionOfContribution($contributionID, $changeDate) {
$memberships = self::getRelatedMemberships($contributionID);
foreach ($memberships as $membership) {
$membershipParams = [
'id' => $membership['id'],
Expand Down Expand Up @@ -5202,9 +5203,10 @@ public static function updateMembershipBasedOnCompletionOfContribution($contribu
// The api assumes num_terms is a special sauce for 'is_renewal' so we need to not pass it when updating a pending to completed.
// ... except testCompleteTransactionMembershipPriceSetTwoTerms hits this line so the above is obviously not true....
// @todo once apiv4 ships with core switch to that & find sanity.
$membershipParams['num_terms'] = $contribution->getNumTermsByContributionAndMembershipType(
$bao = new self();
$membershipParams['num_terms'] = $bao->getNumTermsByContributionAndMembershipType(
$membershipParams['membership_type_id'],
$contribution->id
$contributionID
);
}
// @todo remove all this stuff in favour of letting the api call further down handle in
Expand Down

0 comments on commit 4711a5d

Please sign in to comment.