From 8682b58154b0d42cb0dcc9f85efa806bea68210c Mon Sep 17 00:00:00 2001 From: Anton Khorev Date: Wed, 27 Dec 2023 18:16:57 +0300 Subject: [PATCH 1/5] Add revoke all blocks link --- app/abilities/ability.rb | 2 +- app/views/users/show.html.erb | 6 +++ config/locales/en.yml | 1 + config/routes.rb | 1 + .../user_blocks_controller_test.rb | 8 ++++ test/system/user_blocks_test.rb | 38 +++++++++++++++++++ 6 files changed, 55 insertions(+), 1 deletion(-) create mode 100644 test/system/user_blocks_test.rb diff --git a/app/abilities/ability.rb b/app/abilities/ability.rb index b9da5d08ac..83ab69ee22 100644 --- a/app/abilities/ability.rb +++ b/app/abilities/ability.rb @@ -61,7 +61,7 @@ def initialize(user) can [:index, :show, :resolve, :ignore, :reopen], Issue can :create, IssueComment can [:new, :create, :edit, :update, :destroy], Redaction - can [:new, :edit, :create, :update, :revoke], UserBlock + can [:new, :edit, :create, :update, :revoke, :revoke_all], UserBlock end if user.administrator? diff --git a/app/views/users/show.html.erb b/app/views/users/show.html.erb index 0c803ebb03..253945b9b6 100644 --- a/app/views/users/show.html.erb +++ b/app/views/users/show.html.erb @@ -101,6 +101,12 @@ <% end %> + <% if can?(:revoke_all, UserBlock) and @user.blocks.active.exists? %> +
  • + <%= link_to t(".revoke_all_blocks"), revoke_all_user_blocks_path(@user) %> +
    • + <% end %> + <% if can?(:create, UserBlock) %>
  • <%= link_to t(".create_block"), new_user_block_path(@user) %> diff --git a/config/locales/en.yml b/config/locales/en.yml index 335b8ad84a..8bb239ca55 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -2790,6 +2790,7 @@ en: importer: "Revoke importer access" block_history: "Active Blocks" moderator_history: "Blocks Given" + revoke_all_blocks: "Revoke all blocks" comments: "Comments" create_block: "Block this User" activate_user: "Activate this User" diff --git a/config/routes.rb b/config/routes.rb index 110a67a498..a38f8450ff 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -321,6 +321,7 @@ get "/blocks/new/:display_name" => "user_blocks#new", :as => "new_user_block" resources :user_blocks match "/blocks/:id/revoke" => "user_blocks#revoke", :via => [:get, :post], :as => "revoke_user_block" + match "/user/:display_name/blocks/revoke_all" => "user_blocks#revoke_all", :via => [:get, :post], :as => "revoke_all_user_blocks" # issues and reports resources :issues do diff --git a/test/controllers/user_blocks_controller_test.rb b/test/controllers/user_blocks_controller_test.rb index 2c363be3d9..efd16f859a 100644 --- a/test/controllers/user_blocks_controller_test.rb +++ b/test/controllers/user_blocks_controller_test.rb @@ -54,6 +54,14 @@ def test_routes { :path => "/user/username/blocks_by", :method => :get }, { :controller => "user_blocks", :action => "blocks_by", :display_name => "username" } ) + assert_routing( + { :path => "/user/username/blocks/revoke_all", :method => :get }, + { :controller => "user_blocks", :action => "revoke_all", :display_name => "username" } + ) + assert_routing( + { :path => "/user/username/blocks/revoke_all", :method => :post }, + { :controller => "user_blocks", :action => "revoke_all", :display_name => "username" } + ) end ## diff --git a/test/system/user_blocks_test.rb b/test/system/user_blocks_test.rb new file mode 100644 index 0000000000..e1247d6a40 --- /dev/null +++ b/test/system/user_blocks_test.rb @@ -0,0 +1,38 @@ +require "application_system_test_case" + +class ReportNoteTest < ApplicationSystemTestCase + test "revoke all link is absent for anonymous users when viewed user has active blocks" do + blocked_user = create(:user) + create(:user_block, :user => blocked_user) + + visit user_path(blocked_user) + assert_no_link "Revoke all blocks" + end + + test "revoke all link is absent for regular users when viewed user has active blocks" do + blocked_user = create(:user) + create(:user_block, :user => blocked_user) + sign_in_as(create(:user)) + + visit user_path(blocked_user) + assert_no_link "Revoke all blocks" + end + + test "revoke all link is absent for moderators when viewed user has no active blocks" do + blocked_user = create(:user) + create(:user_block, :expired, :user => blocked_user) + sign_in_as(create(:moderator_user)) + + visit user_path(blocked_user) + assert_no_link "Revoke all blocks" + end + + test "revoke all link is present for moderators when viewed user has active blocks" do + blocked_user = create(:user) + create(:user_block, :user => blocked_user) + sign_in_as(create(:moderator_user)) + + visit user_path(blocked_user) + assert_link "Revoke all blocks" + end +end From ba53dc7b04b4f081f748f2c506b25a3a6563f63d Mon Sep 17 00:00:00 2001 From: Anton Khorev Date: Wed, 27 Dec 2023 18:39:17 +0300 Subject: [PATCH 2/5] Create an empty revoke all blocks page --- app/controllers/user_blocks_controller.rb | 6 ++++ app/views/user_blocks/revoke_all.html.erb | 0 .../user_blocks_controller_test.rb | 30 +++++++++++++++++++ 3 files changed, 36 insertions(+) create mode 100644 app/views/user_blocks/revoke_all.html.erb diff --git a/app/controllers/user_blocks_controller.rb b/app/controllers/user_blocks_controller.rb index 885318cbe1..bf61f906b3 100644 --- a/app/controllers/user_blocks_controller.rb +++ b/app/controllers/user_blocks_controller.rb @@ -89,6 +89,12 @@ def revoke end end + ## + # revokes all active blocks + def revoke_all + # TODO revoke + end + ## # shows a list of all the blocks on the given user def blocks_on diff --git a/app/views/user_blocks/revoke_all.html.erb b/app/views/user_blocks/revoke_all.html.erb new file mode 100644 index 0000000000..e69de29bb2 diff --git a/test/controllers/user_blocks_controller_test.rb b/test/controllers/user_blocks_controller_test.rb index efd16f859a..ea0e7d4883 100644 --- a/test/controllers/user_blocks_controller_test.rb +++ b/test/controllers/user_blocks_controller_test.rb @@ -394,6 +394,36 @@ def test_revoke assert_select "p", "Sorry, the user block with ID 99999 could not be found." end + ## + # test the revoke all action + def test_revoke_all + blocked_user = create(:user) + create(:user_block, :user => blocked_user) + + # Asking for the revoke all blocks page with a bogus user name should fail + get user_blocks_on_path(:display_name => "non_existent_user") + assert_response :not_found + + # Check that the revoke all blocks page requires us to login + get revoke_all_user_blocks_path(blocked_user) + assert_redirected_to login_path(:referer => revoke_all_user_blocks_path(blocked_user)) + + # Login as a normal user + session_for(create(:user)) + + # Check that normal users can't load the revoke all blocks page + get revoke_all_user_blocks_path(blocked_user) + assert_response :redirect + assert_redirected_to :controller => "errors", :action => "forbidden" + + # Login as a moderator + session_for(create(:moderator_user)) + + # Check that the revoke all blocks page loads for moderators + get revoke_all_user_blocks_path(blocked_user) + assert_response :success + end + ## # test the blocks_on action def test_blocks_on From 3443533ce3836673b0533c8474717a41e7f107fe Mon Sep 17 00:00:00 2001 From: Anton Khorev Date: Wed, 27 Dec 2023 18:54:29 +0300 Subject: [PATCH 3/5] Add revoke all blocks page title --- app/controllers/user_blocks_controller.rb | 2 +- app/views/user_blocks/revoke_all.html.erb | 8 ++++++++ config/locales/en.yml | 3 +++ test/system/user_blocks_test.rb | 6 +++++- 4 files changed, 17 insertions(+), 2 deletions(-) diff --git a/app/controllers/user_blocks_controller.rb b/app/controllers/user_blocks_controller.rb index bf61f906b3..a671f0d16f 100644 --- a/app/controllers/user_blocks_controller.rb +++ b/app/controllers/user_blocks_controller.rb @@ -8,7 +8,7 @@ class UserBlocksController < ApplicationController authorize_resource - before_action :lookup_user, :only => [:new, :create, :blocks_on, :blocks_by] + before_action :lookup_user, :only => [:new, :create, :revoke_all, :blocks_on, :blocks_by] before_action :lookup_user_block, :only => [:show, :edit, :update, :revoke] before_action :require_valid_params, :only => [:create, :update] before_action :check_database_readable diff --git a/app/views/user_blocks/revoke_all.html.erb b/app/views/user_blocks/revoke_all.html.erb index e69de29bb2..982792dddc 100644 --- a/app/views/user_blocks/revoke_all.html.erb +++ b/app/views/user_blocks/revoke_all.html.erb @@ -0,0 +1,8 @@ +<% @title = t ".title", + :block_on => @user.display_name %> + +<% content_for :heading do %> +

    <%= t ".heading_html", + :block_on => link_to(@user.display_name, + user_path(@user)) %>

    +<% end %> diff --git a/config/locales/en.yml b/config/locales/en.yml index 8bb239ca55..449b6e5404 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -2893,6 +2893,9 @@ en: confirm: "Are you sure you wish to revoke this block?" revoke: "Revoke!" flash: "This block has been revoked." + revoke_all: + title: "Revoking all blocks on %{block_on}" + heading_html: "Revoking all blocks on %{block_on}" helper: time_future_html: "Ends in %{time}." until_login: "Active until the user logs in." diff --git a/test/system/user_blocks_test.rb b/test/system/user_blocks_test.rb index e1247d6a40..66a8befec2 100644 --- a/test/system/user_blocks_test.rb +++ b/test/system/user_blocks_test.rb @@ -27,12 +27,16 @@ class ReportNoteTest < ApplicationSystemTestCase assert_no_link "Revoke all blocks" end - test "revoke all link is present for moderators when viewed user has active blocks" do + test "revoke all link is present and working for moderators when viewed user has active blocks" do blocked_user = create(:user) create(:user_block, :user => blocked_user) sign_in_as(create(:moderator_user)) visit user_path(blocked_user) assert_link "Revoke all blocks" + + click_link "Revoke all blocks" + assert_title "Revoking all blocks on #{blocked_user.display_name}" + assert_text "Revoking all blocks on #{blocked_user.display_name}" end end From 1ff7b6217ae596cc2a3b74500ac99e710546a54b Mon Sep 17 00:00:00 2001 From: Anton Khorev Date: Wed, 27 Dec 2023 19:19:37 +0300 Subject: [PATCH 4/5] Add controls to revoke all blocks page --- app/views/user_blocks/revoke_all.html.erb | 19 ++++++++++++++ config/locales/en.yml | 6 +++++ test/system/user_blocks_test.rb | 31 ++++++++++++++++++++++- 3 files changed, 55 insertions(+), 1 deletion(-) diff --git a/app/views/user_blocks/revoke_all.html.erb b/app/views/user_blocks/revoke_all.html.erb index 982792dddc..7fef69b57f 100644 --- a/app/views/user_blocks/revoke_all.html.erb +++ b/app/views/user_blocks/revoke_all.html.erb @@ -6,3 +6,22 @@ :block_on => link_to(@user.display_name, user_path(@user)) %> <% end %> + +<% unless @user.blocks.active.empty? %> + + <%= bootstrap_form_for :revoke_all, :url => { :action => "revoke_all" } do |f| %> +
    +
    + <%= check_box_tag "confirm", "yes", false, { :class => "form-check-input" } %> + <%= label_tag "confirm", t(".confirm", + :active_blocks => t(".active_blocks", + :count => @user.blocks.active.count)), { :class => "form-check-label" } %> +
    +
    + + <%= f.primary t(".revoke") %> + <% end %> + +<% else %> +

    <%= t ".empty", :name => @user.display_name %>

    +<% end %> diff --git a/config/locales/en.yml b/config/locales/en.yml index 449b6e5404..ff1ee89849 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -2896,6 +2896,12 @@ en: revoke_all: title: "Revoking all blocks on %{block_on}" heading_html: "Revoking all blocks on %{block_on}" + empty: "%{name} has no active blocks." + confirm: "Are you sure you wish to revoke %{active_blocks}?" + active_blocks: + one: "%{count} active block" + other: "%{count} active blocks" + revoke: "Revoke!" helper: time_future_html: "Ends in %{time}." until_login: "Active until the user logs in." diff --git a/test/system/user_blocks_test.rb b/test/system/user_blocks_test.rb index 66a8befec2..957ecb662a 100644 --- a/test/system/user_blocks_test.rb +++ b/test/system/user_blocks_test.rb @@ -27,7 +27,17 @@ class ReportNoteTest < ApplicationSystemTestCase assert_no_link "Revoke all blocks" end - test "revoke all link is present and working for moderators when viewed user has active blocks" do + test "revoke all page has no controls when viewed user has no active blocks" do + blocked_user = create(:user) + sign_in_as(create(:moderator_user)) + + visit revoke_all_user_blocks_path(blocked_user) + assert_title "Revoking all blocks on #{blocked_user.display_name}" + assert_text "Revoking all blocks on #{blocked_user.display_name}" + assert_no_button "Revoke!" + end + + test "revoke all link is present and working for moderators when viewed user has one active block" do blocked_user = create(:user) create(:user_block, :user => blocked_user) sign_in_as(create(:moderator_user)) @@ -38,5 +48,24 @@ class ReportNoteTest < ApplicationSystemTestCase click_link "Revoke all blocks" assert_title "Revoking all blocks on #{blocked_user.display_name}" assert_text "Revoking all blocks on #{blocked_user.display_name}" + assert_unchecked_field "Are you sure you wish to revoke 1 active block?" + assert_button "Revoke!" + end + + test "revoke all link is present and working for moderators when viewed user has multiple active blocks" do + blocked_user = create(:user) + create(:user_block, :user => blocked_user) + create(:user_block, :user => blocked_user) + create(:user_block, :expired, :user => blocked_user) + sign_in_as(create(:moderator_user)) + + visit user_path(blocked_user) + assert_link "Revoke all blocks" + + click_link "Revoke all blocks" + assert_title "Revoking all blocks on #{blocked_user.display_name}" + assert_text "Revoking all blocks on #{blocked_user.display_name}" + assert_unchecked_field "Are you sure you wish to revoke 2 active blocks?" + assert_button "Revoke!" end end From 366ffd9bd0d1501da4db11ad4f58c125f04aac76 Mon Sep 17 00:00:00 2001 From: Anton Khorev Date: Thu, 28 Dec 2023 05:57:58 +0300 Subject: [PATCH 5/5] Add revoke all blocks action --- app/controllers/user_blocks_controller.rb | 8 ++- config/locales/en.yml | 1 + .../user_blocks_controller_test.rb | 52 ++++++++++++++++++- 3 files changed, 57 insertions(+), 4 deletions(-) diff --git a/app/controllers/user_blocks_controller.rb b/app/controllers/user_blocks_controller.rb index a671f0d16f..07d0bc43c0 100644 --- a/app/controllers/user_blocks_controller.rb +++ b/app/controllers/user_blocks_controller.rb @@ -12,7 +12,7 @@ class UserBlocksController < ApplicationController before_action :lookup_user_block, :only => [:show, :edit, :update, :revoke] before_action :require_valid_params, :only => [:create, :update] before_action :check_database_readable - before_action :check_database_writable, :only => [:create, :update, :revoke] + before_action :check_database_writable, :only => [:create, :update, :revoke, :revoke_all] def index @params = params.permit @@ -92,7 +92,11 @@ def revoke ## # revokes all active blocks def revoke_all - # TODO revoke + if request.post? && params[:confirm] + @user.blocks.active.each { |block| block.revoke!(current_user) } + flash[:notice] = t ".flash" + redirect_to user_blocks_on_path(@user) + end end ## diff --git a/config/locales/en.yml b/config/locales/en.yml index ff1ee89849..6218bfea24 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -2902,6 +2902,7 @@ en: one: "%{count} active block" other: "%{count} active blocks" revoke: "Revoke!" + flash: "All active blocks have been revoked." helper: time_future_html: "Ends in %{time}." until_login: "Active until the user logs in." diff --git a/test/controllers/user_blocks_controller_test.rb b/test/controllers/user_blocks_controller_test.rb index ea0e7d4883..0877fa39e4 100644 --- a/test/controllers/user_blocks_controller_test.rb +++ b/test/controllers/user_blocks_controller_test.rb @@ -395,8 +395,8 @@ def test_revoke end ## - # test the revoke all action - def test_revoke_all + # test the revoke all page + def test_revoke_all_page blocked_user = create(:user) create(:user_block, :user => blocked_user) @@ -424,6 +424,54 @@ def test_revoke_all assert_response :success end + ## + # test the revoke all action + def test_revoke_all_action + blocked_user = create(:user) + active_block1 = create(:user_block, :user => blocked_user) + active_block2 = create(:user_block, :user => blocked_user) + expired_block1 = create(:user_block, :expired, :user => blocked_user) + blocks = [active_block1, active_block2, expired_block1] + moderator_user = create(:moderator_user) + + assert_predicate active_block1, :active? + assert_predicate active_block2, :active? + assert_not_predicate expired_block1, :active? + + # Login as a normal user + session_for(create(:user)) + + # Check that normal users can't load the block revoke page + get revoke_all_user_blocks_path(:blocked_user) + assert_response :redirect + assert_redirected_to :controller => "errors", :action => "forbidden" + + # Login as a moderator + session_for(moderator_user) + + # Check that revoking blocks using GET should fail + get revoke_all_user_blocks_path(blocked_user, :confirm => true) + assert_response :success + assert_template "revoke_all" + + blocks.each(&:reload) + assert_predicate active_block1, :active? + assert_predicate active_block2, :active? + assert_not_predicate expired_block1, :active? + + # Check that revoking blocks works using POST + post revoke_all_user_blocks_path(blocked_user, :confirm => true) + assert_redirected_to user_blocks_on_path(blocked_user) + + blocks.each(&:reload) + assert_not_predicate active_block1, :active? + assert_not_predicate active_block2, :active? + assert_not_predicate expired_block1, :active? + assert_equal moderator_user, active_block1.revoker + assert_equal moderator_user, active_block2.revoker + assert_not_equal moderator_user, expired_block1.revoker + end + ## # test the blocks_on action def test_blocks_on