Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,52 @@ def taxon_ids_string=(taxon_ids)
self.taxons = Spree::Taxon.find(taxon_ids)
end

def taxons_ids_with_children=(args)
@taxon_ids_with_children = args
end

private

# ids of taxons conditions and taxons conditions children
def condition_taxon_ids_with_children
taxons.flat_map { |taxon| taxon.self_and_descendants.ids }.uniq
# Returns the cached list of taxon subtree id collections for the selected taxons.
#
# Executes a single SQL query using the nested set (lft/rgt) boundaries to
# fetch each root taxon (one of this condition's taxons) together with all
# of its descendants. The result is memoized for the lifetime of the
# condition instance.
#
# Each inner array contains the IDs of a root taxon and all of its
# descendants (including the root itself). The outer array is ordered by
# the root taxon id.
#
# @return [Array<Array<Integer>>] array of arrays of taxon ids, one per root taxon
# @example
# # For condition with taxons [10, 42]
# condition.condition_taxon_ids_with_children
# # => [[10, 11, 12], [42, 43]]
def taxon_ids_with_children
@taxon_ids_with_children ||= load_taxon_ids_with_children
end

def load_taxon_ids_with_children
aggregation_function = if ActiveRecord::Base.connection.adapter_name.downcase.match?(/postgres/)
"string_agg(child.id::text, ',')"
else
"group_concat(child.id, ',')"
end

sql = <<~SQL
SELECT
parent.id AS root_id,
#{aggregation_function} AS descendant_ids
FROM spree_taxons AS parent
JOIN spree_taxons AS child
ON child.lft BETWEEN parent.lft AND parent.rgt
WHERE parent.id IN (#{taxon_ids.join(',')})
GROUP BY parent.id
ORDER BY parent.id
SQL
rows = ActiveRecord::Base.connection.exec_query(sql)
rows.map { |r| r["descendant_ids"].split(",").map(&:to_i) }
end
end
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ class LineItemOptionValue < Condition

def line_item_eligible?(line_item, _options = {})
pid = line_item.product.id
ovids = line_item.variant.option_values.pluck(:id)
ovids = line_item.variant.option_value_ids

product_ids.include?(pid) && (value_ids(pid) & ovids).present?
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,16 +15,13 @@ class LineItemTaxon < Condition
preference :match_policy, :string, default: MATCH_POLICIES.first

def line_item_eligible?(line_item, _options = {})
found = Spree::Classification.where(
product_id: line_item.variant.product_id,
taxon_id: condition_taxon_ids_with_children
).exists?
line_item_taxon_ids = line_item.variant.product.classifications.map(&:taxon_id)

case preferred_match_policy
when "include"
found
taxon_ids_with_children.any? { |taxon_and_descendant_ids| (line_item_taxon_ids & taxon_and_descendant_ids).any? }
when "exclude"
!found
taxon_ids_with_children.none? { |taxon_and_descendant_ids| (line_item_taxon_ids & taxon_and_descendant_ids).any? }
else
raise "unexpected match policy: #{preferred_match_policy.inspect}"
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,27 +12,26 @@ class OrderTaxon < Condition
preference :match_policy, :string, default: MATCH_POLICIES.first

def order_eligible?(order, _options = {})
order_taxons = taxons_in_order(order)
line_item_taxon_ids = taxon_ids_in_order(order)

case preferred_match_policy
when "all"
matches_all = taxons.all? do |condition_taxon|
order_taxons.where(id: condition_taxon.self_and_descendants.ids).exists?
end
unless taxon_ids_with_children.all? { |taxon_and_descendant_ids| (line_item_taxon_ids & taxon_and_descendant_ids).any? }

unless matches_all
eligibility_errors.add(:base, eligibility_error_message(:missing_taxon), error_code: :missing_taxon)
end
when "any"
unless order_taxons.where(id: condition_taxon_ids_with_children).exists?
if taxon_ids_with_children.none? { |taxon_and_descendant_ids| (line_item_taxon_ids & taxon_and_descendant_ids).any? }

eligibility_errors.add(
:base,
eligibility_error_message(:no_matching_taxons),
error_code: :no_matching_taxons
)
end
when "none"
if order_taxons.where(id: condition_taxon_ids_with_children).exists?
if taxon_ids_with_children.any? { |taxon_and_descendant_ids| (line_item_taxon_ids & taxon_and_descendant_ids).any? }

eligibility_errors.add(
:base,
eligibility_error_message(:has_excluded_taxon),
Expand All @@ -50,12 +49,11 @@ def to_partial_path

private

# All taxons in an order
def taxons_in_order(order)
Spree::Taxon
.joins(products: { variants_including_master: :line_items })
.where(spree_line_items: { order_id: order.id })
.distinct
# All taxon IDs in an order
def taxon_ids_in_order(order)
order.line_items.flat_map do |line_item|
line_item.variant.product.classifications.map(&:taxon_id)
end
end
end
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,10 @@ class Taxon < Condition

def order_eligible?(order, _options = {})
order_condition = OrderTaxon.new(taxons:, preferred_match_policy:)

# Hydrate the instance cache with our @taxon_ids_with_children cache
order_condition.taxons_ids_with_children = taxon_ids_with_children

order_condition.order_eligible?(order)
@eligibility_errors = order_condition.eligibility_errors
eligibility_errors.empty?
Expand All @@ -23,6 +27,10 @@ def order_eligible?(order, _options = {})
def line_item_eligible?(line_item, _options = {})
line_item_match_policy = preferred_match_policy.in?(%w[any all]) ? "include" : "exclude"
line_item_condition = LineItemTaxon.new(taxons:, preferred_match_policy: line_item_match_policy)

# Hydrate the instance cache with our @taxon_ids_with_children cache\
line_item_condition.taxons_ids_with_children = taxon_ids_with_children

result = line_item_condition.line_item_eligible?(line_item)
@eligibility_errors = line_item_condition.eligibility_errors
result
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ def initialize(line_items, total_amount)
# @param line_item [LineItem] one of the line_items distributed over
# @return [BigDecimal] the weighted adjustment for this line_item
def amount(line_item)
distributed_amounts[line_item.id].to_d
distributed_amounts[line_item].to_d
end

private
Expand All @@ -21,11 +21,7 @@ def amount(line_item)
# @return [Hash<Integer, BigDecimal>] a hash of line item IDs and their
# corresponding weighted adjustments
def distributed_amounts
line_item_ids.zip(allocated_amounts).to_h
end

def line_item_ids
line_items.map(&:id)
line_items.zip(allocated_amounts).to_h
end

def elligible_amounts
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,17 @@
let(:condition) do
described_class.create!(benefit: promotion_benefit)
end
let(:product) { order.products.first }
let(:order) { create :order_with_line_items }
let(:product) { create(:product) }
let(:order) { create :order }
let(:taxon_one) { create :taxon, name: "first" }
let(:taxon_two) { create :taxon, name: "second" }

it_behaves_like "a taxon condition"

before do
order.line_items.new(quantity: 1, variant: product.master)
end

it { is_expected.to be_updateable }

describe "#eligible?(order)" do
Expand Down Expand Up @@ -66,7 +70,7 @@

it "is eligible order has all prefered taxons" do
product.taxons << taxon_two
order.products.last.taxons << taxon_one
product.taxons << taxon_one

condition.taxons = [taxon_one, taxon_two]

Expand Down Expand Up @@ -121,7 +125,7 @@

context "one of the order's products is in a listed taxon" do
before do
order.products.first.taxons << taxon_one
product.taxons << taxon_one
condition.taxons << taxon_one
end

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -187,4 +187,21 @@
end
end
end

describe "performance" do
let(:product_one) { create(:product, taxons: [taxon_one]) }
let(:product_two) { create(:product, taxons: [taxon_two]) }
let!(:promotion) { create(:solidus_promotion, :with_adjustable_benefit, apply_automatically: true) }
let(:benefit) { promotion.benefits.first }
let!(:condition) { described_class.create!(taxons: [taxon_one, taxon_two], benefit:) }
let!(:order) { create(:order_with_line_items, line_items_attributes: [{ variant: product_one.master }, { variant: product_two.master }]) }

subject { order.recalculate }

it "only loads the taxon IDs with children once" do
expect_any_instance_of(SolidusPromotions::Conditions::TaxonCondition).to receive(:load_taxon_ids_with_children).once { [[taxon_one.id], [taxon_two.id]] }

subject
end
end
end
Loading