Deprecate Promotion any Match Policy
#4304
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mamhoff
force-pushed
the
deprecate-promotion-any-policy
branch
3 times, most recently
from
d1e72cd to
e23326e
Compare
tvdeyen
reviewed
Mar 18, 2022
kennyadsl
reviewed
Mar 22, 2022
core/db/migrate/20220317165036_set_promotions_with_any_policy_to_all_if_possible.rb
Show resolved
Hide resolved
5minpause
added a commit
to TILCRA/solidus
that referenced
this pull request
Mar 22, 2022
If we reduce the amount of promotions that can be activated, the performance when refreshing the cart improves dramatically. Eventually, this might not be necessary since Solidus#master branch is updated with solidusio#4304 and solidusio#4296. Until those land in master and we are able to update to Solidus 3.2, we need to maintain this fork.
mamhoff
force-pushed
the
deprecate-promotion-any-policy
branch
5 times, most recently
from
38c394a to
dded3f0
Compare
core/db/migrate/20220317165036_set_promotions_with_any_policy_to_all_if_possible.rb
Outdated
Show resolved
Hide resolved
Using the "any" match policy on promotions creates a lot of difficulty when working with the promotion system. A recent poll on the Solidus Slack indicated that most stores do not use these. It is also from the user's point of view preferable to create separate promotions for each rule in this case, as this will allow store owners to set individual start and end dates and so on for each rule.
mamhoff
force-pushed
the
deprecate-promotion-any-policy
branch
from
dded3f0 to
5732d42
Compare
core/db/migrate/20220317165036_set_promotions_with_any_policy_to_all_if_possible.rb
Outdated
Show resolved
Hide resolved
core/lib/tasks/solidus/split_promotions_with_any_match_policy.rake
Outdated
Show resolved
Hide resolved
A promotion that has a match_policy of "any", but not more than 1 rule, can also have a match_policy of "all" with no change in behavior. This will stop a lot of people from getting deprecation warnings.
This allows us to mark a block as testing deprecated code so that it can easily be identified and removed in the future.
mamhoff
force-pushed
the
deprecate-promotion-any-policy
branch
from
5732d42 to
b3ea863
Compare
spaghetticode
approved these changes
Apr 4, 2022
jarednorman
approved these changes
Apr 4, 2022
tvdeyen
approved these changes
Apr 11, 2022
RyanofWoods
reviewed
Nov 25, 2022
| new_promotion = promotion.dup | ||
| new_promotion.promotion_rules = [rule] | ||
| new_promotion.match_policy = "all" | ||
| new_promotion.promotion_actions = promotion.actions.map do |action| |
There was a problem hiding this comment.
It seems like this is going to create many extra actions 🤔 For example, having 3 rules will cause 12 actions to be created due to the nesting. Though, it will end up with the correct set of actions due to promotion_actions =
kennyadsl
added a commit
to nebulab/solidus
that referenced
this pull request
Mar 22, 2023
We are going to remove this preference in Solidus 4.0. The only accepted value for who wants to upgrade is false. Ref solidusio#4304
3 tasks
kennyadsl
added a commit
to nebulab/solidus
that referenced
this pull request
Mar 27, 2023
…g policy Matching "all" rules is the only supported path now. You can have the same effect of the previoys "any" matching policy by creating multiple promotions with the same configuration. Ref solidusio#4304
kennyadsl
added a commit
to nebulab/solidus
that referenced
this pull request
Mar 27, 2023
We are going to remove this preference in Solidus 4.0. The only accepted value for who wants to upgrade is false. Ref solidusio#4304
kennyadsl
added a commit
to nebulab/solidus
that referenced
this pull request
Mar 27, 2023
…g policy Matching "all" rules is the only supported path now. You can have the same effect of the previoys "any" matching policy by creating multiple promotions with the same configuration. Ref solidusio#4304
kennyadsl
added a commit
to nebulab/solidus
that referenced
this pull request
Mar 27, 2023
…g policy Matching "all" rules is the only supported path now. You can have the same effect of the previoys "any" matching policy by creating multiple promotions with the same configuration. This change also removes the rake task we provided to update the existing promotions. That script needs to be executed on previous versions of Solidus only. Ref solidusio#4304
kennyadsl
added a commit
to nebulab/solidus
that referenced
this pull request
Apr 12, 2023
…g policy Matching "all" rules is the only supported path now. You can have the same effect of the previoys "any" matching policy by creating multiple promotions with the same configuration. This change also removes the rake task we provided to update the existing promotions. That script needs to be executed on previous versions of Solidus only. Ref solidusio#4304
kennyadsl
added a commit
to nebulab/solidus
that referenced
this pull request
Apr 14, 2023
…g policy Matching "all" rules is the only supported path now. You can have the same effect of the previoys "any" matching policy by creating multiple promotions with the same configuration. This change also removes the rake task we provided to update the existing promotions. That script needs to be executed on previous versions of Solidus only. Ref solidusio#4304
kennyadsl
added a commit
to nebulab/solidus
that referenced
this pull request
Apr 18, 2023
Matching "all" rules is the only supported path now. You can have the same effect of the previoys "any" matching policy by creating multiple promotions with the same configuration. This change also removes the rake task we provided to update the existing promotions. That script needs to be executed on previous versions of Solidus only. Ref solidusio#4304
kennyadsl
added a commit
that referenced
this pull request
Apr 18, 2023
Matching "all" rules is the only supported path now. You can have the same effect of the previoys "any" matching policy by creating multiple promotions with the same configuration. This change also removes the rake task we provided to update the existing promotions. That script needs to be executed on previous versions of Solidus only. Ref #4304
3 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Working with the promotion system is made much more complex because Solidus has a feature that allows a promotion to consider an order / line item eligible if any of its rules matches. The same behavior can be obtained - with much greater flexibility - by creating separate promotions for each rule.
Currently, we need to check order-level and line-item level eligibility on once for every line item, and that's only necessary because we allow a match policy of
any. If onlyallmatch_policy promotions existed, we could first check the order eligibility and then check line item eligibility using only line-item applicable rules, which would be a major performance boost and would make the eligibility checks a lot more understandable.Checklist: