Sylius: Promotion integrity error in total price of items from taxon

[ping86]

Sylius version affected: 1.1.6

Description
Promotion integrity error when Sylius tries to choose between 2 promotions applyable in the same order

Steps to reproduce

Create 2 Promotions:

First promotion:
Rule1: At least 2 elements in cart
Rule2: At least 50€ in Taxon ABC

Action: 20% Discount in product of Taxon ABC
Promotion is exclusive.
Priority: 1

Second Promotion:
Rule1: At last 2 elements in cart
Rule2: At least 80€ in Taxon ABC

Action: 40% Discount in product of Taxon ABC
Promotion is exclusive.
Priority 2

If you insert 85€ in cart without discount, in review cart page you see that sylius apply promotion to cart and the total cart is 68€ (-20%).

When you want complete the payment, sylius now check the cart rules with the total amount and then launch an error with message:

promotion_integrity: 'You are no longer eligible for this promotion %promotionName%.'

Possible Solution

[pamil]

It sounds like a more advanced case of #9512.

[Chrysweel]

I think that in an ecommerce that claims to be stable and wants to face big projects like Prestashop or Magento, any error of this kind in sales promotions should be a mayor priority, don't you think?

The issue relational #9512 has been created 28 days ago ... :(

[jdeveloper]

I was thinking about the problem. The problem relies that when we add an item to the cart, sylius checks if promotions can be applied and recalculates the the prices with the adjustments applied. So far good.

The problem appears on the complete order process when we verify that promotions are consistent, witch by the way must be done. And the problem is that it takes the total of items with the adjustments applied, therefore, the conditions are not the same as when we applied the promotions. I think a workaround to this problem is to store the promotions in a array, remove them from the cart, recalculate the totals and then loop through each promotion checking it and adding it back to the cart and apply the recalculation.

I don't promise nothing but I will try to implement it.

I think this solution covers also ticker #9512

[pamil]

@jdeveloper exactly, I replied with a similar solution in #9512.

[jdeveloper]

@pamil I'm half done with the solution, just have to fix phpspec, but it seems to work

I think by tomorrow we have the workaround.

[jdeveloper]

@pamil finally a pushed the solution.

I took the behat tests described in #9512. If you think this is not enough or needs more cases just comment it please, it would be a pleasure to finish this issue.

By the way, sorry I haven't read your solution proposal. I haven't done the DTOs and the service. Instead I just injected the PromotionApplicator and on removing I reverted and as I checked if a promotion was eligible I added it back to the cart by apply to update the state.

[pamil]

@jdeveloper at the first sight it looks better than my solution (as it's a continuous way to make it work properly), good job!

[jdeveloper]

@pamil I just pushe it again, I realized I don't need to call cart->addPromotion since the apply method does it.

[Chrysweel]

Great @jdeveloper !! Goodjob ! Fix this error is very necessary :+1: