raggedstaff/openfoodnetwork
1
0
Fork 0
mirror of https://github.com/openfoodfoundation/openfoodnetwork synced 2026-03-25 05:45:15 +00:00
Code Issues Packages Projects Releases Wiki Activity

Refactor ProductsRenderer variants queries

Browse Source 
This removes another N+1 and allows pagination applied to the inital query to also affect the returned variants
This commit is contained in:
Matt-Yorkley
2019-09-29 16:20:20 +01:00
parent e9acf6e0de
commit 032741c54f
3 changed files with 104 additions and 75 deletions
Download Patch File Download Diff File Expand all files Collapse all files

14
app/services/shop_products_service.rb
View File

@@ -1,4 +1,4 @@
# Returns a (paginatable) AR object for the products in stock for a given distributor and OC.
# 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 ShopProductsService
@@ -7,13 +7,19 @@ class ShopProductsService
@order_cycle = order_cycle
end
def relation
Spree::Product.where(id: distributed_products)
def products_relation
Spree::Product.where(id: stocked_products)
end
def variants_relation
@order_cycle.
variants_distributed_by(@distributor).
merge(stocked_variants_and_overrides)
end
private
def distributed_products
def stocked_products
@order_cycle.
variants_distributed_by(@distributor).
merge(stocked_variants_and_overrides).

37
lib/open_food_network/products_renderer.rb
View File

@@ -30,13 +30,17 @@ module OpenFoodNetwork
def shop_products
return unless @order_cycle
@shop_products ||= ShopProductsService.new(@distributor, @order_cycle).relation.
order(taxon_order).
each { |product| scoper.scope(product) }
@shop_products ||= begin
scoper = ScopeProductToHub.new(@distributor)
shop_products_service.products_relation.
order(taxon_order).
each { |product| scoper.scope(product) }
end
end
def scoper
ScopeProductToHub.new(@distributor)
def shop_products_service
ShopProductsService.new(@distributor, @order_cycle)
end
def taxon_order
@@ -50,25 +54,22 @@ 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)
shop_products_service.variants_relation.
where(product_id: shop_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)

128
spec/services/shop_products_service_spec.rb
View File

@@ -1,68 +1,90 @@
require 'spec_helper'
describe ShopProductsService 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
describe "product distributed by distributor in the OC" do
it "returns products" do
expect(described_class.new(distributor, order_cycle).relation).to eq([product])
end
end
describe "product distributed by distributor in another OC" do
let(:reference_variant) { create(:product).variants.first }
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: [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).relation).to_not include 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
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])
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 }
it "returns variants in the oc" do
expect(variants).to include v1
expect(variants).to_not include v2
end
it "does not return product" do
expect(described_class.new(distributor, order_cycle).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).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).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).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).relation).to include product
end
it "does not return variants where override is out of stock" do
expect(variants).to_not include v3
end
end
end