Reviewers: mp+176682_code.launchpad.net,

Message:
Please take a look.

Description:
Subordinates should not be deployed with unit ct.

Deploying a subordinate with a unit count causes an error in core.  This
prevents one from entering a unit count in the charm panel (did not
touch ghost
inspector, that may also need it) and defaults to 0.  This also affects
fakebackend.

https://code.launchpad.net/~gary/juju-gui/core-sub-num-units/+merge/176682

(do not edit description out of merge proposal)


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

Affected files:
   A [revision details]
   M app/store/env/fakebackend.js
   M app/templates/charm-pre-configuration.handlebars
   M app/views/charm-panel.js
   M test/test_fakebackend.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_fakebackend.js
=== modified file 'test/test_fakebackend.js'
--- test/test_fakebackend.js	2013-07-09 15:11:25 +0000
+++ test/test_fakebackend.js	2013-07-24 13:06:18 +0000
@@ -168,7 +168,7 @@
        assert.isUndefined(charm.get('is_subordinate'));
        // The _set forces a change to a writeOnly attribute.
        charm._set('is_subordinate', true);
-      fakebackend.deploy('cs:wordpress', callback);
+      fakebackend.deploy('cs:wordpress', callback, {unitCount: 0});
        assert.isUndefined(result.error);
        assert.strictEqual(
            fakebackend.db.charms.getById('cs:precise/wordpress-10'), charm);
@@ -859,6 +859,25 @@
            'Invalid number of units.');
      });

+    it('returns error for invalid number of subordinate units', function()  
{
+      fakebackend.deploy('cs:puppet', callback);
+      assert.isUndefined(deployResult.error);
+      assert.equal(
+          fakebackend.addUnit('puppet', 'goyesca').error,
+          'Invalid number of units.');
+      assert.equal(
+          fakebackend.addUnit('puppet', 1).error,
+          'Invalid number of units.');
+      assert.equal(
+          fakebackend.addUnit('puppet', -1).error,
+          'Invalid number of units.');
+      // It also ignores empty requests
+      assert.isUndefined(
+          fakebackend.addUnit('puppet', 0).error);
+      assert.isUndefined(
+          fakebackend.addUnit('puppet').error);
+    });
+
      it('returns an error if the service does not exist.', function() {
        assert.equal(
            fakebackend.addUnit('foo').error,


Index: app/templates/charm-pre-configuration.handlebars
=== modified file 'app/templates/charm-pre-configuration.handlebars'
--- app/templates/charm-pre-configuration.handlebars	2013-07-17 19:25:41  
+0000
+++ app/templates/charm-pre-configuration.handlebars	2013-07-24 13:03:35  
+0000
@@ -26,18 +26,22 @@
            </div>
          </div>
        </div>
-      <div class="charm-entry">
-        <div class="control-group">
-          <div class="well control-description">
-            Number of units to deploy for this service.
-          </div>
-          <div class="control-label" for="number-units">Number of  
units</div>
-          <div class="controls">
-            <input class="config-field" type="text" name="number-units"
-                id="number-units" value="1"/>
+      {{#if is_subordinate}}
+        <input type="hidden" id="number-units" value="0" />
+      {{else}}
+        <div class="charm-entry">
+          <div class="control-group">
+            <div class="well control-description">
+              Number of units to deploy for this service.
+            </div>
+            <div class="control-label" for="number-units">Number of  
units</div>
+            <div class="controls">
+              <input class="config-field" type="text" name="number-units"
+                  id="number-units" value="1"/>
+            </div>
            </div>
          </div>
-      </div>
+      {{/if}}
        {{/with}}

        {{#if settings}}


Index: app/views/charm-panel.js
=== modified file 'app/views/charm-panel.js'
--- app/views/charm-panel.js	2013-07-16 22:05:06 +0000
+++ app/views/charm-panel.js	2013-07-24 03:22:41 +0000
@@ -629,11 +629,11 @@
            if (this.configFileContent) {
              config = null;
            }
-          numUnits = parseInt(numUnits, 10);
+          numUnits = charm.get('is_subordinate') ? 0 : parseInt(numUnits,  
10);
            env.deploy(url, serviceName, config, this.configFileContent,
                numUnits, function(ev) {
                  if (ev.err) {
-                  console.log(url + ' deployment failed');
+                  console.log(url + ' deployment failed', ev.err);
                    db.notifications.add(
                        new models.Notification({
                          title: 'Error deploying ' + serviceName,


Index: app/store/env/fakebackend.js
=== modified file 'app/store/env/fakebackend.js'
--- app/store/env/fakebackend.js	2013-07-11 14:56:15 +0000
+++ app/store/env/fakebackend.js	2013-07-24 13:03:35 +0000
@@ -600,19 +600,19 @@
        if (!this.get('authenticated')) {
          return UNAUTHENTICATED_ERROR;
        }
+      var service = this.db.services.getById(serviceName);
+      if (!service) {
+        return {error: 'Service "' + serviceName + '" does not exist.'};
+      }
+      var is_subordinate = service.get('subordinate');
        if (Y.Lang.isUndefined(numUnits)) {
-        numUnits = 1;
+        numUnits = is_subordinate ? 0 : 1;
        }
-      if (!Y.Lang.isNumber(numUnits) || numUnits < 1) {
+      if (!Y.Lang.isNumber(numUnits) ||
+          (!is_subordinate && numUnits < 1 ||
+          (is_subordinate && numUnits !== 0))) {
          return {error: 'Invalid number of units.'};
        }
-      var service = this.db.services.getById(serviceName);
-      if (!service) {
-        return {error: 'Service "' + serviceName + '" does not exist.'};
-      }
-      if (service && service.get('is_subordinate')) {
-        return {error: serviceName + ' is a subordinate, cannot add  
unit(s).'};
-      }
        if (!Y.Lang.isValue(service.unitSequence)) {
          service.unitSequence = 0;
        }