From: Anton Khorev Date: Sun, 28 Jul 2024 15:54:42 +0000 (+0300) Subject: Merge branch 'pull/5020' X-Git-Tag: live~773 X-Git-Url: https://git.openstreetmap.org./rails.git/commitdiff_plain/898731ed8162eb9d61a461d353249b5ca3e65177?hp=1a7d68ccbceb07e2da90fc0efa61ef448f5383fe Merge branch 'pull/5020' --- diff --git a/app/controllers/user_blocks_controller.rb b/app/controllers/user_blocks_controller.rb index 3ab217a60..664ac5681 100644 --- a/app/controllers/user_blocks_controller.rb +++ b/app/controllers/user_blocks_controller.rb @@ -63,8 +63,9 @@ class UserBlocksController < ApplicationController def update if @valid_params - if @user_block.creator != current_user - flash[:error] = t(".only_creator_can_edit") + if current_user != @user_block.creator && + current_user != @user_block.revoker + flash[:error] = t(@user_block.revoker ? ".only_creator_or_revoker_can_edit" : ".only_creator_can_edit") redirect_to :action => "edit" elsif @user_block.update( :ends_at => Time.now.utc + @block_period.hours, diff --git a/app/views/user_blocks/_block.html.erb b/app/views/user_blocks/_block.html.erb index 461dc7a8d..a18d1dbdb 100644 --- a/app/views/user_blocks/_block.html.erb +++ b/app/views/user_blocks/_block.html.erb @@ -15,7 +15,8 @@ <% end %> <%= link_to t(".show"), block %> - <% if current_user and current_user.id == block.creator_id %><%= link_to t(".edit"), edit_user_block_path(block) %><% end %> + <% 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 %> <% if show_revoke_link %> <% if block.active? %><%= link_to t(".revoke"), revoke_user_block_path(block) %><% end %> <% end %> diff --git a/app/views/user_blocks/show.html.erb b/app/views/user_blocks/show.html.erb index 619cd6c3f..c36c043cf 100644 --- a/app/views/user_blocks/show.html.erb +++ b/app/views/user_blocks/show.html.erb @@ -26,9 +26,12 @@
<%= @user_block.reason.to_html %>
-<% if current_user&.id == @user_block.creator_id || can?(:revoke, UserBlock) && @user_block.active? %> +<% if current_user && (current_user.id == @user_block.creator_id || + current_user.id == @user_block.revoker_id) || + can?(:revoke, UserBlock) && @user_block.active? %>
- <% if current_user&.id == @user_block.creator_id %> + <% if current_user && (current_user.id == @user_block.creator_id || + current_user.id == @user_block.revoker_id) %> <%= link_to t(".edit"), edit_user_block_path(@user_block), :class => "btn btn-outline-primary" %> <% end %> <% if can?(:revoke, UserBlock) && @user_block.active? %> diff --git a/config/locales/en.yml b/config/locales/en.yml index 2b83fec1f..dc7f1a1c0 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -2940,6 +2940,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_or_revoker_can_edit: "Only the moderators who created or revoked this block can edit it." success: "Block updated." index: title: "User blocks" diff --git a/config/routes.rb b/config/routes.rb index 12f6325ae..c832cbb35 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -84,9 +84,11 @@ OpenStreetMap::Application.routes.draw do delete "gpx/:id" => "traces#destroy", :id => /\d+/ get "gpx/:id/details" => "traces#show", :id => /\d+/ get "gpx/:id/data" => "traces#data", :as => :api_trace_data + end + namespace :api, :path => "api/0.6" do # Map notes API - resources :notes, :except => [:new, :edit, :update], :constraints => { :id => /\d+/ }, :controller => "notes", :as => :api_notes do + resources :notes, :except => [:new, :edit, :update], :id => /\d+/, :controller => "notes" do collection do get "search" get "feed", :defaults => { :format => "rss" } @@ -99,7 +101,7 @@ OpenStreetMap::Application.routes.draw do end end - resources :user_blocks, :only => [:show], :constraints => { :id => /\d+/ }, :controller => "user_blocks", :as => :api_user_blocks + resources :user_blocks, :only => :show, :id => /\d+/, :controller => "user_blocks" end # Data browsing diff --git a/test/controllers/api/changeset_comments_controller_test.rb b/test/controllers/api/changeset_comments_controller_test.rb index 01d5ecb5d..f479b24b3 100644 --- a/test/controllers/api/changeset_comments_controller_test.rb +++ b/test/controllers/api/changeset_comments_controller_test.rb @@ -46,7 +46,7 @@ module Api assert_difference "ChangesetComment.count", 1 do assert_no_difference "ActionMailer::Base.deliveries.size" do perform_enqueued_jobs do - post changeset_comment_path(:id => private_user_closed_changeset, :text => "This is a comment"), :headers => auth_header + post changeset_comment_path(private_user_closed_changeset, :text => "This is a comment"), :headers => auth_header end end end @@ -61,7 +61,7 @@ module Api assert_difference "ChangesetComment.count", 1 do assert_difference "ActionMailer::Base.deliveries.size", 1 do perform_enqueued_jobs do - post changeset_comment_path(:id => changeset, :text => "This is a comment"), :headers => auth_header + post changeset_comment_path(changeset, :text => "This is a comment"), :headers => auth_header end end end @@ -79,7 +79,7 @@ module Api assert_difference "ChangesetComment.count", 1 do assert_difference "ActionMailer::Base.deliveries.size", 2 do perform_enqueued_jobs do - post changeset_comment_path(:id => changeset, :text => "This is a comment"), :headers => auth_header + post changeset_comment_path(changeset, :text => "This is a comment"), :headers => auth_header end end end @@ -102,32 +102,32 @@ module Api # create comment fail def test_create_comment_fail # unauthorized - post changeset_comment_path(:id => create(:changeset, :closed), :text => "This is a comment") + post changeset_comment_path(create(:changeset, :closed), :text => "This is a comment") assert_response :unauthorized auth_header = basic_authorization_header create(:user).email, "test" # bad changeset id assert_no_difference "ChangesetComment.count" do - post changeset_comment_path(:id => 999111, :text => "This is a comment"), :headers => auth_header + post changeset_comment_path(999111, :text => "This is a comment"), :headers => auth_header end assert_response :not_found # not closed changeset assert_no_difference "ChangesetComment.count" do - post changeset_comment_path(:id => create(:changeset), :text => "This is a comment"), :headers => auth_header + post changeset_comment_path(create(:changeset), :text => "This is a comment"), :headers => auth_header end assert_response :conflict # no text assert_no_difference "ChangesetComment.count" do - post changeset_comment_path(:id => create(:changeset, :closed)), :headers => auth_header + post changeset_comment_path(create(:changeset, :closed)), :headers => auth_header end assert_response :bad_request # empty text assert_no_difference "ChangesetComment.count" do - post changeset_comment_path(:id => create(:changeset, :closed), :text => ""), :headers => auth_header + post changeset_comment_path(create(:changeset, :closed), :text => ""), :headers => auth_header end assert_response :bad_request end @@ -142,13 +142,13 @@ module Api assert_difference "ChangesetComment.count", Settings.initial_changeset_comments_per_hour do 1.upto(Settings.initial_changeset_comments_per_hour) do |count| - post changeset_comment_path(:id => changeset, :text => "Comment #{count}"), :headers => auth_header + post changeset_comment_path(changeset, :text => "Comment #{count}"), :headers => auth_header assert_response :success end end assert_no_difference "ChangesetComment.count" do - post changeset_comment_path(:id => changeset, :text => "One comment too many"), :headers => auth_header + post changeset_comment_path(changeset, :text => "One comment too many"), :headers => auth_header assert_response :too_many_requests end end @@ -164,13 +164,13 @@ module Api assert_difference "ChangesetComment.count", Settings.max_changeset_comments_per_hour do 1.upto(Settings.max_changeset_comments_per_hour) do |count| - post changeset_comment_path(:id => changeset, :text => "Comment #{count}"), :headers => auth_header + post changeset_comment_path(changeset, :text => "Comment #{count}"), :headers => auth_header assert_response :success end end assert_no_difference "ChangesetComment.count" do - post changeset_comment_path(:id => changeset, :text => "One comment too many"), :headers => auth_header + post changeset_comment_path(changeset, :text => "One comment too many"), :headers => auth_header assert_response :too_many_requests end end @@ -186,13 +186,13 @@ module Api assert_difference "ChangesetComment.count", Settings.initial_changeset_comments_per_hour / 2 do 1.upto(Settings.initial_changeset_comments_per_hour / 2) do |count| - post changeset_comment_path(:id => changeset, :text => "Comment #{count}"), :headers => auth_header + post changeset_comment_path(changeset, :text => "Comment #{count}"), :headers => auth_header assert_response :success end end assert_no_difference "ChangesetComment.count" do - post changeset_comment_path(:id => changeset, :text => "One comment too many"), :headers => auth_header + post changeset_comment_path(changeset, :text => "One comment too many"), :headers => auth_header assert_response :too_many_requests end end @@ -207,13 +207,13 @@ module Api assert_difference "ChangesetComment.count", Settings.moderator_changeset_comments_per_hour do 1.upto(Settings.moderator_changeset_comments_per_hour) do |count| - post changeset_comment_path(:id => changeset, :text => "Comment #{count}"), :headers => auth_header + post changeset_comment_path(changeset, :text => "Comment #{count}"), :headers => auth_header assert_response :success end end assert_no_difference "ChangesetComment.count" do - post changeset_comment_path(:id => changeset, :text => "One comment too many"), :headers => auth_header + post changeset_comment_path(changeset, :text => "One comment too many"), :headers => auth_header assert_response :too_many_requests end end @@ -307,7 +307,7 @@ module Api changeset = create(:changeset, :closed) assert_difference "ChangesetComment.count", 0 do - post changeset_comment_path(:id => changeset), :params => { :text => "This is a comment" }, :headers => bearer_authorization_header(token.token) + post changeset_comment_path(changeset), :params => { :text => "This is a comment" }, :headers => bearer_authorization_header(token.token) end assert_response :forbidden @@ -316,7 +316,7 @@ module Api user.save! assert_difference "ChangesetComment.count", 1 do - post changeset_comment_path(:id => changeset), :params => { :text => "This is a comment" }, :headers => bearer_authorization_header(token.token) + post changeset_comment_path(changeset), :params => { :text => "This is a comment" }, :headers => bearer_authorization_header(token.token) end assert_response :success end @@ -330,7 +330,7 @@ module Api auth_header = basic_authorization_header user.email, "test" assert_difference "ChangesetComment.count", 0 do - post changeset_comment_path(:id => changeset, :text => "This is a comment"), :headers => auth_header + post changeset_comment_path(changeset, :text => "This is a comment"), :headers => auth_header end assert_response :forbidden @@ -339,7 +339,7 @@ module Api user.save! assert_difference "ChangesetComment.count", 1 do - post changeset_comment_path(:id => changeset, :text => "This is a comment"), :headers => auth_header + post changeset_comment_path(changeset, :text => "This is a comment"), :headers => auth_header end assert_response :success end diff --git a/test/controllers/api/changesets_controller_test.rb b/test/controllers/api/changesets_controller_test.rb index 8bb229294..cfb424169 100644 --- a/test/controllers/api/changesets_controller_test.rb +++ b/test/controllers/api/changesets_controller_test.rb @@ -396,7 +396,7 @@ module Api # check that a changeset that doesn't exist returns an appropriate message def test_show_not_found [0, -32, 233455644, "afg", "213"].each do |id| - get changeset_show_path(:id => id) + get changeset_show_path(id) assert_response :not_found, "should get a not found" rescue ActionController::UrlGenerationError => e assert_match(/No route matches/, e.to_s) @@ -424,7 +424,7 @@ module Api auth_header = basic_authorization_header user.email, "test" cs_id = changeset.id - put changeset_close_path(:id => cs_id), :headers => auth_header + put changeset_close_path(cs_id), :headers => auth_header assert_response :success # test that it really is closed now @@ -470,7 +470,7 @@ module Api # First try to do it with no auth cs_ids.each do |id| - put changeset_close_path(:id => id) + put changeset_close_path(id) assert_response :unauthorized, "Shouldn't be able close the non-existant changeset #{id}, when not authorized" rescue ActionController::UrlGenerationError => e assert_match(/No route matches/, e.to_s) @@ -479,7 +479,7 @@ module Api # Now try with auth auth_header = basic_authorization_header create(:user).email, "test" cs_ids.each do |id| - put changeset_close_path(:id => id), :headers => auth_header + put changeset_close_path(id), :headers => auth_header assert_response :not_found, "The changeset #{id} doesn't exist, so can't be closed" rescue ActionController::UrlGenerationError => e assert_match(/No route matches/, e.to_s) @@ -788,7 +788,7 @@ module Api # upload it, which used to cause an error like "PGError: ERROR: # integer out of range" (bug #2152). but shouldn't any more. - post changeset_upload_path(:id => changeset_id), :params => diff, :headers => auth_header + post changeset_upload_path(changeset_id), :params => diff, :headers => auth_header assert_response :success, "can't upload a spatially-large diff to changeset: #{@response.body}" @@ -1457,7 +1457,7 @@ module Api diff.root << modify # upload it - post changeset_upload_path(:id => changeset_id), :params => diff.to_s, :headers => auth_header + post changeset_upload_path(changeset_id), :params => diff.to_s, :headers => auth_header assert_response :success, "diff should have uploaded OK" @@ -1496,7 +1496,7 @@ module Api diff.root << modify # upload it - post changeset_upload_path(:id => changeset_id), :params => diff.to_s, :headers => auth_header + post changeset_upload_path(changeset_id), :params => diff.to_s, :headers => auth_header assert_response :success, "diff should have uploaded OK" @@ -1911,11 +1911,11 @@ module Api CHANGESET # upload it - post changeset_upload_path(:id => changeset_id), :params => diff, :headers => auth_header + post changeset_upload_path(changeset_id), :params => diff, :headers => auth_header assert_response :success, "can't upload multiple versions of an element in a diff: #{@response.body}" - get changeset_download_path(:id => changeset_id) + get changeset_download_path(changeset_id) assert_response :success assert_select "osmChange", 1 @@ -1969,11 +1969,11 @@ module Api OSMFILE # upload it - post changeset_upload_path(:id => changeset_id), :params => diff, :headers => auth_header + post changeset_upload_path(changeset_id), :params => diff, :headers => auth_header assert_response :success, "can't upload a diff from JOSM: #{@response.body}" - get changeset_download_path(:id => changeset_id) + get changeset_download_path(changeset_id) assert_response :success assert_select "osmChange", 1 @@ -2024,11 +2024,11 @@ module Api CHANGESET # upload it - post changeset_upload_path(:id => changeset_id), :params => diff, :headers => auth_header + post changeset_upload_path(changeset_id), :params => diff, :headers => auth_header assert_response :success, "can't upload multiple versions of an element in a diff: #{@response.body}" - get changeset_download_path(:id => changeset_id) + get changeset_download_path(changeset_id) assert_response :success assert_select "osmChange", 1 @@ -2121,7 +2121,7 @@ module Api end # get the bounding box back from the changeset - get changeset_show_path(:id => changeset_id) + get changeset_show_path(changeset_id) assert_response :success, "Couldn't read back changeset." assert_select "osm>changeset[min_lon='0.1000000']", 1 assert_select "osm>changeset[max_lon='0.1000000']", 1 @@ -2136,7 +2136,7 @@ module Api end # get the bounding box back from the changeset - get changeset_show_path(:id => changeset_id) + get changeset_show_path(changeset_id) assert_response :success, "Couldn't read back changeset for the second time." assert_select "osm>changeset[min_lon='0.1000000']", 1 assert_select "osm>changeset[max_lon='0.2000000']", 1 @@ -2151,7 +2151,7 @@ module Api end # get the bounding box back from the changeset - get changeset_show_path(:id => changeset_id) + get changeset_show_path(changeset_id) assert_response :success, "Couldn't read back changeset for the third time." assert_select "osm>changeset[min_lon='0.1000000']", 1 assert_select "osm>changeset[max_lon='0.3000000']", 1 @@ -2642,7 +2642,7 @@ module Api # bad changeset id assert_no_difference "changeset.subscribers.count" do - post api_changeset_unsubscribe_path(:id => 999111), :headers => auth_header + post api_changeset_unsubscribe_path(999111), :headers => auth_header end assert_response :not_found diff --git a/test/controllers/api/relations_controller_test.rb b/test/controllers/api/relations_controller_test.rb index 982df1dd7..eba3d255e 100644 --- a/test/controllers/api/relations_controller_test.rb +++ b/test/controllers/api/relations_controller_test.rb @@ -1089,7 +1089,7 @@ module Api # now download the changeset to check its bounding box with_controller(Api::ChangesetsController.new) do - get changeset_show_path(:id => changeset_id) + get changeset_show_path(changeset_id) assert_response :success, "can't re-read changeset for modify test" assert_select "osm>changeset", 1, "Changeset element doesn't exist in #{@response.body}" assert_select "osm>changeset[id='#{changeset_id}']", 1, "Changeset id=#{changeset_id} doesn't exist in #{@response.body}" @@ -1153,7 +1153,7 @@ module Api change << modify modify << doc.import(rel.find("//osm/relation").first) - post changeset_upload_path(:id => cs_id), :params => doc.to_s, :headers => headers + post changeset_upload_path(cs_id), :params => doc.to_s, :headers => headers assert_response :success, "can't upload diff relation: #{@response.body}" version = xml_parse(@response.body).find("//diffResult/relation").first["new_version"].to_i end diff --git a/test/controllers/issues_controller_test.rb b/test/controllers/issues_controller_test.rb index a988fc909..167bcc7b6 100644 --- a/test/controllers/issues_controller_test.rb +++ b/test/controllers/issues_controller_test.rb @@ -27,22 +27,22 @@ class IssuesControllerTest < ActionDispatch::IntegrationTest issue = create(:issue, :reportable => target_user, :reported_user => target_user, :assigned_role => "moderator") # Access issue without login - get issue_path(:id => issue) + get issue_path(issue) assert_redirected_to login_path(:referer => issue_path(issue)) # Access issue as normal user session_for(create(:user)) - get issue_path(:id => issue) + get issue_path(issue) assert_redirected_to :controller => :errors, :action => :forbidden # Access issue as administrator session_for(create(:administrator_user)) - get issue_path(:id => issue) + get issue_path(issue) assert_redirected_to :controller => :errors, :action => :not_found # Access issue as moderator session_for(create(:moderator_user)) - get issue_path(:id => issue) + get issue_path(issue) assert_response :success end @@ -51,22 +51,22 @@ class IssuesControllerTest < ActionDispatch::IntegrationTest issue = create(:issue, :reportable => target_user, :reported_user => target_user, :assigned_role => "administrator") # Access issue without login - get issue_path(:id => issue) + get issue_path(issue) assert_redirected_to login_path(:referer => issue_path(issue)) # Access issue as normal user session_for(create(:user)) - get issue_path(:id => issue) + get issue_path(issue) assert_redirected_to :controller => :errors, :action => :forbidden # Access issue as moderator session_for(create(:moderator_user)) - get issue_path(:id => issue) + get issue_path(issue) assert_redirected_to :controller => :errors, :action => :not_found # Access issue as administrator session_for(create(:administrator_user)) - get issue_path(:id => issue) + get issue_path(issue) assert_response :success end @@ -75,23 +75,23 @@ class IssuesControllerTest < ActionDispatch::IntegrationTest issue = create(:issue, :reportable => target_user, :reported_user => target_user, :assigned_role => "moderator") # Resolve issue without login - post resolve_issue_path(:id => issue) + post resolve_issue_path(issue) assert_response :forbidden # Resolve issue as normal user session_for(create(:user)) - post resolve_issue_path(:id => issue) + post resolve_issue_path(issue) assert_redirected_to :controller => :errors, :action => :forbidden # Resolve issue as administrator session_for(create(:administrator_user)) - post resolve_issue_path(:id => issue) + post resolve_issue_path(issue) assert_redirected_to :controller => :errors, :action => :not_found - assert_not issue.reload.resolved? + assert_not_predicate issue.reload, :resolved? # Resolve issue as moderator session_for(create(:moderator_user)) - post resolve_issue_path(:id => issue) + post resolve_issue_path(issue) assert_response :redirect assert_predicate issue.reload, :resolved? end @@ -101,23 +101,23 @@ class IssuesControllerTest < ActionDispatch::IntegrationTest issue = create(:issue, :reportable => target_user, :reported_user => target_user, :assigned_role => "administrator") # Resolve issue without login - post resolve_issue_path(:id => issue) + post resolve_issue_path(issue) assert_response :forbidden # Resolve issue as normal user session_for(create(:user)) - post resolve_issue_path(:id => issue) + post resolve_issue_path(issue) assert_redirected_to :controller => :errors, :action => :forbidden # Resolve issue as moderator session_for(create(:moderator_user)) - post resolve_issue_path(:id => issue) + post resolve_issue_path(issue) assert_redirected_to :controller => :errors, :action => :not_found - assert_not issue.reload.resolved? + assert_not_predicate issue.reload, :resolved? # Resolve issue as administrator session_for(create(:administrator_user)) - post resolve_issue_path(:id => issue) + post resolve_issue_path(issue) assert_response :redirect assert_predicate issue.reload, :resolved? end @@ -127,23 +127,23 @@ class IssuesControllerTest < ActionDispatch::IntegrationTest issue = create(:issue, :reportable => target_user, :reported_user => target_user, :assigned_role => "moderator") # Ignore issue without login - post ignore_issue_path(:id => issue) + post ignore_issue_path(issue) assert_response :forbidden # Ignore issue as normal user session_for(create(:user)) - post ignore_issue_path(:id => issue) + post ignore_issue_path(issue) assert_redirected_to :controller => :errors, :action => :forbidden # Ignore issue as administrator session_for(create(:administrator_user)) - post ignore_issue_path(:id => issue) + post ignore_issue_path(issue) assert_redirected_to :controller => :errors, :action => :not_found - assert_not issue.reload.ignored? + assert_not_predicate issue.reload, :ignored? # Ignore issue as moderator session_for(create(:moderator_user)) - post ignore_issue_path(:id => issue) + post ignore_issue_path(issue) assert_response :redirect assert_predicate issue.reload, :ignored? end @@ -153,23 +153,23 @@ class IssuesControllerTest < ActionDispatch::IntegrationTest issue = create(:issue, :reportable => target_user, :reported_user => target_user, :assigned_role => "administrator") # Ignore issue without login - post ignore_issue_path(:id => issue) + post ignore_issue_path(issue) assert_response :forbidden # Ignore issue as normal user session_for(create(:user)) - post ignore_issue_path(:id => issue) + post ignore_issue_path(issue) assert_redirected_to :controller => :errors, :action => :forbidden # Ignore issue as moderator session_for(create(:moderator_user)) - post ignore_issue_path(:id => issue) + post ignore_issue_path(issue) assert_redirected_to :controller => :errors, :action => :not_found - assert_not issue.reload.ignored? + assert_not_predicate issue.reload, :ignored? # Ignore issue as administrator session_for(create(:administrator_user)) - post ignore_issue_path(:id => issue) + post ignore_issue_path(issue) assert_response :redirect assert_predicate issue.reload, :ignored? end @@ -181,23 +181,23 @@ class IssuesControllerTest < ActionDispatch::IntegrationTest issue.resolve! # Reopen issue without login - post reopen_issue_path(:id => issue) + post reopen_issue_path(issue) assert_response :forbidden # Reopen issue as normal user session_for(create(:user)) - post reopen_issue_path(:id => issue) + post reopen_issue_path(issue) assert_redirected_to :controller => :errors, :action => :forbidden # Reopen issue as administrator session_for(create(:administrator_user)) - post reopen_issue_path(:id => issue) + post reopen_issue_path(issue) assert_redirected_to :controller => :errors, :action => :not_found - assert_not issue.reload.open? + assert_not_predicate issue.reload, :open? # Reopen issue as moderator session_for(create(:moderator_user)) - post reopen_issue_path(:id => issue) + post reopen_issue_path(issue) assert_response :redirect assert_predicate issue.reload, :open? end @@ -209,23 +209,23 @@ class IssuesControllerTest < ActionDispatch::IntegrationTest issue.resolve! # Reopen issue without login - post reopen_issue_path(:id => issue) + post reopen_issue_path(issue) assert_response :forbidden # Reopen issue as normal user session_for(create(:user)) - post reopen_issue_path(:id => issue) + post reopen_issue_path(issue) assert_redirected_to :controller => :errors, :action => :forbidden # Reopen issue as moderator session_for(create(:moderator_user)) - post reopen_issue_path(:id => issue) + post reopen_issue_path(issue) assert_redirected_to :controller => :errors, :action => :not_found - assert_not issue.reload.open? + assert_not_predicate issue.reload, :open? # Reopen issue as administrator session_for(create(:administrator_user)) - post reopen_issue_path(:id => issue) + post reopen_issue_path(issue) assert_response :redirect assert_predicate issue.reload, :open? end diff --git a/test/controllers/user_blocks_controller_test.rb b/test/controllers/user_blocks_controller_test.rb index dd0b1287b..2e9d79ef7 100644 --- a/test/controllers/user_blocks_controller_test.rb +++ b/test/controllers/user_blocks_controller_test.rb @@ -212,22 +212,9 @@ class UserBlocksControllerTest < ActionDispatch::IntegrationTest check_block_buttons block, :edit => 1 session_for(revoker_user) - check_block_buttons block - end - - private - - def check_block_buttons(block, edit: 0, revoke: 0) - [user_blocks_path, user_block_path(block)].each do |path| - get path - assert_response :success - assert_select "a[href='#{edit_user_block_path block}']", :count => edit - assert_select "a[href='#{revoke_user_block_path block}']", :count => revoke - end + check_block_buttons block, :edit => 1 end - public - ## # test the new action def test_new @@ -483,23 +470,32 @@ class UserBlocksControllerTest < ActionDispatch::IntegrationTest assert_equal "Original Reason", block.reason session_for(creator_user) + check_block_updates(block) + end + + ## + # test the update action on revoked blocks + def test_update_revoked + 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, :reason => "Original Reason") + + session_for(other_moderator_user) 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] + assert_redirected_to edit_user_block_path(block) + assert_equal "Only the moderators who created or revoked this block can edit it.", flash[:error] block.reload - assert_not block.active? - assert_equal "Updated Reason", block.reason + assert_not_predicate block, :active? + assert_equal "Original Reason", block.reason - put user_block_path(block, - :user_block_period => "0", - :user_block => { :needs_view => true, :reason => "Updated Reason 2" }) - assert_redirected_to user_block_path(block) - assert_equal "Block updated.", flash[:notice] - block.reload - assert_predicate block, :active? - assert_equal "Updated Reason 2", block.reason + session_for(creator_user) + check_block_updates(block) + + session_for(revoker_user) + check_block_updates(block) end ## @@ -794,6 +790,35 @@ class UserBlocksControllerTest < ActionDispatch::IntegrationTest private + def check_block_buttons(block, edit: 0, revoke: 0) + [user_blocks_path, user_block_path(block)].each do |path| + get path + assert_response :success + assert_select "a[href='#{edit_user_block_path block}']", :count => edit + assert_select "a[href='#{revoke_user_block_path block}']", :count => revoke + end + end + + def check_block_updates(block) + 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 "Updated Reason", block.reason + + put user_block_path(block, + :user_block_period => "0", + :user_block => { :needs_view => true, :reason => "Updated Reason 2" }) + assert_redirected_to user_block_path(block) + assert_equal "Block updated.", flash[:notice] + block.reload + assert_predicate block, :active? + assert_equal "Updated Reason 2", block.reason + end + def check_user_blocks_table(user_blocks) assert_dom "table#block_list tbody tr" do |rows| assert_equal user_blocks.count, rows.count, "unexpected number of rows in user blocks table" diff --git a/test/models/user_test.rb b/test/models/user_test.rb index 92e7d419c..10fd3d97e 100644 --- a/test/models/user_test.rb +++ b/test/models/user_test.rb @@ -111,10 +111,10 @@ class UserTest < ActiveSupport::TestCase user = build(:user) user.display_name = "user_#{existing_user.id}" - assert_not user.valid?, "user_ name is valid for existing user id when it shouldn't be" + assert_not_predicate user, :valid?, "user_ name is valid for existing user id when it shouldn't be" user.display_name = "user_#{existing_user.id + 1}" - assert_not user.valid?, "user_ name is valid for new user id when it shouldn't be" + assert_not_predicate user, :valid?, "user_ name is valid for new user id when it shouldn't be" end def test_display_name_user_id_rename @@ -122,7 +122,7 @@ class UserTest < ActiveSupport::TestCase user = create(:user) user.display_name = "user_#{existing_user.id}" - assert_not user.valid?, "user_ name is valid for existing user id when it shouldn't be" + assert_not_predicate user, :valid?, "user_ name is valid for existing user id when it shouldn't be" user.display_name = "user_#{user.id}" assert_predicate user, :valid?, "user_ name is invalid for own id, when it should be" diff --git a/yarn.lock b/yarn.lock index df127bc8c..c027f1479 100644 --- a/yarn.lock +++ b/yarn.lock @@ -19,10 +19,10 @@ resolved "https://registry.yarnpkg.com/@eslint-community/regexpp/-/regexpp-4.11.0.tgz#b0ffd0312b4a3fd2d6f77237e7248a5ad3a680ae" integrity sha512-G/M/tIiMrTAxEWRfLfQJMmGNX28IxBg4PBz8XqQhqUHLFI6TL2htpIB1iQCj144V5ee/JaKyT9/WZ0MGZWfA7A== -"@eslint/config-array@^0.17.0": - version "0.17.0" - resolved "https://registry.yarnpkg.com/@eslint/config-array/-/config-array-0.17.0.tgz#ff305e1ee618a00e6e5d0485454c8d92d94a860d" - integrity sha512-A68TBu6/1mHHuc5YJL0U0VVeGNiklLAL6rRmhTCP2B5XjWLMnrX+HkO+IAXyHvks5cyyY1jjK5ITPQ1HGS2EVA== +"@eslint/config-array@^0.17.1": + version "0.17.1" + resolved "https://registry.yarnpkg.com/@eslint/config-array/-/config-array-0.17.1.tgz#d9b8b8b6b946f47388f32bedfd3adf29ca8f8910" + integrity sha512-BlYOpej8AQ8Ev9xVqroV7a02JK3SkBAaN9GfMMH9W6Ch8FlQlkjGw4Ir7+FgYwfirivAf4t+GtzuAxqfukmISA== dependencies: "@eslint/object-schema" "^2.1.4" debug "^4.3.1" @@ -43,10 +43,10 @@ minimatch "^3.1.2" strip-json-comments "^3.1.1" -"@eslint/js@9.7.0": - version "9.7.0" - resolved "https://registry.yarnpkg.com/@eslint/js/-/js-9.7.0.tgz#b712d802582f02b11cfdf83a85040a296afec3f0" - integrity sha512-ChuWDQenef8OSFnvuxv0TCVxEwmu3+hPNKvM9B34qpM0rDRbjL8t5QkQeHHeAfsKQjuH9wS82WeCi1J/owatng== +"@eslint/js@9.8.0": + version "9.8.0" + resolved "https://registry.yarnpkg.com/@eslint/js/-/js-9.8.0.tgz#ae9bc14bb839713c5056f5018bcefa955556d3a4" + integrity sha512-MfluB7EUfxXtv3i/++oh89uzAr4PDI4nn201hsp+qaXqsjAWzinlZEHEfPgAX4doIlKvPG/i0A9dpKxOLII8yA== "@eslint/object-schema@^2.1.4": version "2.1.4" @@ -246,15 +246,15 @@ eslint-visitor-keys@^4.0.0: integrity sha512-OtIRv/2GyiF6o/d8K7MYKKbXrOUBIK6SfkIRM4Z0dY3w+LiQ0vy3F57m0Z71bjbyeiWFiHJ8brqnmE6H6/jEuw== eslint@^9.0.0: - version "9.7.0" - resolved "https://registry.yarnpkg.com/eslint/-/eslint-9.7.0.tgz#bedb48e1cdc2362a0caaa106a4c6ed943e8b09e4" - integrity sha512-FzJ9D/0nGiCGBf8UXO/IGLTgLVzIxze1zpfA8Ton2mjLovXdAPlYDv+MQDcqj3TmrhAGYfOpz9RfR+ent0AgAw== + version "9.8.0" + resolved "https://registry.yarnpkg.com/eslint/-/eslint-9.8.0.tgz#a4f4a090c8ea2d10864d89a6603e02ce9f649f0f" + integrity sha512-K8qnZ/QJzT2dLKdZJVX6W4XOwBzutMYmt0lqUS+JdXgd+HTYFlonFgkJ8s44d/zMPPCnOOk0kMWCApCPhiOy9A== dependencies: "@eslint-community/eslint-utils" "^4.2.0" "@eslint-community/regexpp" "^4.11.0" - "@eslint/config-array" "^0.17.0" + "@eslint/config-array" "^0.17.1" "@eslint/eslintrc" "^3.1.0" - "@eslint/js" "9.7.0" + "@eslint/js" "9.8.0" "@humanwhocodes/module-importer" "^1.0.1" "@humanwhocodes/retry" "^0.3.0" "@nodelib/fs.walk" "^1.2.8"