From a6c3369614dce8061048501f23e95342ab296aaf Mon Sep 17 00:00:00 2001
From: Bianca Nenciu <nbianca@users.noreply.github.com>
Date: Tue, 5 Jul 2022 10:51:21 +0300
Subject: [PATCH] FIX: Posts can belong to hard-deleted topics (#17329)

* FIX: Posts can belong to hard-deleted topics

This was a problem when serializing deleted posts because they might
belong to a topic that was permanently deleted. This caused to DB
lookup to fail immediately and raise an exception. In this case, the
endpoint returned a 404.

* FIX: Remove N+1 queries

Deleted topics were not loaded because of the default scope that
filters out all deleted topics. It executed a query for each deleted
topic.
---
 app/controllers/posts_controller.rb             | 11 +++++++----
 app/serializers/admin_user_action_serializer.rb | 17 +++--------------
 spec/requests/posts_controller_spec.rb          | 10 ++++++++++
 3 files changed, 20 insertions(+), 18 deletions(-)

diff --git a/app/controllers/posts_controller.rb b/app/controllers/posts_controller.rb
index b75866ba71e..2a905751ab7 100644
--- a/app/controllers/posts_controller.rb
+++ b/app/controllers/posts_controller.rb
@@ -705,10 +705,13 @@ class PostsController < ApplicationController
   private
 
   def user_posts(guardian, user_id, opts)
-    posts = Post.includes(:user, :topic, :deleted_by, :user_actions)
-      .where(user_id: user_id)
-      .with_deleted
-      .order(created_at: :desc)
+    # Topic.unscoped is necessary to remove the default deleted_at: nil scope
+    posts = Topic.unscoped do
+      Post.includes(:user, :topic, :deleted_by, :user_actions)
+        .where(user_id: user_id)
+        .with_deleted
+        .order(created_at: :desc)
+    end
 
     if guardian.user.moderator?
 
diff --git a/app/serializers/admin_user_action_serializer.rb b/app/serializers/admin_user_action_serializer.rb
index 497f472ffe3..4dd05564baf 100644
--- a/app/serializers/admin_user_action_serializer.rb
+++ b/app/serializers/admin_user_action_serializer.rb
@@ -52,15 +52,15 @@ class AdminUserActionSerializer < ApplicationSerializer
   end
 
   def slug
-    topic.slug
+    object.topic&.slug
   end
 
   def title
-    topic.title
+    object.topic&.title
   end
 
   def category_id
-    topic.category_id
+    object.topic&.category_id
   end
 
   def moderator_action
@@ -80,15 +80,4 @@ class AdminUserActionSerializer < ApplicationSerializer
       .select { |ua| [UserAction::REPLY, UserAction::RESPONSE].include? ua.action_type }
       .first.try(:action_type)
   end
-
-  private
-
-  # we need this to handle deleted topics which aren't loaded via the .includes(:topic)
-  # because Rails 4 "unscoped" support is bugged (cf. https://github.com/rails/rails/issues/13775)
-  def topic
-    return @topic if @topic
-    @topic = object.topic || Topic.with_deleted.find(object.topic_id)
-    @topic
-  end
-
 end
diff --git a/spec/requests/posts_controller_spec.rb b/spec/requests/posts_controller_spec.rb
index c7c1eb0a3f1..f975533ed21 100644
--- a/spec/requests/posts_controller_spec.rb
+++ b/spec/requests/posts_controller_spec.rb
@@ -1893,6 +1893,16 @@ describe PostsController do
         expect(response.status).to eq(200)
       end
 
+      it "does not raise if topic has been permanently deleted" do
+        post = Fabricate(:post, user: admin)
+        PostDestroyer.new(admin, post).destroy
+        post.update!(topic_id: -1000)
+
+        sign_in(admin)
+        get "/posts/#{admin.username}/deleted.json"
+        expect(response.status).to eq(200)
+      end
+
       it "doesn't return secured categories for moderators if they don't have access" do
         Fabricate(:moderator)