[REF] Simplification around contributionRecur ID

diff --git a/CRM/Core/Payment/PayPalIPN.php b/CRM/Core/Payment/PayPalIPN.php
index 6c1d56db99f7e648242cb09c64dbdf3d54fd13cd..fbcbe42e84b8424c17953573a732d1e0bc43175c 100644 (file)
--- a/CRM/Core/Payment/PayPalIPN.php
+++ b/CRM/Core/Payment/PayPalIPN.php
@@ -60,7 +60,6 @@ class CRM_Core_Payment_PayPalIPN extends CRM_Core_Payment_BaseIPN {
 
   /**
    * @param array $input
-   * @param array $ids
    * @param CRM_Contribute_BAO_ContributionRecur $recur
    * @param CRM_Contribute_BAO_Contribution $contribution
    * @param bool $first
@@ -70,7 +69,7 @@ class CRM_Core_Payment_PayPalIPN extends CRM_Core_Payment_BaseIPN {
    * @throws \CRM_Core_Exception
    * @throws \CiviCRM_API3_Exception
    */
-  public function recur($input, $ids, $recur, $contribution, $first) {
+  public function recur($input, $recur, $contribution, $first) {
     if (!isset($input['txnType'])) {
       Civi::log()->debug('PayPalIPN: Could not find txn_type in input request');
       echo "Failure: Invalid parameters<p>";
@@ -174,15 +173,15 @@ class CRM_Core_Payment_PayPalIPN extends CRM_Core_Payment_BaseIPN {
       // In future moving to create pending & then complete, but this OK for now.
       // Also consider accepting 'Failed' like other processors.
       $input['contribution_status_id'] = $contributionStatuses['Completed'];
-      $input['original_contribution_id'] = $ids['contribution'];
-      $input['contribution_recur_id'] = $ids['contributionRecur'];
+      $input['original_contribution_id'] = $contribution->id;
+      $input['contribution_recur_id'] = $recur->id;
 
       civicrm_api3('Contribution', 'repeattransaction', $input);
       return;
     }
 
     $this->single($input, [
-      'participant' => $ids['participant'] ?? NULL,
+      'participant' => NULL,
       'contributionRecur' => $recur->id,
     ], $contribution, TRUE);
   }
@@ -248,20 +247,19 @@ class CRM_Core_Payment_PayPalIPN extends CRM_Core_Payment_BaseIPN {
 
       $this->getInput($input);
 
-      if ($component == 'event') {
+      if ($component === 'event') {
         $ids['event'] = $this->retrieve('eventID', 'Integer', TRUE);
         $ids['participant'] = $this->retrieve('participantID', 'Integer', TRUE);
       }
       else {
         // get the optional ids
         $ids['membership'] = $membershipID;
-        $ids['contributionRecur'] = $contributionRecurID;
         $ids['contributionPage'] = $this->retrieve('contributionPageID', 'Integer', FALSE);
         $ids['related_contact'] = $this->retrieve('relatedContactID', 'Integer', FALSE);
         $ids['onbehalf_dupe_alert'] = $this->retrieve('onBehalfDupeAlert', 'Integer', FALSE);
       }
 
-      $paymentProcessorID = $this->getPayPalPaymentProcessorID($input, $ids);
+      $paymentProcessorID = $this->getPayPalPaymentProcessorID($input, $contributionRecurID);
 
       Civi::log()->debug('PayPalIPN: Received (ContactID: ' . $ids['contact'] . '; trxn_id: ' . $input['trxn_id'] . ').');
 
@@ -315,16 +313,6 @@ class CRM_Core_Payment_PayPalIPN extends CRM_Core_Payment_BaseIPN {
         $ids['contact'] = $contribution->contact_id;
       }
 
-      if (!empty($ids['contributionRecur'])) {
-        $contributionRecur = new CRM_Contribute_BAO_ContributionRecur();
-        $contributionRecur->id = $ids['contributionRecur'];
-        if (!$contributionRecur->find(TRUE)) {
-          CRM_Core_Error::debug_log_message("Could not find contribution recur record: {$ids['ContributionRecur']} in IPN request: " . print_r($input, TRUE));
-          echo "Failure: Could not find contribution recur record: {$ids['ContributionRecur']}<p>";
-          return FALSE;
-        }
-      }
-
       // CRM-19478: handle oddity when p=null is set in place of contribution page ID,
       if (!empty($ids['contributionPage']) && !is_numeric($ids['contributionPage'])) {
         // We don't need to worry if about removing contribution page id as it will be set later in
@@ -338,14 +326,21 @@ class CRM_Core_Payment_PayPalIPN extends CRM_Core_Payment_BaseIPN {
 
       $input['payment_processor_id'] = $paymentProcessorID;
 
-      if (!empty($ids['contributionRecur'])) {
+      if ($contributionRecurID) {
+        $contributionRecur = new CRM_Contribute_BAO_ContributionRecur();
+        $contributionRecur->id = $contributionRecurID;
+        if (!$contributionRecur->find(TRUE)) {
+          CRM_Core_Error::debug_log_message("Could not find contribution recur record: {$ids['ContributionRecur']} in IPN request: " . print_r($input, TRUE));
+          echo "Failure: Could not find contribution recur record: {$ids['ContributionRecur']}<p>";
+          return FALSE;
+        }
         // check if first contribution is completed, else complete first contribution
         $first = TRUE;
         $completedStatusId = CRM_Core_PseudoConstant::getKey('CRM_Contribute_BAO_Contribution', 'contribution_status_id', 'Completed');
         if ($contribution->contribution_status_id == $completedStatusId) {
           $first = FALSE;
         }
-        $this->recur($input, $ids, $contributionRecur, $contribution, $first);
+        $this->recur($input, $contributionRecur, $contribution, $first);
         if ($this->getFirstOrLastInSeriesStatus()) {
           //send recurring Notification email for user
           CRM_Contribute_BAO_ContributionPage::recurringNotify(
@@ -438,16 +433,16 @@ class CRM_Core_Payment_PayPalIPN extends CRM_Core_Payment_BaseIPN {
   }
 
   /**
-   * Gets PaymentProcessorID for PayPal
+   * Get PaymentProcessorID for PayPal
    *
    * @param array $input
-   * @param array $ids
+   * @param int|null $contributionRecurID
    *
    * @return int
    * @throws \CRM_Core_Exception
    * @throws \CiviCRM_API3_Exception
    */
-  public function getPayPalPaymentProcessorID($input, $ids) {
+  public function getPayPalPaymentProcessorID(array $input, ?int $contributionRecurID): int {
     // First we try and retrieve from POST params
     $paymentProcessorID = $this->retrieve('processor_id', 'Integer', FALSE);
     if (!empty($paymentProcessorID)) {
@@ -455,9 +450,9 @@ class CRM_Core_Payment_PayPalIPN extends CRM_Core_Payment_BaseIPN {
     }
 
     // Then we try and get it from recurring contribution ID
-    if (!empty($ids['contributionRecur'])) {
+    if ($contributionRecurID) {
       $contributionRecur = civicrm_api3('ContributionRecur', 'getsingle', [
-        'id' => $ids['contributionRecur'],
+        'id' => $contributionRecurID,
         'return' => ['payment_processor_id'],
       ]);
       if (!empty($contributionRecur['payment_processor_id'])) {
@@ -485,7 +480,7 @@ class CRM_Core_Payment_PayPalIPN extends CRM_Core_Payment_BaseIPN {
     if (empty($paymentProcessorID)) {
       throw new CRM_Core_Exception('PayPalIPN: Could not get Payment Processor ID');
     }
-    return $paymentProcessorID;
+    return (int) $paymentProcessorID;
   }
 
   /**

diff --git a/tests/phpunit/CRM/Core/Payment/PayPalIPNTest.php b/tests/phpunit/CRM/Core/Payment/PayPalIPNTest.php
index 80a030be6ab99585cd62ea2940c379b450101375..696a654bf05d90ed98740c0e6700ab5b29a048cd 100644 (file)
--- a/tests/phpunit/CRM/Core/Payment/PayPalIPNTest.php
+++ b/tests/phpunit/CRM/Core/Payment/PayPalIPNTest.php
@@ -97,12 +97,18 @@ class CRM_Core_Payment_PayPalIPNTest extends CiviUnitTestCase {
   }
 
   /**
-   * Test IPN response updates contribution_recur & contribution for first & second contribution.
+   * Test IPN response updates contribution_recur & contribution for first &
+   * second contribution.
+   *
+   * The scenario is that a pending contribution exists and the first call will
+   * update it to completed. The second will create a new contribution.
    *
-   * The scenario is that a pending contribution exists and the first call will update it to completed.
-   * The second will create a new contribution.
+   * @throws \API_Exception
+   * @throws \CRM_Core_Exception
+   * @throws \CiviCRM_API3_Exception
+   * @throws \Civi\API\Exception\UnauthorizedException
    */
-  public function testIPNPaymentRecurSuccess() {
+  public function testIPNPaymentRecurSuccess(): void {
     $this->setupRecurringPaymentProcessorTransaction([], ['total_amount' => '15.00']);
     $mut = new CiviMailUtils($this, TRUE);
     $paypalIPN = new CRM_Core_Payment_PayPalIPN($this->getPaypalRecurTransaction());