From 5d37f8673bc73f04a2632515c3e072d9a8fa34be Mon Sep 17 00:00:00 2001 From: Guo Xiang Tan Date: Tue, 26 Sep 2017 14:42:27 +0800 Subject: [PATCH] PERF: Only send down suggested payload when loading last chunk. --- .../discourse/models/post-stream.js.es6 | 5 +++ .../discourse/models/topic-details.js.es6 | 7 ---- .../javascripts/discourse/models/topic.js.es6 | 11 ++++++ .../templates/components/suggested-topics.hbs | 4 +- .../javascripts/discourse/templates/topic.hbs | 2 +- app/controllers/topics_controller.rb | 7 +++- app/serializers/suggested_topics_mixin.rb | 15 ++++++++ .../topic_view_posts_serializer.rb | 1 + app/serializers/topic_view_serializer.rb | 7 +--- .../serializers/topic_view_serializer_spec.rb | 38 +++++++++++++++++++ 10 files changed, 80 insertions(+), 17 deletions(-) create mode 100644 app/serializers/suggested_topics_mixin.rb create mode 100644 spec/serializers/topic_view_serializer_spec.rb diff --git a/app/assets/javascripts/discourse/models/post-stream.js.es6 b/app/assets/javascripts/discourse/models/post-stream.js.es6 index de1ab50f782..1a9ea463b9a 100644 --- a/app/assets/javascripts/discourse/models/post-stream.js.es6 +++ b/app/assets/javascripts/discourse/models/post-stream.js.es6 @@ -741,6 +741,11 @@ export default RestModel.extend({ const store = this.store; return ajax(url, {data}).then(result => { const posts = Ember.get(result, "post_stream.posts"); + + if (result.suggested_topics) { + this.set('topic.suggested_topics', result.suggested_topics); + } + if (posts) { posts.forEach(p => this.storePost(store.createRecord('post', p))); } diff --git a/app/assets/javascripts/discourse/models/topic-details.js.es6 b/app/assets/javascripts/discourse/models/topic-details.js.es6 index 72ef0d0fe9d..4c9fdc2c763 100644 --- a/app/assets/javascripts/discourse/models/topic-details.js.es6 +++ b/app/assets/javascripts/discourse/models/topic-details.js.es6 @@ -18,13 +18,6 @@ const TopicDetails = RestModel.extend({ }); } - if (details.suggested_topics) { - const store = this.store; - details.suggested_topics = details.suggested_topics.map(function (st) { - return store.createRecord('topic', st); - }); - } - if (details.participants) { details.participants = details.participants.map(function (p) { p.topic = topic; diff --git a/app/assets/javascripts/discourse/models/topic.js.es6 b/app/assets/javascripts/discourse/models/topic.js.es6 index 60282358f0c..fb3e16b8887 100644 --- a/app/assets/javascripts/discourse/models/topic.js.es6 +++ b/app/assets/javascripts/discourse/models/topic.js.es6 @@ -99,6 +99,17 @@ const Topic = RestModel.extend({ return newTags; }, + @computed("suggested_topics") + suggestedTopics(suggestedTopics) { + if (suggestedTopics) { + const store = this.store; + + return this.set('suggested_topics', suggestedTopics.map(st => { + return store.createRecord('topic', st); + })); + } + }, + replyCount: function() { return this.get('posts_count') - 1; }.property('posts_count'), diff --git a/app/assets/javascripts/discourse/templates/components/suggested-topics.hbs b/app/assets/javascripts/discourse/templates/components/suggested-topics.hbs index e4304644f51..76127f2bf50 100644 --- a/app/assets/javascripts/discourse/templates/components/suggested-topics.hbs +++ b/app/assets/javascripts/discourse/templates/components/suggested-topics.hbs @@ -4,9 +4,9 @@ {{basic-topic-list hideCategory="true" showPosters="true" - topics=topic.details.suggested_topics}} + topics=topic.suggestedTopics}} {{else}} - {{basic-topic-list topics=topic.details.suggested_topics}} + {{basic-topic-list topics=topic.suggestedTopics}} {{/if}}

{{{browseMoreMessage}}}

diff --git a/app/assets/javascripts/discourse/templates/topic.hbs b/app/assets/javascripts/discourse/templates/topic.hbs index 1489bf621df..d25a035a66e 100644 --- a/app/assets/javascripts/discourse/templates/topic.hbs +++ b/app/assets/javascripts/discourse/templates/topic.hbs @@ -245,7 +245,7 @@ {{plugin-outlet name="topic-above-suggested" args=(hash model=model)}} - {{#if model.details.suggested_topics.length}} + {{#if model.suggestedTopics.length}} {{suggested-topics topic=model}} {{/if}} {{/if}} diff --git a/app/controllers/topics_controller.rb b/app/controllers/topics_controller.rb index 7a915c9870e..c6d29c67347 100644 --- a/app/controllers/topics_controller.rb +++ b/app/controllers/topics_controller.rb @@ -717,7 +717,12 @@ class TopicsController < ApplicationController return end - topic_view_serializer = TopicViewSerializer.new(@topic_view, scope: guardian, root: false, include_raw: !!params[:include_raw]) + topic_view_serializer = TopicViewSerializer.new(@topic_view, + scope: guardian, + root: false, + include_raw: !!params[:include_raw], + page: params[:page] + ) respond_to do |format| format.html do diff --git a/app/serializers/suggested_topics_mixin.rb b/app/serializers/suggested_topics_mixin.rb new file mode 100644 index 00000000000..361a7f62a8c --- /dev/null +++ b/app/serializers/suggested_topics_mixin.rb @@ -0,0 +1,15 @@ +module SuggestedTopicsMixin + def self.included(klass) + klass.attributes :suggested_topics + end + + def include_suggested_topics? + object.next_page.nil? && object.suggested_topics&.topics.present? + end + + def suggested_topics + object.suggested_topics.topics.map do |t| + SuggestedTopicSerializer.new(t, scope: scope, root: false) + end + end +end diff --git a/app/serializers/topic_view_posts_serializer.rb b/app/serializers/topic_view_posts_serializer.rb index 0bba907794e..98c6cabff76 100644 --- a/app/serializers/topic_view_posts_serializer.rb +++ b/app/serializers/topic_view_posts_serializer.rb @@ -1,5 +1,6 @@ class TopicViewPostsSerializer < ApplicationSerializer include PostStreamSerializerMixin + include SuggestedTopicsMixin attributes :id diff --git a/app/serializers/topic_view_serializer.rb b/app/serializers/topic_view_serializer.rb index ffe6f706b76..ab1068e4a51 100644 --- a/app/serializers/topic_view_serializer.rb +++ b/app/serializers/topic_view_serializer.rb @@ -3,6 +3,7 @@ require_dependency 'new_post_manager' class TopicViewSerializer < ApplicationSerializer include PostStreamSerializerMixin + include SuggestedTopicsMixin include ApplicationHelper def self.attributes_from_topic(*list) @@ -94,12 +95,6 @@ class TopicViewSerializer < ApplicationSerializer end end - if object.suggested_topics&.topics.present? - result[:suggested_topics] = object.suggested_topics.topics.map do |t| - SuggestedTopicSerializer.new(t, scope: scope, root: false) - end - end - if object.links.present? result[:links] = object.links.map do |user| TopicLinkSerializer.new(user, scope: scope, root: false) diff --git a/spec/serializers/topic_view_serializer_spec.rb b/spec/serializers/topic_view_serializer_spec.rb new file mode 100644 index 00000000000..21b75d19e3f --- /dev/null +++ b/spec/serializers/topic_view_serializer_spec.rb @@ -0,0 +1,38 @@ +require 'rails_helper' + +describe TopicViewSerializer do + let(:topic) { Fabricate(:topic) } + let(:user) { Fabricate(:user) } + + describe '#suggested_topics' do + let(:topic2) { Fabricate(:topic) } + + before do + TopicUser.update_last_read(user, topic2.id, 0, 0) + end + + describe 'when loading last chunk' do + it 'should include suggested topics' do + topic_view = TopicView.new(topic.id, user) + json = described_class.new(topic_view, scope: Guardian.new(user), root: false).as_json + + expect(json[:suggested_topics].first.id).to eq(topic2.id) + end + end + + describe 'when not loading last chunk' do + let(:post) { Fabricate(:post, topic: topic) } + let(:post2) { Fabricate(:post, topic: topic) } + + it 'should not include suggested topics' do + post + post2 + topic_view = TopicView.new(topic.id, user, post_ids: [post.id]) + topic_view.next_page + json = described_class.new(topic_view, scope: Guardian.new(user), root: false).as_json + + expect(json[:suggested_topics]).to eq(nil) + end + end + end +end