solidus icon indicating copy to clipboard operation
solidus copied to clipboard

Promotion eligibility? ambiguous behavior

Open deodad opened this issue 7 years ago • 5 comments

On 1.3 (and on):

  1. Create a coupon promotion that applies a discount to line items
  2. Add a non-promotionable product to the cart
  3. Add a promotionable product to the cart
  4. 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.

deodad avatar Sep 13 '16 22:09 deodad

On it

softr8 avatar May 22 '17 12:05 softr8

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 by promotion#eligible?(line_item)

If one of these seems like a good fix, I could send a PR.

loicginoux avatar Dec 14 '18 18:12 loicginoux

Closing as stale.

deodad avatar Mar 26 '19 15:03 deodad

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 ?

loicginoux avatar Mar 26 '19 15:03 loicginoux