Merge lp:~bac/juju-gui/fix-dnd-bundles-wrt-id into lp:juju-gui/experimental

[Brad Crittenden]

Send bundle id when d'n'd from sidebar.

This path was initially missed. The Token had to be fixed to include the
ID in the drag data.

Note, for selfish reasons, test/utils.js loadFixtures was updated to retry on
failures. The reason for the occassional 404 in this function when run by
lbox is still unknown. Hopefully the dumb retry will help.

QA Instructions:

# Use juju-quickstart trunk
% cd projects/juju-quickstart
% bzr pull
% make
# Deploy on EC2
% juju switch ec2
% .venv/bin/python juju-quickstart --gui-charm-url cs:~juju-gui/precise/juju-gui-128
% juju set juju-gui "juju-gui-source=lp:~bac/juju-gui/fix-dnd-bundles-wrt-id"
# May want to wait a bit. Perhaps it isn't necessary.
% juju set juju-gui "charmworld-url=http://staging.jujucharms.com"

Find one of Jorge's bundles on http://staging.jujucharms.com.
(wordpress-simple is a fast one.) Look at the deployment counts.

In the GUI search for 'jorge', find the bundle from above, and *drag* it onto
the canvas. Have a light snack. When everything is green and happy, look at
the page again on staging and see that the count has increased by one. NOTE:
in my QA I didn't see the incremented values reflected immediately which is
really strange. A few minutes later they were updated.

https://codereview.appspot.com/29960045/

[Brad Crittenden (bac)]

Reviewers: mp+196139_code.launchpad.net,

Message:
Please take a look.

Description:
Send bundle id when d'n'd from sidebar.

This path was initially missed. The Token had to be fixed to include
the
ID in the drag data.

Note, for selfish reasons, test/utils.js loadFixtures was updated to
retry on
failures. The reason for the occassional 404 in this function when run
by
lbox is still unknown. Hopefully the dumb retry will help.

QA Instructions:

# Use juju-quickstart trunk
% cd projects/juju-quickstart
% bzr pull
% make
# Deploy on EC2
% juju switch ec2
% .venv/bin/python juju-quickstart --gui-charm-url
cs:~juju-gui/precise/juju-gui-128
% juju set juju-gui
"juju-gui-source=lp:~bac/juju-gui/fix-dnd-bundles-wrt-id"
# May want to wait a bit. Perhaps it isn't necessary.
% juju set juju-gui "charmworld-url=http://staging.jujucharms.com"

Find one of Jorge's bundles on http://staging.jujucharms.com.
(wordpress-simple is a fast one.) Look at the deployment counts.

In the GUI search for 'jorge', find the bundle from above, and *drag* it
onto
the canvas. Have a light snack. When everything is green and happy,
look at
the page again on staging and see that the count has increased by one.
NOTE:
in my QA I didn't see the incremented values reflected immediately which
is
really strange. A few minutes later they were updated.

https://code.launchpad.net/~bac/juju-gui/fix-dnd-bundles-wrt-id/+merge/196139

(do not edit description out of merge proposal)

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

Affected files (+31, -8 lines):
   A [revision details]
   M app/views/topology/service.js
   M app/widgets/token.js
   M test/test_service_module.js
   M test/utils.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_service_module.js
=== modified file 'test/test_service_module.js'
--- test/test_service_module.js 2013-11-19 17:00:32 +0000
+++ test/test_service_module.js 2013-11-21 12:18:26 +0000
@@ -316,7 +316,8 @@
              dataTransfer: {
                getData: function(name) {
                  return JSON.stringify({
- data: '{"basket_name": "foo", "data": "BUNDLE DATA"}',
+ data: '{"basket_name": "foo", "data": "BUNDLE DATA",' +
+ ' "id": "~jorge/basket/thing"}',
                    dataType: 'token-drag-and-drop',
                    iconSrc: src
                  });
@@ -330,8 +331,9 @@

      // mock out the Y.BundleHelpers call.
      var _deployBundle = juju.BundleHelpers.deployBundle;
- juju.BundleHelpers.deployBundle = function(deployerData, env, db) {
+ juju.BundleHelpers.deployBundle = function(deployerData, id, env, db) {
        assert.include(deployerData, 'BUNDLE DATA');
+ assert.equal(id, '~jorge/basket/thing');
        // Restore the deployBundle call for future tests.
        juju.BundleHelpers.deployBundle = _deployBundle;
        done();
@@ -353,11 +355,11 @...

[Benji York (benji)]

Both code and QA LGTM.

https://codereview.appspot.com/29960045/

[Brad Crittenden (bac)]

*** Submitted:

Send bundle id when d'n'd from sidebar.

This path was initially missed. The Token had to be fixed to include
the
ID in the drag data.

Note, for selfish reasons, test/utils.js loadFixtures was updated to
retry on
failures. The reason for the occassional 404 in this function when run
by
lbox is still unknown. Hopefully the dumb retry will help.

QA Instructions:

# Use juju-quickstart trunk
% cd projects/juju-quickstart
% bzr pull
% make
# Deploy on EC2
% juju switch ec2
% .venv/bin/python juju-quickstart --gui-charm-url
cs:~juju-gui/precise/juju-gui-128
% juju set juju-gui
"juju-gui-source=lp:~bac/juju-gui/fix-dnd-bundles-wrt-id"
# May want to wait a bit. Perhaps it isn't necessary.
% juju set juju-gui "charmworld-url=http://staging.jujucharms.com"

Find one of Jorge's bundles on http://staging.jujucharms.com.
(wordpress-simple is a fast one.) Look at the deployment counts.

In the GUI search for 'jorge', find the bundle from above, and *drag* it
onto
the canvas. Have a light snack. When everything is green and happy,
look at
the page again on staging and see that the count has increased by one.
NOTE:
in my QA I didn't see the incremented values reflected immediately which
is
really strange. A few minutes later they were updated.

R=benji
CC=
https://codereview.appspot.com/29960045

https://codereview.appspot.com/29960045/

=== modified file 'app/views/topology/service.js'
--- app/views/topology/service.js	2013-11-19 16:54:54 +0000
+++ app/views/topology/service.js	2013-11-21 15:20:57 +0000
@@ -675,6 +675,7 @@
                 Y.JSON.stringify({
                   bundle: entityData.data
                 }),
+                entityData.id,
                 env,
                 db
             );

=== modified file 'app/widgets/token.js'
--- app/widgets/token.js	2013-10-31 19:01:36 +0000
+++ app/widgets/token.js	2013-11-21 15:20:57 +0000
@@ -48,15 +48,19 @@
       // Extract the charm/bundle values from the jumble of widget
       // cfg options.
       var attributes;
+      var type;
       if (utils.determineEntityDataType(cfg) === 'charm') {
         attributes = Y.Object.keys(Y.juju.models.Charm.ATTRS);
         this.TEMPLATE = templates['charm-token'];
+        type = 'charm';
       } else {
         attributes = Y.Object.keys(Y.juju.models.Bundle.ATTRS);
         this.TEMPLATE = templates['bundle-token'];
+        type = 'bundle';
       }
       // @property tokenData Contains the extracted information.
       this.tokenData = Y.aggregate({}, cfg, false, attributes);
+      this.tokenData.type = type;
     },
 
     /**
@@ -137,7 +141,9 @@
       var tokenData,
           container = this.get('boundingBox');
       // Adjust the ID to meet model expectations.
-      this.tokenData.id = this.tokenData.url;
+      if (this.tokenData.type === 'charm') {
+        this.tokenData.id = this.tokenData.url;
+      }
       // Since the browser's dataTransfer mechanism only accepts string values
       // we have to JSON encode the data.  This passed-in config includes
       // charm/bundle attributes.

=== modified file 'test/test_service_module.js'
--- test/test_service_module.js	2013-11-19 17:00:32 +0000
+++ test/test_service_module.js	2013-11-21 15:20:57 +0000
@@ -316,7 +316,8 @@
             dataTransfer: {
               getData: function(name) {
                 return JSON.stringify({
-                  data: '{"basket_name": "foo", "data": "BUNDLE DATA"}',
+                  data: '{"basket_name": "foo", "data": "BUNDLE DATA",' +
+                        ' "id": "~jorge/basket/thing"}',
                   dataType: 'token-drag-and-drop',
                   iconSrc: src
                 });
@@ -330,8 +331,9 @@
 
     // mock out the Y.BundleHelpers call.
     var _deployBundle = juju.BundleHelpers.deployBundle;
-    juju.BundleHelpers.deployBundle = function(deployerData, env, db) {
+    juju.BundleHelpers.deployBundle = function(deployerData, id, env, db) {
       assert.include(deployerData, 'BUNDLE DATA');
+      assert.equal(id, '~jorge/basket/thing');
       // Restore the deployBundle call for future tests.
       juju.BundleHelpers.deployBundle = _deployBundle;
       done();
@@ -353,11 +355,11 @@
     };
 
     // mock out the Y.BundleHelpers call.
-    var _deployBundle = juju.BundleHelpers.deployBundleFiles;
+    var _deployBundleFiles = juju.BundleHelpers.deployBundleFiles;
     juju.BundleHelpers.deployBundleFiles = function(files, env, db) {
       assert.deepEqual(files, [1]);
       // Restore the deployBundleFiles call for future tests.
-      juju.BundleHelpers.deployBundleFiles = _deployBundle;
+      juju.BundleHelpers.deployBundleFiles = _deployBundleFiles;
       done();
     };
 

=== modified file 'test/utils.js'
--- test/utils.js	2013-11-05 18:10:05 +0000
+++ test/utils.js	2013-11-21 15:20:57 +0000
@@ -101,9 +101,21 @@
      * @return {Object} fixture data resulting from call.
      */
     loadFixture: function(url, parseJson) {
-      var response = Y.io(url, {sync: true}).responseText;
-      if (parseJson) {
-        response = Y.JSON.parse(response);
+      var tries = 3;
+      var response;
+      while (true) {
+        try {
+          response = Y.io(url, {sync: true}).responseText;
+          if (parseJson) {
+            response = Y.JSON.parse(response);
+          }
+          break;
+        } catch (e) {
+          tries -= 1;
+          if (tries <= 0) {
+            throw e;
+          }
+        }
       }
       return response;
     },