From 0cf244b2113072a3e7f13b025f9a7fc0fac93ccd Mon Sep 17 00:00:00 2001 From: Gaetan Craig-Riou Date: Fri, 29 Aug 2025 13:57:25 +1000 Subject: [PATCH] Document variant filtering rule behavior Add test to conver scenario where we have conficting rules --- app/services/product_tag_rules_filterer.rb | 7 ++ .../product_tag_rules_filterer_spec.rb | 97 ++++++++++++++++++- 2 files changed, 100 insertions(+), 4 deletions(-) diff --git a/app/services/product_tag_rules_filterer.rb b/app/services/product_tag_rules_filterer.rb index 39a5c1e585..36255e256d 100644 --- a/app/services/product_tag_rules_filterer.rb +++ b/app/services/product_tag_rules_filterer.rb @@ -4,6 +4,13 @@ # Tag rules exists in the context of enterprise, customer, and variant_overrides, # and are applied to variant_overrides only. Returns a Spree::Variant AR object. +# The filtering is somewhat not intuitive when they are conflicting rules in play: +# * When a variant is hidden by a default rule, the order of customer related rules doesn't matter +# ( despite the use of `TagRule::FilterProducts.prioritised` ). It will apply the "show rule" +# if any +# * When there is no default rule, the order of customer related rules doesn't matter, it will +# apply the "hide rule" if any +# class ProductTagRulesFilterer def initialize(distributor, customer, variants_relation) @distributor = distributor diff --git a/spec/services/product_tag_rules_filterer_spec.rb b/spec/services/product_tag_rules_filterer_spec.rb index 5fd88f7dfa..3df5a41a10 100644 --- a/spec/services/product_tag_rules_filterer_spec.rb +++ b/spec/services/product_tag_rules_filterer_spec.rb @@ -18,7 +18,7 @@ RSpec.describe ProductTagRulesFilterer do } let(:customer) { create(:customer, enterprise: distributor) } let(:variants_relation) { - Spree::Variant.where(supplier: distributor.id) + Spree::Variant.left_joins(:variant_overrides).where(supplier: distributor.id) } let(:default_hide_rule) { create(:filter_products_tag_rule, @@ -50,9 +50,98 @@ RSpec.describe ProductTagRulesFilterer do } let(:filterer) { described_class.new(distributor, customer, variants_relation) } - context "when the distributor has no rules" do - it "returns the relation unchanged" do - expect(filterer.call).to eq variants_relation + describe "#call" do + before do + v1 + v2 + v3 + v4 + end + + context "when the distributor has no rules" do + it "returns the relation unchanged" do + expect(filterer.call).to eq variants_relation + end + end + + context "with hide rule" do + it "hides the variant matching the rule" do + customer.update_attribute(:tag_list, hide_rule.preferred_customer_tags) + variant_hidden_by_rule.update_attribute(:tag_list, hide_rule.preferred_variant_tags) + + expect(filterer.call).not_to include(variant_hidden_by_rule.variant) + end + + context "with mutiple conflicting rules" do + it "applies the hide rule" do + # Customer has show rule tag and hide rule tag + customer.update_attribute(:tag_list, + [hide_rule.preferred_customer_tags, + show_rule.preferred_customer_tags]) + # Variant has show rule tag and hide rule tag + variant_hidden_by_rule.update_attribute(:tag_list, + [hide_rule.preferred_variant_tags, + hide_rule.preferred_variant_tags,]) + hide_rule.update_attribute(:priority, 1) + show_rule.update_attribute(:priority, 2) + + expect(filterer.call).not_to include(variant_hidden_by_rule.variant) + + # Re order rule + hide_rule.update_attribute(:priority, 2) + show_rule.update_attribute(:priority, 1) + + expect(filterer.call).not_to include(variant_hidden_by_rule.variant) + end + end + end + + context "with variant hidden by default" do + before do + variant_hidden_by_default.update_attribute(:tag_list, + default_hide_rule.preferred_variant_tags) + end + + it "excludes variant hidden by default" do + expect(filterer.call).not_to include(variant_hidden_by_default.variant) + end + + context "with variant rule overriding default rule" do + it "includes variant hidden by default" do + customer.update_attribute(:tag_list, show_rule.preferred_customer_tags) + # Variant has default rule tag and show rule tag + variant_hidden_by_default.update_attribute(:tag_list, + [default_hide_rule.preferred_variant_tags, + show_rule.preferred_variant_tags]) + + expect(filterer.call).to include(variant_hidden_by_default.variant) + end + + context "with mutiple conflicting rules applying to same variant" do + it "applies the show rule" do + # customer has show rule and hide rule tag + customer.update_attribute(:tag_list, + [show_rule.preferred_customer_tags, + hide_rule.preferred_customer_tags]) + + # Variant has default rule tag and show rule tag and hide rule tag + show_rule.update_attribute(:priority, 1) + hide_rule.update_attribute(:priority, 2) + variant_hidden_by_default.update_attribute(:tag_list, + [default_hide_rule.preferred_variant_tags, + show_rule.preferred_variant_tags, + hide_rule.preferred_variant_tags]) + + expect(filterer.call).to include(variant_hidden_by_default.variant) + + # Re order rule + show_rule.update_attribute(:priority, 2) + hide_rule.update_attribute(:priority, 1) + + expect(filterer.call).to include(variant_hidden_by_default.variant) + end + end + end end end