From 1a3e9cf571a794b28af2c6e1fea250fe02b852ef Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9gis=20Hanol?= Date: Mon, 12 Oct 2015 21:58:40 +0200 Subject: [PATCH] FIX: sorting was not working in /top PERF: remove double request when sorting topics lists --- .../controllers/discovery-sortable.js.es6 | 4 +- .../controllers/discovery/topics.js.es6 | 5 +- .../discourse/models/topic-list.js.es6 | 56 ++++++------------- .../javascripts/discourse/models/user.js.es6 | 4 ++ .../dynamic-route-builders.js.es6 | 12 ++-- .../discourse/routes/build-topic-route.js.es6 | 3 +- 6 files changed, 33 insertions(+), 51 deletions(-) diff --git a/app/assets/javascripts/discourse/controllers/discovery-sortable.js.es6 b/app/assets/javascripts/discourse/controllers/discovery-sortable.js.es6 index 3e7c2e52a9a..c0d0b0178d9 100644 --- a/app/assets/javascripts/discourse/controllers/discovery-sortable.js.es6 +++ b/app/assets/javascripts/discourse/controllers/discovery-sortable.js.es6 @@ -16,8 +16,6 @@ var controllerOpts = { }; // Aliases for the values -controllerOpts.queryParams.forEach(function(p) { - controllerOpts[p] = Em.computed.alias('controllers.discovery/topics.' + p); -}); +controllerOpts.queryParams.forEach(p => controllerOpts[p] = Em.computed.alias(`controllers.discovery/topics.${p}`)); export default Ember.Controller.extend(controllerOpts); diff --git a/app/assets/javascripts/discourse/controllers/discovery/topics.js.es6 b/app/assets/javascripts/discourse/controllers/discovery/topics.js.es6 index 99f0a4b025a..0a2b0462378 100644 --- a/app/assets/javascripts/discourse/controllers/discovery/topics.js.es6 +++ b/app/assets/javascripts/discourse/controllers/discovery/topics.js.es6 @@ -25,6 +25,7 @@ const controllerOpts = { } else { this.setProperties({ order: sortBy, ascending: false }); } + this.get('model').refreshSort(sortBy, this.get('ascending')); }, @@ -41,7 +42,7 @@ const controllerOpts = { refresh() { const filter = this.get('model.filter'); - this.setProperties({ order: 'default', ascending: false }); + this.setProperties({ order: "default", ascending: false }); // Don't refresh if we're still loading if (this.get('controllers.discovery.loading')) { return; } @@ -51,7 +52,7 @@ const controllerOpts = { // Lesson learned: Don't call `loading` yourself. this.set('controllers.discovery.loading', true); - this.store.findFiltered('topicList', {filter}).then((list) => { + this.store.findFiltered('topicList', {filter}).then(list => { Discourse.TopicList.hideUniformCategory(list, this.get('category')); this.setProperties({ model: list }); diff --git a/app/assets/javascripts/discourse/models/topic-list.js.es6 b/app/assets/javascripts/discourse/models/topic-list.js.es6 index a600221e65c..3422f9df4e3 100644 --- a/app/assets/javascripts/discourse/models/topic-list.js.es6 +++ b/app/assets/javascripts/discourse/models/topic-list.js.es6 @@ -25,51 +25,33 @@ function topicsFrom(result, store) { const TopicList = RestModel.extend({ canLoadMore: Em.computed.notEmpty("more_topics_url"), - forEachNew: function(topics, callback) { + forEachNew(topics, callback) { const topicIds = []; - _.each(this.get('topics'),function(topic) { - topicIds[topic.get('id')] = true; - }); - _.each(topics,function(topic) { - if(!topicIds[topic.id]) { + _.each(this.get('topics'), topic => topicIds[topic.get('id')] = true); + + _.each(topics, topic => { + if (!topicIds[topic.id]) { callback(topic); } }); }, - refreshSort: function(order, ascending) { - const self = this; - var params = this.get('params') || {}; - - params.order = order || params.order; - - if (ascending === undefined) { - params.ascending = ascending; - } else { - params.ascending = ascending; - } + refreshSort(order, ascending) { + let params = this.get('params') || {}; if (params.q) { // search is unique, nothing else allowed with it - params = {q: params.q}; + params = { q: params.q }; + } else { + params.order = order || params.order; + params.ascending = ascending; } - this.set('loaded', false); this.set('params', params); - - const store = this.store; - store.findFiltered('topicList', {filter: this.get('filter'), params}).then(function(tl) { - const newTopics = tl.get('topics'), - topics = self.get('topics'); - - topics.clear(); - topics.pushObjects(newTopics); - self.setProperties({ loaded: true, more_topics_url: tl.get('topic_list.more_topics_url') }); - }); }, - loadMore: function() { + loadMore() { if (this.get('loadingMore')) { return Ember.RSVP.resolve(); } const moreUrl = this.get('more_topics_url'); @@ -108,19 +90,17 @@ const TopicList = RestModel.extend({ // loads topics with these ids "before" the current topics - loadBefore: function(topic_ids){ + loadBefore(topic_ids) { const topicList = this, - topics = this.get('topics'); + topics = this.get('topics'); // refresh dupes - topics.removeObjects(topics.filter(function(topic){ - return topic_ids.indexOf(topic.get('id')) >= 0; - })); - - const url = Discourse.getURL("/") + this.get('filter') + "?topic_ids=" + topic_ids.join(","); + topics.removeObjects(topics.filter(topic => topic_ids.indexOf(topic.get('id')) >= 0)); + const url = `${Discourse.getURL("/")}${this.get('filter')}?topic_ids=${topic_ids.join(",")}`; const store = this.store; - return Discourse.ajax({ url }).then(function(result) { + + return Discourse.ajax({ url }).then(result => { let i = 0; topicList.forEachNew(topicsFrom(result, store), function(t) { // highlight the first of the new topics so we can get a visual feedback diff --git a/app/assets/javascripts/discourse/models/user.js.es6 b/app/assets/javascripts/discourse/models/user.js.es6 index d7727237177..87278959ef7 100644 --- a/app/assets/javascripts/discourse/models/user.js.es6 +++ b/app/assets/javascripts/discourse/models/user.js.es6 @@ -17,6 +17,10 @@ const User = RestModel.extend({ hasNotPosted: Em.computed.not("hasPosted"), canBeDeleted: Em.computed.and("can_be_deleted", "hasNotPosted"), + redirected_to_top: { + reason: null, + }, + @computed() stream() { return UserStream.create({ user: this }); diff --git a/app/assets/javascripts/discourse/pre-initializers/dynamic-route-builders.js.es6 b/app/assets/javascripts/discourse/pre-initializers/dynamic-route-builders.js.es6 index cbc4cdce543..acaef8f0577 100644 --- a/app/assets/javascripts/discourse/pre-initializers/dynamic-route-builders.js.es6 +++ b/app/assets/javascripts/discourse/pre-initializers/dynamic-route-builders.js.es6 @@ -12,27 +12,27 @@ export default { app.DiscoveryCategoryNoneRoute = buildCategoryRoute('latest', {no_subcategories: true}); const site = Discourse.Site.current(); - site.get('filters').forEach(function(filter) { + site.get('filters').forEach(filter => { app["Discovery" + filter.capitalize() + "Controller"] = DiscoverySortableController.extend(); app["Discovery" + filter.capitalize() + "Route"] = buildTopicRoute(filter); app["Discovery" + filter.capitalize() + "CategoryRoute"] = buildCategoryRoute(filter); app["Discovery" + filter.capitalize() + "CategoryNoneRoute"] = buildCategoryRoute(filter, {no_subcategories: true}); }); + Discourse.DiscoveryTopController = DiscoverySortableController.extend(); Discourse.DiscoveryTopRoute = buildTopicRoute('top', { actions: { - willTransition: function() { - this._super(); + willTransition() { Discourse.User.currentProp("should_be_redirected_to_top", false); Discourse.User.currentProp("redirected_to_top.reason", null); - return true; + return this._super(); } } }); - Discourse.DiscoveryTopCategoryRoute = buildCategoryRoute('top'); Discourse.DiscoveryTopCategoryNoneRoute = buildCategoryRoute('top', {no_subcategories: true}); - site.get('periods').forEach(function(period) { + + site.get('periods').forEach(period => { app["DiscoveryTop" + period.capitalize() + "Controller"] = DiscoverySortableController.extend(); app["DiscoveryTop" + period.capitalize() + "Route"] = buildTopicRoute('top/' + period); app["DiscoveryTop" + period.capitalize() + "CategoryRoute"] = buildCategoryRoute('top/' + period); diff --git a/app/assets/javascripts/discourse/routes/build-topic-route.js.es6 b/app/assets/javascripts/discourse/routes/build-topic-route.js.es6 index 36d54fd844f..342b2258eea 100644 --- a/app/assets/javascripts/discourse/routes/build-topic-route.js.es6 +++ b/app/assets/javascripts/discourse/routes/build-topic-route.js.es6 @@ -15,7 +15,6 @@ function filterQueryParams(params, defaultParams) { function findTopicList(store, tracking, filter, filterParams, extras) { extras = extras || {}; return new Ember.RSVP.Promise(function(resolve) { - const session = Discourse.Session.current(); if (extras.cached) { @@ -72,7 +71,7 @@ export default function(filter, extras) { // attempt to stop early cause we need this to be called before .sync ScreenTrack.current().stop(); - const findOpts = filterQueryParams(transition.queryParams), + const findOpts = filterQueryParams(data), findExtras = { cached: this.isPoppedState(transition) }; return findTopicList(this.store, this.topicTrackingState, filter, findOpts, findExtras);