Fix cancel payment action to reverse financial items related to cancelled payment

The unit test to test & ensure this was failing to pick up that the records were not being created because it iterated
through the created rows & checked they were right but there was no check to ensure the rows were created :-(

This still leaves a gap I think for refunds not tied to a specific payment but that can be a later step

diff --git a/CRM/Financial/BAO/Payment.php b/CRM/Financial/BAO/Payment.php
index 1a9676379843904c2a557d4d666508111a2f6fbb..3f08b2e8843798fb1606e58ed7483c3f5d658bae 100644 (file)
--- a/CRM/Financial/BAO/Payment.php
+++ b/CRM/Financial/BAO/Payment.php
@@ -157,6 +157,22 @@ class CRM_Financial_BAO_Payment {
     }
     elseif ($params['total_amount'] < 0) {
       $trxn = self::recordRefundPayment($params['contribution_id'], $params, FALSE);
+      if (!empty($params['cancelled_payment_id'])) {
+        // Do a direct reversal of any entity_financial_trxn records being cancelled.
+        $entityFinancialTrxns = civicrm_api3('EntityFinancialTrxn', 'get', [
+          'entity_table' => 'civicrm_financial_item',
+          'options' => ['limit' => 0],
+          'financial_trxn_id.id' => $params['cancelled_payment_id'],
+        ])['values'];
+        foreach ($entityFinancialTrxns as $entityFinancialTrxn) {
+          civicrm_api3('EntityFinancialTrxn', 'create', [
+            'entity_table' => 'civicrm_financial_item',
+            'entity_id' => $entityFinancialTrxn['entity_id'],
+            'amount' => -$entityFinancialTrxn['amount'],
+            'financial_trxn_id' => $trxn->id,
+          ]);
+        }
+      }
     }
 
     if ($isPaymentCompletesContribution) {
@@ -399,6 +415,7 @@ class CRM_Financial_BAO_Payment {
    *   - deprecate this param
    *
    * @return CRM_Financial_DAO_FinancialTrxn
+   * @throws \CiviCRM_API3_Exception
    */
   protected static function recordRefundPayment($contributionId, $trxnData, $updateStatus) {
     list($contributionDAO, $params) = self::getContributionAndParamsInFormatForRecordFinancialTransaction($contributionId);

diff --git a/api/v3/Payment.php b/api/v3/Payment.php
index 84023028667580f8889d065ff9522fe6037cb74f..3dad955fbd219458fc132c40def66b6be5e0631f 100644 (file)
--- a/api/v3/Payment.php
+++ b/api/v3/Payment.php
@@ -97,9 +97,11 @@ function civicrm_api3_payment_delete($params) {
  * @param array $params
  *   Input parameters.
  *
- * @throws API_Exception
  * @return array
  *   Api result array
+ *
+ * @throws \CiviCRM_API3_Exception
+ * @throws API_Exception
  */
 function civicrm_api3_payment_cancel($params) {
   $eftParams = [
@@ -112,6 +114,7 @@ function civicrm_api3_payment_cancel($params) {
     'total_amount' => -$entity['amount'],
     'contribution_id' => $entity['entity_id'],
     'trxn_date' => CRM_Utils_Array::value('trxn_date', $params, 'now'),
+    'cancelled_payment_id' => $params['id'],
   ];
 
   foreach (['trxn_id', 'payment_instrument_id'] as $permittedParam) {

diff --git a/tests/phpunit/api/v3/PaymentTest.php b/tests/phpunit/api/v3/PaymentTest.php
index 0b85d83aa8e0713be20d629804dfa3e641413938..b0ad5c5f5908e878840714b976ce5a2c8df462e6 100644 (file)
--- a/tests/phpunit/api/v3/PaymentTest.php
+++ b/tests/phpunit/api/v3/PaymentTest.php
@@ -620,10 +620,11 @@ class api_v3_PaymentTest extends CiviUnitTestCase {
       'financial_trxn_id' => $payment['id'] - 1,
     ];
 
-    $eft = $this->callAPISuccess('EntityFinancialTrxn', 'get', $minParams);
+    $eft = $this->callAPISuccess('EntityFinancialTrxn', 'get', $minParams)['values'];
+    $this->assertCount(2, $eft);
     $amounts = [-33.33, -16.67];
 
-    foreach ($eft['values'] as $value) {
+    foreach ($eft as $value) {
       $this->assertEquals($value['amount'], array_pop($amounts));
     }
 
@@ -632,9 +633,9 @@ class api_v3_PaymentTest extends CiviUnitTestCase {
       'entity_table' => 'civicrm_financial_item',
       'financial_trxn_id' => $payment['id'],
     ];
-    $eft = $this->callAPISuccess('EntityFinancialTrxn', 'get', $params);
+    $eft = $this->callAPISuccess('EntityFinancialTrxn', 'get', $params)['values'];
     $amounts = [66.67, 33.33];
-    foreach ($eft['values'] as $value) {
+    foreach ($eft as $value) {
       $this->assertEquals($value['amount'], array_pop($amounts));
     }
     $items = $this->callAPISuccess('FinancialItem', 'get', [])['values'];