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"