Sylius: Removing promotion adjustment never recalculate the tax on the order item unit

[Prometee]

Sylius version affected: 1.4.1

Steps to reproduce

Into the cart : 1 article at $149 (without taxes)
Applied promotion : 50% off (order_promotion)
Tax 14,975% (Canadian tax)
Total : $85.66

When I want to complete the order checkout I have this error :
Your order total has been changed, check your order information and confirm it again.

Description

This error occurred because OrderPromotionIntegrityChecker remove the promotion to check it, but the tax adjustment on the OrderItemUnit is never updated when the promotion adjustment is removed.

[oallain]

Hello,

I also think there is a bug.

I test with your values, like you can see.

IMO total tax must be $11.16 but it is $10.43 actualy.
I wrote Behat test into features/checkout/order_promotion/order_promotion_integrity_validation.feature like this :

    @ui
    Scenario: Removing promotion adjustment recalculate the tax on the order item unit
        Given this promotion gives "50%" discount to every order
        And default tax zone is "US"
        And the store has "VAT" tax rate of 14,975% for "Clothes" within the "US" zone
        And I added product "PHP T-Shirt" to the cart
        And this product's price is "$149.00"
        And it belongs to "Clothes" tax category
        When I proceed selecting "Offline" payment method
        Then my tax total should be "$11.16"
        And my order total should be "$85.66"

I have an fail, but I do not know how to fix it :(

Someone, can help us ?

[jeroendk]

Go the exact same problem, any workaround or fix for this issue?
Non-percentage discounts work fine...

[Prometee]

The quick workaround for me is to decorate : sylius.listener.order_promotion_integrity_checker and silent the public function check(ResourceControllerEvent $event): void method.

[jeroendk]

@Prometee So no integrity check... Fine for now thanks!

[Prometee]

It's quick ... but really not safe :(

[vvasiloi]

I think I finally found the issue.

Given:

• $10 price
• 20% tax excl.
• 50% discount

Here's what's happening:

1. the promotion processor deducts the discount amount from the price without taxes
   
   
   
   • 50% of $10 = $5
   
   
2. the taxes processor calculates the tax amount from the discounted price
   
   
   
   • 20% of $5 = $1
   
   
   • at this point the order total is $6 ($5 + $1 tax)
   
   
3. the order promotion integrity checker reverts and reapply the promotion
   
   
   
   • reverting removes the $5 adjustment, so the order total is now $11 ($10 + $1 tax)
   
   
   • reapplying now deducts the amount from $11 instead of $10
   
   
   • now the total is $5.5 instead of $6
   
   
4. order total integrity check fails

[vvasiloi]

A possible solution would be to inject and run the order processor after the promotion integrity checking.

[ofriedrich]

The OrderPromotionIntegrityChecker only removes the promotion adjustments from the order. But you have to remove all adjustments and add them in the right order while checking its integrity.

There is another issue concerning the OrderPromotionIntegrityChecker https://github.com/Sylius/Sylius/issues/9940 which may also have an effect on the calculation. The order in the apply loop in the OrderPromotionIntegrityChecker does not consider the priority.

[vvasiloi]

I think not applying the promotion directly in the integrity checker using the promotion applicator, but running the order processor instead will fix both issues.

Example implementation: https://gist.github.com/vvasiloi/3fbb91130801c7553cab725b5551f901

Don't know how to properly test it though.

[ofriedrich]

I don't think that your solution will do the right thing. The price calculation may be ok in the end, but the promotionEligibilityChecker will start with the wrong prices. See example above: 11$ vs. 10$.

[vvasiloi]

I don't think so.
If you notice, the still eligible promotions are not reapplied in the checker, but just re-added to the order and then the order processor is run.

[pamil]

Should be fixed by #10603.