FEATURE: Allow admins to permanently delete posts and topics (#14406)

Sometimes administrators want to permanently delete posts and topics
from the database. To make sure that this is done for a good reasons,
administrators can do this only after one minute has passed since the
post was deleted or immediately if another administrator does it.
This commit is contained in:
Bianca Nenciu 2021-10-13 12:53:23 +03:00 committed by GitHub
parent 76c9de2d04
commit c4843fc1c1
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
27 changed files with 290 additions and 99 deletions

View File

@ -595,9 +595,9 @@ export default Controller.extend(bufferedProperty("model"), {
post.get("post_number") === 1 ? this.recoverTopic() : post.recover();
},
deletePost(post) {
deletePost(post, opts) {
if (post.get("post_number") === 1) {
return this.deleteTopic();
return this.deleteTopic(opts);
} else if (!post.can_delete) {
return false;
}
@ -611,7 +611,7 @@ export default Controller.extend(bufferedProperty("model"), {
ajax(`/posts/${post.id}/reply-ids.json`).then((replies) => {
if (replies.length === 0) {
return post
.destroy(user)
.destroy(user, opts)
.then(refresh)
.catch((error) => {
popupAjaxError(error);
@ -630,7 +630,7 @@ export default Controller.extend(bufferedProperty("model"), {
label: I18n.t("post.controls.delete_replies.just_the_post"),
callback() {
post
.destroy(user)
.destroy(user, opts)
.then(refresh)
.catch((error) => {
popupAjaxError(error);
@ -685,7 +685,7 @@ export default Controller.extend(bufferedProperty("model"), {
});
} else {
return post
.destroy(user)
.destroy(user, opts)
.then(refresh)
.catch((error) => {
popupAjaxError(error);
@ -694,6 +694,19 @@ export default Controller.extend(bufferedProperty("model"), {
}
},
permanentlyDeletePost(post) {
return bootbox.confirm(
I18n.t("post.controls.permanently_delete_confirmation"),
I18n.t("no_value"),
I18n.t("yes_value"),
(result) => {
if (result) {
this.send("deletePost", post, { force_destroy: true });
}
}
);
},
editPost(post) {
if (!this.currentUser) {
return bootbox.alert(I18n.t("post.controls.edit_anonymous"));
@ -1497,13 +1510,13 @@ export default Controller.extend(bufferedProperty("model"), {
this.model.recover();
},
deleteTopic() {
deleteTopic(opts) {
if (
this.model.views > this.siteSettings.min_topic_views_for_delete_confirm
) {
this.deleteTopicModal();
} else {
this.model.destroy(this.currentUser);
this.model.destroy(this.currentUser, opts);
}
},

View File

@ -52,6 +52,7 @@ export function transformBasicPost(post) {
created_at: post.created_at,
updated_at: post.updated_at,
canDelete: post.can_delete,
canPermanentlyDelete: post.can_permanently_delete,
showFlagDelete: false,
canRecover: post.can_recover,
canEdit: post.can_edit,
@ -261,6 +262,7 @@ export default function transformPost(
postAtts.canRecoverTopic = postAtts.isDeleted && details.can_recover;
postAtts.canDeleteTopic = !postAtts.isDeleted && details.can_delete;
postAtts.expandablePost = topic.expandable_first_post;
postAtts.canPermanentlyDeleteTopic = details.can_permanently_delete;
// Show a "Flag to delete" message if not staff and you can't
// otherwise delete it.

View File

@ -249,10 +249,10 @@ const Post = RestModel.extend({
}
},
destroy(deletedBy) {
destroy(deletedBy, opts) {
return this.setDeletedState(deletedBy).then(() => {
return ajax("/posts/" + this.id, {
data: { context: window.location.pathname },
data: { context: window.location.pathname, ...opts },
type: "DELETE",
});
});

View File

@ -433,9 +433,9 @@ const Topic = RestModel.extend({
},
// Delete this topic
destroy(deleted_by) {
destroy(deleted_by, opts) {
return ajax(`/t/${this.id}`, {
data: { context: window.location.pathname },
data: { context: window.location.pathname, ...opts },
type: "DELETE",
})
.then(() => {

View File

@ -217,6 +217,7 @@
showLogin=(route-action "showLogin")
showRawEmail=(route-action "showRawEmail")
deletePost=(action "deletePost")
permanentlyDeletePost=(action "permanentlyDeletePost")
recoverPost=(action "recoverPost")
expandHidden=(action "expandHidden")
toggleBookmark=(action "toggleBookmark")

View File

@ -37,6 +37,15 @@ export function buildManageButtons(attrs, currentUser, siteSettings) {
});
}
if (attrs.canPermanentlyDelete || attrs.canPermanentlyDeleteTopic) {
contents.push({
icon: "trash-alt",
className: "popup-menu-button permanently-delete",
label: "post.controls.permanently_delete",
action: "permanentlyDeletePost",
});
}
if (!attrs.isWhisper && currentUser.staff) {
const buttonAtts = {
action: "togglePostType",

View File

@ -649,6 +649,36 @@ discourseModule("Integration | Component | Widget | post", function (hooks) {
},
});
componentTest("permanently delete topic", {
template: hbs`{{mount-widget widget="post" args=args permanentlyDeletePost=permanentlyDeletePost}}`,
beforeEach() {
this.set("args", { canManage: true, canPermanentlyDeleteTopic: true });
this.set("permanentlyDeletePost", () => (this.deleted = true));
},
async test(assert) {
await click(".post-menu-area .show-post-admin-menu");
await click(".post-admin-menu .permanently-delete");
assert.ok(this.deleted);
assert.ok(!exists(".post-admin-menu"), "also hides the menu");
},
});
componentTest("permanently delete post", {
template: hbs`
{{mount-widget widget="post" args=args permanentlyDeletePost=permanentlyDeletePost}}
`,
beforeEach() {
this.set("args", { canManage: true, canPermanentlyDelete: true });
this.set("permanentlyDeletePost", () => (this.deleted = true));
},
async test(assert) {
await click(".post-menu-area .show-post-admin-menu");
await click(".post-admin-menu .permanently-delete");
assert.ok(this.deleted);
assert.ok(!exists(".post-admin-menu"), "also hides the menu");
},
});
componentTest("toggle moderator post", {
template: hbs`
{{mount-widget widget="post" args=args togglePostType=togglePostType}}

View File

@ -303,14 +303,24 @@ class PostsController < ApplicationController
def destroy
post = find_post_from_params
force_destroy = false
if params[:force_destroy].present?
if !guardian.can_permanently_delete?(post)
return render_json_error post.cannot_permanently_delete_reason(current_user), status: 403
end
force_destroy = true
else
guardian.ensure_can_delete!(post)
end
unless guardian.can_moderate_topic?(post.topic)
RateLimiter.new(current_user, "delete_post_per_min", SiteSetting.max_post_deletions_per_minute, 1.minute).performed!
RateLimiter.new(current_user, "delete_post_per_day", SiteSetting.max_post_deletions_per_day, 1.day).performed!
end
destroyer = PostDestroyer.new(current_user, post, context: params[:context])
destroyer = PostDestroyer.new(current_user, post, context: params[:context], force_destroy: force_destroy)
destroyer.destroy
render body: nil

View File

@ -606,11 +606,21 @@ class TopicsController < ApplicationController
end
def destroy
topic = Topic.find_by(id: params[:id])
guardian.ensure_can_delete!(topic)
topic = Topic.with_deleted.find_by(id: params[:id])
first_post = topic.ordered_posts.first
PostDestroyer.new(current_user, first_post, context: params[:context]).destroy
force_destroy = false
if params[:force_destroy].present?
if !guardian.can_permanently_delete?(topic)
return render_json_error topic.cannot_permanently_delete_reason(current_user), status: 403
end
force_destroy = true
else
guardian.ensure_can_delete!(topic)
end
first_post = topic.posts.with_deleted.order(:post_number).first
PostDestroyer.new(current_user, first_post, context: params[:context], force_destroy: force_destroy).destroy
render body: nil
rescue Discourse::InvalidAccess

View File

@ -920,6 +920,21 @@ class Category < ActiveRecord::Base
end
end
def cannot_delete_reason
return I18n.t('category.cannot_delete.uncategorized') if self.uncategorized?
return I18n.t('category.cannot_delete.has_subcategories') if self.has_children?
if self.topic_count != 0
oldest_topic = self.topics.where.not(id: self.topic_id).order('created_at ASC').limit(1).first
if oldest_topic
I18n.t('category.cannot_delete.topic_exists', count: self.topic_count, topic_link: "<a href=\"#{oldest_topic.url}\">#{CGI.escapeHTML(oldest_topic.title)}</a>")
else
# This is a weird case, probably indicating a bug.
I18n.t('category.cannot_delete.topic_exists_no_oldest', count: self.topic_count)
end
end
end
private
def should_update_reviewables?

View File

@ -25,6 +25,9 @@ class Post < ActiveRecord::Base
# Version 2 15-12-2017, introduces CommonMark and a huge number of onebox fixes
BAKED_VERSION = 2
# Time between the delete and permanent delete of a post
PERMANENT_DELETE_TIMER = 5.minutes
rate_limit
rate_limit :limit_posts_per_day
@ -1090,6 +1093,13 @@ class Post < ActiveRecord::Base
UrlHelper.cook_url(raw_url, secure: image_upload&.secure?, local: true) if raw_url
end
def cannot_permanently_delete_reason(user)
if self.deleted_by_id == user&.id && self.deleted_at >= Post::PERMANENT_DELETE_TIMER.ago
time_left = RateLimiter.time_left(Post::PERMANENT_DELETE_TIMER.to_i - Time.zone.now.to_i + self.deleted_at.to_i)
I18n.t('post.cannot_permanently_delete.wait_or_different_admin', time_left: time_left)
end
end
private
def parse_quote_into_arguments(quote)

View File

@ -1787,6 +1787,15 @@ class Topic < ActiveRecord::Base
def apply_per_day_rate_limit_for(key, method_name)
RateLimiter.new(user, "#{key}-per-day", SiteSetting.get(method_name), 1.day.to_i)
end
def cannot_permanently_delete_reason(user)
if self.posts_count > 1
I18n.t('post.cannot_permanently_delete.many_posts')
elsif self.deleted_by_id == user&.id && self.deleted_at >= Post::PERMANENT_DELETE_TIMER.ago
time_left = RateLimiter.time_left(Post::PERMANENT_DELETE_TIMER.to_i - Time.zone.now.to_i + self.deleted_at.to_i)
I18n.t('post.cannot_permanently_delete.wait_or_different_admin', time_left: time_left)
end
end
end
# == Schema Information

View File

@ -71,10 +71,6 @@ class CategorySerializer < SiteCategorySerializer
scope && scope.can_delete?(object)
end
def cannot_delete_reason
scope && scope.cannot_delete_category_reason(object)
end
def include_cannot_delete_reason?
!include_can_delete? && scope && scope.can_edit?(object)
end

View File

@ -43,6 +43,7 @@ class PostSerializer < BasicPostSerializer
:version,
:can_edit,
:can_delete,
:can_permanently_delete,
:can_recover,
:can_wiki,
:link_counts,
@ -166,6 +167,14 @@ class PostSerializer < BasicPostSerializer
scope.can_delete?(object)
end
def can_permanently_delete
true
end
def include_can_permanently_delete?
SiteSetting.can_permanently_delete && object.deleted_at
end
def can_recover
scope.can_recover_post?(object)
end

View File

@ -5,6 +5,7 @@ class TopicViewDetailsSerializer < ApplicationSerializer
def self.can_attributes
[:can_move_posts,
:can_delete,
:can_permanently_delete,
:can_recover,
:can_remove_allowed_users,
:can_invite_to,
@ -110,6 +111,10 @@ class TopicViewDetailsSerializer < ApplicationSerializer
scope.can_delete?(object.topic)
end
def include_can_permanently_delete?
SiteSetting.can_permanently_delete && object.topic.deleted_at
end
def include_can_recover?
scope.can_recover_topic?(object.topic)
end

View File

@ -3125,6 +3125,8 @@ en:
other: "Yes, and all %{count} replies"
just_the_post: "No, just this post"
admin: "post admin actions"
permanently_delete: "Permanently Delete"
permanently_delete_confirmation: "Are you sure you permanently want to delete this post? You will not be able to recover it."
wiki: "Make Wiki"
unwiki: "Remove Wiki"
convert_to_moderator: "Add Staff Color"

View File

@ -721,6 +721,9 @@ en:
has_likes:
one: "%{count} Like"
other: "%{count} Likes"
cannot_permanently_delete:
many_posts: "You can not permanently delete this topic because there are other posts."
wait_or_different_admin: "You must wait %{time_left} before permanently deleting this post or a different administrator must do it."
rate_limiter:
slow_down: "Youve performed this action too many times, please try again later."

View File

@ -1677,6 +1677,9 @@ security:
disable_onebox_media_download_controls:
default: false
hidden: true
can_permanently_delete:
default: false
hidden: true
onebox:
post_onebox_maxlength:

View File

@ -172,6 +172,10 @@ class Guardian
can_do?(:delete, obj)
end
def can_permanently_delete?(obj)
can_do?(:permanently_delete, obj)
end
def can_moderate?(obj)
obj && authenticated? && !is_silenced? && (
is_staff? ||

View File

@ -38,23 +38,6 @@ module CategoryGuardian
!category.has_children?
end
def cannot_delete_category_reason(category)
return I18n.t('category.cannot_delete.uncategorized') if category.uncategorized?
return I18n.t('category.cannot_delete.has_subcategories') if category.has_children?
if category.topic_count != 0
oldest_topic = category.topics.where.not(id: category.topic_id).order('created_at ASC').limit(1).first
if oldest_topic
return I18n.t('category.cannot_delete.topic_exists', count: category.topic_count, topic_link: "<a href=\"#{oldest_topic.url}\">#{CGI.escapeHTML(oldest_topic.title)}</a>")
else
# This is a weird case, probably indicating a bug.
return I18n.t('category.cannot_delete.topic_exists_no_oldest', count: category.topic_count)
end
end
nil
end
def can_see_serialized_category?(category_id:, read_restricted: true)
# Guard to ensure only a boolean is passed in
read_restricted = true unless !!read_restricted == read_restricted

View File

@ -205,6 +205,16 @@ module PostGuardian
false
end
def can_permanently_delete_post?(post)
return false if !SiteSetting.can_permanently_delete
return false if !post
return false if post.is_first_post?
return false if !is_admin? || !can_edit_post?(post)
return false if !post.deleted_at
return false if post.deleted_by_id == @user.id && post.deleted_at >= Post::PERMANENT_DELETE_TIMER.ago
true
end
def can_recover_post?(post)
return false unless post

View File

@ -153,6 +153,16 @@ module TopicGuardian
!Discourse.static_doc_topic_ids.include?(topic.id)
end
def can_permanently_delete_topic?(topic)
return false if !SiteSetting.can_permanently_delete
return false if !topic
return false if topic.posts_count > 1
return false if !is_admin? || !can_see_topic?(topic)
return false if !topic.deleted_at
return false if topic.deleted_by_id == @user.id && topic.deleted_at >= Post::PERMANENT_DELETE_TIMER.ago
true
end
def can_toggle_topic_visibility?(topic)
can_moderate?(topic) || can_perform_action_available_to_group_moderators?(topic)
end

View File

@ -55,7 +55,7 @@ class PostDestroyer
def destroy
payload = WebHook.generate_payload(:post, @post) if WebHook.active_web_hooks(:post).exists?
topic = @post.topic
topic = Topic.with_deleted.find_by(id: @post.topic_id)
is_first_post = @post.is_first_post? && topic
has_topic_web_hooks = is_first_post && WebHook.active_web_hooks(:topic).exists?
@ -79,7 +79,7 @@ class PostDestroyer
Jobs.enqueue(:sync_topic_user_bookmarked, topic_id: topic.id) if topic
if is_first_post
UserProfile.remove_featured_topic_from_all_profiles(@topic)
UserProfile.remove_featured_topic_from_all_profiles(topic)
UserActionManager.topic_destroyed(topic)
DiscourseEvent.trigger(:topic_destroyed, topic, @user)
WebHook.enqueue_topic_hooks(:topic_destroyed, topic, topic_payload) if has_topic_web_hooks

View File

@ -2,6 +2,18 @@
class RateLimiter
def self.time_left(available_in)
if available_in <= 3
I18n.t("rate_limiter.short_time")
elsif available_in < 1.minute.to_i
I18n.t("rate_limiter.seconds", count: available_in)
elsif available_in < 1.hour.to_i
I18n.t("rate_limiter.minutes", count: (available_in / 1.minute.to_i))
else
I18n.t("rate_limiter.hours", count: (available_in / 1.hour.to_i))
end
end
# A rate limit has been exceeded.
class LimitExceeded < StandardError
attr_reader :type, :available_in
@ -12,16 +24,7 @@ class RateLimiter
end
def time_left
@time_left ||=
if @available_in <= 3
I18n.t("rate_limiter.short_time")
elsif @available_in < 1.minute.to_i
I18n.t("rate_limiter.seconds", count: @available_in)
elsif @available_in < 1.hour.to_i
I18n.t("rate_limiter.minutes", count: (@available_in / 1.minute.to_i))
else
I18n.t("rate_limiter.hours", count: (@available_in / 1.hour.to_i))
end
@time_left ||= RateLimiter.time_left(@available_in)
end
def description

View File

@ -1,49 +0,0 @@
# frozen_string_literal: true
require 'rails_helper'
RSpec.describe CategoryGuardian do
fab!(:admin) { Fabricate(:admin) }
let(:guardian) { Guardian.new(admin) }
fab!(:category) { Fabricate(:category) }
describe '#cannot_delete_category_reason' do
describe 'when category is uncategorized' do
it 'should return the reason' do
category = Category.find(SiteSetting.uncategorized_category_id)
expect(guardian.cannot_delete_category_reason(category)).to eq(
I18n.t('category.cannot_delete.uncategorized')
)
end
end
describe 'when category has subcategories' do
it 'should return the right reason' do
category.subcategories << Fabricate(:category)
expect(guardian.cannot_delete_category_reason(category)).to eq(
I18n.t('category.cannot_delete.has_subcategories')
)
end
end
describe 'when category has topics' do
it 'should return the right reason' do
topic = Fabricate(:topic,
title: '</a><script>alert(document.cookie);</script><a>',
category: category
)
category.reload
expect(guardian.cannot_delete_category_reason(category)).to eq(
I18n.t('category.cannot_delete.topic_exists',
count: 1,
topic_link: "<a href=\"#{topic.url}\">&lt;/a&gt;&lt;script&gt;alert(document.cookie);&lt;/script&gt;&lt;a&gt;</a>"
)
)
end
end
end
end

View File

@ -1220,4 +1220,48 @@ describe Category do
expect(Category.find_by_slug_path(["#{category.id}-category", "#{subcategory.id}-category"])).to eq(subcategory)
end
end
describe '#cannot_delete_reason' do
fab!(:admin) { Fabricate(:admin) }
let(:guardian) { Guardian.new(admin) }
fab!(:category) { Fabricate(:category) }
describe 'when category is uncategorized' do
it 'should return the reason' do
category = Category.find(SiteSetting.uncategorized_category_id)
expect(category.cannot_delete_reason).to eq(
I18n.t('category.cannot_delete.uncategorized')
)
end
end
describe 'when category has subcategories' do
it 'should return the right reason' do
category.subcategories << Fabricate(:category)
expect(category.cannot_delete_reason).to eq(
I18n.t('category.cannot_delete.has_subcategories')
)
end
end
describe 'when category has topics' do
it 'should return the right reason' do
topic = Fabricate(:topic,
title: '</a><script>alert(document.cookie);</script><a>',
category: category
)
category.reload
expect(category.cannot_delete_reason).to eq(
I18n.t('category.cannot_delete.topic_exists',
count: 1,
topic_link: "<a href=\"#{topic.url}\">&lt;/a&gt;&lt;script&gt;alert(document.cookie);&lt;/script&gt;&lt;a&gt;</a>"
)
)
end
end
end
end

View File

@ -224,6 +224,65 @@ describe PostsController do
delete "/posts/#{post.id}.json"
end
context "permanently destroy" do
let!(:post) { Fabricate(:post, topic_id: topic.id, post_number: 3) }
before do
SiteSetting.can_permanently_delete = true
end
it "does not work for a post that was not deleted yet" do
sign_in(admin)
delete "/posts/#{post.id}.json", params: { force_destroy: true }
expect(response.status).to eq(403)
end
it "needs some time to pass to permanently delete a topic" do
sign_in(admin)
delete "/posts/#{post.id}.json"
expect(response.status).to eq(200)
expect(post.reload.deleted_by_id).to eq(admin.id)
delete "/posts/#{post.id}.json", params: { force_destroy: true }
expect(response.status).to eq(403)
post.update!(deleted_at: 10.minutes.ago)
delete "/posts/#{post.id}.json", params: { force_destroy: true }
expect(response.status).to eq(200)
expect { post.reload }.to raise_error(ActiveRecord::RecordNotFound)
end
it "needs two users to permanently delete a topic" do
sign_in(admin)
delete "/posts/#{post.id}.json"
expect(response.status).to eq(200)
expect(post.reload.deleted_by_id).to eq(admin.id)
sign_in(Fabricate(:admin))
delete "/posts/#{post.id}.json", params: { force_destroy: true }
expect(response.status).to eq(200)
expect { post.reload }.to raise_error(ActiveRecord::RecordNotFound)
end
it "moderators cannot permanently delete topics" do
sign_in(admin)
delete "/posts/#{post.id}.json"
expect(response.status).to eq(200)
expect(post.reload.deleted_by_id).to eq(admin.id)
sign_in(moderator)
delete "/posts/#{post.id}.json", params: { force_destroy: true }
expect(response.status).to eq(403)
end
end
end
end