Reduce cognitive complexity of VariantsController#search

app/controllers/spree/admin/variants_controller_decorator.rb

@@ -1,29 +1,11 @@
+require 'open_food_network/scope_variants_for_search'
+
 Spree::Admin::VariantsController.class_eval do
   helper 'spree/products'
 
   def search
-    search_params = { :product_name_cont => params[:q], :sku_cont => params[:q] }
-
-    @variants = Spree::Variant.where(is_master: false).ransack(search_params.merge(:m => 'or')).result
-
-    if params[:schedule_id].present?
-      schedule = Schedule.find params[:schedule_id]
-      @variants = @variants.in_schedule(schedule)
-    end
-
-    if params[:order_cycle_id].present?
-      order_cycle = OrderCycle.find params[:order_cycle_id]
-      @variants = @variants.in_order_cycle(order_cycle)
-    end
-
-    if params[:distributor_id].present?
-      distributor = Enterprise.find params[:distributor_id]
-      @variants = @variants.in_distributor(distributor)
-      scoper = OpenFoodNetwork::ScopeVariantToHub.new(distributor)
-      # Perform scoping after all filtering is done.
-      # Filtering could be a problem on scoped variants.
-      @variants.each { |v| scoper.scope(v) }
-    end
+    scoper = OpenFoodNetwork::ScopeVariantsForSearch.new(params)
+    @variants = scoper.search
   end
 
   def destroy

app/models/spree/variant_decorator.rb

@@ -43,7 +43,7 @@ Spree::Variant.class_eval do
   }
 
   scope :in_schedule, lambda { |schedule|
-    joins(exchanges: { order_cycle: :schedule}).
+    joins(exchanges: { order_cycle: :schedules}).
       merge(Exchange.outgoing).
       where(schedules: { id: schedule}).
       select('DISTINCT spree_variants.*')

lib/open_food_network/scope_variants_for_search.rb

@@ -0,0 +1,51 @@
+# Used to return a set of variants which match the criteria provided
+# A query string is required, which will be match to the name and/or SKU of a product
+# Further restrictions on the schedule, order_cycle or distributor through which the
+# products are available are also possible
+
+module OpenFoodNetwork
+  class ScopeVariantsForSearch
+    def initialize(params)
+      @params = params
+    end
+
+    def search
+      @variants = query_scope
+
+      scope_to_schedule if params[:schedule_id]
+      scope_to_order_cycle if params[:order_cycle_id]
+      scope_to_distributor if params[:distributor_id]
+
+      @variants
+    end
+
+    private
+
+    attr_reader :params
+
+    def search_params
+      { :product_name_cont => params[:q], :sku_cont => params[:q] }
+    end
+
+    def query_scope
+      Spree::Variant.where(is_master: false).ransack(search_params.merge(:m => 'or')).result
+    end
+
+    def scope_to_schedule
+      @variants = @variants.in_schedule(params[:schedule_id])
+    end
+
+    def scope_to_order_cycle
+      @variants = @variants.in_order_cycle(params[:order_cycle_id])
+    end
+
+    def scope_to_distributor
+      distributor = Enterprise.find params[:distributor_id]
+      @variants = @variants.in_distributor(distributor)
+      scoper = OpenFoodNetwork::ScopeVariantToHub.new(distributor)
+      # Perform scoping after all filtering is done.
+      # Filtering could be a problem on scoped variants.
+      @variants.each { |v| scoper.scope(v) }
+    end
+  end
+end

spec/lib/open_food_network/scope_variants_to_search_spec.rb

@@ -0,0 +1,62 @@
+require 'open_food_network/scope_variants_for_search'
+
+describe OpenFoodNetwork::ScopeVariantsForSearch do
+  let!(:p1) { create(:simple_product, name: 'Product 1') }
+  let!(:p2) { create(:simple_product, sku: 'Product 1a') }
+  let!(:p3) { create(:simple_product, name: 'Product 3') }
+  let!(:p4) { create(:simple_product, name: 'Product 4') }
+  let!(:v1) { p1.variants.first }
+  let!(:v2) { p2.variants.first }
+  let!(:v3) { p3.variants.first }
+  let!(:v4) { p4.variants.first }
+  let!(:d1)  { create(:distributor_enterprise) }
+  let!(:d2)  { create(:distributor_enterprise) }
+  let!(:oc1) { create(:simple_order_cycle, distributors: [d1], variants: [v1, v3]) }
+  let!(:oc2) { create(:simple_order_cycle, distributors: [d1], variants: [v2]) }
+  let!(:oc3) { create(:simple_order_cycle, distributors: [d2], variants: [v4]) }
+  let!(:s1) { create(:schedule, order_cycles: [oc1]) }
+  let!(:s2) { create(:schedule, order_cycles: [oc2]) }
+
+  let(:scoper) { OpenFoodNetwork::ScopeVariantsForSearch.new(params) }
+
+  describe "search" do
+    let(:result) { scoper.search }
+
+    context "when a search query is provided" do
+      let(:params) { { q: "product 1" } }
+
+      it "returns all products whose names or SKUs match the query" do
+        expect(result).to include v1, v2
+        expect(result).to_not include v3, v4
+      end
+    end
+
+    context "when a schedule_id is specified" do
+      let(:params) { { q: "product", schedule_id: s1.id } }
+
+      it "returns all products distributed through that schedule" do
+        lala = result
+        expect(lala).to include v1, v3
+        expect(result).to_not include v2, v4
+      end
+    end
+
+    context "when an order_cycle_id is specified" do
+      let(:params) { { q: "product", order_cycle_id: oc2.id } }
+
+      it "returns all products distributed through that order cycle" do
+        expect(result).to include v2
+        expect(result).to_not include v1, v3, v4
+      end
+    end
+
+    context "when a distributor_id is specified" do
+      let(:params) { { q: "product", distributor_id: d2.id } }
+
+      it "returns all products distributed through that distributor" do
+        expect(result).to include v4
+        expect(result).to_not include v1, v2, v3
+      end
+    end
+  end
+end