From ae65911569d843cc0c6ae93e21f65e6ede959a4a Mon Sep 17 00:00:00 2001 From: eileen Date: Sat, 1 Aug 2020 17:10:20 +1200 Subject: [PATCH] Add testing to Authorize.net and remove the lines that are repeated In Authorize.netIPN updates are made to the contributionRecur that are later made in ContributionRecur::updateOnNewPayment This adds test cover to ensure no change and then removes them. With these lines gone it's apparent that the transaction is not required here --- CRM/Core/Payment/AuthorizeNetIPN.php | 16 +++++------- .../CRM/Core/Payment/AuthorizeNetIPNTest.php | 25 +++++++++++-------- 2 files changed, 21 insertions(+), 20 deletions(-) diff --git a/CRM/Core/Payment/AuthorizeNetIPN.php b/CRM/Core/Payment/AuthorizeNetIPN.php index 7fa588e8d5..12aa6cce84 100644 --- a/CRM/Core/Payment/AuthorizeNetIPN.php +++ b/CRM/Core/Payment/AuthorizeNetIPN.php @@ -114,8 +114,6 @@ class CRM_Core_Payment_AuthorizeNetIPN extends CRM_Core_Payment_BaseIPN { $contributionStatus = CRM_Contribute_PseudoConstant::contributionStatus(NULL, 'name'); - $transaction = new CRM_Core_Transaction(); - $now = date('YmdHis'); //load new contribution object if required. @@ -148,18 +146,17 @@ class CRM_Core_Payment_AuthorizeNetIPN extends CRM_Core_Payment_BaseIPN { $recur->trxn_id = $recur->processor_id; $isFirstOrLastRecurringPayment = CRM_Core_Payment::RECURRING_PAYMENT_START; } - $statusName = 'In Progress'; + if (($recur->installments > 0) && ($input['subscription_paynum'] >= $recur->installments) ) { // this is the last payment - $statusName = 'Completed'; $recur->end_date = $now; $isFirstOrLastRecurringPayment = CRM_Core_Payment::RECURRING_PAYMENT_END; + // This end date update should occur in ContributionRecur::updateOnNewPayment + // testIPNPaymentRecurNoReceipt has test cover. + $recur->save(); } - $recur->modified_date = $now; - $recur->contribution_status_id = array_search($statusName, $contributionStatus); - $recur->save(); } else { // Declined @@ -168,7 +165,7 @@ class CRM_Core_Payment_AuthorizeNetIPN extends CRM_Core_Payment_BaseIPN { $recur->cancel_date = $now; $recur->save(); - $message = ts("Subscription payment failed - %1", [1 => htmlspecialchars($input['response_reason_text'])]); + $message = ts('Subscription payment failed - %1', [1 => htmlspecialchars($input['response_reason_text'])]); CRM_Core_Error::debug_log_message($message); // the recurring contribution has declined a payment or has failed @@ -180,13 +177,12 @@ class CRM_Core_Payment_AuthorizeNetIPN extends CRM_Core_Payment_BaseIPN { // check if contribution is already completed, if so we ignore this ipn if ($objects['contribution']->contribution_status_id == 1) { - $transaction->commit(); CRM_Core_Error::debug_log_message("Returning since contribution has already been handled."); echo "Success: Contribution has already been handled

"; return TRUE; } - $this->completeTransaction($input, $ids, $objects, $transaction, $recur); + $this->completeTransaction($input, $ids, $objects); // 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 diff --git a/tests/phpunit/CRM/Core/Payment/AuthorizeNetIPNTest.php b/tests/phpunit/CRM/Core/Payment/AuthorizeNetIPNTest.php index b402fbab73..8e89b2c7c6 100644 --- a/tests/phpunit/CRM/Core/Payment/AuthorizeNetIPNTest.php +++ b/tests/phpunit/CRM/Core/Payment/AuthorizeNetIPNTest.php @@ -93,7 +93,7 @@ class CRM_Core_Payment_AuthorizeNetIPNTest extends CiviUnitTestCase { 'billing_country_id-5' => 1228, 'frequency_interval' => 1, 'frequency_unit' => 'month', - 'installments' => '', + 'installments' => 2, 'hidden_AdditionalDetail' => 1, 'hidden_Premium' => 1, 'payment_processor_id' => $this->_paymentProcessorID, @@ -109,24 +109,29 @@ class CRM_Core_Payment_AuthorizeNetIPNTest extends CiviUnitTestCase { $this->_contributionID = $contribution->id; $this->ids['Contribution'][0] = $contribution->id; $this->_contributionRecurID = $contribution->contribution_recur_id; - $recur_params = [ - 'id' => $this->_contributionRecurID, - 'return' => 'processor_id', - ]; - $processor_id = civicrm_api3('ContributionRecur', 'getvalue', $recur_params); + + $contributionRecur = $this->callAPISuccessGetSingle('ContributionRecur', ['id' => $this->_contributionRecurID]); + $processor_id = $contributionRecur['processor_id']; + $this->assertEquals('Pending', CRM_Core_PseudoConstant::getName('CRM_Contribute_BAO_Contribution', 'contribution_status_id', $contributionRecur['contribution_status_id'])); // Process the initial one. $IPN = new CRM_Core_Payment_AuthorizeNetIPN( $this->getRecurTransaction(['x_subscription_id' => $processor_id]) ); $IPN->main(); + $updatedContributionRecur = $this->callAPISuccessGetSingle('ContributionRecur', ['id' => $this->_contributionRecurID]); + $this->assertEquals('In Progress', CRM_Core_PseudoConstant::getName('CRM_Contribute_BAO_ContributionRecur', 'contribution_status_id', $updatedContributionRecur['contribution_status_id'])); + $this->assertTrue(strtotime($updatedContributionRecur['modified_date']) > strtotime($contributionRecur['modified_date'])); // Now send a second one (authorize seems to treat first and second contributions // differently. - $IPN = new CRM_Core_Payment_AuthorizeNetIPN($this->getRecurSubsequentTransaction( - ['x_subscription_id' => $processor_id] - )); + $IPN = new CRM_Core_Payment_AuthorizeNetIPN($this->getRecurSubsequentTransaction([ + 'x_subscription_id' => $processor_id, + 'x_subscription_paynum' => 2, + ])); $IPN->main(); - + $updatedContributionRecurAgain = $this->callAPISuccessGetSingle('ContributionRecur', ['id' => $this->_contributionRecurID]); + $this->assertEquals('Completed', CRM_Core_PseudoConstant::getName('CRM_Contribute_BAO_ContributionRecur', 'contribution_status_id', $updatedContributionRecurAgain['contribution_status_id'])); + $this->assertEquals(date('Y-m-d'), substr($updatedContributionRecurAgain['end_date'], 0, 10)); // There should not be any email. $mut->assertMailLogEmpty(); } -- 2.25.1