Merge lp://staging/~gary/juju-gui/nameCheck into lp://staging/juju-gui/experimental

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'});
+ in...

LGTM with a small comment request.
QA OK
Thanks for this fix!

https://codereview.appspot.com/22500043/diff/1/app/views/viewlets/inspector-header.js
File app/views/viewlets/inspector-header.js (right):

https://codereview.appspot.com/22500043/diff/1/app/views/viewlets/inspector-header.js#newcode59
app/views/viewlets/inspector-header.js:59: var name =
pojoModel.displayName.match(/^\(([^)]*)\)$/)[1];
I'd love it if there was a quick comment describing what this regex
looks for to make the code easier to scan.

https://codereview.appspot.com/22500043/diff/1/test/test_ghost_inspector.js
File test/test_ghost_inspector.js (right):

https://codereview.appspot.com/22500043/diff/1/test/test_ghost_inspector.js#newcode50
test/test_ghost_inspector.js:50: env.connect();
Ahh the shibboleth of writing environment tests.....you may proceed.

https://codereview.appspot.com/22500043/

*** Submitted:

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.

R=jeff.pihach
CC=
https://codereview.appspot.com/22500043

https://codereview.appspot.com/22500043/

On 2013/11/06 21:20:50, jeff.pihach wrote:
...

https://codereview.appspot.com/22500043/diff/1/app/views/viewlets/inspector-header.js#newcode59
> app/views/viewlets/inspector-header.js:59: var name =
> pojoModel.displayName.match(/^\(([^)]*)\)$/)[1];
> I'd love it if there was a quick comment describing what this regex
looks for to
> make the code easier to scan.

You want comments? I'm your man! Done. :-)

Thanks for the review!

https://codereview.appspot.com/22500043/