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.
This commit is contained in:
evazion
2021-01-19 03:31:53 -06:00
parent 07bdc6eab0
commit e1e3604f46
7 changed files with 141 additions and 53 deletions

View File

@@ -15,6 +15,15 @@
* You can now see the list of comments and forum posts you've reported to * You can now see the list of comments and forum posts you've reported to
the moderators at <https://danbooru.donmai.us/moderation_reports>. the moderators at <https://danbooru.donmai.us/moderation_reports>.
* 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 ## 2021-01-12
### Changes ### Changes

View File

@@ -1,27 +1,26 @@
# frozen_string_literal: true # frozen_string_literal: true
class CommentComponent < ApplicationComponent 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 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 @comment = comment
@context = context @context = context
@dtext_data = dtext_data @dtext_data = dtext_data
@show_deleted = show_deleted
@current_user = current_user @current_user = current_user
end end
def render?
!comment.is_deleted? || show_deleted || current_user.is_moderator?
end
def dimmed? 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 end
def thresholded? 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 end
def has_moderation_reports? def has_moderation_reports?

View File

@@ -2,8 +2,8 @@
<article id="comment_<%= comment.id %>" class="comment message" <article id="comment_<%= comment.id %>" class="comment message"
data-id="<%= comment.id %>" data-id="<%= comment.id %>"
data-post-id="<%= comment.post_id %>" data-post-id="<%= comment.post_id %>"
data-creator-id="<%= comment.creator_id %>" data-creator-id="<%= comment.creator_id unless redact_deleted? %>"
data-updater-id="<%= comment.updater_id %>" data-updater-id="<%= comment.updater_id unless redact_deleted? %>"
data-score="<%= comment.score %>" data-score="<%= comment.score %>"
data-do-not-bump-post="<%= comment.do_not_bump_post? %>" data-do-not-bump-post="<%= comment.do_not_bump_post? %>"
data-is-deleted="<%= comment.is_deleted? %>" data-is-deleted="<%= comment.is_deleted? %>"
@@ -14,9 +14,13 @@
data-is-voted="<%= comment.voted_by?(current_user) %>"> data-is-voted="<%= comment.voted_by?(current_user) %>">
<div class="author"> <div class="author">
<div class="author-name"> <div class="author-name">
<%= link_to_user comment.creator %> <% if redact_deleted? %>
<% if comment.is_deleted? %> [deleted]
(deleted) <% else %>
<%= link_to_user comment.creator %>
<% if comment.is_deleted? %>
(deleted)
<% end %>
<% end %> <% end %>
</div> </div>
<%= link_to time_ago_in_words_tagged(comment.created_at), post_path(comment.post, anchor: "comment_#{comment.id}"), class: "message-timestamp" %> <%= 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 %> <% end %>
<%= tag.div class: "body prose", style: ("display: none;" if thresholded?) do %> <%= tag.div class: "body prose", style: ("display: none;" if thresholded?) do %>
<%= format_text(comment.body, data: dtext_data) %> <% if redact_deleted? %>
<p>[deleted]</p>
<% else %>
<%= format_text(comment.body, data: dtext_data) %>
<% end %>
<%= render "application/update_notice", record: comment %> <%= render "application/update_notice", record: comment %>
<% end %> <% end %>
<% if policy(comment).create? %> <menu>
<menu> <% if policy(comment).reply? %>
<% if context == :index_by_comment %> <% if context == :index_by_comment %>
<li><%= link_to "Reply", new_comment_path(id: comment, comment: { post_id: comment.post_id }), class: "reply-link" %></li> <li><%= link_to "Reply", new_comment_path(id: comment, comment: { post_id: comment.post_id }), class: "reply-link" %></li>
<% else %> <% else %>
<li><%= link_to "Reply", new_comment_path(id: comment, comment: { post_id: comment.post_id }), class: "reply-link", remote: true %></li> <li><%= link_to "Reply", new_comment_path(id: comment, comment: { post_id: comment.post_id }), class: "reply-link", remote: true %></li>
<% end %> <% end %>
<% end %>
<% if policy(comment).update? %> <% if policy(comment).update? %>
<% if comment.is_deleted? %> <% if comment.is_deleted? %>
<li><%= link_to "Undelete", undelete_comment_path(comment.id), method: :post, remote: true %></li> <li><%= link_to "Undelete", undelete_comment_path(comment.id), method: :post, remote: true %></li>
<% else %> <% else %>
<li><%= link_to "Delete", comment_path(comment.id), "data-confirm": "Are you sure you want to delete this comment?", method: :delete, remote: true %></li> <li><%= link_to "Delete", comment_path(comment.id), "data-confirm": "Are you sure you want to delete this comment?", method: :delete, remote: true %></li>
<% end %>
<li><%= link_to "Edit", edit_comment_path(comment.id), id: "edit_comment_link_#{comment.id}", class: "edit_comment_link" %></li>
<% end %> <% end %>
<li><%= link_to "Edit", edit_comment_path(comment.id), id: "edit_comment_link_#{comment.id}", class: "edit_comment_link" %></li>
<% end %>
<% if policy(comment).vote? %>
<li class="comment-vote-up-link"> <li class="comment-vote-up-link">
<%= link_to "Vote up", comment_comment_votes_path(comment_id: comment.id, score: "up"), method: :post, remote: true %> <%= link_to "Vote up", comment_comment_votes_path(comment_id: comment.id, score: "up"), method: :post, remote: true %>
</li> </li>
@@ -56,16 +68,19 @@
<li class="comment-unvote-link"> <li class="comment-unvote-link">
<%= link_to "Unvote", comment_comment_votes_path(comment_id: comment.id), method: :delete, remote: true %> <%= link_to "Unvote", comment_comment_votes_path(comment_id: comment.id), method: :delete, remote: true %>
</li> </li>
<% if policy(comment).reportable? %>
<li><%= link_to "Report", new_moderation_report_path(moderation_report: { model_type: "Comment", model_id: comment.id }), remote: true %></li>
<% end %>
<% if has_moderation_reports? %>
<li class="moderation-report-notice">This comment has been reported! (<%= link_to pluralize(comment.moderation_reports.length, "report"), moderation_reports_path(search: { model_type: "Comment", model_id: comment.id }) %>)</li>
<% end %>
</menu>
<% if policy(comment).update? %>
<%= render "comments/form", comment: comment, hidden: true %>
<% end %> <% end %>
<% if policy(comment).reportable? %>
<li><%= link_to "Report", new_moderation_report_path(moderation_report: { model_type: "Comment", model_id: comment.id }), remote: true %></li>
<% end %>
<% if has_moderation_reports? %>
<li class="moderation-report-notice">This comment has been reported! (<%= link_to pluralize(comment.moderation_reports.length, "report"), moderation_reports_path(search: { model_type: "Comment", model_id: comment.id }) %>)</li>
<% end %>
</menu>
<% if policy(comment).update? %>
<%= render "comments/form", comment: comment, hidden: true %>
<% end %> <% end %>
</div> </div>
</article> </article>

View File

@@ -1,16 +1,33 @@
class CommentPolicy < ApplicationPolicy class CommentPolicy < ApplicationPolicy
def create?
unbanned?
end
def update? def update?
unbanned? && (user.is_moderator? || record.updater_id == user.id) unbanned? && (user.is_moderator? || (record.updater_id == user.id && !record.is_deleted?))
end end
def reportable? 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 end
def can_sticky_comment? def can_sticky_comment?
user.is_moderator? user.is_moderator?
end 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 def permitted_attributes_for_create
[:body, :post_id, :do_not_bump_post, (:is_sticky if can_sticky_comment?)].compact [:body, :post_id, :do_not_bump_post, (:is_sticky if can_sticky_comment?)].compact
end end
@@ -19,5 +36,11 @@ class CommentPolicy < ApplicationPolicy
[:body, :is_deleted, (:is_sticky if can_sticky_comment?)].compact [:body, :is_deleted, (:is_sticky if can_sticky_comment?)].compact
end 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? alias_method :undelete?, :update?
end end

View File

@@ -1,16 +1,16 @@
<div id="p-index-by-comment" class="comments-for-post"> <div id="p-index-by-comment" class="comments-for-post">
<div class="list-of-comments list-of-messages"> <div class="list-of-comments list-of-messages">
<% dtext_data = DText.preprocess(@comments.map(&:body)) %> <% dtext_data = DText.preprocess(@comments.map(&:body)) %>
<% @comments.each do |comment| %> <% @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 %>
<%= tag.div id: "post_#{comment.post.id}", **PostPreviewComponent.new(post: comment.post).article_attrs("post") do %> <div class="preview">
<div class="preview"> <% if policy(comment.post).visible? %>
<% if policy(comment.post).visible? %> <%= link_to(image_tag(comment.post.preview_file_url), post_path(comment.post)) %>
<%= link_to(image_tag(comment.post.preview_file_url), post_path(comment.post)) %> <% end %>
<% end %> </div>
</div>
<%= 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) %> <%= render_comment(comment, dtext_data: dtext_data, context: :index_by_comment, current_user: CurrentUser.user) %>
<% end %>
<% end %> <% end %>
<% end %> <% end %>
</div> </div>

View File

@@ -26,6 +26,29 @@ class CommentComponentTest < ViewComponent::TestCase
end end
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 context "for a comment with moderation reports" do
should "show the report notice to moderators" do should "show the report notice to moderators" do
create(:moderation_report, model: @comment) create(:moderation_report, model: @comment)

View File

@@ -113,6 +113,19 @@ class CommentsControllerTest < ActionDispatch::IntegrationTest
get comment_path(@comment.id) get comment_path(@comment.id)
assert_redirected_to post_path(@comment.post, anchor: "comment_#{@comment.id}") assert_redirected_to post_path(@comment.post, anchor: "comment_#{@comment.id}")
end 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 end
context "edit action" do context "edit action" do
@@ -157,6 +170,16 @@ class CommentsControllerTest < ActionDispatch::IntegrationTest
end end
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 should "update the body" do
put_auth comment_path(@comment.id), @user, params: {comment: {body: "abc"}} put_auth comment_path(@comment.id), @user, params: {comment: {body: "abc"}}
assert_equal("abc", @comment.reload.body) assert_equal("abc", @comment.reload.body)
@@ -224,20 +247,16 @@ class CommentsControllerTest < ActionDispatch::IntegrationTest
end end
context "undelete action" do 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) @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_redirected_to(@comment)
assert_equal(false, @comment.reload.is_deleted)
end end
should "not allow undeleting comments deleted by a moderator" do should "not allow normal Members to undelete their own comments" do
@comment = create(:comment, post: @post) @comment = create(:comment, post: @post, is_deleted: true)
delete_auth comment_path(@comment.id), @mod
assert_redirected_to @comment
assert(@comment.reload.is_deleted?)
post_auth undelete_comment_path(@comment.id), @user post_auth undelete_comment_path(@comment.id), @user
assert_response 403 assert_response 403