Merge pull request #4334 from Matt-Yorkley/products_renderer

Products renderer

app/services/order_cycle_distributed_products.rb

@@ -1,41 +1,76 @@
-# 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.
+# Returns a (paginatable) AR object for the products or variants in stock for a given shop and OC.
+# The stock-checking includes on_demand and stock level overrides from variant_overrides.
+
 class OrderCycleDistributedProducts
-  def initialize(order_cycle, distributor)
-    @order_cycle = order_cycle
+  def initialize(distributor, order_cycle)
     @distributor = distributor
+    @order_cycle = order_cycle
   end
 
-  # 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
+  def products_relation
+    Spree::Product.where(id: stocked_products)
+  end
 
-    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)
+  def variants_relation
+    @order_cycle.
+      variants_distributed_by(@distributor).
+      merge(stocked_variants_and_overrides)
   end
 
   private
 
-  attr_reader :order_cycle, :distributor
+  def stocked_products
+    @order_cycle.
+      variants_distributed_by(@distributor).
+      merge(stocked_variants_and_overrides).
+      select("DISTINCT spree_variants.product_id")
+  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.
-  def product_has_only_obsolete_master_in_distribution?(product, distributed_variants)
-    product.has_variants? &&
-      distributed_variants.include?(product.master) &&
-      (product.variants & distributed_variants).empty?
+  def stocked_variants_and_overrides
+    Spree::Variant.
+      joins("LEFT OUTER JOIN variant_overrides ON variant_overrides.variant_id = spree_variants.id
+            AND variant_overrides.hub_id = #{@distributor.id}").
+      joins(:stock_items).
+      where(query_stock_with_overrides)
+  end
+
+  def query_stock_with_overrides
+    "( #{variant_not_overriden} AND ( #{variant_on_demand} OR #{variant_in_stock} ) )
+      OR ( #{variant_overriden} AND ( #{override_on_demand} OR #{override_in_stock} ) )
+      OR ( #{variant_overriden} AND ( #{override_on_demand_null} AND #{variant_on_demand} ) )
+      OR ( #{variant_overriden} AND ( #{override_on_demand_null}
+                                      AND #{variant_not_on_demand} AND #{variant_in_stock} ) )"
+  end
+
+  def variant_not_overriden
+    "variant_overrides.id IS NULL"
+  end
+
+  def variant_overriden
+    "variant_overrides.id IS NOT NULL"
+  end
+
+  def variant_in_stock
+    "spree_stock_items.count_on_hand > 0"
+  end
+
+  def variant_on_demand
+    "spree_stock_items.backorderable IS TRUE"
+  end
+
+  def variant_not_on_demand
+    "spree_stock_items.backorderable IS FALSE"
+  end
+
+  def override_on_demand
+    "variant_overrides.on_demand IS TRUE"
+  end
+
+  def override_in_stock
+    "variant_overrides.count_on_hand > 0"
+  end
+
+  def override_on_demand_null
+    "variant_overrides.on_demand IS NULL"
   end
 end

lib/open_food_network/products_renderer.rb

@@ -10,8 +10,6 @@ module OpenFoodNetwork
     end
 
     def products_json
-      products = load_products
-
       if products
         enterprise_fee_calculator = EnterpriseFeeCalculator.new @distributor, @order_cycle
 
@@ -29,17 +27,20 @@ module OpenFoodNetwork
 
     private
 
-    def load_products
+    def products
       return unless @order_cycle
-      scoper = ScopeProductToHub.new(@distributor)
 
-      OrderCycleDistributedProducts.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
+      @products ||= begin
+        scoper = ScopeProductToHub.new(@distributor)
+
+        distributed_products.products_relation.
+          order(taxon_order).
+          each { |product| scoper.scope(product) }
+      end
+    end
+
+    def distributed_products
+      OrderCycleDistributedProducts.new(@distributor, @order_cycle)
     end
 
     def taxon_order
@@ -53,25 +54,23 @@ module OpenFoodNetwork
       end
     end
 
-    def all_variants_for_shop
-      @all_variants_for_shop ||= begin
-                                   # We use the in_stock? method here instead of the in_stock scope
-                                   # because we need to look up the stock as overridden by
-                                   # VariantOverrides, and the scope method is not affected by them.
-                                   scoper = OpenFoodNetwork::ScopeVariantToHub.new(@distributor)
-                                   Spree::Variant.
-                                     for_distribution(@order_cycle, @distributor).
-                                     each { |v| scoper.scope(v) }.
-                                     select(&:in_stock?)
-                                 end
+    def variants_for_shop
+      @variants_for_shop ||= begin
+        scoper = OpenFoodNetwork::ScopeVariantToHub.new(@distributor)
+
+        distributed_products.variants_relation.
+          includes(:default_price, :stock_locations, :product).
+          where(product_id: products).
+          each { |v| scoper.scope(v) }
+      end
     end
 
     def variants_for_shop_by_id
-      index_by_product_id all_variants_for_shop.reject(&:is_master)
+      index_by_product_id variants_for_shop.reject(&:is_master)
     end
 
     def master_variants_for_shop_by_id
-      index_by_product_id all_variants_for_shop.select(&:is_master)
+      index_by_product_id variants_for_shop.select(&:is_master)
     end
 
     def index_by_product_id(variants)

spec/lib/open_food_network/products_renderer_spec.rb

@@ -25,13 +25,13 @@ module OpenFoodNetwork
 
       it "sorts products by the distributor's preferred taxon list" do
         allow(distributor).to receive(:preferred_shopfront_taxon_order) { "#{t1.id},#{t2.id}" }
-        products = pr.send(:load_products)
+        products = pr.send(:products)
         expect(products).to eq([p2, p4, p1, p3])
       end
 
       it "alphabetizes products by name when taxon list is not set" do
         allow(distributor).to receive(:preferred_shopfront_taxon_order) { "" }
-        products = pr.send(:load_products)
+        products = pr.send(:products)
         expect(products).to eq([p1, p2, p3, p4])
       end
     end

spec/services/order_cycle_distributed_products_spec.rb

@@ -1,93 +1,90 @@
 require 'spec_helper'
 
 describe OrderCycleDistributedProducts do
-  let(:order_cycle) { OrderCycle.new }
-  let(:distributor) { instance_double(Enterprise) }
-
-  it 'returns valid products but not invalid products' do
-    valid_product = create(:product)
-    invalid_product = create(:product)
-    valid_variant = valid_product.variants.first
-
-    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.relation).to eq([valid_product])
-  end
-
-  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] }
-
-    before do
-      allow(order_cycle)
-        .to receive(:variants_distributed_by).with(distributor) { distributed_variants }
+  describe "#products_relation" do
+    let(:distributor) { create(:distributor_enterprise) }
+    let(:product) { create(:product) }
+    let(:variant) { product.variants.first }
+    let(:order_cycle) do
+      create(:simple_order_cycle, distributors: [distributor], variants: [variant])
     end
 
-    it 'does not return the obsolete product' do
-      distributed_valid_products = described_class.new(order_cycle, distributor)
-      expect(distributed_valid_products.relation).to eq([unassociated_variant.product])
+    describe "product distributed by distributor in the OC" do
+      it "returns products" do
+        expect(described_class.new(distributor, order_cycle).products_relation).to eq([product])
+      end
+    end
+
+    describe "product distributed by distributor in another OC" do
+      let(:reference_variant) { create(:product).variants.first }
+      let(:order_cycle) do
+        create(:simple_order_cycle, distributors: [distributor], variants: [reference_variant])
+      end
+      let(:another_order_cycle) do
+        create(:simple_order_cycle, distributors: [distributor], variants: [variant])
+      end
+
+      it "does not return product" do
+        expect(described_class.new(distributor, order_cycle).products_relation).to_not include product
+      end
+    end
+
+    describe "product distributed by another distributor in the OC" do
+      let(:another_distributor) { create(:distributor_enterprise) }
+      let(:order_cycle) do
+        create(:simple_order_cycle, distributors: [another_distributor], variants: [variant])
+      end
+
+      it "does not return product" do
+        expect(described_class.new(distributor, order_cycle).products_relation).to_not include product
+      end
+    end
+
+    describe "filtering products that are out of stock" do
+      context "with regular variants" do
+        it "returns product when variant is in stock" do
+          expect(described_class.new(distributor, order_cycle).products_relation).to include product
+        end
+
+        it "does not return product when variant is out of stock" do
+          variant.update_attribute(:on_hand, 0)
+          expect(described_class.new(distributor, order_cycle).products_relation).to_not include product
+        end
+      end
+
+      context "with variant overrides" do
+        let!(:override) { create(:variant_override, hub: distributor, variant: variant, count_on_hand: 0) }
+
+        it "does not return product when an override is out of stock" do
+          expect(described_class.new(distributor, order_cycle).products_relation).to_not include product
+        end
+
+        it "returns product when an override is in stock" do
+          variant.update_attribute(:on_hand, 0)
+          override.update_attribute(:count_on_hand, 10)
+          expect(described_class.new(distributor, order_cycle).products_relation).to include product
+        end
+      end
     end
   end
 
-  context "when the product doesn't have variants" do
-    let(:master) { build(:variant) }
-    let(:product) { create(:product, master: master) }
-    let(:distributed_variants) { [master] }
+  describe "#variants_relation" do
+    let(:distributor) { create(:distributor_enterprise) }
+    let(:oc) { create(:simple_order_cycle, distributors: [distributor], variants: [v1, v3]) }
+    let(:product) { create(:simple_product) }
+    let!(:v1) { create(:variant, product: product) }
+    let!(:v2) { create(:variant, product: product) }
+    let!(:v3) { create(:variant, product: product) }
+    let!(:vo) { create(:variant_override, hub: distributor, variant_id: v3.id, count_on_hand: 0) }
+    let(:variants) { described_class.new(distributor, oc).variants_relation }
 
-    before do
-      allow(product).to receive(:has_variants?) { false }
-      allow(order_cycle)
-        .to receive(:variants_distributed_by).with(distributor) { distributed_variants }
+    it "returns variants in the oc" do
+      expect(variants).to include v1
+      expect(variants).to_not include v2
     end
 
-    it 'returns the product' do
-      distributed_valid_products = described_class.new(order_cycle, distributor)
-      expect(distributed_valid_products.relation).to eq([product])
-    end
-  end
-
-  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] }
-
-    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.relation).to eq([product])
-    end
-  end
-
-  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] }
-
-    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.relation).to eq([product])
+    it "does not return variants where override is out of stock" do
+      expect(variants).to_not include v3
     end
   end
 end