From 5e7ab68721b2d7fe916690b8521bd94c60fbe583 Mon Sep 17 00:00:00 2001 From: Anton Khorev Date: Wed, 21 Aug 2024 16:51:19 +0300 Subject: [PATCH] Let all moderators revoke blocks by editing --- app/controllers/user_blocks_controller.rb | 8 ++-- config/locales/en.yml | 1 + .../user_blocks_controller_test.rb | 46 +++++++++++++------ 3 files changed, 37 insertions(+), 18 deletions(-) diff --git a/app/controllers/user_blocks_controller.rb b/app/controllers/user_blocks_controller.rb index e77f83999..7027359cc 100644 --- a/app/controllers/user_blocks_controller.rb +++ b/app/controllers/user_blocks_controller.rb @@ -70,8 +70,7 @@ class UserBlocksController < ApplicationController def update if @valid_params - if current_user != @user_block.creator && - current_user != @user_block.revoker + if cannot?(:update, @user_block) flash[:error] = t(@user_block.revoker ? ".only_creator_or_revoker_can_edit" : ".only_creator_can_edit") redirect_to :action => "edit" else @@ -81,7 +80,10 @@ class UserBlocksController < ApplicationController @user_block.ends_at = Time.now.utc + @block_period.hours @user_block.deactivates_at = (@user_block.ends_at unless @user_block.needs_view) @user_block.revoker = current_user if user_block_was_active && !@user_block.active? - if !user_block_was_active && @user_block.active? + if user_block_was_active && @user_block.active? && current_user != @user_block.creator + flash.now[:error] = t(".only_creator_can_edit_without_revoking") + render :action => "edit" + elsif !user_block_was_active && @user_block.active? flash.now[:error] = t(".inactive_block_cannot_be_reactivated") render :action => "edit" else diff --git a/config/locales/en.yml b/config/locales/en.yml index 4d5ce7f58..a1cf2cecf 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -2949,6 +2949,7 @@ en: flash: "Created a block on user %{name}." update: only_creator_can_edit: "Only the moderator who created this block can edit it." + only_creator_can_edit_without_revoking: "Only the moderator who created this block can edit it without revoking." only_creator_or_revoker_can_edit: "Only the moderators who created or revoked this block can edit it." inactive_block_cannot_be_reactivated: "This block is inactive and cannot be reactivated." success: "Block updated." diff --git a/test/controllers/user_blocks_controller_test.rb b/test/controllers/user_blocks_controller_test.rb index 68fc313c3..dbeb7569f 100644 --- a/test/controllers/user_blocks_controller_test.rb +++ b/test/controllers/user_blocks_controller_test.rb @@ -391,7 +391,6 @@ class UserBlocksControllerTest < ActionDispatch::IntegrationTest # test the update action def test_update moderator_user = create(:moderator_user) - second_moderator_user = create(:moderator_user) active_block = create(:user_block, :creator => moderator_user) # Not logged in yet, so updating a block should fail @@ -405,19 +404,7 @@ class UserBlocksControllerTest < ActionDispatch::IntegrationTest put user_block_path(:id => active_block) assert_redirected_to :controller => "errors", :action => "forbidden" - # Login as the wrong moderator - session_for(second_moderator_user) - - # Check that only the person who created a block can update it - assert_no_difference "UserBlock.count" do - put user_block_path(:id => active_block, - :user_block_period => "12", - :user_block => { :needs_view => true, :reason => "Vandalism" }) - end - assert_redirected_to edit_user_block_path(active_block) - assert_equal "Only the moderator who created this block can edit it.", flash[:error] - - # Login as the correct moderator + # Login as the moderator session_for(moderator_user) # A bogus block period should result in an error @@ -495,7 +482,7 @@ class UserBlocksControllerTest < ActionDispatch::IntegrationTest ## # test the update action revoking the block - def test_revoke_using_update + def test_revoke_using_update_by_creator moderator_user = create(:moderator_user) block = create(:user_block, :creator => moderator_user) @@ -503,6 +490,8 @@ class UserBlocksControllerTest < ActionDispatch::IntegrationTest put user_block_path(block, :user_block_period => "24", :user_block => { :needs_view => false, :reason => "Updated Reason" }) + assert_redirected_to user_block_path(block) + assert_equal "Block updated.", flash[:notice] block.reload assert_predicate block, :active? assert_nil block.revoker @@ -510,11 +499,38 @@ class UserBlocksControllerTest < ActionDispatch::IntegrationTest put user_block_path(block, :user_block_period => "0", :user_block => { :needs_view => false, :reason => "Updated Reason" }) + assert_redirected_to user_block_path(block) + assert_equal "Block updated.", flash[:notice] block.reload assert_not_predicate block, :active? assert_equal moderator_user, block.revoker end + def test_revoke_using_update_by_other_moderator + creator_user = create(:moderator_user) + other_moderator_user = create(:moderator_user) + block = create(:user_block, :creator => creator_user) + + session_for(other_moderator_user) + put user_block_path(block, + :user_block_period => "24", + :user_block => { :needs_view => false, :reason => "Updated Reason" }) + assert_response :success + assert_equal "Only the moderator who created this block can edit it without revoking.", flash[:error] + block.reload + assert_predicate block, :active? + assert_nil block.revoker + + put user_block_path(block, + :user_block_period => "0", + :user_block => { :needs_view => false, :reason => "Updated Reason" }) + assert_redirected_to user_block_path(block) + assert_equal "Block updated.", flash[:notice] + block.reload + assert_not_predicate block, :active? + assert_equal other_moderator_user, block.revoker + end + ## # test the revoke action def test_revoke -- 2.39.5