solidus
solidus copied to clipboard
Promotion eligibility? ambiguous behavior
On 1.3 (and on):
- Create a coupon promotion that applies a discount to line items
- Add a non-promotionable product to the cart
- Add a promotionable product to the cart
- Apply coupon
Expected behavior: the promotion applies to one line item but not the other Got: the order is ineligible for this coupon
This is being missed due to a logical discrepancy in the spec:
there are two test cases in "eligible?" that aren't both possible:
"and at least one item is non-promotionable" should be return false
"and at least one item is promotionable" should return true
What should the case where one is promotionable and is isn't? Both cases can't be right.
I'm guessing this arises from having both Order and LineItem level promotion actions, so I'm not sure how to resolve. Looking for guidance and I'll work on a PR.
On it
Any news on this ? I have an issue that looks really similar.
I have one product on my order, with a line item promotion adjustment applied to it (10% discount on the product). I add a second product with promotionable = false
to the cart and the promotion on the first product is canceled, more precisely it's still eligible but the amount is set to 0.
After adding the second product, the promotion eligibility is recalculated. it's still found the line item promotion eligible.
We also recalculate its amount.
At this point the Spree::Promotion::Actions::CreateItemAdjustments#compute_amount
return 0
because of this line:
return 0 unless promotion.line_item_actionable?(order, adjustable)
this is because in promotion#line_item_actionable?
the promotion is not eligible at the order level:
if eligible?(order, promotion_code: promotion_code) # return false
because the order is "blacklisted" in promotion#eligible?
.
2 quick suggestions for a fix:
- First one, in
line_item_actionable?
we remove the condition on
if eligible?(order, promotion_code: promotion_code)
- Second one, this method
promotion#line_item_actionable?
is not necessary and we should replace it everywhere bypromotion#eligible?(line_item)
If one of these seems like a good fix, I could send a PR.
Closing as stale.
Sorry I don't understand why you close it ? I think there is still a bug, I was asking what would be a good path for a fix and didn't have any response since.
Is it that a ticket without active discussion should be closed automatically ?