From: eileen <emcnaughton@wikimedia.org>
Date: Thu, 8 Apr 2021 22:50:50 +0000 (+1200)
Subject: Fix Payment.create to update (recalculate) contribution fee_amount
X-Git-Url: https://vcs.fsf.org/?a=commitdiff_plain;h=dbdad838d2d72d35376ebba1e2a49c82ffd3974e;p=civicrm-core.git

Fix Payment.create to update (recalculate) contribution fee_amount

This recalculates the fee amount when a payment is received. Our data integrity expects the fee
amount on the contribution to equal the sum of the fee amount on the payments. However,
it's possible a pending contribution has an 'estimated' fee amount so we can't just add on
the fee_amount and assume it will be right
---

diff --git a/CRM/Financial/BAO/Payment.php b/CRM/Financial/BAO/Payment.php
index fb2d374f76..02fbf35561 100644
--- a/CRM/Financial/BAO/Payment.php
+++ b/CRM/Financial/BAO/Payment.php
@@ -36,7 +36,7 @@ class CRM_Financial_BAO_Payment {
    * @throws \CRM_Core_Exception
    * @throws \CiviCRM_API3_Exception
    */
-  public static function create($params) {
+  public static function create(array $params): CRM_Financial_DAO_FinancialTrxn {
     $contribution = civicrm_api3('Contribution', 'getsingle', ['id' => $params['contribution_id']]);
     $contributionStatus = CRM_Core_PseudoConstant::getName('CRM_Contribute_BAO_Contribution', 'contribution_status_id', $contribution['contribution_status_id']);
     $isPaymentCompletesContribution = self::isPaymentCompletesContribution($params['contribution_id'], $params['total_amount'], $contributionStatus);
@@ -96,7 +96,7 @@ class CRM_Financial_BAO_Payment {
       self::reverseAllocationsFromPreviousPayment($params, $trxn->id);
     }
     else {
-      list($ftIds, $taxItems) = CRM_Contribute_BAO_Contribution::getLastFinancialItemIds($params['contribution_id']);
+      [$ftIds, $taxItems] = CRM_Contribute_BAO_Contribution::getLastFinancialItemIds($params['contribution_id']);
 
       foreach ($lineItems as $key => $value) {
         if ($value['allocation'] === (float) 0) {
@@ -185,15 +185,32 @@ class CRM_Financial_BAO_Payment {
 
   /**
    * Function to update contribution's check_number and trxn_id by
-   *  concatenating values from financial trxn's check_number and trxn_id respectively
+   *  concatenating values from financial trxn's check_number and trxn_id
+   * respectively
    *
    * @param array $params
    * @param int $contributionID
+   *
+   * @throws \CiviCRM_API3_Exception
    */
-  public static function updateRelatedContribution($params, $contributionID) {
+  public static function updateRelatedContribution(array $params, int $contributionID): void {
     $contributionDAO = new CRM_Contribute_DAO_Contribution();
     $contributionDAO->id = $contributionID;
     $contributionDAO->find(TRUE);
+    if (isset($params['fee_amount'])) {
+      // Update contribution.fee_amount to be be the total of all fees
+      // since the payment is already saved the total here will be right.
+      $payments = civicrm_api3('Payment', 'get', [
+        'contribution_id' => $contributionID,
+        'return' => 'fee_amount',
+      ])['values'];
+      $totalFees = 0;
+      foreach ($payments as $payment) {
+        $totalFees += $payment['fee_amount'] ?? 0;
+      }
+      $contributionDAO->fee_amount = $totalFees;
+      $contributionDAO->net_amount = $contributionDAO->total_amount - $contributionDAO->fee_amount;
+    }
 
     foreach (['trxn_id', 'check_number'] as $fieldName) {
       if (!empty($params[$fieldName])) {
@@ -292,7 +309,7 @@ class CRM_Financial_BAO_Payment {
     );
 
     $contactID = self::getPaymentContactID($contributionID);
-    list($displayName, $email)  = CRM_Contact_BAO_Contact_Location::getEmailDetails($contactID);
+    [$displayName, $email]  = CRM_Contact_BAO_Contact_Location::getEmailDetails($contactID);
     $entities['contact'] = ['id' => $contactID, 'display_name' => $displayName, 'email' => $email];
     $contact = civicrm_api3('Contact', 'getsingle', ['id' => $contactID, 'return' => 'email_greeting']);
     $entities['contact']['email_greeting'] = $contact['email_greeting_display'];
diff --git a/tests/phpunit/api/v3/PaymentTest.php b/tests/phpunit/api/v3/PaymentTest.php
index 4d1cff33bb..4e6721c963 100644
--- a/tests/phpunit/api/v3/PaymentTest.php
+++ b/tests/phpunit/api/v3/PaymentTest.php
@@ -1290,7 +1290,7 @@ class api_v3_PaymentTest extends CiviUnitTestCase {
   public function testPaymentCreateTrxnIdAndDates(): void {
 
     $trxnDate = '2010-01-01 09:00:00';
-    $trxnID = 'aabbccddeeffggh';
+    $trxnID = 'abcd';
     $originalReceiveDate = '2010-02-02 22:22:22';
 
     $contributionID = $this->contributionCreate([
@@ -1298,6 +1298,7 @@ class api_v3_PaymentTest extends CiviUnitTestCase {
       'total_amount'           => 100,
       'contribution_status_id' => 'Pending',
       'receive_date'           => $originalReceiveDate,
+      'fee_amount' => 0,
     ]);
 
     $this->callAPISuccess('Payment', 'create', [
@@ -1305,10 +1306,13 @@ class api_v3_PaymentTest extends CiviUnitTestCase {
       'order_id'     => $contributionID,
       'trxn_date'    => $trxnDate,
       'trxn_id'      => $trxnID,
+      'fee_amount' => .2,
     ]);
 
     $contribution = $this->callAPISuccessGetSingle('Contribution', ['id' => $contributionID]);
     $this->assertEquals('Completed', $contribution['contribution_status']);
+    $this->assertEquals(.2, $contribution['fee_amount']);
+    $this->assertEquals(99.8, $contribution['net_amount']);
 
     $this->assertEquals($trxnID, $contribution['trxn_id'],
       "Contribution trxn_id should have been set to that of the payment.");