Create single-responsibility conditions for products, taxons, and option values #6363
Conversation
This file contains hidden or 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
We have two conditions that have a HABTM relation to products. This is a bit of boilerplate, so let's extract it into a module.
We have a bunch of shared code between the LineItemTaxon and Taxon condition. Also moves the shared specs to a new module spec.
This is not quite as much boilerplate, but still a sizeable amount.
github-actions
bot
added
the
changelog:solidus_promotions
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #6363 +/- ##
==========================================
+ Coverage 89.37% 89.43% +0.06%
==========================================
Files 961 971 +10
Lines 20200 20288 +88
==========================================
+ Hits 18054 18145 +91
+ Misses 2146 2143 -3 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
mamhoff
force-pushed
the
order-product-condition
branch
2 times, most recently
from
12eeda3 to
94d195a
Compare
jarednorman
approved these changes
Nov 4, 2025
Member
jarednorman
left a comment
jarednorman
left a comment
There was a problem hiding this comment.
This is an excellently structured and explained PR.
10/10 would review again
tvdeyen
approved these changes
Nov 5, 2025
promotions/spec/models/concerns/conditions/option_value_condition_spec.rb
Show resolved
Hide resolved
mamhoff
force-pushed
the
order-product-condition
branch
from
94d195a to
7bfd3bf
Compare
This condition is like the `Product` condition with the `apply_to_line_items` preference set to false. This condition is true for an order that matches the condition, but does not restrict the matching line items. This is useful for things like "10 % off lids if you also bought the matching jar". The intent here is to be able to remove the `LineItemApplicableOrderCondition`, because that's a hard one to parse for users.
We want to remove the hard-to-understand "Also applies to line items" preference, so we just make a new, simpler condition.
We want users to choose whether they want to match a) orders that contain a taxon b) orders that contain a taxon AND line items that contain a taxon c) line items that contain a taxon. This new class adds case a). This also removes an unreachable condition and the unrealistic spec for it.
Removes a confusing checkbox. This also migrates existing conditions.
The LineItemApplicableOrderLevelCondition module has a single feature: Allowing store admins to prevent a combined order/line item condition from evaluating the line item part. This is quite confusing, and probably not used a lot at all. Deprecate it in favor of using simpler Conditions.
mamhoff
force-pushed
the
order-product-condition
branch
from
7bfd3bf to
d0a4ded
Compare
tvdeyen
requested changes
Nov 5, 2025
Member
tvdeyen
left a comment
tvdeyen
left a comment
There was a problem hiding this comment.
I have strong feelings about how to test sharable code. Let's maybe talk IRL and find an agreement?
This allows implementors to create product-based conditions themselves, and test them against our shared examples.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
Summary
Prior to this PR, we had the
preferred_line_item_applicablepreference on theProduct,Taxon, andOptionValueconditions. This preference stopped the condition instance from being applicable for line items, thus matching every line item in the order.The functionality is useful: It allows a promotion that gives a discount on related items - for example, I want to give a discount on belts if the user has already purchased a pair of jeans.
This requires the
Productrule to match on orders, but not on line items. Instead of solving this via a preference though, it's much preferable to have a separate rule for matching products on orders and one for matching products on line items.The same concept applies for taxons and option values.
The amount of LOC changes in this PR is giant, but the actual impact on real-world stores will be tiny given that this feature was largely undocumented and hidden behind a checkbox. Still, I've added deprecation warnings and a migration for the rules we ship.
This also extracts the commonalities between the different Product/Taxon/OptionValue conditions into reusable modules.
Checklist
Check out our PR guidelines for more details.
The following are mandatory for all PRs:
The following are not always needed: