From ae0b8266b0f31d729a137c718d4be5f42f3db2a6 Mon Sep 17 00:00:00 2001 From: eileen Date: Thu, 25 Mar 2021 17:29:05 +1300 Subject: [PATCH] [REF] Simplification around contributionRecur ID --- CRM/Core/Payment/PayPalIPN.php | 47 +++++++++---------- .../CRM/Core/Payment/PayPalIPNTest.php | 14 ++++-- 2 files changed, 31 insertions(+), 30 deletions(-) diff --git a/CRM/Core/Payment/PayPalIPN.php b/CRM/Core/Payment/PayPalIPN.php index 6c1d56db99..fbcbe42e84 100644 --- 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

"; @@ -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']}

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

"; + 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 80a030be6a..696a654bf0 100644 --- 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()); -- 2.25.1