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