Merge lp:~rharding/juju-gui/bundle-sandbox-noop into lp:juju-gui/experimental

[Richard Harding]

fakebackend/sandbox no op deployer watches

- The fakebackend and the sandbox need to just ignore calls to watch deployer
status. It's too fast and we don't want to flood the users with a bunch of
notices all at once.
- Had to rename the WatchUpdate to deployerNext since the fakebackend auto
builds the names of the handle methods based off the actual call. So it needed
to be deployerNext though I hate it's no longer worded to tie to 'watch'.

QA:

Simply deploy a bundle in sandbox mode.

- Drag/drop a yaml file into the canvas
- Deploy from the deploy button from bundle details

Note: Dragging/dropping a bundle token is not working. This is also true on
coming soon. This is due to the recent bundle.id tracking code and will be
done in a follow up.

https://codereview.appspot.com/29860044/

[Richard Harding (rharding)]

Reviewers: mp+196000_code.launchpad.net,

Message:
Please take a look.

Description:
fakebackend/sandbox no op deployer watches

- The fakebackend and the sandbox need to just ignore calls to watch
deployer
status. It's too fast and we don't want to flood the users with a bunch
of
notices all at once.
- Had to rename the WatchUpdate to deployerNext since the fakebackend
auto
builds the names of the handle methods based off the actual call. So it
needed
to be deployerNext though I hate it's no longer worded to tie to
'watch'.
- Drive by fix for deploying a bundle via drag-n-drop.

QA:

Simply deploy a bundle in sandbox mode.

- Drag/drop a yaml file into the canvas
- Deploy from the deploy button from bundle details
-

Note: Dragging/dropping a bundle token is not working. This is also true
on
coming soon. This is due to the recent bundle.id tracking code and will
be
done in a follow up.

https://code.launchpad.net/~rharding/juju-gui/bundle-sandbox-noop/+merge/196000

(do not edit description out of merge proposal)

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

Affected files (+139, -15 lines):
   A [revision details]
   M app/assets/javascripts/bundle-import-helpers.js
   M app/store/env/fakebackend.js
   M app/store/env/go.js
   M app/store/env/sandbox.js
   M test/test_bundle_import_helpers.js
   M test/test_env_go.js
   M test/test_fakebackend.js
   M test/test_sandbox_go.js

[Brad Crittenden (bac)]

LGTM with some crazy nit-picking mess.

https://codereview.appspot.com/29860044/diff/20001/app/store/env/fakebackend.js
File app/store/env/fakebackend.js (right):

https://codereview.appspot.com/29860044/diff/20001/app/store/env/fakebackend.js#newcode1592
app/store/env/fakebackend.js:1592: This method is a noop in the
fakebackend. We've already sent the user a
noon looks too much like noob.

Pick one of 'no op' or 'no-op' throughout.

https://codereview.appspot.com/29860044/diff/20001/test/test_fakebackend.js
File test/test_fakebackend.js (right):

https://codereview.appspot.com/29860044/diff/20001/test/test_fakebackend.js#newcode740
test/test_fakebackend.js:740: // This is no op'd in the fakebackend
since it doens't make sense to
typo: doesn't

https://codereview.appspot.com/29860044/diff/20001/test/test_fakebackend.js#newcode741
test/test_fakebackend.js:741: // provide a watch status as things happen
near instantly.
grammar-o: nearly

https://codereview.appspot.com/29860044/diff/20001/test/test_fakebackend.js#newcode744
test/test_fakebackend.js:744: assert.fail('The watcher callback shoould
never be called.');
shoooooooould

https://codereview.appspot.com/29860044/diff/20001/test/test_fakebackend.js#newcode751
test/test_fakebackend.js:751: // This is no op'd in the fakebackend
since it doens't make sense to
Dang, on a roll!

https://codereview.appspot.com/29860044/diff/20001/test/test_fakebackend.js#newcode752
test/test_fakebackend.js:752: // provide a watch status as things happen
near instantly.
ditto

https://codereview.appspot.com/29860044/diff/20001/test/test_fakebackend.js#newcode755
test/test_fakebackend.js:755: assert.fail('The watcher update callback
shoould never be called.');
and again

https://codereview.appspot.com/29860044/

[Richard Harding (rharding)]

Please take a look.

https://codereview.appspot.com/29860044/

[lp:~rharding/juju-gui/bundle-sandbox-noop]

1205. By Richard Harding on 2013-11-20

Fix all the engrish

[Richard Harding (rharding)]

*** Submitted:

fakebackend/sandbox no op deployer watches

- The fakebackend and the sandbox need to just ignore calls to watch
deployer
status. It's too fast and we don't want to flood the users with a bunch
of
notices all at once.
- Had to rename the WatchUpdate to deployerNext since the fakebackend
auto
builds the names of the handle methods based off the actual call. So it
needed
to be deployerNext though I hate it's no longer worded to tie to
'watch'.

QA:

Simply deploy a bundle in sandbox mode.

- Drag/drop a yaml file into the canvas
- Deploy from the deploy button from bundle details

Note: Dragging/dropping a bundle token is not working. This is also true
on
coming soon. This is due to the recent bundle.id tracking code and will
be
done in a follow up.

R=bac
CC=
https://codereview.appspot.com/29860044

https://codereview.appspot.com/29860044/

=== modified file 'app/assets/javascripts/bundle-import-helpers.js'
--- app/assets/javascripts/bundle-import-helpers.js	2013-11-19 16:54:54 +0000
+++ app/assets/javascripts/bundle-import-helpers.js	2013-11-20 19:38:27 +0000
@@ -234,14 +234,14 @@
             // There's nothing else to see here.
             return;
           } else {
-            env.deployerWatchUpdate(watchId, processUpdate);
+            env.deployerNext(watchId, processUpdate);
           }
         }
       };
 
       // Make the first call to the env and the processUpdate callback will
       // handle re-calling on each update.
-      env.deployerWatchUpdate(watchId, processUpdate);
+      env.deployerNext(watchId, processUpdate);
     }
 
   };

=== modified file 'app/store/env/fakebackend.js'
--- app/store/env/fakebackend.js	2013-11-08 18:57:35 +0000
+++ app/store/env/fakebackend.js	2013-11-20 19:38:27 +0000
@@ -1584,6 +1584,44 @@
         return callback(UNAUTHENTICATED_ERROR);
       }
       callback({LastChanges: this._importChanges});
+    },
+
+    /**
+      *no op* Create a watcher for the deployment specified.
+
+      This method is a no op in the fakebackend. We've already sent the user a
+      notification that things are complete in the normal deployer call.
+      There's no time to get a watcher and send/sync the watch update down
+      the road.
+
+      @method deployerWatch
+      @param {Integer} deploymentId The id of the deployment the watch is
+      for.
+      @param {Function} callback The callback to send the watcherId to.
+
+     */
+    deployerWatch: function(deploymentId, callback) {
+      // No op in the fakebackend. Just return and ignore the callback.
+      return;
+    },
+
+    /**
+      *no op* Perform a check for updates of a given deployment watcher.
+
+      This should never be called and is provided just to aid in grep-ability
+      of the codebase. Typically this is called via the callback in the
+      deployerWatch function.
+
+
+      @method deployerNext
+      @param {Integer} watcherId The id of the watcher from deployerWatch
+      @param {Function} callback The callback to handle the update response
+      from the deployer information.
+
+     */
+    deployerNext: function(watcherId, callback) {
+      // No op in the fakebackend. Just return and ignore the callback.
+      return;
     }
 
   });

=== modified file 'app/store/env/go.js'
--- app/store/env/go.js	2013-11-18 18:25:08 +0000
+++ app/store/env/go.js	2013-11-20 19:38:27 +0000
@@ -554,15 +554,15 @@
       The callback will receive an {Object} An object with err, and the
       list of Changes.
 
-      @method deployerWatchUpdate
+      @method deployerNext
       @param {Integer} watchId The ID of the watcher created in depployWatch.
       @param {Function} callback The caller's callback function to process
       the response.
      */
-    deployerWatchUpdate: function(watchId, callback) {
+    deployerNext: function(watchId, callback) {
       var intermediateCallback;
       if (callback) {
-        intermediateCallback = Y.bind(this.handleDeployerWatchUpdate,
+        intermediateCallback = Y.bind(this.handleDeployerNext,
                                       this, callback);
       }
 
@@ -576,15 +576,15 @@
     },
 
     /**
-      Wrapper for the deployerWatchUpdate call.
+      Wrapper for the deployerNext call.
 
-      @method handleDeployerWatchUpdate
+      @method handleDeployerNext
       @param {Function} userCallback The original callback to the
-      deployerWatchUpdate function.
-      @param {Object} data The servers response to the deployerWatchUpdate
+      deployerNext function.
+      @param {Object} data The servers response to the deployerNext
       call.
      */
-    handleDeployerWatchUpdate: function(userCallback, data) {
+    handleDeployerNext: function(userCallback, data) {
       var transformedData = {
         err: data.Error,
         Changes: data.Response.Changes

=== modified file 'app/store/env/sandbox.js'
--- app/store/env/sandbox.js	2013-11-19 22:30:18 +0000
+++ app/store/env/sandbox.js	2013-11-20 19:38:27 +0000
@@ -1347,7 +1347,34 @@
       state.statusDeployer(Y.bind(callback, this));
     },
 
+    /**
+     * *no op* Handle the response from a deployerWatch call from the backend.
+     *
+     * @method handleDeployerWatch
+     * @param {Object} data The contents of the API arguments.
+     * @param {Object} client The active ClientConnection.
+     * @param {Object} state An instance of FakeBackend.
+     * @return {undefined} Side effects only.
+     *
+     */
+    handleDeployerWatch: function(data, client, state) {
+      return;
+    },
 
+    /**
+     * *no op* Handle the response from a deployerNext call from the
+     * backend.
+     *
+     * @method handleDeployerNext
+     * @param {Object} data The contents of the API arguments.
+     * @param {Object} client The active ClientConnection.
+     * @param {Object} state An instance of FakeBackend.
+     * @return {undefined} Side effects only.
+     *
+     */
+    handleDeployerNext: function(data, client, state) {
+      return;
+    },
 
     /**
     Handle SetAnnotations messages

=== modified file 'test/test_bundle_import_helpers.js'
--- test/test_bundle_import_helpers.js	2013-11-19 16:54:54 +0000
+++ test/test_bundle_import_helpers.js	2013-11-20 19:38:27 +0000
@@ -139,7 +139,7 @@
       };
 
       var called = false;
-      env.deployerWatchUpdate = function(watchId, callback) {
+      env.deployerNext = function(watchId, callback) {
         if (!called) {
           called = true;
           callback({
@@ -192,7 +192,7 @@
         done();
       };
 
-      env.deployerWatchUpdate = function(watchId, callback) {
+      env.deployerNext = function(watchId, callback) {
         callback({
           err: undefined,
           Changes: [
@@ -239,7 +239,7 @@
       };
 
       var updated = false;
-      env.deployerWatchUpdate = function(watchId, callback) {
+      env.deployerNext = function(watchId, callback) {
         if (!updated) {
           updated = true;
           callback({

=== modified file 'test/test_env_go.js'
--- test/test_env_go.js	2013-11-13 21:56:04 +0000
+++ test/test_env_go.js	2013-11-20 19:38:27 +0000
@@ -368,7 +368,7 @@
       });
 
       it('builds a proper watch next request', function() {
-        env.deployerWatchUpdate(5);
+        env.deployerNext(5);
         var last_message = conn.last_message();
         var expected = {
           Type: 'Deployer',

=== modified file 'test/test_fakebackend.js'
--- test/test_fakebackend.js	2013-11-08 18:57:35 +0000
+++ test/test_fakebackend.js	2013-11-20 19:38:27 +0000
@@ -735,6 +735,28 @@
             done();
           }).then(undefined, done);
     });
+
+    it('should ignore calls to the deployer watcher', function(done) {
+      // This is no op'd in the fakebackend since it doesn't make sense to
+      // provide a watch status as things happen nearly instantly.
+      fakebackend.deployerWatch(10, function(reply) {
+        // Should never be called.
+        assert.fail('The watcher callback should never be called.');
+      });
+
+      done();
+    });
+
+    it('should ignore calls to the watcher updater', function(done) {
+      // This is no op'd in the fakebackend since it doens't make sense to
+      // provide a watch status as things happen nearly instantly.
+      fakebackend.deployerWatch(10, function(reply) {
+        // Should never be called.
+        assert.fail('The watcher update callback should never be called.');
+      });
+      done();
+    });
+
   });
 
   describe('FakeBackend.uniformOperations', function() {

=== modified file 'test/test_sandbox_go.js'
--- test/test_sandbox_go.js	2013-10-31 18:40:57 +0000
+++ test/test_sandbox_go.js	2013-11-20 19:38:27 +0000
@@ -1242,7 +1242,6 @@
       env.deployerImport(fixture, null, callback);
     });
 
-
     it('should support deployer status without imports', function(done) {
       var data = {
         Type: 'Deployer',
@@ -1261,6 +1260,42 @@
       client.send(Y.JSON.stringify(data));
     });
 
+    it('should not deal with deployer watches', function(done) {
+      var data = {
+        Type: 'Deployer',
+        Request: 'Watch',
+        Params: {
+          DeploymentId: 10
+        },
+        RequestId: 42
+      };
+      client.onmessage = function(received) {
+        assert.fail('Should never get a response.');
+      };
+      client.open();
+      client.send(Y.JSON.stringify(data));
+
+      done();
+    });
+
+    it('should not deal with watch updates', function(done) {
+      var data = {
+        Type: 'Deployer',
+        Request: 'Next',
+        Params: {
+          WatcherId: 11
+        },
+        RequestId: 42
+      };
+      client.onmessage = function(received) {
+        assert.fail('Should never get a response.');
+      };
+      client.open();
+      client.send(Y.JSON.stringify(data));
+
+      done();
+    });
+
   });
 
 })();