Inspector Factory & Single Inspector

app/views/environment.js

 /*
 This file is part of the Juju GUI, which lets users view and manage Juju
 environments within a graphical interface (https://launchpad.net/juju-gui).
 Copyright (C) 2012-2013 Canonical Ltd.
 
 This program is free software: you can redistribute it and/or modify it under
 the terms of the GNU Affero General Public License version 3, as published by
 the Free Software Foundation.
 
 This program is distributed in the hope that it will be useful, but WITHOUT
@@ skipping 82 matching lines @@
           } else {
             this._inspectors[name] = inspector;
           }
           return this;
         },
 
         /**
           Creates a new service inspector instance of the passed in type.
 
           @method createServiceInspector
-          @param {String} type of inspector to create (ghost, service).
           @param {Y.Model} model service or charm depending on inspector type.
           @param {Object} config object of options to overwrite default config.
         */
-        createServiceInspector: function(type, model, config) {
+        createServiceInspector: function(model, config) {
           config = config || {};
+          var type = 'service',
+              charm = this.get('db').charms.getById(model.get('charm'));
 
           // This method is called with a charm or service depending on if it's
           // called from the charm browser or from the environment. If it is
           // called from the environment with a ghost it still needs access to
           // the charm so that's what this switcheroo is doing here.

[gary.poster, 2013/06/28 13:11:54]

Nice simple fix, thanks

[jeff.pihach, 2013/06/28 14:42:09]

was contemplating a 'there be dragons' warning here somewhere :)

           if (model.get('pending')) {
             type = 'ghost';
             config.ghostService = model;

[benjamin.saller, 2013/06/28 17:07:17]

You do this dance explicitly when calling this method, I think its fine to do it
only here.

[jeff.pihach, 2013/06/28 17:42:26]

Done.

-            model = this.get('db').charms.getById(model.get('charm'));
+            model = charm;
           }
 
           // If the user is trying to open the same inspector twice
           if (this.getInspector(model.get('id'))) {
             return;
           }
 
           var serviceInspector = this.getInspector(model.get('id'));

[gary.poster, 2013/06/28 13:11:54]

hm.  does this mean that you can't have two ghost services running the same
charm?  That would be a bug that needs to be addressed.

[jeff.pihach, 2013/06/28 14:42:09]

Correct this limits that - working on fix now.

[benjamin.saller, 2013/06/28 17:07:17]

They would have to have different ids computed to render in the first place. I
think ghostDeploy does this properly.

           if (serviceInspector) { return serviceInspector; }
 
           var combinedConfig = {};
           var configs = this._generateConfigs(model);
 
           if (type === 'ghost') {
             combinedConfig = Y.mix(configs.configBase, configs.configGhost,
                                    true, undefined, 0, true);
           } else if (type === 'service') {
             combinedConfig = Y.mix(configs.configBase, configs.configService,
                                    true, undefined, 0, true);
-          } else {
-            console.log('Service inspector type not supported.');

[gary.poster, 2013/06/28 13:11:54]

It would be interesting to use Google Analytics to send errors like this for us
to review.  Not for this branch, just a thought.

[jeff.pihach, 2013/06/28 14:42:09]

Agreed!

[benjamin.saller, 2013/06/28 17:07:17]

This particular error goes away if we sniff the type from the model. But point
taken, almost any console.log could potentially become a custom event.

[jeff.pihach, 2013/06/28 17:42:26]

Done.

-            return;
           }
 
           Y.mix(combinedConfig, config, true, undefined, 0, true);
 
           serviceInspector = new views.ServiceInspector(model, combinedConfig);
 
+          // Because the inspector can trigger it's own destruction we need to
+          // listen for the event and remove it from the list of open inspectors
           serviceInspector.inspector.after('destroy', function(e) {
             this.setInspector(e.currentTarget, true);

[gary.poster, 2013/06/28 13:11:54]

comment as to motive would help me read this.

[jeff.pihach, 2013/06/28 14:42:09]

Done.

           }, this);
 
           // Restrict to a single inspector instance
           if (Y.Object.size(this._inspectors) >= 1) {
             Y.Object.each(this._inspectors, function(inspector) {
               inspector.inspector.destroy();
             });
           }
 
           this.setInspector(serviceInspector);
@@ skipping 164 matching lines @@
 }, '0.1.0', {
   requires: ['juju-templates',
              'juju-view-utils',
              'juju-models',
              'juju-topology',
              'base-build',
              'handlebars-base',
              'node',
              'view']
 });