Apply the same user filter to every page of account members

Administrators see banned users in the account settings, but only the
first page counted them: the next pages listed active users alone. A
banned member before the page boundary shifted the offset, so one active
member was never listed. Both pages now share User.visible_to.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0142qgjggdJ2KDdGk7RF9Xm9
This commit is contained in:
Marcello Costagliola
2026-10-05 15:19:21 +02:00
parent f864403c2c
commit ce4c05da81
5 changed files with 20 additions and 12 deletions
+1 -1
View File
@@ -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.member, per_page: 500
set_page_and_extract_portion_from User.visible_to(Current.user).ordered.member, per_page: 500
end
def update
+1 -9
View File
@@ -3,7 +3,7 @@ class AccountsController < ApplicationController
before_action :set_account
def edit
users = account_users.ordered
users = User.visible_to(Current.user).ordered
@administrators = users.administrator
set_page_and_extract_portion_from users.member, per_page: 500
end
@@ -21,12 +21,4 @@ class AccountsController < ApplicationController
def account_params
params.require(:account).permit(:name, :logo, settings: {})
end
def account_users
if Current.user.can_administer?
User.where(status: [ :active, :banned ])
else
User.active
end
end
end
+5
View File
@@ -1,6 +1,11 @@
module User::Bannable
extend ActiveSupport::Concern
included do
# Administrators still see banned users, so they can lift the ban.
scope :visible_to, ->(user) { user.can_administer? ? where(status: %i[ active banned ]) : active }
end
def ban
transaction do
create_bans_from_sessions
+3 -2
View File
@@ -44,6 +44,7 @@ class AccountsControllerTest < ActionDispatch::IntegrationTest
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" } }
users(:kevin).banned!
get edit_account_url
first_page = listed_user_ids
@@ -51,7 +52,7 @@ class AccountsControllerTest < ActionDispatch::IntegrationTest
next_page = listed_user_ids
assert_not_empty next_page
assert_equal User.active.without_bots.ids.sort, (first_page + next_page).sort
assert_equal User.where(status: [ :active, :banned ]).without_bots.ids.sort, (first_page + next_page).sort
end
test "update" do
@@ -73,6 +74,6 @@ class AccountsControllerTest < ActionDispatch::IntegrationTest
private
def listed_user_ids
response.body.scan(/id="role_user_(\d+)"/).flatten.map(&:to_i)
response.body.scan(%r{href="/users/(\d+)"}).flatten.map(&:to_i)
end
end
+10
View File
@@ -0,0 +1,10 @@
require "test_helper"
class User::BannableTest < ActiveSupport::TestCase
test "banned users are visible to administrators only" do
users(:kevin).banned!
assert_includes User.visible_to(users(:david)), users(:kevin)
assert_not_includes User.visible_to(users(:jz)), users(:kevin)
end
end