diff --git a/app/controllers/posts_controller.rb b/app/controllers/posts_controller.rb index 2a905751ab7..520aab3bccd 100644 --- a/app/controllers/posts_controller.rb +++ b/app/controllers/posts_controller.rb @@ -321,14 +321,12 @@ class PostsController < ApplicationController def destroy post = find_post_from_params + force_destroy = ActiveModel::Type::Boolean.new.cast(params[:force_destroy]) - force_destroy = false - if params[:force_destroy].present? + if force_destroy 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 @@ -338,8 +336,12 @@ class PostsController < ApplicationController 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], force_destroy: force_destroy) - destroyer.destroy + PostDestroyer.new( + current_user, + post, + context: params[:context], + force_destroy: force_destroy + ).destroy render body: nil end diff --git a/app/controllers/topics_controller.rb b/app/controllers/topics_controller.rb index 909f540af1f..be5f02f41ac 100644 --- a/app/controllers/topics_controller.rb +++ b/app/controllers/topics_controller.rb @@ -621,20 +621,22 @@ class TopicsController < ApplicationController def destroy topic = Topic.with_deleted.find_by(id: params[:id]) + force_destroy = ActiveModel::Type::Boolean.new.cast(params[:force_destroy]) - force_destroy = false - if params[:force_destroy].present? + if force_destroy 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 + PostDestroyer.new( + current_user, + topic.ordered_posts.with_deleted.first, + context: params[:context], + force_destroy: force_destroy + ).destroy render body: nil rescue Discourse::InvalidAccess diff --git a/app/models/topic.rb b/app/models/topic.rb index 9b2dff1e98a..54af4242de7 100644 --- a/app/models/topic.rb +++ b/app/models/topic.rb @@ -1786,7 +1786,12 @@ class Topic < ActiveRecord::Base end def cannot_permanently_delete_reason(user) - if self.posts_count > 0 + all_posts_count = Post.with_deleted + .where(topic_id: self.id) + .where(post_type: [Post.types[:regular], Post.types[:moderator_action], Post.types[:whisper]]) + .count + + if posts_count > 0 || all_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) diff --git a/lib/guardian/topic_guardian.rb b/lib/guardian/topic_guardian.rb index f5cff4eefc8..06141292247 100644 --- a/lib/guardian/topic_guardian.rb +++ b/lib/guardian/topic_guardian.rb @@ -158,7 +158,20 @@ module TopicGuardian def can_permanently_delete_topic?(topic) return false if !SiteSetting.can_permanently_delete return false if !topic + + # Ensure that all posts (including small actions) are at least soft + # deleted. return false if topic.posts_count > 0 + + # All other posts that were deleted still must be permanently deleted + # before the topic can be deleted with the exception of small action + # posts that will be deleted right before the topic is. + all_posts_count = Post.with_deleted + .where(topic_id: topic.id) + .where(post_type: [Post.types[:regular], Post.types[:moderator_action], Post.types[:whisper]]) + .count + return false if all_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 diff --git a/lib/post_destroyer.rb b/lib/post_destroyer.rb index 57d9bb3d011..cd0f037b168 100644 --- a/lib/post_destroyer.rb +++ b/lib/post_destroyer.rb @@ -145,6 +145,14 @@ class PostDestroyer # show up in the topic # Permanent option allows to hard delete. def perform_delete + # All posts in the topic must be force deleted if the first is force + # deleted (except @post which is destroyed by current instance). + if @topic && @post.is_first_post? && permanent? + @topic.ordered_posts.with_deleted.reverse_order.find_each do |post| + PostDestroyer.new(@user, post, @opts).destroy if post.id != @post.id + end + end + Post.transaction do permanent? ? @post.destroy! : @post.trash!(@user) if @post.topic diff --git a/spec/models/topic_spec.rb b/spec/models/topic_spec.rb index b21761e952e..54a6bd0e0ed 100644 --- a/spec/models/topic_spec.rb +++ b/spec/models/topic_spec.rb @@ -3030,6 +3030,9 @@ RSpec.describe Topic do expect(topic.reload.cannot_permanently_delete_reason(Fabricate(:admin))).to eq(I18n.t('post.cannot_permanently_delete.many_posts')) PostDestroyer.new(admin, post_2.reload).destroy + expect(topic.reload.cannot_permanently_delete_reason(Fabricate(:admin))).to eq(I18n.t('post.cannot_permanently_delete.many_posts')) + + PostDestroyer.new(admin, post_2.reload, force_destroy: true).destroy expect(topic.reload.cannot_permanently_delete_reason(Fabricate(:admin))).to eq(nil) end diff --git a/spec/requests/topics_controller_spec.rb b/spec/requests/topics_controller_spec.rb index b67f235d8b6..0ed754a064a 100644 --- a/spec/requests/topics_controller_spec.rb +++ b/spec/requests/topics_controller_spec.rb @@ -1188,6 +1188,48 @@ RSpec.describe TopicsController do end end end + + describe 'force destroy' do + fab!(:post) { Fabricate(:post, topic: topic, post_number: 1) } + + before do + SiteSetting.can_permanently_delete = true + + sign_in(admin) + end + + it 'force destroys all deleted small actions in topic too' do + small_action_post = Fabricate(:small_action, topic: topic) + PostDestroyer.new(Discourse.system_user, post).destroy + PostDestroyer.new(Discourse.system_user, small_action_post).destroy + + delete "/t/#{topic.id}.json", params: { force_destroy: true } + + expect(response.status).to eq(200) + + expect(Topic.find_by(id: topic.id)).to eq(nil) + expect(Post.find_by(id: post.id)).to eq(nil) + expect(Post.find_by(id: small_action_post.id)).to eq(nil) + end + + it 'does not allow to destroy topic if not all posts were force destroyed' do + other_post = Fabricate(:post, topic: topic, post_number: 2) + PostDestroyer.new(Discourse.system_user, post).destroy + + delete "/t/#{topic.id}.json", params: { force_destroy: true } + + expect(response.status).to eq(403) + end + + it 'does not allow to destroy topic if not all small action posts were deleted' do + small_action_post = Fabricate(:small_action, topic: topic) + PostDestroyer.new(Discourse.system_user, small_action_post).destroy + + delete "/t/#{topic.id}.json", params: { force_destroy: true } + + expect(response.status).to eq(403) + end + end end describe '#id_for_slug' do