diff --git a/core/app/models/spree/promotion/rules/taxon.rb b/core/app/models/spree/promotion/rules/taxon.rb index 7c1bbd3e077..4ff7cf919fc 100644 --- a/core/app/models/spree/promotion/rules/taxon.rb +++ b/core/app/models/spree/promotion/rules/taxon.rb @@ -16,25 +16,29 @@ def applicable?(promotable) end def eligible?(order, _options = {}) - order_taxons = taxons_in_order_including_parents(order) + order_taxons = taxons_in_order(order) case preferred_match_policy when 'all' - unless (taxons.to_a - order_taxons).empty? + matches_all = taxons.all? do |rule_taxon| + order_taxons.where(id: rule_taxon.self_and_descendants.ids).exists? + end + + unless matches_all eligibility_errors.add(:base, eligibility_error_message(:missing_taxon)) end when 'any' - unless taxons.any?{ |taxon| order_taxons.include? taxon } + unless order_taxons.where(id: rule_taxon_ids_with_children).exists? eligibility_errors.add(:base, eligibility_error_message(:no_matching_taxons)) end when 'none' - unless taxons.none?{ |taxon| order_taxons.include? taxon } + if order_taxons.where(id: rule_taxon_ids_with_children).exists? eligibility_errors.add(:base, eligibility_error_message(:has_excluded_taxon)) end else # Change this to an exception in a future version of Solidus warn_invalid_match_policy(assume: 'any') - unless taxons.any? { |taxon| order_taxons.include? taxon } + unless order_taxons.where(id: rule_taxon_ids_with_children).exists? eligibility_errors.add(:base, eligibility_error_message(:no_matching_taxons)) end end @@ -42,16 +46,24 @@ def eligible?(order, _options = {}) eligibility_errors.empty? end + # TODO: Fix bug - well described by jhawthorn in #1409: + # `eligible?` checks the configured taxons and all descendants, + # `actionable?` only seems to check against the taxons themselves (not children) def actionable?(line_item) + found = Spree::Classification.where( + product_id: line_item.variant.product_id, + taxon_id: taxon_ids + ).exists? + case preferred_match_policy when 'any', 'all' - taxon_product_ids.include?(line_item.variant.product_id) + found when 'none' - taxon_product_ids.exclude? line_item.variant.product_id + !found else # Change this to an exception in a future version of Solidus warn_invalid_match_policy(assume: 'any') - taxon_product_ids.include?(line_item.variant.product_id) + found end end @@ -74,27 +86,16 @@ def warn_invalid_match_policy(assume:) end # All taxons in an order - def order_taxons(order) - Spree::Taxon.joins(products: { variants_including_master: :line_items }).where(spree_line_items: { order_id: order.id }).distinct + def taxons_in_order(order) + Spree::Taxon.joins(products: { variants_including_master: :line_items }) + .where(spree_line_items: { order_id: order.id }).distinct end # ids of taxons rules and taxons rules children - def taxons_including_children_ids - taxons.flat_map { |taxon| taxon.self_and_descendants.ids } - end - - # taxons order vs taxons rules and taxons rules children - def order_taxons_in_taxons_and_children(order) - order_taxons(order).where(id: taxons_including_children_ids) - end - - def taxons_in_order_including_parents(order) - order_taxons_in_taxons_and_children(order).flat_map(&:self_and_ancestors).uniq + def rule_taxon_ids_with_children + taxons.flat_map { |taxon| taxon.self_and_descendants.ids }.uniq end - def taxon_product_ids - Spree::Product.joins(:taxons).where(spree_taxons: { id: taxons.pluck(:id) }).pluck(:id).uniq - end end end end diff --git a/core/spec/models/spree/promotion/rules/taxon_spec.rb b/core/spec/models/spree/promotion/rules/taxon_spec.rb index 600e67c9ebd..3ac66d92a14 100644 --- a/core/spec/models/spree/promotion/rules/taxon_spec.rb +++ b/core/spec/models/spree/promotion/rules/taxon_spec.rb @@ -1,29 +1,30 @@ require 'spec_helper' describe Spree::Promotion::Rules::Taxon, type: :model do + let(:taxon) { create :taxon, name: 'first' } + let(:taxon2) { create :taxon, name: 'second' } + let(:order) { create :order_with_line_items } + let(:product) { order.products.first } + let(:rule) do Spree::Promotion::Rules::Taxon.create!(promotion: create(:promotion)) end context '#eligible?(order)' do - let(:taxon){ create :taxon, name: 'first' } - let(:taxon2){ create :taxon, name: 'second' } - let(:order){ create :order_with_line_items } - context 'with any match policy' do before do rule.update!(preferred_match_policy: 'any') end it 'is eligible if order does have any prefered taxon' do - order.products.first.taxons << taxon + product.taxons << taxon rule.taxons << taxon expect(rule).to be_eligible(order) end context 'when order contains items from different taxons' do before do - order.products.first.taxons << taxon + product.taxons << taxon rule.taxons << taxon end @@ -50,7 +51,7 @@ context 'when a product has a taxon child of a taxon rule' do before do taxon.children << taxon2 - order.products.first.taxons << taxon2 + product.taxons << taxon2 rule.taxons << taxon end @@ -64,7 +65,7 @@ end it 'is eligible order has all prefered taxons' do - order.products.first.taxons << taxon2 + product.taxons << taxon2 order.products.last.taxons << taxon rule.taxons = [taxon, taxon2] @@ -83,17 +84,16 @@ end context 'when a product has a taxon child of a taxon rule' do - let(:taxon3){ create :taxon } + let(:taxon3) { create :taxon } before do taxon.children << taxon2 - order.products.first.taxons << taxon2 - order.products.last.taxons << taxon3 - rule.taxons << taxon2 - rule.taxons << taxon3 + taxon.save! + product.taxons = [taxon2, taxon3] + rule.taxons = [taxon, taxon3] end - it{ expect(rule).to be_eligible(order) } + it { expect(rule).to be_eligible(order) } end end