PERF: Fix N+1 for non-staff users when tagging is enabled.

This commit is contained in:
Guo Xiang Tan 2018-11-19 12:53:33 +08:00
parent d1e3c213a7
commit b50fab2d72
3 changed files with 52 additions and 5 deletions

View File

@ -9,10 +9,22 @@ module TopicTagsMixin
def tags
# Calling method `pluck` along with `includes` causing N+1 queries
DiscourseTagging.filter_visible(topic.tags, scope).map(&:name)
tags = topic.tags.map(&:name)
if scope.is_staff?
tags
else
tags - (@options[:hidden_tag_names] || hidden_tag_names)
end
end
def topic
object.is_a?(Topic) ? object : object.topic
end
private
def hidden_tag_names
@hidden_tag_names ||= DiscourseTagging.hidden_tag_names(scope)
end
end

View File

@ -9,12 +9,27 @@ class TopicListSerializer < ApplicationSerializer
:per_page,
:top_tags,
:tags,
:shared_drafts
:shared_drafts,
:topics
has_many :topics, serializer: TopicListItemSerializer, embed: :objects
has_many :shared_drafts, serializer: TopicListItemSerializer, embed: :objects
has_many :tags, serializer: TagSerializer, embed: :objects
def topics
opts = {
scope: scope,
root: false
}
if SiteSetting.tagging_enabled && !scope.is_staff?
opts.merge!(hidden_tag_names: DiscourseTagging.hidden_tag_names(scope))
end
object.topics.map do |topic|
TopicListItemSerializer.new(topic, opts)
end
end
def can_create_topic
scope.can_create?(Topic)
end

View File

@ -45,6 +45,8 @@ describe TopicListItemSerializer do
end
describe 'hidden tags' do
let(:admin) { Fabricate(:admin) }
let(:user) { Fabricate(:user) }
let(:hidden_tag) { Fabricate(:tag, name: 'hidden') }
let(:staff_tag_group) { Fabricate(:tag_group, permissions: { "staff" => 1 }, tag_names: [hidden_tag.name]) }
@ -55,12 +57,30 @@ describe TopicListItemSerializer do
end
it 'returns hidden tag to staff' do
json = TopicListItemSerializer.new(topic, scope: Guardian.new(Fabricate(:admin)), root: false).as_json
json = TopicListItemSerializer.new(topic,
scope: Guardian.new(admin),
root: false
).as_json
expect(json[:tags]).to eq([hidden_tag.name])
end
it 'does not return hidden tag to non-staff' do
json = TopicListItemSerializer.new(topic, scope: Guardian.new(Fabricate(:user)), root: false).as_json
json = TopicListItemSerializer.new(topic,
scope: Guardian.new(user),
root: false
).as_json
expect(json[:tags]).to eq([])
end
it 'accepts an option to remove hidden tags' do
json = TopicListItemSerializer.new(topic,
scope: Guardian.new(user),
hidden_tag_names: [hidden_tag.name],
root: false
).as_json
expect(json[:tags]).to eq([])
end
end