cst1/openstreetmap-website
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Merge remote-tracking branch 'upstream/pull/5104'

Browse source
This commit is contained in:
Tom Hughes 2024-08-22 18:06:09 +01:00
commit 40c46a2ab8
10 changed files with 155 additions and 31 deletions
Download patch file Download diff file Expand all files Collapse all files

5
app/abilities/ability.rb
View file

@ -60,7 +60,10 @@ class Ability
can [:index, :show, :resolve, :ignore, :reopen], Issue
can :create, IssueComment
can [:new, :create, :edit, :update, :destroy], Redaction
can [:new, :edit, :create, :update, :revoke, :revoke_all], UserBlock
can [:new, :create, :revoke, :revoke_all], UserBlock
can :update, UserBlock, :creator => user
can :update, UserBlock, :revoker => user
can :update, UserBlock, :active? => true
end
if user.administrator?

8
app/controllers/user_blocks_controller.rb
View file

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

3
app/views/user_blocks/_block.html.erb
View file

@ -15,8 +15,7 @@
<% end %>
</td>
<td><%= link_to t(".show"), block %></td>
<td><% if current_user && (current_user.id == block.creator_id ||
current_user.id == block.revoker_id) %><%= link_to t(".edit"), edit_user_block_path(block) %><% end %></td>
<td><% if can?(:edit, block) %><%= link_to t(".edit"), edit_user_block_path(block) %><% end %></td>
<% if can?(:revoke, UserBlock) %>
<td><% if block.active? %><%= link_to t(".revoke"), revoke_user_block_path(block) %><% end %></td>
<% end %>

20
app/views/user_blocks/edit.html.erb
View file

@ -9,7 +9,7 @@
<%= bootstrap_form_for(@user_block) do |f| %>
<%= f.richtext_field :reason, :cols => 80, :rows => 20, :format => @user_block.reason_format %>
<% if @user_block.active? %>
<% if @user_block.active? && @user_block.creator == current_user %>
<%= f.form_group do %>
<%= label_tag "user_block_period", t(".period"), :class => "form-label" %>
<%= select_tag "user_block_period",
@ -21,14 +21,24 @@
<%= f.form_group :needs_view do %>
<%= f.check_box :needs_view %>
<% end %>
<%= f.primary %>
<% else %>
<div class="alert alert-info">
<%= t "user_blocks.update.inactive_block_cannot_be_reactivated" %>
<% if @user_block.active? %>
<%= t "user_blocks.update.only_creator_can_edit_without_revoking" %>
<% else %>
<%= t "user_blocks.update.inactive_block_cannot_be_reactivated" %>
<% end %>
</div>
<%= hidden_field_tag "user_block_period", 0 %>
<%= f.hidden_field :needs_view %>
<% end %>
<%= hidden_field_tag "user_block[needs_view]", false %>
<%= f.primary %>
<% if @user_block.active? %>
<%= f.submit t(".revoke"), :class => "btn btn-danger" %>
<% else %>
<%= f.primary %>
<% end %>
<% end %>
<% end %>

3
app/views/user_blocks/show.html.erb
View file

@ -30,8 +30,7 @@
current_user.id == @user_block.revoker_id) ||
can?(:revoke, UserBlock) && @user_block.active? %>
<div>
<% if current_user && (current_user.id == @user_block.creator_id ||
current_user.id == @user_block.revoker_id) %>
<% if can?(:edit, @user_block) %>
<%= link_to t(".edit"), edit_user_block_path(@user_block), :class => "btn btn-outline-primary" %>
<% end %>
<% if can?(:revoke, UserBlock) && @user_block.active? %>

2
config/locales/en.yml
View file

@ -2943,12 +2943,14 @@ en:
title: "Editing block on %{name}"
heading_html: "Editing block on %{name}"
period: "How long, starting now, the user will be blocked from the API for."
revoke: "Revoke block"
filter:
block_period: "The blocking period must be one of the values selectable in the drop-down list."
create:
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."

47
test/abilities/abilities_test.rb
View file

@ -95,6 +95,53 @@ class ModeratorAbilityTest < AbilityTest
assert ability.can?(action, DiaryComment), "should be able to #{action} DiaryComment"
end
end
test "Active block update permissions" do
creator_user = create(:moderator_user)
other_moderator_user = create(:moderator_user)
block = create(:user_block, :creator => creator_user)
creator_ability = Ability.new creator_user
assert creator_ability.can?(:edit, block)
assert creator_ability.can?(:update, block)
other_moderator_ability = Ability.new other_moderator_user
assert other_moderator_ability.can?(:edit, block)
assert other_moderator_ability.can?(:update, block)
end
test "Expired block update permissions" do
creator_user = create(:moderator_user)
other_moderator_user = create(:moderator_user)
block = create(:user_block, :expired, :creator => creator_user)
creator_ability = Ability.new creator_user
assert creator_ability.can?(:edit, block)
assert creator_ability.can?(:update, block)
other_moderator_ability = Ability.new other_moderator_user
assert other_moderator_ability.cannot?(:edit, block)
assert other_moderator_ability.cannot?(:update, block)
end
test "Revoked block update permissions" do
creator_user = create(:moderator_user)
revoker_user = create(:moderator_user)
other_moderator_user = create(:moderator_user)
block = create(:user_block, :revoked, :creator => creator_user, :revoker => revoker_user)
creator_ability = Ability.new creator_user
assert creator_ability.can?(:edit, block)
assert creator_ability.can?(:update, block)
revoker_ability = Ability.new revoker_user
assert revoker_ability.can?(:edit, block)
assert revoker_ability.can?(:update, block)
other_moderator_ability = Ability.new other_moderator_user
assert other_moderator_ability.cannot?(:edit, block)
assert other_moderator_ability.cannot?(:update, block)
end
end
class AdministratorAbilityTest < AbilityTest

74
test/controllers/user_blocks_controller_test.rb
View file

@ -173,7 +173,7 @@ class UserBlocksControllerTest < ActionDispatch::IntegrationTest
block = create(:user_block, :creator => creator_user)
session_for(other_moderator_user)
check_block_buttons block, :revoke => 1
check_block_buttons block, :edit => 1, :revoke => 1
session_for(creator_user)
check_block_buttons block, :edit => 1, :revoke => 1
@ -252,7 +252,9 @@ class UserBlocksControllerTest < ActionDispatch::IntegrationTest
##
# test the edit action
def test_edit
active_block = create(:user_block)
creator_user = create(:moderator_user)
other_moderator_user = create(:moderator_user)
active_block = create(:user_block, :creator => creator_user)
# Check that the block edit page requires us to login
get edit_user_block_path(:id => active_block)
@ -266,17 +268,37 @@ class UserBlocksControllerTest < ActionDispatch::IntegrationTest
assert_redirected_to :controller => "errors", :action => "forbidden"
# Login as a moderator
session_for(create(:moderator_user))
session_for(other_moderator_user)
# Check that the block edit page loads for moderators
get edit_user_block_path(:id => active_block)
assert_response :success
assert_select "h1 a[href='#{user_path active_block.user}']", :text => active_block.user.display_name
assert_select "form#edit_user_block_#{active_block.id}", :count => 1 do
assert_select "textarea#user_block_reason", :count => 1
assert_select "select#user_block_period", :count => 0
assert_select "input#user_block_needs_view[type='checkbox']", :count => 0
assert_select "input[type='submit'][value='Update block']", :count => 0
assert_select "input#user_block_period[type='hidden']", :count => 1
assert_select "input#user_block_needs_view[type='hidden']", :count => 1
assert_select "input[type='submit'][value='Revoke block']", :count => 1
end
# Login as the block creator
session_for(creator_user)
# Check that the block edit page loads for the creator
get edit_user_block_path(:id => active_block)
assert_response :success
assert_select "h1 a[href='#{user_path active_block.user}']", :text => active_block.user.display_name
assert_select "form#edit_user_block_#{active_block.id}", :count => 1 do
assert_select "textarea#user_block_reason", :count => 1
assert_select "select#user_block_period", :count => 1
assert_select "input#user_block_needs_view[type='checkbox']", :count => 1
assert_select "input[type='submit'][value='Update block']", :count => 1
assert_select "input#user_block_period[type='hidden']", :count => 0
assert_select "input#user_block_needs_view[type='hidden']", :count => 0
assert_select "input[type='submit'][value='Revoke block']", :count => 0
end
# We should get an error if the user doesn't exist
@ -391,7 +413,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 +426,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 +504,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 +512,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 +521,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

6
test/factories/user_blocks.rb
View file

@ -7,6 +7,12 @@ FactoryBot.define do
user
creator :factory => :moderator_user
trait :zero_hour do
now = Time.now.utc
created_at { now }
ends_at { now }
end
trait :needs_view do
needs_view { true }
deactivates_at { nil }

18
test/system/user_blocks_test.rb
View file

@ -98,4 +98,22 @@ class UserBlocksSystemTest < ApplicationSystemTestCase
click_on "Update block"
assert_text(/Reason for block:\s+Editing expired blocks works/)
end
test "other moderator can revoke 0-hour blocks" do
creator_user = create(:moderator_user)
other_moderator_user = create(:moderator_user)
block = create(:user_block, :zero_hour, :needs_view, :creator => creator_user, :reason => "Testing revoking 0-hour blocks")
sign_in_as(other_moderator_user)
visit edit_user_block_path(block)
assert_field "Reason", :with => "Testing revoking 0-hour blocks"
assert_no_select "user_block_period"
assert_no_field "Needs view"
fill_in "Reason", :with => "Revoking 0-hour blocks works"
click_on "Revoke block"
assert_text(/Revoker:\s+#{Regexp.escape other_moderator_user.display_name}/)
assert_text(/Status:\s+Ended/)
assert_text(/Reason for block:\s+Revoking 0-hour blocks works/)
end
end