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