diff --git a/app/controllers/comments_controller.rb b/app/controllers/comments_controller.rb index 05b7ce921ad..48b7bdadc2d 100644 --- a/app/controllers/comments_controller.rb +++ b/app/controllers/comments_controller.rb @@ -93,8 +93,10 @@ def rate_limited def check_pseud_ownership return unless params[:comment][:pseud_id] + pseud = Pseud.find(params[:comment][:pseud_id]) return if pseud && current_user && current_user.pseuds.include?(pseud) + flash[:error] = ts("You can't comment with that pseud.") redirect_to root_path end @@ -218,14 +220,17 @@ def check_not_replying_to_spam def check_permission_to_review parent = find_parent return if logged_in_as_admin? || current_user_owns?(parent) + flash[:error] = ts("Sorry, you don't have permission to see those unreviewed comments.") redirect_to logged_in? ? root_path : new_user_session_path(return_to: request.fullpath) end def check_permission_to_access_single_unreviewed return unless @comment.unreviewed? + parent = find_parent return if logged_in_as_admin? || current_user_owns?(parent) || current_user_owns?(@comment) + flash[:error] = ts("Sorry, that comment is currently in moderation.") redirect_to logged_in? ? root_path : new_user_session_path(return_to: request.fullpath) end @@ -337,7 +342,7 @@ def set_page_subtitle def index return raise_not_found if @commentable.blank? - return unless @commentable.class == Comment + return unless @commentable.instance_of?(Comment) # we link to the parent object at the top @commentable = @commentable.ultimate_parent @@ -436,7 +441,7 @@ def create elsif @comment.unreviewed? redirect_to_all_comments(@commentable) else - redirect_to_comment(@comment, { view_full_work: (params[:view_full_work] == "true"), page: params[:page] }) + redirect_to_comment(@comment, { view_full_work: (params[:view_full_work] && params[:view_full_work] == "true"), page: params[:page] }) end end end @@ -456,6 +461,7 @@ def update respond_to do |format| format.html do redirect_to comment_path(@comment) and return if @comment.unreviewed? + redirect_to_comment(@comment) end format.js # updating the comment in place @@ -487,7 +493,7 @@ def destroy redirect_to_comment(parent_comment) else flash[:comment_notice] = ts("Comment deleted.") - redirect_to_all_comments(parent, {show_comments: true}) + redirect_to_all_comments(parent, { show_comments: true }) end end @@ -612,10 +618,10 @@ def show_comments format.html do # if non-ajax it could mean sudden javascript failure OR being redirected from login # so we're being extra-nice and preserving any intention to comment along with the show comments option - options = {show_comments: true} + options = { show_comments: true } options[:add_comment_reply_id] = params[:add_comment_reply_id] if params[:add_comment_reply_id] - options[:view_full_work] = params[:view_full_work] if params[:view_full_work] options[:page] = params[:page] + options[:view_full_work] = params[:view_full_work] if params[:view_full_work] redirect_to_all_comments(@commentable, options) end @@ -640,11 +646,11 @@ def add_comment_reply @comment = Comment.new respond_to do |format| format.html do - options = {show_comments: true} + options = { show_comments: true } options[:controller] = @commentable.class.to_s.underscore.pluralize options[:anchor] = "comment_#{params[:id]}" options[:page] = params[:page] - options[:view_full_work] = params[:view_full_work] + options[:view_full_work] = params[:view_full_work] if params[:view_full_work] if @thread_view options[:id] = @thread_root options[:add_comment_reply_id] = params[:id] @@ -706,44 +712,58 @@ def cancel_comment_delete # if necessary to display it def redirect_to_comment(comment, options = {}) if comment.depth > ArchiveConfig.COMMENT_THREAD_MAX_DEPTH - if comment.ultimate_parent.is_a?(Tag) - default_options = { - controller: :comments, - action: :show, - id: comment.commentable.id, - tag_id: comment.ultimate_parent.to_param, - anchor: "comment_#{comment.id}" - } - else - default_options = { - controller: comment.commentable.class.to_s.underscore.pluralize, - action: :show, - id: (comment.commentable.is_a?(Tag) ? comment.commentable.to_param : comment.commentable.id), - anchor: "comment_#{comment.id}" - } - end + default_options = if comment.ultimate_parent.is_a?(Tag) + { + controller: :comments, + action: :show, + id: comment.commentable.id, + tag_id: comment.ultimate_parent.to_param, + anchor: "comment_#{comment.id}" + } + else + { + controller: comment.commentable.class.to_s.underscore.pluralize, + action: :show, + id: (comment.commentable.is_a?(Tag) ? comment.commentable.to_param : comment.commentable.id), + anchor: "comment_#{comment.id}" + } + end # display the comment's direct parent (and its associated thread) redirect_to(url_for(default_options.merge(options))) else # need to redirect to the specific chapter; redirect_to_all will then retrieve full work view if applicable - redirect_to_all_comments(comment.parent, options.merge({show_comments: true, anchor: "comment_#{comment.id}"})) + redirect_to_all_comments(comment.parent, options.merge({ show_comments: true, anchor: "comment_#{comment.id}" })) end end def redirect_to_all_comments(commentable, options = {}) - default_options = {anchor: "comments"} + default_options = { anchor: "comments" } options = default_options.merge(options) if commentable.is_a?(Tag) redirect_to comments_path(tag_id: commentable.to_param, - add_comment_reply_id: options[:add_comment_reply_id], - delete_comment_id: options[:delete_comment_id], - page: options[:page], - anchor: options[:anchor]) + add_comment_reply_id: options[:add_comment_reply_id], + delete_comment_id: options[:delete_comment_id], + page: options[:page], + anchor: options[:anchor]) else - if commentable.is_a?(Chapter) && (options[:view_full_work] || current_user.try(:preference).try(:view_full_works)) - commentable = commentable.work - end + is_coming_from_chapter_view = request.referer&.match(/chapters/).present? + is_not_coming_from_work_view = request.referer&.match(/works/).nil? + view_full_work = if is_coming_from_chapter_view + false + elsif is_not_coming_from_work_view + case params[:view_full_work] + when "true" + true + when "false" + false + else + current_user.try(:preference).try(:view_full_works).present? + end + else + true + end + commentable = commentable.work if commentable.is_a?(Chapter) && view_full_work redirect_to polymorphic_path(commentable, options.slice(:show_comments, :add_comment_reply_id, diff --git a/features/comments_and_kudos/comments_redirect.feature b/features/comments_and_kudos/comments_redirect.feature index ec20253a7f9..3e8cc1e5082 100644 --- a/features/comments_and_kudos/comments_redirect.feature +++ b/features/comments_and_kudos/comments_redirect.feature @@ -49,8 +49,7 @@ Scenario: Posting top level comment on a chaptered work, with view full work in And I post a comment "Woohoo" Then I should see "Woohoo" And I should see "Chapter 2" within "div#chapters" - # Once you've commented, it defaults back to your preference - And I should see "Chapter 1" within "div#chapters" + And I should not see "Chapter 1" within "div#chapters" # REPLY COMMENTS @@ -115,5 +114,4 @@ Scenario: Posting top level comment on a middle chapter, while in temporary view When I reply to a comment with "Supercalifragelistic" Then I should see "Supercalifragelistic" And I should see "Chapter 2" within "div#chapters" - # Once you've commented, it defaults back to your preference - And I should see "Chapter 1" within "div#chapters" + And I should not see "Chapter 1" within "div#chapters" diff --git a/spec/controllers/comments/comments_controller_spec.rb b/spec/controllers/comments/comments_controller_spec.rb index c6806b40b86..4c68059be55 100644 --- a/spec/controllers/comments/comments_controller_spec.rb +++ b/spec/controllers/comments/comments_controller_spec.rb @@ -79,10 +79,56 @@ end describe "GET #cancel_comment_delete" do - it "redirects to the comment on the commentable without an error" do - get :cancel_comment_delete, params: { id: comment.id } - expect(flash[:error]).to be_nil - expect(response).to redirect_to(chapter_path(comment.commentable, show_comments: true, anchor: "comment_#{comment.id}")) + context "when cancelling from chapter by chapter view" do + before do + request.env["HTTP_REFERER"] = "/works/1/chapters/1" + end + + it "redirects to the comment on the chapter view without an error" do + get :cancel_comment_delete, params: { id: comment.id } + expect(flash[:error]).to be_nil + expect(response).to redirect_to(chapter_path(comment.commentable, show_comments: true, anchor: "comment_#{comment.id}")) + end + end + + context "when cancelling from the work view" do + before do + request.env["HTTP_REFERER"] = "/works/1" + end + + it "redirects to the comment on the work view without an error" do + get :cancel_comment_delete, params: { id: comment.id } + expect(flash[:error]).to be_nil + expect(response).to redirect_to(work_path(comment.commentable.work, show_comments: true, anchor: "comment_#{comment.id}")) + end + end + + context "when cancelling from comments view" do + before do + request.env["HTTP_REFERER"] = "/comments/1" + end + + context "when user set preference to full work view" do + before do + known_user = create(:user) + known_user.preference.update!(view_full_works: true) + fake_login_known_user(known_user) + end + + it "redirects to the comment on the work view without an error" do + get :cancel_comment_delete, params: { id: comment.id } + expect(flash[:error]).to be_nil + expect(response).to redirect_to(work_path(comment.commentable.work, show_comments: true, anchor: "comment_#{comment.id}")) + end + end + + context "when user set preference to chapter by chapter view" do + it "redirects to the comment on the chapter view without an error" do + get :cancel_comment_delete, params: { id: comment.id } + expect(flash[:error]).to be_nil + expect(response).to redirect_to(chapter_path(comment.commentable, show_comments: true, anchor: "comment_#{comment.id}")) + end + end end end @@ -91,10 +137,54 @@ before { fake_login_known_user(comment.pseud.user) } context "when the format is html" do - it "redirects to the comment on the commentable without an error" do - get :cancel_comment_edit, params: { id: comment.id } - expect(flash[:error]).to be_nil - expect(response).to redirect_to(chapter_path(comment.commentable, show_comments: true, anchor: "comment_#{comment.id}")) + context "when user is navigating from chapter view" do + before do + request.env["HTTP_REFERER"] = "/works/1/chapters/1" + end + + it "redirects to the comment on the chapter view without an error" do + get :cancel_comment_edit, params: { id: comment.id } + expect(flash[:error]).to be_nil + expect(response).to redirect_to(chapter_path(comment.commentable, show_comments: true, anchor: "comment_#{comment.id}")) + end + end + + context "when user is navigating from work view" do + before do + request.env["HTTP_REFERER"] = "/works/1" + end + + it "redirects to the comment on the full work view without error" do + get :cancel_comment_edit, params: { id: comment.id } + expect(flash[:error]).to be_nil + expect(response).to redirect_to(work_path(comment.commentable.work, show_comments: true, anchor: "comment_#{comment.id}")) + end + end + + context "when user is navigating from comment view" do + before do + request.env["HTTP_REFERER"] = "/comments/1" + end + + context "when user preference is full work view" do + before do + comment.pseud.user.preference.update!(view_full_works: true) + end + + it "redirects to the comment on the full work view without error" do + get :cancel_comment_edit, params: { id: comment.id } + expect(flash[:error]).to be_nil + expect(response).to redirect_to(work_path(comment.commentable.work, show_comments: true, anchor: "comment_#{comment.id}")) + end + end + + context "when user preference is not full work view" do + it "redirects to the comment on the chapter view without error" do + get :cancel_comment_edit, params: { id: comment.id } + expect(flash[:error]).to be_nil + expect(response).to redirect_to(chapter_path(comment.commentable, show_comments: true, anchor: "comment_#{comment.id}")) + end + end end end @@ -298,7 +388,8 @@ delete :destroy, params: { id: comment.id } expect(flash[:comment_notice]).to eq "Comment deleted." it_redirects_to_simple(work_path(work, show_comments: true, anchor: :comments)) - expect { comment.reload }.to raise_exception(ActiveRecord::RecordNotFound) + expect { comment.reload } + .to raise_exception(ActiveRecord::RecordNotFound) end it "GET #add_comment_reply redirects to the work with an error" do @@ -382,7 +473,8 @@ delete :destroy, params: { id: comment.id } expect(flash[:comment_notice]).to eq "Comment deleted." it_redirects_to_simple(work_path(work, show_comments: true, anchor: :comments)) - expect { comment.reload }.to raise_exception(ActiveRecord::RecordNotFound) + expect { comment.reload } + .to raise_exception(ActiveRecord::RecordNotFound) end end end diff --git a/spec/controllers/comments/default_rails_actions_spec.rb b/spec/controllers/comments/default_rails_actions_spec.rb index 35d57d398c3..2b36e0ef265 100644 --- a/spec/controllers/comments/default_rails_actions_spec.rb +++ b/spec/controllers/comments/default_rails_actions_spec.rb @@ -325,6 +325,106 @@ end end + context "when work is not restricted for a logged in user" do + let(:work) { create(:work) } + let(:user) { create(:user) } + let(:comment_attributes) do + { + pseud_id: user.default_pseud_id, + comment_content: "Hello fellow human!" + } + end + before { fake_login_known_user(user) } + + context "when user is commenting from a work view" do + before do + request.env["HTTP_REFERER"] = "/works/1" + end + + it "redirects to the work view" do + post :create, params: { work_id: work.id, comment: comment_attributes } + commentable_chapter = work.chapters.last + created_comment = user.comments.where(commentable: commentable_chapter).last + it_redirects_to_with_comment_notice( + work_path(work.id, show_comments: true, anchor: "comment_#{created_comment.id}"), + "Comment created!" + ) + end + end + + context "when user is commenting from a chapter view" do + before do + request.env["HTTP_REFERER"] = "/works/1/chapters/2" + end + + it "redirects to the chapter view" do + post :create, params: { work_id: work.id, comment: comment_attributes } + commentable_chapter = work.chapters.last + created_comment = user.comments.where(commentable: commentable_chapter).last + it_redirects_to_with_comment_notice( + chapter_path(commentable_chapter.id, show_comments: true, anchor: "comment_#{created_comment.id}"), + "Comment created!" + ) + end + end + + context "when user is commenting from neither a work view nor chapter view" do + context "when param view_full_work == true" do + it "redirects to the work view" do + post :create, params: { work_id: work.id, comment: comment_attributes, view_full_work: true } + commentable_chapter = work.chapters.last + created_comment = user.comments.where(commentable: commentable_chapter).last + it_redirects_to_with_comment_notice( + work_path(work.id, show_comments: true, anchor: "comment_#{created_comment.id}", view_full_work: true), + "Comment created!" + ) + end + end + + context "when param view_full_work == false" do + it "redirects to the chapter view" do + post :create, params: { work_id: work.id, comment: comment_attributes, view_full_work: false } + commentable_chapter = work.chapters.last + created_comment = user.comments.where(commentable: commentable_chapter).last + it_redirects_to_with_comment_notice( + chapter_path(commentable_chapter.id, show_comments: true, anchor: "comment_#{created_comment.id}", view_full_work: false), + "Comment created!" + ) + end + end + + context "when param view_full_work is nil" do + context "when user preference is set to view full work" do + before do + user.preference.update!(view_full_works: true) + end + + it "redirects to the full work view" do + post :create, params: { work_id: work.id, comment: comment_attributes } + commentable_chapter = work.chapters.last + created_comment = user.comments.where(commentable: commentable_chapter).last + it_redirects_to_with_comment_notice( + work_path(work.id, show_comments: true, anchor: "comment_#{created_comment.id}"), + "Comment created!" + ) + end + end + + context "when user preference is not set to view full work" do + it "redirects to the chapter view" do + post :create, params: { work_id: work.id, comment: comment_attributes } + commentable_chapter = work.chapters.last + created_comment = user.comments.where(commentable: commentable_chapter).last + it_redirects_to_with_comment_notice( + chapter_path(commentable_chapter.id, show_comments: true, anchor: "comment_#{created_comment.id}"), + "Comment created!" + ) + end + end + end + end + end + context "when the work has all comments disabled" do let(:work) { create(:work, comment_permissions: :disable_all) } @@ -520,7 +620,7 @@ post :create, params: { work_id: work.id, comment: comment_attributes } comment = Comment.last expect(flash[:error]).to be_nil - expect(response).to redirect_to(chapter_path(comment.commentable, show_comments: true, view_full_work: false, anchor: "comment_#{comment.id}")) + expect(response).to redirect_to(chapter_path(comment.commentable, show_comments: true, anchor: "comment_#{comment.id}")) end end end @@ -638,7 +738,7 @@ fake_login_known_user(user) post :create, params: { work_id: work.id, comment: comment_attributes } comment = assigns[:comment] - it_redirects_to_with_comment_notice(chapter_path(comment.commentable, show_comments: true, view_full_work: false, anchor: "comment_#{comment.id}"), "Comment created!") + it_redirects_to_with_comment_notice(chapter_path(comment.commentable, show_comments: true, anchor: "comment_#{comment.id}"), "Comment created!") expect(comment.user_agent.length).to eq(500) end end @@ -656,7 +756,7 @@ fake_login_known_user(user) post :create, params: { work_id: work.id, comment: comment_attributes } comment = assigns[:comment] - it_redirects_to_with_comment_notice(chapter_path(comment.commentable, show_comments: true, view_full_work: false, anchor: "comment_#{comment.id}"), "Comment created!") + it_redirects_to_with_comment_notice(chapter_path(comment.commentable, show_comments: true, anchor: "comment_#{comment.id}"), "Comment created!") expect(comment.user_agent).to be_nil end end