[REF] IPN - move unshared chunk of code out of shared function
authoreileen <emcnaughton@wikimedia.org>
Fri, 25 Sep 2020 21:54:54 +0000 (09:54 +1200)
committereileen <emcnaughton@wikimedia.org>
Fri, 25 Sep 2020 23:48:37 +0000 (11:48 +1200)
We are passing a lot of variables into recur (& going to a lot of work to construct them) when they are not
actually processed in a common way - returning them to the calling functions makes it clear we are
a skip & a jump from not needing to instantiate objects at all

CRM/Contribute/BAO/Contribution.php
CRM/Core/Payment/AuthorizeNetIPN.php

index f59f205c87a04ad49b635b146d2f8bb369fd9e07..c279e4df9045fd21534d58bd39d7bd2748413df8 100644 (file)
@@ -4377,6 +4377,8 @@ INNER JOIN civicrm_activity ON civicrm_activity_contact.activity_id = civicrm_ac
   public static function completeOrder($input, $ids, $objects, $isPostPaymentCreate = FALSE) {
     $transaction = new CRM_Core_Transaction();
     $contribution = $objects['contribution'];
+    // Unset objects just to make it clear it's not used again.
+    unset($objects);
     // @todo see if we even need this - it's used further down to create an activity
     // but the BAO layer should create that - we just need to add a test to cover it & can
     // maybe remove $ids altogether.
@@ -4418,7 +4420,7 @@ INNER JOIN civicrm_activity ON civicrm_activity_contact.activity_id = civicrm_ac
 
     $contributionParams['payment_processor'] = $paymentProcessorId;
 
-    if (empty($contributionParams['payment_instrument_id']) && isset($contribution->_relatedObjects['paymentProcessor']['payment_instrument_id'])) {
+    if (empty($contributionParams['payment_instrument_id']) && $paymentProcessorId) {
       $contributionParams['payment_instrument_id'] = PaymentProcessor::get(FALSE)->addWhere('id', '=', $paymentProcessorId)->addSelect('payment_instrument_id')->execute()->first()['payment_instrument_id'];
     }
 
index bad241276fad93abba143068b5b904f9edeca9e0..06e3049cf4e95c4c1d76009383c3bb3d160834bc 100644 (file)
@@ -124,6 +124,21 @@ class CRM_Core_Payment_AuthorizeNetIPN extends CRM_Core_Payment_BaseIPN {
           $first = TRUE;
           if ($objects['contribution']->contribution_status_id == 1) {
             $first = FALSE;
+            //load new contribution object if required.
+            // create a contribution and then get it processed
+            $contribution = new CRM_Contribute_BAO_Contribution();
+            $contribution->contact_id = $ids['contact'];
+            $contribution->financial_type_id = $objects['contributionType']->id;
+            $contribution->contribution_page_id = $ids['contributionPage'];
+            $contribution->contribution_recur_id = $ids['contributionRecur'];
+            $contribution->receive_date = $input['receive_date'];
+            $contribution->currency = $objects['contribution']->currency;
+            $contribution->amount_level = $objects['contribution']->amount_level;
+            $contribution->address_id = $objects['contribution']->address_id;
+            $contribution->campaign_id = $objects['contribution']->campaign_id;
+            $contribution->_relatedObjects = $objects['contribution']->_relatedObjects;
+
+            $objects['contribution'] = &$contribution;
           }
           $input['payment_processor_id'] = $paymentProcessorID;
           return $this->recur($input, $ids, $objects, $first);
@@ -144,11 +159,11 @@ class CRM_Core_Payment_AuthorizeNetIPN extends CRM_Core_Payment_BaseIPN {
    * @param $first
    *
    * @return bool
+   * @throws \CRM_Core_Exception
+   * @throws \CiviCRM_API3_Exception
    */
   public function recur($input, $ids, $objects, $first) {
-    $this->_isRecurring = TRUE;
     $recur = &$objects['contributionRecur'];
-    $paymentProcessorObject = $objects['contribution']->_relatedObjects['paymentProcessor']['object'];
 
     // do a subscription check
     if ($recur->processor_id != $input['subscription_id']) {
@@ -159,24 +174,6 @@ class CRM_Core_Payment_AuthorizeNetIPN extends CRM_Core_Payment_BaseIPN {
 
     $now = date('YmdHis');
 
-    //load new contribution object if required.
-    if (!$first) {
-      // create a contribution and then get it processed
-      $contribution = new CRM_Contribute_BAO_Contribution();
-      $contribution->contact_id = $ids['contact'];
-      $contribution->financial_type_id = $objects['contributionType']->id;
-      $contribution->contribution_page_id = $ids['contributionPage'];
-      $contribution->contribution_recur_id = $ids['contributionRecur'];
-      $contribution->receive_date = $input['receive_date'];
-      $contribution->currency = $objects['contribution']->currency;
-      $contribution->payment_instrument_id = $objects['contribution']->payment_instrument_id;
-      $contribution->amount_level = $objects['contribution']->amount_level;
-      $contribution->address_id = $objects['contribution']->address_id;
-      $contribution->campaign_id = $objects['contribution']->campaign_id;
-      $contribution->_relatedObjects = $objects['contribution']->_relatedObjects;
-
-      $objects['contribution'] = &$contribution;
-    }
     $objects['contribution']->invoice_id = md5(uniqid(rand(), TRUE));
     $objects['contribution']->total_amount = $input['amount'];
     $objects['contribution']->trxn_id = $input['trxn_id'];
@@ -229,7 +226,7 @@ class CRM_Core_Payment_AuthorizeNetIPN extends CRM_Core_Payment_BaseIPN {
       'related_contact' => $ids['related_contact'] ?? NULL,
       'participant' => !empty($objects['participant']) ? $objects['participant']->id : NULL,
       'contributionRecur' => !empty($objects['contributionRecur']) ? $objects['contributionRecur']->id : NULL,
-    ], $objects);
+    ], ['contribution' => $objects['contribution']]);
 
     // Only Authorize.net does this so it is on the a.net class. If there is a need for other processors
     // to do this we should make it available via the api, e.g as a parameter, changing the nuance