Move diary comments hide/unhide actions to comments controller

app/abilities/ability.rb

@@ -54,7 +54,7 @@ class Ability
         can [:index, :create, :destroy], UserMute
 
         if user.moderator?
-          can [:hide, :unhide, :hidecomment, :unhidecomment], DiaryEntry
+          can [:hide, :unhide], [DiaryEntry, DiaryComment]
           can [:index, :show, :resolve, :ignore, :reopen], Issue
           can :create, IssueComment
           can [:new, :create, :edit, :update, :destroy], Redaction
@@ -62,7 +62,7 @@ class Ability
         end
 
         if user.administrator?
-          can [:hide, :unhide, :hidecomment, :unhidecomment], DiaryEntry
+          can [:hide, :unhide], [DiaryEntry, DiaryComment]
           can [:index, :show, :resolve, :ignore, :reopen], Issue
           can :create, IssueComment
           can [:set_status, :destroy, :index], User

app/controllers/diary_comments_controller.rb

@@ -11,6 +11,7 @@ class DiaryCommentsController < ApplicationController
   authorize_resource
 
   before_action :lookup_user, :only => :index
+  before_action :check_database_writable, :only => [:hide, :unhide]
 
   allow_thirdparty_images :only => :index
 
@@ -18,10 +19,22 @@ class DiaryCommentsController < ApplicationController
     @title = t ".title", :user => @user.display_name
 
     comments = DiaryComment.where(:user => @user)
-    comments = comments.visible unless can? :unhidecomment, DiaryEntry
+    comments = comments.visible unless can? :unhide, DiaryComment
 
     @params = params.permit(:display_name, :before, :after)
 
     @comments, @newer_comments_id, @older_comments_id = get_page_items(comments, :includes => [:user])
   end
+
+  def hide
+    comment = DiaryComment.find(params[:comment])
+    comment.update(:visible => false)
+    redirect_to diary_entry_path(comment.diary_entry.user, comment.diary_entry)
+  end
+
+  def unhide
+    comment = DiaryComment.find(params[:comment])
+    comment.update(:visible => true)
+    redirect_to diary_entry_path(comment.diary_entry.user, comment.diary_entry)
+  end
 end

app/controllers/diary_entries_controller.rb

@@ -11,7 +11,7 @@ class DiaryEntriesController < ApplicationController
   authorize_resource
 
   before_action :lookup_user, :only => :show
-  before_action :check_database_writable, :only => [:new, :create, :edit, :update, :comment, :hide, :hidecomment, :subscribe, :unsubscribe]
+  before_action :check_database_writable, :only => [:new, :create, :edit, :update, :comment, :hide, :unhide, :subscribe, :unsubscribe]
 
   allow_thirdparty_images :only => [:new, :create, :edit, :update, :index, :show]
 
@@ -69,7 +69,7 @@ class DiaryEntriesController < ApplicationController
     if @entry
       @title = t ".title", :user => params[:display_name], :title => @entry.title
       @og_image = @entry.body.image
-      @comments = can?(:unhidecomment, DiaryEntry) ? @entry.comments : @entry.visible_comments
+      @comments = can?(:unhide, DiaryComment) ? @entry.comments : @entry.visible_comments
     else
       @title = t "diary_entries.no_such_entry.title", :id => params[:id]
       render :action => "no_such_entry", :status => :not_found
@@ -229,18 +229,6 @@ class DiaryEntriesController < ApplicationController
     redirect_to :action => "index", :display_name => entry.user.display_name
   end
 
-  def hidecomment
-    comment = DiaryComment.find(params[:comment])
-    comment.update(:visible => false)
-    redirect_to diary_entry_path(comment.diary_entry.user, comment.diary_entry)
-  end
-
-  def unhidecomment
-    comment = DiaryComment.find(params[:comment])
-    comment.update(:visible => true)
-    redirect_to diary_entry_path(comment.diary_entry.user, comment.diary_entry)
-  end
-
   private
 
   ##

app/views/diary_entries/_diary_comment.html.erb

@@ -10,7 +10,7 @@
     </p>
 
     <div class="richtext text-break"><%= diary_comment.body.to_html %></div>
-    <% if can? :hidecomment, DiaryEntry %>
+    <% if can? :hide, DiaryComment %>
       <span>
         <% if diary_comment.visible? %>
           <%= link_to t(".hide_link"), hide_diary_comment_path(diary_comment.diary_entry.user, diary_comment.diary_entry, diary_comment), :method => :post, :data => { :confirm => t(".confirm") } %>

config/routes.rb

@@ -246,8 +246,8 @@ OpenStreetMap::Application.routes.draw do
   post "/user/:display_name/diary/:id/newcomment" => "diary_entries#comment", :id => /\d+/, :as => :comment_diary_entry
   post "/user/:display_name/diary/:id/hide" => "diary_entries#hide", :id => /\d+/, :as => :hide_diary_entry
   post "/user/:display_name/diary/:id/unhide" => "diary_entries#unhide", :id => /\d+/, :as => :unhide_diary_entry
-  post "/user/:display_name/diary/:id/hidecomment/:comment" => "diary_entries#hidecomment", :id => /\d+/, :comment => /\d+/, :as => :hide_diary_comment
-  post "/user/:display_name/diary/:id/unhidecomment/:comment" => "diary_entries#unhidecomment", :id => /\d+/, :comment => /\d+/, :as => :unhide_diary_comment
+  post "/user/:display_name/diary/:id/hidecomment/:comment" => "diary_comments#hide", :id => /\d+/, :comment => /\d+/, :as => :hide_diary_comment
+  post "/user/:display_name/diary/:id/unhidecomment/:comment" => "diary_comments#unhide", :id => /\d+/, :comment => /\d+/, :as => :unhide_diary_comment
   match "/user/:display_name/diary/:id/subscribe" => "diary_entries#subscribe", :via => [:get, :post], :as => :diary_entry_subscribe, :id => /\d+/
   match "/user/:display_name/diary/:id/unsubscribe" => "diary_entries#unsubscribe", :via => [:get, :post], :as => :diary_entry_unsubscribe, :id => /\d+/
 

test/abilities/abilities_test.rb

@@ -25,9 +25,13 @@ class GuestAbilityTest < AbilityTest
       assert ability.can?(action, DiaryComment), "should be able to #{action} DiaryComments"
     end
 
-    [:create, :edit, :comment, :subscribe, :unsubscribe, :hide, :hidecomment].each do |action|
+    [:create, :edit, :comment, :subscribe, :unsubscribe, :hide, :unhide].each do |action|
       assert ability.cannot?(action, DiaryEntry), "should not be able to #{action} DiaryEntries"
     end
+
+    [:hide, :unhide].each do |action|
+      assert ability.cannot?(action, DiaryComment), "should not be able to #{action} DiaryComments"
+    end
   end
 
   test "note permissions for a guest" do
@@ -59,8 +63,9 @@ class UserAbilityTest < AbilityTest
       assert ability.can?(action, DiaryComment), "should be able to #{action} DiaryComments"
     end
 
-    [:hide, :hidecomment].each do |action|
+    [:hide, :unhide].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} DiaryComment"
     end
 
     [:index, :show, :resolve, :ignore, :reopen].each do |action|
@@ -85,8 +90,9 @@ class ModeratorAbilityTest < AbilityTest
       assert ability.cannot?(action, UserRole), "should not be able to #{action} UserRoles"
     end
 
-    [:hide, :hidecomment].each do |action|
+    [:hide, :unhide].each do |action|
       assert ability.can?(action, DiaryEntry), "should be able to #{action} DiaryEntries"
+      assert ability.can?(action, DiaryComment), "should be able to #{action} DiaryComment"
     end
   end
 end
@@ -94,11 +100,11 @@ end
 class AdministratorAbilityTest < AbilityTest
   test "Diary for an administrator" do
     ability = Ability.new create(:administrator_user)
-    [:index, :rss, :show, :create, :edit, :comment, :subscribe, :unsubscribe, :hide, :hidecomment].each do |action|
+    [:index, :rss, :show, :create, :edit, :comment, :subscribe, :unsubscribe, :hide, :unhide].each do |action|
       assert ability.can?(action, DiaryEntry), "should be able to #{action} DiaryEntries"
     end
 
-    [:index].each do |action|
+    [:index, :hide, :unhide].each do |action|
       assert ability.can?(action, DiaryComment), "should be able to #{action} DiaryComments"
     end
   end

test/controllers/diary_comments_controller_test.rb

@@ -12,6 +12,14 @@ class DiaryCommentsControllerTest < ActionDispatch::IntegrationTest
       { :path => "/user/username/diary/comments", :method => :get },
       { :controller => "diary_comments", :action => "index", :display_name => "username" }
     )
+    assert_routing(
+      { :path => "/user/username/diary/1/hidecomment/2", :method => :post },
+      { :controller => "diary_comments", :action => "hide", :display_name => "username", :id => "1", :comment => "2" }
+    )
+    assert_routing(
+      { :path => "/user/username/diary/1/unhidecomment/2", :method => :post },
+      { :controller => "diary_comments", :action => "unhide", :display_name => "username", :id => "1", :comment => "2" }
+    )
 
     get "/user/username/diary/comments/1"
     assert_redirected_to "/user/username/diary/comments"
@@ -60,4 +68,68 @@ class DiaryCommentsControllerTest < ActionDispatch::IntegrationTest
       assert_redirected_to :controller => :errors, :action => :bad_request
     end
   end
+
+  def test_hide
+    user = create(:user)
+    diary_entry = create(:diary_entry, :user => user)
+    diary_comment = create(:diary_comment, :diary_entry => diary_entry)
+
+    # Try without logging in
+    post hide_diary_comment_path(user, diary_entry, diary_comment)
+    assert_response :forbidden
+    assert DiaryComment.find(diary_comment.id).visible
+
+    # Now try as a normal user
+    session_for(user)
+    post hide_diary_comment_path(user, diary_entry, diary_comment)
+    assert_redirected_to :controller => :errors, :action => :forbidden
+    assert DiaryComment.find(diary_comment.id).visible
+
+    # Try as a moderator
+    session_for(create(:moderator_user))
+    post hide_diary_comment_path(user, diary_entry, diary_comment)
+    assert_redirected_to diary_entry_path(user, diary_entry)
+    assert_not DiaryComment.find(diary_comment.id).visible
+
+    # Reset
+    diary_comment.reload.update(:visible => true)
+
+    # Finally try as an administrator
+    session_for(create(:administrator_user))
+    post hide_diary_comment_path(user, diary_entry, diary_comment)
+    assert_redirected_to diary_entry_path(user, diary_entry)
+    assert_not DiaryComment.find(diary_comment.id).visible
+  end
+
+  def test_unhide
+    user = create(:user)
+    diary_entry = create(:diary_entry, :user => user)
+    diary_comment = create(:diary_comment, :diary_entry => diary_entry, :visible => false)
+
+    # Try without logging in
+    post unhide_diary_comment_path(user, diary_entry, diary_comment)
+    assert_response :forbidden
+    assert_not DiaryComment.find(diary_comment.id).visible
+
+    # Now try as a normal user
+    session_for(user)
+    post unhide_diary_comment_path(user, diary_entry, diary_comment)
+    assert_redirected_to :controller => :errors, :action => :forbidden
+    assert_not DiaryComment.find(diary_comment.id).visible
+
+    # Now try as a moderator
+    session_for(create(:moderator_user))
+    post unhide_diary_comment_path(user, diary_entry, diary_comment)
+    assert_redirected_to diary_entry_path(user, diary_entry)
+    assert DiaryComment.find(diary_comment.id).visible
+
+    # Reset
+    diary_comment.reload.update(:visible => true)
+
+    # Finally try as an administrator
+    session_for(create(:administrator_user))
+    post unhide_diary_comment_path(user, diary_entry, diary_comment)
+    assert_redirected_to diary_entry_path(user, diary_entry)
+    assert DiaryComment.find(diary_comment.id).visible
+  end
 end

test/controllers/diary_entries_controller_test.rb

@@ -81,14 +81,6 @@ class DiaryEntriesControllerTest < ActionDispatch::IntegrationTest
       { :path => "/user/username/diary/1/unhide", :method => :post },
       { :controller => "diary_entries", :action => "unhide", :display_name => "username", :id => "1" }
     )
-    assert_routing(
-      { :path => "/user/username/diary/1/hidecomment/2", :method => :post },
-      { :controller => "diary_entries", :action => "hidecomment", :display_name => "username", :id => "1", :comment => "2" }
-    )
-    assert_routing(
-      { :path => "/user/username/diary/1/unhidecomment/2", :method => :post },
-      { :controller => "diary_entries", :action => "unhidecomment", :display_name => "username", :id => "1", :comment => "2" }
-    )
     assert_routing(
       { :path => "/user/username/diary/1/subscribe", :method => :get },
       { :controller => "diary_entries", :action => "subscribe", :display_name => "username", :id => "1" }
@@ -828,70 +820,6 @@ class DiaryEntriesControllerTest < ActionDispatch::IntegrationTest
     assert DiaryEntry.find(diary_entry.id).visible
   end
 
-  def test_hidecomment
-    user = create(:user)
-    diary_entry = create(:diary_entry, :user => user)
-    diary_comment = create(:diary_comment, :diary_entry => diary_entry)
-
-    # Try without logging in
-    post hide_diary_comment_path(user, diary_entry, diary_comment)
-    assert_response :forbidden
-    assert DiaryComment.find(diary_comment.id).visible
-
-    # Now try as a normal user
-    session_for(user)
-    post hide_diary_comment_path(user, diary_entry, diary_comment)
-    assert_redirected_to :controller => :errors, :action => :forbidden
-    assert DiaryComment.find(diary_comment.id).visible
-
-    # Try as a moderator
-    session_for(create(:moderator_user))
-    post hide_diary_comment_path(user, diary_entry, diary_comment)
-    assert_redirected_to :action => :show, :display_name => user.display_name, :id => diary_entry.id
-    assert_not DiaryComment.find(diary_comment.id).visible
-
-    # Reset
-    diary_comment.reload.update(:visible => true)
-
-    # Finally try as an administrator
-    session_for(create(:administrator_user))
-    post hide_diary_comment_path(user, diary_entry, diary_comment)
-    assert_redirected_to :action => :show, :display_name => user.display_name, :id => diary_entry.id
-    assert_not DiaryComment.find(diary_comment.id).visible
-  end
-
-  def test_unhidecomment
-    user = create(:user)
-    diary_entry = create(:diary_entry, :user => user)
-    diary_comment = create(:diary_comment, :diary_entry => diary_entry, :visible => false)
-
-    # Try without logging in
-    post unhide_diary_comment_path(user, diary_entry, diary_comment)
-    assert_response :forbidden
-    assert_not DiaryComment.find(diary_comment.id).visible
-
-    # Now try as a normal user
-    session_for(user)
-    post unhide_diary_comment_path(user, diary_entry, diary_comment)
-    assert_redirected_to :controller => :errors, :action => :forbidden
-    assert_not DiaryComment.find(diary_comment.id).visible
-
-    # Now try as a moderator
-    session_for(create(:moderator_user))
-    post unhide_diary_comment_path(user, diary_entry, diary_comment)
-    assert_redirected_to :action => :show, :display_name => user.display_name, :id => diary_entry.id
-    assert DiaryComment.find(diary_comment.id).visible
-
-    # Reset
-    diary_comment.reload.update(:visible => true)
-
-    # Finally try as an administrator
-    session_for(create(:administrator_user))
-    post unhide_diary_comment_path(user, diary_entry, diary_comment)
-    assert_redirected_to :action => :show, :display_name => user.display_name, :id => diary_entry.id
-    assert DiaryComment.find(diary_comment.id).visible
-  end
-
   def test_subscribe_page
     user = create(:user)
     other_user = create(:user)