Total Tax Amount on the Contribution (generated in backoffice/offline) no longer adds up to sum of the Tax Amount for the individual line items

tax_amount is indeed 0.00 in the database for this contribution.

Interesting is that the tax amount is correct ($35.09) on the Print Invoice PDF. This suggests that the Invoice code is doing Math as well.

Ok so invoice.php -> tax_amount on the Contribution is not used -> instead the tax_amount for the individual line items is summed up:

$dataArray[(string) $taxRate['tax_rate']] = $dataArray[(string) $taxRate['tax_rate']] + CRM_Utils_Array::value('tax_amount', $taxRate);

https://github.com/civicrm/civicrm-core/blob/master/CRM/Contribute/Form/Task/Invoice.php#L278

Looking at the previous sale tax unit tests I've added in the past to see if I can expand one to cover this scenario.

changed the description

Pradeep put up a patch for a different scenario (Contribution Page) https://github.com/civicrm/civicrm-core/pull/18284/files - but with the same symptoms -> so trying that here.

Confirming that it fixes the backoffice issue as well.

[karin@ectemnia BAO]$ diff  PriceSet.php.DIST  PriceSet.php
676c676
<       list($params, $lineItem, $totalTax, $totalPrice) = self::getLine($params, $lineItem, $priceSetID, $field, $id, $totalPrice);
---
>       list($params, $lineItem, $totalTax, $totalPrice) = self::getLine($params, $lineItem, $priceSetID, $field, $id, $totalPrice, $totalTax);
1694,1695c1694
<   public static function getLine(&$params, &$lineItem, $priceSetID, $field, $id, $totalPrice): array {
<     $totalTax = 0;
---
>   public static function getLine(&$params, &$lineItem, $priceSetID, $field, $id, $totalPrice, &$totalTax = 0): array {
[karin@ectemnia BAO]$

I've expanded

public function testSubmitContributionPageWithPriceSetQuantity($thousandSeparator) {

to two Line Items hoping to be able to then run a test that checks if the tax_amount at the Contribution level matches the sum of the tax_amount for the two Line Items, but the

$contribution = $this->callAPISuccessGetSingle('contribution', [
  'contribution_page_id' => $this->_ids['contribution_page'],
]);

Does not return any $contribution['tax_amount'] It's unset.

So not sure how we can test the tax_amount at the Contribution level? And why is it unset?

@KarinG what about adding in the following at the end of your test

$finalContribution = $this->callAPISuccess('Contribution', 'getsingle', ['id' => $contribution['id']]);
$this->assertEquals($lineItem_TaxAmount, $finalContribution['tax_amount']);

@seamuslee - that's similar to what I was trying. The problem is that there isn't a ['tax_amount'] -> in scope in this test:

https://github.com/civicrm/civicrm-core/blob/master/tests/phpunit/api/v3/ContributionPageTest.php#L1933

Check my variables ->

@KarinG I tried this locally and seemed to do something half sensible for me

diff --git a/tests/phpunit/api/v3/ContributionPageTest.php b/tests/phpunit/api/v3/ContributionPageTest.php
index 25eb05e65f..a5d47d9864 100644
--- a/tests/phpunit/api/v3/ContributionPageTest.php
+++ b/tests/phpunit/api/v3/ContributionPageTest.php
@@ -1988,7 +1988,9 @@ class api_v3_ContributionPageTest extends CiviUnitTestCase {
     $lineItem_TaxAmount = round($lineItem['tax_amount'], 2);

     $this->assertEquals($lineItem['line_total'], $contribution['total_amount'], 'Contribution total should match line total');
-    $this->assertEquals($lineItem_TaxAmount, round(180 * 16.95 * 0.10, 2), 'Wrong Sales Tax Amount is calculated and stored.');
+    $this->assertEquals(round(180 * 16.95 * 0.10, 2), $lineItem_TaxAmount, 'Wrong Sales Tax Amount is calculated and stored.');
+    $finalContribution = $this->callAPISuccess('Contribution', 'getsingle', ['id' => $contribution['id'], 'return' => ['tax_amount']]);
+    $this->assertEquals($lineItem_TaxAmount, $finalContribution['tax_amount']);
   }

   /**

But there is no $finalContribution['tax_amount']

My rewritten version of that test is:

  public function testSubmitContributionPageWithPriceSetQuantity($thousandSeparator) {
    $this->setCurrencySeparators($thousandSeparator);
    $this->_priceSetParams['is_quick_config'] = 0;
    $this->enableTaxAndInvoicing();
    $financialType = $this->createFinancialType();
    $financialTypeId = $financialType['id'];
    // This function sets the Tax Rate at 10% - it currently has no way to pass Tax Rate into it - so let's work with 10%
    $this->relationForFinancialTypeWithFinancialAccount($financialType['id']);

    $this->setUpContributionPage();
    $submitParams = [
      'price_' . $this->_ids['price_field'][0] => reset($this->_ids['price_field_value']),
      'id' => (int) $this->_ids['contribution_page'],
      'first_name' => 'J',
      'last_name' => 'T',
      'email' => 'JT@ohcanada.ca',
      'is_pay_later' => TRUE,
      'receive_date' => date('Y-m-d H:i:s'),
    ];

    // Create PriceSet/PriceField
    $priceSetID = reset($this->_ids['price_set']);
    $priceField = $this->callAPISuccess('price_field', 'create', [
      'price_set_id' => $priceSetID,
      'label' => 'Printing Rights',
      'html_type' => 'Text',
    ]);

    $this->callAPISuccess('price_field_value', 'create', [
      'price_set_id' => $priceSetID,
      'price_field_id' => $priceField['id'],
      'label' => 'Printing Rights',
      'financial_type_id' => $financialTypeId,
      'amount' => '16.95',
    ]);
    $priceFieldId = $priceField['id'];

    // Set quantity for our test
    $submitParams['price_' . $priceFieldId] = 180;

    $priceField = $this->callAPISuccess('price_field', 'create', [
      'price_set_id' => $priceSetID,
      'label' => 'Another Line Item',
      'html_type' => 'Text',
    ]);

    $this->callAPISuccess('price_field_value', 'create', [
      'price_set_id' => $priceSetID,
      'price_field_id' => $priceField['id'],
      'label' => 'Another Line Item',
      'financial_type_id' => $financialTypeId,
      'amount' => '1',
    ]);
    $priceFieldId = $priceField['id'];

    // Set quantity for our test
    $submitParams['price_' . $priceFieldId] = 100;

    // contribution_page submit requires amount and tax_amount - and that's ok we're not testing that - we're testing at the LineItem level
    $submitParams['amount'] = $this->formatMoneyInput(180 * 16.95 + 100 * 1);
    // This is the correct Tax Amount - use it later to compare to what the CiviCRM Core came up with at the LineItem level
    $submitParams['tax_amount'] = $this->formatMoneyInput(180 * 16.95 * 0.10 + 100 * 1 * 0.10);

    $this->callAPISuccess('contribution_page', 'submit', $submitParams);
    $contribution = $this->callAPISuccessGetSingle('contribution', [
      'contribution_page_id' => $this->_ids['contribution_page'],
    ]);

    // Retrieve the lineItem that belongs to the Printing Rights and check the tax_amount CiviCRM Core calculated for it
    $lineItem1 = $this->callAPISuccessGetSingle('LineItem', [
      'contribution_id' => $contribution['id'],
      'label' => 'Printing Rights',
    ]);

    // Retrieve the lineItem that belongs to the Another Line Item and check the tax_amount CiviCRM Core calculated for it
    $lineItem2 = $this->callAPISuccessGetSingle('LineItem', [
      'contribution_id' => $contribution['id'],
      'label' => 'Another Line Item',
    ]);

    $lineItem_TaxAmount = round($lineItem1['tax_amount'] + $lineItem2['tax_amount'], 2);

    $finalContribution = $this->callAPISuccess('Contribution', 'getsingle', ['id' => $contribution['id']]);

    $this->assertEquals($lineItem1['line_total'] + $lineItem2['line_total'], $contribution['total_amount'], 'Contribution total should match line item totals');
    $this->assertEquals($lineItem_TaxAmount, round(180 * 16.95 * 0.10 + 100 * 1 * 0.10, 2), 'Wrong Sales Tax Amount is calculated and stored.');

  }

if you noticed I put in as an extra param return => ['tax_amount'] onto the getsingle api call

Ah ok let me try that!

And there it is!

But the $315 make no sense: a) is not reproducing the bug that's being reported (is only recording the Tax on the last item -> so that would be $10 not $315 b) total tax should be $315.10 (lineItem 1 tax = $305.10 and lineItem 2 tax = $10);

Ok I can get the two new assertions to work now [changed the $thousandSeparator], however they all pass and they should be failing.

$this->assertEquals($lineItem1['line_total'] + $lineItem2['line_total'], round(180 * 16.95 + 110 * 2.95, 2), 'Line Item Total is incorrect.');
$this->assertEquals($lineItem1['line_total'] + $lineItem2['line_total'], $finalContribution['total_amount'], 'Contribution total should match line item totals');

$this->assertEquals(round($lineItem1['tax_amount'] + $lineItem2['tax_amount'], 2), round(180 * 16.95 * 0.10 + 110 * 2.95 * 0.10, 2), 'Wrong Sales Tax Amount is calculated and stored.');
$this->assertEquals(round($lineItem1['tax_amount'] + $lineItem2['tax_amount'], 2), $finalContribution['tax_amount'], 'Sales Tax Amount on Contribution does not match sum of Sales Tax Amount across the Line Items.');

I've put up https://github.com/civicrm/civicrm-core/pull/18290 - thanks to @pradeep & @KarinG this was very easy to hone in on and my version is substantatively the same as @pradeep's - it just removes the weird construct of expecting a function called 'getLineItem' to also manage a running total

mentioned in commit d09ec772

mentioned in commit dfdee387

changed milestone to %5.28.4

closed

mentioned in commit 54f3de04

Total Tax Amount on the Contribution (generated in backoffice/offline) no longer adds up to sum of the Tax Amount for the individual line items

Child items ...

Activity