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.
This commit is contained in:
Bianca Nenciu 2022-07-05 10:51:21 +03:00 committed by GitHub
parent 526e6e7a3b
commit a6c3369614
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 20 additions and 18 deletions

View File

@ -705,10 +705,13 @@ class PostsController < ApplicationController
private private
def user_posts(guardian, user_id, opts) def user_posts(guardian, user_id, opts)
posts = Post.includes(:user, :topic, :deleted_by, :user_actions) # Topic.unscoped is necessary to remove the default deleted_at: nil scope
.where(user_id: user_id) posts = Topic.unscoped do
.with_deleted Post.includes(:user, :topic, :deleted_by, :user_actions)
.order(created_at: :desc) .where(user_id: user_id)
.with_deleted
.order(created_at: :desc)
end
if guardian.user.moderator? if guardian.user.moderator?

View File

@ -52,15 +52,15 @@ class AdminUserActionSerializer < ApplicationSerializer
end end
def slug def slug
topic.slug object.topic&.slug
end end
def title def title
topic.title object.topic&.title
end end
def category_id def category_id
topic.category_id object.topic&.category_id
end end
def moderator_action def moderator_action
@ -80,15 +80,4 @@ class AdminUserActionSerializer < ApplicationSerializer
.select { |ua| [UserAction::REPLY, UserAction::RESPONSE].include? ua.action_type } .select { |ua| [UserAction::REPLY, UserAction::RESPONSE].include? ua.action_type }
.first.try(:action_type) .first.try(:action_type)
end 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 end

View File

@ -1893,6 +1893,16 @@ describe PostsController do
expect(response.status).to eq(200) expect(response.status).to eq(200)
end 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 it "doesn't return secured categories for moderators if they don't have access" do
Fabricate(:moderator) Fabricate(:moderator)