Merge lp:~huwshimi/juju-gui/bundle-charm-list-updates into lp:juju-gui/experimental

[Huw Wilkins]

Bundle promise removal and service updates

Removed promises (merged from Rick)

Visual updates:
1) summary tab: delete charm stuff
2) change charms tab name to services
3) add service name to top of config info
4) sort services by charm name and then by service name
5) Add this text to top of Services tab: "These are the services that the bundle will deploy. Non-default configuration values are listed."
6) if charm is subordinate, do not show number of units; if charm is not subordinate and does not list number of units, show "1"

https://codereview.appspot.com/14920057/

[Huw Wilkins (huwshimi)]

Reviewers: mp+192264_code.launchpad.net,

Message:
Please take a look.

Description:
Bundle promise removal and service updates

Removed promises (merged from Rick)

Visual updates:
1) summary tab: delete charm stuff
2) change charms tab name to services
3) add service name to top of config info
4) sort services by charm name and then by service name
5) Add this text to top of Services tab: "These are the services that
the bundle will deploy. Non-default configuration values are listed."
6) if charm is subordinate, do not show number of units; if charm is not
subordinate and does not list number of units, show "1"

https://code.launchpad.net/~huwshimi/juju-gui/bundle-charm-list-updates/+merge/192264

(do not edit description out of merge proposal)

Please review this at https://codereview.appspot.com/14920057/

Affected files (+236, -213 lines):
   [revision details]
   app/subapps/browser/views/bundle.js
   app/templates/bundle.handlebars
   lib/views/browser/bundle-panel.less
   test/test_bundle_details_view.js

[lp:~huwshimi/juju-gui/bundle-charm-list-updates]

1150. By Huw Wilkins on 2013-10-22

Merged trunk.

[Huw Wilkins (huwshimi)]

Please take a look.

https://codereview.appspot.com/14920057/

[Gary Poster (gary)]

I'd like to talk through my comments before giving a blessing, but it
looks very close. Thank you!

QA good. I think I want to try and get rid of the Summary tab, but that
can be a separate branch, maybe based on some work Benji and Brad are
doing.

https://codereview.appspot.com/14920057/diff/30001/app/subapps/browser/views/bundle.js
File app/subapps/browser/views/bundle.js (right):

https://codereview.appspot.com/14920057/diff/30001/app/subapps/browser/views/bundle.js#newcode120
app/subapps/browser/views/bundle.js:120:
_model is a pretty confusing name, IMO. Why not simply service._charm?
Also, this is using an underline effectively as a namespace tool, which
I don't love.

https://codereview.appspot.com/14920057/diff/30001/app/subapps/browser/views/bundle.js#newcode138
app/subapps/browser/views/bundle.js:138:
The 'entity' is a bundle here, right? It is called 'entity' because we
are sharing code with a charm view as well, right, so in other files
'entity' is a charm? If so, please add a comment explaining this (at
least the fact that this.get('entity') is a bundle).

https://codereview.appspot.com/14920057/diff/30001/app/subapps/browser/views/bundle.js#newcode142
app/subapps/browser/views/bundle.js:142:
I'd personally feel that this would be an easier-to-read data structure
if you changed this approach. For instance, one example would be to
build a new services mapping that was like this:
newServicesMappingExample[key] = {charm: new Y.juju.models.Charm(charm),
data: service}.

https://codereview.appspot.com/14920057/diff/30001/app/subapps/browser/views/bundle.js#newcode194
app/subapps/browser/views/bundle.js:194:
Per Jeff's comments
(http://jujugui.wordpress.com/2013/10/11/javascript-promise-error-handling-tricks/),
this should be in a separate .then(null, function(error) {...}) block so
that you can also catch and report errors from the success function.

https://codereview.appspot.com/14920057/

[Richard Harding (rharding)]

O>

https://codereview.appspot.com/14920057/diff/30001/app/subapps/browser/views/bundle.js#newcode120
> app/subapps/browser/views/bundle.js:120:
> _model is a pretty confusing name, IMO. Why not simply
service._charm? Also,
> this is using an underline effectively as a namespace tool, which I
don't love.

We have to be careful not to collide with keys that could occur in the
service block in the deployer file section. Using the _ is a
"collision-avoider". What would you prefer as an alternative?

https://codereview.appspot.com/14920057/diff/30001/app/subapps/browser/views/bundle.js#newcode138
> app/subapps/browser/views/bundle.js:138:
> The 'entity' is a bundle here, right? It is called 'entity' because
we are
> sharing code with a charm view as well, right, so in other files
'entity' is a
> charm? If so, please add a comment explaining this (at least the fact
that
> this.get('entity') is a bundle).

Yep, the details shares a lot of common code in entity-base.js which was
refactored out to share between charm and bundle details views.

https://codereview.appspot.com/14920057/

[Gary Poster (gary)]

On 2013/10/23 13:57:52, rharding wrote:
> O>
> >

https://codereview.appspot.com/14920057/diff/30001/app/subapps/browser/views/bundle.js#newcode120
> > app/subapps/browser/views/bundle.js:120:
> > _model is a pretty confusing name, IMO. Why not simply
service._charm? Also,
> > this is using an underline effectively as a namespace tool, which I
don't
> love.

> We have to be careful not to collide with keys that could occur in the
service
> block in the deployer file section. Using the _ is a
"collision-avoider".

Right, that's what I meant by namespace

> What
> would you prefer as an alternative?

See my comment starting with "I'd personally feel that this would be an
easier-to-read data structure..."

> >

https://codereview.appspot.com/14920057/diff/30001/app/subapps/browser/views/bundle.js#newcode138
> > app/subapps/browser/views/bundle.js:138:
> > The 'entity' is a bundle here, right? It is called 'entity' because
we are
> > sharing code with a charm view as well, right, so in other files
'entity' is a
> > charm? If so, please add a comment explaining this (at least the
fact that
> > this.get('entity') is a bundle).

> Yep, the details shares a lot of common code in entity-base.js which
was
> refactored out to share between charm and bundle details views.

Cool.

Thank you.

https://codereview.appspot.com/14920057/

[lp:~huwshimi/juju-gui/bundle-charm-list-updates]

1151. By Huw Wilkins on 2013-10-23

Review changes.

1152. By Huw Wilkins on 2013-10-23

Merged trunk.

[Huw Wilkins (huwshimi)]

Please take a look.

https://codereview.appspot.com/14920057/

[Jeff Pihach (hatch)]

LGTM after some small changes below - Thanks for putting in the time on
this one!

https://codereview.appspot.com/14920057/diff/170001/app/subapps/browser/views/bundle.js
File app/subapps/browser/views/bundle.js (right):

https://codereview.appspot.com/14920057/diff/170001/app/subapps/browser/views/bundle.js#newcode139
app/subapps/browser/views/bundle.js:139: var attrs =
this.get('entity').getAttrs();
You have all of this information when you call this function so as a
micro performance improvement you could pass only the required data in
so that this is a utility method instead of just a private method which
isn't really a private method.

https://codereview.appspot.com/14920057/diff/170001/app/subapps/browser/views/bundle.js#newcode165
app/subapps/browser/views/bundle.js:165: var templateData =
Y.merge(bundleData);
If bundle is a Y.Model then this should be returning a new object so
this merge is not required. Then you can use bundleData below instead of
templateData

https://codereview.appspot.com/14920057/diff/170001/app/subapps/browser/views/bundle.js#newcode198
app/subapps/browser/views/bundle.js:198: // Fired event to test the
topology is rendered
Fire event to listen to during the tests so that we know when it's
rendered

https://codereview.appspot.com/14920057/diff/170001/app/subapps/browser/views/bundle.js#newcode221
app/subapps/browser/views/bundle.js:221: this.set('rendered', true);
Now that render is synchronous this is no longer required and the checks
can be removed from the tests.

https://codereview.appspot.com/14920057/

[lp:~huwshimi/juju-gui/bundle-charm-list-updates]

1153. By Huw Wilkins on 2013-10-24

Review fixes.

[Huw Wilkins (huwshimi)]

Please take a look.

https://codereview.appspot.com/14920057/

[Huw Wilkins (huwshimi)]

*** Submitted:

Bundle promise removal and service updates

Removed promises (merged from Rick)

Visual updates:
1) summary tab: delete charm stuff
2) change charms tab name to services
3) add service name to top of config info
4) sort services by charm name and then by service name
5) Add this text to top of Services tab: "These are the services that
the bundle will deploy. Non-default configuration values are listed."
6) if charm is subordinate, do not show number of units; if charm is not
subordinate and does not list number of units, show "1"

R=gary.poster, rharding, jeff.pihach
CC=
https://codereview.appspot.com/14920057

https://codereview.appspot.com/14920057/

=== modified file 'app/subapps/browser/views/bundle.js'
--- app/subapps/browser/views/bundle.js	2013-10-17 16:55:19 +0000
+++ app/subapps/browser/views/bundle.js	2013-10-24 00:19:58 +0000
@@ -77,34 +77,6 @@
     },
 
     /**
-      Fetches and prepares the data for the bundle details page rendering.
-
-      @method _fetchData
-    */
-    _fetchData: function() {
-      var self = this;
-
-      return new Y.Promise(function(resolve, reject) {
-        var entity = self.get('entity');
-        // An entity here is a fully populated charm/bundle model so
-        // it's entirely possible that we have an id to load but
-        // no model has been populated yet.
-        if (entity) {
-          resolve(entity);
-        } else {
-          self.get('store').bundle(self.get('entityId'), {
-            'success': function(data) {
-              var bundle = new models.Bundle(data);
-              self.set('entity', bundle);
-              resolve(bundle);
-            },
-            'failure': reject
-          }, self);
-        }
-      });
-    },
-
-    /**
       Sends the bundle data to the local fakebackend to
       import and then returns a promise when complete.
 
@@ -140,36 +112,66 @@
       Render the list of charms in the bundle.
 
       @method _renderCharmListing
+      @param {Object} services the services in the bundle.
 
      */
-    _renderCharmListing: function() {
-      var attrs = this.get('entity').getAttrs();
-      Y.Object.each(attrs.charm_metadata, function(charm, key) {
-        var charmModel = new Y.juju.models.Charm(charm);
-        charm = charmModel.getAttrs();
+    _renderCharmListing: function(services) {
+      services.forEach(function(service) {
+        var charm = service.charmModel.getAttrs();
         charm.size = 'tiny';
         charm.isDraggable = false;
         var token = new widgets.browser.Token(charm);
-        var node = Y.one('[data-config="' + key + '"]');
+        var node = Y.one('[data-config="' + service.origService.name + '"]');
         token.render(node);
         this._cleanup.tokens.push(token);
       }, this);
     },
 
     /**
+      Build and order a list of charms.
+
+      @method _buildCharmList
+      @param {Object} the bundle entity attrs.
+      @return {Array} the ordered list of charms in the bundle.
+
+     */
+    _buildCharmList: function(bundleData) {
+      var services = [];
+      Y.Object.each(bundleData.services, function(service, key) {
+        var charm = bundleData.charm_metadata[key];
+        services.push({
+          origService: {
+            name: key,
+            data: service
+          },
+          charmModel: new Y.juju.models.Charm(charm)
+        });
+      }, this);
+      services.sort(function(a, b) {
+        return a.charmModel.get('name') > b.charmModel.get('name') ? 1 : -1;
+      });
+      return services;
+    },
+
+    /**
       Renders the bundle view template into the DOM.
 
       @method _renderBundleView
     */
     _renderBundleView: function() {
-      var entity = this.get('entity');
-      var attrs = entity.getAttrs();
-      attrs.charmIcons = utils.charmIconParser(attrs.charm_metadata);
+      var bundle = this.get('entity');
+      var bundleData = bundle.getAttrs();
+      // Copy the bundle for use in the template so we can modify the content
+      // without munipulating the entity.
+      var templateData = Y.merge(bundleData);
+      templateData.charmIcons = utils.charmIconParser(
+          templateData.charm_metadata);
       // Remove the svg files from the file list
-      attrs.files = attrs.files.filter(function(fileName) {
+      templateData.files = templateData.files.filter(function(fileName) {
         return !/\.svg$/.test(fileName);
       });
-      var content = this.template(attrs);
+      templateData.services = this._buildCharmList(bundleData);
+      var content = this.template(templateData);
       var node = this.get('container').setHTML(content);
       var renderTo = this.get('renderTo');
       var options = {size: [480, 360]};
@@ -179,15 +181,27 @@
       // remove the flag in the test(test_bundle_details_view.js)
       // when this flag is no longer needed.
       if (window.flags && window.flags.strictBundle) {
-        showTopo = this._positionAnnotationsIncluded(attrs.data.services);
+        showTopo = this._positionAnnotationsIncluded(
+            bundleData.data.services);
       }
       if (showTopo) {
-        this.environment = new views.BundleTopology(Y.mix({
-          db: this.fakebackend.db,
-          container: node.one('#bws-bundle'), // Id because of Y.TabView
-          store: this.get('store')
-        }, options));
-        this.environment.render();
+        // Setup the fake backend to create topology to display the canvas-like
+        // rendering of the bundle.
+        this._setupLocalFakebackend();
+        var self = this;
+        this._parseData(bundle).then(function() {
+          self.environment = new views.BundleTopology(Y.mix({
+            db: self.fakebackend.db,
+            container: node.one('#bws-bundle'), // Id because of Y.TabView
+            store: self.get('store')
+          }, options));
+          self.environment.render();
+          // Fire event to listen to during the tests so that we know when
+          // it's rendered.
+          self.fire('topologyRendered');
+        }).then(null, function(error) {
+          console.error(error.message, error);
+        });
       } else {
         // Remove the bundle tab so it doesn't get PE'd when
         // we instantiate the tabview.
@@ -205,9 +219,7 @@
       }
       this._dispatchTabEvents(this.tabview);
       this._showActiveTab();
-      this._renderCharmListing();
-
-      this.set('rendered', true);
+      this._renderCharmListing(templateData.services);
     },
 
     /**
@@ -266,11 +278,20 @@
     */
     render: function() {
       this.showIndicator(this.get('renderTo'));
-      this._setupLocalFakebackend();
-      this._fetchData().
-          then(this._parseData.bind(this)).
-          then(this._renderBundleView.bind(this)).
-          then(null, this.apiFailure.bind(this));
+      var entity = this.get('entity');
+      if (entity) {
+        this._renderBundleView();
+      } else {
+        this.get('store').bundle(
+            this.get('entityId'), {
+              'success': function(data) {
+                this.set('entity', new models.Bundle(data));
+                this._renderBundleView();
+              },
+              'failure': this.apiFailure
+            },
+            this);
+      }
     }
 
   }, {
@@ -291,6 +312,7 @@
 }, '', {
   requires: [
     'browser-overlay-indicator',
+    'juju-bundle-models',
     'juju-charm-models',
     'juju-view-utils',
     'view',

=== modified file 'app/templates/bundle.handlebars'
--- app/templates/bundle.handlebars	2013-10-17 06:12:35 +0000
+++ app/templates/bundle.handlebars	2013-10-24 00:19:58 +0000
@@ -39,7 +39,7 @@
                 <li><a href="#bws-bundle" class="bundle">Bundle</a></li>
                 <li><a href="#bws-summary" class="summary">Summary</a></li>
                 <li><a href="#bws-readme" class="readme">Readme</a></li>
-                <li><a href="#bws-charms" class="charms">Charms</a></li>
+                <li><a href="#bws-services" class="services">Services</a></li>
                 <li><a href="#bws-code" class="code">Code</a></li>
             </ul>
             <div>
@@ -54,8 +54,6 @@
                             <p>No description</p>
                         {{/if}}
                     </div>
-                    <h2>Charms within this bundle</h2>
-                    [Charm list to go here]
                     <div class="changelog">
                         <h3 class="section-title">
                             {{#if recent_commits}}
@@ -119,7 +117,11 @@
                     </div>
                 </div>
                 <div id="bws-readme"></div>
-                <div id="bws-charms">
+                <div id="bws-services">
+                    <p>
+                        These are the services that the bundle will deploy.
+                        Non-default configuration values are listed.
+                    </p>
                     <div class="yui3-g">
                         <div class="interfaces-heading yui3-u-1-2">
                             <h3>Charm</h3>
@@ -129,21 +131,31 @@
                         </div>
                     </div>
                     <ul class="interface-list">
-                        {{#arrayObject services}}
+                        {{#each services}}
                             <li class="interface-row yui3-g">
-                                <div class="yui3-u-1-2" data-config="{{ key }}"></div>
+                                <div class="yui3-u-1-2" data-config="{{ origService.name }}"></div>
                                 <div class="yui3-u-1-2 charm-config">
                                     <ul>
-                                        <li>Number of units: {{ value.num_units }}</li>
-                                        {{#if value.config}}
-                                            {{#arrayObject value.options}}
+                                        <li>Service name: {{ origService.name }}</li>
+                                        {{#unless charmModel.is_subordinate}}
+                                            <li>
+                                                Number of units:
+                                                {{#if num_units }}
+                                                    {{ num_units }}
+                                                {{else}}
+                                                    1
+                                                {{/if}}
+                                            </li>
+                                        {{/unless}}
+                                        {{#if origService.options}}
+                                            {{#arrayObject origService.options}}
                                                 <li>{{ key }}: {{ value }}</li>
                                             {{/arrayObject}}
                                         {{/if}}
                                     </ul>
                                 </div>
                             </li>
-                        {{/arrayObject}}
+                        {{/each}}
                     </ul>
                 </div>
                 <div id="bws-code">

=== modified file 'lib/views/browser/bundle-panel.less'
--- lib/views/browser/bundle-panel.less	2013-10-10 00:05:02 +0000
+++ lib/views/browser/bundle-panel.less	2013-10-24 00:19:58 +0000
@@ -50,4 +50,7 @@
     .topology-canvas {
         .crosshatch-background;
     }
+    #bws-services {
+        padding-top: 50px;
+    }
 }

=== modified file 'test/test_bundle_details_view.js'
--- test/test_bundle_details_view.js	2013-10-17 16:55:19 +0000
+++ test/test_bundle_details_view.js	2013-10-24 00:19:58 +0000
@@ -19,11 +19,12 @@
 'use strict';
 
 describe('Browser bundle detail view', function() {
-  var Y, utils, data, container, origData, view, fakestore, browser;
+  var Y, cleanUp, utils, data, container, view, fakestore, browser, models;
 
   before(function(done) {
     Y = YUI(GlobalConfig).use(
         'view',
+        'juju-bundle-models',
         'juju-env-fakebackend',
         'juju-view-bundle',
         'subapp-browser', // required for handlebars helpers
@@ -36,14 +37,13 @@
         'event-simulate',
         'node-event-simulate',
         function(Y) {
+          models = Y.namespace('juju.models');
           utils = Y.namespace('juju-tests.utils');
-          var sampleData = Y.io('data/browserbundle.json', {sync: true});
-
-          origData = Y.JSON.parse(sampleData.responseText);
+          data = utils.loadFixture('data/browserbundle.json', true);
 
           // Required to register the handlebars helpers
           browser = new Y.juju.subapps.Browser({
-            store: modifyFakeStore()
+            store: utils.makeFakeStore()
           });
 
           done();
@@ -55,19 +55,20 @@
     view._setupLocalFakebackend = function() {
       this.fakebackend = utils.makeFakeBackend();
     };
+    cleanUp = utils.stubCharmIconPath();
   });
 
   afterEach(function() {
     container.remove().destroy(true);
     view.destroy();
+    cleanUp();
   });
 
   function generateBundleView(options) {
-    data = Y.clone(origData);
     container = utils.makeContainer();
     container.append('<div class="bws-view-data"></div>');
     var defaults = {
-      store: modifyFakeStore(),
+      store: utils.makeFakeStore(3),
       db: {},
       entityId: data.id,
       renderTo: container
@@ -77,85 +78,60 @@
     return view;
   }
 
-  function modifyFakeStore(options) {
-    var defaults = {
-      bundle: function(id, callbacks) {
-        callbacks.success(data);
-      },
-      iconpath: function(id) {
-        return 'foo';
-      }
-    };
-
-    var fakebackend = Y.mix(defaults, options, true);
-    if (view) {
-      view.set('store', fakebackend);
-    }
-    return fakebackend;
-  }
-
   it('can be instantiated', function() {
     assert.equal(view instanceof Y.juju.browser.views.BrowserBundleView, true);
   });
 
-  it('displays the bundle data in a tabview', function(done) {
-    view.after('renderedChange', function(e) {
-      assert.isNotNull(container.one('.yui3-tabview'));
-      done();
-    });
+  it('displays the bundle data in a tabview', function() {
+    view.set('entity', new models.Bundle(data));
     view.render();
+    assert.isNotNull(container.one('.yui3-tabview'));
   });
 
   it('fetches the readme when requested', function(done) {
-    modifyFakeStore({
-      file: function(id, filename, entityType, callbacks) {
-        assert.equal(entityType, 'bundle');
-        assert.equal(id, data.id);
-        assert.equal(filename, 'README');
-        assert.isFunction(callbacks.success);
-        assert.isFunction(callbacks.failure);
-        callbacks.success.call(view, '<div id="testit"></div>');
-        assert.isNotNull(container.one('#testit'));
-        done();
-      }
-    });
-    view.after('renderedChange', function(e) {
-      container.one('a.readme').simulate('click');
-    });
+    var fakeStore = utils.makeFakeStore();
+    fakeStore.file = function(id, filename, entityType, callbacks) {
+      assert.equal(entityType, 'bundle');
+      assert.equal(id, data.id);
+      assert.equal(filename, 'README');
+      assert.isFunction(callbacks.success);
+      assert.isFunction(callbacks.failure);
+      callbacks.success.call(view, '<div id="testit"></div>');
+      assert.isNotNull(container.one('#testit'));
+      done();
+    };
+    view.set('store', fakeStore);
+    view.set('entity', new models.Bundle(data));
     view.render();
+    container.one('a.readme').simulate('click');
   });
 
   it('fetches a source file when requested', function(done) {
-    modifyFakeStore({
-      file: function(id, filename, entityType, callbacks) {
-        assert.equal(entityType, 'bundle');
-        assert.equal(id, data.id);
-        assert.equal(filename, 'bundles.yaml');
-        assert.isFunction(callbacks.success);
-        assert.isFunction(callbacks.failure);
-        callbacks.success.call(view, '<div id="testit"></div>');
-        assert.isNotNull(container.one('#testit'));
-        done();
-      }
-    });
-    view.after('renderedChange', function(e) {
-      container.one('a.code').simulate('click');
-      var codeNode = container.one('#bws-code');
-      codeNode.all('select option').item(2).set('selected', 'selected');
-      codeNode.one('select').simulate('change');
-    });
-
-    view.render();
-  });
-
-  it('renders the proper charm icons into the header', function(done) {
-    view.after('renderedChange', function(e) {
-      assert.equal(
-          container.one('.header .details .charms').all('img').size(),
-          4);
+    var fakeStore = utils.makeFakeStore();
+    fakeStore.file = function(id, filename, entityType, callbacks) {
+      assert.equal(entityType, 'bundle');
+      assert.equal(id, data.id);
+      assert.equal(filename, 'bundles.yaml');
+      assert.isFunction(callbacks.success);
+      assert.isFunction(callbacks.failure);
+      callbacks.success.call(view, '<div id="testit"></div>');
+      assert.isNotNull(container.one('#testit'));
       done();
-    });
-    view.render();
+    };
+    view.set('store', fakeStore);
+    view.set('entity', new models.Bundle(data));
+    view.render();
+    container.one('a.code').simulate('click');
+    var codeNode = container.one('#bws-code');
+    codeNode.all('select option').item(2).set('selected', 'selected');
+    codeNode.one('select').simulate('change');
+  });
+
+  it('renders the proper charm icons into the header', function() {
+    view.set('entity', new models.Bundle(data));
+    view.render();
+    assert.equal(
+        container.one('.header .details .charms').all('img').size(), 4);
   });
 
   it('deploys a bundle when \'add\' button is clicked', function(done) {
@@ -165,14 +141,13 @@
       assert.isObject(data);
       done();
     });
-    view.after('renderedChange', function(e) {
-      container.one('.bundle .add').simulate('click');
-    });
+    view.set('entity', new models.Bundle(data));
     view.render();
+    container.one('.bundle .add').simulate('click');
   });
 
   it('fails gracefully if services don\'t provide xy annotations',
-     function(done) {
+     function() {
        window.flags = { strictBundle: true };
        view._parseData = function() {
          return new Y.Promise(function(resolve) { resolve(); });
@@ -195,15 +170,13 @@
               }
             };
           }});
-       view.after('renderedChange', function(e) {
-         assert.isNull(container.one('#bws-bundle'));
-         assert.isNull(container.one('a[href=#bws-bundle]'));
-         // Check that the charms tab is the landing tab
-         assert.equal(view.tabview.get('selection').get('index'), 2);
-         window.flags = {};
-         done();
-       });
+       view.set('entity', new models.Bundle(data));
        view.render();
+       assert.isNull(container.one('#bws-bundle'));
+       assert.isNull(container.one('a[href=#bws-bundle]'));
+       // Check that the charms tab is the landing tab
+       assert.equal(view.tabview.get('selection').get('index'), 2);
+       window.flags = {};
      });
 
   it('renders the bundle topology into the view', function(done) {
@@ -211,112 +184,120 @@
     view._parseData = function() {
       return new Y.Promise(function(resolve) { resolve(); });
     };
-    view.set('entity', {
-      getAttrs: function() {
-        return {
-          charm_metadata: {},
-          files: [],
-          data: {
-            services: {
-              foo: {
-                annotations: {
-                  'gui-x': '1',
-                  'gui-y': '2'
-                }
-              },
-              bar: {
-                annotations: {
-                  'gui-x': '3',
-                  'gui-y': '4'
-                }
-              }
+    var entity = {
+      charm_metadata: {
+        foo: {
+          id: 'precise/foo-9',
+          storeId: 'testid',
+          name: 'foo'
+        },
+        bar: {
+          id: 'precise/bar-10',
+          storeId: 'testid',
+          name: 'bar'
+        }
+      },
+      files: [],
+      data: {
+        services: {
+          foo: {
+            annotations: {
+              'gui-x': '1',
+              'gui-y': '2'
+            }
+          },
+          bar: {
+            annotations: {
+              'gui-x': '3',
+              'gui-y': '4'
             }
           }
-        };
-      }});
-    view.after('renderedChange', function(e) {
+        }
+      }
+    };
+    view.on('topologyRendered', function(e) {
       assert.isNotNull(container.one('.topology-canvas'));
       // Check that the bundle topology tab is the landing tab.
       assert.equal(view.tabview.get('selection').get('index'), 0);
       window.flags = {};
       done();
     });
-    view.render();
-  });
-
-  it('renders the charm list tab properly', function(done) {
-    view._parseData = function() {
-      return new Y.Promise(function(resolve) { resolve(); });
-    };
-    view.set('entity', {
-      getAttrs: function() {
-        return {
-          charm_metadata: {
-            foo: {
-              id: 'precise/foo-9',
-              storeId: 'testid',
-              name: 'foo'
-            },
-            bar: {
-              id: 'precise/bar-10',
-              storeId: 'testid',
-              name: 'bar'
-            }
-          },
-          files: [],
-          data: {
-            services: {
-              foo: {
-                annotations: {
-                  'gui-x': '1',
-                  'gui-y': '2'
-                }
-              },
-              bar: {
-                annotations: {
-                  'gui-x': '3',
-                  'gui-y': '4'
-                }
-              }
-            }
-          },
-          services: {
-            foo: {
-              annotations: {
-                'gui-x': '1',
-                'gui-y': '2'
-              }
-            },
-            bar: {
-              annotations: {
-                'gui-x': '3',
-                'gui-y': '4'
-              }
-            }
-          }
-        };
-      }});
-    view.after('renderedChange', function(e) {
-      var tab = container.one('#bws-charms');
-      assert.equal(tab.all('.token').size(), 2);
-      done();
-    });
-    view.render();
-  });
-
-  it('selects the proper tab when given one', function(done) {
-    view = generateBundleView({
-      activeTab: '#bws-charms'
-    });
-    view._parseData = function() {
-      return new Y.Promise(function(resolve) { resolve(); });
-    };
-    view.render();
-    view.after('renderedChange', function(e) {
-      var selected = view.get('container').one('.yui3-tab-selected a');
-      assert.equal(selected.getAttribute('href'), '#bws-charms');
-      done();
-    });
+    view.set('entity', new models.Bundle(entity));
+    view.render();
+  });
+
+  it('renders the charm list tab properly', function() {
+    // This is not under test. It's async and only causes trouble in other
+    // tests.
+    view._parseData = function() {
+      return new Y.Promise(function(resolve) { resolve(); });
+    };
+
+    var entity = {
+      charm_metadata: {
+        foo: {
+          id: 'precise/foo-9',
+          storeId: 'testid',
+          name: 'foo'
+        },
+        bar: {
+          id: 'precise/bar-10',
+          storeId: 'testid',
+          name: 'bar'
+        }
+      },
+      files: [],
+      data: {
+        services: {
+          foo: {
+            annotations: {
+              'gui-x': '1',
+              'gui-y': '2'
+            }
+          },
+          bar: {
+            annotations: {
+              'gui-x': '3',
+              'gui-y': '4'
+            }
+          }
+        }
+      },
+      services: {
+        foo: {
+          annotations: {
+            'gui-x': '1',
+            'gui-y': '2'
+          }
+        },
+        bar: {
+          annotations: {
+            'gui-x': '3',
+            'gui-y': '4'
+          }
+        }
+      }
+    };
+    view.set('entity', new models.Bundle(entity));
+    view.render();
+    var tab = container.one('#bws-services');
+    assert.equal(tab.all('.token').size(), 2);
+    var charmConfigNodes = tab.all('.charm-config');
+    assert.equal(
+        charmConfigNodes.item(0).one('li').get('text'), 'Service name: bar');
+    assert.equal(
+        charmConfigNodes.item(1).one('li').get('text'), 'Service name: foo');
+  });
+
+  it('selects the proper tab when given one', function() {
+    view.set('activeTab', '#bws-services');
+    view._parseData = function() {
+      return new Y.Promise(function(resolve) { resolve(); });
+    };
+    view.set('entity', new models.Bundle(data));
+    view.render();
+    var selected = view.get('container').one('.yui3-tab-selected a');
+    assert.equal(selected.getAttribute('href'), '#bws-services');
   });
 
 });