mirror of
https://github.com/openfoodfoundation/openfoodnetwork
synced 2026-02-26 01:33:22 +00:00
Merge pull request #13716 from chahmedejaz/bugfix/13554-sorting-on-demand-products
"On hand" value influences sorting of "on demand" products/variants
This commit is contained in:
@@ -128,11 +128,21 @@ module Admin
|
||||
@per_page = params[:per_page].presence || 15
|
||||
@q = params.permit(q: {})[:q] || { s: 'name asc' }
|
||||
|
||||
# Transform on_hand sorting to include backorderable_priority (on-demand) for proper ordering
|
||||
# Transform on_hand sorting to properly handle On-Demand products:
|
||||
# - On-Demand products should ignore on_hand completely and sort alphabetically.
|
||||
# - Non-On-Demand products should continue sorting by on_hand as usual.
|
||||
if @q[:s] == 'on_hand asc'
|
||||
@q[:s] = ['backorderable_priority asc', @q[:s]]
|
||||
@q[:s] = [
|
||||
'backorderable_priority asc',
|
||||
'backorderable_name asc',
|
||||
@q[:s]
|
||||
]
|
||||
elsif @q[:s] == 'on_hand desc'
|
||||
@q[:s] = ['backorderable_priority desc', @q[:s]]
|
||||
@q[:s] = [
|
||||
'backorderable_priority desc',
|
||||
'backorderable_name asc',
|
||||
@q[:s]
|
||||
]
|
||||
end
|
||||
end
|
||||
|
||||
@@ -180,6 +190,7 @@ module Admin
|
||||
product_query = product_query.select(
|
||||
Arel.sql('spree_products.*'),
|
||||
Spree::Product.backorderable_priority_sql,
|
||||
Spree::Product.backorderable_name_sql,
|
||||
Spree::Product.on_hand_sql
|
||||
)
|
||||
end
|
||||
|
||||
@@ -3,7 +3,7 @@
|
||||
module ProductSortByStocks
|
||||
extend ActiveSupport::Concern
|
||||
|
||||
included do
|
||||
included do # rubocop:disable Metrics/BlockLength
|
||||
@on_hand_sql = Arel.sql("(
|
||||
SELECT COALESCE(SUM(si.count_on_hand), 0)
|
||||
FROM spree_variants v
|
||||
@@ -20,8 +20,18 @@ module ProductSortByStocks
|
||||
GROUP BY v.product_id
|
||||
)")
|
||||
|
||||
# When a product is On-Demand (backorderable = true), return the product name.
|
||||
# This allows alphabetical ordering inside the On-Demand group.
|
||||
# For non-On-Demand products, return NULL so normal on_hand sorting still applies.
|
||||
@backorderable_name_sql = Arel.sql("
|
||||
CASE
|
||||
WHEN (#{@backorderable_priority_sql}) THEN spree_products.name
|
||||
ELSE NULL
|
||||
END
|
||||
")
|
||||
|
||||
class << self
|
||||
attr_reader :on_hand_sql, :backorderable_priority_sql
|
||||
attr_reader :on_hand_sql, :backorderable_priority_sql, :backorderable_name_sql
|
||||
end
|
||||
|
||||
ransacker :on_hand do
|
||||
@@ -31,5 +41,9 @@ module ProductSortByStocks
|
||||
ransacker :backorderable_priority do
|
||||
@backorderable_priority_sql
|
||||
end
|
||||
|
||||
ransacker :backorderable_name do
|
||||
@backorderable_name_sql
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
@@ -31,7 +31,8 @@ module Spree
|
||||
|
||||
acts_as_paranoid
|
||||
|
||||
searchable_attributes :meta_keywords, :sku, :on_hand, :backorderable_priority
|
||||
searchable_attributes :meta_keywords, :sku, :on_hand, :backorderable_priority,
|
||||
:backorderable_name
|
||||
searchable_associations :properties, :variants
|
||||
searchable_scopes :active, :with_properties
|
||||
|
||||
|
||||
@@ -9,6 +9,7 @@ RSpec.describe 'ProductSortByStocks' do
|
||||
it 'exposes SQL Arel nodes for sorting' do
|
||||
expect(Spree::Product.on_hand_sql).to be_a(Arel::Nodes::SqlLiteral)
|
||||
expect(Spree::Product.backorderable_priority_sql).to be_a(Arel::Nodes::SqlLiteral)
|
||||
expect(Spree::Product.backorderable_name_sql).to be_a(Arel::Nodes::SqlLiteral)
|
||||
end
|
||||
end
|
||||
|
||||
@@ -45,41 +46,82 @@ RSpec.describe 'ProductSortByStocks' do
|
||||
end
|
||||
end
|
||||
|
||||
describe 'combined sorting' do
|
||||
# shared products for combined sorting examples
|
||||
let!(:low) { create(:product) }
|
||||
let!(:mid) { create(:product) }
|
||||
let!(:high) { create(:product) }
|
||||
describe 'backorderable_name ransacker behaviour' do
|
||||
it 'sorts alphabetically *only* within backorderable products' do
|
||||
stock_c = create(:product, name: "Product-C")
|
||||
bo_b = create(:product, name: "Product-B")
|
||||
bo_a = create(:product, name: "Product-A")
|
||||
|
||||
it 'supports combined sorting: backorderable_priority (on-demad) then on_hand (asc)' do
|
||||
low.variants.first.stock_items.update_all(count_on_hand: 1)
|
||||
mid.variants.first.stock_items.update_all(count_on_hand: 5)
|
||||
high.variants.first.stock_items.update_all(count_on_hand: 10)
|
||||
# Mark only Product-A and Product-B as backorderable
|
||||
[bo_a, bo_b].each do |p|
|
||||
p.variants.first.stock_items.update_all(backorderable: true)
|
||||
end
|
||||
|
||||
# Make 'mid' backorderable so it sorts before 'low' in backorderable_priority asc
|
||||
mid.variants.first.stock_items.update_all(backorderable: true)
|
||||
# Product-C stays non-backorderable
|
||||
stock_c.variants.first.stock_items.update_all(backorderable: false)
|
||||
|
||||
# Controller transforms 'on_hand asc' into ['backorderable_priority asc', 'on_hand asc']
|
||||
result = Spree::Product.ransack(s: ['backorderable_priority asc',
|
||||
'on_hand asc']).result.to_a
|
||||
result = Spree::Product.ransack(s: ['backorderable_priority desc',
|
||||
'backorderable_name asc']).result.to_a
|
||||
|
||||
expect(result).to eq([low, high, mid])
|
||||
# backorderable products come first, alphabetically:
|
||||
# Product-A, Product-B, then non-backorderable Product-C
|
||||
expect(result).to eq([bo_a, bo_b, stock_c])
|
||||
end
|
||||
|
||||
it 'supports combined sorting: backorderable_priority (on-demand) then on_hand (desc)' do
|
||||
low.variants.first.stock_items.update_all(count_on_hand: 2)
|
||||
mid.variants.first.stock_items.update_all(count_on_hand: 6)
|
||||
high.variants.first.stock_items.update_all(count_on_hand: 9)
|
||||
it 'returns NULL for non-backorderable products so alphabetical ordering does NOT apply' do
|
||||
bo_z = create(:product, name: "Product-Z")
|
||||
stock_a = create(:product, name: "Product-A")
|
||||
|
||||
# make 'mid' backorderable so with primary sort desc by backorderable_priority,
|
||||
# so mid should appear before high and low
|
||||
mid.variants.first.stock_items.update_all(backorderable: true)
|
||||
|
||||
# Controller transforms 'on_hand desc' into ['backorderable_priority desc', 'on_hand desc']
|
||||
# only Product-Z is backorderable → its name is used for sorting
|
||||
bo_z.variants.first.stock_items.update_all(backorderable: true)
|
||||
stock_a.variants.first.stock_items.update_all(backorderable: false)
|
||||
result = Spree::Product.ransack(s: ['backorderable_priority desc',
|
||||
'on_hand desc']).result.to_a
|
||||
'backorderable_name asc']).result.to_a
|
||||
|
||||
expect(result).to eq([mid, high, low])
|
||||
# Product-Z (on-demand) comes before Product-A (normal stock)
|
||||
expect(result).to eq([bo_z, stock_a])
|
||||
end
|
||||
end
|
||||
|
||||
describe 'combined sorting' do
|
||||
let!(:bo_a) { create(:product, name: "Backorder-A") }
|
||||
let!(:bo_b) { create(:product, name: "Backorder-B") }
|
||||
let!(:stock_low) { create(:product, name: "Stock-Low") }
|
||||
let!(:stock_high) { create(:product, name: "Stock-High") }
|
||||
|
||||
before do
|
||||
stock_low.variants.first.stock_items.update_all(count_on_hand: 1)
|
||||
stock_high.variants.first.stock_items.update_all(count_on_hand: 10)
|
||||
|
||||
bo_a.variants.first.stock_items.update_all(count_on_hand: 5, backorderable: true)
|
||||
bo_b.variants.first.stock_items.update_all(count_on_hand: 6, backorderable: true)
|
||||
end
|
||||
|
||||
it 'supports combined sorting: backorderable_priority then alphabetical then on_hand asc' do
|
||||
result = Spree::Product.ransack(
|
||||
s: [
|
||||
'backorderable_priority asc',
|
||||
'backorderable_name asc',
|
||||
'on_hand asc'
|
||||
]
|
||||
).result.to_a
|
||||
|
||||
expect(result).to eq([stock_low, stock_high, bo_a, bo_b])
|
||||
end
|
||||
|
||||
it 'supports combined sorting: backorderable_priority then alphabetical then on_hand desc' do
|
||||
result = Spree::Product.ransack(
|
||||
s: [
|
||||
'backorderable_priority desc',
|
||||
'backorderable_name asc',
|
||||
'on_hand desc'
|
||||
]
|
||||
).result.to_a
|
||||
|
||||
# Explanation:
|
||||
# backorderables sorted first → A, B (alphabetical still applies)
|
||||
# then normal stock sorted by stock desc → High (10), Low (1)
|
||||
expect(result).to eq([bo_a, bo_b, stock_high, stock_low])
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
@@ -181,7 +181,7 @@ RSpec.describe 'As an enterprise user, I can manage my products' do
|
||||
|
||||
# product with multiple variants having one on-demand item
|
||||
product_d = create(:simple_product, name: "Dates")
|
||||
product_d.variants.first.stock_items.update_all(count_on_hand: 1, backorderable: false)
|
||||
product_d.variants.first.stock_items.update_all(count_on_hand: 100, backorderable: false)
|
||||
create(:variant, product: product_d, on_hand: 0, on_demand: true)
|
||||
|
||||
within products_table do
|
||||
@@ -195,7 +195,8 @@ RSpec.describe 'As an enterprise user, I can manage my products' do
|
||||
# Sort in descending order
|
||||
on_hand_header.click
|
||||
expect(page).to have_content("On Hand ▼") # this indicates the re-sorted
|
||||
expect(all_input_values).to match /Dates.*Bananas.*Cherries.*Apples/
|
||||
# For all on-demand products, alphabetical order is also applied
|
||||
expect(all_input_values).to match /Bananas.*Dates.*Cherries.*Apples/
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
Reference in New Issue
Block a user