From be36968b6d8ae4b2c8d2b2dfd544eada20046bea Mon Sep 17 00:00:00 2001 From: evazion Date: Sat, 31 Aug 2019 16:15:31 -0500 Subject: [PATCH] Fix #3351: Mod+: Treat deleted comments as below score threshold. Comments have three states: visible, hidden, and invisible. Visible comments are always shown. Hidden comments are not shown until the user clicks 'Show all comments'. Invisible comments are never shown to the user. Deleted comments are treated as hidden for moderators and invisible for normal users. Thresholded comments are treated as hidden for all users. --- app/controllers/posts_controller.rb | 2 +- app/models/comment.rb | 17 +++- app/views/comments/_index_by_post.html.erb | 4 +- .../comments/partials/index/_list.html.erb | 2 +- test/functional/comments_controller_test.rb | 95 +++++++++++++++---- test/functional/posts_controller_test.rb | 6 +- 6 files changed, 99 insertions(+), 27 deletions(-) diff --git a/app/controllers/posts_controller.rb b/app/controllers/posts_controller.rb index 768bb2415..46734e2e6 100644 --- a/app/controllers/posts_controller.rb +++ b/app/controllers/posts_controller.rb @@ -26,7 +26,7 @@ class PostsController < ApplicationController @comments = @post.comments @comments = @comments.includes(:creator) @comments = @comments.includes(:votes) if CurrentUser.is_member? - @comments = @comments.select { |c| c.visible_by?(CurrentUser.user) } + @comments = @comments.visible(CurrentUser.user) include_deleted = @post.is_deleted? || (@post.parent_id.present? && @post.parent.is_deleted?) || CurrentUser.user.show_deleted_children? @parent_post_set = PostSets::PostRelationship.new(@post.parent_id, :include_deleted => include_deleted) diff --git a/app/models/comment.rb b/app/models/comment.rb index a03f949e4..ed77ca463 100644 --- a/app/models/comment.rb +++ b/app/models/comment.rb @@ -130,10 +130,19 @@ class Comment < ApplicationRecord user.id.in?(votes.map(&:user_id)) end - def visible_by?(user, show_thresholded: false, show_deleted: false) - return false if is_deleted? && !show_deleted && !user.is_moderator? - return false if score < user.comment_threshold && !is_sticky? && !show_thresholded - true + def visibility(user) + return :invisible if is_deleted? && !user.is_moderator? + return :hidden if is_deleted? && user.is_moderator? + return :hidden if score < user.comment_threshold && !is_sticky? + return :visible + end + + def self.hidden(user) + select { |comment| comment.visibility(user) == :hidden } + end + + def self.visible(user) + select { |comment| comment.visibility(user) == :visible } end def hidden_attributes diff --git a/app/views/comments/_index_by_post.html.erb b/app/views/comments/_index_by_post.html.erb index 08faa93af..17d9d1298 100644 --- a/app/views/comments/_index_by_post.html.erb +++ b/app/views/comments/_index_by_post.html.erb @@ -10,12 +10,12 @@ <% end %> <% @posts.select(&:visible?).each do |post| %> - <% if post.comments.any? { |c| c.visible_by?(CurrentUser.user, show_thresholded: true) } %> + <% if post.comments.visible(CurrentUser.user).any? || post.comments.hidden(CurrentUser.user).any? %> <%= content_tag(:div, { id: "post_#{post.id}", class: ["post", *PostPresenter.preview_class(post)].join(" ") }.merge(PostPresenter.data_attributes(post))) do %>
<%= link_to(image_tag(post.preview_file_url), post_path(post)) %>
- <%= render "comments/partials/index/list", post: post, comments: post.comments.select { |c| c.visible_by?(CurrentUser.user) }.last(6), page: :comments %> + <%= render "comments/partials/index/list", post: post, comments: post.comments.visible(CurrentUser.user).last(6), page: :comments %>
<% end %> <% end %> diff --git a/app/views/comments/partials/index/_list.html.erb b/app/views/comments/partials/index/_list.html.erb index a424094c8..21f3c4690 100644 --- a/app/views/comments/partials/index/_list.html.erb +++ b/app/views/comments/partials/index/_list.html.erb @@ -4,7 +4,7 @@ <% end %>
- <% if post.comments.any? { |c| !c.visible_by?(CurrentUser.user, show_deleted: true) } || (page == :comments && post.comments.size > 6) %> + <% if post.comments.hidden(CurrentUser.user).any? || (page == :comments && post.comments.size > 6) %> <%= link_to "Show all comments", comments_path(post_id: post.id), id: "show-all-comments-link", remote: true %> diff --git a/test/functional/comments_controller_test.rb b/test/functional/comments_controller_test.rb index 6f9de1a73..59badcecd 100644 --- a/test/functional/comments_controller_test.rb +++ b/test/functional/comments_controller_test.rb @@ -5,15 +5,12 @@ class CommentsControllerTest < ActionDispatch::IntegrationTest setup do @mod = FactoryBot.create(:moderator_user) @user = FactoryBot.create(:member_user) + @post = create(:post) + CurrentUser.user = @user CurrentUser.ip_addr = "127.0.0.1" Danbooru.config.stubs(:member_comment_time_threshold).returns(1.week.from_now) - - @post = FactoryBot.create(:post) - @comment = FactoryBot.create(:comment, :post => @post) - CurrentUser.scoped(@mod) do - @mod_comment = FactoryBot.create(:comment, :post => @post) - end + Danbooru.config.stubs(:member_comment_limit).returns(100) end teardown do @@ -22,14 +19,70 @@ class CommentsControllerTest < ActionDispatch::IntegrationTest end context "index action" do - should "render for post" do - get comments_path(post_id: @post.id, group_by: "post", format: "js"), xhr: true - assert_response :success - end + context "grouped by post" do + should "render all comments for .js" do + get comments_path(post_id: @post.id, group_by: "post", format: "js"), xhr: true + assert_response :success + end - should "render by post" do - get comments_path(group_by: "post") - assert_response :success + should "show posts with visible comments" do + @comment = as(@user) { create(:comment, post: @post) } + get comments_path(group_by: "post") + + assert_response :success + assert_select "#post_#{@post.id}", 1 + assert_select "#post_#{@post.id} .comment", 1 + assert_select "#post_#{@post.id} #show-all-comments-link", 0 + end + + should "show the 'Show all comments' link on posts with thresholded comments" do + as(@user) { create(:comment, post: @post, score: -10) } + get comments_path(group_by: "post") + + assert_response :success + assert_select "#post_#{@post.id}", 1 + assert_select "#post_#{@post.id} #show-all-comments-link", 1 + assert_select "#post_#{@post.id} .comment", 0 + assert_select "#post_#{@post.id} .list-of-comments", /There are no visible comments/ + end + + should "not show the 'Show all comments' link on posts with deleted comments to Members" do + @comment1 = as(@user) { create(:comment, post: @post) } + @comment2 = as(@user) { create(:comment, post: @post, is_deleted: true) } + get comments_path(group_by: "post") + + assert_response :success + assert_select "#post_#{@post.id}", 1 + assert_select "#post_#{@post.id} .comment", 1 + assert_select "#post_#{@post.id} #show-all-comments-link", 0 + end + + should "show the 'Show all comments' link on posts with deleted comments to Moderators" do + @comment1 = as(@user) { create(:comment, post: @post) } + @comment2 = as(@user) { create(:comment, post: @post, is_deleted: true) } + get_auth comments_path(group_by: "post"), @mod + + assert_response :success + assert_select "#post_#{@post.id}", 1 + assert_select "#post_#{@post.id} .comment", 1 + assert_select "#post_#{@post.id} #show-all-comments-link", 1 + end + + should "not bump posts with nonbumping comments" do + as(@user) { create(:comment, post: @post, do_not_bump_post: true) } + get comments_path(group_by: "post") + + assert_response :success + assert_select "#post_#{@post.id}", 0 + end + + should "not bump posts with only deleted comments" do + as(@user) { create(:comment, post: @post, is_deleted: true) } + get comments_path(group_by: "post") + + assert_response :success + assert_select "#post_#{@post.id}", 0 + end end should "render by comment" do @@ -45,6 +98,7 @@ class CommentsControllerTest < ActionDispatch::IntegrationTest context "search action" do should "render" do + @comment = create(:comment, post: @post) get search_comments_path assert_response :success end @@ -52,6 +106,7 @@ class CommentsControllerTest < ActionDispatch::IntegrationTest context "show action" do should "render" do + @comment = create(:comment, post: @post) get comment_path(@comment.id) assert_response :success end @@ -59,12 +114,17 @@ class CommentsControllerTest < ActionDispatch::IntegrationTest context "edit action" do should "render" do + @comment = create(:comment, post: @post) get_auth edit_comment_path(@comment.id), @user assert_response :success end end context "update action" do + setup do + @comment = create(:comment, post: @post) + end + context "when updating another user's comment" do should "succeed if updater is a moderator" do put_auth comment_path(@comment.id), @user, params: {comment: {body: "abc"}} @@ -73,6 +133,7 @@ class CommentsControllerTest < ActionDispatch::IntegrationTest end should "fail if updater is not a moderator" do + @mod_comment = as(@mod) { create(:comment, post: @post) } put_auth comment_path(@mod_comment.id), @user, params: {comment: {body: "abc"}} assert_not_equal("abc", @mod_comment.reload.body) assert_response 403 @@ -141,19 +202,19 @@ class CommentsControllerTest < ActionDispatch::IntegrationTest context "destroy action" do should "mark comment as deleted" do + @comment = create(:comment, post: @post) delete_auth comment_path(@comment.id), @user + assert_equal(true, @comment.reload.is_deleted) assert_redirected_to @comment end end context "undelete action" do - setup do - @comment.delete! - end - should "mark comment as undeleted" do + @comment = create(:comment, post: @post, is_deleted: true) post_auth undelete_comment_path(@comment.id), @user + assert_equal(false, @comment.reload.is_deleted) assert_redirected_to(@comment) end diff --git a/test/functional/posts_controller_test.rb b/test/functional/posts_controller_test.rb index 39d0e1626..cbed45d93 100644 --- a/test/functional/posts_controller_test.rb +++ b/test/functional/posts_controller_test.rb @@ -157,12 +157,14 @@ class PostsControllerTest < ActionDispatch::IntegrationTest assert_select "div.list-of-comments p", /There are no comments/ end - should "show deleted comments to moderators" do + should "not show deleted comments to moderators by default, but allow them to be unhidden" do mod = create(:mod_user) get_auth post_path(@post), mod, params: { id: @post.id } assert_response :success - assert_select "article.comment", 1 + assert_select "article.comment", 0 + assert_select "a#show-all-comments-link", 1 + assert_select "div.list-of-comments p", /There are no comments/ end end