Extract DistributedValidProducts from OrderCycle

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

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

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

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)