diff --git a/app/services/user_destroyer.rb b/app/services/user_destroyer.rb index fee04664395..cc70cc3d911 100644 --- a/app/services/user_destroyer.rb +++ b/app/services/user_destroyer.rb @@ -35,32 +35,9 @@ class UserDestroyer category_topic_ids = Category.where("topic_id IS NOT NULL").pluck(:topic_id) if opts[:delete_posts] - user.posts.each do |post| - - # agree with flags - if opts[:delete_as_spammer] && reviewable = post.reviewable_flag - reviewable.perform(@actor, :agree_and_keep) - end - - # block all external urls - if opts[:block_urls] - post.topic_links.each do |link| - next if link.internal - next if Oneboxer.engine(link.url) != Onebox::Engine::AllowlistedGenericOnebox - ScreenedUrl.watch(link.url, link.domain, ip_address: user.ip_address)&.record_match! - end - end - - if post.is_first_post? && category_topic_ids.include?(post.topic_id) - post.update!(user: Discourse.system_user) - else - PostDestroyer.new(@actor.staff? ? @actor : Discourse.system_user, post).destroy - end - - if post.topic && post.is_first_post? - Topic.unscoped.where(id: post.topic_id).update_all(user_id: nil) - end - end + agree_with_flags(user) if opts[:delete_as_spammer] + block_external_urls(user) if opts[:block_urls] + delete_posts(user, category_topic_ids, opts) end user.post_actions.each do |post_action| @@ -131,6 +108,33 @@ class UserDestroyer protected + def block_external_urls(user) + TopicLink.where(user: user, internal: false).find_each do |link| + next if Oneboxer.engine(link.url) != Onebox::Engine::AllowlistedGenericOnebox + ScreenedUrl.watch(link.url, link.domain, ip_address: user.ip_address)&.record_match! + end + end + + def agree_with_flags(user) + ReviewableFlaggedPost.where(target_created_by: user).find_each do |reviewable| + reviewable.perform(@actor, :agree_and_keep) + end + end + + def delete_posts(user, category_topic_ids, opts) + user.posts.find_each do |post| + if post.is_first_post? && category_topic_ids.include?(post.topic_id) + post.update!(user: Discourse.system_user) + else + PostDestroyer.new(@actor.staff? ? @actor : Discourse.system_user, post).destroy + end + + if post.topic && post.is_first_post? + Topic.unscoped.where(id: post.topic_id).update_all(user_id: nil) + end + end + end + def prepare_for_destroy(user) PostAction.where(user_id: user.id).delete_all UserAction.where('user_id = :user_id OR target_user_id = :user_id OR acting_user_id = :user_id', user_id: user.id).delete_all diff --git a/lib/post_destroyer.rb b/lib/post_destroyer.rb index c85fcbca2f5..5f8d793add8 100644 --- a/lib/post_destroyer.rb +++ b/lib/post_destroyer.rb @@ -186,7 +186,7 @@ class PostDestroyer end def permanent? - @opts[:permanent] && @user == @post.user && @post.topic.private_message? + @opts[:force_destroy] || (@opts[:permanent] && @user == @post.user && @post.topic.private_message?) end # When a user 'deletes' their own post. We just change the text. @@ -247,9 +247,10 @@ class PostDestroyer end def make_previous_post_the_last_one - last_post = Post + last_post = Post.where("topic_id = ? and id <> ?", @post.topic_id, @post.id) .select(:created_at, :user_id, :post_number) .where("topic_id = ? and id <> ?", @post.topic_id, @post.id) + .where.not(user_id: nil) .order('created_at desc') .limit(1) .first diff --git a/spec/components/post_destroyer_spec.rb b/spec/components/post_destroyer_spec.rb index 6ec8622ed3f..88f14b07470 100644 --- a/spec/components/post_destroyer_spec.rb +++ b/spec/components/post_destroyer_spec.rb @@ -967,6 +967,7 @@ describe PostDestroyer do fab!(:private_post) { Fabricate(:private_message_post, topic: private_message_topic) } fab!(:post_action) { Fabricate(:post_action, post: private_post) } fab!(:reply) { Fabricate(:private_message_post, topic: private_message_topic) } + it "destroys the post and topic if deleting first post" do PostDestroyer.new(reply.user, reply, permanent: true).destroy expect { reply.reload }.to raise_error(ActiveRecord::RecordNotFound) @@ -985,5 +986,14 @@ describe PostDestroyer do PostDestroyer.new(post.user, post, permanent: true).destroy expect(post.user_deleted).to be true end + + it 'always destroy the post when the force_destroy option is passed' do + PostDestroyer.new(moderator, reply, force_destroy: true).destroy + expect { reply.reload }.to raise_error(ActiveRecord::RecordNotFound) + + regular_post = Fabricate(:post) + PostDestroyer.new(moderator, regular_post, force_destroy: true).destroy + expect { regular_post.reload }.to raise_error(ActiveRecord::RecordNotFound) + end end end diff --git a/spec/services/user_destroyer_spec.rb b/spec/services/user_destroyer_spec.rb index c54ff7a0948..ea23dbf0b96 100644 --- a/spec/services/user_destroyer_spec.rb +++ b/spec/services/user_destroyer_spec.rb @@ -290,7 +290,7 @@ describe UserDestroyer do before do @post = Fabricate(:post_with_external_links, user: user) TopicLink.extract_from(@post) - TopicLink.any_instance.stubs(:internal).returns(true) + TopicLink.where(user: user).update_all(internal: true) end it "doesn't add ScreenedUrl records" do