Merge lp:~bac/juju-gui/fix-goof into lp:juju-gui/experimental

[Brad Crittenden]

Fix bug in charm load handler.

https://codereview.appspot.com/8364044/

[Brad Crittenden (bac)]

Reviewers: mp+157197_code.launchpad.net,

Message:
Please take a look.

Description:
Fix bug in charm load handler.

https://code.launchpad.net/~bac/juju-gui/fix-goof/+merge/157197

(do not edit description out of merge proposal)

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

Affected files:
   A [revision details]
   M app/store/endpoints.js
   M test/test_endpoints.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_endpoints.js
=== modified file 'test/test_endpoints.js'
--- test/test_endpoints.js 2013-04-04 13:28:35 +0000
+++ test/test_endpoints.js 2013-04-04 18:18:21 +0000
@@ -292,7 +292,9 @@
                                 'juju-models',
                                 'juju-tests-utils',
                                 'juju-endpoints-controller',
- 'juju-controllers'],
+ 'juju-controllers',
+ 'juju-charm-store',
+ 'datasource-local'],
      function(Y) {
        juju = Y.namespace('juju');
        utils = Y.namespace('juju-tests.utils');
@@ -415,4 +417,40 @@
      app.db.charms.getById(charm_id).destroy();
    });

+ it('should add the service to the endpoints map when the charm is
loaded',
+ function(done) {
+ var service_name = 'wordpress';
+ var charm_id = 'cs:precise/wordpress-2';
+ app.db.charms.add({id: charm_id});
+ controller.endpointsMap.should.eql({});
+ var charm = app.db.charms.getById(charm_id);
+ var data = [
+ { responseText: Y.JSON.stringify(
+ { summary: 'wowza', subordinate: true, store_revision: 7 })}];
+ var charmStore = new juju.CharmStore({
+ datasource: new Y.DataSource.Local({source: data})});
+
+ app.db.services.add({
+ id: service_name,
+ pending: false,
+ charm: charm_id});
+
+ charm.load(charmStore, function(err, data) {
+ if (err) { assert.fail('should succeed!'); }
+ assert(charm.loaded);
+ charm.get('summary').should.equal('wowza');
+
+ controller.endpointsMap.should.eql({
+ 'wordpress': {
+ provides: [],
+ requires: []
+ }});
+
+ done();
+ });
+
+
+ }
+ );
+
  });

Index: app/store/endpoints.js
=== modified file 'app/store/endpoints.js'
--- app/store/endpoints.js 2013-04-04 14:21:31 +0000
+++ app/store/endpoints.js 2013-04-04 18:18:21 +0000
@@ -111,7 +111,7 @@
          setupCharmOnceLoad: function(charm, svcName) {
            charm.once('load', Y.bind(function(svcName, evt) {
              this.addServiceToEndpointsMap(svcName, evt.currentTarget);
- }, null, svcName));
+ }, this, svcName));
          },

          /**
@@ -133,7 +133,7 @@
              charm = db.charms.add({id: charm_id})
                .load(env,
                  // If views are bound to the charm mode...

[Jeff Pihach (hatch)]

LGTM with comment below. Thanks for the quick fix!

https://codereview.appspot.com/8364044/diff/1/test/test_endpoints.js
File test/test_endpoints.js (right):

https://codereview.appspot.com/8364044/diff/1/test/test_endpoints.js#newcode430
test/test_endpoints.js:430: var charmStore = new juju.CharmStore({
charmStore needs to be destroyed at the end of the test.

https://codereview.appspot.com/8364044/

[lp:~bac/juju-gui/fix-goof]

498. By Brad Crittenden on 2013-04-04

Destroy object at end of test

[Richard Harding (rharding)]

LGTM with Jeff's cleanup comment.

https://codereview.appspot.com/8364044/

[Brad Crittenden (bac)]

*** Submitted:

Fix bug in charm load handler.

R=jeff.pihach, rharding
CC=
https://codereview.appspot.com/8364044

https://codereview.appspot.com/8364044/

=== modified file 'app/store/endpoints.js'
--- app/store/endpoints.js	2013-04-04 14:21:31 +0000
+++ app/store/endpoints.js	2013-04-04 18:45:25 +0000
@@ -111,7 +111,7 @@
         setupCharmOnceLoad: function(charm, svcName) {
           charm.once('load', Y.bind(function(svcName, evt) {
             this.addServiceToEndpointsMap(svcName, evt.currentTarget);
-          }, null, svcName));
+          }, this, svcName));
         },
 
         /**
@@ -133,7 +133,7 @@
             charm = db.charms.add({id: charm_id})
               .load(env,
                 // If views are bound to the charm model, firing "update" is
-                    // unnecessary, and potentially even mildly harmful.
+                // unnecessary, and potentially even mildly harmful.
                 function(err, result) { db.fire('update'); });
           }
 

=== modified file 'test/test_endpoints.js'
--- test/test_endpoints.js	2013-04-04 14:45:42 +0000
+++ test/test_endpoints.js	2013-04-04 18:45:25 +0000
@@ -292,7 +292,9 @@
                                'juju-models',
                                'juju-tests-utils',
                                'juju-endpoints-controller',
-                               'juju-controllers'],
+                               'juju-controllers',
+                               'juju-charm-store',
+                               'datasource-local'],
     function(Y) {
       juju = Y.namespace('juju');
       utils = Y.namespace('juju-tests.utils');
@@ -415,4 +417,41 @@
     app.db.charms.getById(charm_id).destroy();
   });
 
+  it('should add the service to the endpoints map when the charm is loaded',
+     function(done) {
+       var service_name = 'wordpress';
+       var charm_id = 'cs:precise/wordpress-2';
+       app.db.charms.add({id: charm_id});
+       controller.endpointsMap.should.eql({});
+       var charm = app.db.charms.getById(charm_id);
+       var data = [
+         { responseText: Y.JSON.stringify(
+         { summary: 'wowza', subordinate: true, store_revision: 7 })}];
+       var charmStore = new juju.CharmStore({
+         datasource: new Y.DataSource.Local({source: data})});
+
+       app.db.services.add({
+         id: service_name,
+         pending: false,
+         charm: charm_id});
+
+       charm.load(charmStore, function(err, data) {
+         if (err) { assert.fail('should succeed!'); }
+         assert(charm.loaded);
+         charm.get('summary').should.equal('wowza');
+
+         controller.endpointsMap.should.eql({
+           'wordpress': {
+             provides: [],
+             requires: []
+           }});
+
+         charmStore.destroy();
+         done();
+       });
+
+
+     }
+  );
+
 });