From 273056c515461d14f23b9fe82c08a0cfb01b5ce0 Mon Sep 17 00:00:00 2001
From: Pradeep Nayak <pradpnayak@gmail.com>
Date: Mon, 11 Sep 2017 18:28:07 +0530
Subject: [PATCH] CRM-20276, fixed code to store correct value in financial
 item table for amount when contribution amount is changed

----------------------------------------
* CRM-20276: When editing a contribution the value in civicrm_financial_item_amount is not updated
  https://issues.civicrm.org/jira/browse/CRM-20276
---
 CRM/Contribute/BAO/Contribution.php           | 26 ++++++++++++++-----
 .../CRM/Contribute/Form/ContributionTest.php  |  5 ++--
 2 files changed, 21 insertions(+), 10 deletions(-)

diff --git a/CRM/Contribute/BAO/Contribution.php b/CRM/Contribute/BAO/Contribution.php
index 374296ed58..d05ea37a6c 100644
--- a/CRM/Contribute/BAO/Contribution.php
+++ b/CRM/Contribute/BAO/Contribution.php
@@ -3633,6 +3633,7 @@ INNER JOIN civicrm_activity ON civicrm_activity_contact.activity_id = civicrm_ac
     if ($context != 'changePaymentInstrument') {
       $itemParams['entity_table'] = 'civicrm_line_item';
       $trxnIds['id'] = $params['entity_id'];
+      $previousLineItems = CRM_Price_BAO_LineItem::getLineItemsByContributionID($params['contribution']->id);
       foreach ($params['line_item'] as $fieldId => $fields) {
         foreach ($fields as $fieldValueId => $lineItemDetails) {
           $prevFinancialItem = CRM_Financial_BAO_FinancialItem::getPreviousFinancialItem($lineItemDetails['id']);
@@ -3647,12 +3648,12 @@ INNER JOIN civicrm_activity ON civicrm_activity_contact.activity_id = civicrm_ac
           if ($params['contribution']->currency) {
             $currency = $params['contribution']->currency;
           }
-
+          $previousLineItemTotal = CRM_Utils_Array::value('line_total', CRM_Utils_Array::value($fieldValueId, $previousLineItems), 0);
           $itemParams = array(
             'transaction_date' => $receiveDate,
             'contact_id' => $params['prevContribution']->contact_id,
             'currency' => $currency,
-            'amount' => self::getFinancialItemAmountFromParams($inputParams, $context, $lineItemDetails, $isARefund),
+            'amount' => self::getFinancialItemAmountFromParams($inputParams, $context, $lineItemDetails, $isARefund, $previousLineItemTotal),
             'description' => $prevFinancialItem['description'],
             'status_id' => $prevFinancialItem['status_id'],
             'financial_account_id' => $financialAccount,
@@ -3666,7 +3667,11 @@ INNER JOIN civicrm_activity ON civicrm_activity_contact.activity_id = civicrm_ac
           if ($lineItemDetails['tax_amount'] && $lineItemDetails['tax_amount'] !== 'null') {
             $invoiceSettings = Civi::settings()->get('contribution_invoice_settings');
             $taxTerm = CRM_Utils_Array::value('tax_term', $invoiceSettings);
-            $itemParams['amount'] = self::getMultiplier($params['contribution']->contribution_status_id, $context) * $lineItemDetails['tax_amount'];
+            $taxAmount = $lineItemDetails['tax_amount'];
+            if ($previousLineItemTotal != $lineItemDetails['line_total']) {
+              $taxAmount -= CRM_Utils_Array::value('tax_amount', CRM_Utils_Array::value($fieldValueId, $previousLineItems), 0);
+            }
+            $itemParams['amount'] = self::getMultiplier($params['contribution']->contribution_status_id, $context) * $taxAmount;
             $itemParams['description'] = $taxTerm;
             if ($lineItemDetails['financial_type_id']) {
               $itemParams['financial_account_id'] = CRM_Contribute_PseudoConstant::getRelationalFinancialAccount(
@@ -5353,9 +5358,16 @@ LEFT JOIN  civicrm_contribution on (civicrm_contribution.contact_id = civicrm_co
    *
    * @return float
    */
-  protected static function getFinancialItemAmountFromParams($params, $context, $lineItemDetails, $isARefund) {
-    if ($context == 'changedAmount' || $context == 'changeFinancialType') {
-      return $params['total_amount'] - $params['prevContribution']->total_amount;
+  protected static function getFinancialItemAmountFromParams($params, $context, $lineItemDetails, $isARefund, $previousLineItemTotal) {
+    if ($context == 'changedAmount') {
+      $lineTotal = $lineItemDetails['line_total'];
+      if ($lineTotal != $previousLineItemTotal) {
+        $lineTotal -= $previousLineItemTotal;
+      }
+      return $lineTotal;
+    }
+    else if ($context == 'changeFinancialType') {
+      return -$lineItemDetails['line_total'];
     }
     elseif ($context == 'changedStatus') {
       $cancelledTaxAmount = 0;
@@ -5366,7 +5378,7 @@ LEFT JOIN  civicrm_contribution on (civicrm_contribution.contact_id = civicrm_co
     }
     elseif ($context === NULL) {
       // erm, yes because? but, hey, it's tested.
-      return $params['total_amount'];
+      return $lineItemDetails['line_total'];
     }
     elseif (empty($lineItemDetails['line_total'])) {
       // follow legacy code path
diff --git a/tests/phpunit/CRM/Contribute/Form/ContributionTest.php b/tests/phpunit/CRM/Contribute/Form/ContributionTest.php
index f24154d1b8..c022cadcb3 100644
--- a/tests/phpunit/CRM/Contribute/Form/ContributionTest.php
+++ b/tests/phpunit/CRM/Contribute/Form/ContributionTest.php
@@ -1185,9 +1185,8 @@ Price Field - Price Field 1        1   $ 100.00      $ 100.00
 
     $this->assertEquals(100, $items['values'][0]['amount']);
     $this->assertEquals(10, $items['values'][1]['amount']);
-    // @todo currently its $110 which is incorrect, the proper value should be $200
-    // $this->assertEquals(200, $items['values'][2]['amount']);
-    $this->assertEquals(20, $items['values'][3]['amount']);
+    $this->assertEquals(100, $items['values'][2]['amount']);
+    $this->assertEquals(10, $items['values'][3]['amount']);
   }
 
   /**
-- 
2.25.1