From 342a3dcfadd72e03d120c84ea3e90fa7426673bd Mon Sep 17 00:00:00 2001 From: Ikraam Ghoor Date: Fri, 26 Jun 2026 10:27:50 +0200 Subject: [PATCH] Fixed N+1 queries in admin and API collection endpoints Several index and show endpoints rendered per-row associations that were not eager loaded, so the query count grew with the number of rows: - solidus_admin products index: the stock column reads total_on_hand, loading each variant's stock items and their stock location - solidus_admin stock items index: each row loads its variant, the variant's product, images, option values, and the stock location - API stock movements index: each movement renders its stock item and the variant graph the listing shows (product, prices, stock, images, option values) - API users index: each user renders its billing and shipping address - admin order payments list: each payment renders its payment method Eager load the associations each view renders so the queries are batched. --- .../solidus_admin/products_controller.rb | 3 ++- .../solidus_admin/stock_items_controller.rb | 7 +++++- .../requests/solidus_admin/products_spec.rb | 9 ++++++++ .../solidus_admin/stock_items_spec.rb | 22 +++++++++++++++++++ .../spree/api/stock_movements_controller.rb | 1 + .../controllers/spree/api/users_controller.rb | 1 + .../spree/api/stock_movements_spec.rb | 12 ++++++++++ api/spec/requests/spree/api/users_spec.rb | 8 +++++++ .../spree/admin/payments_controller.rb | 2 +- .../spree/admin/payments_controller_spec.rb | 12 ++++++++++ 10 files changed, 74 insertions(+), 3 deletions(-) create mode 100644 admin/spec/requests/solidus_admin/stock_items_spec.rb diff --git a/admin/app/controllers/solidus_admin/products_controller.rb b/admin/app/controllers/solidus_admin/products_controller.rb index 156654662a4..90822de245f 100644 --- a/admin/app/controllers/solidus_admin/products_controller.rb +++ b/admin/app/controllers/solidus_admin/products_controller.rb @@ -16,7 +16,8 @@ def index Spree::Product.includes( :variant_images, master: :prices, - variants: :prices + variants: :prices, + variants_including_master: {stock_items: :stock_location} ), param: :q, distinct: false diff --git a/admin/app/controllers/solidus_admin/stock_items_controller.rb b/admin/app/controllers/solidus_admin/stock_items_controller.rb index 46650575b91..b4696956200 100644 --- a/admin/app/controllers/solidus_admin/stock_items_controller.rb +++ b/admin/app/controllers/solidus_admin/stock_items_controller.rb @@ -33,7 +33,12 @@ def update def resource_class = Spree::StockItem - def resources_collection = Spree::StockItem.reorder(nil) + def resources_collection + Spree::StockItem.reorder(nil).includes( + :stock_location, + variant: [:product, :images, {option_values: :option_type}] + ) + end def resources_sorting_options { diff --git a/admin/spec/requests/solidus_admin/products_spec.rb b/admin/spec/requests/solidus_admin/products_spec.rb index ea6830b1328..2d13b3b1a35 100644 --- a/admin/spec/requests/solidus_admin/products_spec.rb +++ b/admin/spec/requests/solidus_admin/products_spec.rb @@ -9,6 +9,15 @@ allow_any_instance_of(SolidusAdmin::BaseController).to receive(:spree_current_user).and_return(admin_user) end + describe "GET #index" do + it "loads variant stock in a single query" do + create(:stock_location) + create_list(:product, 3) + + expect { get solidus_admin.products_path }.to make_database_queries(matching: /from .spree_stock_items./i, count: 1) + end + end + describe "PATCH #update" do let(:product) { create(:product) } let(:params) do diff --git a/admin/spec/requests/solidus_admin/stock_items_spec.rb b/admin/spec/requests/solidus_admin/stock_items_spec.rb new file mode 100644 index 00000000000..c805c0bc339 --- /dev/null +++ b/admin/spec/requests/solidus_admin/stock_items_spec.rb @@ -0,0 +1,22 @@ +# frozen_string_literal: true + +require "spec_helper" + +RSpec.describe "SolidusAdmin::StockItemsController", type: :request do + let(:admin_user) { create(:admin_user) } + + before do + allow_any_instance_of(SolidusAdmin::BaseController).to receive(:spree_current_user).and_return(admin_user) + end + + describe "GET #index" do + it "loads stock item stock locations in a single query" do + 3.times do + location = create(:stock_location, propagate_all_variants: false) + create(:stock_item, stock_location: location, variant: create(:variant)) + end + + expect { get solidus_admin.stock_items_path }.to make_database_queries(matching: /from .spree_stock_locations./i, count: 2) + end + end +end diff --git a/api/app/controllers/spree/api/stock_movements_controller.rb b/api/app/controllers/spree/api/stock_movements_controller.rb index 164f25a2174..87108fe4c9b 100644 --- a/api/app/controllers/spree/api/stock_movements_controller.rb +++ b/api/app/controllers/spree/api/stock_movements_controller.rb @@ -34,6 +34,7 @@ def stock_location def scope @stock_location.stock_movements.accessible_by(current_ability) + .includes(stock_item: {variant: [:product, :prices, :stock_items, :images, {option_values: :option_type}]}) end def stock_movement_params diff --git a/api/app/controllers/spree/api/users_controller.rb b/api/app/controllers/spree/api/users_controller.rb index d46c4737769..6c2f3d1f356 100644 --- a/api/app/controllers/spree/api/users_controller.rb +++ b/api/app/controllers/spree/api/users_controller.rb @@ -5,6 +5,7 @@ class Spree::Api::UsersController < Spree::Api::BaseController def index user_scope = user_class.accessible_by(current_ability, :show) + .includes(bill_address: [:state, :country], ship_address: [:state, :country]) if params[:ids] ids = params[:ids].split(",").flatten @users = user_scope.where(id: ids) diff --git a/api/spec/requests/spree/api/stock_movements_spec.rb b/api/spec/requests/spree/api/stock_movements_spec.rb index 7ce914b39c5..ce24d27f884 100644 --- a/api/spec/requests/spree/api/stock_movements_spec.rb +++ b/api/spec/requests/spree/api/stock_movements_spec.rb @@ -76,6 +76,18 @@ module Spree::Api expect(response.status).to eq(201) expect(json_response).to have_attributes(attributes) end + + it "loads stock movement variant prices in a single query" do + 3.times do + variant = create(:variant) + item = Spree::StockItem.where(stock_location:, variant:).first_or_create! + create(:stock_movement, stock_item: item) + end + + expect { + get spree.api_stock_location_stock_movements_path(stock_location) + }.to make_database_queries(matching: /from .spree_prices./i, count: 1) + end end end end diff --git a/api/spec/requests/spree/api/users_spec.rb b/api/spec/requests/spree/api/users_spec.rb index 771db322dbf..6d08f99565a 100644 --- a/api/spec/requests/spree/api/users_spec.rb +++ b/api/spec/requests/spree/api/users_spec.rb @@ -173,6 +173,14 @@ module Spree::Api expect(json_response["users"].size).to eq 2 end + it "loads user addresses without an N+1" do + allow(Spree::LegacyUser).to receive(:find_by).with(hash_including(:spree_api_key)) { current_api_user } + + 3.times { create(:user_with_addresses) } + + expect { get spree.api_users_path }.to make_database_queries(matching: /from .spree_addresses./i, count: 1) + end + it "can control the page size through a parameter" do 2.times { create(:user) } get spree.api_users_path, params: {per_page: 1} diff --git a/backend/app/controllers/spree/admin/payments_controller.rb b/backend/app/controllers/spree/admin/payments_controller.rb index 2a093a0ae5d..0381d50e11d 100644 --- a/backend/app/controllers/spree/admin/payments_controller.rb +++ b/backend/app/controllers/spree/admin/payments_controller.rb @@ -17,7 +17,7 @@ class PaymentsController < Spree::Admin::BaseController respond_to :html def index - @payments = @order.payments.includes(refunds: :reason) + @payments = @order.payments.includes(:payment_method, refunds: :reason) @refunds = @payments.flat_map(&:refunds) redirect_to new_admin_order_payment_url(@order) if @payments.empty? end diff --git a/backend/spec/controllers/spree/admin/payments_controller_spec.rb b/backend/spec/controllers/spree/admin/payments_controller_spec.rb index 72a28354fa9..1d78eefc45c 100644 --- a/backend/spec/controllers/spree/admin/payments_controller_spec.rb +++ b/backend/spec/controllers/spree/admin/payments_controller_spec.rb @@ -12,6 +12,18 @@ module Admin let(:user) { create(:admin_user) } let(:order) { create(:order) } + describe "#index" do + let(:order) { create(:order, bill_address: create(:address)) } + + it "loads the listed payments' methods without an N+1" do + 3.times { create(:payment, order:, payment_method: create(:check_payment_method)) } + + expect { + get :index, params: {order_id: order.number} + }.to make_database_queries(matching: /from .spree_payment_methods./i, count: 2) + end + end + describe "#create" do context "with a valid credit card" do let(:order) { create(:order_with_line_items, state: "payment") }