Merge lp:~benji/juju-gui/critical-config-panel-fix into lp:juju-gui/experimental

[Benji York]

fix critical charm option attribute layout bug

https://codereview.appspot.com/10887043/

[Brad Crittenden (bac)]

LGTM TY OAO

https://codereview.appspot.com/10887043/

[Jeff Pihach (hatch)]

LGTM Thanks for qa'ing that extra bit for me!

https://codereview.appspot.com/10887043/

=== modified file 'app/subapps/browser/views/charm.js'
--- app/subapps/browser/views/charm.js	2013-06-27 21:19:35 +0000
+++ app/subapps/browser/views/charm.js	2013-07-02 20:29:30 +0000
@@ -79,9 +79,6 @@
       ev.halt();
       var browserCharm = this.get('charm');
       var charm = new models.Charm(browserCharm.getAttrs());
-      charm.set('config', {
-        options: browserCharm.get('options')
-      });
       if (this.get('isFullscreen')) {
         this.fire('viewNavigate',
             {change: {viewmode: 'sidebar', charmID: null}});

=== modified file 'app/views/charm-panel.js'
--- app/views/charm-panel.js	2013-06-27 13:01:27 +0000
+++ app/views/charm-panel.js	2013-07-02 20:29:30 +0000
@@ -852,14 +852,18 @@
      * @param {String} charmUrl The URL of the charm to configure/deploy.
      * @return {undefined} Nothing.
      */
-    function deploy(charm, ghostXY) {
+    function deploy(charm, ghostXY, _setPanel) {
+      _setPanel = _setPanel || setPanel; // Injection point for tests.
       // Any passed-in charm is fully loaded but the caller doesn't know about
       // the charm panel's internal detail of marking loaded charms, so we will
       // do the marking here.
       charm.loaded = true;
+      // The config panel expects the config options here instead of the
+      // "options" attribute. <shrug>
+      charm.set('config', {options: charm.get('options')});
       charms.add(charm);
       // Show the configuration panel.
-      setPanel({
+      _setPanel({
         name: 'configuration',
         charmId: charm.get('id'),
         ghostXY: ghostXY

=== modified file 'test/test_browser_charm_details.js'
--- test/test_browser_charm_details.js	2013-07-01 17:28:53 +0000
+++ test/test_browser_charm_details.js	2013-07-02 20:29:30 +0000
@@ -238,11 +238,6 @@
         assert.notDeepEqual(charm, browserCharm);
         var madeCharm = new models.Charm(browserCharm.getAttrs());
         assert.equal(charm.get('id'), madeCharm.get('id'));
-        // Verify that the charm config from the BrowserCharm is now part of
-        // config.options in the new Charm instance.
-        assert.equal(
-            browserCharm.get('options').configName,
-            charm.get('config').options.configName);
         done();
       });
       view._addCharmEnvironment({halt: function() {}});

=== modified file 'test/test_charm_panel.js'
--- test/test_charm_panel.js	2013-06-13 16:15:01 +0000
+++ test/test_charm_panel.js	2013-07-02 20:29:30 +0000
@@ -85,6 +85,39 @@
     node.getStyle('display').should.equal('none');
   });
 
+  it('has a deploy function that munges the charm options', function() {
+    var panel = Y.namespace('juju.views').CharmPanel
+          .getInstance({
+          testing: true,
+          container: container,
+          app: { views: { environment: {}}}
+        }),
+        node = panel.node;
+    panel.setActivePanelName('configuration');
+    // For whatever reason, the charm configuration panel now expects the charm
+    // options as an attribute of the charm config.  The deploy function
+    // therefore arranges for that to be true.
+    var OPTIONS = 'we are the options, my friend';
+    var setCalled = false;
+    var charm = {
+      id: 'cs:precise/juju-gui-7',
+      get: function(name) {
+        if (name === 'id') {
+          return 'cs:precise/juju-gui-7';
+        }
+        assert.equal(name, 'options');
+        return OPTIONS;
+      },
+      set: function(name, value) {
+        assert.equal(name, 'config');
+        assert.deepEqual(value, {options: OPTIONS});
+        setCalled = true;
+      }
+    };
+    panel.deploy(charm, undefined, function() {});
+    assert.isTrue(setCalled);
+  });
+
   describe('service ghost', function() {
     var app, db, env, panel, serviceName, store;