diff --git a/app/controllers/accounts/users_controller.rb b/app/controllers/accounts/users_controller.rb index 94e2d8e..128a27b 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.member, per_page: 500 + set_page_and_extract_portion_from User.visible_to(Current.user).ordered.member, per_page: 500 end def update diff --git a/app/controllers/accounts_controller.rb b/app/controllers/accounts_controller.rb index 0de26b8..2043d20 100644 --- a/app/controllers/accounts_controller.rb +++ b/app/controllers/accounts_controller.rb @@ -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 diff --git a/app/models/user/bannable.rb b/app/models/user/bannable.rb index 9d714ab..594993d 100644 --- a/app/models/user/bannable.rb +++ b/app/models/user/bannable.rb @@ -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 diff --git a/test/controllers/accounts_controller_test.rb b/test/controllers/accounts_controller_test.rb index 83c77ff..12ea1e3 100644 --- a/test/controllers/accounts_controller_test.rb +++ b/test/controllers/accounts_controller_test.rb @@ -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 diff --git a/test/models/user/bannable_test.rb b/test/models/user/bannable_test.rb new file mode 100644 index 0000000..5347c76 --- /dev/null +++ b/test/models/user/bannable_test.rb @@ -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