diff --git a/app/services/order_cycle_distributed_products.rb b/app/services/order_cycle_distributed_products.rb index c07ddd849f..d2c9d3c4ca 100644 --- a/app/services/order_cycle_distributed_products.rb +++ b/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] - 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 diff --git a/lib/open_food_network/products_renderer.rb b/lib/open_food_network/products_renderer.rb index 350395ca40..527385b6e8 100644 --- a/lib/open_food_network/products_renderer.rb +++ b/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) diff --git a/spec/lib/open_food_network/products_renderer_spec.rb b/spec/lib/open_food_network/products_renderer_spec.rb index e7ee727c41..252f26f517 100644 --- a/spec/lib/open_food_network/products_renderer_spec.rb +++ b/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 diff --git a/spec/services/order_cycle_distributed_products_spec.rb b/spec/services/order_cycle_distributed_products_spec.rb index 9670c179dd..b02baf4974 100644 --- a/spec/services/order_cycle_distributed_products_spec.rb +++ b/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