From 7aa9b164e674e3a1201f501df92ba82edd9c5e7c Mon Sep 17 00:00:00 2001 From: Ahmed Ejaz Date: Sun, 17 Aug 2025 07:16:46 +0500 Subject: [PATCH 1/8] Add scope for ordering products by stock levels and update admin table header for on_hand sorting --- app/models/spree/product.rb | 13 +++++++++++++ app/views/admin/products_v3/_table.html.haml | 3 ++- 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/app/models/spree/product.rb b/app/models/spree/product.rb index a0b9da8dfe..c36d2b58b7 100755 --- a/app/models/spree/product.rb +++ b/app/models/spree/product.rb @@ -168,6 +168,19 @@ module Spree scope :by_producer, -> { joins(variants: :supplier).order('enterprises.name') } scope :by_name, -> { order('spree_products.name') } + # Scope for ordering by stock levels + scope :order_by_stock, lambda { |direction = :asc| + b_value = direction != :asc + on_hand_direction = direction == :asc ? 'ASC' : 'DESC' + + joins(variants: :stock_items) + .group('spree_products.id, spree_products.name') + .order( + Arel.sql("CASE WHEN BOOL_OR(spree_stock_items.backorderable) = #{b_value} THEN 1 END"), + Arel.sql("SUM(spree_stock_items.count_on_hand) #{on_hand_direction}") + ) + } + scope :managed_by, lambda { |user| if user.admin? where(nil) diff --git a/app/views/admin/products_v3/_table.html.haml b/app/views/admin/products_v3/_table.html.haml index 5226461c94..eaa399d36b 100644 --- a/app/views/admin/products_v3/_table.html.haml +++ b/app/views/admin/products_v3/_table.html.haml @@ -59,7 +59,8 @@ %th.align-left.col-unit_scale.with-input= t('admin.products_page.columns.unit_scale') %th.align-left.col-unit.with-input= t('admin.products_page.columns.unit') %th.align-left.col-price.with-input= t('admin.products_page.columns.price') - %th.align-left.col-on_hand.with-input= t('admin.products_page.columns.on_hand') + = render partial: 'spree/admin/shared/stimulus_sortable_header', + locals: { column: :on_hand, sorted: params.dig(:q, :s), default: 'on_hand asc' } %th.align-left.col-producer= t('admin.products_page.columns.producer') %th.align-left.col-category= t('admin.products_page.columns.category') %th.align-left.col-tax_category= t('admin.products_page.columns.tax_category') From da843d1ba1dbf8e496d23311018310eaeb075391 Mon Sep 17 00:00:00 2001 From: Ahmed Ejaz Date: Sat, 13 Sep 2025 01:11:21 +0500 Subject: [PATCH 2/8] Add sorting by stock levels using ransacker and update locale for 'on_hand' header --- .../admin/products_v3_controller.rb | 25 ++++++++++++- app/models/concerns/product_sort_by_stocks.rb | 37 +++++++++++++++++++ app/models/spree/product.rb | 16 +------- config/locales/en.yml | 1 + 4 files changed, 63 insertions(+), 16 deletions(-) create mode 100644 app/models/concerns/product_sort_by_stocks.rb diff --git a/app/controllers/admin/products_v3_controller.rb b/app/controllers/admin/products_v3_controller.rb index 7b05201c47..83149d9e30 100644 --- a/app/controllers/admin/products_v3_controller.rb +++ b/app/controllers/admin/products_v3_controller.rb @@ -123,6 +123,13 @@ module Admin @page = params[:page].presence || 1 @per_page = params[:per_page].presence || 15 @q = params.permit(q: {})[:q] || { s: 'name asc' } + + # Transform on_hand sorting to include backorderable_priority for proper ordering + if @q[:s] == 'on_hand asc' + @q[:s] = ['backorderable_priority asc', @q[:s]] + elsif @q[:s] == 'on_hand desc' + @q[:s] = ['backorderable_priority desc', @q[:s]] + end end def producers @@ -155,8 +162,22 @@ module Admin product_query = OpenFoodNetwork::Permissions.new(spree_current_user) .editable_products.merge(product_scope_with_includes).ransack(ransack_query).result - @pagy, @products = pagy(product_query.order(:name), limit: @per_page, page: @page, - size: [1, 2, 2, 1]) + # Postgres requires ORDER BY expressions to appear in the SELECT list when using DISTINCT. + # If the current ransack sort uses the computed stock columns, include them in the select + # so the generated COUNT/DISTINCT query is valid. + sort_columns = Array(@q && @q[:s]).flatten + if sort_columns.any? { |s| + s.to_s.include?('on_hand') || s.to_s.include?('backorderable_priority') + } + + product_query = product_query.select( + Arel.sql('spree_products.*'), + Spree::Product.backorderable_priority_sql, + Spree::Product.on_hand_sql + ) + end + + @pagy, @products = pagy(product_query, limit: @per_page, page: @page, size: [1, 2, 2, 1]) end def product_scope diff --git a/app/models/concerns/product_sort_by_stocks.rb b/app/models/concerns/product_sort_by_stocks.rb new file mode 100644 index 0000000000..c2e549c56e --- /dev/null +++ b/app/models/concerns/product_sort_by_stocks.rb @@ -0,0 +1,37 @@ +# frozen_string_literal: true + +module ProductSortByStocks + extend ActiveSupport::Concern + + included do + @on_hand_sql = Arel.sql("( + SELECT COALESCE(SUM(si.count_on_hand), 0) + FROM spree_variants v + JOIN spree_stock_items si ON si.variant_id = v.id + WHERE v.product_id = spree_products.id + GROUP BY v.product_id + )") + + @backorderable_priority_sql = Arel.sql("( + SELECT CASE WHEN BOOL_OR(si.backorderable) = true THEN 1 ELSE 0 END + FROM spree_variants v + JOIN spree_stock_items si ON si.variant_id = v.id + WHERE v.product_id = spree_products.id + GROUP BY v.product_id + )") + + class << self + attr_reader :on_hand_sql, :backorderable_priority_sql + end + + # Ransacker for ordering by stock levels + ransacker :on_hand do + @on_hand_sql + end + + # Ransacker for backorderable status (used for complex sorting) + ransacker :backorderable_priority do + @backorderable_priority_sql + end + end +end diff --git a/app/models/spree/product.rb b/app/models/spree/product.rb index c36d2b58b7..5b2c4d2838 100755 --- a/app/models/spree/product.rb +++ b/app/models/spree/product.rb @@ -19,6 +19,7 @@ require 'open_food_network/property_merge' module Spree class Product < ApplicationRecord include LogDestroyPerformer + include ProductSortByStocks self.belongs_to_required_by_default = false # These columns have been moved to variant. Currently this is only for documentation purposes, @@ -30,7 +31,7 @@ module Spree acts_as_paranoid - searchable_attributes :meta_keywords, :sku + searchable_attributes :meta_keywords, :sku, :on_hand, :backorderable_priority searchable_associations :properties, :variants searchable_scopes :active, :with_properties @@ -168,19 +169,6 @@ module Spree scope :by_producer, -> { joins(variants: :supplier).order('enterprises.name') } scope :by_name, -> { order('spree_products.name') } - # Scope for ordering by stock levels - scope :order_by_stock, lambda { |direction = :asc| - b_value = direction != :asc - on_hand_direction = direction == :asc ? 'ASC' : 'DESC' - - joins(variants: :stock_items) - .group('spree_products.id, spree_products.name') - .order( - Arel.sql("CASE WHEN BOOL_OR(spree_stock_items.backorderable) = #{b_value} THEN 1 END"), - Arel.sql("SUM(spree_stock_items.count_on_hand) #{on_hand_direction}") - ) - } - scope :managed_by, lambda { |user| if user.admin? where(nil) diff --git a/config/locales/en.yml b/config/locales/en.yml index 1644dec151..406c7d38d7 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -4744,6 +4744,7 @@ See the %{link} to find out more about %{sitename}'s features and to start using terms_of_service: "Terms of Service" sortable_header: name: "Name" + on_hand: "On Hand" number: "Number" completed_at: "Completed At" state: "State" From ec91d717c79a0e83cc9be93db976e6b674585123 Mon Sep 17 00:00:00 2001 From: Ahmed Ejaz Date: Sat, 13 Sep 2025 01:31:38 +0500 Subject: [PATCH 3/8] Fix default sorting for 'on_hand' column to 'name asc' in admin products table --- app/views/admin/products_v3/_table.html.haml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/admin/products_v3/_table.html.haml b/app/views/admin/products_v3/_table.html.haml index eaa399d36b..4189149d81 100644 --- a/app/views/admin/products_v3/_table.html.haml +++ b/app/views/admin/products_v3/_table.html.haml @@ -60,7 +60,7 @@ %th.align-left.col-unit.with-input= t('admin.products_page.columns.unit') %th.align-left.col-price.with-input= t('admin.products_page.columns.price') = render partial: 'spree/admin/shared/stimulus_sortable_header', - locals: { column: :on_hand, sorted: params.dig(:q, :s), default: 'on_hand asc' } + locals: { column: :on_hand, sorted: params.dig(:q, :s), default: 'name asc' } %th.align-left.col-producer= t('admin.products_page.columns.producer') %th.align-left.col-category= t('admin.products_page.columns.category') %th.align-left.col-tax_category= t('admin.products_page.columns.tax_category') From c326aa6b2324aafdd81a17349285bb5c0161e3ca Mon Sep 17 00:00:00 2001 From: Ahmed Ejaz Date: Mon, 15 Sep 2025 01:59:40 +0500 Subject: [PATCH 4/8] Add comprehensive specs for sorting functionality --- .../admin/products_v3_controller.rb | 9 +- .../spree/product_sort_by_stocks_spec.rb | 93 +++++++++++++++++++ spec/system/admin/products_v3/index_spec.rb | 35 +++++++ 3 files changed, 135 insertions(+), 2 deletions(-) create mode 100644 spec/models/spree/product_sort_by_stocks_spec.rb diff --git a/app/controllers/admin/products_v3_controller.rb b/app/controllers/admin/products_v3_controller.rb index 83149d9e30..221d4c46aa 100644 --- a/app/controllers/admin/products_v3_controller.rb +++ b/app/controllers/admin/products_v3_controller.rb @@ -124,7 +124,7 @@ 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 for proper ordering + # Transform on_hand sorting to include backorderable_priority (on-demand) for proper ordering if @q[:s] == 'on_hand asc' @q[:s] = ['backorderable_priority asc', @q[:s]] elsif @q[:s] == 'on_hand desc' @@ -177,7 +177,12 @@ module Admin ) end - @pagy, @products = pagy(product_query, limit: @per_page, page: @page, size: [1, 2, 2, 1]) + @pagy, @products = pagy( + product_query.order(:name), + limit: @per_page, + page: @page, + size: [1, 2, 2, 1] + ) end def product_scope diff --git a/spec/models/spree/product_sort_by_stocks_spec.rb b/spec/models/spree/product_sort_by_stocks_spec.rb new file mode 100644 index 0000000000..a852f4b7c9 --- /dev/null +++ b/spec/models/spree/product_sort_by_stocks_spec.rb @@ -0,0 +1,93 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'ProductSortByStocks' do + let(:product) { create(:product) } + + describe 'class level SQL accessors' do + it 'exposes SQL Arel nodes for sorting' do + expect(Spree::Product.on_hand_sql).to be_a(Arel::Nodes::SqlLiteral) + expect(Spree::Product.on_hand_sql.to_s).to include('SELECT COALESCE(SUM') + + expect(Spree::Product.backorderable_priority_sql).to be_a(Arel::Nodes::SqlLiteral) + expect(Spree::Product.backorderable_priority_sql.to_s).to include('CASE WHEN') + end + end + + describe 'on_hand ransacker behaviour' do + it 'can be sorted via ransack by on_hand' do + product1 = create(:product) + product2 = create(:product) + + product1.variants.first.stock_items.update_all(count_on_hand: 2) + product2.variants.first.stock_items.update_all(count_on_hand: 7) + + # ransack sort key: 'on_hand asc' should put product1 before product2 + result = Spree::Product.ransack(s: 'on_hand asc').result.to_a + + expect(result.index(product1)).to be < result.index(product2) + end + end + + describe 'backorderable_priority ransacker behaviour' do + it 'can be sorted via ransack by backorderable_priority' do + product1 = create(:product) + product2 = create(:product) + + [product1, product2].each { |p| + p.variants.each { |v| + v.stock_items.update_all(backorderable: false) + } + } + product2.variants.first.stock_items.update_all(backorderable: true) + + result = Spree::Product.ransack(s: 'backorderable_priority desc').result.to_a + + expect(result.index(product2)).to be < result.index(product1) + end + end + + describe 'combined sorting' do + # shared products for combined sorting examples + let!(:low) { create(:product) } + let!(:mid) { create(:product) } + let!(:high) { create(:product) } + + 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) + + # But make 'mid' backorderable so it should be sorted before 'low' when backorderable_priority asc + mid.variants.first.stock_items.update_all(backorderable: true) + + # 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 + + # backorderable_priority asc places non-backorderable products first, + # so low (non-backorderable) should appear before mid (backorderable) + expect(result.index(low)).to be < result.index(mid) + # and low should appear before high + expect(result.index(low)).to be < result.index(high) + 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) + + # 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'] + result = Spree::Product.ransack(s: ['backorderable_priority desc', + 'on_hand desc']).result.to_a + + expect(result.index(mid)).to be < result.index(high) + expect(result.index(high)).to be < result.index(low) + end + end +end diff --git a/spec/system/admin/products_v3/index_spec.rb b/spec/system/admin/products_v3/index_spec.rb index 83e72a885e..130831ca66 100644 --- a/spec/system/admin/products_v3/index_spec.rb +++ b/spec/system/admin/products_v3/index_spec.rb @@ -164,6 +164,41 @@ RSpec.describe 'As an enterprise user, I can manage my products' do end end end + + context "when clicked on 'On Hand' column header" do + it 'sorts products with on-demand at the top in descending order (bottom in ascending), + then by total stock across all variants' do + # Setup products with different stock levels and backorderable (on-demand) settings + # product with 2 on_hand stock items + product_a.variants.first.stock_items.update_all(count_on_hand: 2) + # product with on-demand stock items + product_b.variants.first.stock_items.update_all(count_on_hand: 0, backorderable: true) + + # product with multiple variants having different on_hand stock items + product_c = create(:simple_product, name: "Cherries") + product_c.variants.first.stock_items.update_all(count_on_hand: 1, backorderable: false) + create(:variant, product: product_c, on_hand: 3) + + # 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) + create(:variant, product: product_d, on_hand: 0, on_demand: true) + + within products_table do + on_hand_header = page.find('th > a[data-column="on_hand"]') + + # Sort in acscending order + on_hand_header.click + expect(page).to have_content("On Hand ▲") # this indicates the re-sorted + expect(all_input_values).to match /Apples.*Cherries.*Bananas.*Dates/ + + # 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/ + end + end + end end describe "pagination" do From 49dbe1d039da1877a950e9637f7bb2d01674c239 Mon Sep 17 00:00:00 2001 From: Ahmed Ejaz Date: Mon, 15 Sep 2025 02:15:03 +0500 Subject: [PATCH 5/8] Refactor comments for clarity in product sorting concerns --- app/controllers/admin/products_v3_controller.rb | 2 +- app/models/concerns/product_sort_by_stocks.rb | 2 -- spec/models/spree/product_sort_by_stocks_spec.rb | 2 +- 3 files changed, 2 insertions(+), 4 deletions(-) diff --git a/app/controllers/admin/products_v3_controller.rb b/app/controllers/admin/products_v3_controller.rb index 221d4c46aa..6d1ae388cb 100644 --- a/app/controllers/admin/products_v3_controller.rb +++ b/app/controllers/admin/products_v3_controller.rb @@ -163,7 +163,7 @@ module Admin .editable_products.merge(product_scope_with_includes).ransack(ransack_query).result # Postgres requires ORDER BY expressions to appear in the SELECT list when using DISTINCT. - # If the current ransack sort uses the computed stock columns, include them in the select + # When the current ransack sort uses the computed stock columns, include them in the select # so the generated COUNT/DISTINCT query is valid. sort_columns = Array(@q && @q[:s]).flatten if sort_columns.any? { |s| diff --git a/app/models/concerns/product_sort_by_stocks.rb b/app/models/concerns/product_sort_by_stocks.rb index c2e549c56e..f777910fd7 100644 --- a/app/models/concerns/product_sort_by_stocks.rb +++ b/app/models/concerns/product_sort_by_stocks.rb @@ -24,12 +24,10 @@ module ProductSortByStocks attr_reader :on_hand_sql, :backorderable_priority_sql end - # Ransacker for ordering by stock levels ransacker :on_hand do @on_hand_sql end - # Ransacker for backorderable status (used for complex sorting) ransacker :backorderable_priority do @backorderable_priority_sql end diff --git a/spec/models/spree/product_sort_by_stocks_spec.rb b/spec/models/spree/product_sort_by_stocks_spec.rb index a852f4b7c9..0a6e49c397 100644 --- a/spec/models/spree/product_sort_by_stocks_spec.rb +++ b/spec/models/spree/product_sort_by_stocks_spec.rb @@ -59,7 +59,7 @@ RSpec.describe 'ProductSortByStocks' do mid.variants.first.stock_items.update_all(count_on_hand: 5) high.variants.first.stock_items.update_all(count_on_hand: 10) - # But make 'mid' backorderable so it should be sorted before 'low' when backorderable_priority asc + # Make 'mid' backorderable so it sorts before 'low' in backorderable_priority asc mid.variants.first.stock_items.update_all(backorderable: true) # Controller transforms 'on_hand asc' into ['backorderable_priority asc', 'on_hand asc'] From 0b5efae8c4847c5a53f81cb000ec1c7d2b685b02 Mon Sep 17 00:00:00 2001 From: Ahmed Ejaz Date: Mon, 15 Sep 2025 11:56:07 +0500 Subject: [PATCH 6/8] Refactor sorting expectations in product_sort_by_stocks_spec for clarity and accuracy --- spec/models/spree/product_sort_by_stocks_spec.rb | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/spec/models/spree/product_sort_by_stocks_spec.rb b/spec/models/spree/product_sort_by_stocks_spec.rb index 0a6e49c397..701ce46927 100644 --- a/spec/models/spree/product_sort_by_stocks_spec.rb +++ b/spec/models/spree/product_sort_by_stocks_spec.rb @@ -66,11 +66,7 @@ RSpec.describe 'ProductSortByStocks' do result = Spree::Product.ransack(s: ['backorderable_priority asc', 'on_hand asc']).result.to_a - # backorderable_priority asc places non-backorderable products first, - # so low (non-backorderable) should appear before mid (backorderable) - expect(result.index(low)).to be < result.index(mid) - # and low should appear before high - expect(result.index(low)).to be < result.index(high) + expect(result).to eq([low, high, mid]) end it 'supports combined sorting: backorderable_priority (on-demand) then on_hand (desc)' do @@ -86,8 +82,7 @@ RSpec.describe 'ProductSortByStocks' do result = Spree::Product.ransack(s: ['backorderable_priority desc', 'on_hand desc']).result.to_a - expect(result.index(mid)).to be < result.index(high) - expect(result.index(high)).to be < result.index(low) + expect(result).to eq([mid, high, low]) end end end From 1987f0b6672cfed9f67fd874b59f07adabf08ed3 Mon Sep 17 00:00:00 2001 From: Ahmed Ejaz Date: Mon, 15 Sep 2025 11:57:33 +0500 Subject: [PATCH 7/8] Remove redundant SQL string checks in product sorting specs for clarity --- spec/models/spree/product_sort_by_stocks_spec.rb | 3 --- 1 file changed, 3 deletions(-) diff --git a/spec/models/spree/product_sort_by_stocks_spec.rb b/spec/models/spree/product_sort_by_stocks_spec.rb index 701ce46927..573fea404e 100644 --- a/spec/models/spree/product_sort_by_stocks_spec.rb +++ b/spec/models/spree/product_sort_by_stocks_spec.rb @@ -8,10 +8,7 @@ RSpec.describe 'ProductSortByStocks' do describe 'class level SQL accessors' do it 'exposes SQL Arel nodes for sorting' do expect(Spree::Product.on_hand_sql).to be_a(Arel::Nodes::SqlLiteral) - expect(Spree::Product.on_hand_sql.to_s).to include('SELECT COALESCE(SUM') - expect(Spree::Product.backorderable_priority_sql).to be_a(Arel::Nodes::SqlLiteral) - expect(Spree::Product.backorderable_priority_sql.to_s).to include('CASE WHEN') end end From dffcd446fda6e2c45981f616e084881de787a394 Mon Sep 17 00:00:00 2001 From: Ahmed Ejaz Date: Mon, 15 Sep 2025 12:03:04 +0500 Subject: [PATCH 8/8] Simplify backorderable priority SQL query in product sorting concern --- app/models/concerns/product_sort_by_stocks.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/concerns/product_sort_by_stocks.rb b/app/models/concerns/product_sort_by_stocks.rb index f777910fd7..1fce5fda3f 100644 --- a/app/models/concerns/product_sort_by_stocks.rb +++ b/app/models/concerns/product_sort_by_stocks.rb @@ -13,7 +13,7 @@ module ProductSortByStocks )") @backorderable_priority_sql = Arel.sql("( - SELECT CASE WHEN BOOL_OR(si.backorderable) = true THEN 1 ELSE 0 END + SELECT BOOL_OR(si.backorderable) FROM spree_variants v JOIN spree_stock_items si ON si.variant_id = v.id WHERE v.product_id = spree_products.id