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

Add no other promotions rule #2959

Closed
wants to merge 3 commits into
base: master
from

Conversation

Projects
None yet
3 participants
@jacobherrington
Member

jacobherrington commented Nov 20, 2018

This is a promotion rule that seems like it should ship with Solidus. The primary reason I feel this way is because it requires modifying the Spree::PromotionHandler::Coupon class.

Another solution might be extracting promotion_stacking_disabled? into some kind of flag on the promotion model instead of a method that is less than efficient.

jacobherrington added some commits Nov 20, 2018

Add a promotion rule to disable promotion stacking
This seems like a rule that many stores would like to have implemented
and it requires a change to a class in core.

@jacobherrington jacobherrington force-pushed the jacobherrington:add-no-other-promotions-rule branch from 28ec21d to 0e442df Nov 20, 2018

@jacobherrington

This comment has been minimized.

Member

jacobherrington commented Nov 20, 2018

I'd love to hear any constructive feedback on this idea.

@ericsaupe

This comment has been minimized.

Contributor

ericsaupe commented Nov 20, 2018

I like this idea but I'm afraid of how it would work. The way it's currently written the NoOtherPromotions promo wouldn't apply if other promotions were already there. What if that was a better promotion than what is currently applied? AFAIK the default frontend doesn't allow users to manage promos applied to an order which would entirely lock the user out of using this promo. Should the application of this promo remove other promos if better?

Not really sure what the best path is just thinking out loud.

@jacobherrington

This comment has been minimized.

Member

jacobherrington commented Nov 20, 2018

@ericsaupe I don't disagree with you. However, I do think there should be an option for stores who don't want coupon stacking of any kind and this is a rather generic implementation of that which can be extended for more specific use cases (e.g. picking the best coupon).

I would also really like if there was a global config option that disabled coupon stacking on ALL coupons, but that is a little less granular than this method.

Just to clarify, the way it works, a promotion with this rule could not be applied if another promotion was already applied AND no other other promotion could be applied after it was applied. So it is only possible to use such a promotion as the ONLY promotion.

@jacobherrington

This comment has been minimized.

Member

jacobherrington commented Nov 24, 2018

I think it's worth revisiting this later, but I'll leave the PR open to gather thoughts.

@ericsaupe

I'm OK with it as long as the limitations are known. I do like the idea overall just wanted to make sure I understood when/how it would work

@jacobherrington

This comment has been minimized.

Member

jacobherrington commented Nov 30, 2018

@ericsaupe we put this in production and we are having some trouble with it. I'd advise not merging this until I can revisit it and do something more robust.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment