Stop passing / using object when all we need is the id

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

CRM/Contribute/BAO/Contribution.php

@@ -444,7 +444,7 @@ public static function getPaymentProcessorReadyAddressParams($params, $billingLo
    *
    * @return int
    */
-  public function getNumTermsByContributionAndMembershipType($membershipTypeID, $contributionID) {
+  public static function getNumTermsByContributionAndMembershipType($membershipTypeID, $contributionID) {
     $numTerms = CRM_Core_DAO::singleValueQuery("
       SELECT membership_num_terms FROM civicrm_line_item li
       LEFT JOIN civicrm_price_field_value v ON li.price_field_value_id = v.id
@@ -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
         );
       }
@@ -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) {
@@ -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'];
@@ -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
@@ -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");
@@ -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 $contributionID
    * @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'],
@@ -5202,9 +5203,9 @@ 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(
+        $membershipParams['num_terms'] = self::getNumTermsByContributionAndMembershipType(
           $membershipParams['membership_type_id'],
-          $contribution->id
+          $contributionID
         );
       }
       // @todo remove all this stuff in favour of letting the api call further down handle in