From ec2ed5b7f640f87d7dbc6fe2d289f45a7be372d9 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Lo=C3=AFc=20Guitaut?= <loic@discourse.org>
Date: Wed, 18 Jan 2023 17:40:21 +0100
Subject: [PATCH] FIX: Delete reviewables associated to posts automatically
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Currently we don’t have an association between reviewables and posts.
This sometimes leads to inconsistencies in the DB as a post can have
been deleted but an associated reviewable is still present.

This patch addresses this issue simply by adding a new association to
the `Post` model and by using the `dependent: :destroy` option.
---
 app/models/post.rb                              |  1 +
 ...39_drop_orphaned_reviewable_flagged_posts.rb | 17 +++++++++++++++++
 spec/models/post_spec.rb                        |  4 +++-
 3 files changed, 21 insertions(+), 1 deletion(-)
 create mode 100644 db/post_migrate/20230119091939_drop_orphaned_reviewable_flagged_posts.rb

diff --git a/app/models/post.rb b/app/models/post.rb
index 7aa9155264a..736295205cb 100644
--- a/app/models/post.rb
+++ b/app/models/post.rb
@@ -61,6 +61,7 @@ class Post < ActiveRecord::Base
   belongs_to :image_upload, class_name: "Upload"
 
   has_many :post_hotlinked_media, dependent: :destroy, class_name: "PostHotlinkedMedia"
+  has_many :reviewables, as: :target, dependent: :destroy
 
   validates_with PostValidator, unless: :skip_validation
 
diff --git a/db/post_migrate/20230119091939_drop_orphaned_reviewable_flagged_posts.rb b/db/post_migrate/20230119091939_drop_orphaned_reviewable_flagged_posts.rb
new file mode 100644
index 00000000000..3efe3c06882
--- /dev/null
+++ b/db/post_migrate/20230119091939_drop_orphaned_reviewable_flagged_posts.rb
@@ -0,0 +1,17 @@
+# frozen_string_literal: true
+
+class DropOrphanedReviewableFlaggedPosts < ActiveRecord::Migration[7.0]
+  def up
+    DB.exec(<<~SQL)
+      DELETE FROM reviewables
+      WHERE reviewables.type = 'ReviewableFlaggedPost'
+        AND reviewables.status = 0
+        AND reviewables.target_type = 'Post'
+        AND NOT EXISTS(SELECT 1 FROM posts WHERE posts.id = reviewables.target_id)
+    SQL
+  end
+
+  def down
+    raise ActiveRecord::IrreversibleMigration
+  end
+end
diff --git a/spec/models/post_spec.rb b/spec/models/post_spec.rb
index deb5e637f9e..7fdb40fbaa1 100644
--- a/spec/models/post_spec.rb
+++ b/spec/models/post_spec.rb
@@ -3,9 +3,11 @@
 RSpec.describe Post do
   fab!(:coding_horror) { Fabricate(:coding_horror) }
 
+  let(:upload_path) { Discourse.store.upload_path }
+
   before { Oneboxer.stubs :onebox }
 
-  let(:upload_path) { Discourse.store.upload_path }
+  it { is_expected.to have_many(:reviewables).dependent(:destroy) }
 
   describe "#hidden_reasons" do
     context "when verifying enum sequence" do