Refactoring tag rule application

To allow rules to be loaded and counted before being checked for relevance

app/helpers/enterprises_helper.rb

@@ -6,7 +6,11 @@ module EnterprisesHelper
   def available_shipping_methods
     shipping_methods = current_distributor.shipping_methods
     if current_distributor.present?
-      current_distributor.apply_tag_rules_to(shipping_methods, customer: current_order.customer)
+      current_distributor.apply_tag_rules(
+        type: "FilterShippingMethods",
+        subject: shipping_methods,
+        customer_tags: current_order.andand.customer.andand.tag_list
+      )
     end
     shipping_methods.uniq
   end

app/models/enterprise.rb

@@ -344,9 +344,9 @@ class Enterprise < ActiveRecord::Base
     abn.present?
   end
 
-  def apply_tag_rules_to(subject, context)
-    tag_rules.each do |rule|
-      rule.set_context(subject,context)
+  def apply_tag_rules(context)
+    tag_rules_for(context).each do |rule|
+      rule.set_context(context)
       rule.apply
     end
   end
@@ -459,4 +459,10 @@ class Enterprise < ActiveRecord::Base
   def initialize_permalink
     self.permalink = Enterprise.find_available_permalink(name)
   end
+
+  def tag_rules_for(context)
+    rules = context[:rules] || tag_rules
+    return rules unless context[:type]
+    rules.merge(TagRule.of_type(context[:type]))
+  end
 end

app/models/spree/order_decorator.rb

@@ -180,7 +180,7 @@ Spree::Order.class_eval do
       end
 
       if distributor.present? && customer.present?
-        distributor.apply_tag_rules_to(self, customer: customer)
+        distributor.apply_tag_rules(type: "DiscountOrder", subject: self, customer_tags: customer.andand.tag_list)
       end
     end
   end

app/models/tag_rule.rb

@@ -9,11 +9,15 @@ class TagRule < ActiveRecord::Base
 
   attr_accessible :enterprise, :enterprise_id, :preferred_customer_tags
 
-  scope :for, lambda { |enterprises| where(enterprise_id: enterprises) }
+  scope :for, ->(enterprise) { where(enterprise_id: enterprise) }
+  scope :of_type, ->(type) { where(type: "TagRule::#{type}") }
+
+  def set_context(context)
+    raise "Context for tag rule cannot be nil" if context.nil?
+    raise "Subject for tag rule cannot be nil" if context[:subject].nil?
 
-  def set_context(subject, context)
-    @subject = subject
     @context = context
+    @subject = context[:subject]
   end
 
   def apply
@@ -50,8 +54,8 @@ class TagRule < ActiveRecord::Base
   end
 
   def customer_tags_match?
-    context_customer_tags = context.andand[:customer].andand.tag_list || []
+    context_customer_tags = context[:customer_tags] || []
     preferred_tags = preferred_customer_tags.split(",")
-    ( context_customer_tags & preferred_tags ).any?
+    (context_customer_tags & preferred_tags).any?
   end
 end

spec/helpers/injection_helper_spec.rb

@@ -30,7 +30,7 @@ describe InjectionHelper do
     shipping_methods = double(:shipping_methods, uniq: [sm])
     current_distributor = double(:distributor, shipping_methods: shipping_methods)
     allow(helper).to receive(:current_distributor) { current_distributor }
-    allow(current_distributor).to receive(:apply_tag_rules_to).with(shipping_methods, {customer: nil} )
+    allow(current_distributor).to receive(:apply_tag_rules).with({type: "FilterShippingMethods", subject: shipping_methods, customer_tags: nil})
     helper.inject_available_shipping_methods.should match sm.id.to_s
     helper.inject_available_shipping_methods.should match sm.compute_amount(order).to_s
   end

spec/models/enterprise_spec.rb

@@ -859,4 +859,54 @@ describe Enterprise do
       end
     end
   end
+
+  describe "finding tag rules" do
+    let(:enterprise) { create(:enterprise) }
+    let(:tag_rule1) { create(:filter_products_tag_rule, enterprise: enterprise) }
+    let(:tag_rule2) { create(:filter_shipping_methods_tag_rule, enterprise: enterprise) }
+    let(:tag_rule3) { create(:tag_rule, enterprise: enterprise) }
+    let(:context) { { subject: "something" } }
+
+    context "when a set of rules are passed in with the context" do
+      let(:tag_rules) { TagRule.where(id: [tag_rule1.id, tag_rule3.id]) }
+      before { context[:rules] = tag_rules }
+
+      context "when a rule type has been passed in with the context" do
+        before { context[:type] = "FilterProducts" }
+
+        it "returns rules of the specified type from within the list of passed-in rules" do
+          rules = enterprise.send(:tag_rules_for, context)
+          expect(rules).to include tag_rule1
+          expect(rules).to_not include tag_rule2, tag_rule3
+        end
+      end
+
+      context "when a rule type has not been passed in with the context" do
+        it "returns the list of passed-in rules" do
+          rules = enterprise.send(:tag_rules_for, context)
+          expect(rules).to include tag_rule1, tag_rule3
+          expect(rules).to_not include tag_rule2
+        end
+      end
+    end
+
+    context "when a set of rules are not passed in with the context" do
+      context "when a rule type has been passed in with the context" do
+        before { context[:type] = "FilterShippingMethods" }
+
+        it "returns rules of the specified type belonging to the enterprise" do
+          rules = enterprise.send(:tag_rules_for, context)
+          expect(rules).to include tag_rule2
+          expect(rules).to_not include tag_rule1, tag_rule3
+        end
+      end
+
+      context "when a rule type has not been passed in with the context" do
+        it "returns all rules belonging to the enterprise" do
+          rules = enterprise.send(:tag_rules_for, context)
+          expect(rules).to include tag_rule1, tag_rule2, tag_rule3
+        end
+      end
+    end
+  end
 end

spec/models/tag_rule/discount_order_spec.rb

@@ -7,7 +7,7 @@ describe TagRule::DiscountOrder, type: :model do
     let(:subject) { double(:subject) }
 
     before do
-      tag_rule.set_context(subject,{})
+      tag_rule.set_context({subject: subject})
       allow(tag_rule).to receive(:customer_tags_match?) { true }
       allow(subject).to receive(:class) { Spree::Order }
     end
@@ -34,7 +34,7 @@ describe TagRule::DiscountOrder, type: :model do
     let!(:adjustment) { order.adjustments.create({:amount => 12.34, :source => order, :originator => tag_rule, :label => 'discount' }, :without_protection => true) }
 
     before do
-      tag_rule.set_context(order, nil)
+      tag_rule.set_context({subject: order})
     end
 
     context "where adjustments originating from the rule already exist" do
@@ -56,7 +56,7 @@ describe TagRule::DiscountOrder, type: :model do
     before do
       order.update_distribution_charge!
       tag_rule.calculator.update_attribute(:preferred_flat_percent, -10.00)
-      tag_rule.set_context(order, nil)
+      tag_rule.set_context({subject: order})
     end
 
     context "in a simple scenario" do

spec/models/tag_rule/filter_shipping_methods_spec.rb

@@ -41,7 +41,7 @@ describe TagRule::FilterShippingMethods, type: :model do
 
     before do
       tag_rule.update_attribute(:preferred_shipping_method_tags, "tag2")
-      tag_rule.set_context(shipping_methods, nil)
+      tag_rule.set_context({subject: shipping_methods})
     end
 
     context "apply!" do

spec/models/tag_rule_spec.rb

@@ -11,9 +11,18 @@ describe TagRule, type: :model do
 
   describe 'setting the context' do
     let(:subject) { double(:subject) }
-    let(:context) { double(:context) }
+    let(:context) { { subject: subject, some_other_property: "yay"} }
+
+    it "raises an error when context is nil" do
+      expect{ tag_rule.set_context(nil) }.to raise_error "Context for tag rule cannot be nil"
+    end
+
+    it "raises an error when subject is nil" do
+      expect{ tag_rule.set_context({}) }.to raise_error "Subject for tag rule cannot be nil"
+    end
+
     it "stores the subject and context provided as instance variables on the model" do
-      tag_rule.set_context(subject, context)
+      tag_rule.set_context(context)
       expect(tag_rule.subject).to eq subject
       expect(tag_rule.context).to eq context
       expect(tag_rule.instance_variable_get(:@subject)).to eq subject
@@ -32,7 +41,7 @@ describe TagRule, type: :model do
       let(:subject) { double(:subject) }
 
       before do
-        tag_rule.set_context(subject,{})
+        tag_rule.set_context({subject: subject})
         allow(tag_rule).to receive(:customer_tags_match?) { :customer_tags_match_result }
         allow(tag_rule).to receive(:subject_class) { Spree::Order}
       end
@@ -93,27 +102,20 @@ describe TagRule, type: :model do
     end
 
     describe "determining whether specified customer tags match the given context" do
-      context "when the context is nil" do
-        before { tag_rule.set_context(nil, nil) }
-        it "returns false" do
-          expect(tag_rule.send(:customer_tags_match?)).to be false
-        end
-      end
+      context "when the context has no customer tags specified" do
+        let(:context) { { subject: double(:something), not_tags: double(:not_tags) } }
 
-      context "when the context has no customer specified" do
-        let(:context) { { something_that_is_not_a_customer: double(:something) } }
-
-        before { tag_rule.set_context(nil, context) }
+        before { tag_rule.set_context(context) }
 
         it "returns false" do
           expect(tag_rule.send(:customer_tags_match?)).to be false
         end
       end
 
-      context "when the context has a customer specified" do
-        let(:context) { { customer: double(:customer, tag_list: ["member","local","volunteer"] ) } }
+      context "when the context has customer tags specified" do
+        let(:context) { { subject: double(:something), customer_tags: ["member","local","volunteer"] } }
 
-        before { tag_rule.set_context(nil, context) }
+        before { tag_rule.set_context(context) }
 
         context "when the rule has no preferred customer tags specified" do
           before do