From: Anton Khorev Date: Mon, 17 Jun 2024 15:10:32 +0000 (+0300) Subject: Move diary comments hide/unhide actions to comments controller X-Git-Tag: live~391^2 X-Git-Url: https://git.openstreetmap.org./rails.git/commitdiff_plain/a128b4f585cb2f75ad1057d0b7c5af6713ed4186?ds=sidebyside Move diary comments hide/unhide actions to comments controller --- diff --git a/app/abilities/ability.rb b/app/abilities/ability.rb index 8de756ccd..c0b2f3982 100644 --- a/app/abilities/ability.rb +++ b/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 diff --git a/app/controllers/diary_comments_controller.rb b/app/controllers/diary_comments_controller.rb index f1add85f0..8abf2071b 100644 --- a/app/controllers/diary_comments_controller.rb +++ b/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 diff --git a/app/controllers/diary_entries_controller.rb b/app/controllers/diary_entries_controller.rb index d1b44acd0..bf6e8d0b1 100644 --- a/app/controllers/diary_entries_controller.rb +++ b/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 ## diff --git a/app/views/diary_entries/_diary_comment.html.erb b/app/views/diary_entries/_diary_comment.html.erb index c3c68fbc9..dbf8a439e 100644 --- a/app/views/diary_entries/_diary_comment.html.erb +++ b/app/views/diary_entries/_diary_comment.html.erb @@ -10,7 +10,7 @@

<%= diary_comment.body.to_html %>
- <% if can? :hidecomment, DiaryEntry %> + <% if can? :hide, DiaryComment %> <% 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") } %> diff --git a/config/routes.rb b/config/routes.rb index a8d2b1d7e..acf2256a3 100644 --- a/config/routes.rb +++ b/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+/ diff --git a/test/abilities/abilities_test.rb b/test/abilities/abilities_test.rb index 4947351b6..58ef2b514 100644 --- a/test/abilities/abilities_test.rb +++ b/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 diff --git a/test/controllers/diary_comments_controller_test.rb b/test/controllers/diary_comments_controller_test.rb index adb96dccb..e2ad4c91b 100644 --- a/test/controllers/diary_comments_controller_test.rb +++ b/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 diff --git a/test/controllers/diary_entries_controller_test.rb b/test/controllers/diary_entries_controller_test.rb index 1dfd5ec1a..4eeeb2742 100644 --- a/test/controllers/diary_entries_controller_test.rb +++ b/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)