#2897190: Tax calculations do not take discounts (promotions) into account fixed the interaction between taxes and discounts for cases where prices are both entered and displayed with/without taxes.

However, if the prices are entered with tax but displayed without (also including tax-exempt prices), or vice-versa, the discounts are incorrect.
This is because LocalTaxTypeBase / TaxOrderProcessor modify the unit price (which modifies the total price) after the discounts were already calculated. So a 10% discount on 12 eur gets calculated and added as 1.2, then the price gets lowered to 10 eur. The discount is no longer correct, it is too large.

This means we probably need to process taxes in two phases:
1) Resolve rates, figure out if unit price needs to be modified, modify it
2) Calculate discounts
3) Now calculate taxes based on the resolved rates

CommentFileSizeAuthor
#61 commerce-tax_processor-2982355-61.patch29.62 KBcslevy
#60 commerce-tax_processor-2982355-60.patch29.62 KBcslevy
#59 commerce-tax_processor-2982355-59.patch29.62 KBcslevy
#58 commerce-tax_processor-2982355-58-on_top_of-2881056-MR371.patch29.41 KBmatthiasm11
#58 commerce-tax_processor-2982355-58.patch29.28 KBmatthiasm11
#57 myscreen.png33.92 KBjohaziel
#57 tax_processor-2982355-tax-module-only.patch19.18 KBjohaziel
#55 tax_processor-2982355-55-round-down.patch30.99 KBcslevy
#55 tax_processor-2982355-55-round-up.patch30.97 KBcslevy
#54 tax_processor-2982355-54.patch31.07 KBaarnau
#49 promo.png24.37 KBtrickfun
#47 interdiff_2982355-45-47.txt3.49 KBrossb89
#47 tax_processor-2982355-47.patch39.39 KBrossb89
#45 tax_processor-2982355-45.patch39.03 KBmatthiasm11
#43 intra_community.JPG22.95 KBtimlie
#43 tax_with_promotion.JPG21.75 KBtimlie
#41 tax_processor-2982355-41.patch38.91 KBmatthiasm11
#40 tax_processor-2982355-40.patch38.88 KBmatthiasm11
#38 tax_processor-2982355-38.patch30.01 KBmatthiasm11
#32 tax_processor-2982355-32.patch29.15 KBmatthiasm11
#30 with-patch.png25.39 KBmistrae
#30 without-patch.png24.23 KBmistrae
#28 tax_processor-2982355-28.patch28.13 KBmatthiasm11
#27 tax_processor-2982355-27.patch28.21 KBmatthiasm11
#24 tax_processor-2982355-24.patch23.44 KBmatthiasm11
#23 tax_processor-2982355-23.patch24.73 KBmatthiasm11
#22 tax_processor-2982355-22.patch24.58 KBmatthiasm11
#21 tax_processor-2982355-21.patch14.38 KBmatthiasm11
#14 correct-tax-on-adjustments-2982355-14.patch5.21 KBmattjones86
#3 localtaxtypebase_price_tax_exc_display_inc.patch3.03 KBmaurizio.ganovelli
#12 Screen Shot 2018-07-30 at 13.11.09.png20.23 KBmattjones86

Issue fork commerce-2982355

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

bojanz created an issue. See original summary.

bojanz’s picture

Issue summary: View changes
maurizio.ganovelli’s picture

Hi bojanz,

after updating to latest stable version of commerce, i've found that in cart, order summary and orders, prices are totally wrong. In my specific case prices are inserted taxes excluded (only Vat is configured) but displayed tax included. Tracking down the problem, i've found that the price alteration done in LocalTaxTypeBase, re-add Vat (also when no promotion is active in order) to every order item. Moreover when adding a coupon, also discounts are wrong.

I've developed a quick (and surely dirty) workaround that seems to work, at least for my case. I try to recalculate prices without changing unit price.

I hope would be a starting point for who got same problem.

Suggestions and corrections are welcome!

Thank you.

bojanz’s picture

@maurizio.ganovelli
Please give us a test case, at least in comment form (entered price, configured discount, expected price, shown price).

maurizio.ganovelli’s picture

Thank you for quick response.

While preparing some test data, i found that after deleting the cart order from backend and restarting checkout
process, VAT is not more added twice in order items (in the example above, item total was 66.61 instead of 54.60).
Also cannot get patch provided above to working correctly on new carts and orders, so don't take in account.

I get also a correct discount amount. The only thing that goes wrong is tax amount on totals.
See the following example.

Configuration:

Taxes:
VAT: European Union VAT, Display taxes of this type inclusive in product prices. (TRUE)

Promotions:
Discount: 20% off on order total

Store:
Prices are entered with taxes included. (FALSE)

Test product price entered by user:
44.75 EUR

Store/Profile country:
Italy

Expected results in order summary during checkout:

Item total: 54.60 EUR
Subtotal: 54.60 EUR
Discount: -8.95 EUR
VAT: 7.88 EUR
Total: 43.68 EUR

Current result:

Item total: 54.60 EUR
Subtotal: 54.60 EUR
Discount: -8.95 EUR
VAT: 8.23 EUR
Total: 45.65 EUR

Thank you.

maurizio.ganovelli’s picture

Hi bojanz, i made some tests on a vanilla install (Drupal 8.5.5 + Commerce 8.x-2.8) reproducing same conditions and configurations.

I got same results as above.

The issue seems related to how taxes are applied in LocalTaxTypeBase for my specific case (price entered vat excluded, displayed vat included).

VAT is first added to unit price:

$unit_price = $unit_price->add($tax_amount);
$order_item->setUnitPrice($unit_price);

then tax amount is determined:

$adjusted_total_price = $order_item->getAdjustedTotalPrice(['promotion', 'fee']);
$tax_amount = $percentage->calculateTaxAmount($adjusted_total_price, $this->isDisplayInclusive());

So adjustments are calculated on VAT included prices, leading to a wrong tax amount.

I'm wrong?

maurizio.ganovelli’s picture

For the sake of completeness, if i try to change configuration using "Prices are entered with taxes included." and changing product variation price to 54.60 EUR (44.75 + 22%), calculated discount seems too big (-10.92 EUR).

bojanz’s picture

We have extensive tests that confirm that discounts and taxes are correct when prices are both entered and displayed with VAT included.
You are free to check them with a calculator.

The logic that we're using for prices entered without tax, but shown with tax is:
1) Take the original unit price, and determine which tax it has by default. Add that tax to the unit price, making it tax-inclusive.
2) Taking the tax inclusive price as a base, calculate the included tax.

Using your numbers, with a 22% tax rate:
1) 44.75eur price + 9.85eur tax (price * 0.22 rounded up) = 54.60 eur (new price)
2) (54.60 * 0.22) / (1 + 0.22) = 9.85eur tax

The problem that is exposed in this issue is that promotions run before #1, using 44.75eur as a base, instead of running between #1 and #2, using 54.60 as a base. That's the part we need to fix, by making the unit price modification run before promotions. I hope to tackle that in August.

maurizio.ganovelli’s picture

Thank you for explanation.

This logic is absolutely correct, so my problem is (at least partially) unrelated.

I need to calculate promotions always on net price (tax excluded) and then calculate taxes.

This is exactly what happens now:

1. promotion amount is correctly calculated (at least for my case) on "net" price (44.75 * 0.20 = 8.95)

2. unit price is then updated adding tax amount (44.75 * 1.22 = 54.60)

3. order item adjusted price is calculated with getAdjustedTotalPrice(['promotion', 'fee']) (54.60 - 8.95 = 45.65)

4. tax amount is determined with (45.65 * 0.22) / (1 + 0.22) = 8.23

I need something like this:

1. promotion amount is calculated on "net" price (44.75 * 0.20 = 8.95)

2. order item adjusted price is then calculated with getAdjustedTotalPrice(['promotion', 'fee']) (44.75 - 8.95 = 35.80)

3. tax amount is determined with (35.80 * 0.22) = 7.88

4. unit price is then updated adding tax amount

I think that i must implement a custom solution.

Thank you for your time and help!

bojanz’s picture

@maurizio.ganovelli
Yeah, that sounds highly irregular. I suggest checking with your accountant, cause I'd never expect prices to be shown with tax in such a scenario.

maurizio.ganovelli’s picture

I can confirm that these are the project requirements requested by committent. Anyway is not the first time i found this kind of tax/discount configuration in B2C sites. Instead, in B2B sites prices are often shown tax excluded. In both cases discounts are almost always calculated from net price.

Thanks again and see you soon.

mattjones86’s picture

StatusFileSize
new20.23 KB

Just ran into this myself, my prices are entered excluding VAT and shown including VAT.

A 10% discount produces the following:

Incorrect discount

I would expect the calculated discount to be £42.94

I'm going to take a look and see if I can produce a patch for this.

maurizio.ganovelli’s picture

@orphans supposing a VAT rate of 20% is this what you want to get?

subtotal vat inc: 429.40

subtotal vat exc: 357.83
discount (vat exc): -35.78
vat: 64.41
total: 386.46

if yes, i think we have a similar need.

mattjones86’s picture

Yes, that's exactly what I get.

This is my patch which corrects the issue, it's certainly not the ideal way to resolve this but it works for me until the official solution is released.

Note that I've also bundled the patch for #2874158: Support taxing the shipping cost into this, since it really seems to be the same issue with adjustments not being taxable.

mattjones86’s picture

Component: User experience » Tax
Related issues: +#2897190: Tax calculations do not take discounts (promotions) into account
bojanz’s picture

The issue summary is still correct:

This means we probably need to process taxes in two phases:
1) Resolve rates, figure out if unit price needs to be modified, modify it
2) Calculate discounts
3) Now calculate taxes based on the resolved rates

We need an EarlyTaxOrderProcessor and a LateTaxOrderProcessor.
The EarlyTaxOrderProcessor needs to be the only one modifying unit prices (no more alterations in LocalTaxTypeBase).
That's where #2874167: Handle the prices-include-tax and Intra-Community-Supply scenario would be resolved as well.
This will also help us with #3026301: Allow order items to show the full (unreduced) unit price.

Note that modifying the unit price before discounts are applied means that percentage discounts will now correct, but fixed amount discounts won't. That means that as a second step we'll need to introduce a helper for getting the amount without tax before it is applied (so that 6eur becomes 5eur, without a 20% store VAT, for example), and then use it from promotions. This might prove to be tricky dependency-wise.

trebormc’s picture

The discount calculation still doesn't work well when it's configured:
- prices are entered without tax
- prices are shown with taxes included

I have been looking at the code, and in my specific case I have fixed it by removing a 1 line of code.

In the file:
modules/tax/src/Plugin/Commerce/TaxType/LocalTaxTypeBase.php

          elseif (!$prices_include_tax && $this->isDisplayInclusive()) {
            $unit_price = $unit_price->add($tax_amount);

            // In this case we also need to add VAT to any promotion adjustments
            foreach ($order_item->getAdjustments() as &$adjustment) {
              if ($adjustment->getType() == 'promotion') {
                $promotion_amount = $adjustment->getAmount();
                $tax_amount = $percentage->calculateTaxAmount($promotion_amount, $prices_include_tax);
                $tax_amount = $this->rounder->round($tax_amount);

                $new_adjustment = new Adjustment([
                    'amount' => $promotion_amount->add($tax_amount),
//                    'included' => $prices_include_tax,
                  ] + $adjustment->toArray());

                $order_item->removeAdjustment($adjustment);
                $order_item->addAdjustment($new_adjustment);
              }
            }
          }
          $order_item->setUnitPrice($unit_price);

The problem is "'included' => $prices_include_tax"

We are overwriting the promotion adjustment options .... why?

From what I understand we can have promotions of order or product, and some are included and others should not be included in the price.

In my case, it seems to work, but I don't know if I'm altering the expected behavior, or if there's something I don't understand.

I don't upload a patch because I'm not sure this is a solution, but in my case it works.

lexsoft’s picture

I'm also having an issue with Discounts.

My options are as follows:

Display taxes inclusive + Prices are entered with taxes included. The tax system used is called Quaderno and it's using the RemoteTaxTypeBase plugin.
19% tax
15% discount

On checkout review, the following is displayed:
Subtotal £150.00
Discount -£22.50
TVA £20.36
Total £127.50

The discount is applied on the full price including tax, where I would need to apply it on the product variation item-unit price - tax = 126.05 where the discount would be -£18.9075.

Any idea how can I achieve this? I've also tried the option where

Display taxes NOT inclusive + Prices are entered with taxes included
On checkout review, the following is displayed (Order total price £150.00 including VAT):
Subtotal £129.64
Discount -£22.50
TVA £20.36
Total £127.50

The discount is applied again on full-price and before tax is calculated where it should be 0.15 x 126.05 = 18.9075 instead of 22.50.

The expected result should be:

Subtotal £150.00
Discount -£18.90
TVA £20.36
Total £127.50

Or

Subtotal £129.64
Discount -£18.90
TVA £20.36
Total £127.50

JeremyFrench’s picture

The patch in #14 works for me in that it calculates the total correctly, however, the display on checkout pages and emails is still wrong.

I've made some code to recalculate all of this but it's not really solving the issue it just allows me to display something correct to the customer.

timlie’s picture

It's clear to me now why these tax calculations are wrong.
I follow bojanz his idea to start with an EarlyTaxOrderProcessor to determine if the unit prices need to be modfied (which is only the case if prices are entered tax inclusive, displayed tax exclusive, or vice-versa).
So some of the code from LocalTaxTypeBase should be moved into the EarlyTaxOrderProcessor. I am only not sure where the resolving of the rates needs to be done. Should this be some helper class?

matthiasm11’s picture

Assigned: Unassigned » matthiasm11
Status: Active » Needs work
Related issues: +#2874167: Handle the prices-include-tax and Intra-Community-Supply scenario
StatusFileSize
new14.38 KB

Work in progress.

1) Resolve rates, figure out if unit price needs to be modified, modify it.
=> The EarlyTaxOrderProcessor calculates the unit price without taxes.

2) Calculate discounts
=> Percentage discounts are working, but fixed amount discounts are not. Will need to create a service which converts the discount amount to a discount amount excluding taxes, depending on the order item it is applied to.

3) Now calculate taxes based on the resolved rates
=> LateTaxOrderProcessor adds the taxes. LocalTaxTypeBase will add the taxes back into the unit price if the price should be displayed tax inclusive.

TODO

matthiasm11’s picture

Assigned: matthiasm11 » Unassigned
Status: Needs work » Needs review
Related issues: +#3139183: Tax is adding twice
StatusFileSize
new24.58 KB

1) Resolve rates, figure out if unit price needs to be modified, modify it.
=> The EarlyTaxOrderProcessor calculates the unit price without taxes.

2) Calculate discounts
=> Percentage discounts are working out of the box. Fixed amount discounts are now calculated excluding taxes with the helper service TaxCalculator.

3) Now calculate taxes based on the resolved rates
=> LateTaxOrderProcessor adds the taxes. LocalTaxTypeBase will add the taxes back into the unit price if the price should be displayed tax inclusive.

Remarks
I think I've managed to solve everything Bojanz mentioned in this ticket.

  • The patch applies to commerce 2.27.0.
  • Tested on a store with the prices entered and displayed including taxes.
  • I didn't test it on a store with prices entered/displayed excluding taxes, but the logic should apply there too.
  • Used the suggestion of Clive in https://drupal.stackexchange.com/questions/273178/dependency-injection-i... to conditionally add the TaxCalculator service.
  • Also added some code to make Intra-Community-Supply taxes work, so #2874167: Handle the prices-include-tax and Intra-Community-Supply scenario can probably be closed.
  • #3139183: Tax is adding twice can probably be closed too.
  • Custom/contrib modules using/extending the old TaxOrderProcessor service should update their code to use/extend EarlyTaxOrderProcessor and/or LateTaxOrderProcessor.
  • Custom/contrib modules extending a promotion with fixed amount should update their code to make sure they calculate with a discount amount excluding taxes. Examples can be found in OrderFixedAmountOff and OrderItemFixedAmountOff.
matthiasm11’s picture

StatusFileSize
new24.73 KB

Altered the patch to apply against 8.x-2.x-dev so it can be tested.

matthiasm11’s picture

StatusFileSize
new23.44 KB

Cleaned patch to remove unwanted changes. Sorry for that!

matthiasm11’s picture

I've no idea why the test fails on composer. Nothing regarding composer dependencies has been changed in this patch...

longwave’s picture

The Composer 1.x endpoint appears to be having some issues today, as other people are reporting similar 404s in #drupal-infrastructure.

matthiasm11’s picture

StatusFileSize
new28.21 KB

Fixed inheriting constructors of PromotionOfferBase.

matthiasm11’s picture

StatusFileSize
new28.13 KB

The tests are running with PHP 7.1 apparently. Removed variable type hinting since this is only supported as of PHP 7.4.

Status: Needs review » Needs work

The last submitted patch, 28: tax_processor-2982355-28.patch, failed testing. View results

mistrae’s picture

StatusFileSize
new24.23 KB
new25.39 KB

This patch cause bugs with my carts.

I have prices entered without taxes but displayed with taxes.
Base price of this example is 23.21CHF and taxe is 7.7%

Without patch :
25 x 6 = 150CHF

23.21 + 7.7% = 24.9972 (rounded to 25)
25 x 6 = 150CHF

With patch :
25 x 6 = 203.58CHF

23.21 + 7.7% = 24.9972 (rounded to 25)
25 x 6 = 150CHF
150 - 6 x 23.21 = 10.74 (total taxes)
5 x 10.74 = 53.7
150 + 53.7 = 203.7CHF (displayed 203.58 because my calculation is probably wrong somewhere)

See screenshots with and without patch.

So with the patch, the first product add its taxes right, but every other same product add somehow the total taxes to its price.

This has been tested on a clean drupal and the behavior is the same.
Taxes is Swiss tax and checkbox "Display taxes of this type inclusive in product prices" is checked.

timlie’s picture

Hi @matthiasm11 thanks for your hard work on this issue.
I tried you're patch.

In LocalTaxTypeBase I changed the code to this:

// Include the calculated tax back in the unit price if needed.
if ($this->isDisplayInclusive()) {
  $unit_price = $order_item->getUnitPrice();
  if ($unit_price instanceof Price) {
    $tax_amount_unit_price = $percentage->calculateTaxAmount($unit_price);
    $unit_price = $unit_price->add($tax_amount_unit_price);
    $order_item->setUnitPrice($unit_price, $order_item->isUnitPriceOverridden());
  }
}

Otherwise the wrong tax amount is added.

matthiasm11’s picture

Status: Needs work » Needs review
StatusFileSize
new29.15 KB

Thank you for testing the patch @Mistrae. The project I'm working on does not allow to combine multiple products in one order item. The suggestion from @timlie should indeed fix this.

I've rerolled the patch to apply to the current 8.x-2.x-dev branch and added the code of @timlie.
@Mistrae, can you please verify if this fixes your use case?

Status: Needs review » Needs work

The last submitted patch, 32: tax_processor-2982355-32.patch, failed testing. View results

timlie’s picture

Hi @matthiasm11,

Thanks for adding fix to your patch.

After a lot of debugging I think we need to swap what is happening in EarlyTaxOrderProcessor and LateTaxOrderProcessor.

Like mentioned by @bojanz in comment #16 I think we need to first resolve rates in EarlyTaxOrderProcessor and adjust the unit price here if necessary (add tax when it should be included, ...)

In the LateTaxOrderProcessor we should only calculate tax adjustment.

Maybe we should create an issue fork?
Thanks!

timlie’s picture

I should clarify a bit where this still goes wrong.

When prices are entered without tax but shown with, this is what is happening with patch #32:

1) EarlyTaxProcessor checks if the order items unit price should be lowered with the store's tax rate to get the unit price exclusive tax. In the case of this example, this is not needed as prices are entered without tax so unit price stays untouched.
The EarlyTaxProcessor makes the unit price always tax exclusive.

2) Promotions are applied in a next phase, the code in patch #32 checks for the fixed amount promotions and makes the fixed amount tax exclusive. The promotion adjustment is added with this tax exclusive amount.

3) LateTaxOrderProcessor applies all tax types. The patch changed the code in LocalTaxTypeBase to calculate the tax amount and to change the unit price to tax inclusive if requested (which is the case in this example).

What still goes wrong is the display of the promotion amount on cart or checkout summary. Promotion adjustments are added tax exclusive (both percentage as fixed amounts). They will be shown (if the promotions are not included in the price) without tax which makes the order total amount wrong and although the tax amount is calculated correct, this is wrong for this order total amount.

mistrae’s picture

Hi @matthiasm11,

Thank you for your patch, it seems to work pretty well.

Now with the same example ( a product at 23.21CHF ) this is what I can see :

With one product :
Base price 23.21 + 7.7% : 25CHF

With 6 products :
Base price 23.21 * 6 + 7.7% : 149.98CHF

Is this the excepted behavior ?
Is there a missing round somewhere ?
I think the total price should display as 150CHF as the "one product" is rounded to 25CHF

matthiasm11’s picture

@Mistrae
From #21 in #2567089: Round up summed tax price components when calculating the order total: D8 calculates taxes per-line.
And from #5 in #3044185: Rounding can cause VAT in orders to be higher than expected: D8 taxes are calculated by applying the percentage to the [row] total price, then rounding to get the tax amount. This is known as per-line calculation in accounting. It is also the more common way of calculating VAT. [...] Per-line has the benefit of being much more precise when a large quantity of products is being ordered (a thousand, for example).

Which means (23.21 * 6) + 7.7% : 149.98CHF is the correct calculation. (per-line calculation)

matthiasm11’s picture

Status: Needs work » Needs review
StatusFileSize
new30.01 KB

Altered some calculations in an attempt to fix the test. This results in a total of 150CHF, so the expected result is the same as before the patch.
Made the code more performant too.

@Mistrae can you please verify if your case is solved?

Still need to look into the promotion display in the cart mentioned by @timlie.

Status: Needs review » Needs work

The last submitted patch, 38: tax_processor-2982355-38.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

matthiasm11’s picture

StatusFileSize
new38.88 KB

Logic
Fixed promotions/fees when not included in the unit price. Beware, there may be a rounding difference depending on the discount included or excluded in the unit price. Therefore I added the following description to the discount form:
Due to rounding, this can result in a slightly different order total when using taxes.

An example with entering tax exclusive, displaying tax inclusive.
A unit price (excl tax) of 23.21 CHF and a discount (excl tax) of 1 CHF, ordering a quantity of 6:
- Include the discount in the displayed unit price:

23.21 - 1 = 22.21 * 6 = 133.26 CHF // The discount is added in the unit price.
133.26 * 0.077 (7.7%) = 10.26 CHF
133.26 + 10.26 = 143.52 CHF // This is the order line total (and since this example only has one order item, this is the order total too)

- Only show the discount on the order total summary:

round(23.21 * 1.077) * 6 = 150 CHF // No discount in the unit price. This is the order line total.
1 * 6 * 1.077 = 6.46 CHF
150 - 6.46 CHF = 143.54 CHF // This is the order total.

If we undo the rounding introduced in #38, the above problem will be fixed, but we will end up with 149.98 CHF again when there is no discount. So it's one or the other. I believe we should round on the order line total level, according to this code:

 * if $order_item->usesLegacyAdjustments() is true, the order item adjustments
 * were calculated based on the order item unit price, which was the default
 * logic prior to Commerce 2.8, changed in #2980713.

Can someone (@bojanz?) shine a light on the way to go?

Tests
Since LocalTaxTypeBase.php now expects all unit prices to be tax exclusive, the tests on tax inclusive prices fail.
I've cleaned up those tests. Also added a "non-tax-inclusive prices + non-display-inclusive taxes."-test, since that was the only one of the 4 possiblilities missing.

- Entered tax exclusive - Displayed tax exclusive: 10.33 unit price + 2.07 taxes => Test covered.
- Entered tax exclusive - Displayed tax inclusive: 12.40 unit price (including 2.07 taxes) => Test covered.
- Entered tax inclusive - Displayed tax exclusive: 8.61 unit price + 1.72 taxes => No test yet.
- Entered tax inclusive - Displayed tax inclusive: 10.33 unit price (including 1.72 taxes) => No test yet.

Did the same for testDiscountedPrices(), since LocalTaxTypeBase.php now expects all unit prices to be tax exclusive before the promotion/fee adjusted total is calculated.

To be absolutely sure, we could create a Drupal\Tests\commerce_tax\Kernel\TaxTest.php, which covers those not covered yet.

Also fixed some coding standards and comments.

matthiasm11’s picture

Status: Needs work » Needs review
StatusFileSize
new38.91 KB

This ticket was a difficult one.

Since it would complicate the logic too much (would require a whole different approach), I kept the code with the message Due to rounding, this can result in a slightly different order total when using taxes..

Verified the automated tests, any errors in ProductLayoutBuilderIntegrationTest.php are due to a Drupal core regression, not a Commerce regression. More information in comment #12 of #3253323: Missing block's section tag after price calculation and #79 of #3020876: Contextual links of reusable content blocks are not displayed when rendering entities built via Layout Builder.

Also did some manually testing in a clients project: prices entered tax included, displayed tax included. Tested with products with different tax rates in one cart. Tested the editing of orders in the admin backend. Also tested with an intra-EU-VAT billing address. Al seems to work fine.

Did some code clean up too.

This should fix the issue in my opinion. I would appreciate it if someone (preferable more than one person) could test those changes too, because taxes are quite important.

Status: Needs review » Needs work

The last submitted patch, 41: tax_processor-2982355-41.patch, failed testing. View results

timlie’s picture

StatusFileSize
new21.75 KB
new22.95 KB

Hi matthiasm11,

Thanks for great work on this issue.

I tested you're patch with "prices entered tax exclusive" and "displayed tax inclusive".

Promotions are now tax aware and tax calculations are correct.

This screenshot shows order with percentage promotion added:

tax

This screenshot shows the intra community vat is now also respected:

prom

I'll review you're code in the following days and will test with other cases like mine here (prices entered tax exlusive, displayed inclusive) as well.

This looks very promising.

matthiasm11’s picture

Status: Needs work » Needs review
matthiasm11’s picture

StatusFileSize
new39.03 KB

Improved the precision of the early tax order calculation.
This was needed when the prices are entered tax inclusive and displayed inclusive.

Example: unit price of €170 including 21% VAT. Without patch that would give €140.50 excl VAT (rounded) in the EarlyTaxOrderProccessor. €140.50 * 0.21 VAT = €29,51 (rounded). Unit price displaying tax inclusive: €140.50 + €29.51 = €170.01; which is not correct.
With the patch that would be €140,495867 + €29.50 (rounded) = €170 (rounded); which is correct.

rossb89’s picture

Status: Needs review » Needs work

Thank you for the excellent work here matthiasm11!

I originally came to this issue as I was testing promotions with a % off the order subtotal, and found issues with the default code - having product prices entered without tax, and then displaying tax inclusive.

Your latest patch #45 works brilliantly for '% off' promotions. After applying the patch and testing out various % level discounts, it resolved any issues I was seeing when it came to the correct amount of tax etc.

However, I have noticed today that when testing discounts of type 'Fixed amount off' (the order) the patch is falling down slightly here IF the store does not have the prices_include_tax option enabled.

The problem i'm seeing is this; Although the EarlyTaxOrderProcessor is correctly stripping taxes from the order item (if any are present), when
$amount = $this->taxCalculator->getUnitPriceExcludingTaxes($order_item, $amount);
is being called from e.g. OrderFixedAmountOff, the $amount being passed in from the promotion is inc tax (e.g. I have set my promotion to minus £9 from the order subtotal) and because my store is set to NOT have prices including tax enabled, the logic block inside of TaxCalculator:

    if ($store->get('prices_include_tax')->value) {
      // If Intra-Community-Supply tax has been applied, the price is already excluding taxes.
      $adjustments = $order_item->getAdjustments(['tax']);
      foreach ($adjustments as $adjustment) {
        if ($this->isIntraCommunityAdjustment($adjustment)) {
          return $unit_price;
        }
      }

      // Determine the tax amount and set the unit price without taxes.
      $calculation_date = $order->getCalculationDate();

      $rates = $this->storeTax->getDefaultRates($store, $order_item);
      foreach ($rates as $rate) {
        $percentage = $rate->getPercentage($calculation_date);
        if ($percentage instanceof TaxRatePercentage) {
          $tax_amount = $percentage->calculateTaxAmount($unit_price, TRUE);
          // Not rounding the tax amount on purpose, so the unit price becomes as specific as possible.
          // The unit price and taxes are being rounded in LocalTaxTypeBase.php.
          $unit_price = $unit_price->subtract($tax_amount);
        }
      }
    }

never gets ran, and thus the fixed $amount of discount - £9 - does NOT get the tax taken off.

When the LateTaxOrderProcessor then runs, it will apply any tax types to the order, and thus the -£9 discount now becomes -£10.80 because the 20% VAT has been applied and the customer ends up getting more money off than they should have.

An obvious 'workaround' to this would be to enter fixed amount discounts excluding tax but that doesn't feel quite right to me. As there isn't a way (that i've seen!?) in the promotion creation to specify whether the promotion fixed amount off is inc/ex tax, I think we need to assume that it is inc tax and thus needs to be handled correctly for the instances where the store prices_include_tax is FALSE.

rossb89’s picture

StatusFileSize
new39.39 KB
new3.49 KB

Attached is a patch that adds a third parameter to getUnitPriceExcludingTaxes() which is a flag specifying if the unit_price is inclusive of tax or not. Set to false by default and only set to true in calls to getUnitPriceExcludingTaxes() in OrderFixedAmountOff and OrderItemFixedAmountOff where we know the amount (off) from the promotion is inc tax.

This leaves the other call to getUnitPriceExcludingTaxes() from EarlyTaxOrderProcessor alone, to only use the prices_include_tax store setting, as the flag will be false by default.

This change now lets the 'fixed amount off' promotion discounts work correctly in addition to the 'percentage amount off' style promotions.

rossb89’s picture

Status: Needs work » Needs review
trickfun’s picture

StatusFileSize
new24.37 KB

Patch 47 works with "Percentage off the order subtotal" promotion

thank you

trickfun’s picture

alumni’s picture

Patch #47 works well for me. Nice work !

EDIT : The calculation of the Tax ajusted for a country is now broken. Even if the tax is included in the price, the calculation of the Total is calculated from the tax.

Exemple : Price is 120€ Included

If the customer comes from a 21% VAT country, the total is now 121€.

luksak’s picture

Status: Needs review » Needs work

This needs a re-roll.

The patch has got BC. I have this error, because i have commerce_shipping promotions:

ArgumentCountError: Too few arguments to function Drupal\commerce_promotion\Plugin\Commerce\PromotionOffer\PromotionOfferBase::__construct(), 4 passed in /app/web/modules/contrib/commerce_shipping/src/Plugin/Commerce/PromotionOffer/ShipmentPromotionOfferBase.php on line 53 and exactly 5 expected in Drupal\commerce_promotion\Plugin\Commerce\PromotionOffer\PromotionOfferBase->__construct() (line 52 of modules/contrib/commerce/modules/promotion/src/Plugin/Commerce/PromotionOffer/PromotionOfferBase.php).

Can we achieve this without BC?

The patch fixes the tax and promotion calculations. But since applying it, I see rounding errors.

micka’s picture

Hi,
Do you have a solution for stack : PHP 8.1 - D9.5.9 - Drupal Commerce 8.x-2.36
With a clean install and following parameters :
- Custom Tax config : include in the product price (10%)
- Promotion config : not include in the product price (displayed in total summary) (10%)
- original product variation price : 100$

Tax is not well if we consider it is calculated on without tax price.

VAT : ((priceTTC - actual VAT) - discount) * actual VAT percentage
VAT : ((110 - 9.09) - 10) * 0.1 = 9.09

Subtotal : $110.00
Discount : -$10.00
VAT : $9.09
Total : $100.00

Expected :
VAT : ((priceTTC - old VAT) - discount) * actual VAT percentage
VAT : ((110 - 10) - 10) * 0.1 = 9

Subtotal : $110.00
Discount : -$10.00
VAT : $9
Total : $100.00

Thanks in advance for your answer.
And sorry if i misunderstood this issue (french english ^^).

aarnau’s picture

StatusFileSize
new31.07 KB

New re-roll here

cslevy’s picture

I recreated this patch to work with the latest commerce.

But I also encounter rounding issues, and I think that the discount amounts should be always rounded half down, not up. I'm not sure if it's correct or not, but I created the patch in 2 versions. one as it was in the previous comment with round up, and one with round down.

Please advise on this

anybody’s picture

Version: 8.x-2.x-dev » 3.0.x-dev
johaziel’s picture

Issue summary: View changes
StatusFileSize
new19.18 KB
new33.92 KB

I had the same probleme too but I 'don't use promotion module to manage promotions,
my config :
product price entered without VAT (I didn't find where you manage prices are displayed with or without VAT )
I using also commerce_tax_product
one field_discount added to an order item type

I directly add promotions in module like this

          $promotion = $order_item->get('field_promotion')->value;
          if($promotion > 0){

            // Calculate promotion
            $unit_price = $order_item->getUnitPrice();
            $discount_amount = $unit_price->multiply($order_item->getQuantity())->multiply('-' . strval( $promotion/100)); 

            $adjustment = new Adjustment([
              'type' => 'promotion',
              'label' => t('Discount'),
              'amount' => $discount_amount,
              'percentage' => strval($discount/100),
              'source_id' => 'custom_discount',
              'locked' => TRUE,
            ]);
            $order_item->addAdjustment($adjustment);
          }

So I separated the patch to only be applied to tax module and calculations of price a ok
my price without VAT is 50 €
screen capture

matthiasm11’s picture

Status: Needs work » Needs review
StatusFileSize
new29.28 KB
new29.41 KB

@ #47: I would advise against the mixing of excluding taxes in products and including taxes in promotions at the same time.

A webshop including taxes in products, is often a B2C webshop, where the product price is something rounded (€50 incl. 21% VAT for example). The fixed amount of promotion should thus be € -10 incl. 21% VAT.

A webshop excluding taxes in products, is often a B2B webshop, where both product prices and promotions can be communicated excluding VAT.

Depending on the webshop requirements, one could argue to change this.

At the moment, the tax setting is called "Prices are entered with taxes included.". It does not differentiate between products or promotions. I therefore rerolled patch #45 against 3.0.0-beta2.

I also rerolled patch #45 against 3.0.0-beta2 containing merge request 371 from #2881056: Can't distinguish two taxes of different percentages in the order summary.

IF we allow mixing this, the incl/excluding of VAT in promotions should be configurable, just like the VAT incl/excluded in products. It should fallback to the same setting as the incl/excluded VAT of products in an update hook. The todo based on the attached patch would then be:

  • Rename "Prices are entered with taxes included." to "Product prices are entered with taxes included."
  • Add setting "Promotion prices are entered with taxes included."
  • Alter TaxCalculator.php to use if ($store->get('prices_include_tax')->value || $store->get('promotion_prices_include_tax')->value) {
  • Add an update hook to set promotion_prices_include_tax based on prices_include_tax
cslevy’s picture

StatusFileSize
new29.62 KB

Re-rolled patch from #58

cslevy’s picture

StatusFileSize
new29.62 KB

Fix php 8.4 warning from last patch

cslevy’s picture

StatusFileSize
new29.62 KB

Further php 8.4 fixes

anybody’s picture

Issue tags: +Needs tests

I think this needs tests and should be proceeded in a MR. Only with tests we can ensure this works as expected in the future.

matthijs made their first commit to this issue’s fork.

matthijs’s picture

Version: 3.0.x-dev » 3.x-dev

matthijs’s picture

Created an MR from #61 and added an extra commit to fix the calculation of the tax excluded unit price if it's overridden.

matthijs’s picture

anybody’s picture

Status: Needs review » Needs work

@matthijs thank you very much - I can only see removed or modified tests (which scares me a bit), but no added ones that show the test-only error proving the issue. I think that still needs to be done to be save here?

Are the test modifications correct and required?

matthijs’s picture

@anybody It wasn't my intention to change the patch in the first commit. I see the test changes are also in https://www.drupal.org/files/issues/2025-10-09/commerce-tax_processor-29..., so they got lost earlier? Or am I missing something?

anybody’s picture

@matthijs sorry I didn't mean you personally - just has to be checked and done by someone.

tbkot’s picture

Is there any reason to remove this condition

if (!$tax_type->applies($order)) {
  continue;
}

in LateTaxOrderProcessor? Tests are failing because of this.