Merge lp:~benji/juju-gui/no-foot-shooting into lp:juju-gui/experimental

[Benji York]

Prevent destruction of the Juju GUI service

https://codereview.appspot.com/7129049/

[Gary Poster (gary)]

Good stuff, Benji, thank you.

Land with changes. What I'm bringing is usually trivial, and should be
fairly easy to address, I hope, even when I think it is important.

Gary

https://codereview.appspot.com/7129049/diff/1/app/templates/service-footer-common-controls.partial
File app/templates/service-footer-common-controls.partial (right):

https://codereview.appspot.com/7129049/diff/1/app/templates/service-footer-common-controls.partial#newcode1
app/templates/service-footer-common-controls.partial:1: {{#unless
serviceIsJujuGUI}}
My inclination is to let the GUI increase the unit count, but not be
exposed--IOW, move serviceIsJujuGUI into only be around
"control-expose," below. Given the basic approach that I think we have
now of "innocent until proven guilty" for disabling GUI service
functionality, do you have a concrete reason to exclude unit count--a
reason to say that this is a problem? Note that I am pretty sure that
unit count may not be reduced beneath one, thanks to a pre-existing
constraint in the GUI.

https://codereview.appspot.com/7129049/diff/1/app/templates/service.handlebars
File app/templates/service.handlebars (right):

https://codereview.appspot.com/7129049/diff/1/app/templates/service.handlebars#newcode25
app/templates/service.handlebars:25: {{#unless serviceIsJujuGUI}}
If you agree that we should reinstate unit count, then IIUC you would
also remove this conditional.

https://codereview.appspot.com/7129049/diff/1/app/views/service.js
File app/views/service.js (right):

https://codereview.appspot.com/7129049/diff/1/app/views/service.js#newcode170
app/views/service.js:170: // it would break the application they are
currently using.
Suggest you add another comment similar to the following:

// Note that the destroy button is not shown for the Juju GUI, so this
is a fail-safe.

https://codereview.appspot.com/7129049/diff/1/app/views/service.js#newcode381
app/views/service.js:381: * Aside from a nice seperation of concerns,
this method also
typo: separation

https://codereview.appspot.com/7129049/diff/1/app/views/service.js#newcode382
app/views/service.js:382: * facilitates testing.
Interesting approach. I like it so far.

https://codereview.appspot.com/7129049/diff/1/app/views/topology/service.js
File app/views/topology/service.js (right):

https://codereview.appspot.com/7129049/diff/1/app/views/topology/service.js#newcode79
app/views/topology/service.js:79: // it would break the application they
are currently using.
I'm not sure yet from the code, but I'm hoping that this is again a
fail-safe because the "destroy" option is not given to the user, in
which case a comment to that effect would again be my preference.

https://codereview.appspot.com/7129049/diff/1/app/views/topology/service.js#newcode740
app/views/topology/service.js:740: var menu =
view.get('container').one('#service-menu'),
nicer variable name, thank you :-)

https://codereview.appspot.com/7129049/diff/1/app/views/topology/service.js#newcode755
app/views/topology/service.js:755:
menu.one('.destroy-service').addClass('disabled');
Per my previous comment, yep, I see you disable it visually as well.
Cool.

https://codereview.appspot.com/7129049/diff/1/app/vie...

[Gary Poster (gary)]

Sounds good benji. Thanks!

Gary

https://codereview.appspot.com/7129049/diff/1/app/views/service.js
File app/views/service.js (right):

https://codereview.appspot.com/7129049/diff/1/app/views/service.js#newcode170
app/views/service.js:170: // it would break the application they are
currently using.
On 2013/01/17 19:19:50, benji wrote:
> On 2013/01/17 17:34:06, gary.poster wrote:
> > Suggest you add another comment similar to the following:
> >
> > // Note that the destroy button is not shown for the Juju GUI, so
this is a
> > fail-safe.

> I would be inclined to remove the code altogether. I accidentally
duplicated
> this between the menu's destroy handler and the destroy button at the
bottom of
> the service view. It is needed for the menu because the menu item is
only
> disabled in as far as its visual styling is different, the click event
is still
> fired.

> I have removed the code but if you like the fail-safe, I will be glad
to put it
> back (with commentary).

Cool, that's fine. I could go either way.

https://codereview.appspot.com/7129049/diff/1/app/views/topology/service.js
File app/views/topology/service.js (right):

https://codereview.appspot.com/7129049/diff/1/app/views/topology/service.js#newcode79
app/views/topology/service.js:79: // it would break the application they
are currently using.
On 2013/01/17 19:19:50, benji wrote:
> On 2013/01/17 17:34:06, gary.poster wrote:
> > I'm not sure yet from the code, but I'm hoping that this is again a
fail-safe
> > because the "destroy" option is not given to the user, in which case
a comment
> > to that effect would again be my preference.

> Nope, this is really the code that keeps the destroy from happening.
The menu
> item is only "disabled" visually. The click event is still fired.
This is the
> pre-existing pattern in the code (for the always-dead "Units" menu
item.

Ah, OK, cool.

https://codereview.appspot.com/7129049/diff/1/app/views/utils.js
File app/views/utils.js (right):

https://codereview.appspot.com/7129049/diff/1/app/views/utils.js#newcode963
app/views/utils.js:963: var charmUrl = candidate.charm ||
candidate.get('charm');
On 2013/01/17 19:19:50, benji wrote:
> On 2013/01/17 17:34:06, gary.poster wrote:
> > In the previous branch, Nicola and I both found this to be too
fragile. See
> the
> > version in
https://codereview.appspot.com/7069068/diff/1/app/views/utils.js .
> > You don't need to use that approach, and *maybe* it is not necessary
now for
> > some reason, but it was necessary in the previous branch for tests
to pass.

> All the tests pass with this version, so I am inclined to avoid the
"sloppier"
> version. The things passed in really should have a "charm" attribute
one way or
> the other.

> That being said, I don't feel strongly about it, so one more nudge
will get me
> to use the other branch's version.

Naah, let's stick with what you have, thanks.

https://codereview.appspot.com/7129049/diff/1/lib/views/stylesheet.less
File lib/views/stylesheet.less (right):

https://codereview.appspot.com/7129049/diff/1/lib/views/stylesheet.less#newcode1507
lib/views/stylesheet.less:1507: .warning-message {
On 2013/01/17 19:19:50, benji wrote:
> On 2013/01/17 17:3...

[lp:~benji/juju-gui/no-foot-shooting]

320. By Benji York on 2013-01-17

merge from trunk

321. By Benji York on 2013-01-17

checkpoint

322. By Benji York on 2013-01-17

more review changes

323. By Benji York on 2013-01-17

fix lint

[Benjamin Saller (bcsaller)]

LGTM, a minor suggestion or two included.

https://codereview.appspot.com/7129049/diff/15001/app/views/service.js
File app/views/service.js (right):

https://codereview.appspot.com/7129049/diff/15001/app/views/service.js#newcode566
app/views/service.js:566: *
Agreed, each of these changes wrt gatherRenderData is nice.

https://codereview.appspot.com/7129049/diff/15001/app/views/topology/service.js
File app/views/topology/service.js (right):

https://codereview.appspot.com/7129049/diff/15001/app/views/topology/service.js#newcode769
app/views/topology/service.js:769:
menu.one('.destroy-service').addClass('disabled');
Does this actually work? .destroy-service still has the click handler
bound even if the class greys out the link or something, no?

Might we remove the link from the menu all together?

https://codereview.appspot.com/7129049/diff/15001/test/test_topology.js
File test/test_topology.js (right):

https://codereview.appspot.com/7129049/diff/15001/test/test_topology.js#newcode118
test/test_topology.js:118:
topo.events.ServiceModule.scene['.destroy-service'].click.callback(
a simulate('click') test might be better here if you leave this in the
menu.

https://codereview.appspot.com/7129049/

=== modified file 'app/templates/overview.handlebars'
--- app/templates/overview.handlebars	2012-12-24 03:17:11 +0000
+++ app/templates/overview.handlebars	2013-01-17 20:33:22 +0000
@@ -5,7 +5,7 @@
             <ul>
                 <li class="view-service">View</li>
                 <li class="add-relation">Build Relation</li>
-                <li class="add-remove-units">Units</li>
+                <li class="add-remove-units disabled">Units</li>
                 <li class="destroy-service">Destroy</li>
             </ul>
         </div>
 
=== modified file 'app/templates/service-config.handlebars'
--- app/templates/service-config.handlebars	2012-10-12 18:27:36 +0000
+++ app/templates/service-config.handlebars	2013-01-17 20:33:22 +0000
@@ -2,12 +2,18 @@
 <div class="view-container">
   <form id="service-config" class="form-horizontal">
     <legend>{{service.id}} settings</legend>
-      {{> service-config}}
-      <div class="control-group">
-        <div class="controls">
-          <button type="button" id="save-service-config">Update</button>
-        </div>
+    {{> service-config}}
+    <div class="control-group">
+      <div class="controls">
+        <button type="button" id="save-service-config">Update</button>
+        {{#if serviceIsJujuGUI}}
+          <span class="warning-message">
+            Warning: Changing the settings on this page may adversely affect
+            your ability to use the Juju GUI.
+          </span>
+        {{/if}}
       </div>
+    </div>
   </form>
 </div>
-{{> service-footer}} 
+{{> service-footer}}
 
=== modified file 'app/templates/service-footer-common-controls.partial'
--- app/templates/service-footer-common-controls.partial	2012-10-25 07:57:11 +0000
+++ app/templates/service-footer-common-controls.partial	2013-01-17 20:33:22 +0000
@@ -1,28 +1,31 @@
 <div class="inline service-common-controls">
- {{#unless charm.is_subordinate}}
- <div class="control-unit-count">
-   <div class="inline"><span>Unit count</span></div>
-   <div class="inline">
-     <input type="text" id="num-service-units" value="{{service.unit_count}}">
-     <img class="divider"
-           src="/juju-ui/assets/images/bottom_bar_small_div.png" />
-   </div>
- </div>
- {{/unless}}
- <div class="control-expose">
-   <div class="inline"><span>Expose</span></div>
-   <div class="inline">
-   {{#if service.exposed}}
-     <img class="unexposeService"
-          alt="Exposed"
-          src="/juju-ui/assets/images/slider_on.png" />
-     <span class="on">On</span>
-   {{else}}
-     <img class="exposeService"
-          alt="Not exposed"
-          src="/juju-ui/assets/images/slider_off.png" />
-     <span class="off">Off</span>
-   {{/if}}
-   </div>
-  </div>
-</div>
+{{#unless charm.is_subordinate}}
+  <div class="control-unit-count">
+    <div class="inline"><span>Unit count</span></div>
+    <div class="inline">
+      <input type="text" id="num-service-units"
+          value="{{service.unit_count}}">
+      <img class="divider"
+          src="/juju-ui/assets/images/bottom_bar_small_div.png" />
+    </div>
+  </div>
+{{/unless}}
+{{#unless serviceIsJujuGUI}}
+  <div class="control-expose">
+    <div class="inline"><span>Expose</span></div>
+    <div class="inline">
+    {{#if service.exposed}}
+      <img class="unexposeService"
+            alt="Exposed"
+            src="/juju-ui/assets/images/slider_on.png" />
+      <span class="on">On</span>
+    {{else}}
+      <img class="exposeService"
+            alt="Not exposed"
+            src="/juju-ui/assets/images/slider_off.png" />
+      <span class="off">Off</span>
+    {{/if}}
+    </div>
+    </div>
+  </div>
+{{/unless}}
 
=== modified file 'app/templates/service-footer-destroy-service.partial'
--- app/templates/service-footer-destroy-service.partial	2012-10-22 11:57:15 +0000
+++ app/templates/service-footer-destroy-service.partial	2013-01-17 20:33:22 +0000
@@ -1,8 +1,7 @@
-<div class="destroy-control">
-  <a id="destroy-service" data-toggle="modal" 
-     href="#destroy-service-modal">
-    <img src="/juju-ui/assets/images/destroy_icon.png" />Destroy
-  </a>
-  <img class="divider" 
-       src="/juju-ui/assets/images/bottom_bar_big_div.png" />
-</div>
 \ No newline at end of file
+{{#unless serviceIsJujuGUI}}
+  <div class="destroy-control">
+    <a id="destroy-service" data-toggle="modal" href="#destroy-service-modal"
+      ><img src="/juju-ui/assets/images/destroy_icon.png" />Destroy</a>
+    <img class="divider" src="/juju-ui/assets/images/bottom_bar_big_div.png" />
+  </div>
+{{/unless}}
 
=== modified file 'app/templates/service.handlebars'
--- app/templates/service.handlebars	2012-11-05 23:10:01 +0000
+++ app/templates/service.handlebars	2013-01-17 20:33:22 +0000
@@ -14,17 +14,16 @@
         <div class="inline filter-label"><span>Filter</span></div>
         <div class="state-buttons">
           {{#states}}
-          <div 
-            class="inline {{#if active}}active{{/if}}">
-            <a href="{{link}}">
-              <span class="state-title {{#if active}}active{{/if}}">{{title}}
-              </span>
-            </a>
-          </div>
+            <div class="inline {{#if active}}active{{/if}}">
+              <a href="{{link}}">
+                <span class="state-title {{#if active}}active{{/if}}">{{title}}
+                </span>
+              </a>
+            </div>
           {{/states}}
         </div>
         <div class="inline control-separator">
-          <img class="divider" 
+          <img class="divider"
                 src="/juju-ui/assets/images/bottom_bar_big_div.png" />
         </div>
     </div>
 
=== modified file 'app/views/service.js'
--- app/views/service.js	2012-11-08 16:23:16 +0000
+++ app/views/service.js	2013-01-17 20:33:22 +0000
@@ -164,9 +164,9 @@
     },
 
     confirmDestroy: function(ev) {
+      ev.preventDefault();
       // We wait to make the panel until now, because in the render method
       // the container is not yet part of the document.
-      ev.preventDefault();
       if (Y.Lang.isUndefined(this.panel)) {
         this.panel = views.createModalPanel(
             'Are you sure you want to destroy the service?  ' +
@@ -360,7 +360,7 @@
       });
   views.serviceBase = ServiceViewBase;
 
-  var ServiceRelations = Y.Base.create(
+  views.service_relations = Y.Base.create(
       'ServiceRelationsView', ServiceViewBase, [views.JujuBaseView], {
 
         template: Templates['service-relations'],
@@ -369,17 +369,20 @@
           '#service-relations .btn': {click: 'confirmRemoved'}
         },
 
-        render: function() {
+        /**
+         * Gather up all of the data required for the template.
+         *
+         * Aside from a nice separation of concerns, this method also
+         * facilitates testing.
+         *
+         * @method gatherRenderData
+         * @return {Object} The data the template will render.
+         */
+        gatherRenderData: function() {
           var container = this.get('container'),
-              getModelURL = this.get('getModelURL'),
               service = this.get('model'),
               db = this.get('db'),
               querystring = this.get('querystring');
-          if (!service || !service.get('loaded')) {
-            container.setHTML('<div class="alert">Loading...</div>');
-            console.log('waiting on service data');
-            return this;
-          }
           var relation_data = utils.getRelationDataForService(db, service);
           Y.each(relation_data, function(rel) {
             if (rel.relation_id === querystring.rel_id) {
@@ -389,14 +392,27 @@
           var charm_id = service.get('charm'),
               charm = db.charms.getById(charm_id),
               charm_attrs = charm ? charm.getAttrs() : undefined;
-          container.setHTML(this.template(
-              { viewName: 'relations',
-                tabs: this.getServiceTabs('relations'),
-                service: service.getAttrs(),
-                relations: relation_data,
-                charm: charm_attrs,
-                charm_id: charm_id}));
+          return {
+            viewName: 'relations',
+            tabs: this.getServiceTabs('relations'),
+            service: service.getAttrs(),
+            relations: relation_data,
+            charm: charm_attrs,
+            charm_id: charm_id,
+            serviceIsJujuGUI: utils.isGuiCharmUrl(charm_id)
+          };
+        },
 
+        render: function() {
+          var container = this.get('container');
+          var service = this.get('model');
+          if (!service || !service.get('loaded')) {
+            container.setHTML('<div class="alert">Loading...</div>');
+            console.log('waiting on service data');
+          } else {
+            container.setHTML(this.template(this.gatherRenderData()));
+          }
+          return this;
         },
 
         confirmRemoved: function(ev) {
@@ -474,9 +490,7 @@
         }
       });
 
-  views.service_relations = ServiceRelations;
-
-  var ServiceConstraints = Y.Base.create(
+  views.service_constraints = Y.Base.create(
       'ServiceConstraintsView', ServiceViewBase, [views.JujuBaseView], {
 
         template: Templates['service-constraints'],
@@ -544,11 +558,17 @@
           }
         },
 
-        render: function() {
-          var container = this.get('container'),
-              service = this.get('model'),
-              getModelURL = this.get('getModelURL'),
-              db = this.get('db'),
+        /**
+         * Gather up all of the data required for the template.
+         *
+         * Aside from a nice separation of concerns, this method also
+         * facilitates testing.
+         *
+         * @method gatherRenderData
+         * @return {Object} The data the template will render.
+         */
+        gatherRenderData: function() {
+          var service = this.get('model'),
               constraints = service.get('constraints'),
               display_constraints = [];
 
@@ -574,7 +594,8 @@
           });
 
           console.log('service constraints', display_constraints);
-          container.setHTML(this.template({
+          var charm_id = service.get('charm');
+          return {
             viewName: 'constraints',
             tabs: this.getServiceTabs('constraints'),
             service: service.getAttrs(),
@@ -586,17 +607,20 @@
               });
               return arr;
             })(),
-            charm_id: service.get('charm')}
-          ));
+            charm_id: charm_id,
+            serviceIsJujuGUI: utils.isGuiCharmUrl(charm_id)
+          };
+        },
 
+        render: function() {
+          var container = this.get('container');
+          container.setHTML(this.template(this.gatherRenderData()));
           return this;
         }
 
       });
 
-  views.service_constraints = ServiceConstraints;
-
-  var ServiceConfigView = Y.Base.create(
+  views.service_config = Y.Base.create(
       'ServiceConfigView', ServiceViewBase, [views.JujuBaseView], {
 
         template: Templates['service-config'],
@@ -605,27 +629,27 @@
           '#save-service-config': {click: 'saveConfig'}
         },
 
-        render: function() {
-          var container = this.get('container'),
-              db = this.get('db'),
-              service = this.get('model');
-
-          if (!service || !service.get('loaded')) {
-            container.setHTML('<div class="alert">Loading...</div>');
-            console.log('waiting on service data');
-            return this;
-          }
-
-          console.log('config', service.get('config'));
-
-          // combine the charm schema and the service values for display.
-          var charm = db.charms.getById(service.get('charm')),
-              config = service.get('config'),
-              getModelURL = this.get('getModelURL'),
-              charm_config = charm.get('config'),
-              schema = charm_config && charm_config.options,
-              settings = [],
-              field_def;
+        /**
+         * Gather up all of the data required for the template.
+         *
+         * Aside from a nice separation of concerns, this method also
+         * facilitates testing.
+         *
+         * @method gatherRenderData
+         * @return {Object} The data the template will render.
+         */
+        gatherRenderData: function() {
+          var container = this.get('container');
+          var db = this.get('db');
+          var service = this.get('model');
+          var charm = db.charms.getById(service.get('charm'));
+          var config = service.get('config');
+          var getModelURL = this.get('getModelURL');
+          var charm_config = charm.get('config');
+          var schema = charm_config && charm_config.options;
+          var settings = [];
+          var charm_id = service.get('charm');
+          var field_def;
 
           Y.Object.each(schema, function(field_def, field_name) {
             var entry = {
@@ -651,16 +675,25 @@
             settings.push(Y.mix(entry, field_def));
           });
 
-          console.log('render view svc config', service.getAttrs(), settings);
-
-          container.setHTML(this.template(
-              { viewName: 'config',
-                tabs: this.getServiceTabs('config'),
-                service: service.getAttrs(),
-                settings: settings,
-                charm_id: service.get('charm')}
-              ));
-
+          return {
+            viewName: 'config',
+            tabs: this.getServiceTabs('config'),
+            service: service.getAttrs(),
+            settings: settings,
+            charm_id: charm_id,
+            serviceIsJujuGUI: utils.isGuiCharmUrl(charm_id)
+          };
+        },
+
+        render: function() {
+          var container = this.get('container');
+          var service = this.get('model');
+          if (!service || !service.get('loaded')) {
+            container.setHTML('<div class="alert">Loading...</div>');
+            console.log('waiting on service data');
+          } else {
+            container.setHTML(this.template(this.gatherRenderData()));
+          }
           return this;
         },
 
@@ -719,8 +752,8 @@
           container.one('#save-service-config').set('disabled', 'disabled');
 
           var new_values = utils.getElementsValuesMapping(
-                                            container, '.config-field'),
-              errors = utils.validate(new_values, schema);
+              container, '.config-field');
+          var errors = utils.validate(new_values, schema);
 
           if (Y.Object.isEmpty(errors)) {
             env.set_config(
@@ -767,8 +800,6 @@
         }
       });
 
-  views.service_config = ServiceConfigView;
-
   // Display a unit grid based on the total number of units.
   Y.Handlebars.registerHelper('show_units', function(units) {
     var template;
@@ -797,29 +828,29 @@
 
         template: Templates.service,
 
-        render: function() {
-          console.log('service view render');
-
-          var container = this.get('container'),
-              getModelURL = this.get('getModelURL'),
-              db = this.get('db'),
-              service = this.get('model'),
-              filter_state = this.get('querystring').state;
-
-          if (!service) {
-            container.setHTML('<div class="alert">Loading...</div>');
-            console.log('waiting on service data');
-            return this;
-          }
-
-          var units = db.units.get_units_for_service(service),
-              state_data = [{
-                title: 'All',
-                link: '.',
-                active: !filter_state,
-                count: this.filterUnits(null, units).length
-              }];
-
+        /**
+         * Gather up all of the data required for the template.
+         *
+         * Aside from a nice separation of concerns, this method also
+         * facilitates testing.
+         *
+         * @method gatherRenderData
+         * @return {Object} The data the template will render.
+         */
+        gatherRenderData: function() {
+          var db = this.get('db');
+          var service = this.get('model');
+          var filter_state = this.get('querystring').state;
+          var units = db.units.get_units_for_service(service);
+          var charm_id = service.get('charm');
+          var charm = db.charms.getById(charm_id);
+          var charm_attrs = charm ? charm.getAttrs() : undefined;
+          var state_data = [{
+            title: 'All',
+            link: '.',
+            active: !filter_state,
+            count: this.filterUnits(null, units).length
+          }];
           Y.each(['Running', 'Pending', 'Error'], function(title) {
             var lower = title.toLowerCase();
             state_data.push({
@@ -828,18 +859,29 @@
               count: this.filterUnits(lower, units).length,
               link: '?state=' + lower});
           }, this);
-          var charm_id = service.get('charm'),
-              charm = db.charms.getById(charm_id),
-              charm_attrs = charm ? charm.getAttrs() : undefined;
-          container.setHTML(this.template(
-              { viewName: 'units',
-                tabs: this.getServiceTabs('.'),
-                service: service.getAttrs(),
-                charm_id: charm_id,
-                charm: charm_attrs,
-                state: filter_state,
-                units: this.filterUnits(filter_state, units),
-                states: state_data}));
+          return {
+            viewName: 'units',
+            tabs: this.getServiceTabs('.'),
+            service: service.getAttrs(),
+            charm_id: charm_id,
+            charm: charm_attrs,
+            serviceIsJujuGUI: utils.isGuiCharmUrl(charm_id),
+            state: filter_state,
+            units: this.filterUnits(filter_state, units),
+            states: state_data
+          };
+        },
+
+        render: function() {
+          console.log('service view render');
+          var container = this.get('container');
+          var service = this.get('model');
+          if (!service || !service.get('loaded')) {
+            container.setHTML('<div class="alert">Loading...</div>');
+            console.log('waiting on service data');
+          } else {
+            container.setHTML(this.template(this.gatherRenderData()));
+          }
           return this;
         },
 
 
=== modified file 'app/views/topology/service.js'
--- app/views/topology/service.js	2013-01-15 22:49:05 +0000
+++ app/views/topology/service.js	2013-01-17 20:33:22 +0000
@@ -75,6 +75,11 @@
             var topo = context.get('component');
             var box = topo.get('active_service');
             var service = topo.serviceForBox(box);
+            // The user is not allowed to destroy the Juju GUI service because
+            // it would break the application they are currently using.
+            if (utils.isGuiService(service)) {
+              return;
+            }
             context.service_click_actions
               .toggleControlPanel(box, context);
             context.service_click_actions
@@ -738,21 +743,31 @@
      */
     service_click_actions: {
       /*
-       * Default action: show or hide control panel.
+       * Default action: show or hide control panel (service menu).
+       *
+       * @method toggleControlPanel
+       * @param {Object} service The service in question (if any).
+       * @param {Object} view The current view.
+       * @param {object} context The active context.
        */
-      toggleControlPanel: function(m, view, context) {
-        var container = view.get('container'),
-            topo = view.get('component'),
-            cp = container.one('#service-menu');
+      toggleControlPanel: function(service, view, context) {
+        var menu = view.get('container').one('#service-menu'),
+            topo = view.get('component');
 
-        if (cp.hasClass('active') || !m) {
-          cp.removeClass('active');
+        if (menu.hasClass('active') || !service) {
+          menu.removeClass('active');
           topo.set('active_service', null);
           topo.set('active_context', null);
+          // Most services can be destroyed via the GUI.
+          menu.one('.destroy-service').removeClass('disabled');
         } else {
-          topo.set('active_service', m);
+          topo.set('active_service', service);
           topo.set('active_context', context);
-          cp.addClass('active');
+          menu.addClass('active');
+          // We do not want the user destroying the Juju GUI service.
+          if (utils.isGuiService(service)) {
+            menu.one('.destroy-service').addClass('disabled');
+          }
           view.updateServiceMenuLocation();
         }
       },
 
=== modified file 'app/views/utils.js'
--- app/views/utils.js	2013-01-15 09:43:29 +0000
+++ app/views/utils.js	2013-01-17 20:33:22 +0000
@@ -906,6 +906,68 @@
     return result;
   };
 
+  /**
+   * Determine if a service is the Juju GUI by inspecting the charm URL.
+   *
+   * @method isGuiCharmUrl
+   * @param {String} charmUrl The service to inspect.
+   * @return {Boolean} True if the charm URL is that of the Juju GUI.
+   */
+  utils.isGuiCharmUrl = function(charmUrl) {
+    // Regular expression to match charm URLs.  Explanation generated by
+    // http://rick.measham.id.au/paste/explain.pl and then touched up a bit to
+    // fit in our maximum line width.  Note that the bit about an optional
+    // newline matched by $ is a Perlism that JS does not share.
+    //
+    // NODE                     EXPLANATION
+    // ------------------------------------------------------------------------
+    //   ^                        the beginning of the string
+    // ------------------------------------------------------------------------
+    //   (?:                      group, but do not capture:
+    // ------------------------------------------------------------------------
+    //     [^:]+                    any character except: ':' (1 or more
+    //                              times (matching the most amount
+    //                              possible))
+    // ------------------------------------------------------------------------
+    //     :                        ':'
+    // ------------------------------------------------------------------------
+    //   )                        end of grouping
+    // ------------------------------------------------------------------------
+    //   (?:                      group, but do not capture (0 or more times
+    //                            (matching the most amount possible)):
+    // ------------------------------------------------------------------------
+    //     [^\/]+                   any character except: '\/' (1 or more
+    //                              times (matching the most amount
+    //                              possible))
+    // ------------------------------------------------------------------------
+    //     \/                       '/'
+    // ------------------------------------------------------------------------
+    //   )*                       end of grouping
+    // ------------------------------------------------------------------------
+    //   juju-gui-                'juju-gui-'
+    // ------------------------------------------------------------------------
+    //   \d+                      digits (0-9) (1 or more times (matching
+    //                            the most amount possible))
+    // ------------------------------------------------------------------------
+    //   $                        before an optional \n, and the end of the
+    //                            string
+    return (/^(?:[^:]+:)(?:[^\/]+\/)*juju-gui-\d+$/).test(charmUrl);
+  };
+
+  /**
+   * Determine if a service is the Juju GUI by inspecting the charm URL.
+   *
+   * @method isGuiService
+   * @param {Object} candidate The service to inspect.
+   * @return {Boolean} True if the service is the Juju GUI.
+   */
+  utils.isGuiService = function(candidate) {
+    // Some candidates have the charm URL as an attribute, others require a
+    // "get".
+    var charmUrl = candidate.charm || candidate.get('charm');
+    return utils.isGuiCharmUrl(charmUrl);
+  };
+
   Y.Handlebars.registerHelper('unitState', function(relation_errors,
       agent_state) {
         if ('started' === agent_state && relation_errors &&
 
=== modified file 'lib/views/stylesheet.less'
--- lib/views/stylesheet.less	2013-01-10 10:25:55 +0000
+++ lib/views/stylesheet.less	2013-01-17 20:33:22 +0000
@@ -96,8 +96,15 @@
 }
 
 .view-container {
-  overflow: auto;
-  padding: 10px;
+    overflow: auto;
+    padding: 10px;
+
+    .control-group {
+        .warning-message {
+            color: red;
+            padding: 1em;
+        }
+    }
 }
 
 .navbar {
@@ -291,11 +298,6 @@
             }
             &.add-remove-units {
                 background-image: url(/juju-ui/assets/images/icons/icon_noshadow_relation.png);
-                color: gray;
-                cursor: default;
-                &:hover {
-                    background-color: inherit;
-                }
             }
             &.destroy-service {
                 background-image: url(/juju-ui/assets/images/icons/icon_noshadow_destroy.png);
@@ -307,6 +309,13 @@
             &:hover {
                 background-color: lighten(@background_color, 10%);
             }
+            &.disabled {
+                color: gray;
+                cursor: default;
+                &:hover {
+                    background-color: inherit;
+                }
+            }
         }
 
     }
 
=== modified file 'test/index.html'
--- test/index.html	2013-01-16 09:06:43 +0000
+++ test/index.html	2013-01-17 20:33:22 +0000
@@ -54,6 +54,7 @@
   <script src="test_unit_view.js"></script>
   <script src="test_utils.js"></script>
   <script src="test_viewport_module.js"></script>
+  <script src="test_templates.js"></script>
 
 
   <script>
 
=== modified file 'test/test_application_notifications.js'
--- test/test_application_notifications.js	2013-01-15 10:56:19 +0000
+++ test/test_application_notifications.js	2013-01-17 20:33:22 +0000
@@ -104,6 +104,8 @@
              if ('unit_count' === key) {
                // We simulate a model with 2 units
                return 2;
+             } else if ('loaded' === key) {
+               return true;
              }
              return null;
            }
 
=== modified file 'test/test_service_view.js'
--- test/test_service_view.js	2012-12-21 20:28:25 +0000
+++ test/test_service_view.js	2013-01-17 20:33:22 +0000
@@ -1,6 +1,10 @@
 'use strict';
 
 (function() {
+  // XXX This "describe" is a lie.  There are tests here for both the service
+  // view (views.service) and the relations view (views.service_relations).
+  // This test suite needs to be broken out into two that just contain tests
+  // for one or the other.
   describe('juju service view', function() {
     var models, Y, container, service, db, conn, env, charm, ENTER, ESC,
         makeServiceView, makeServiceRelationsView, views, unit;
@@ -670,5 +674,112 @@
       assert.equal('error2', filtered[1].testKey);
     });
 
-  });
-}) ();
+    it('tells the template if the service is the GUI (service)', function() {
+      var view = makeServiceView();
+      var renderData = view.gatherRenderData();
+      assert.equal(renderData.serviceIsJujuGUI, false);
+    });
+
+    it('tells the template if the service is the GUI (relations)', function() {
+      // This would ideally be in its own suite; see the XXX at top of this
+      // file.
+      var view = makeServiceRelationsView();
+      var renderData = view.gatherRenderData();
+      assert.equal(renderData.serviceIsJujuGUI, false);
+    });
+
+    it('loading message if the service is not loaded (service)', function() {
+      var view = makeServiceView();
+      view.get('model').set('loaded', false);
+      view.render();
+      var html = container.getHTML();
+      assert.match(html, /Loading\.\.\./);
+    });
+
+    it('loading message if the service is not loaded (relations)', function() {
+      // This would ideally be in its own suite; see the XXX at top of this
+      // file.
+      var view = makeServiceRelationsView();
+      view.get('model').set('loaded', false);
+      view.render();
+      var html = container.getHTML();
+      assert.match(html, /Loading\.\.\./);
+    });
+
+  });
+})();
+
+(function() {
+  describe('Service config view (views.service_config)', function() {
+    var models, Y, container, service, db, conn, env, charm, views, view;
+
+    before(function(done) {
+      Y = YUI(GlobalConfig).use(
+          'juju-views', 'juju-models', 'base', 'node', 'json-parse',
+          'juju-env', 'node-event-simulate', 'juju-tests-utils', 'event-key',
+          function(Y) {
+            models = Y.namespace('juju.models');
+            views = Y.namespace('juju.views');
+            done();
+          });
+    });
+
+    beforeEach(function(done) {
+      conn = new (Y.namespace('juju-tests.utils')).SocketStub(),
+      env = new (Y.namespace('juju')).Environment({conn: conn});
+      env.connect();
+      conn.open();
+      container = Y.Node.create('<div/>').hide();
+      Y.one('#main').append(container);
+      db = new models.Database();
+      charm = new models.Charm({id: 'cs:precise/mysql-7', description: 'A DB'});
+      db.charms.add([charm]);
+      service = new models.Service(
+          { id: 'mysql',
+            charm: 'cs:precise/mysql-7',
+            unit_count: db.units.size(),
+            loaded: true,
+            exposed: false});
+
+      db.services.add([service]);
+      view = new views.service_config({
+        db: db,
+        env: env,
+        getModelURL: function(model, intent) {
+          return model.get('name');
+        },
+        model: service,
+        container: container
+      });
+      done();
+    });
+
+    afterEach(function(done) {
+      container.remove(true);
+      service.destroy();
+      db.destroy();
+      env.destroy();
+      done();
+    });
+
+    it('displays a loading message if the service is not loaded', function() {
+      view.get('model').set('loaded', false);
+      view.render();
+      var html = container.getHTML();
+      assert.match(html, /Loading\.\.\./);
+    });
+
+    it('displays no loading message if the service is loaded', function() {
+      view.get('model').set('loaded', true);
+      view.render();
+      var html = container.getHTML();
+      assert.notMatch(html, /Loading\.\.\./);
+    });
+
+    it('informs the template if the service is the GUI', function() {
+      var renderData = view.gatherRenderData();
+      assert.equal(renderData.serviceIsJujuGUI, false);
+    });
+
+  });
+})();
 
=== added file 'test/test_templates.js'
--- test/test_templates.js	1970-01-01 00:00:00 +0000
+++ test/test_templates.js	2013-01-17 20:33:22 +0000
@@ -0,0 +1,128 @@
+'use strict';
+
+(function() {
+
+  describe('Template: service-footer-destroy-service.partial', function() {
+    var requires = ['node', 'juju-gui', 'juju-views', 'juju-tests-utils'];
+    var Y, conn, env, partial;
+
+    before(function(done) {
+      Y = YUI(GlobalConfig).use(requires, function(Y) {
+        var partials = Y.Handlebars.partials;
+        partial = partials['service-footer-destroy-service'];
+        done();
+      });
+    });
+
+    it('does not display a "Destroy" button for the Juju GUI', function() {
+      // Disallow foot-shooting.
+      var html = partial({serviceIsJujuGUI: true});
+      assert.notMatch(html, /Destroy/);
+    });
+
+    it('does display a "Destroy" button for other services', function() {
+      var html = partial({serviceIsJujuGUI: false});
+      assert.match(html, /Destroy/);
+    });
+
+  });
+})();
+
+(function() {
+
+  describe('Template: service-footer-common-controls.partial', function() {
+    var requires = ['node', 'juju-gui', 'juju-views', 'juju-tests-utils'];
+    var Y, conn, env, partial;
+
+    before(function(done) {
+      Y = YUI(GlobalConfig).use(requires, function(Y) {
+        var partials = Y.Handlebars.partials;
+        partial = partials['service-footer-common-controls'];
+        done();
+      });
+    });
+
+    it('includes unit count UI for the Juju GUI service', function() {
+      var html = partial({serviceIsJujuGUI: true});
+      assert.match(html, /Unit count/);
+    });
+
+    it('does not include (un)expose for the Juju GUI service', function() {
+      var html = partial({serviceIsJujuGUI: true});
+      assert.notMatch(html, /Expose/);
+    });
+
+    it('includes unit count UI for non-Juju-GUI services', function() {
+      var html = partial({serviceIsJujuGUI: false});
+      assert.match(html, /Unit count/);
+    });
+
+    it('includes (un)expose for non-Juju-GUI services', function() {
+      var html = partial({serviceIsJujuGUI: false});
+      assert.match(html, /Expose/);
+    });
+
+
+  });
+})();
+
+(function() {
+
+  describe('Template: service-config.handlebars', function() {
+    var requires = ['node', 'juju-gui', 'juju-views', 'juju-tests-utils'];
+    var warningMessage = /Warning: Changing the settings/;
+    var Y, conn, env, template;
+
+    before(function(done) {
+      Y = YUI(GlobalConfig).use(requires, function(Y) {
+        var templates = Y.namespace('juju.views').Templates;
+        template = templates['service-config'];
+        done();
+      });
+    });
+
+    it('includes a warning about reconfiguring the Juju GUI', function() {
+      // Reconfiguring the Juju GUI service could break it, causing the user to
+      // lose access to the app they are in the process of using.
+      var html = template({serviceIsJujuGUI: true});
+      assert.match(html, warningMessage);
+    });
+
+    it('does not warn about about reconfiguring other services', function() {
+      var html = template({serviceIsJujuGUI: false});
+      assert.notMatch(html, warningMessage);
+    });
+
+  });
+})();
+
+(function() {
+
+  describe('Template: service.handlebars', function() {
+    var requires = ['node', 'juju-gui', 'juju-views', 'juju-tests-utils'];
+    var warningMessage = 'Warning: Changing the settings';
+    var divider = '<img class="divider"';
+    var Y, conn, env, template;
+
+    before(function(done) {
+      Y = YUI(GlobalConfig).use(requires, function(Y) {
+        var templates = Y.namespace('juju.views').Templates;
+        template = templates.service;
+        done();
+      });
+    });
+
+    it('does not show the destroy or expose UI for Juju GUI', function() {
+      var html = template({serviceIsJujuGUI: true, units: []});
+      assert.notMatch(html, /Destroy/);
+      assert.notMatch(html, /Expose/);
+    });
+
+    it('shows the destroy or expose UI for non-Juju-GUI services', function() {
+      var html = template({serviceIsJujuGUI: false, units: []});
+      assert.match(html, /Destroy/);
+      assert.match(html, /Expose/);
+    });
+
+  });
+})();
 
=== modified file 'test/test_topology.js'
--- test/test_topology.js	2013-01-09 17:17:46 +0000
+++ test/test_topology.js	2013-01-17 20:33:22 +0000
@@ -1,4 +1,3 @@
-
 'use strict';
 
 describe('topology', function() {
@@ -94,6 +93,75 @@
        Y.Lang.isValue(topo.vis).should.equal(true);
      });
 
-});
-
-
+  it('should prevent the Juju GUI service from being destroyed', function() {
+    var service = {
+      charm: 'cs:precise/juju-gui-7'
+    };
+    var fauxTopo = {
+      get: function() {
+        return null;
+      },
+      serviceForBox: function() {
+        return service;
+      }
+    };
+    // The context is used to do the destroying, so if it does not have the
+    // destroy method, an exception will be raised if the service would be
+    // destroyed.
+    var context = {
+      get: function(name) {
+        if (name === 'component') {
+          return fauxTopo;
+        }
+      }
+    };
+    topo.events.ServiceModule.scene['.destroy-service'].click.callback(
+        undefined, context);
+  });
+});
+
+describe('service menu', function() {
+  var Y, views;
+
+  before(function(done) {
+    Y = YUI(GlobalConfig).use(['juju-topology'], function(Y) {
+      views = Y.namespace('juju.views');
+      done();
+    });
+  });
+
+  it('should disable the "Destroy" menu for the Juju GUI service', function() {
+    var service = {
+      charm: 'cs:precise/juju-gui-7'
+    };
+    var addedClassName;
+    var menu = {
+      hasClass: function() {
+        return false;
+      },
+      addClass: function() {},
+      one: function() {
+        return {
+          addClass: function(className) {
+            addedClassName = className;
+          }
+        };
+      }
+    };
+    var fauxView = {
+      get: function(name) {
+        if (name === 'container') {
+          return {one: function() { return menu; }};
+        } else if (name === 'component') {
+          return {set: function() {}};
+        }
+      },
+      updateServiceMenuLocation: function() {}
+    };
+    var view = new views.ServiceModule();
+    view.service_click_actions.toggleControlPanel(
+        service, fauxView, undefined);
+    assert.equal(addedClassName, 'disabled');
+  });
+
+});
 
=== modified file 'test/test_utils.js'
--- test/test_utils.js	2012-10-12 23:52:48 +0000
+++ test/test_utils.js	2013-01-17 20:33:22 +0000
@@ -339,3 +339,70 @@
 
   });
 })();
+
+(function() {
+  describe('utils.isGuiService', function() {
+
+    var utils, Y;
+
+    before(function(done) {
+      Y = YUI(GlobalConfig).use('juju-views', function(Y) {
+        utils = Y.namespace('juju.views.utils');
+        done();
+      });
+    });
+
+    it('should extract values from "charm" attribute', function() {
+      var candidate = {charm: 'cs:precise/juju-gui-7'};
+      assert.isTrue(utils.isGuiService(candidate));
+    });
+
+    it('should extract values from .get("charm")', function() {
+      var candidate = {
+        get: function(name) {
+          if (name === 'charm') {
+            return 'cs:precise/juju-gui-7';
+          }
+        }
+      };
+      assert.isTrue(utils.isGuiService(candidate));
+    });
+
+  });
+})();
+
+(function() {
+  describe('utils.isGuiCharmUrl', function() {
+
+    var utils, Y;
+
+    before(function(done) {
+      Y = YUI(GlobalConfig).use('juju-views', function(Y) {
+        utils = Y.namespace('juju.views.utils');
+        done();
+      });
+    });
+
+    it('should recognize charm store URLs', function() {
+      assert.isTrue(utils.isGuiCharmUrl('cs:precise/juju-gui-7'));
+    });
+
+    it('should recognize unofficial charm store URLs', function() {
+      assert.isTrue(utils.isGuiCharmUrl('cs:~foobar/precise/juju-gui-7'));
+    });
+
+    it('should ignore owners of unofficial charm store URLs', function() {
+      assert.isFalse(utils.isGuiCharmUrl('cs:~juju-gui/precise/foobar-7'));
+    });
+
+    it('should recognize local charm URLs', function() {
+      assert.isTrue(utils.isGuiCharmUrl('local:juju-gui-3'));
+    });
+
+    it('should not allow junk on the end of the URL', function() {
+      assert.isFalse(utils.isGuiCharmUrl('local:juju-gui-3 ROFLCOPTR!'));
+    });
+
+  });
+})();
+