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

diff --git a/CRM/Core/Payment/AuthorizeNetIPN.php b/CRM/Core/Payment/AuthorizeNetIPN.php
index 7fa588e8d5b4de9065f8a26db7d2dd7b0fa92e16..12aa6cce8476938574177ca372a8ade390788ac7 100644 (file)
--- 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<p>";
       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 b402fbab73c727a514c21d6943dff5cbc8a652ff..8e89b2c7c6fa0a84fb701272993049a529331d57 100644 (file)
--- 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();
   }