Fix N+1 queries on identity in UsersController and BoardsController (#2365)

* Fix N+1 queries on identity in UsersController and BoardsController

- Add .includes(:identity) to UsersController#index
- Add .includes(creator: :identity) to BoardsController#index
- Add N+1 regression tests for both controllers

* Simplify controller N+1 tests with query assertions

---------

Co-authored-by: Mike Dalessio <mike@37signals.com>
This commit is contained in:
Aleksander
2026-02-25 20:10:56 +02:00
committed by GitHub
parent 16abe6b38b
commit 446f22a292
4 changed files with 28 additions and 2 deletions
+1 -1
View File
@@ -5,7 +5,7 @@ class BoardsController < ApplicationController
before_action :ensure_permission_to_admin_board, only: %i[ update destroy ]
def index
set_page_and_extract_portion_from Current.user.boards.ordered_by_recently_accessed
set_page_and_extract_portion_from Current.user.boards.ordered_by_recently_accessed.includes(creator: :identity)
fresh_when etag: @page.records
end
+1 -1
View File
@@ -3,7 +3,7 @@ class UsersController < ApplicationController
before_action :ensure_permission_to_change_user, only: %i[ update destroy ]
def index
set_page_and_extract_portion_from Current.account.users.active.alphabetically
set_page_and_extract_portion_from Current.account.users.active.alphabetically.includes(:identity)
end
def show
@@ -278,6 +278,20 @@ class BoardsControllerTest < ActionDispatch::IntegrationTest
assert_response :no_content
end
test "index avoids N+1 queries on creator and identity" do
assert_queries_match(/FROM [`"]users[`"].* IN \(/, count: 1) do
assert_queries_match(/FROM [`"]identities[`"].* IN \(/, count: 1) do
get boards_path, as: :json
assert_response :success
end
end
json = @response.parsed_body
first_board = json.first
assert first_board["creator"].present?
assert first_board["creator"]["email_address"].present?
end
private
def next_page_from_link_header(link_header)
url = link_header&.match(/<([^>]+)>;\s*rel="next"/)&.captures&.first
+12
View File
@@ -144,4 +144,16 @@ class UsersControllerTest < ActionDispatch::IntegrationTest
assert_response :no_content
end
test "index avoids N+1 queries on identity" do
sign_in_as :kevin
assert_queries_match(/FROM [`"]identities[`"].* IN \(/, count: 1) do
get users_path, as: :json
assert_response :success
end
json = @response.parsed_body
assert json.first["email_address"].present?
end
end