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

diff --git a/CRM/Price/BAO/LineItem.php b/CRM/Price/BAO/LineItem.php
index f381e971fd6421d03babd9085e67d99f04aba021..b94b20755af5c0f7a12ce73bdaa4305cdbc5f7a1 100644 (file)
--- 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 bc87b6e9fe53c44ffdab2645e7d17b3d58d6e313..d2c1643071a2aa24f8b65f24e6ff20f2071a0a2d 100644 (file)
--- 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 3a618e512ed723cb007df2c6ea902eecab3d3152..329850de106deb12fc510a5069a12db7a9c8f748 100644 (file)
--- 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 ef0d0e99bda8a0b0c14ca9583844f1e9c8f1a361..6ac5753209f4f1e9fd953e26415b90fad69b1d3c 100644 (file)
--- 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