diff --git a/app/services/order_available_payment_methods.rb b/app/services/order_available_payment_methods.rb index c3f71cc1da..dc0784d909 100644 --- a/app/services/order_available_payment_methods.rb +++ b/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 diff --git a/app/services/order_available_shipping_methods.rb b/app/services/order_available_shipping_methods.rb index 9751ee2d64..95603f9a68 100644 --- a/app/services/order_available_shipping_methods.rb +++ b/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 diff --git a/app/services/shop/order_cycles_list.rb b/app/services/shop/order_cycles_list.rb index 6203637372..2328b0b0b7 100644 --- a/app/services/shop/order_cycles_list.rb +++ b/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 diff --git a/lib/open_food_network/tag_rule_applicator.rb b/lib/open_food_network/tag_rule_applicator.rb index 09060a8ecf..d37d485e1d 100644 --- a/lib/open_food_network/tag_rule_applicator.rb +++ b/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) } diff --git a/spec/lib/open_food_network/tag_rule_applicator_spec.rb b/spec/lib/open_food_network/tag_rule_applicator_spec.rb index 82d7fac903..7ec0e29f2a 100644 --- a/spec/lib/open_food_network/tag_rule_applicator_spec.rb +++ b/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 diff --git a/spec/services/order_available_shipping_methods_spec.rb b/spec/services/order_available_shipping_methods_spec.rb index dc4c80134d..315f2397c4 100644 --- a/spec/services/order_available_shipping_methods_spec.rb +++ b/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