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

Exclude Expired Promotions from possible_promotions #2058

Closed
wants to merge 2 commits into
base: 1-2-stable
from

Conversation

Projects
None yet
3 participants
@iloveitaly
Member

iloveitaly commented Oct 6, 2012

  • Added spec for Spree::Product#possible_promotions
  • Made create_per_product_promotion a bit more flexible
  • Excluded expired promotions from possible_promotions
@msevestre

This comment has been minimized.

Show comment
Hide comment
@msevestre

msevestre Oct 6, 2012

Why are you using the advertised scope here?

Why are you using the advertised scope here?

This comment has been minimized.

Show comment
Hide comment
@iloveitaly

iloveitaly Oct 6, 2012

Owner

I believe the intended use for this method is to display promotions publicly on your site (at least that is what I use it for). This enables someone to determine if a promotion should be displayed on the site or not from the admin. Advertised scopes out 'private' promotions.

Owner

iloveitaly replied Oct 6, 2012

I believe the intended use for this method is to display promotions publicly on your site (at least that is what I use it for). This enables someone to determine if a promotion should be displayed on the site or not from the admin. Advertised scopes out 'private' promotions.

This comment has been minimized.

Show comment
Hide comment
@msevestre

msevestre Oct 6, 2012

msevestre replied Oct 6, 2012

This comment has been minimized.

Show comment
Hide comment
@iloveitaly

iloveitaly Oct 6, 2012

Owner

To clarify: possible_promotions doesn't play a part in applying promotions on a site. It actually is not used anywhere in the spree codebase. The method only exists to allow the developer an easy way to query a product and see which promotions are available to it.

So possible_promotions will only return:

  • promotions which are marked as advertised
  • promotions that are associated with the product the method was called from
  • promotions which have not expired

make sense? I am missing something here?

Owner

iloveitaly replied Oct 6, 2012

To clarify: possible_promotions doesn't play a part in applying promotions on a site. It actually is not used anywhere in the spree codebase. The method only exists to allow the developer an easy way to query a product and see which promotions are available to it.

So possible_promotions will only return:

  • promotions which are marked as advertised
  • promotions that are associated with the product the method was called from
  • promotions which have not expired

make sense? I am missing something here?

@radar radar closed this in e3bd861 Oct 8, 2012

radar added a commit that referenced this pull request Oct 8, 2012

Exclude expired promotions from Spree::Product#possible_promotions
Fixes #2058

Conflicts:

	promo/spec/requests/promotion_adjustments_spec.rb
@radar

This comment has been minimized.

Show comment
Hide comment
@radar

radar Oct 8, 2012

Member

Makes sense to me. Going to apply this to 1-2-stable and master now.

Member

radar commented Oct 8, 2012

Makes sense to me. Going to apply this to 1-2-stable and master now.

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