diff --git a/app/controllers/admin/products_v3_controller.rb b/app/controllers/admin/products_v3_controller.rb index c503b5e899..19243b4278 100644 --- a/app/controllers/admin/products_v3_controller.rb +++ b/app/controllers/admin/products_v3_controller.rb @@ -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 diff --git a/app/models/concerns/product_sort_by_stocks.rb b/app/models/concerns/product_sort_by_stocks.rb index 1fce5fda3f..763327fb42 100644 --- a/app/models/concerns/product_sort_by_stocks.rb +++ b/app/models/concerns/product_sort_by_stocks.rb @@ -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 diff --git a/app/models/spree/product.rb b/app/models/spree/product.rb index 5b2c4d2838..2ad3561b6c 100755 --- a/app/models/spree/product.rb +++ b/app/models/spree/product.rb @@ -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 diff --git a/spec/models/spree/product_sort_by_stocks_spec.rb b/spec/models/spree/product_sort_by_stocks_spec.rb index 573fea404e..791957f8bb 100644 --- a/spec/models/spree/product_sort_by_stocks_spec.rb +++ b/spec/models/spree/product_sort_by_stocks_spec.rb @@ -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 diff --git a/spec/system/admin/products_v3/index_spec.rb b/spec/system/admin/products_v3/index_spec.rb index 130831ca66..ac56ec1cfb 100644 --- a/spec/system/admin/products_v3/index_spec.rb +++ b/spec/system/admin/products_v3/index_spec.rb @@ -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