From bbfd35c861b1fc4280237ce39f3e8fbf0a8b700a Mon Sep 17 00:00:00 2001
From: Pau Perez <saulopefa@gmail.com>
Date: Wed, 6 Mar 2019 10:45:45 +0100
Subject: [PATCH 1/6] Fix CacheSettingsController violations

---
 .rubocop_manual_todo.yml                      |  1 -
 .rubocop_todo.yml                             |  3 --
 .../admin/cache_settings_controller.rb        | 29 ++++++++++++-------
 3 files changed, 19 insertions(+), 14 deletions(-)

diff --git a/.rubocop_manual_todo.yml b/.rubocop_manual_todo.yml
index 71812eb60a..098b6b6c90 100644
--- a/.rubocop_manual_todo.yml
+++ b/.rubocop_manual_todo.yml
@@ -25,7 +25,6 @@ Metrics/LineLength:
   - app/controllers/admin/accounts_and_billing_settings_controller.rb
   - app/controllers/admin/bulk_line_items_controller.rb
   - app/controllers/admin/business_model_configuration_controller.rb
-  - app/controllers/admin/cache_settings_controller.rb
   - app/controllers/admin/contents_controller.rb
   - app/controllers/admin/customers_controller.rb
   - app/controllers/admin/enterprise_fees_controller.rb
diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml
index a856678c9a..9905760a71 100644
--- a/.rubocop_todo.yml
+++ b/.rubocop_todo.yml
@@ -310,7 +310,6 @@ Layout/EmptyLinesAroundBlockBody:
 Layout/EmptyLinesAroundClassBody:
   Exclude:
     - 'app/controllers/admin/account_controller.rb'
-    - 'app/controllers/admin/cache_settings_controller.rb'
     - 'app/controllers/admin/enterprise_fees_controller.rb'
     - 'app/controllers/admin/inventory_items_controller.rb'
     - 'app/controllers/admin/tag_rules_controller.rb'
@@ -763,7 +762,6 @@ Layout/SpaceInsideBlockBraces:
 # SupportedStylesForEmptyBraces: space, no_space
 Layout/SpaceInsideHashLiteralBraces:
   Exclude:
-    - 'app/controllers/admin/cache_settings_controller.rb'
     - 'app/controllers/admin/enterprise_relationships_controller.rb'
     - 'app/controllers/admin/enterprise_roles_controller.rb'
     - 'app/controllers/api/statuses_controller.rb'
@@ -1601,7 +1599,6 @@ Style/ClassAndModuleChildren:
     - 'app/controllers/admin/account_controller.rb'
     - 'app/controllers/admin/accounts_and_billing_settings_controller.rb'
     - 'app/controllers/admin/business_model_configuration_controller.rb'
-    - 'app/controllers/admin/cache_settings_controller.rb'
     - 'app/controllers/spree/store_controller_decorator.rb'
     - 'app/helpers/angular_form_helper.rb'
     - 'app/models/calculator/flat_percent_per_item.rb'
diff --git a/app/controllers/admin/cache_settings_controller.rb b/app/controllers/admin/cache_settings_controller.rb
index 1074f13e9f..e922031f68 100644
--- a/app/controllers/admin/cache_settings_controller.rb
+++ b/app/controllers/admin/cache_settings_controller.rb
@@ -1,18 +1,27 @@
 require 'open_food_network/products_cache_integrity_checker'
 
-class Admin::CacheSettingsController < Spree::Admin::BaseController
-  def edit
-    @results = Exchange.cachable.map do |exchange|
-      checker = OpenFoodNetwork::ProductsCacheIntegrityChecker.new(exchange.receiver, exchange.order_cycle)
+module Admin
+  class CacheSettingsController < Spree::Admin::BaseController
+    def edit
+      @results = Exchange.cachable.map do |exchange|
+        checker = OpenFoodNetwork::ProductsCacheIntegrityChecker
+          .new(exchange.receiver, exchange.order_cycle)
 
-      {distributor: exchange.receiver, order_cycle: exchange.order_cycle, status: checker.ok?, diff: checker.diff}
+        {
+          distributor: exchange.receiver,
+          order_cycle: exchange.order_cycle,
+          status: checker.ok?,
+          diff: checker.diff
+        }
+      end
     end
-  end
 
-  def update
-    Spree::Config.set(params[:preferences])
-    respond_to do |format|
-      format.html { redirect_to main_app.edit_admin_cache_settings_path }
+    def update
+      Spree::Config.set(params[:preferences])
+
+      respond_to do |format|
+        format.html { redirect_to main_app.edit_admin_cache_settings_path }
+      end
     end
   end
 end

From 2b58aab197e7a67549f59ba744014b03aa2a65ae Mon Sep 17 00:00:00 2001
From: Pau Perez <saulopefa@gmail.com>
Date: Wed, 6 Mar 2019 10:49:56 +0100
Subject: [PATCH 2/6] Extract method from cache key generation

---
 app/jobs/refresh_products_cache_job.rb | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/app/jobs/refresh_products_cache_job.rb b/app/jobs/refresh_products_cache_job.rb
index 0c66925be8..2662237dc1 100644
--- a/app/jobs/refresh_products_cache_job.rb
+++ b/app/jobs/refresh_products_cache_job.rb
@@ -2,12 +2,15 @@ require 'open_food_network/products_renderer'
 
 RefreshProductsCacheJob = Struct.new(:distributor_id, :order_cycle_id) do
   def perform
-    Rails.cache.write "products-json-#{distributor_id}-#{order_cycle_id}", products_json
+    Rails.cache.write(key, products_json)
   end
 
-
   private
 
+  def key
+    "products-json-#{distributor_id}-#{order_cycle_id}"
+  end
+
   def products_json
     distributor = Enterprise.find distributor_id
     order_cycle = OrderCycle.find order_cycle_id

From 7c533c63471e8a6e6db76fbc8c9b73341b708611 Mon Sep 17 00:00:00 2001
From: Pau Perez <saulopefa@gmail.com>
Date: Wed, 6 Mar 2019 11:17:44 +0100
Subject: [PATCH 3/6] Extract DistributedValidProducts from OrderCycle

---
 app/models/distributed_valid_products.rb      | 35 +++++++++++
 app/models/order_cycle.rb                     | 17 +----
 .../models/distributed_valid_products_spec.rb | 63 +++++++++++++++++++
 spec/models/order_cycle_spec.rb               | 54 ----------------
 4 files changed, 99 insertions(+), 70 deletions(-)
 create mode 100644 app/models/distributed_valid_products.rb
 create mode 100644 spec/models/distributed_valid_products_spec.rb

diff --git a/app/models/distributed_valid_products.rb b/app/models/distributed_valid_products.rb
new file mode 100644
index 0000000000..2151320b91
--- /dev/null
+++ b/app/models/distributed_valid_products.rb
@@ -0,0 +1,35 @@
+# Finds valid products distributed by a particular distributor in an order cycle
+class DistributedValidProducts
+  def initialize(order_cycle, distributor)
+    @order_cycle = order_cycle
+    @distributor = distributor
+  end
+
+  def all
+    variants = order_cycle.variants_distributed_by(distributor)
+    products = variants.map(&:product).uniq
+
+    valid_products = products.reject do |product|
+      product_has_only_obsolete_master_in_distribution?(product, variants)
+    end
+    product_ids = valid_products.map(&:id)
+
+    Spree::Product.where(id: product_ids)
+  end
+
+  private
+
+  attr_reader :order_cycle, :distributor
+
+  # If a product without variants is added to an order cycle, and then some variants are added
+  # to that product, but not the order cycle, then the master variant should not available for
+  # customers to purchase.
+  #
+  # This method is used by #valid_products_distributed_by to filter out such products so that
+  # the customer cannot purchase them.
+  def product_has_only_obsolete_master_in_distribution?(product, distributed_variants)
+    product.has_variants? &&
+      distributed_variants.include?(product.master) &&
+      (product.variants & distributed_variants).empty?
+  end
+end
diff --git a/app/models/order_cycle.rb b/app/models/order_cycle.rb
index ac9b7d8cb8..7122d9cee7 100644
--- a/app/models/order_cycle.rb
+++ b/app/models/order_cycle.rb
@@ -156,7 +156,6 @@ class OrderCycle < ActiveRecord::Base
   end
 
   def distributed_variants
-    # TODO: only used in DistributionChangeValidator, can we remove?
     self.exchanges.outgoing.map(&:variants).flatten.uniq.reject(&:deleted?)
   end
 
@@ -180,10 +179,7 @@ class OrderCycle < ActiveRecord::Base
   # to purchase.
   # This method filters out such products so that the customer cannot purchase them.
   def valid_products_distributed_by(distributor)
-    variants = variants_distributed_by(distributor)
-    products = variants.map(&:product).uniq
-    product_ids = products.reject{ |p| product_has_only_obsolete_master_in_distribution?(p, variants) }.map(&:id)
-    Spree::Product.where(id: product_ids)
+    DistributedValidProducts.new(self, distributor).all
   end
 
   def products
@@ -265,17 +261,6 @@ class OrderCycle < ActiveRecord::Base
 
   private
 
-  # If a product without variants is added to an order cycle, and then some variants are added
-  # to that product, but not the order cycle, then the master variant should not available for customers
-  # to purchase.
-  # This method is used by #valid_products_distributed_by to filter out such products so that
-  # the customer cannot purchase them.
-  def product_has_only_obsolete_master_in_distribution?(product, distributed_variants)
-    product.has_variants? &&
-      distributed_variants.include?(product.master) &&
-      (product.variants & distributed_variants).empty?
-  end
-
   def orders_close_at_after_orders_open_at?
     return if orders_open_at.blank? || orders_close_at.blank?
     return if orders_close_at > orders_open_at
diff --git a/spec/models/distributed_valid_products_spec.rb b/spec/models/distributed_valid_products_spec.rb
new file mode 100644
index 0000000000..1aa06d20a2
--- /dev/null
+++ b/spec/models/distributed_valid_products_spec.rb
@@ -0,0 +1,63 @@
+require 'spec_helper'
+
+describe DistributedValidProducts do
+  it "returns valid products but not invalid products" do
+    p_valid = create(:product)
+    p_invalid = create(:product)
+    v_valid = p_valid.variants.first
+    v_invalid = p_invalid.variants.first
+
+    d = create(:distributor_enterprise)
+    oc = create(:simple_order_cycle, distributors: [d], variants: [v_valid, p_invalid.master])
+
+    expect(oc.valid_products_distributed_by(d)).to eq([p_valid])
+  end
+
+  describe "checking if a product has only an obsolete master variant in a distributution" do
+    it "returns true when so" do
+      master = double(:master)
+      unassociated_variant = double(:variant)
+      product = double(:product, has_variants?: true, master: master, variants: [])
+      distributed_variants = [master, unassociated_variant]
+
+      oc = OrderCycle.new
+      distributed_valid_products = described_class.new(oc, nil)
+
+      expect(distributed_valid_products.send(:product_has_only_obsolete_master_in_distribution?, product, distributed_variants)).to be true
+    end
+
+    it "returns false when the product doesn't have variants" do
+      master = double(:master)
+      product = double(:product, has_variants?: false, master: master, variants: [])
+      distributed_variants = [master]
+
+      oc = OrderCycle.new
+      distributed_valid_products = described_class.new(oc, nil)
+
+      expect(distributed_valid_products.send(:product_has_only_obsolete_master_in_distribution?, product, distributed_variants)).to be false
+    end
+
+    it "returns false when the master isn't distributed" do
+      master = double(:master)
+      product = double(:product, has_variants?: true, master: master, variants: [])
+      distributed_variants = []
+
+      oc = OrderCycle.new
+      distributed_valid_products = described_class.new(oc, nil)
+
+      expect(distributed_valid_products.send(:product_has_only_obsolete_master_in_distribution?, product, distributed_variants)).to be false
+    end
+
+    it "returns false when the product has other variants distributed" do
+      master = double(:master)
+      variant = double(:variant)
+      product = double(:product, has_variants?: true, master: master, variants: [variant])
+      distributed_variants = [master, variant]
+
+      oc = OrderCycle.new
+      distributed_valid_products = described_class.new(oc, nil)
+
+      expect(distributed_valid_products.send(:product_has_only_obsolete_master_in_distribution?, product, distributed_variants)).to be false
+    end
+  end
+end
diff --git a/spec/models/order_cycle_spec.rb b/spec/models/order_cycle_spec.rb
index 3ffd650ddd..3c17fba457 100644
--- a/spec/models/order_cycle_spec.rb
+++ b/spec/models/order_cycle_spec.rb
@@ -283,60 +283,6 @@ describe OrderCycle do
     end
   end
 
-  describe "finding valid products distributed by a particular distributor" do
-    it "returns valid products but not invalid products" do
-      p_valid = create(:product)
-      p_invalid = create(:product)
-      v_valid = p_valid.variants.first
-      v_invalid = p_invalid.variants.first
-
-      d = create(:distributor_enterprise)
-      oc = create(:simple_order_cycle, distributors: [d], variants: [v_valid, p_invalid.master])
-
-      oc.valid_products_distributed_by(d).should == [p_valid]
-    end
-
-    describe "checking if a product has only an obsolete master variant in a distributution" do
-      it "returns true when so" do
-        master = double(:master)
-        unassociated_variant = double(:variant)
-        product = double(:product, :has_variants? => true, :master => master, :variants => [])
-        distributed_variants = [master, unassociated_variant]
-
-        oc = OrderCycle.new
-        oc.send(:product_has_only_obsolete_master_in_distribution?, product, distributed_variants).should be true
-      end
-
-      it "returns false when the product doesn't have variants" do
-        master = double(:master)
-        product = double(:product, :has_variants? => false, :master => master, :variants => [])
-        distributed_variants = [master]
-
-        oc = OrderCycle.new
-        oc.send(:product_has_only_obsolete_master_in_distribution?, product, distributed_variants).should be false
-      end
-
-      it "returns false when the master isn't distributed" do
-        master = double(:master)
-        product = double(:product, :has_variants? => true, :master => master, :variants => [])
-        distributed_variants = []
-
-        oc = OrderCycle.new
-        oc.send(:product_has_only_obsolete_master_in_distribution?, product, distributed_variants).should be false
-      end
-
-      it "returns false when the product has other variants distributed" do
-        master = double(:master)
-        variant = double(:variant)
-        product = double(:product, :has_variants? => true, :master => master, :variants => [variant])
-        distributed_variants = [master, variant]
-
-        oc = OrderCycle.new
-        oc.send(:product_has_only_obsolete_master_in_distribution?, product, distributed_variants).should be false
-      end
-    end
-  end
-
   describe "exchanges" do
     before(:each) do
       @oc = create(:simple_order_cycle)

From adb40d6c73b06c5ce1487b66a03400486ed62c9b Mon Sep 17 00:00:00 2001
From: Pau Perez <saulopefa@gmail.com>
Date: Wed, 6 Mar 2019 11:36:41 +0100
Subject: [PATCH 4/6] Test just the public method

This one is the one calling the private method that was tested now. It
involves some refactoring of the specs as well.
---
 .../models/distributed_valid_products_spec.rb | 110 +++++++++++-------
 1 file changed, 70 insertions(+), 40 deletions(-)

diff --git a/spec/models/distributed_valid_products_spec.rb b/spec/models/distributed_valid_products_spec.rb
index 1aa06d20a2..b2c50ef022 100644
--- a/spec/models/distributed_valid_products_spec.rb
+++ b/spec/models/distributed_valid_products_spec.rb
@@ -1,63 +1,93 @@
 require 'spec_helper'
 
 describe DistributedValidProducts do
-  it "returns valid products but not invalid products" do
-    p_valid = create(:product)
-    p_invalid = create(:product)
-    v_valid = p_valid.variants.first
-    v_invalid = p_invalid.variants.first
+  let(:order_cycle) { OrderCycle.new }
+  let(:distributor) { instance_double(Enterprise) }
 
-    d = create(:distributor_enterprise)
-    oc = create(:simple_order_cycle, distributors: [d], variants: [v_valid, p_invalid.master])
+  it 'returns valid products but not invalid products' do
+    valid_product = create(:product)
+    invalid_product = create(:product)
+    valid_variant = valid_product.variants.first
 
-    expect(oc.valid_products_distributed_by(d)).to eq([p_valid])
+    distributor = create(:distributor_enterprise)
+    order_cycle = create(
+      :simple_order_cycle,
+      distributors: [distributor],
+      variants: [valid_variant, invalid_product.master]
+    )
+
+    distributed_valid_products = described_class.new(order_cycle, distributor)
+
+    expect(distributed_valid_products.all).to eq([valid_product])
   end
 
-  describe "checking if a product has only an obsolete master variant in a distributution" do
-    it "returns true when so" do
-      master = double(:master)
-      unassociated_variant = double(:variant)
-      product = double(:product, has_variants?: true, master: master, variants: [])
-      distributed_variants = [master, unassociated_variant]
+  context 'when the product has only an obsolete master variant in a distribution' do
+    let(:master) { create(:variant, product: product) }
+    let(:product) { create(:product, variants: [build(:variant)]) }
+    let(:unassociated_variant) { create(:variant) }
+    let(:distributed_variants) { [product.master, unassociated_variant] }
 
-      oc = OrderCycle.new
-      distributed_valid_products = described_class.new(oc, nil)
-
-      expect(distributed_valid_products.send(:product_has_only_obsolete_master_in_distribution?, product, distributed_variants)).to be true
+    before do
+      allow(order_cycle)
+        .to receive(:variants_distributed_by).with(distributor) { distributed_variants }
     end
 
-    it "returns false when the product doesn't have variants" do
-      master = double(:master)
-      product = double(:product, has_variants?: false, master: master, variants: [])
-      distributed_variants = [master]
+    it 'does not return the obsolete product' do
+      distributed_valid_products = described_class.new(order_cycle, distributor)
+      expect(distributed_valid_products.all).to eq([unassociated_variant.product])
+    end
+  end
 
-      oc = OrderCycle.new
-      distributed_valid_products = described_class.new(oc, nil)
+  context "when the product doesn't have variants" do
+    let(:master) { build(:variant) }
+    let(:product) { create(:product, master: master) }
+    let(:distributed_variants) { [master] }
 
-      expect(distributed_valid_products.send(:product_has_only_obsolete_master_in_distribution?, product, distributed_variants)).to be false
+    before do
+      allow(product).to receive(:has_variants?) { false }
+      allow(order_cycle)
+        .to receive(:variants_distributed_by).with(distributor) { distributed_variants }
     end
 
-    it "returns false when the master isn't distributed" do
-      master = double(:master)
-      product = double(:product, has_variants?: true, master: master, variants: [])
-      distributed_variants = []
+    it 'returns the product' do
+      distributed_valid_products = described_class.new(order_cycle, distributor)
+      expect(distributed_valid_products.all).to eq([product])
+    end
+  end
 
-      oc = OrderCycle.new
-      distributed_valid_products = described_class.new(oc, nil)
+  context "when the master isn't distributed" do
+    let(:master) { build(:variant) }
+    let(:variant) { build(:variant) }
+    let(:product) { create(:product, master: master, variants: [variant]) }
+    let(:distributed_variants) { [variant] }
 
-      expect(distributed_valid_products.send(:product_has_only_obsolete_master_in_distribution?, product, distributed_variants)).to be false
+    before do
+      allow(product).to receive(:has_variants?) { true }
+      allow(order_cycle)
+        .to receive(:variants_distributed_by).with(distributor) { distributed_variants }
     end
 
-    it "returns false when the product has other variants distributed" do
-      master = double(:master)
-      variant = double(:variant)
-      product = double(:product, has_variants?: true, master: master, variants: [variant])
-      distributed_variants = [master, variant]
+    it 'returns the product' do
+      distributed_valid_products = described_class.new(order_cycle, distributor)
+      expect(distributed_valid_products.all).to eq([product])
+    end
+  end
 
-      oc = OrderCycle.new
-      distributed_valid_products = described_class.new(oc, nil)
+  context 'when the product has the master and other variants distributed' do
+    let(:master) { build(:variant) }
+    let(:variant) { build(:variant) }
+    let(:product) { create(:product, master: master, variants: [variant]) }
+    let(:distributed_variants) { [master, variant] }
 
-      expect(distributed_valid_products.send(:product_has_only_obsolete_master_in_distribution?, product, distributed_variants)).to be false
+    before do
+      allow(product).to receive(:has_variants?) { true }
+      allow(order_cycle)
+        .to receive(:variants_distributed_by).with(distributor) { distributed_variants }
+    end
+
+    it 'returns the product' do
+      distributed_valid_products = described_class.new(order_cycle, distributor)
+      expect(distributed_valid_products.all).to eq([product])
     end
   end
 end

From 347aa3c4ae6f64c528c59a652856121ab9b105b7 Mon Sep 17 00:00:00 2001
From: Pau Perez <saulopefa@gmail.com>
Date: Wed, 6 Mar 2019 13:49:53 +0100
Subject: [PATCH 5/6] Replace #valid_products_distributed_by by class

---
 .rubocop_todo.yml                              |  1 -
 app/models/distributed_valid_products.rb       | 14 ++++++++++----
 app/models/order_cycle.rb                      |  9 ---------
 lib/open_food_network/products_renderer.rb     | 17 +++++++++--------
 spec/models/distributed_valid_products_spec.rb | 10 +++++-----
 5 files changed, 24 insertions(+), 27 deletions(-)

diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml
index 9905760a71..d8a8fcb5bd 100644
--- a/.rubocop_todo.yml
+++ b/.rubocop_todo.yml
@@ -151,7 +151,6 @@ Layout/EmptyLines:
     - 'app/models/exchange.rb'
     - 'app/models/exchange_fee.rb'
     - 'app/models/inventory_item.rb'
-    - 'app/models/order_cycle.rb'
     - 'app/models/producer_property.rb'
     - 'app/models/product_distribution.rb'
     - 'app/models/spree/calculator_decorator.rb'
diff --git a/app/models/distributed_valid_products.rb b/app/models/distributed_valid_products.rb
index 2151320b91..af411e0748 100644
--- a/app/models/distributed_valid_products.rb
+++ b/app/models/distributed_valid_products.rb
@@ -1,11 +1,20 @@
 # Finds valid products distributed by a particular distributor in an order cycle
+#
+# If a product without variants is added to an order cycle, and then some
+# variants are added to that product, but not the order cycle, then the master
+# variant should not available for customers to purchase. This class filters
+# out such products so that the customer cannot purchase them.
 class DistributedValidProducts
   def initialize(order_cycle, distributor)
     @order_cycle = order_cycle
     @distributor = distributor
   end
 
-  def all
+  # Returns an ActiveRecord relation without invalid products. Check
+  # #valid_products_distributed_by for details
+  #
+  # @return [ActiveRecord::Relation<Spree::Product>]
+  def relation
     variants = order_cycle.variants_distributed_by(distributor)
     products = variants.map(&:product).uniq
 
@@ -24,9 +33,6 @@ class DistributedValidProducts
   # If a product without variants is added to an order cycle, and then some variants are added
   # to that product, but not the order cycle, then the master variant should not available for
   # customers to purchase.
-  #
-  # This method is used by #valid_products_distributed_by to filter out such products so that
-  # the customer cannot purchase them.
   def product_has_only_obsolete_master_in_distribution?(product, distributed_variants)
     product.has_variants? &&
       distributed_variants.include?(product.master) &&
diff --git a/app/models/order_cycle.rb b/app/models/order_cycle.rb
index 7122d9cee7..59547ee086 100644
--- a/app/models/order_cycle.rb
+++ b/app/models/order_cycle.rb
@@ -50,7 +50,6 @@ class OrderCycle < ActiveRecord::Base
     joins(:exchanges).merge(Exchange.outgoing).merge(Exchange.to_enterprise(distributor))
   }
 
-
   scope :managed_by, lambda { |user|
     if user.has_spree_role?('admin')
       scoped
@@ -174,14 +173,6 @@ class OrderCycle < ActiveRecord::Base
     variants_distributed_by(distributor).map(&:product).uniq
   end
 
-  # If a product without variants is added to an order cycle, and then some variants are added
-  # to that product, but not the order cycle, then the master variant should not available for customers
-  # to purchase.
-  # This method filters out such products so that the customer cannot purchase them.
-  def valid_products_distributed_by(distributor)
-    DistributedValidProducts.new(self, distributor).all
-  end
-
   def products
     self.variants.map(&:product).uniq
   end
diff --git a/lib/open_food_network/products_renderer.rb b/lib/open_food_network/products_renderer.rb
index 6dd0d5f5ed..9ccee412da 100644
--- a/lib/open_food_network/products_renderer.rb
+++ b/lib/open_food_network/products_renderer.rb
@@ -32,15 +32,16 @@ module OpenFoodNetwork
     private
 
     def load_products
-      if @order_cycle
-        scoper = ScopeProductToHub.new(@distributor)
+      return unless @order_cycle
+      scoper = ScopeProductToHub.new(@distributor)
 
-        @order_cycle.
-          valid_products_distributed_by(@distributor).
-          order(taxon_order).
-          each { |p| scoper.scope(p) }.
-          select { |p| !p.deleted? && p.has_stock_for_distribution?(@order_cycle, @distributor) }
-      end
+      DistributedValidProducts.new(@order_cycle, @distributor).
+        relation.
+        order(taxon_order).
+        each { |product| scoper.scope(product) }.
+        select do |product|
+          !product.deleted? && product.has_stock_for_distribution?(@order_cycle, @distributor)
+        end
     end
 
     def taxon_order
diff --git a/spec/models/distributed_valid_products_spec.rb b/spec/models/distributed_valid_products_spec.rb
index b2c50ef022..b5ce260393 100644
--- a/spec/models/distributed_valid_products_spec.rb
+++ b/spec/models/distributed_valid_products_spec.rb
@@ -18,7 +18,7 @@ describe DistributedValidProducts do
 
     distributed_valid_products = described_class.new(order_cycle, distributor)
 
-    expect(distributed_valid_products.all).to eq([valid_product])
+    expect(distributed_valid_products.relation).to eq([valid_product])
   end
 
   context 'when the product has only an obsolete master variant in a distribution' do
@@ -34,7 +34,7 @@ describe DistributedValidProducts do
 
     it 'does not return the obsolete product' do
       distributed_valid_products = described_class.new(order_cycle, distributor)
-      expect(distributed_valid_products.all).to eq([unassociated_variant.product])
+      expect(distributed_valid_products.relation).to eq([unassociated_variant.product])
     end
   end
 
@@ -51,7 +51,7 @@ describe DistributedValidProducts do
 
     it 'returns the product' do
       distributed_valid_products = described_class.new(order_cycle, distributor)
-      expect(distributed_valid_products.all).to eq([product])
+      expect(distributed_valid_products.relation).to eq([product])
     end
   end
 
@@ -69,7 +69,7 @@ describe DistributedValidProducts do
 
     it 'returns the product' do
       distributed_valid_products = described_class.new(order_cycle, distributor)
-      expect(distributed_valid_products.all).to eq([product])
+      expect(distributed_valid_products.relation).to eq([product])
     end
   end
 
@@ -87,7 +87,7 @@ describe DistributedValidProducts do
 
     it 'returns the product' do
       distributed_valid_products = described_class.new(order_cycle, distributor)
-      expect(distributed_valid_products.all).to eq([product])
+      expect(distributed_valid_products.relation).to eq([product])
     end
   end
 end

From 931cb89a9dcdfeb33f1611d9d5c0cb8325542479 Mon Sep 17 00:00:00 2001
From: Pau Perez <saulopefa@gmail.com>
Date: Mon, 11 Mar 2019 18:28:25 +0100
Subject: [PATCH 6/6] Move new class to services and rename it

---
 .../order_cycle_distributed_products.rb}                        | 2 +-
 lib/open_food_network/products_renderer.rb                      | 2 +-
 .../order_cycle_distributed_products_spec.rb}                   | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)
 rename app/{models/distributed_valid_products.rb => services/order_cycle_distributed_products.rb} (97%)
 rename spec/{models/distributed_valid_products_spec.rb => services/order_cycle_distributed_products_spec.rb} (98%)

diff --git a/app/models/distributed_valid_products.rb b/app/services/order_cycle_distributed_products.rb
similarity index 97%
rename from app/models/distributed_valid_products.rb
rename to app/services/order_cycle_distributed_products.rb
index af411e0748..c07ddd849f 100644
--- a/app/models/distributed_valid_products.rb
+++ b/app/services/order_cycle_distributed_products.rb
@@ -4,7 +4,7 @@
 # variants are added to that product, but not the order cycle, then the master
 # variant should not available for customers to purchase. This class filters
 # out such products so that the customer cannot purchase them.
-class DistributedValidProducts
+class OrderCycleDistributedProducts
   def initialize(order_cycle, distributor)
     @order_cycle = order_cycle
     @distributor = distributor
diff --git a/lib/open_food_network/products_renderer.rb b/lib/open_food_network/products_renderer.rb
index 9ccee412da..ef9afb3b1e 100644
--- a/lib/open_food_network/products_renderer.rb
+++ b/lib/open_food_network/products_renderer.rb
@@ -35,7 +35,7 @@ module OpenFoodNetwork
       return unless @order_cycle
       scoper = ScopeProductToHub.new(@distributor)
 
-      DistributedValidProducts.new(@order_cycle, @distributor).
+      OrderCycleDistributedProducts.new(@order_cycle, @distributor).
         relation.
         order(taxon_order).
         each { |product| scoper.scope(product) }.
diff --git a/spec/models/distributed_valid_products_spec.rb b/spec/services/order_cycle_distributed_products_spec.rb
similarity index 98%
rename from spec/models/distributed_valid_products_spec.rb
rename to spec/services/order_cycle_distributed_products_spec.rb
index b5ce260393..9670c179dd 100644
--- a/spec/models/distributed_valid_products_spec.rb
+++ b/spec/services/order_cycle_distributed_products_spec.rb
@@ -1,6 +1,6 @@
 require 'spec_helper'
 
-describe DistributedValidProducts do
+describe OrderCycleDistributedProducts do
   let(:order_cycle) { OrderCycle.new }
   let(:distributor) { instance_double(Enterprise) }