From 7f6167b885fd5e0415e0fa6c98d3a8b260c67d5b Mon Sep 17 00:00:00 2001 From: Pablo Brasero Date: Tue, 25 Nov 2025 14:34:41 +0000 Subject: [PATCH] Add UI for admins to suspend users --- app/controllers/users/lists_controller.rb | 1 + app/controllers/users/statuses_controller.rb | 1 + app/models/user.rb | 6 ++ app/views/users/lists/_page.html.erb | 1 + app/views/users/show.html.erb | 6 ++ config/locales/en.yml | 2 + .../users/lists_controller_test.rb | 64 +++++++++++++++++++ .../users/statuses_controller_test.rb | 21 +++++- test/models/user_test.rb | 14 ++++ test/system/user_status_change_test.rb | 18 ++++++ 10 files changed, 133 insertions(+), 1 deletion(-) diff --git a/app/controllers/users/lists_controller.rb b/app/controllers/users/lists_controller.rb index da495ef4f..06f369c0b 100644 --- a/app/controllers/users/lists_controller.rb +++ b/app/controllers/users/lists_controller.rb @@ -37,6 +37,7 @@ module Users User.where(:id => ids).update_all(:status => "confirmed") if params[:confirm] User.where(:id => ids).update_all(:status => "deleted") if params[:hide] + User.where(:id => ids).each(&:suspend_if_possible!) if params[:suspend] redirect_to url_for(params.permit(:status, :username, :ip, :edits, :before, :after)) end diff --git a/app/controllers/users/statuses_controller.rb b/app/controllers/users/statuses_controller.rb index ccd87a08b..00711b1cf 100644 --- a/app/controllers/users/statuses_controller.rb +++ b/app/controllers/users/statuses_controller.rb @@ -21,6 +21,7 @@ module Users @user.hide! if params[:event] == "hide" @user.unhide! if params[:event] == "unhide" @user.unsuspend! if params[:event] == "unsuspend" + @user.suspend! if params[:event] == "suspend" @user.soft_destroy! if params[:event] == "soft_destroy" # destroy a user, marking them as deleted and removing personal data redirect_to user_path(params[:user_display_name]) end diff --git a/app/models/user.rb b/app/models/user.rb index 150e2fcd1..1fd44340e 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -405,6 +405,12 @@ class User < ApplicationRecord suspend! if !confirmed? && may_suspend? && spam_score > Settings.spam_threshold end + ## + # suspend the user only if allowed by aasm rules + def suspend_if_possible! + suspend! if may_suspend? + end + ## # return an oauth 2 access token for a specified application def oauth_token(application_id) diff --git a/app/views/users/lists/_page.html.erb b/app/views/users/lists/_page.html.erb index 89313569b..0cbbeba0d 100644 --- a/app/views/users/lists/_page.html.erb +++ b/app/views/users/lists/_page.html.erb @@ -46,6 +46,7 @@
<%= submit_tag t(".confirm"), :name => "confirm", :class => "btn btn-primary" %> <%= submit_tag t(".hide"), :name => "hide", :class => "btn btn-primary" %> + <%= submit_tag t(".suspend"), :name => "suspend", :class => "btn btn-primary" %>
<% end %> <% else -%> diff --git a/app/views/users/show.html.erb b/app/views/users/show.html.erb index d67c82431..48cb890e3 100644 --- a/app/views/users/show.html.erb +++ b/app/views/users/show.html.erb @@ -180,6 +180,12 @@ <% end %> + <% if @user.may_suspend? %> +
  • + <%= link_to t(".suspend_user"), user_status_path(@user, :event => "suspend"), :method => :put, :data => { :confirm => t(".confirm") } %> +
    • + <% end %> + <% if @user.may_unsuspend? %>
  • <%= link_to t(".unsuspend_user"), user_status_path(@user, :event => "unsuspend"), :method => :put, :data => { :confirm => t(".confirm") } %> diff --git a/config/locales/en.yml b/config/locales/en.yml index fd2a4cde1..f0f5431af 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -3228,6 +3228,7 @@ en: activate_user: "Activate this User" confirm_user: "Confirm this User" unconfirm_user: "Unconfirm this User" + suspend_user: "Suspend this User" unsuspend_user: "Unsuspend this User" hide_user: "Hide this User" unhide_user: "Unhide this User" @@ -3293,6 +3294,7 @@ en: other: "%{count} users found" confirm: Confirm Selected Users hide: Hide Selected Users + suspend: Suspend Selected Users empty: No matching users found user: summary_html: "%{name} created from %{ip_address} on %{date}" diff --git a/test/controllers/users/lists_controller_test.rb b/test/controllers/users/lists_controller_test.rb index 9577db389..a943df757 100644 --- a/test/controllers/users/lists_controller_test.rb +++ b/test/controllers/users/lists_controller_test.rb @@ -305,6 +305,70 @@ module Users assert_equal "deleted", confirmed_user.reload.status end + def test_update_suspend + normal_user = create(:user) + confirmed_user = create(:user, :confirmed) + suspended_user = create(:user, :suspended) + issue = create(:issue, :reportable => normal_user) + + # Shouldn't work when not logged in + assert_no_difference "User.active.count" do + put users_list_path, :params => { :suspend => 1, :user => { normal_user.id => 1, confirmed_user.id => 1, suspended_user.id => 1 } } + end + assert_response :forbidden + + assert_equal "active", normal_user.reload.status + assert_equal "confirmed", confirmed_user.reload.status + assert_equal "suspended", suspended_user.reload.status + assert_equal "open", issue.reload.status + + session_for(create(:user)) + + # Shouldn't work when logged in as a normal user + assert_no_difference "User.active.count" do + put users_list_path, :params => { :suspend => 1, :user => { normal_user.id => 1, confirmed_user.id => 1, suspended_user.id => 1 } } + end + assert_redirected_to :controller => "/errors", :action => :forbidden + assert_equal "active", normal_user.reload.status + assert_equal "confirmed", confirmed_user.reload.status + assert_equal "suspended", suspended_user.reload.status + assert_equal "open", issue.reload.status + + session_for(create(:moderator_user)) + + # Shouldn't work when logged in as a moderator + assert_no_difference "User.active.count" do + put users_list_path, :params => { :suspend => 1, :user => { normal_user.id => 1, confirmed_user.id => 1, suspended_user.id => 1 } } + end + assert_redirected_to :controller => "/errors", :action => :forbidden + assert_equal "active", normal_user.reload.status + assert_equal "confirmed", confirmed_user.reload.status + assert_equal "suspended", suspended_user.reload.status + assert_equal "open", issue.reload.status + + session_for(create(:administrator_user)) + + # Should do nothing when no users selected + assert_no_difference "User.active.count" do + put users_list_path, :params => { :suspend => 1 } + end + assert_redirected_to :action => :show + assert_equal "active", normal_user.reload.status + assert_equal "confirmed", confirmed_user.reload.status + assert_equal "suspended", suspended_user.reload.status + assert_equal "open", issue.reload.status + + # Should work when logged in as an administrator + assert_difference "User.active.count", -2 do + put users_list_path, :params => { :suspend => 1, :user => { normal_user.id => 1, confirmed_user.id => 1, suspended_user.id => 1 } } + end + assert_redirected_to :action => :show + assert_equal "suspended", normal_user.reload.status + assert_equal "suspended", confirmed_user.reload.status + assert_equal "suspended", suspended_user.reload.status + assert_equal "resolved", issue.reload.status + end + private def check_no_page_link(name) diff --git a/test/controllers/users/statuses_controller_test.rb b/test/controllers/users/statuses_controller_test.rb index d7d1d4df3..649ae17fe 100644 --- a/test/controllers/users/statuses_controller_test.rb +++ b/test/controllers/users/statuses_controller_test.rb @@ -13,7 +13,7 @@ module Users ) end - def test_update + def test_confirm user = create(:user) # Try without logging in @@ -32,6 +32,25 @@ module Users assert_equal "confirmed", User.find(user.id).status end + def test_suspend + user = create(:user) + + # Try without logging in + put user_status_path(user, :event => "suspend") + assert_response :forbidden + + # Now try as a normal user + session_for(user) + put user_status_path(user, :event => "suspend") + assert_redirected_to :controller => "/errors", :action => :forbidden + + # Finally try as an administrator + session_for(create(:administrator_user)) + put user_status_path(user, :event => "suspend") + assert_redirected_to user_path(user) + assert_equal "suspended", User.find(user.id).status + end + def test_destroy user = create(:user, :home_lat => 12.1, :home_lon => 12.1, :description => "test") diff --git a/test/models/user_test.rb b/test/models/user_test.rb index d37cc2f4b..520d91cdf 100644 --- a/test/models/user_test.rb +++ b/test/models/user_test.rb @@ -155,6 +155,20 @@ class UserTest < ActiveSupport::TestCase assert_equal 12, user.spam_score end + def test_suspend_if_possible + active = create(:user, :active) + active.suspend_if_possible! + assert_equal "suspended", active.reload.status + + confirmed = create(:user, :confirmed) + confirmed.suspend_if_possible! + assert_equal "suspended", confirmed.reload.status + + suspended = create(:user, :suspended) + suspended.suspend_if_possible! + assert_equal "suspended", suspended.reload.status + end + def test_follows alice = create(:user, :active) bob = create(:user, :active) diff --git a/test/system/user_status_change_test.rb b/test/system/user_status_change_test.rb index e1c661644..5c4c7a156 100644 --- a/test/system/user_status_change_test.rb +++ b/test/system/user_status_change_test.rb @@ -20,6 +20,24 @@ class UserStatusChangeTest < ApplicationSystemTestCase assert_equal "active", user.status end + test "Admin can suspend a user" do + # There's another instance of "Suspend" in the page. + # This test uses a more specific text, putting it in + # a variable to avoid a misspelling when doing + # `assert_no_content` later + suspend_action_text = "Suspend this User" + + user = create(:user) + visit user_path(user) + accept_confirm do + click_on suspend_action_text + end + + assert_no_content suspend_action_text + user.reload + assert_equal "suspended", user.status + end + test "Admin can confirm a user" do user = create(:user, :suspended) visit user_path(user) -- 2.39.5