From 6624beff110cbbe20709e2ca18dcc9668c3126fe Mon Sep 17 00:00:00 2001 From: Anton Khorev Date: Mon, 10 Jun 2024 16:09:07 +0300 Subject: [PATCH] Move diary comments index action to comments controller --- app/abilities/ability.rb | 3 +- app/controllers/diary_comments_controller.rb | 27 ++++++++ app/controllers/diary_entries_controller.rb | 15 +---- .../index.html.erb} | 4 +- config/locales/en.yml | 15 ++--- config/routes.rb | 2 +- test/abilities/abilities_test.rb | 18 +++++- .../diary_comments_controller_test.rb | 63 +++++++++++++++++++ .../diary_entries_controller_test.rb | 52 --------------- 9 files changed, 120 insertions(+), 79 deletions(-) create mode 100644 app/controllers/diary_comments_controller.rb rename app/views/{diary_entries/comments.html.erb => diary_comments/index.html.erb} (89%) create mode 100644 test/controllers/diary_comments_controller_test.rb diff --git a/app/abilities/ability.rb b/app/abilities/ability.rb index 3aba63c330..8de756ccda 100644 --- a/app/abilities/ability.rb +++ b/app/abilities/ability.rb @@ -18,7 +18,8 @@ def initialize(user) can [:index, :feed, :show], Changeset can :index, ChangesetComment can [:confirm, :confirm_resend, :confirm_email], :confirmation - can [:index, :rss, :show, :comments], DiaryEntry + can [:index, :rss, :show], DiaryEntry + can :index, DiaryComment can [:index], Note can [:new, :create, :edit, :update], :password can [:index, :show], Redaction diff --git a/app/controllers/diary_comments_controller.rb b/app/controllers/diary_comments_controller.rb new file mode 100644 index 0000000000..f1add85f03 --- /dev/null +++ b/app/controllers/diary_comments_controller.rb @@ -0,0 +1,27 @@ +class DiaryCommentsController < ApplicationController + include UserMethods + include PaginationMethods + + layout "site" + + before_action :authorize_web + before_action :set_locale + before_action :check_database_readable + + authorize_resource + + before_action :lookup_user, :only => :index + + allow_thirdparty_images :only => :index + + def index + @title = t ".title", :user => @user.display_name + + comments = DiaryComment.where(:user => @user) + comments = comments.visible unless can? :unhidecomment, DiaryEntry + + @params = params.permit(:display_name, :before, :after) + + @comments, @newer_comments_id, @older_comments_id = get_page_items(comments, :includes => [:user]) + end +end diff --git a/app/controllers/diary_entries_controller.rb b/app/controllers/diary_entries_controller.rb index f3fbcd1fd0..d1b44acd0e 100644 --- a/app/controllers/diary_entries_controller.rb +++ b/app/controllers/diary_entries_controller.rb @@ -10,10 +10,10 @@ class DiaryEntriesController < ApplicationController authorize_resource - before_action :lookup_user, :only => [:show, :comments] + before_action :lookup_user, :only => :show before_action :check_database_writable, :only => [:new, :create, :edit, :update, :comment, :hide, :hidecomment, :subscribe, :unsubscribe] - allow_thirdparty_images :only => [:new, :create, :edit, :update, :index, :show, :comments] + allow_thirdparty_images :only => [:new, :create, :edit, :update, :index, :show] def index if params[:display_name] @@ -241,17 +241,6 @@ def unhidecomment redirect_to diary_entry_path(comment.diary_entry.user, comment.diary_entry) end - def comments - @title = t ".title", :user => @user.display_name - - comments = DiaryComment.where(:user => @user) - comments = comments.visible unless can? :unhidecomment, DiaryEntry - - @params = params.permit(:display_name, :before, :after) - - @comments, @newer_comments_id, @older_comments_id = get_page_items(comments, :includes => [:user]) - end - private ## diff --git a/app/views/diary_entries/comments.html.erb b/app/views/diary_comments/index.html.erb similarity index 89% rename from app/views/diary_entries/comments.html.erb rename to app/views/diary_comments/index.html.erb index aa5c163847..0dd03d9d09 100644 --- a/app/views/diary_entries/comments.html.erb +++ b/app/views/diary_comments/index.html.erb @@ -27,8 +27,8 @@ <%= render "shared/pagination", - :newer_key => "diary_entries.comments.newer_comments", - :older_key => "diary_entries.comments.older_comments", + :newer_key => "diary_comments.index.newer_comments", + :older_key => "diary_comments.index.older_comments", :newer_id => @newer_comments_id, :older_id => @older_comments_id %> <% end -%> diff --git a/config/locales/en.yml b/config/locales/en.yml index 32ecbf7cd0..e8dde95213 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -592,7 +592,14 @@ en: all: title: "OpenStreetMap diary entries" description: "Recent diary entries from users of OpenStreetMap" - comments: + subscribe: + heading: Subscribe to the following diary entry discussion? + button: Subscribe to discussion + unsubscribe: + heading: Unsubscribe from the following diary entry discussion? + button: Unsubscribe from discussion + diary_comments: + index: title: "Diary Comments added by %{user}" heading: "%{user}'s Diary Comments" subheading_html: "Diary Comments added by %{user}" @@ -602,12 +609,6 @@ en: comment: Comment newer_comments: "Newer Comments" older_comments: "Older Comments" - subscribe: - heading: Subscribe to the following diary entry discussion? - button: Subscribe to discussion - unsubscribe: - heading: Unsubscribe from the following diary entry discussion? - button: Unsubscribe from discussion doorkeeper: errors: messages: diff --git a/config/routes.rb b/config/routes.rb index c44064ba32..a8d2b1d7e6 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -237,7 +237,7 @@ get "/diary/:language/rss" => "diary_entries#rss", :defaults => { :format => :rss } get "/diary/rss" => "diary_entries#rss", :defaults => { :format => :rss } get "/user/:display_name/diary/comments/:page", :page => /[1-9][0-9]*/, :to => redirect(:path => "/user/%{display_name}/diary/comments") - get "/user/:display_name/diary/comments" => "diary_entries#comments", :as => :diary_comments + get "/user/:display_name/diary/comments" => "diary_comments#index", :as => :diary_comments get "/user/:display_name/diary" => "diary_entries#index" get "/diary/:language" => "diary_entries#index" scope "/user/:display_name" do diff --git a/test/abilities/abilities_test.rb b/test/abilities/abilities_test.rb index 139f270fee..4947351b6f 100644 --- a/test/abilities/abilities_test.rb +++ b/test/abilities/abilities_test.rb @@ -17,10 +17,14 @@ class GuestAbilityTest < AbilityTest test "diary permissions for a guest" do ability = Ability.new nil - [:index, :rss, :show, :comments].each do |action| + [:index, :rss, :show].each do |action| assert ability.can?(action, DiaryEntry), "should be able to #{action} DiaryEntries" end + [:index].each do |action| + assert ability.can?(action, DiaryComment), "should be able to #{action} DiaryComments" + end + [:create, :edit, :comment, :subscribe, :unsubscribe, :hide, :hidecomment].each do |action| assert ability.cannot?(action, DiaryEntry), "should not be able to #{action} DiaryEntries" end @@ -47,10 +51,14 @@ class UserAbilityTest < AbilityTest test "Diary permissions" do ability = Ability.new create(:user) - [:index, :rss, :show, :comments, :create, :edit, :comment, :subscribe, :unsubscribe].each do |action| + [:index, :rss, :show, :create, :edit, :comment, :subscribe, :unsubscribe].each do |action| assert ability.can?(action, DiaryEntry), "should be able to #{action} DiaryEntries" end + [:index].each do |action| + assert ability.can?(action, DiaryComment), "should be able to #{action} DiaryComments" + end + [:hide, :hidecomment].each do |action| assert ability.cannot?(action, DiaryEntry), "should not be able to #{action} DiaryEntries" end @@ -86,9 +94,13 @@ class ModeratorAbilityTest < AbilityTest class AdministratorAbilityTest < AbilityTest test "Diary for an administrator" do ability = Ability.new create(:administrator_user) - [:index, :rss, :show, :comments, :create, :edit, :comment, :subscribe, :unsubscribe, :hide, :hidecomment].each do |action| + [:index, :rss, :show, :create, :edit, :comment, :subscribe, :unsubscribe, :hide, :hidecomment].each do |action| assert ability.can?(action, DiaryEntry), "should be able to #{action} DiaryEntries" end + + [:index].each do |action| + assert ability.can?(action, DiaryComment), "should be able to #{action} DiaryComments" + end end test "User Roles permissions for an administrator" do diff --git a/test/controllers/diary_comments_controller_test.rb b/test/controllers/diary_comments_controller_test.rb new file mode 100644 index 0000000000..adb96dccba --- /dev/null +++ b/test/controllers/diary_comments_controller_test.rb @@ -0,0 +1,63 @@ +require "test_helper" + +class DiaryCommentsControllerTest < ActionDispatch::IntegrationTest + def setup + super + # Create the default language for diary entries + create(:language, :code => "en") + end + + def test_routes + assert_routing( + { :path => "/user/username/diary/comments", :method => :get }, + { :controller => "diary_comments", :action => "index", :display_name => "username" } + ) + + get "/user/username/diary/comments/1" + assert_redirected_to "/user/username/diary/comments" + end + + def test_index + user = create(:user) + other_user = create(:user) + suspended_user = create(:user, :suspended) + deleted_user = create(:user, :deleted) + + # Test a user with no comments + get diary_comments_path(:display_name => user.display_name) + assert_response :success + assert_template :index + assert_select "h4", :html => "No diary comments" + + # Test a user with a comment + create(:diary_comment, :user => other_user) + + get diary_comments_path(:display_name => other_user.display_name) + assert_response :success + assert_template :index + assert_dom "a[href='#{user_path(other_user)}']", :text => other_user.display_name + assert_select "table.table-striped tbody" do + assert_select "tr", :count => 1 + end + + # Test a suspended user + get diary_comments_path(:display_name => suspended_user.display_name) + assert_response :not_found + + # Test a deleted user + get diary_comments_path(:display_name => deleted_user.display_name) + assert_response :not_found + end + + def test_index_invalid_paged + user = create(:user) + + %w[-1 0 fred].each do |id| + get diary_comments_path(:display_name => user.display_name, :before => id) + assert_redirected_to :controller => :errors, :action => :bad_request + + get diary_comments_path(:display_name => user.display_name, :after => id) + assert_redirected_to :controller => :errors, :action => :bad_request + end + end +end diff --git a/test/controllers/diary_entries_controller_test.rb b/test/controllers/diary_entries_controller_test.rb index b6d11c62af..1dfd5ec1a2 100644 --- a/test/controllers/diary_entries_controller_test.rb +++ b/test/controllers/diary_entries_controller_test.rb @@ -49,11 +49,6 @@ def test_routes { :controller => "diary_entries", :action => "rss", :display_name => "username", :format => :rss } ) - assert_routing( - { :path => "/user/username/diary/comments", :method => :get }, - { :controller => "diary_entries", :action => "comments", :display_name => "username" } - ) - assert_routing( { :path => "/diary/new", :method => :get }, { :controller => "diary_entries", :action => "new" } @@ -110,9 +105,6 @@ def test_routes { :path => "/user/username/diary/1/unsubscribe", :method => :post }, { :controller => "diary_entries", :action => "unsubscribe", :display_name => "username", :id => "1" } ) - - get "/user/username/diary/comments/1" - assert_redirected_to "/user/username/diary/comments" end def test_new_no_login @@ -900,50 +892,6 @@ def test_unhidecomment assert DiaryComment.find(diary_comment.id).visible end - def test_comments - user = create(:user) - other_user = create(:user) - suspended_user = create(:user, :suspended) - deleted_user = create(:user, :deleted) - - # Test a user with no comments - get diary_comments_path(:display_name => user.display_name) - assert_response :success - assert_template :comments - assert_select "h4", :html => "No diary comments" - - # Test a user with a comment - create(:diary_comment, :user => other_user) - - get diary_comments_path(:display_name => other_user.display_name) - assert_response :success - assert_template :comments - assert_dom "a[href='#{user_path(other_user)}']", :text => other_user.display_name - assert_select "table.table-striped tbody" do - assert_select "tr", :count => 1 - end - - # Test a suspended user - get diary_comments_path(:display_name => suspended_user.display_name) - assert_response :not_found - - # Test a deleted user - get diary_comments_path(:display_name => deleted_user.display_name) - assert_response :not_found - end - - def test_comments_invalid_paged - user = create(:user) - - %w[-1 0 fred].each do |id| - get diary_comments_path(:display_name => user.display_name, :before => id) - assert_redirected_to :controller => :errors, :action => :bad_request - - get diary_comments_path(:display_name => user.display_name, :after => id) - assert_redirected_to :controller => :errors, :action => :bad_request - end - end - def test_subscribe_page user = create(:user) other_user = create(:user)