From 5fc150e057893bd71e3d7235d3e446418aace93a Mon Sep 17 00:00:00 2001
From: Robin Ward <robin.ward@gmail.com>
Date: Thu, 9 Oct 2014 14:37:23 -0400
Subject: [PATCH] A mucher saner API for updating the title of routes, even
 when nested. Properly sends the title of the page to google analytics

---
 .../javascripts/admin/routes/admin_route.js   | 12 ++--------
 app/assets/javascripts/discourse.js           | 14 ++++-------
 .../discourse/controllers/topic.js.es6        | 14 +++++++++--
 .../initializers/page-tracking.js.es6         | 10 ++++----
 .../javascripts/discourse/lib/page_tracker.js |  4 ++--
 .../javascripts/discourse/routes/about.js.es6 |  6 ++---
 .../discourse/routes/application.js.es6       |  7 ++++++
 .../discourse/routes/badges-index.js.es6      |  6 ++---
 .../discourse/routes/badges-show.js.es6       | 10 ++++++--
 .../routes/build-category-route.js.es6        | 12 ++++++----
 .../discourse/routes/build-topic-route.js.es6 | 16 ++++++-------
 .../discourse/routes/discourse_route.js       | 23 +++++++++++++++++++
 .../routes/discovery_categories_route.js      | 14 ++++-------
 .../discourse/routes/topic_route.js           |  8 +++++++
 .../javascripts/discourse/routes/user.js.es6  |  8 +++++++
 .../javascripts/discourse/views/topic.js.es6  |  5 ----
 .../javascripts/discourse/views/user.js.es6   |  9 +-------
 17 files changed, 106 insertions(+), 72 deletions(-)

diff --git a/app/assets/javascripts/admin/routes/admin_route.js b/app/assets/javascripts/admin/routes/admin_route.js
index 2ffc74c97e2..e90a18f5fb3 100644
--- a/app/assets/javascripts/admin/routes/admin_route.js
+++ b/app/assets/javascripts/admin/routes/admin_route.js
@@ -1,17 +1,9 @@
-/**
-  The base admin route
-
-  @class AdminRoute
-  @extends Discourse.Route
-  @namespace Discourse
-  @module Discourse
-**/
 Discourse.AdminRoute = Discourse.Route.extend({
   renderTemplate: function() {
     this.render('admin/templates/admin');
   },
 
-  activate: function() {
-    Discourse.set('title', I18n.t('admin_title'));
+  titleToken: function() {
+    return I18n.t('admin_title');
   }
 });
diff --git a/app/assets/javascripts/discourse.js b/app/assets/javascripts/discourse.js
index dfaa061720d..27b12c53abb 100644
--- a/app/assets/javascripts/discourse.js
+++ b/app/assets/javascripts/discourse.js
@@ -10,6 +10,7 @@ var DiscourseResolver = require('discourse/ember/resolver').default;
 
 window.Discourse = Ember.Application.createWithMixins(Discourse.Ajax, {
   rootElement: '#main',
+  _docTitle: null,
 
   getURL: function(url) {
     // If it's a non relative URL, return it.
@@ -25,13 +26,8 @@ window.Discourse = Ember.Application.createWithMixins(Discourse.Ajax, {
 
   Resolver: DiscourseResolver,
 
-  titleChanged: function() {
-    var title = "";
-
-    if (this.get('title')) {
-      title += "" + (this.get('title')) + " - ";
-    }
-    title += Discourse.SiteSettings.title;
+  _titleChanged: function() {
+    var title = this.get('_docTitle') || Discourse.SiteSettings.title;
 
     // if we change this we can trigger changes on document.title
     // only set if changed.
@@ -44,14 +40,14 @@ window.Discourse = Ember.Application.createWithMixins(Discourse.Ajax, {
       title = "(" + notifyCount + ") " + title;
     }
 
-    if(title !== document.title) {
+    if (title !== document.title) {
       // chrome bug workaround see: http://stackoverflow.com/questions/2952384/changing-the-window-title-when-focussing-the-window-doesnt-work-in-chrome
       window.setTimeout(function() {
         document.title = ".";
         document.title = title;
       }, 200);
     }
-  }.observes('title', 'hasFocus', 'notifyCount'),
+  }.observes('_docTitle', 'hasFocus', 'notifyCount'),
 
   faviconChanged: function() {
     if(Discourse.User.currentProp('dynamic_favicon')) {
diff --git a/app/assets/javascripts/discourse/controllers/topic.js.es6 b/app/assets/javascripts/discourse/controllers/topic.js.es6
index d163fff5a7a..8c54c841a54 100644
--- a/app/assets/javascripts/discourse/controllers/topic.js.es6
+++ b/app/assets/javascripts/discourse/controllers/topic.js.es6
@@ -11,11 +11,21 @@ export default ObjectController.extend(Discourse.SelectedPostsCount, {
 
   maxTitleLength: Discourse.computed.setting('max_topic_title_length'),
 
-  contextChanged: function(){
+  contextChanged: function() {
     this.set('controllers.search.searchContext', this.get('model.searchContext'));
   }.observes('topic'),
 
-  termChanged: function(){
+  _titleChanged: function() {
+    var title = this.get('title');
+    if (!Em.empty(title)) {
+
+      // Note normally you don't have to trigger this, but topic titles can be updated
+      // and are sometimes lazily loaded.
+      this.send('refreshTitle');
+    }
+  }.observes('title'),
+
+  termChanged: function() {
     var dropdown = this.get('controllers.header.visibleDropdown');
     var term = this.get('controllers.search.term');
 
diff --git a/app/assets/javascripts/discourse/initializers/page-tracking.js.es6 b/app/assets/javascripts/discourse/initializers/page-tracking.js.es6
index 9faab0c4acd..bfb3f3d7fc8 100644
--- a/app/assets/javascripts/discourse/initializers/page-tracking.js.es6
+++ b/app/assets/javascripts/discourse/initializers/page-tracking.js.es6
@@ -12,16 +12,18 @@ export default {
     // Out of the box, Discourse tries to track google analytics
     // if it is present
     if (typeof window._gaq !== 'undefined') {
-      pageTracker.on('change', function() {
-        window._gaq.push(['_trackPageview', window.location.pathname+window.location.search]);
+      pageTracker.on('change', function(url, title) {
+        window._gaq.push(["_set", "title", title]);
+        window._gaq.push(['_trackPageview', url]);
       });
       return;
     }
 
+
     // Also use Universal Analytics if it is present
     if (typeof window.ga !== 'undefined') {
-      pageTracker.on('change', function() {
-        window.ga('send', 'pageview', window.location.pathname+window.location.search);
+      pageTracker.on('change', function(url, title) {
+        window.ga('send', 'pageview', {page: url, title: title});
       });
     }
   }
diff --git a/app/assets/javascripts/discourse/lib/page_tracker.js b/app/assets/javascripts/discourse/lib/page_tracker.js
index cd581ee01e1..52da9ff6cb3 100644
--- a/app/assets/javascripts/discourse/lib/page_tracker.js
+++ b/app/assets/javascripts/discourse/lib/page_tracker.js
@@ -22,8 +22,8 @@ Discourse.PageTracker = Ember.Object.extend(Ember.Evented, {
         self = this;
 
     router.on('didTransition', function() {
-      var router = this;
-      self.trigger('change', router.get('url'));
+      this.send('refreshTitle');
+      self.trigger('change', this.get('url'), Discourse.get('_docTitle'));
     });
     this.set('started', true);
   }
diff --git a/app/assets/javascripts/discourse/routes/about.js.es6 b/app/assets/javascripts/discourse/routes/about.js.es6
index 962b0f50d73..3021dd0835f 100644
--- a/app/assets/javascripts/discourse/routes/about.js.es6
+++ b/app/assets/javascripts/discourse/routes/about.js.es6
@@ -5,9 +5,7 @@ export default Discourse.Route.extend({
     });
   },
 
-  setupController: function(controller, model) {
-    controller.set('model', model);
-    Discourse.set('title', I18n.t('about.simple_title'));
+  titleToken: function() {
+    return I18n.t('about.simple_title');
   }
 });
-
diff --git a/app/assets/javascripts/discourse/routes/application.js.es6 b/app/assets/javascripts/discourse/routes/application.js.es6
index ae75165f696..ddc4d635938 100644
--- a/app/assets/javascripts/discourse/routes/application.js.es6
+++ b/app/assets/javascripts/discourse/routes/application.js.es6
@@ -1,6 +1,13 @@
 var ApplicationRoute = Em.Route.extend({
 
+  siteTitle: Discourse.computed.setting('title'),
+
   actions: {
+    _collectTitleTokens: function(tokens) {
+      tokens.push(this.get('siteTitle'));
+      Discourse.set('_docTitle', tokens.join(' - '));
+    },
+
     showTopicEntrance: function(data) {
       this.controllerFor('topic-entrance').send('show', data);
     },
diff --git a/app/assets/javascripts/discourse/routes/badges-index.js.es6 b/app/assets/javascripts/discourse/routes/badges-index.js.es6
index 48992756b25..fb7e9905654 100644
--- a/app/assets/javascripts/discourse/routes/badges-index.js.es6
+++ b/app/assets/javascripts/discourse/routes/badges-index.js.es6
@@ -1,5 +1,4 @@
 export default Discourse.Route.extend({
-
   model: function() {
     if (PreloadStore.get('badges')) {
       return PreloadStore.getAndRemove('badges').then(function(json) {
@@ -10,8 +9,7 @@ export default Discourse.Route.extend({
     }
   },
 
-  setupController: function(controller, model) {
-    controller.set('model', model);
-    Discourse.set('title', I18n.t('badges.title'));
+  titleToken: function() {
+    return I18n.t('badges.title');
   }
 });
diff --git a/app/assets/javascripts/discourse/routes/badges-show.js.es6 b/app/assets/javascripts/discourse/routes/badges-show.js.es6
index e0b817a5e50..a9dbc00f16b 100644
--- a/app/assets/javascripts/discourse/routes/badges-show.js.es6
+++ b/app/assets/javascripts/discourse/routes/badges-show.js.es6
@@ -1,4 +1,4 @@
-export default Ember.Route.extend({
+export default Discourse.Route.extend({
   serialize: function(model) {
     return {id: model.get('id'), slug: model.get('name').replace(/[^A-Za-z0-9_]+/g, '-').toLowerCase()};
   },
@@ -13,12 +13,18 @@ export default Ember.Route.extend({
     }
   },
 
+  titleToken: function() {
+    var model = this.modelFor('badges.show');
+    if (model) {
+      return model.get('displayName');
+    }
+  },
+
   setupController: function(controller, model) {
     Discourse.UserBadge.findByBadgeId(model.get('id')).then(function(userBadges) {
       controller.set('userBadges', userBadges);
       controller.set('userBadgesLoaded', true);
     });
     controller.set('model', model);
-    Discourse.set('title', model.get('displayName'));
   }
 });
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 645df896dda..26d753eb43a 100644
--- a/app/assets/javascripts/discourse/routes/build-category-route.js.es6
+++ b/app/assets/javascripts/discourse/routes/build-category-route.js.es6
@@ -61,13 +61,17 @@ export default function(filter, params) {
       });
     },
 
+    titleToken: function() {
+      var filterText = I18n.t('filters.' + filter.replace('/', '.') + '.title', {count: 0}),
+          model = this.currentModel;
+
+      return I18n.t('filters.with_category', { filter: filterText, category: model.get('name') });
+    },
+
     setupController: function(controller, model) {
       var topics = this.get('topics'),
           periods = this.controllerFor('discovery').get('periods'),
-          periodId = topics.get('for_period') || (filter.indexOf('/') > 0 ? filter.split('/')[1] : ''),
-          filterText = I18n.t('filters.' + filter.replace('/', '.') + '.title', {count: 0});
-
-      Discourse.set('title', I18n.t('filters.with_category', { filter: filterText, category: model.get('name') }));
+          periodId = topics.get('for_period') || (filter.indexOf('/') > 0 ? filter.split('/')[1] : '');
 
       this.controllerFor('navigation/category').set('canCreateTopic', topics.get('can_create_topic'));
       this.controllerFor('discovery/topics').setProperties({
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 98d5bae6df1..3b764cc6a7d 100644
--- a/app/assets/javascripts/discourse/routes/build-topic-route.js.es6
+++ b/app/assets/javascripts/discourse/routes/build-topic-route.js.es6
@@ -32,6 +32,13 @@ export default function(filter, extras) {
       return Discourse.TopicList.list(filter, findOpts, extras);
     },
 
+    titleToken: function() {
+      if (filter === Discourse.Utilities.defaultHomepage()) { return; }
+
+      var filterText = I18n.t('filters.' + filter.replace('/', '.') + '.title', {count: 0});
+      return I18n.t('filters.with_topics', {filter: filterText});
+    },
+
     setupController: function(controller, model, trans) {
 
       controller.setProperties(Em.getProperties(trans, _.keys(queryParams).map(function(v){
@@ -39,14 +46,7 @@ export default function(filter, extras) {
       })));
 
       var periods = this.controllerFor('discovery').get('periods'),
-          periodId = model.get('for_period') || (filter.indexOf('/') > 0 ? filter.split('/')[1] : ''),
-          filterText = I18n.t('filters.' + filter.replace('/', '.') + '.title', {count: 0});
-
-      if (filter === Discourse.Utilities.defaultHomepage()) {
-        Discourse.set('title', '');
-      } else {
-        Discourse.set('title', I18n.t('filters.with_topics', {filter: filterText}));
-      }
+          periodId = model.get('for_period') || (filter.indexOf('/') > 0 ? filter.split('/')[1] : '');
 
       this.controllerFor('discovery/topics').setProperties({
         model: model,
diff --git a/app/assets/javascripts/discourse/routes/discourse_route.js b/app/assets/javascripts/discourse/routes/discourse_route.js
index 820c3c0b09a..2302716085a 100644
--- a/app/assets/javascripts/discourse/routes/discourse_route.js
+++ b/app/assets/javascripts/discourse/routes/discourse_route.js
@@ -21,6 +21,29 @@ Discourse.Route = Em.Route.extend({
     Em.run.scheduleOnce('afterRender', Discourse.Route, 'cleanDOM');
   },
 
+  actions: {
+    _collectTitleTokens: function(tokens) {
+      // If there's a title token method, call it and get the token
+      if (this.titleToken) {
+        var t = this.titleToken();
+        if (t && t.length) {
+          if (t instanceof Array) {
+            t.forEach(function(ti) {
+              tokens.push(ti);
+            });
+          } else {
+            tokens.push(t);
+          }
+        }
+      }
+      return true;
+    },
+
+    refreshTitle: function() {
+      this.send('_collectTitleTokens', []);
+    }
+  },
+
   redirectIfLoginRequired: function() {
     var app = this.controllerFor('application');
     if (app.get('loginRequired')) {
diff --git a/app/assets/javascripts/discourse/routes/discovery_categories_route.js b/app/assets/javascripts/discourse/routes/discovery_categories_route.js
index ffc5042a5ff..13f066865be 100644
--- a/app/assets/javascripts/discourse/routes/discovery_categories_route.js
+++ b/app/assets/javascripts/discourse/routes/discovery_categories_route.js
@@ -1,11 +1,3 @@
-/**
-  The route for handling the "Categories" view
-
-  @class DiscoveryCategoriesRoute
-  @extends Discourse.Route
-  @namespace Discourse
-  @module Discourse
-**/
 Discourse.DiscoveryCategoriesRoute = Discourse.Route.extend(Discourse.OpenComposer, {
   renderTemplate: function() {
     this.render('navigation/categories', { outlet: 'navigation-bar' });
@@ -31,16 +23,18 @@ Discourse.DiscoveryCategoriesRoute = Discourse.Route.extend(Discourse.OpenCompos
     });
   },
 
+  titleToken: function() {
+    return I18n.t('filters.categories.title');
+  },
+
   setupController: function(controller, model) {
     controller.set('model', model);
-    Discourse.set('title', I18n.t('filters.categories.title'));
 
     // Only show either the Create Category or Create Topic button
     this.controllerFor('navigation/categories').set('canCreateCategory', model.get('can_create_category'));
     this.controllerFor('navigation/categories').set('canCreateTopic', model.get('can_create_topic') && !model.get('can_create_category'));
 
     this.openTopicDraft(model);
-
   },
 
   actions: {
diff --git a/app/assets/javascripts/discourse/routes/topic_route.js b/app/assets/javascripts/discourse/routes/topic_route.js
index 583f008cc70..a02a635bc67 100644
--- a/app/assets/javascripts/discourse/routes/topic_route.js
+++ b/app/assets/javascripts/discourse/routes/topic_route.js
@@ -12,7 +12,15 @@ Discourse.TopicRoute = Discourse.Route.extend({
     show_deleted: { replace: true }
   },
 
+  titleToken: function() {
+    var model = this.modelFor('topic');
+    if (model) {
+      return model.get('title');
+    }
+  },
+
   actions: {
+
     showTopicAdminMenu: function() {
       this.controllerFor("topic-admin-menu").send("show");
     },
diff --git a/app/assets/javascripts/discourse/routes/user.js.es6 b/app/assets/javascripts/discourse/routes/user.js.es6
index 3bcd085acc7..7ae758de0b4 100644
--- a/app/assets/javascripts/discourse/routes/user.js.es6
+++ b/app/assets/javascripts/discourse/routes/user.js.es6
@@ -1,5 +1,13 @@
 export default Discourse.Route.extend({
 
+  titleToken: function() {
+    var model = this.modelFor('user');
+    var username = model.get('username');
+    if (username) {
+      return [I18n.t("user.profile"), username];
+    }
+  },
+
   actions: {
     logout: function() {
       Discourse.logout();
diff --git a/app/assets/javascripts/discourse/views/topic.js.es6 b/app/assets/javascripts/discourse/views/topic.js.es6
index bbd0f1e170a..0cfc4a3d76a 100644
--- a/app/assets/javascripts/discourse/views/topic.js.es6
+++ b/app/assets/javascripts/discourse/views/topic.js.es6
@@ -17,11 +17,6 @@ export default Discourse.View.extend(AddCategoryClass, Discourse.Scrolling, {
 
   postStream: Em.computed.alias('controller.postStream'),
 
-  _updateTitle: function() {
-    var title = this.get('topic.title');
-    if (title) return Discourse.set('title', _.unescape(title));
-  }.observes('topic.loaded', 'topic.title'),
-
   _composeChanged: function() {
     var composerController = Discourse.get('router.composerController');
     composerController.clearState();
diff --git a/app/assets/javascripts/discourse/views/user.js.es6 b/app/assets/javascripts/discourse/views/user.js.es6
index 25413955b9a..7c795652ba9 100644
--- a/app/assets/javascripts/discourse/views/user.js.es6
+++ b/app/assets/javascripts/discourse/views/user.js.es6
@@ -1,11 +1,4 @@
 export default Ember.View.extend(Discourse.ScrollTop, {
   templateName: 'user/user',
-  userBinding: 'controller.content',
-
-  updateTitle: function() {
-    var username = this.get('user.username');
-    if (username) {
-      Discourse.set('title', "" + (I18n.t("user.profile")) + " - " + username);
-    }
-  }.observes('user.loaded', 'user.username')
+  userBinding: 'controller.content'
 });