Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add revoke all blocks moderator action #4440

Merged
merged 5 commits into from
Jan 7, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion app/abilities/ability.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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?
Expand Down
14 changes: 12 additions & 2 deletions app/controllers/user_blocks_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,11 @@ 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
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
Expand Down Expand Up @@ -89,6 +89,16 @@ def revoke
end
end

##
# revokes all active blocks
def revoke_all
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

##
# shows a list of all the blocks on the given user
def blocks_on
Expand Down
27 changes: 27 additions & 0 deletions app/views/user_blocks/revoke_all.html.erb
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
<% @title = t ".title",
:block_on => @user.display_name %>

<% content_for :heading do %>
<h1><%= t ".heading_html",
:block_on => link_to(@user.display_name,
user_path(@user)) %></h1>
<% end %>

<% unless @user.blocks.active.empty? %>

<%= bootstrap_form_for :revoke_all, :url => { :action => "revoke_all" } do |f| %>
<div class="mb-3">
<div class="form-check">
<%= 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" } %>
</div>
</div>

<%= f.primary t(".revoke") %>
<% end %>

<% else %>
<p><%= t ".empty", :name => @user.display_name %></p>
<% end %>
6 changes: 6 additions & 0 deletions app/views/users/show.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,12 @@
</li>
<% end %>

<% if can?(:revoke_all, UserBlock) and @user.blocks.active.exists? %>
<li>
<%= link_to t(".revoke_all_blocks"), revoke_all_user_blocks_path(@user) %>
</li>
<% end %>

<% if can?(:create, UserBlock) %>
<li>
<%= link_to t(".create_block"), new_user_block_path(@user) %>
Expand Down
11 changes: 11 additions & 0 deletions config/locales/en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -2892,6 +2893,16 @@ 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}"
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!"
flash: "All active blocks have been revoked."
helper:
time_future_html: "Ends in %{time}."
until_login: "Active until the user logs in."
Expand Down
1 change: 1 addition & 0 deletions config/routes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
86 changes: 86 additions & 0 deletions test/controllers/user_blocks_controller_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

##
Expand Down Expand Up @@ -386,6 +394,84 @@ def test_revoke
assert_select "p", "Sorry, the user block with ID 99999 could not be found."
end

##
# test the revoke all page
def test_revoke_all_page
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 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
Expand Down
71 changes: 71 additions & 0 deletions test/system/user_blocks_test.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
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 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))

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 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