From: Tom Hughes <tom@compton.nu>
Date: Tue, 18 Jun 2019 20:10:07 +0000 (+0100)
Subject: Merge remote-tracking branch 'upstream/pull/2226'
X-Git-Tag: live~3559
X-Git-Url: https://git.openstreetmap.org./rails.git/commitdiff_plain/31edc7dff6f4305366d0f93d35ba3fb1956bb71f?hp=657ed03d8b2e262bc965cc38e8af190624c3295b

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

diff --git a/app/abilities/ability.rb b/app/abilities/ability.rb
index 28380392d..c34f357a9 100644
--- a/app/abilities/ability.rb
+++ b/app/abilities/ability.rb
@@ -44,6 +44,7 @@ class Ability
         can [:account, :go_public, :make_friend, :remove_friend], User
 
         if user.moderator?
+          can [:hide, :hidecomment], DiaryEntry
           can [:index, :show, :resolve, :ignore, :reopen], Issue
           can :create, IssueComment
           can [:new, :create, :edit, :update, :destroy], Redaction
@@ -51,7 +52,7 @@ class Ability
         end
 
         if user.administrator?
-          can [:hide, :unhide, :hidecomment, :unhidecomment], [DiaryEntry, DiaryComment]
+          can [:hide, :unhide, :hidecomment, :unhidecomment], DiaryEntry
           can [:index, :show, :resolve, :ignore, :reopen], Issue
           can :create, IssueComment
           can [:set_status, :delete, :index], User
diff --git a/app/views/diary_entries/_diary_comment.html.erb b/app/views/diary_entries/_diary_comment.html.erb
index 301739eef..639ac9a19 100644
--- a/app/views/diary_entries/_diary_comment.html.erb
+++ b/app/views/diary_entries/_diary_comment.html.erb
@@ -7,7 +7,7 @@
   </p>
 
   <div class="richtext"><%= diary_comment.body.to_html %></div>
-  <% if current_user && current_user.administrator? %>
+  <% if can? :hidecomment, DiaryEntry %>
     <span>
       <% if diary_comment.visible? %>
         <%= link_to t(".hide_link"), hide_diary_comment_path(:display_name => diary_comment.diary_entry.user.display_name, :id => diary_comment.diary_entry.id, :comment => diary_comment.id), :method => :post, :data => { :confirm => t(".confirm") } %>
diff --git a/app/views/diary_entries/_diary_entry.html.erb b/app/views/diary_entries/_diary_entry.html.erb
index 5674a37b7..c7dbec386 100644
--- a/app/views/diary_entries/_diary_entry.html.erb
+++ b/app/views/diary_entries/_diary_entry.html.erb
@@ -37,7 +37,7 @@
       </li>
     <% end %>
 
-    <% if current_user && current_user.administrator? %>
+    <% if can? :hide, DiaryEntry %>
       <li>
         <% if diary_entry.visible %>
           <%= link_to t(".hide_link"), hide_diary_entry_path(:display_name => diary_entry.user.display_name, :id => diary_entry.id), :method => :post, :data => { :confirm => t(".confirm") } %>
diff --git a/test/abilities/abilities_test.rb b/test/abilities/abilities_test.rb
index f43b6bf50..99154b4b7 100644
--- a/test/abilities/abilities_test.rb
+++ b/test/abilities/abilities_test.rb
@@ -23,7 +23,6 @@ class GuestAbilityTest < AbilityTest
 
     [:create, :edit, :comment, :subscribe, :unsubscribe, :hide, :hidecomment].each do |action|
       assert ability.cannot?(action, DiaryEntry), "should not be able to #{action} DiaryEntries"
-      assert ability.cannot?(action, DiaryComment), "should not be able to #{action} DiaryEntries"
     end
   end
 
@@ -54,7 +53,6 @@ class UserAbilityTest < AbilityTest
 
     [:hide, :hidecomment].each do |action|
       assert ability.cannot?(action, DiaryEntry), "should not be able to #{action} DiaryEntries"
-      assert ability.cannot?(action, DiaryComment), "should not be able to #{action} DiaryEntries"
     end
 
     [:index, :show, :resolve, :ignore, :reopen].each do |action|
@@ -78,6 +76,10 @@ class ModeratorAbilityTest < AbilityTest
     [:grant, :revoke].each do |action|
       assert ability.cannot?(action, UserRole), "should not be able to #{action} UserRoles"
     end
+
+    [:hide, :hidecomment].each do |action|
+      assert ability.can?(action, DiaryEntry), "should be able to #{action} DiaryEntries"
+    end
   end
 end
 
@@ -87,10 +89,6 @@ class AdministratorAbilityTest < AbilityTest
     [:index, :rss, :show, :comments, :create, :edit, :comment, :subscribe, :unsubscribe, :hide, :hidecomment].each do |action|
       assert ability.can?(action, DiaryEntry), "should be able to #{action} DiaryEntries"
     end
-
-    [:hide, :hidecomment].each do |action|
-      assert ability.can?(action, DiaryComment), "should be able to #{action} DiaryComment"
-    end
   end
 
   test "User Roles permissions for an administrator" do
diff --git a/test/controllers/diary_entries_controller_test.rb b/test/controllers/diary_entries_controller_test.rb
index 2b4230db7..fd0375823 100644
--- a/test/controllers/diary_entries_controller_test.rb
+++ b/test/controllers/diary_entries_controller_test.rb
@@ -733,9 +733,9 @@ class DiaryEntriesControllerTest < ActionController::TestCase
 
   def test_hide
     user = create(:user)
+    diary_entry = create(:diary_entry, :user => user)
 
     # Try without logging in
-    diary_entry = create(:diary_entry, :user => user)
     post :hide,
          :params => { :display_name => user.display_name, :id => diary_entry.id }
     assert_response :forbidden
@@ -749,6 +749,17 @@ class DiaryEntriesControllerTest < ActionController::TestCase
     assert_redirected_to :controller => :errors, :action => :forbidden
     assert_equal true, DiaryEntry.find(diary_entry.id).visible
 
+    # Now try as a moderator
+    post :hide,
+         :params => { :display_name => user.display_name, :id => diary_entry.id },
+         :session => { :user => create(:moderator_user) }
+    assert_response :redirect
+    assert_redirected_to :action => :index, :display_name => user.display_name
+    assert_equal false, DiaryEntry.find(diary_entry.id).visible
+
+    # Reset
+    diary_entry.reload.update(:visible => true)
+
     # Finally try as an administrator
     post :hide,
          :params => { :display_name => user.display_name, :id => diary_entry.id },
@@ -787,9 +798,9 @@ class DiaryEntriesControllerTest < ActionController::TestCase
 
   def test_hidecomment
     user = create(:user)
-    administrator_user = create(:administrator_user)
     diary_entry = create(:diary_entry, :user => user)
     diary_comment = create(:diary_comment, :diary_entry => diary_entry)
+
     # Try without logging in
     post :hidecomment,
          :params => { :display_name => user.display_name, :id => diary_entry.id, :comment => diary_comment.id }
@@ -804,10 +815,21 @@ class DiaryEntriesControllerTest < ActionController::TestCase
     assert_redirected_to :controller => :errors, :action => :forbidden
     assert_equal true, DiaryComment.find(diary_comment.id).visible
 
+    # Try as a moderator
+    post :hidecomment,
+         :params => { :display_name => user.display_name, :id => diary_entry.id, :comment => diary_comment.id },
+         :session => { :user => create(:moderator_user) }
+    assert_response :redirect
+    assert_redirected_to :action => :show, :display_name => user.display_name, :id => diary_entry.id
+    assert_equal false, DiaryComment.find(diary_comment.id).visible
+
+    # Reset
+    diary_comment.reload.update(:visible => true)
+
     # Finally try as an administrator
     post :hidecomment,
          :params => { :display_name => user.display_name, :id => diary_entry.id, :comment => diary_comment.id },
-         :session => { :user => administrator_user }
+         :session => { :user => create(:administrator_user) }
     assert_response :redirect
     assert_redirected_to :action => :show, :display_name => user.display_name, :id => diary_entry.id
     assert_equal false, DiaryComment.find(diary_comment.id).visible