Skip to content

Commit bbb2417

Browse files
committed
Order taxon condition: Allow non-persisted line items
This will lead to an n+1-query if the line items are loaded without their product classifications. Also I'm not super-sure about the semantics of the `all` match policy: Do we want to make sure that all taxons are present in the order, or do we want to make sure that all line items match any of the taxons?
1 parent 542cadc commit bbb2417

File tree

2 files changed

+26
-16
lines changed

2 files changed

+26
-16
lines changed

promotions/app/models/solidus_promotions/conditions/order_taxon.rb

Lines changed: 18 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -12,27 +12,34 @@ class OrderTaxon < Condition
1212
preference :match_policy, :string, default: MATCH_POLICIES.first
1313

1414
def order_eligible?(order, _options = {})
15-
order_taxons = taxons_in_order(order)
15+
line_item_taxons_ids = taxon_ids_in_order(order)
16+
condition_taxon_ids = condition_taxon_ids_with_children
1617

1718
case preferred_match_policy
1819
when "all"
19-
matches_all = taxons.all? do |condition_taxon|
20-
order_taxons.where(id: condition_taxon.self_and_descendants.ids).exists?
20+
unless line_item_taxons_ids.all? do |line_item_taxon_ids|
21+
(line_item_taxon_ids & condition_taxon_ids).any?
2122
end
2223

23-
unless matches_all
2424
eligibility_errors.add(:base, eligibility_error_message(:missing_taxon), error_code: :missing_taxon)
2525
end
2626
when "any"
27-
unless order_taxons.where(id: condition_taxon_ids_with_children).exists?
27+
if line_item_taxons_ids.none? do |line_item_taxon_ids|
28+
(line_item_taxon_ids & condition_taxon_ids).any?
29+
end
30+
2831
eligibility_errors.add(
2932
:base,
3033
eligibility_error_message(:no_matching_taxons),
3134
error_code: :no_matching_taxons
3235
)
3336
end
3437
when "none"
35-
if order_taxons.where(id: condition_taxon_ids_with_children).exists?
38+
matches_none = line_item_taxons_ids.none? do |line_item_taxon_ids|
39+
(line_item_taxon_ids & condition_taxon_ids).any?
40+
end
41+
42+
unless matches_none
3643
eligibility_errors.add(
3744
:base,
3845
eligibility_error_message(:has_excluded_taxon),
@@ -50,12 +57,11 @@ def to_partial_path
5057

5158
private
5259

53-
# All taxons in an order
54-
def taxons_in_order(order)
55-
Spree::Taxon
56-
.joins(products: { variants_including_master: :line_items })
57-
.where(spree_line_items: { order_id: order.id })
58-
.distinct
60+
# All taxon IDs in an order
61+
def taxon_ids_in_order(order)
62+
order.line_items.map do |line_item|
63+
line_item.variant.product.classifications.map(&:taxon_id)
64+
end
5965
end
6066
end
6167
end

promotions/spec/models/solidus_promotions/conditions/order_taxon_spec.rb

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,13 +8,17 @@
88
let(:condition) do
99
described_class.create!(benefit: promotion_benefit)
1010
end
11-
let(:product) { order.products.first }
12-
let(:order) { create :order_with_line_items }
11+
let(:product) { create(:product) }
12+
let(:order) { create :order }
1313
let(:taxon_one) { create :taxon, name: "first" }
1414
let(:taxon_two) { create :taxon, name: "second" }
1515

1616
it_behaves_like "a taxon condition"
1717

18+
before do
19+
order.line_items.new(quantity: 1, variant: product.master)
20+
end
21+
1822
it { is_expected.to be_updateable }
1923

2024
describe "#eligible?(order)" do
@@ -66,7 +70,7 @@
6670

6771
it "is eligible order has all prefered taxons" do
6872
product.taxons << taxon_two
69-
order.products.last.taxons << taxon_one
73+
product.taxons << taxon_one
7074

7175
condition.taxons = [taxon_one, taxon_two]
7276

@@ -121,7 +125,7 @@
121125

122126
context "one of the order's products is in a listed taxon" do
123127
before do
124-
order.products.first.taxons << taxon_one
128+
product.taxons << taxon_one
125129
condition.taxons << taxon_one
126130
end
127131

0 commit comments

Comments
 (0)