FIX: delete orphan post revisions (#12502)

I was adding specs to ensure that post actions and uploads are removed for permanently deleted posts.

I noticed that post revisions were not permanently destroyed. I added a migration to fix old data.

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

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)

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