From dfdee38776797b83b6d970fd4042740bde1186eb Mon Sep 17 00:00:00 2001
From: eileen <emcnaughton@wikimedia.org>
Date: Mon, 31 Aug 2020 10:10:35 +1200
Subject: [PATCH] dev/core#1983 Fix to tax calculation on multi-line-item

This is similar to https://github.com/civicrm/civicrm-core/pull/18284 - it differs in that the totals are calculated by iterating
through the line item array afterwards, rather than expecting the 'getLine' function to calculate totals. Some
obvious follow ups suggest themselves but I will look against master.

This is difficult to test (Karin gave it a really good shot) because of the weird way it's calculated in Main and thenn
used in Confirm. Cleanup should resolve the testability issue too
---
 CRM/Price/BAO/LineItem.php                             | 3 ++-
 CRM/Price/BAO/PriceSet.php                             | 6 ++++--
 tests/phpunit/CRM/Event/BAO/ChangeFeeSelectionTest.php | 2 +-
 tests/phpunit/CRM/Member/Form/MembershipTest.php       | 1 +
 4 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/CRM/Price/BAO/LineItem.php b/CRM/Price/BAO/LineItem.php
index f381e971fd6..b94b20755af 100644
--- a/CRM/Price/BAO/LineItem.php
+++ b/CRM/Price/BAO/LineItem.php
@@ -654,8 +654,9 @@ WHERE li.contribution_id = %1";
     $lineItems
   ) {
     $entityTable = "civicrm_" . $entity;
+    $newLineItems = [];
     CRM_Price_BAO_PriceSet::processAmount($feeBlock,
-      $params, $lineItems
+      $params, $newLineItems
     );
     // initialize empty Lineitem instance to call protected helper functions
     $lineItemObj = new CRM_Price_BAO_LineItem();
diff --git a/CRM/Price/BAO/PriceSet.php b/CRM/Price/BAO/PriceSet.php
index bc87b6e9fe5..d2c1643071a 100644
--- a/CRM/Price/BAO/PriceSet.php
+++ b/CRM/Price/BAO/PriceSet.php
@@ -673,13 +673,15 @@ WHERE  id = %1";
         continue;
       }
 
-      list($params, $lineItem, $totalTax, $totalPrice) = self::getLine($params, $lineItem, $priceSetID, $field, $id, $totalPrice);
+      list($params, $lineItem) = self::getLine($params, $lineItem, $priceSetID, $field, $id, $totalPrice);
     }
 
     $amount_level = [];
     $totalParticipant = 0;
     if (is_array($lineItem)) {
       foreach ($lineItem as $values) {
+        $totalPrice += $values['line_total'] + $values['tax_amount'];
+        $totalTax += $values['tax_amount'];
         $totalParticipant += $values['participant_count'];
         // This is a bit nasty. The logic of 'quick config' was because price set configuration was
         // (and still is) too difficult to replace the 'quick config' price set configuration on the contribution
@@ -1767,7 +1769,7 @@ WHERE     ct.id = cp.financial_type_id AND
         }
         break;
     }
-    return [$params, $lineItem, $totalTax, $totalPrice];
+    return [$params, $lineItem];
   }
 
 }
diff --git a/tests/phpunit/CRM/Event/BAO/ChangeFeeSelectionTest.php b/tests/phpunit/CRM/Event/BAO/ChangeFeeSelectionTest.php
index 3a618e512ed..329850de106 100644
--- a/tests/phpunit/CRM/Event/BAO/ChangeFeeSelectionTest.php
+++ b/tests/phpunit/CRM/Event/BAO/ChangeFeeSelectionTest.php
@@ -490,7 +490,7 @@ class CRM_Event_BAO_ChangeFeeSelectionTest extends CiviUnitTestCase {
     $lineItem = CRM_Price_BAO_LineItem::getLineItems($this->_participantId, 'participant');
     $this->assertEquals($this->_expensiveFee, $lineItem[1]['line_total']);
     CRM_Price_BAO_LineItem::changeFeeSelections($priceSetParams, $this->_participantId, 'participant', $this->_contributionId, $this->_feeBlock, $lineItem);
-    $this->assertDBCompareValue('CRM_Contribute_BAO_Contribution', $this->_contributionId, 'total_amount', 'id', $this->_cheapFee, "Total Amount equals " . $this->_expensiveFee);
+    $this->assertDBCompareValue('CRM_Contribute_BAO_Contribution', $this->_contributionId, 'total_amount', 'id', $this->_cheapFee, 'Total Amount equals ' . $this->_expensiveFee);
     $this->assertDBCompareValue('CRM_Contribute_BAO_Contribution', $this->_contributionId, 'contribution_status_id', 'id', $pendingRefundStatusId, 'Contribution must be refunding');
     $priceSetParams['price_' . $this->priceSetFieldID] = $this->expensiveFeeValueID;
     $lineItem = CRM_Price_BAO_LineItem::getLineItems($this->_participantId, 'participant');
diff --git a/tests/phpunit/CRM/Member/Form/MembershipTest.php b/tests/phpunit/CRM/Member/Form/MembershipTest.php
index ef0d0e99bda..6ac5753209f 100644
--- a/tests/phpunit/CRM/Member/Form/MembershipTest.php
+++ b/tests/phpunit/CRM/Member/Form/MembershipTest.php
@@ -536,6 +536,7 @@ class CRM_Member_Form_MembershipTest extends CiviUnitTestCase {
    *  Check if the related contribuion is also updated if the minimum_fee didn't match
    *
    * @throws \CRM_Core_Exception
+   * @throws \CiviCRM_API3_Exception
    */
   public function testContributionUpdateOnMembershipTypeChange() {
     // Step 1: Create a Membership via backoffice whose with 50.00 payment
-- 
GitLab