From 4b5977aa6a3b0ccd67efd92c8d319ba99dca30fa Mon Sep 17 00:00:00 2001 From: Robin Ward Date: Wed, 28 Mar 2018 15:35:13 -0400 Subject: [PATCH] Revert "PERF: Don't join on shared drafts unless you have to" This reverts commit efedd9745fd9c34ac982941cca79f8ab8e517328. --- 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, 22 insertions(+), 47 deletions(-) diff --git a/app/controllers/list_controller.rb b/app/controllers/list_controller.rb index 4971861a657..659e66f008a 100644 --- a/app/controllers/list_controller.rb +++ b/app/controllers/list_controller.rb @@ -71,10 +71,7 @@ class ListController < ApplicationController list = TopicQuery.new(user, list_opts).public_send("list_#{filter}") - if @category.present? && - guardian.can_create_shared_draft? && - @category.id != SiteSetting.shared_drafts_category.to_i - + if @category.present? && guardian.can_create_shared_draft? 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 5aae90f7782..6de3eafcbe6 100644 --- a/app/models/topic_list.rb +++ b/app/models/topic_list.rb @@ -26,21 +26,18 @@ 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, - :category - ) + 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 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 a4544374349..663cc2c9a26 100644 --- a/app/serializers/topic_list_item_serializer.rb +++ b/app/serializers/topic_list_item_serializer.rb @@ -1,8 +1,6 @@ class TopicListItemSerializer < ListableTopicSerializer include TopicTagsMixin - attr_accessor :include_destination_category - attributes :views, :like_count, :has_summary, @@ -32,9 +30,8 @@ class TopicListItemSerializer < ListableTopicSerializer end def category_id - # If it's a shared draft, show the destination topic instead - if include_destination_category && object.shared_draft + if object.category_id == SiteSetting.shared_drafts_category.to_i && 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 f6dd513bf36..6ee27854ca8 100644 --- a/app/serializers/topic_list_serializer.rb +++ b/app/serializers/topic_list_serializer.rb @@ -9,26 +9,12 @@ class TopicListSerializer < ApplicationSerializer :per_page, :top_tags, :tags, - :shared_drafts, - :topics + :shared_drafts + 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 563eedf67a0..a6cbacf3f49 100644 --- a/lib/topic_query.rb +++ b/lib/topic_query.rb @@ -482,19 +482,17 @@ class TopicQuery end def apply_shared_drafts(result, category_id, options) - drafts_category_id = SiteSetting.shared_drafts_category.to_i - viewing_shared = category_id && category_id == drafts_category_id + viewing_shared = category_id && category_id == SiteSetting.shared_drafts_category.to_i 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]) - 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) + return result.where("shared_drafts.category_id" => destination_category_id) end + + return result.where("shared_drafts.id IS NULL") unless viewing_shared end result