Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Promotion eligibility? ambiguous behavior #1437

Open
deodad opened this issue Sep 13, 2016 · 5 comments
Open

Promotion eligibility? ambiguous behavior #1437

deodad opened this issue Sep 13, 2016 · 5 comments
Labels
confirmed Validated report type:bug Error, flaw or fault

Comments

@deodad
Copy link
Contributor

deodad commented Sep 13, 2016

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.

@softr8
Copy link
Contributor

softr8 commented May 22, 2017

On it

@loicginoux
Copy link
Contributor

loicginoux commented Dec 14, 2018

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.

@deodad deodad closed this as completed Mar 26, 2019
@deodad
Copy link
Contributor Author

deodad commented Mar 26, 2019

Closing as stale.

@loicginoux
Copy link
Contributor

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 ?

@cesartalves
Copy link
Contributor

According to @kennyadsl, "Order level promotions have been downgraded to second class citizens, if not deprecated".

This issue refers to version 1.3, and we will need to take a look if the problem still makes sense / applies.

@cesartalves cesartalves added the confirmed Validated report label Nov 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
confirmed Validated report type:bug Error, flaw or fault
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants