From 06cad0d9a9301b487e880443a9062d365108a66c Mon Sep 17 00:00:00 2001
From: Jon Goldberg <jon@megaphonetech.com>
Date: Wed, 9 Dec 2020 11:20:05 -0500
Subject: [PATCH] financial#160 - set correct from_financial_account_id on an
 edited fee amount FinancialTrxn

---
 CRM/Contribute/BAO/Contribution.php           |  4 ++
 .../CRM/Core/BAO/FinancialTrxnTest.php        | 67 +++++++++++++++++++
 2 files changed, 71 insertions(+)

diff --git a/CRM/Contribute/BAO/Contribution.php b/CRM/Contribute/BAO/Contribution.php
index b7bbf5f323..2509468953 100644
--- a/CRM/Contribute/BAO/Contribution.php
+++ b/CRM/Contribute/BAO/Contribution.php
@@ -3473,6 +3473,10 @@ INNER JOIN civicrm_activity ON civicrm_activity_contact.activity_id = civicrm_ac
       elseif (!empty($params['payment_instrument_id'])) {
         $params['to_financial_account_id'] = CRM_Financial_BAO_FinancialTypeAccount::getInstrumentFinancialAccount($params['payment_instrument_id']);
       }
+      // dev/financial#160 - If this is a contribution update, also check for an existing payment_instrument_id.
+      elseif ($isUpdate && $params['prevContribution']->payment_instrument_id) {
+        $params['to_financial_account_id'] = CRM_Financial_BAO_FinancialTypeAccount::getInstrumentFinancialAccount((int) $params['prevContribution']->payment_instrument_id);
+      }
       else {
         $relationTypeId = key(CRM_Core_PseudoConstant::accountOptionValues('financial_account_type', NULL, " AND v.name LIKE 'Asset' "));
         $queryParams = [1 => [$relationTypeId, 'Integer']];
diff --git a/tests/phpunit/CRM/Core/BAO/FinancialTrxnTest.php b/tests/phpunit/CRM/Core/BAO/FinancialTrxnTest.php
index eb2d3b89e6..3c414a6315 100644
--- a/tests/phpunit/CRM/Core/BAO/FinancialTrxnTest.php
+++ b/tests/phpunit/CRM/Core/BAO/FinancialTrxnTest.php
@@ -249,4 +249,71 @@ class CRM_Core_BAO_FinancialTrxnTest extends CiviUnitTestCase {
     $this->assertEquals(0.01, $amountOwed, 'Amount does not match');
   }
 
+  /**
+   * Test that financial trxns for fee amounts have the correct financial account.
+   *
+   * @throws \CRM_Core_Exception
+   */
+  public function testFeeAmountTrxns() {
+    $contactId = $this->individualCreate();
+
+    // Get the expected financial account of a payment made with a credit card.
+    // Also et the financial account of a payment with the default payment method.
+    // I wish we could join here - maybe when API4 EntityBridge is complete.
+    $creditCardOvId = $this->callAPISuccess('OptionValue', 'getvalue', [
+      'return' => "id",
+      'option_group_id' => "payment_instrument",
+      'name' => "Credit Card",
+    ]);
+    $defaultPaymentMethodOvId = $this->callAPISuccess('OptionValue', 'getvalue', [
+      'return' => "id",
+      'option_group_id' => "payment_instrument",
+      'is_default' => 1,
+    ]);
+    $expectedFinancialAccountId = $this->callAPISuccess('EntityFinancialAccount', 'getvalue', [
+      'return' => "financial_account_id",
+      'entity_id' => $creditCardOvId,
+    ]);
+    $wrongFinancialAccountId = $this->callAPISuccess('EntityFinancialAccount', 'getvalue', [
+      'return' => "financial_account_id",
+      'entity_id' => $defaultPaymentMethodOvId,
+    ]);
+    // If these two are the same, there's no bug but this test is no longer valid and needs rewriting.
+    $this->assertNotEquals($expectedFinancialAccountId, $wrongFinancialAccountId, 'invalid test: Financial Account ID of credit card matches default payment method Financial Account ID');
+
+    // Create a credit card contribution with a fee amount.
+    $price = 100;
+    $cParams = [
+      'contact_id' => $contactId,
+      'total_amount' => $price,
+      'financial_type_id' => 1,
+      'is_active' => 1,
+      'payment_instrument_id' => 'Credit Card',
+      'fee_amount' => 3,
+    ];
+    $contribution = $this->callAPISuccess('Contribution', 'create', $cParams);
+    // Confirm that the from_financial_account_id amount on the fee trxn matches the expected value.
+    $trxnParams = [
+      'sequential' => 1,
+      'return' => ["financial_trxn_id.from_financial_account_id"],
+      'entity_table' => "civicrm_contribution",
+      'entity_id' => $contribution['id'],
+      'financial_trxn_id.total_amount' => 3,
+    ];
+    $firstFeeTrxnFromAccountId = $this->callAPISuccess('EntityFinancialTrxn', 'get', $trxnParams)['values'][0]['financial_trxn_id.from_financial_account_id'];
+    $this->assertEquals($expectedFinancialAccountId, $firstFeeTrxnFromAccountId);
+
+    // dev/financial#160 - ensure the from_financial_account_id is correct on a trxn generated by a contribution edit.
+    $updatedContributionParams = [
+      'id' => $contribution['id'],
+      'fee_amount' => 5,
+    ];
+    $this->callAPISuccess('Contribution', 'create', $updatedContributionParams);
+
+    // 2 = 5 - 3.
+    $trxnParams['financial_trxn_id.total_amount'] = 2;
+    $secondFeeTrxnFromAccountId = $this->callAPISuccess('EntityFinancialTrxn', 'get', $trxnParams)['values'][0]['financial_trxn_id.from_financial_account_id'];
+    $this->assertEquals($expectedFinancialAccountId, $secondFeeTrxnFromAccountId);
+  }
+
 }
-- 
2.25.1