From c093187d342c920d8f411b511ee759cca002a856 Mon Sep 17 00:00:00 2001 From: Sam Date: Tue, 9 Sep 2014 16:56:20 +1000 Subject: [PATCH] FIX: unread/new counts out of sync with tabs We were serving cached lists of topics to non popstate reqs --- app/assets/javascripts/discourse/models/topic_list.js | 5 ++++- .../discourse/routes/build-category-route.js.es6 | 2 ++ .../discourse/routes/build-topic-route.js.es6 | 7 +++++-- .../javascripts/discourse/routes/discourse_location.js | 2 ++ .../javascripts/discourse/routes/discourse_route.js | 9 +++++++++ 5 files changed, 22 insertions(+), 3 deletions(-) diff --git a/app/assets/javascripts/discourse/models/topic_list.js b/app/assets/javascripts/discourse/models/topic_list.js index 0402f4ac188..cf393d5afee 100644 --- a/app/assets/javascripts/discourse/models/topic_list.js +++ b/app/assets/javascripts/discourse/models/topic_list.js @@ -197,9 +197,12 @@ Discourse.TopicList.reopenClass({ **/ list: function(filter, params) { var session = Discourse.Session.current(), - list = session.get('topicList'), + list = params["cache"] && session.get('topicList'), tracking = Discourse.TopicTrackingState.current(); + // only used to control caching + delete params["cache"]; + return new Ember.RSVP.Promise(function(resolve) { // Try to use the cached version if (list && (list.get('filter') === filter) && diff --git a/app/assets/javascripts/discourse/routes/build-category-route.js.es6 b/app/assets/javascripts/discourse/routes/build-category-route.js.es6 index f060bb42d11..3a5d4156baf 100644 --- a/app/assets/javascripts/discourse/routes/build-category-route.js.es6 +++ b/app/assets/javascripts/discourse/routes/build-category-route.js.es6 @@ -52,6 +52,8 @@ export default function(filter, params) { var findOpts = filterQueryParams(transaction.queryParams, params); + findOpts["cache"] = this.get("router.location.poppedState"); + return Discourse.TopicList.list(listFilter, findOpts).then(function(list) { // If all the categories are the same, we can hide them var hideCategory = !list.get('topics').find(function (t) { return t.get('category') !== model; }); 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 e92464e4fb9..8602ff1f10b 100644 --- a/app/assets/javascripts/discourse/routes/build-topic-route.js.es6 +++ b/app/assets/javascripts/discourse/routes/build-topic-route.js.es6 @@ -21,12 +21,15 @@ export default function(filter, extras) { this.controllerFor('navigation/default').set('filterMode', filter); }, - model: function(data, transaction) { + model: function(data, transition) { // attempt to stop early cause we need this to be called before .sync Discourse.ScreenTrack.current().stop(); - var findOpts = filterQueryParams(transaction.queryParams); + var findOpts = filterQueryParams(transition.queryParams); + + findOpts["cache"] = this.get("router.location.poppedState"); + return Discourse.TopicList.list(filter, findOpts); }, diff --git a/app/assets/javascripts/discourse/routes/discourse_location.js b/app/assets/javascripts/discourse/routes/discourse_location.js index 9dda10165a3..be3f0d939a4 100644 --- a/app/assets/javascripts/discourse/routes/discourse_location.js +++ b/app/assets/javascripts/discourse/routes/discourse_location.js @@ -189,6 +189,8 @@ Ember.DiscourseLocation = Ember.Object.extend({ popstateCallbacks.forEach(function(cb) { cb(url); }); + + self.set('poppedState',true); callback(url); }); }, diff --git a/app/assets/javascripts/discourse/routes/discourse_route.js b/app/assets/javascripts/discourse/routes/discourse_route.js index b43148b0576..e030e75e7d5 100644 --- a/app/assets/javascripts/discourse/routes/discourse_route.js +++ b/app/assets/javascripts/discourse/routes/discourse_route.js @@ -19,6 +19,15 @@ Discourse.Route = Em.Route.extend({ activate: function() { this._super(); Em.run.scheduleOnce('afterRender', Discourse.Route, 'cleanDOM'); + + // the chain of events sucks, we get an event from discourse location + // it goes ahead and sets it, then it call model, then it deactivates + // old route and activates new + // + // if we want to pin this on the location, we need to amend onUpdateURL + // to pass this in to the callback it gets, this would require ember + // API changes + this.set('router.location.poppedState', false); }, redirectIfLoginRequired: function() {