Converted service and unit views to use promises

app/app.js

Index: app/app.js
=== modified file 'app/app.js'
--- app/app.js	2013-04-30 15:49:27 +0000
+++ app/app.js	2013-05-02 19:57:17 +0000
@@ -252,6 +252,8 @@
         }
       }
 
+      this.renderEnvironment = true;
+
       // This attribute is used by the namespaced URL tracker.
       // _routeSeen is part of a mechanism to prevent non-namespaced routes
       // from being processed multiple times when multiple namespaces are
@@ -501,24 +503,61 @@
      */
     show_unit: function(req) {
       // This replacement honors service names that have a hyphen in them.
-      var unit_id = req.params.id.replace(/^(\S+)-(\d+)$/, '$1/$2');
-      var unit = this.db.units.getById(unit_id);
-      if (unit) {
-        // Once the unit is loaded we need to get the full details of the
-        // service.  Otherwise the relations data will not be available.
-        var service = this.db.services.getById(unit.service);
-      }
-      this.showView(
-          'unit',
-          // The querystring is used to handle highlighting relation rows in
-          // links from notifications about errors.
-          { getModelURL: Y.bind(this.getModelURL, this),
-            unit: unit,
+      var unitId = req.params.id.replace(/^(\S+)-(\d+)$/, '$1/$2');
+      var serviceId = unitId.split('/')[0];
+      var self = this,
+          options = {
+            getModelURL: Y.bind(this.getModelURL, this),
             db: this.db,
             env: this.env,
             querystring: req.query,
             landscape: this.landscape,
-            nsRouter: this.nsRouter });
+            nsRouter: this.nsRouter
+          };
+      // Give the page 100 milliseconds to try and load the model
+      // before we show a loading screen.
+      var handle = setTimeout(function() {

[bcsaller, 2013/05/02 21:25:46]

I can envision a way that we can declare requirements for views and reuse this
pattern such that 

create an Array of promises for requirements
create timeout handler
Y.batch(requirementPromises)
.then(...
      cancelTimeout
      showView
)

Maybe something for the future, its a DRY violation to have to repeat this setup
pattern. 

Fine for now though.

+        self.showView('unit', options);
+      }, 100);
+
+      var promise = this.modelController.getService(serviceId);
+      promise.then(
+          // If there is a service available then we need to check if the unit
+          // is available.
+          function(models) {
+            clearTimeout(handle);
+            var unit = self.db.units.getById(unitId);
+            if (unit) {
+              options.unit = unit;
+              self.showView('unit', options);
+            } else {
+              // If there is no unit available in this service then we show
+              // a notification and then redirect to the service.
+              self.db.notifications.add(
+                  new Y.juju.models.Notification({
+                    title: 'Unit is not available',
+                    message: 'The unit you are trying to view does not exist',
+                    level: 'error'
+                  })
+              );
+              self.fire('navigateTo', {url: self.nsRouter.url(
+                  {gui: '/service/' + serviceId})});
+            }
+          },
+          // If there is no service available then there definitely is no unit
+          // available so we create a notification and redirect the user to the
+          // environment view.
+          function() {
+            clearTimeout(handle);
+            self.db.notifications.add(
+                new Y.juju.models.Notification({
+                  title: 'Service is not available',
+                  message: 'The service you are trying to view does not exist',
+                  level: 'error'
+                })
+            );
+            self.fire('navigateTo', {url: self.nsRouter.url({gui: '/'})});
+          });
     },
 
     /**
@@ -526,22 +565,43 @@
      * @private
      */
     _buildServiceView: function(req, viewName) {
-      var service = this.db.services.getById(req.params.id);
-      this.showView(viewName, {
-        model: service,
-        db: this.db,
-        env: this.env,
-        landscape: this.landscape,
-        getModelURL: Y.bind(this.getModelURL, this),
-        nsRouter: this.nsRouter,
-        querystring: req.query
-      }, {}, function(view) {
-        // If the view contains a method call fitToWindow,
-        // we will execute it after getting the view rendered.
-        if (view.fitToWindow) {
-          view.fitToWindow();
-        }
-      });
+      var self = this,
+          options = {
+            db: this.db,
+            env: this.env,
+            landscape: this.landscape,
+            getModelURL: Y.bind(self.getModelURL, this),
+            nsRouter: this.nsRouter,
+            querystring: req.query
+          };
+      // Give the page 100 milliseconds to try and load the model
+      // before we show a loading screen.
+      var handle = setTimeout(function() {
+        self.showView(viewName, options);
+      }, 100);
+
+      var promise = this.modelController.getServiceWithCharm(req.params.id);
+      promise.then(
+          function(models) {
+            clearTimeout(handle);
+            options.model = models.service;
+            // Calling update allows showView to be called multiple times but
+            // only have its config updated not re-rendered.
+            self.showView(viewName, options, { update: true });
+          },
+          function() {
+            clearTimeout(handle);
+            self.showView(viewName, options, { update: true },
+                function(view) {
+                  // At this point the service view could be in loading state
+                  // or showing details but the service has become unavailable

[gary.poster, 2013/05/02 21:01:02]

Thanks for the clarification.  It can only be in a loading state, right?  There
should be no way we get here after the page has started showing details?  This
is only important insomuch as we want to be clear in our comment.

+                  // or was never available. This calls a method on the view
+                  // to redirect to the environment and to create a notification
+                  if (typeof view.noServiceAvailable === 'function') {
+                    view.noServiceAvailable();
+                  }
+                });
+          });
     },
 
     /**
@@ -785,18 +845,21 @@
     },
 
     /**
-       Determine if the browser should be visible or not.
+       Determine if the browser or environment should be rendered or not.
 
        When hitting internal :gui: views, the browser needs to disappear
        entirely from the UX for users. However, when we pop back it needs to
        appear back in the previous state.
 
-       @method checkShowBrowser
+       The environment only needs to render when another full page view isn't
+       visible.
+
+       @method toggleStaticViews
        @param {Request} req current request object.
        @param {Response} res current response object.
        @param {function} next callable for the next route in the chain.
      */
-    checkShowBrowser: function(req, res, next) {
+    toggleStaticViews: function(req, res, next) {
       var url = req.url,
           match = /(logout|:gui:\/(charms|service|unit))/;

[bcsaller, 2013/05/02 21:25:46]

I'd almost rather the routes contained metadata, like 
allowBrowser: false than trying to capture it here. The problem with this is we
work to make routes reversible from models and not staticly coded into the app.
This is a DRY violation in that we encode app urls in yet another place.

Notes for the future I guess.

       var subapps = this.get('subApps');
@@ -805,8 +868,13 @@
         var charmstore = subapps.charmstore;
         if (url.match(match)) {
           charmstore.hidden = true;
+          // XXX At some point in the near future we will add the ability to
+          // route on root namespaced paths and this check will no longer
+          // be needed
+          this.renderEnvironment = false;
         } else {
           charmstore.hidden = false;
+          this.renderEnvironment = true;
         }
         charmstore.updateVisible();
       }
@@ -827,6 +895,9 @@
      * @method show_environment
      */
     show_environment: function(req, res, next) {
+      if (!this.renderEnvironment) {
+        next(); return;

[bcsaller, 2013/05/02 21:25:46]

newline

+      }
       var self = this,
           view = this.getViewInfo('environment'),
           options = {
@@ -962,9 +1033,8 @@
           // Called on each request.
           { path: '*', callbacks: 'check_user_credentials'},
           { path: '*', callbacks: 'show_notifications_view'},
-          // Root.
+          { path: '*', callbacks: 'toggleStaticViews'},
           { path: '*', callbacks: 'show_environment'},
-          { path: '*', callbacks: 'checkShowBrowser'},
           // Charms.
           { path: '/charms/',
             callbacks: 'show_charm_collection',