Merge branch 'pull/5020'

diff --git a/app/controllers/user_blocks_controller.rb b/app/controllers/user_blocks_controller.rb
index 3ab217a601fc5bfe1319aefffb5bb2ce8510314b..664ac56815d6e541de2de2e839b04aa188867f38 100644 (file)
--- 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 461dc7a8d9e8ef6c19b34a80ce1a05900c17455f..a18d1dbdbabe51a753515e138fad795483d2b422 100644 (file)
--- a/app/views/user_blocks/_block.html.erb
+++ b/app/views/user_blocks/_block.html.erb
@@ -15,7 +15,8 @@
     <% end %>
   </td>
   <td><%= link_to t(".show"), block %></td>
-  <td><% if current_user and current_user.id == block.creator_id %><%= link_to t(".edit"), edit_user_block_path(block) %><% end %></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>
   <% if show_revoke_link %>
   <td><% if block.active? %><%= link_to t(".revoke"), revoke_user_block_path(block) %><% end %></td>
   <% end %>

diff --git a/app/views/user_blocks/show.html.erb b/app/views/user_blocks/show.html.erb
index 619cd6c3f0c58d3fdee045698ca72b59882fd0b2..c36c043cf80434d2f3120bf767154c69fb33323a 100644 (file)
--- a/app/views/user_blocks/show.html.erb
+++ b/app/views/user_blocks/show.html.erb
@@ -26,9 +26,12 @@
   <dd class="col-sm-9"><div class="richtext text-break"><%= @user_block.reason.to_html %></div></dd>
 </dl>
 
-<% 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? %>
   <div>
-    <% 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 2b83fec1f9d4097cb4e8036f1e2cc8314fe3bd47..dc7f1a1c0e76206f329668cac9f6363533210023 100644 (file)
--- 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 12f6325ae3d78acce9469195f084a4776804e4c9..c832cbb35866fcbfb0f22073e56f8ef5b05fa7b3 100644 (file)
--- 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 01d5ecb5d30363492a8a7fbd30c244c6610d063c..f479b24b3cb99b3b9f68921fd772086f69bd4103 100644 (file)
--- 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 8bb22929444de2e8da30460c5b190544419cc8d9..cfb4241691116597cdcd8c77a293396d14d9e559 100644 (file)
--- 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 982df1dd7e5706bc4c5f17f9c39656a4e3a55b44..eba3d255ef800d28cab49956699fd25237ffb132 100644 (file)
--- 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 a988fc909f7cc3a73ad3417db5ca43958d8c4b01..167bcc7b6ab5ffd2123febf1987d0ee198d004a9 100644 (file)
--- 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 dd0b1287b414c7805395a82bcccb62c0b5c773e6..2e9d79ef759e8add899921259fc241ad3f37b3de 100644 (file)
--- 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 92e7d419c33c50f7ec91d1899581793ffd72508a..10fd3d97ec9cb533494e72946a049def4c936620 100644 (file)
--- 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_<id> name is valid for existing user id when it shouldn't be"
+    assert_not_predicate user, :valid?, "user_<id> 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_<id> name is valid for new user id when it shouldn't be"
+    assert_not_predicate user, :valid?, "user_<id> 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_<id> name is valid for existing user id when it shouldn't be"
+    assert_not_predicate user, :valid?, "user_<id> name is valid for existing user id when it shouldn't be"
 
     user.display_name = "user_#{user.id}"
     assert_predicate user, :valid?, "user_<id> name is invalid for own id, when it should be"

diff --git a/yarn.lock b/yarn.lock
index df127bc8c9bb8d9acb1f032fb2810301fce4c1ae..c027f1479470789c624d8af2b4fe2dde480c4b13 100644 (file)
--- 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"