From efedd9745fd9c34ac982941cca79f8ab8e517328 Mon Sep 17 00:00:00 2001
From: Robin Ward <robin.ward@gmail.com>
Date: Wed, 28 Mar 2018 13:57:20 -0400
Subject: [PATCH] PERF: Don't join on shared drafts unless you have to
---
app/controllers/list_controller.rb | 5 +++-
app/models/topic_list.rb | 27 ++++++++++---------
app/serializers/topic_list_item_serializer.rb | 5 +++-
app/serializers/topic_list_serializer.rb | 18 +++++++++++--
lib/topic_query.rb | 14 +++++-----
5 files changed, 47 insertions(+), 22 deletions(-)
diff --git a/app/controllers/list_controller.rb b/app/controllers/list_controller.rb
index 659e66f008a..4971861a657 100644
--- a/app/controllers/list_controller.rb
+++ b/app/controllers/list_controller.rb
@@ -71,7 +71,10 @@ class ListController < ApplicationController
list = TopicQuery.new(user, list_opts).public_send("list_#{filter}")
- if @category.present? && guardian.can_create_shared_draft?
+ if @category.present? &&
+ guardian.can_create_shared_draft? &&
+ @category.id != SiteSetting.shared_drafts_category.to_i
+
shared_drafts = TopicQuery.new(
user,
category: SiteSetting.shared_drafts_category,
diff --git a/app/models/topic_list.rb b/app/models/topic_list.rb
index 6de3eafcbe6..5aae90f7782 100644
--- a/app/models/topic_list.rb
+++ b/app/models/topic_list.rb
@@ -26,18 +26,21 @@ class TopicList
end
end
- attr_accessor :more_topics_url,
- :prev_topics_url,
- :draft,
- :draft_key,
- :draft_sequence,
- :filter,
- :for_period,
- :per_page,
- :top_tags,
- :current_user,
- :tags,
- :shared_drafts
+ attr_accessor(
+ :more_topics_url,
+ :prev_topics_url,
+ :draft,
+ :draft_key,
+ :draft_sequence,
+ :filter,
+ :for_period,
+ :per_page,
+ :top_tags,
+ :current_user,
+ :tags,
+ :shared_drafts,
+ :category
+ )
def initialize(filter, current_user, topics, opts = nil)
@filter = filter
diff --git a/app/serializers/topic_list_item_serializer.rb b/app/serializers/topic_list_item_serializer.rb
index 663cc2c9a26..a4544374349 100644
--- a/app/serializers/topic_list_item_serializer.rb
+++ b/app/serializers/topic_list_item_serializer.rb
@@ -1,6 +1,8 @@
class TopicListItemSerializer < ListableTopicSerializer
include TopicTagsMixin
+ attr_accessor :include_destination_category
+
attributes :views,
:like_count,
:has_summary,
@@ -30,8 +32,9 @@ class TopicListItemSerializer < ListableTopicSerializer
end
def category_id
+
# If it's a shared draft, show the destination topic instead
- if object.category_id == SiteSetting.shared_drafts_category.to_i && object.shared_draft
+ if include_destination_category && object.shared_draft
return object.shared_draft.category_id
end
diff --git a/app/serializers/topic_list_serializer.rb b/app/serializers/topic_list_serializer.rb
index 6ee27854ca8..f6dd513bf36 100644
--- a/app/serializers/topic_list_serializer.rb
+++ b/app/serializers/topic_list_serializer.rb
@@ -9,12 +9,26 @@ 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
+ object.topics.map do |t|
+ serializer = TopicListItemSerializer.new(t, scope: scope, root: false)
+
+ if scope.can_create_shared_draft? &&
+ object.category&.id == SiteSetting.shared_drafts_category.to_i
+
+ serializer.include_destination_category = true
+ end
+
+ serializer
+ end
+ end
+
def can_create_topic
scope.can_create?(Topic)
end
diff --git a/lib/topic_query.rb b/lib/topic_query.rb
index a6cbacf3f49..563eedf67a0 100644
--- a/lib/topic_query.rb
+++ b/lib/topic_query.rb
@@ -482,17 +482,19 @@ class TopicQuery
end
def apply_shared_drafts(result, category_id, options)
- viewing_shared = category_id && category_id == SiteSetting.shared_drafts_category.to_i
+ drafts_category_id = SiteSetting.shared_drafts_category.to_i
+ viewing_shared = category_id && category_id == drafts_category_id
if guardian.can_create_shared_draft?
- result = result.includes(:shared_draft).references(:shared_draft)
-
if options[:destination_category_id]
destination_category_id = get_category_id(options[:destination_category_id])
- return result.where("shared_drafts.category_id" => destination_category_id)
+ topic_ids = SharedDraft.where(category_id: destination_category_id).pluck(:topic_id)
+ return result.where(id: topic_ids)
+ elsif viewing_shared
+ result = result.includes(:shared_draft).references(:shared_draft)
+ else
+ return result.where('topics.category_id != ?', drafts_category_id)
end
-
- return result.where("shared_drafts.id IS NULL") unless viewing_shared
end
result