Code review comment for lp://staging/~gary/juju-gui/nameCheck

Reviewers: mp+194234_code.launchpad.net,

Message:
Please take a look.

Description:
Fix ghost inspector initial name validation

QA: deploy mediawiki with the default name. Make another ghost, and the
inspector should correctly show that the default name is invalid.

Thank you.

https://code.launchpad.net/~gary/juju-gui/nameCheck/+merge/194234

(do not edit description out of merge proposal)

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

Affected files (+60, -16 lines):
   A [revision details]
   M app/views/viewlets/inspector-header.js
   M test/test_ghost_inspector.js

Index: [revision details]
=== added file '[revision details]'
--- [revision details] 2012-01-01 00:00:00 +0000
+++ [revision details] 2012-01-01 00:00:00 +0000
@@ -0,0 +1,2 @@
+Old revision: <email address hidden>
+New revision: <email address hidden>

Index: test/test_ghost_inspector.js
=== modified file 'test/test_ghost_inspector.js'
--- test/test_ghost_inspector.js 2013-11-05 18:10:05 +0000
+++ test/test_ghost_inspector.js 2013-11-06 21:04:34 +0000
@@ -47,6 +47,7 @@
      conn = new utils.SocketStub();
      db = new models.Database();
      env = juju.newEnvironment({conn: conn});
+ env.connect();
    });

    afterEach(function(done) {
@@ -93,6 +94,52 @@
      return view.createServiceInspector(service, {databinding: {interval:
0}});
    };

+ describe('charm name validity', function() {
+ it('shows when a charm name is invalid initially', function() {
+ db.services.add({id: 'mediawiki', charm: 'cs:precise/mediawiki'});
+ inspector = setUpInspector();
+ var model = inspector.model;
+ var serviceNameInput = Y.one('input[name=service-name]');
+ assert.equal(model.get('displayName'), '(mediawiki)');
+ assert.isTrue(serviceNameInput.hasClass('invalid'));
+ assert.isFalse(serviceNameInput.hasClass('valid'));
+ });
+
+ it('shows when a charm name is valid initially', function() {
+ db.services.add({id: 'mediawiki42', charm: 'cs:precise/mediawiki'});
+ inspector = setUpInspector();
+ var model = inspector.model;
+ var serviceNameInput = Y.one('input[name=service-name]');
+ assert.equal(model.get('displayName'), '(mediawiki)');
+ assert.isFalse(serviceNameInput.hasClass('invalid'));
+ assert.isTrue(serviceNameInput.hasClass('valid'));
+ });
+
+ it('shows when a charm name becomes invalid', function() {
+ db.services.add({id: 'mediawiki42', charm: 'cs:precise/mediawiki'});
+ inspector = setUpInspector();
+ var serviceNameInput = Y.one('input[name=service-name]');
+ // This is usually fired by an event. The event simulation is
broken as
+ // of this writing, and we can do more of a unit test this way.
+ inspector.updateGhostName(
+ {newVal: 'mediawiki42', currentTarget: serviceNameInput});
+ assert.isTrue(serviceNameInput.hasClass('invalid'));
+ assert.isFalse(serviceNameInput.hasClass('valid'));
+ });
+
+ it('shows when a charm name becomes valid', function() {
+ db.services.add({id: 'mediawiki', charm: 'cs:precise/mediawiki'});
+ inspector = setUpInspector();
+ var serviceNameInput = Y.one('input[name=service-name]');
+ // This is usually fired by an event. The event simulation is
broken as
+ // of this writing, and we can do more of a unit test this way.
+ inspector.updateGhostName(
+ {newVal: 'mediawiki42', currentTarget: serviceNameInput});
+ assert.isFalse(serviceNameInput.hasClass('invalid'));
+ assert.isTrue(serviceNameInput.hasClass('valid'));
+ });
+ });
+
    it('updates the service name in the topology when changed in the
inspector',
        function(done) {
          // XXX (Jeff) YUI's simulate can't properly simulate focus or blur
in

Index: app/views/viewlets/inspector-header.js
=== modified file 'app/views/viewlets/inspector-header.js'
--- app/views/viewlets/inspector-header.js 2013-09-30 15:38:34 +0000
+++ app/views/viewlets/inspector-header.js 2013-11-06 21:04:34 +0000
@@ -48,25 +48,20 @@
      'render': function(model, viewContainerAttrs) {
        this.container = Y.Node.create(this.templateWrapper);
        var pojoModel = model.getAttrs();
- if (model instanceof models.Charm) {
- pojoModel.ghost = true;
- pojoModel.charmUrl = pojoModel.id;
- } else if (model instanceof models.Service) {
- pojoModel.charmUrl = pojoModel.charm;
- } else {
- throw 'Programmer error: unknown model type';
- }
+ pojoModel.charmUrl = pojoModel.charm;
        // Manually add the icon url for the charm since we don't have
access to
        // the browser handlebars helper at this location.
        pojoModel.icon =
viewContainerAttrs.store.iconpath(pojoModel.charmUrl);
-
- // Check if there is already a service using the default name to
- // trigger the name ux.
- if (utils.checkForExistingService(pojoModel.name,
- viewContainerAttrs.db)) {
- pojoModel.invalidName = 'invalid';
- } else {
- pojoModel.invalidName = 'valid';
+ if (pojoModel.pending) {
+ // Check if there is already a service using the default name to
+ // trigger the name ux.
+ // If the regex doesn't match, blow up. It should match.
+ var name = pojoModel.displayName.match(/^\(([^)]*)\)$/)[1];
+ if (utils.checkForExistingService(name, viewContainerAttrs.db)) {
+ pojoModel.invalidName = 'invalid';
+ } else {
+ pojoModel.invalidName = 'valid';
+ }
        }

        this.container.setHTML(this.template(pojoModel));