From 9f68fe61087b99fe6a795f13d30a206be51d125a Mon Sep 17 00:00:00 2001
From: Matthew Wire <devel@mrwire.co.uk>
Date: Fri, 6 Jul 2018 17:34:18 +0100
Subject: [PATCH] Improve PayPalIPN unit tests

---
 CRM/Core/Payment/PayPalIPN.php                |  1 -
 .../CRM/Core/Payment/PayPalIPNTest.php        | 29 ++++++++++++-----
 tests/phpunit/CiviTest/CiviUnitTestCase.php   | 32 +++++++++++--------
 3 files changed, 40 insertions(+), 22 deletions(-)

diff --git a/CRM/Core/Payment/PayPalIPN.php b/CRM/Core/Payment/PayPalIPN.php
index ff16f6af5d..fa4f164a42 100644
--- a/CRM/Core/Payment/PayPalIPN.php
+++ b/CRM/Core/Payment/PayPalIPN.php
@@ -226,7 +226,6 @@ 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['invoice_id'] = md5(uniqid(rand(), TRUE));
       $input['original_contribution_id'] = $ids['contribution'];
       $input['contribution_recur_id'] = $ids['contributionRecur'];
 
diff --git a/tests/phpunit/CRM/Core/Payment/PayPalIPNTest.php b/tests/phpunit/CRM/Core/Payment/PayPalIPNTest.php
index 8e13270b4d..9de32d4ea3 100644
--- a/tests/phpunit/CRM/Core/Payment/PayPalIPNTest.php
+++ b/tests/phpunit/CRM/Core/Payment/PayPalIPNTest.php
@@ -121,24 +121,37 @@ class CRM_Core_Payment_PayPalIPNTest extends CiviUnitTestCase {
    * The second will create a new contribution.
    */
   public function testIPNPaymentRecurSuccess() {
-    $this->setupRecurringPaymentProcessorTransaction();
+    $this->setupRecurringPaymentProcessorTransaction([], ['total_amount' => '15.00']);
     $paypalIPN = new CRM_Core_Payment_PayPalIPN($this->getPaypalRecurTransaction());
     $paypalIPN->main();
-    $contribution = $this->callAPISuccess('contribution', 'getsingle', array('id' => $this->_contributionID));
-    $this->assertEquals(1, $contribution['contribution_status_id']);
-    $this->assertEquals('8XA571746W2698126', $contribution['trxn_id']);
+    $contribution1 = $this->callAPISuccess('contribution', 'getsingle', array('id' => $this->_contributionID));
+    $this->assertEquals(1, $contribution1['contribution_status_id']);
+    $this->assertEquals('8XA571746W2698126', $contribution1['trxn_id']);
     // source gets set by processor
-    $this->assertTrue(substr($contribution['contribution_source'], 0, 20) == "Online Contribution:");
+    $this->assertTrue(substr($contribution1['contribution_source'], 0, 20) == "Online Contribution:");
     $contributionRecur = $this->callAPISuccess('contribution_recur', 'getsingle', array('id' => $this->_contributionRecurID));
     $this->assertEquals(5, $contributionRecur['contribution_status_id']);
     $paypalIPN = new CRM_Core_Payment_PayPalIPN($this->getPaypalRecurSubsequentTransaction());
     $paypalIPN->main();
-    $contribution = $this->callAPISuccess('contribution', 'get', array(
+    $contributions = $this->callAPISuccess('contribution', 'get', array(
         'contribution_recur_id' => $this->_contributionRecurID,
         'sequential' => 1,
       ));
-    $this->assertEquals(2, $contribution['count']);
-    $this->assertEquals('secondone', $contribution['values'][1]['trxn_id']);
+    $this->assertEquals(2, $contributions['count']);
+    $contribution2 = $contributions['values'][1];
+    $this->assertEquals('secondone', $contribution2['trxn_id']);
+    $paramsThatShouldMatch = [
+      'total_amount',
+      'net_amount',
+      'fee_amount',
+      'payment_instrument',
+      'payment_instrument_id',
+      'financial_type',
+      'financial_type_id',
+    ];
+    foreach ($paramsThatShouldMatch as $match) {
+      $this->assertEquals($contribution1[$match], $contribution2[$match]);
+    }
   }
 
   /**
diff --git a/tests/phpunit/CiviTest/CiviUnitTestCase.php b/tests/phpunit/CiviTest/CiviUnitTestCase.php
index 71cd39915e..89d4720d09 100644
--- a/tests/phpunit/CiviTest/CiviUnitTestCase.php
+++ b/tests/phpunit/CiviTest/CiviUnitTestCase.php
@@ -2775,8 +2775,24 @@ AND    ( TABLE_NAME LIKE 'civicrm_value_%' )
 
   /**
    * Set up initial recurring payment allowing subsequent IPN payments.
+   *
+   * @param array $recurParams (Optional)
+   * @param array $contributionParams (Optional)
    */
-  public function setupRecurringPaymentProcessorTransaction($params = array()) {
+  public function setupRecurringPaymentProcessorTransaction($recurParams = [], $contributionParams = []) {
+    $contributionParams = array_merge([
+        'total_amount' => '200',
+        'invoice_id' => $this->_invoiceID,
+        'financial_type_id' => 'Donation',
+        'contribution_status_id' => 'Pending',
+        'contact_id' => $this->_contactID,
+        'contribution_page_id' => $this->_contributionPageID,
+        'payment_processor_id' => $this->_paymentProcessorID,
+        'is_test' => 0,
+        'skipCleanMoney' => TRUE,
+      ],
+      $contributionParams
+    );
     $contributionRecur = $this->callAPISuccess('contribution_recur', 'create', array_merge(array(
       'contact_id' => $this->_contactID,
       'amount' => 1000,
@@ -2789,18 +2805,8 @@ AND    ( TABLE_NAME LIKE 'civicrm_value_%' )
       'payment_processor_id' => $this->_paymentProcessorID,
       // processor provided ID - use contact ID as proxy.
       'processor_id' => $this->_contactID,
-      'api.contribution.create' => array(
-        'total_amount' => '200',
-        'invoice_id' => $this->_invoiceID,
-        'financial_type_id' => 1,
-        'contribution_status_id' => 'Pending',
-        'contact_id' => $this->_contactID,
-        'contribution_page_id' => $this->_contributionPageID,
-        'payment_processor_id' => $this->_paymentProcessorID,
-        'is_test' => 0,
-        'skipCleanMoney' => TRUE,
-      ),
-    ), $params));
+      'api.contribution.create' => $contributionParams,
+    ), $recurParams));
     $this->_contributionRecurID = $contributionRecur['id'];
     $this->_contributionID = $contributionRecur['values']['0']['api.contribution.create']['id'];
   }
-- 
2.25.1