From 1a3d69b0c03e36e23cca14935f583a2a9bee7e31 Mon Sep 17 00:00:00 2001
From: Seamus Lee <seamuslee001@gmail.com>
Date: Mon, 25 May 2020 11:47:13 +1000
Subject: [PATCH] dev/core#1775 Fix fee amount reversals when doing a change of
 financial type

---
 CRM/Contribute/BAO/Contribution.php       |  8 ++++++
 tests/phpunit/api/v3/ContributionTest.php | 34 ++++++++++++++++++++++-
 2 files changed, 41 insertions(+), 1 deletion(-)

diff --git a/CRM/Contribute/BAO/Contribution.php b/CRM/Contribute/BAO/Contribution.php
index 350bb0ff92..101e4ca9bd 100644
--- a/CRM/Contribute/BAO/Contribution.php
+++ b/CRM/Contribute/BAO/Contribution.php
@@ -3680,6 +3680,10 @@ INNER JOIN civicrm_activity ON civicrm_activity_contact.activity_id = civicrm_ac
           $newFinancialAccount = CRM_Financial_BAO_FinancialAccount::getFinancialAccountForFinancialTypeByRelationship($params['financial_type_id'], $accountRelationship);
           if ($oldFinancialAccount != $newFinancialAccount) {
             $params['total_amount'] = 0;
+            // If we have a fee amount set reverse this as well.
+            if (isset($params['fee_amount'])) {
+              $params['trxnParams']['fee_amount'] = 0 - $params['fee_amount'];
+            }
             if (in_array($params['contribution']->contribution_status_id, $pendingStatus)) {
               $params['trxnParams']['to_financial_account_id'] = CRM_Financial_BAO_FinancialAccount::getFinancialAccountForFinancialTypeByRelationship(
                 $params['prevContribution']->financial_type_id, $accountRelationship);
@@ -3701,6 +3705,10 @@ INNER JOIN civicrm_activity ON civicrm_activity_contact.activity_id = civicrm_ac
             /* $params['trxnParams']['to_financial_account_id'] = $trxnParams['to_financial_account_id']; */
             $params['financial_account_id'] = $newFinancialAccount;
             $params['total_amount'] = $params['trxnParams']['total_amount'] = $params['trxnParams']['net_amount'] = $trxnParams['total_amount'];
+            // Set the transaction fee amount back to the original value for creating the new positive financial trxn.
+            if (isset($params['fee_amount'])) {
+              $params['trxnParams']['fee_amount'] = $params['fee_amount'];
+            }
             self::updateFinancialAccounts($params);
             CRM_Core_BAO_FinancialTrxn::createDeferredTrxn(CRM_Utils_Array::value('line_item', $params), $params['contribution'], TRUE);
             $params['trxnParams']['to_financial_account_id'] = $trxnParams['to_financial_account_id'];
diff --git a/tests/phpunit/api/v3/ContributionTest.php b/tests/phpunit/api/v3/ContributionTest.php
index 16cfb90e51..b0781a9349 100644
--- a/tests/phpunit/api/v3/ContributionTest.php
+++ b/tests/phpunit/api/v3/ContributionTest.php
@@ -1710,6 +1710,32 @@ class api_v3_ContributionTest extends CiviUnitTestCase {
     $this->_checkFinancialItem($contribution['id'], 'changeFinancial');
   }
 
+  /**
+   * Function tets that financial records are correctly added when financial type is changed
+   *
+   * @throws \CRM_Core_Exception
+   */
+  public function testCreateUpdateContributionWithFeeAmountChangeFinancialType() {
+    $contribParams = [
+      'contact_id' => $this->_individualId,
+      'receive_date' => '2012-01-01',
+      'total_amount' => 100.00,
+      'fee_amount' => 0.57,
+      'financial_type_id' => 1,
+      'payment_instrument_id' => 1,
+      'contribution_status_id' => 1,
+
+    ];
+    $contribution = $this->callAPISuccess('contribution', 'create', $contribParams);
+    $newParams = array_merge($contribParams, [
+      'id' => $contribution['id'],
+      'financial_type_id' => 3,
+    ]);
+    $contribution = $this->callAPISuccess('contribution', 'create', $newParams);
+    $this->_checkFinancialTrxn($contribution, 'changeFinancial', NULL, ['fee_amount' => '-0.57', 'net_amount' => '-99.43']);
+    $this->_checkFinancialItem($contribution['id'], 'changeFinancial');
+  }
+
   /**
    * Test that update does not change status id CRM-15105.
    */
@@ -3900,7 +3926,7 @@ class api_v3_ContributionTest extends CiviUnitTestCase {
           'status_id' => 1,
         ];
       }
-      else {
+      elseif ($context !== 'changeFinancial') {
         $compareParams = [
           'total_amount' => 100,
           'status_id' => 1,
@@ -3915,6 +3941,12 @@ class api_v3_ContributionTest extends CiviUnitTestCase {
       }
       $this->assertDBCompareValues('CRM_Financial_DAO_FinancialTrxn', $trxnParams1, array_merge($compareParams, $extraParams));
       $compareParams['total_amount'] = 100;
+      // Reverse the extra params now that we will be checking the new positive transaction.
+      if ($context === 'changeFinancial' && !empty($extraParams)) {
+        foreach ($extraParams as $param => $value) {
+          $extraParams[$param] = 0 - $value;
+        }
+      }
     }
 
     $this->assertDBCompareValues('CRM_Financial_DAO_FinancialTrxn', $params, array_merge($compareParams, $extraParams));
-- 
2.25.1