Merge pull request #10509 from mkllnk/filter-shipping-methods-by-category

Filter shipping methods by category

app/services/order_available_payment_methods.rb

@@ -18,9 +18,7 @@ class OrderAvailablePaymentMethods
 
     applicator = OpenFoodNetwork::TagRuleApplicator.new(distributor,
                                                         "FilterPaymentMethods", customer&.tag_list)
-    applicator.filter!(payment_methods)
-
-    payment_methods.uniq
+    applicator.filter(payment_methods)
   end
 
   private
@@ -32,7 +30,7 @@ class OrderAvailablePaymentMethods
       distributor.payment_methods.where(
         id: available_distributor_payment_methods_ids
       )
-    end.available.select(&:configured?)
+    end.available.select(&:configured?).uniq
   end
 
   def available_distributor_payment_methods_ids

app/services/order_available_shipping_methods.rb

@@ -14,25 +14,32 @@ class OrderAvailableShippingMethods
   def to_a
     return [] if distributor.blank?
 
-    shipping_methods = shipping_methods_before_tag_rules_applied
-
-    applicator = OpenFoodNetwork::TagRuleApplicator.new(distributor,
-                                                        "FilterShippingMethods", customer&.tag_list)
-    applicator.filter!(shipping_methods)
-
-    shipping_methods.uniq
+    filter_by_category(tag_rules.filter(shipping_methods))
   end
 
   private
 
-  def shipping_methods_before_tag_rules_applied
+  def filter_by_category(methods)
+    return methods unless OpenFoodNetwork::FeatureToggle.enabled?(:match_shipping_categories,
+                                                                  distributor&.owner)
+
+    required_category_ids = order.products.pluck(:shipping_category_id).to_set
+    return methods if required_category_ids.empty?
+
+    methods.select do |method|
+      provided_category_ids = method.shipping_categories.pluck(:id).to_set
+      required_category_ids.subset?(provided_category_ids)
+    end
+  end
+
+  def shipping_methods
     if order_cycle.nil? || order_cycle.simple?
       distributor.shipping_methods
     else
       distributor.shipping_methods.where(
         id: available_distributor_shipping_methods_ids
       )
-    end.frontend.to_a
+    end.frontend.to_a.uniq
   end
 
   def available_distributor_shipping_methods_ids
@@ -40,4 +47,10 @@ class OrderAvailableShippingMethods
       .where(distributor_id: distributor.id)
       .select(:shipping_method_id)
   end
+
+  def tag_rules
+    OpenFoodNetwork::TagRuleApplicator.new(
+      distributor, "FilterShippingMethods", customer&.tag_list
+    )
+  end
 end

app/services/shop/order_cycles_list.rb

@@ -29,15 +29,11 @@ module Shop
 
     private
 
-    # order_cycles is a ActiveRecord::Relation that is modified with reject in the TagRuleApplicator
-    # If this relation is reloaded (for example by calling count on it), the modifications are lost
     def apply_tag_rules!(order_cycles)
       applicator = OpenFoodNetwork::TagRuleApplicator.new(@distributor,
                                                           "FilterOrderCycles",
                                                           @customer&.tag_list)
-      applicator.filter!(order_cycles)
-
-      order_cycles
+      applicator.filter(order_cycles)
     end
   end
 end

lib/open_food_network/tag_rule_applicator.rb

@@ -13,10 +13,8 @@ module OpenFoodNetwork
       @customer_tags = customer_tags || []
     end
 
-    def filter!(subject)
-      return unless subject.respond_to?(:any?) && subject.any?
-
-      subject.to_a.reject! do |element|
+    def filter(subject)
+      subject.to_a.reject do |element|
         if rule_class.respond_to?(:tagged_children_for)
           children = rule_class.tagged_children_for(element)
           children.reject! { |child| reject?(child) }

spec/lib/open_food_network/tag_rule_applicator_spec.rb

@@ -109,30 +109,20 @@ module OpenFoodNetwork
       end
     end
 
-    describe "filter!" do
+    describe "filter" do
       let(:applicator) { OpenFoodNetwork::TagRuleApplicator.new(enterprise, "FilterProducts", []) }
 
-      context "when the subject is nil" do
-        let(:subject) { double(:subject, reject!: false) }
-
-        it "returns immediately" do
-          applicator.filter!(subject)
-          expect(subject).to_not have_received(:reject!)
-        end
+      it "handles nil" do
+        applicator.filter(nil)
       end
 
-      context "when subject is empty" do
-        let(:subject) { double(:subject, reject!: false) }
-
-        it "returns immediately" do
-          applicator.filter!(subject)
-          expect(subject).to_not have_received(:reject!)
-        end
+      it "handles an empty collection" do
+        applicator.filter(OrderCycle.none)
       end
 
-      context "when subject is an array" do
+      context "when filtering an array" do
         let(:element) { double(:element, ) }
-        let(:subject) { [element] }
+        let(:list) { [element] }
 
         context "when rule_class reponds to tagged_children_for" do
           let(:child1) { double(:child) }
@@ -146,15 +136,16 @@ module OpenFoodNetwork
             before do
               allow(applicator).to receive(:reject?).with(child1) { true }
               allow(applicator).to receive(:reject?).with(child2) { false }
-              applicator.filter!(subject)
             end
 
             it "rejects the specified children from the array" do
+              applicator.filter(list)
               expect(children).to eq [child2]
             end
 
-            it "does not remove the element from the original subject" do
-              expect(subject).to eq [element]
+            it "does not remove the element from the result" do
+              result = applicator.filter(list)
+              expect(result).to eq [element]
             end
           end
 
@@ -162,15 +153,16 @@ module OpenFoodNetwork
             before do
               allow(applicator).to receive(:reject?).with(child1) { true }
               allow(applicator).to receive(:reject?).with(child2) { true }
-              applicator.filter!(subject)
             end
 
             it "removes all children from the array" do
+              applicator.filter(list)
               expect(children).to eq []
             end
 
-            it "removes the element from the original subject" do
-              expect(subject).to eq []
+            it "removes the element from the result" do
+              result = applicator.filter(list)
+              expect(result).to eq []
             end
           end
         end
@@ -183,22 +175,22 @@ module OpenFoodNetwork
           context "when reject? returns false for the element" do
             before do
               allow(applicator).to receive(:reject?).with(element) { false }
-              applicator.filter!(subject)
             end
 
-            it "does not remove the element from the original subject" do
-              expect(subject).to eq [element]
+            it "does not remove the element from the result" do
+              result = applicator.filter(list)
+              expect(result).to eq [element]
             end
           end
 
           context "when reject? returns true for the element" do
             before do
               allow(applicator).to receive(:reject?).with(element) { true }
-              applicator.filter!(subject)
             end
 
-            it "removes the element from the original subject" do
-              expect(subject).to eq []
+            it "removes the element from the result" do
+              result = applicator.filter(list)
+              expect(result).to eq []
             end
           end
         end
@@ -268,8 +260,8 @@ module OpenFoodNetwork
         }
 
         it "applies the default rule" do
-          applicator.filter!(products_array)
-          expect(products_array).to eq [
+          result = applicator.filter(products_array)
+          expect(result).to eq [
             { :id => 2, :name => 'product 2',
               "variants" => [{ :id => 9, "tag_list" => ["tag2"] }] }, product3
           ]
@@ -283,8 +275,8 @@ module OpenFoodNetwork
 
         it "applies those rules" do
           # product_tag_rule1 and product_tag_rule2 are being applied
-          applicator.filter!(products_array)
-          expect(products_array).to eq [product1, product2]
+          result = applicator.filter(products_array)
+          expect(result).to eq [product1, product2]
         end
       end
     end

spec/services/order_available_shipping_methods_spec.rb

@@ -214,4 +214,44 @@ describe OrderAvailableShippingMethods do
       end
     end
   end
+
+  context "when certain shipping categories are required" do
+    subject { OrderAvailableShippingMethods.new(order) }
+    let(:order) {
+      build(:order, distributor: distributor, order_cycle: oc)
+    }
+    let(:oc) { create(:order_cycle) }
+    let(:distributor) { oc.distributors.first }
+    let(:standard_shipping) {
+      create(:shipping_method, distributors: [distributor], shipping_categories: [bike_transport])
+    }
+    let(:cooled_shipping) {
+      create(:shipping_method, distributors: [distributor], shipping_categories: [refrigerated])
+    }
+    let(:bike_transport) { Spree::ShippingCategory.new(name: "bike") }
+    let(:refrigerated) { Spree::ShippingCategory.new(name: "fridge") }
+
+    before {
+      standard_shipping
+      cooled_shipping
+
+      Flipper.enable(:match_shipping_categories)
+    }
+
+    it "provides all shipping methods for an empty order" do
+      expect(subject.to_a).to match_array [standard_shipping, cooled_shipping]
+    end
+
+    it "provides all shipping methods for normal products" do
+      order.line_items << build(:line_item)
+      expect(subject.to_a).to match_array [standard_shipping, cooled_shipping]
+    end
+
+    it "filters shipping methods for products needing refrigeration" do
+      product = oc.products.first
+      product.update!(shipping_category: refrigerated)
+      order.line_items << build(:line_item, variant: product.variants.first)
+      expect(subject.to_a).to eq [cooled_shipping]
+    end
+  end
 end