diff --git a/db/post_migrate/20210324043327_delete_orphan_post_revisions.rb b/db/post_migrate/20210324043327_delete_orphan_post_revisions.rb new file mode 100644 index 00000000000..668c4cec5d2 --- /dev/null +++ b/db/post_migrate/20210324043327_delete_orphan_post_revisions.rb @@ -0,0 +1,19 @@ +# frozen_string_literal: true + +class DeleteOrphanPostRevisions < ActiveRecord::Migration[6.0] + def up + sql = <<~SQL + DELETE FROM post_revisions + USING post_revisions pr + LEFT JOIN posts ON posts.id = pr.post_id + WHERE posts.id IS NULL + AND post_revisions.id = pr.id + SQL + + execute(sql) + end + + def down + raise ActiveRecord::IrreversibleMigration + end +end diff --git a/lib/post_destroyer.rb b/lib/post_destroyer.rb index 4353adf5e07..21d7973986b 100644 --- a/lib/post_destroyer.rb +++ b/lib/post_destroyer.rb @@ -151,6 +151,7 @@ class PostDestroyer Topic.reset_highest(@post.topic_id) end trash_public_post_actions + trash_revisions trash_user_actions remove_associated_replies remove_associated_notifications @@ -291,6 +292,11 @@ class PostDestroyer end end + def trash_revisions + return unless permanent? + @post.revisions.each(&:destroy!) + end + def agree(reviewable) notify_deletion(reviewable) result = reviewable.perform(@user, :agree_and_keep, post_was_deleted: true) diff --git a/spec/components/post_destroyer_spec.rb b/spec/components/post_destroyer_spec.rb index 1428fe1db6c..f4500c78e3b 100644 --- a/spec/components/post_destroyer_spec.rb +++ b/spec/components/post_destroyer_spec.rb @@ -978,6 +978,9 @@ 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) } + fab!(:post_revision) { Fabricate(:post_revision, post: private_post) } + fab!(:upload1) { Fabricate(:upload_s3, created_at: 5.hours.ago) } + fab!(:post_upload) { PostUpload.create(post: private_post, upload: upload1) } it "destroys the post and topic if deleting first post" do PostDestroyer.new(reply.user, reply, permanent: true).destroy @@ -988,6 +991,13 @@ describe PostDestroyer do expect { private_post.reload }.to raise_error(ActiveRecord::RecordNotFound) expect { private_message_topic.reload }.to raise_error(ActiveRecord::RecordNotFound) expect { post_action.reload }.to raise_error(ActiveRecord::RecordNotFound) + expect { post_revision.reload }.to raise_error(ActiveRecord::RecordNotFound) + expect { post_upload.reload }.to raise_error(ActiveRecord::RecordNotFound) + + Jobs::CleanUpUploads.new.reset_last_cleanup! + SiteSetting.clean_orphan_uploads_grace_period_hours = 1 + Jobs::CleanUpUploads.new.execute({}) + expect { upload1.reload }.to raise_error(ActiveRecord::RecordNotFound) end it 'soft delete if not creator of post or not private message' do @@ -996,6 +1006,8 @@ describe PostDestroyer do PostDestroyer.new(post.user, post, permanent: true).destroy expect(post.user_deleted).to be true + + expect(post_revision.reload.persisted?).to be true end it 'always destroy the post when the force_destroy option is passed' do