From e1e3604f46d1c0c6daa762f4071c06a85ae062a7 Mon Sep 17 00:00:00 2001 From: evazion Date: Tue, 19 Jan 2021 03:31:53 -0600 Subject: [PATCH] comments: rework deleted comments. Let users see when a post has deleted comments. Show normal users a '[deleted]' placeholder when a comment is deleted. Show the full comment to moderators. Also fix it so that the comment creator can't edit or undelete deleted comments, and users can't vote on or report deleted comments. Finally, hide the creator_id, updater_id, and body of deleted comments in the API. --- CHANGELOG.md | 9 +++ app/components/comment_component.rb | 17 +++-- .../comment_component.html.erb | 63 ++++++++++++------- app/policies/comment_policy.rb | 27 +++++++- app/views/comments/_index_by_comment.html.erb | 18 +++--- test/components/comment_component_test.rb | 23 +++++++ test/functional/comments_controller_test.rb | 37 ++++++++--- 7 files changed, 141 insertions(+), 53 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 538b07159..977e23bfc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,15 @@ * You can now see the list of comments and forum posts you've reported to the moderators at . +* You can now see when a post has deleted comments. Deleted comments are now + replaced with the word `[deleted]`, instead of being completely hidden. + +### API Changes + +* Deleted comments now have some of their fields hidden in the API. The + `creator_id`, `updater_id`, and `body` fields are hidden if you're not a + moderator. + ## 2021-01-12 ### Changes diff --git a/app/components/comment_component.rb b/app/components/comment_component.rb index 8dfede7f3..5ea7d34a1 100644 --- a/app/components/comment_component.rb +++ b/app/components/comment_component.rb @@ -1,27 +1,26 @@ # frozen_string_literal: true class CommentComponent < ApplicationComponent - attr_reader :comment, :context, :dtext_data, :show_deleted, :current_user + attr_reader :comment, :context, :dtext_data, :current_user delegate :link_to_user, :time_ago_in_words_tagged, :format_text, to: :helpers - def initialize(comment:, current_user:, context: nil, dtext_data: nil, show_deleted: false) + def initialize(comment:, current_user:, context: nil, dtext_data: nil) @comment = comment @context = context @dtext_data = dtext_data - @show_deleted = show_deleted @current_user = current_user end - def render? - !comment.is_deleted? || show_deleted || current_user.is_moderator? - end - def dimmed? - !comment.is_sticky? && comment.score < current_user.comment_threshold/2.0 + comment.is_deleted? || (!comment.is_sticky? && comment.score < current_user.comment_threshold/2.0) end def thresholded? - !comment.is_sticky? && comment.score < current_user.comment_threshold + !comment.is_deleted? && !comment.is_sticky? && comment.score < current_user.comment_threshold + end + + def redact_deleted? + comment.is_deleted? && !policy(comment).can_see_deleted? end def has_moderation_reports? diff --git a/app/components/comment_component/comment_component.html.erb b/app/components/comment_component/comment_component.html.erb index 45cb60574..1267e9a29 100644 --- a/app/components/comment_component/comment_component.html.erb +++ b/app/components/comment_component/comment_component.html.erb @@ -2,8 +2,8 @@
- <%= link_to_user comment.creator %> - <% if comment.is_deleted? %> - (deleted) + <% if redact_deleted? %> + [deleted] + <% else %> + <%= link_to_user comment.creator %> + <% if comment.is_deleted? %> + (deleted) + <% end %> <% end %>
<%= link_to time_ago_in_words_tagged(comment.created_at), post_path(comment.post, anchor: "comment_#{comment.id}"), class: "message-timestamp" %> @@ -27,26 +31,34 @@ <% end %> <%= tag.div class: "body prose", style: ("display: none;" if thresholded?) do %> - <%= format_text(comment.body, data: dtext_data) %> + <% if redact_deleted? %> +

[deleted]

+ <% else %> + <%= format_text(comment.body, data: dtext_data) %> + <% end %> + <%= render "application/update_notice", record: comment %> <% end %> - <% if policy(comment).create? %> - + + <% if policy(comment).reply? %> <% if context == :index_by_comment %>
  • <%= link_to "Reply", new_comment_path(id: comment, comment: { post_id: comment.post_id }), class: "reply-link" %>
    • <% else %>
  • <%= link_to "Reply", new_comment_path(id: comment, comment: { post_id: comment.post_id }), class: "reply-link", remote: true %>
    • <% end %> + <% end %> - <% if policy(comment).update? %> - <% if comment.is_deleted? %> -
  • <%= link_to "Undelete", undelete_comment_path(comment.id), method: :post, remote: true %>
    • - <% else %> -
  • <%= link_to "Delete", comment_path(comment.id), "data-confirm": "Are you sure you want to delete this comment?", method: :delete, remote: true %>
    • - <% end %> -
  • <%= link_to "Edit", edit_comment_path(comment.id), id: "edit_comment_link_#{comment.id}", class: "edit_comment_link" %>
    • + <% if policy(comment).update? %> + <% if comment.is_deleted? %> +
  • <%= link_to "Undelete", undelete_comment_path(comment.id), method: :post, remote: true %>
    • + <% else %> +
  • <%= link_to "Delete", comment_path(comment.id), "data-confirm": "Are you sure you want to delete this comment?", method: :delete, remote: true %>
    • <% end %> +
  • <%= link_to "Edit", edit_comment_path(comment.id), id: "edit_comment_link_#{comment.id}", class: "edit_comment_link" %>
    • + <% end %> + + <% if policy(comment).vote? %>
  • <%= link_to "Vote up", comment_comment_votes_path(comment_id: comment.id, score: "up"), method: :post, remote: true %>
    • @@ -56,16 +68,19 @@
  • <%= link_to "Unvote", comment_comment_votes_path(comment_id: comment.id), method: :delete, remote: true %>
    • - <% if policy(comment).reportable? %> -
  • <%= link_to "Report", new_moderation_report_path(moderation_report: { model_type: "Comment", model_id: comment.id }), remote: true %>
    • - <% end %> - <% if has_moderation_reports? %> -
  • This comment has been reported! (<%= link_to pluralize(comment.moderation_reports.length, "report"), moderation_reports_path(search: { model_type: "Comment", model_id: comment.id }) %>)
    • - <% end %> -     - <% if policy(comment).update? %> - <%= render "comments/form", comment: comment, hidden: true %> <% end %> + + <% if policy(comment).reportable? %> +
  • <%= link_to "Report", new_moderation_report_path(moderation_report: { model_type: "Comment", model_id: comment.id }), remote: true %>
    • + <% end %> + + <% if has_moderation_reports? %> +
  • This comment has been reported! (<%= link_to pluralize(comment.moderation_reports.length, "report"), moderation_reports_path(search: { model_type: "Comment", model_id: comment.id }) %>)
    • + <% end %> +     + + <% if policy(comment).update? %> + <%= render "comments/form", comment: comment, hidden: true %> <% end %>
    diff --git a/app/policies/comment_policy.rb b/app/policies/comment_policy.rb index 2d0583275..fc5faa8b8 100644 --- a/app/policies/comment_policy.rb +++ b/app/policies/comment_policy.rb @@ -1,16 +1,33 @@ class CommentPolicy < ApplicationPolicy + def create? + unbanned? + end + def update? - unbanned? && (user.is_moderator? || record.updater_id == user.id) + unbanned? && (user.is_moderator? || (record.updater_id == user.id && !record.is_deleted?)) end def reportable? - unbanned? && record.creator_id != user.id && !record.creator.is_moderator? + unbanned? && record.creator_id != user.id && !record.creator.is_moderator? && !record.is_deleted? end def can_sticky_comment? user.is_moderator? end + def can_see_deleted? + user.is_moderator? + end + + def reply? + create? && !record.is_deleted? + end + + def vote? + # XXX should use CommentVotePolicy + unbanned? && !record.is_deleted? + end + def permitted_attributes_for_create [:body, :post_id, :do_not_bump_post, (:is_sticky if can_sticky_comment?)].compact end @@ -19,5 +36,11 @@ class CommentPolicy < ApplicationPolicy [:body, :is_deleted, (:is_sticky if can_sticky_comment?)].compact end + def api_attributes + attributes = super + attributes -= [:creator_id, :updater_id, :body] if record.is_deleted? && !can_see_deleted? + attributes + end + alias_method :undelete?, :update? end diff --git a/app/views/comments/_index_by_comment.html.erb b/app/views/comments/_index_by_comment.html.erb index 752f2c0cd..206ba5a98 100644 --- a/app/views/comments/_index_by_comment.html.erb +++ b/app/views/comments/_index_by_comment.html.erb @@ -1,16 +1,16 @@
    <% dtext_data = DText.preprocess(@comments.map(&:body)) %> + <% @comments.each do |comment| %> - <% if CurrentUser.is_moderator? || (params[:search] && params[:search][:is_deleted] =~ /t/) || !comment.is_deleted? %> - <%= tag.div id: "post_#{comment.post.id}", **PostPreviewComponent.new(post: comment.post).article_attrs("post") do %> -
    - <% if policy(comment.post).visible? %> - <%= link_to(image_tag(comment.post.preview_file_url), post_path(comment.post)) %> - <% end %> -
    - <%= render_comment(comment, dtext_data: dtext_data, context: :index_by_comment, show_deleted: params.dig(:search, :is_deleted).to_s.truthy?, current_user: CurrentUser.user) %> - <% end %> + <%= tag.div id: "post_#{comment.post.id}", **PostPreviewComponent.new(post: comment.post).article_attrs("post") do %> +
    + <% if policy(comment.post).visible? %> + <%= link_to(image_tag(comment.post.preview_file_url), post_path(comment.post)) %> + <% end %> +
    + + <%= render_comment(comment, dtext_data: dtext_data, context: :index_by_comment, current_user: CurrentUser.user) %> <% end %> <% end %>
    diff --git a/test/components/comment_component_test.rb b/test/components/comment_component_test.rb index 2167dee8d..d5f329d58 100644 --- a/test/components/comment_component_test.rb +++ b/test/components/comment_component_test.rb @@ -26,6 +26,29 @@ class CommentComponentTest < ViewComponent::TestCase end end + context "for a deleted comment" do + setup do + @deleted_comment = as(create(:user)) { create(:comment, is_deleted: true) } + end + + should "have the creator and body hidden for a Member" do + render_comment(@deleted_comment, current_user: @deleted_comment.creator) + + assert_css("article[data-is-dimmed=true]") + assert_css("article .author-name", text: "[deleted]") + assert_css("article .body p", text: "[deleted]") + end + + should "be visible for a Moderator" do + render_comment(@deleted_comment, current_user: create(:moderator_user)) + + assert_css("article[data-is-dimmed=true]") + assert_no_css("article .unhide-comment-link") + assert_css("article .author-name", text: @deleted_comment.creator.pretty_name) + assert_css("article .body p", text: @deleted_comment.body) + end + end + context "for a comment with moderation reports" do should "show the report notice to moderators" do create(:moderation_report, model: @comment) diff --git a/test/functional/comments_controller_test.rb b/test/functional/comments_controller_test.rb index 859b7c13e..987e12605 100644 --- a/test/functional/comments_controller_test.rb +++ b/test/functional/comments_controller_test.rb @@ -113,6 +113,19 @@ class CommentsControllerTest < ActionDispatch::IntegrationTest get comment_path(@comment.id) assert_redirected_to post_path(@comment.post, anchor: "comment_#{@comment.id}") end + + context "for a deleted comment" do + should "not show the creator, updater, or body to non-Moderators" do + @comment = create(:comment, post: @post, is_deleted: true) + get comment_path(@comment.id), as: :json + + assert_response :success + assert_equal(@comment.id, response.parsed_body["id"]) + assert_nil(response.parsed_body["creator_id"]) + assert_nil(response.parsed_body["updater_id"]) + assert_nil(response.parsed_body["body"]) + end + end end context "edit action" do @@ -157,6 +170,16 @@ class CommentsControllerTest < ActionDispatch::IntegrationTest end end + context "for a deleted comment" do + should "not allow the creator to edit the comment" do + @comment.update!(is_deleted: true) + put_auth comment_path(@comment.id), @user, params: { comment: { body: "blah" }} + + assert_response 403 + assert_not_equal("blah", @comment.reload.body) + end + end + should "update the body" do put_auth comment_path(@comment.id), @user, params: {comment: {body: "abc"}} assert_equal("abc", @comment.reload.body) @@ -224,20 +247,16 @@ class CommentsControllerTest < ActionDispatch::IntegrationTest end context "undelete action" do - should "mark comment as undeleted" do + should "allow Moderators to undelete comments" do @comment = create(:comment, post: @post, is_deleted: true) - post_auth undelete_comment_path(@comment.id), @user + post_auth undelete_comment_path(@comment.id), @mod - assert_equal(false, @comment.reload.is_deleted) assert_redirected_to(@comment) + assert_equal(false, @comment.reload.is_deleted) end - should "not allow undeleting comments deleted by a moderator" do - @comment = create(:comment, post: @post) - - delete_auth comment_path(@comment.id), @mod - assert_redirected_to @comment - assert(@comment.reload.is_deleted?) + should "not allow normal Members to undelete their own comments" do + @comment = create(:comment, post: @post, is_deleted: true) post_auth undelete_comment_path(@comment.id), @user assert_response 403