From f864403c2c3f85fcc22e56cb80b42080b8b4c315 Mon Sep 17 00:00:00 2001 From: Marcello Costagliola Date: Mon, 5 Oct 2026 14:56:22 +0200 Subject: [PATCH] List one page of account members at a time Since administrators were grouped apart from members (b52c318), the account settings page loads every user to split them and renders all of them. The lazy next page still starts at the 501st user, so on an account with more than 500 people, scrolling down lists those users a second time. Load administrators on their own and page only members, both on the settings page and on the pages that follow it. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_0142qgjggdJ2KDdGk7RF9Xm9 --- app/controllers/accounts/users_controller.rb | 2 +- app/controllers/accounts_controller.rb | 6 +++--- app/views/accounts/edit.html.erb | 4 ++-- test/controllers/accounts_controller_test.rb | 17 +++++++++++++++++ 4 files changed, 23 insertions(+), 6 deletions(-) diff --git a/app/controllers/accounts/users_controller.rb b/app/controllers/accounts/users_controller.rb index 8adefef..94e2d8e 100644 --- a/app/controllers/accounts/users_controller.rb +++ b/app/controllers/accounts/users_controller.rb @@ -2,7 +2,7 @@ class Accounts::UsersController < ApplicationController before_action :ensure_can_administer, :set_user, only: %i[ update destroy ] def index - set_page_and_extract_portion_from User.active.ordered.without_bots, per_page: 500 + set_page_and_extract_portion_from User.active.ordered.member, per_page: 500 end def update diff --git a/app/controllers/accounts_controller.rb b/app/controllers/accounts_controller.rb index dda9d83..0de26b8 100644 --- a/app/controllers/accounts_controller.rb +++ b/app/controllers/accounts_controller.rb @@ -3,9 +3,9 @@ class AccountsController < ApplicationController before_action :set_account def edit - users = account_users.ordered.without_bots - @administrators, @members = users.partition(&:administrator?) - set_page_and_extract_portion_from users, per_page: 500 + users = account_users.ordered + @administrators = users.administrator + set_page_and_extract_portion_from users.member, per_page: 500 end def update diff --git a/app/views/accounts/edit.html.erb b/app/views/accounts/edit.html.erb index cfa750d..f74030a 100644 --- a/app/views/accounts/edit.html.erb +++ b/app/views/accounts/edit.html.erb @@ -102,11 +102,11 @@ <%= render partial: "accounts/users/user", collection: @administrators, as: :user %> - <% if @administrators.any? && @members.any? %> + <% if @administrators.any? && @page.records.any? %>
<% end %> - <%= render partial: "accounts/users/user", collection: @members, as: :user %> + <%= render partial: "accounts/users/user", collection: @page.records, as: :user %> <%= render "accounts/users/next_page_container", page: @page.next_param unless @page.last? %>
diff --git a/test/controllers/accounts_controller_test.rb b/test/controllers/accounts_controller_test.rb index 8292c6f..83c77ff 100644 --- a/test/controllers/accounts_controller_test.rb +++ b/test/controllers/accounts_controller_test.rb @@ -42,6 +42,18 @@ class AccountsControllerTest < ActionDispatch::IntegrationTest end end + test "edit lists a page of members and the next page carries on from it" do + User.insert_all 501.times.map { |i| { name: "Member #{i}", email_address: "member#{i}@37signals.com" } } + + get edit_account_url + first_page = listed_user_ids + get account_users_url(page: 2, format: :turbo_stream) + next_page = listed_user_ids + + assert_not_empty next_page + assert_equal User.active.without_bots.ids.sort, (first_page + next_page).sort + end + test "update" do assert users(:david).administrator? @@ -58,4 +70,9 @@ class AccountsControllerTest < ActionDispatch::IntegrationTest put account_url, params: { account: { name: "Different" } } assert_response :forbidden end + + private + def listed_user_ids + response.body.scan(/id="role_user_(\d+)"/).flatten.map(&:to_i) + end end