raggedstaff/openfoodnetwork
1
0
Fork 0
mirror of https://github.com/openfoodfoundation/openfoodnetwork synced 2026-01-24 20:36:49 +00:00
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #12574 from rioug/refactor-products-renderer

Browse Source 
[Product Refactor] Refactor products renderer
This commit is contained in:
Konrad
2024-07-10 17:59:38 +02:00
committed by GitHub
parent 10898fdcfc 686fe8c028
commit ecfa47cd78
6 changed files with 178 additions and 137 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
app/controllers/api/v0/order_cycles_controller.rb
View File

@@ -96,7 +96,7 @@ module Api
def distributed_products
OrderCycles::DistributedProductsService.new(
distributor, order_cycle, customer
).products_supplier_relation.pluck(:id)
).products_relation.pluck(:id)
end
end
end

109
app/services/order_cycles/distributed_products_service.rb
View File

@@ -4,50 +4,23 @@
# The stock-checking includes on_demand and stock level overrides from variant_overrides.
module OrderCycles
class DistributedProductsService
class DistributedProductsService # rubocop:disable Metrics/ClassLength
def initialize(distributor, order_cycle, customer)
@distributor = distributor
@order_cycle = order_cycle
@customer = customer
end
# TODO refactor products_taxons_relation and products_supplier_relation
# Joins on the first product variant to allow us to filter product by taxon. # This is so
# enterprise can display product sorted by category in a custom order on their shopfront.
#
# Caveat, the category sorting won't work properly if there are multiple variant with different
# category for a given product.
#
def products_taxons_relation
Spree::Product.where(id: stocked_products).
joins("LEFT JOIN (
SELECT DISTINCT ON(product_id) id, product_id, primary_taxon_id
FROM spree_variants WHERE deleted_at IS NULL
) first_variant ON spree_products.id = first_variant.product_id").
select("spree_products.*, first_variant.primary_taxon_id").
group("spree_products.id, first_variant.primary_taxon_id")
def products_relation
relation_by_sorting.order(Arel.sql(order))
end
# Joins on the first product variant to allow us to filter product by supplier. This is so
# enterprise can display product sorted by supplier in a custom order on their shopfront.
#
# Caveat, the supplier sorting won't work properly if there are multiple variant with different
# supplier for a given product.
#
def products_supplier_relation
Spree::Product.where(id: stocked_products).
joins("LEFT JOIN (SELECT DISTINCT ON(product_id) id, product_id, supplier_id
FROM spree_variants WHERE deleted_at IS NULL) first_variant
ON spree_products.id = first_variant.product_id").
select("spree_products.*, first_variant.supplier_id").
group("spree_products.id, first_variant.supplier_id")
end
def products_relation_incl_supplier_properties
query = relation_by_sorting(supplier_properties: true)
def supplier_property_join(query)
query.joins("
JOIN enterprises ON enterprises.id = first_variant.supplier_id
LEFT OUTER JOIN producer_properties ON producer_properties.producer_id = enterprises.id
")
query = supplier_property_join(query)
query.order(Arel.sql(order))
end
def variants_relation
@@ -61,6 +34,72 @@ module OrderCycles
attr_reader :distributor, :order_cycle, :customer
def relation_by_sorting(supplier_properties: false)
query = Spree::Product.where(id: stocked_products)
if sorting == "by_producer" || supplier_properties
# Joins on the first product variant to allow us to filter product by supplier. This is so
# enterprise can display product sorted by supplier in a custom order on their shopfront.
#
# Caveat, the supplier sorting won't work properly if there are multiple variant with
# different supplier for a given product.
query.
joins("LEFT JOIN (SELECT DISTINCT ON(product_id) id, product_id, supplier_id
FROM spree_variants WHERE deleted_at IS NULL) first_variant
ON spree_products.id = first_variant.product_id").
select("spree_products.*, first_variant.supplier_id").
group("spree_products.id, first_variant.supplier_id")
elsif sorting == "by_category"
# Joins on the first product variant to allow us to filter product by taxon. # This is so
# enterprise can display product sorted by category in a custom order on their shopfront.
#
# Caveat, the category sorting won't work properly if there are multiple variant with
# different category for a given product.
query.
joins("LEFT JOIN (
SELECT DISTINCT ON(product_id) id, product_id, primary_taxon_id
FROM spree_variants WHERE deleted_at IS NULL
) first_variant ON spree_products.id = first_variant.product_id").
select("spree_products.*, first_variant.primary_taxon_id").
group("spree_products.id, first_variant.primary_taxon_id")
else
query.group("spree_products.id")
end
end
def sorting
distributor.preferred_shopfront_product_sorting_method
end
def supplier_property_join(query)
query.joins("
JOIN enterprises ON enterprises.id = first_variant.supplier_id
LEFT OUTER JOIN producer_properties ON producer_properties.producer_id = enterprises.id
")
end
def order
if distributor.preferred_shopfront_product_sorting_method == "by_producer" &&
distributor.preferred_shopfront_producer_order.present?
order_by_producer = distributor
.preferred_shopfront_producer_order
.split(",").map { |id| "first_variant.supplier_id=#{id} DESC" }
.join(", ")
"#{order_by_producer}, spree_products.name ASC, spree_products.id ASC"
elsif distributor.preferred_shopfront_product_sorting_method == "by_category" &&
distributor.preferred_shopfront_taxon_order.present?
order_by_category = distributor
.preferred_shopfront_taxon_order
.split(",").map { |id| "first_variant.primary_taxon_id=#{id} DESC" }
.join(", ")
"#{order_by_category}, spree_products.name ASC, spree_products.id ASC"
else
"spree_products.name ASC, spree_products.id"
end
end
def stocked_products
order_cycle.
variants_distributed_by(distributor).

67
app/services/products_renderer.rb
View File

@@ -3,7 +3,7 @@
require 'open_food_network/scope_product_to_hub'
class ProductsRenderer # rubocop:disable Metrics/ClassLength
class ProductsRenderer
include Pagy::Backend
class NoProducts < RuntimeError; end
@@ -35,8 +35,11 @@ class ProductsRenderer # rubocop:disable Metrics/ClassLength
return unless order_cycle
@products ||= begin
results = products_relation.
order(Arel.sql(products_order))
results = if supplier_properties.present?
distributed_products.products_relation_incl_supplier_properties
else
distributed_products.products_relation
end
results = filter(results)
# Scope results with variant_overrides
@@ -52,38 +55,19 @@ class ProductsRenderer # rubocop:disable Metrics/ClassLength
OpenFoodNetwork::EnterpriseFeeCalculator.new distributor, order_cycle
end
# TODO refactor this, distributed_products should be able to give use the relation based
# on the sorting method, same for ordering. It would prevent the SQL implementation from
# leaking here
def products_relation
if distributor.preferred_shopfront_product_sorting_method == "by_category" &&
distributor.preferred_shopfront_taxon_order.present?
return distributed_products.products_taxons_relation
end
distributed_products.products_supplier_relation
end
# TODO: refactor to address CyclomaticComplexity
def filter(query) # rubocop:disable Metrics/CyclomaticComplexity
supplier_properties = args[:q]&.slice("with_variants_supplier_properties")
def filter(query)
ransack_results = query.ransack(args[:q]).result.to_a
return ransack_results if supplier_properties.blank?
with_properties = args[:q]&.dig("with_properties")
supplier_properties_results = []
if supplier_properties.present?
# We can't search on an association's scope with ransack, a work around is to define
# the a scope on the parent (Spree::Product) but because we are joining on "first_variant"
# to get the supplier it doesn't work, so we do the filtering manually here
# see:
# OrderCycleDistributedProducts#products_supplier_relation
# OrderCycleDistributedProducts#supplier_property_join
supplier_property_ids = supplier_properties["with_variants_supplier_properties"]
supplier_properties_results = distributed_products.supplier_property_join(query).
# OrderCycleDistributedProducts#products_relation
supplier_properties_results = query.
where(producer_properties: { property_id: supplier_property_ids }).
where(inherits_properties: true)
end
@@ -101,6 +85,18 @@ class ProductsRenderer # rubocop:disable Metrics/ClassLength
ransack_results
end
def supplier_properties
args[:q]&.slice("with_variants_supplier_properties")
end
def supplier_property_ids
supplier_properties["with_variants_supplier_properties"]
end
def with_properties
args[:q]&.dig("with_properties")
end
def paginate(results)
_pagy, paginated_results = pagy_array(
results,
@@ -115,27 +111,6 @@ class ProductsRenderer # rubocop:disable Metrics/ClassLength
OrderCycles::DistributedProductsService.new(distributor, order_cycle, customer)
end
# TODO refactor, see above
def products_order
if distributor.preferred_shopfront_product_sorting_method == "by_producer" &&
distributor.preferred_shopfront_producer_order.present?
order_by_producer = distributor
.preferred_shopfront_producer_order
.split(",").map { |id| "first_variant.supplier_id=#{id} DESC" }
.join(", ")
"#{order_by_producer}, spree_products.name ASC, spree_products.id ASC"
elsif distributor.preferred_shopfront_product_sorting_method == "by_category" &&
distributor.preferred_shopfront_taxon_order.present?
order_by_category = distributor
.preferred_shopfront_taxon_order
.split(",").map { |id| "first_variant.primary_taxon_id=#{id} DESC" }
.join(", ")
"#{order_by_category}, spree_products.name ASC, spree_products.id ASC"
else
"spree_products.name ASC, spree_products.id"
end
end
def variants_for_shop
@variants_for_shop ||= begin
scoper = OpenFoodNetwork::ScopeVariantToHub.new(distributor)

2
spec/factories/variant_factory.rb
View File

@@ -14,7 +14,7 @@ FactoryBot.define do
primary_taxon { Spree::Taxon.first || FactoryBot.create(:taxon) }
supplier { Enterprise.is_primary_producer.first || FactoryBot.create(:supplier_enterprise) }
# createing a product here will end up creating an extra variant, as creating product will
# creating a product here will end up creating an extra variant, as creating product will
# create a "standard variant" by default. We could try to pass the variant instance we
# are creating but it fails because then the variant instance gets saved and it fails because
# the product isn't associated yet. It's a chicken and egg problem.

106
spec/services/order_cycles/distributed_products_service_spec.rb
View File

@@ -3,7 +3,14 @@
require 'spec_helper'
RSpec.describe OrderCycles::DistributedProductsService do
describe "#products_supplier_relation" do
# NOTE: product_relation_incl_supplier is tested via ProductsRenderer specs:
# spec/services/products_renderer_spec.rb
describe "#products_relation" do
subject(:products_relation) {
described_class.new(distributor, order_cycle, customer).products_relation
}
let(:distributor) { create(:distributor_enterprise) }
let(:product) { create(:product) }
let(:variant) { product.variants.first }
@@ -16,16 +23,12 @@ RSpec.describe OrderCycles::DistributedProductsService do
supplier = create(:supplier_enterprise)
variant.update(supplier: )
create(:variant, product:, supplier: )
expect(
described_class.new(distributor, order_cycle, customer).products_supplier_relation
).to eq([product])
expect(products_relation).to eq([product])
end
describe "product distributed by distributor in the OC" do
it "returns products" do
expect(
described_class.new(distributor, order_cycle, customer).products_supplier_relation
).to eq([product])
expect(products_relation).to eq([product])
end
end
@@ -39,9 +42,7 @@ RSpec.describe OrderCycles::DistributedProductsService do
end
it "does not return product" do
expect(
described_class.new(distributor, order_cycle, customer).products_supplier_relation
).not_to include product
expect(products_relation).not_to include product
end
end
@@ -52,25 +53,20 @@ RSpec.describe OrderCycles::DistributedProductsService do
end
it "does not return product" do
expect(
described_class.new(distributor, order_cycle, customer).products_supplier_relation
).not_to include product
expect(products_relation).not_to 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, customer).products_supplier_relation
).to include product
expect(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, customer).products_supplier_relation
).not_to include product
expect(products_relation).not_to include product
end
end
@@ -80,20 +76,80 @@ RSpec.describe OrderCycles::DistributedProductsService do
}
it "does not return product when an override is out of stock" do
expect(
described_class.new(distributor, order_cycle, customer).products_supplier_relation
).not_to include product
expect(products_relation).not_to 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, customer).products_supplier_relation
).to include product
expect(products_relation).to include product
end
end
end
describe "sorting" do
let(:order_cycle) do
create(:simple_order_cycle, distributors: [distributor])
end
let(:exchange) { order_cycle.exchanges.to_enterprises(distributor).outgoing.first }
let(:fruits) { create(:taxon) }
let(:cakes) { create(:taxon) }
let(:fruits_supplier) { create(:supplier_enterprise) }
let(:cakes_supplier) { create(:supplier_enterprise) }
let!(:product_apples) {
create(:product, name: "apples", primary_taxon_id: fruits.id,
supplier_id: fruits_supplier.id, inherits_properties: true)
}
let!(:product_banana_bread) {
create(:product, name: "banana bread", primary_taxon_id: cakes.id,
supplier_id: cakes_supplier.id, inherits_properties: true)
}
let!(:product_cherries) {
create(:product, name: "cherries", primary_taxon_id: fruits.id,
supplier_id: fruits_supplier.id, inherits_properties: true)
}
let!(:product_doughnuts) {
create(:product, name: "doughnuts", primary_taxon_id: cakes.id,
supplier_id: cakes_supplier.id, inherits_properties: true)
}
before do
exchange.variants << product_apples.variants.first
exchange.variants << product_banana_bread.variants.first
exchange.variants << product_cherries.variants.first
exchange.variants << product_doughnuts.variants.first
end
it "sorts products by the distributor's preferred taxon list" do
allow(distributor)
.to receive(:preferred_shopfront_product_sorting_method) { "by_category" }
allow(distributor)
.to receive(:preferred_shopfront_taxon_order) { "#{cakes.id},#{fruits.id}" }
expect(products_relation)
.to eq([product_banana_bread, product_doughnuts, product_apples, product_cherries])
end
it "sorts products by the distributor's preferred producer list" do
allow(distributor)
.to receive(:preferred_shopfront_product_sorting_method) { "by_producer" }
allow(distributor).to receive(:preferred_shopfront_producer_order) {
"#{cakes_supplier.id},#{fruits_supplier.id}"
}
expect(products_relation)
.to eq([product_banana_bread, product_doughnuts, product_apples, product_cherries])
end
it "alphabetizes products by name when taxon list is not set" do
allow(distributor).to receive(:preferred_shopfront_taxon_order) { "" }
expect(products_relation)
.to eq([product_apples, product_banana_bread, product_cherries, product_doughnuts])
end
end
end
describe "#variants_relation" do

29
spec/services/products_renderer_spec.rb
View File

@@ -38,34 +38,6 @@ RSpec.describe ProductsRenderer do
exchange.variants << product_doughnuts.variants.first
end
describe "sorting" do
it "sorts products by the distributor's preferred taxon list" do
allow(distributor)
.to receive(:preferred_shopfront_taxon_order) { "#{cakes.id},#{fruits.id}" }
products = products_renderer.send(:products)
expect(products)
.to eq([product_banana_bread, product_doughnuts, product_apples, product_cherries])
end
it "sorts products by the distributor's preferred producer list" do
allow(distributor)
.to receive(:preferred_shopfront_product_sorting_method) { "by_producer" }
allow(distributor).to receive(:preferred_shopfront_producer_order) {
"#{cakes_supplier.id},#{fruits_supplier.id}"
}
products = products_renderer.send(:products)
expect(products)
.to eq([product_banana_bread, product_doughnuts, product_apples, product_cherries])
end
it "alphabetizes products by name when taxon list is not set" do
allow(distributor).to receive(:preferred_shopfront_taxon_order) { "" }
products = products_renderer.send(:products)
expect(products)
.to eq([product_apples, product_banana_bread, product_cherries, product_doughnuts])
end
end
context "filtering" do
it "filters products by name_or_meta_keywords_or_variants_display_as_or_" \
"variants_display_name_or_variants_supplier_name_cont" do
@@ -111,7 +83,6 @@ RSpec.describe ProductsRenderer do
expect(products).to eq([product_apples, product_cherries])
end
# TODO this is a bit flaky due to banana bread having two supplier
it "filters products with a product property or a producer property" do
cakes_supplier.producer_properties.create!({ property_id: property_organic.id,
value: '1', position: 1 })