yellow team mailing list archive
-
yellow team
-
Mailing list archive
-
Message #01419
[Merge] lp:~gary/juju-gui/relatedcharms into lp:juju-gui
Gary Poster has proposed merging lp:~gary/juju-gui/relatedcharms into lp:juju-gui.
Requested reviews:
Juju GUI Hackers (juju-gui)
For more details, see:
https://code.launchpad.net/~gary/juju-gui/relatedcharms/+merge/133004
Add related charms section to charm description
This adds a related charms section to the charm panel, per UX design.
It adds new functionality to the charm store find method in order to make
the search efficient.
Tests for the view code are not factored as I had intended: I wanted stubs to
test the composite functions in isolation. Kapil nixed this in favor of
test-all-the-way-through test functions as I have them here.
Thanks.
https://codereview.appspot.com/6814089/
--
https://code.launchpad.net/~gary/juju-gui/relatedcharms/+merge/133004
Your team Juju GUI Hackers is requested to review the proposed merge of lp:~gary/juju-gui/relatedcharms into lp:juju-gui.
=== modified file 'app/store/charm.js'
--- app/store/charm.js 2012-10-23 20:02:17 +0000
+++ app/store/charm.js 2012-11-06 02:12:19 +0000
@@ -23,11 +23,22 @@
// CharmId compare function.
find: function(query, options) {
if (!Y.Lang.isString(query)) {
- var tmp = [];
+ var operator = query.op || 'intersection',
+ join_string = {union: ' OR ', intersection: ' '}[operator],
+ tmp = [];
+ delete query.op;
+ if (!Y.Lang.isValue(join_string)) {
+ throw 'Developer error: unknown operator ' + operator;
+ }
Y.each(query, function(val, key) {
- tmp.push(key + ':' + val);
+ if (Y.Lang.isString(val)) {
+ val = [val];
+ }
+ Y.each(val, function(v) {
+ tmp.push(key + ':' + v);
+ });
});
- query = escape(tmp.join(' '));
+ query = escape(tmp.join(join_string));
}
this.get('datasource').sendRequest({
request: 'search/json?search_text=' + query,
=== added file 'app/templates/charm-description-related.handlebars'
--- app/templates/charm-description-related.handlebars 1970-01-01 00:00:00 +0000
+++ app/templates/charm-description-related.handlebars 2012-11-06 02:12:19 +0000
@@ -0,0 +1,15 @@
+{{#charms}}
+<div class="series-charms">
+ <h6>{{series}}</h6>
+ <ul class="unstyled">
+ {{#charms}}
+ <li class="charm-entry">
+ <div>
+ <a class="charm-detail" href="{{id}}">
+ {{#if owner}}{{owner}}/{{/if}}{{package_name}}</a>
+ </div>
+ </li>
+ {{/charms}}
+ </ul>
+</div>
+{{/charms}}
=== modified file 'app/templates/charm-description.handlebars'
--- app/templates/charm-description.handlebars 2012-11-01 13:12:28 +0000
+++ app/templates/charm-description.handlebars 2012-11-06 02:12:19 +0000
@@ -15,7 +15,11 @@
<h5>Description</h5>
<div>{{description}}</div>
<h5>Owner</h5>
+ {{#if owner}}
<p>{{owner}}</p>
+ {{else}}
+ <p>[Official charm]</p>
+ {{/if}}
{{#if store_revision}}
<h5>Store Revision</h5>
<p>{{store_revision}}</p>
@@ -57,5 +61,13 @@
</div>
</div>
{{/if}}
+ {{#any requires provides}}
+ <div class="charm-section">
+ <h4><i class="icon-chevron-up"></i>Related Charms</h4>
+ <div class="collapsible" id="related-charms">
+ Loading...
+ </div>
+ </div>
+ {{/any}}
</div>
</div>
=== modified file 'app/views/charm-panel.js'
--- app/views/charm-panel.js 2012-11-01 14:42:29 +0000
+++ app/views/charm-panel.js 2012-11-06 02:12:19 +0000
@@ -31,7 +31,8 @@
var el = ev.currentTarget.ancestor('.charm-section')
.one('.collapsible'),
icon = ev.currentTarget.one('i');
- if (el.getStyle('height') === '0px') {
+ // clientHeight and offsetHeight are not as reliable in tests.
+ if (parseInt(el.getStyle('height'), 10) === 0) {
el.show('sizeIn', {duration: 0.25, width: null});
icon.replaceClass('icon-chevron-up', 'icon-chevron-down');
} else {
@@ -60,6 +61,47 @@
scrollHeight = height - diff - clientDiff;
scrollContainer.setStyle('height', scrollHeight + 'px');
}
+ },
+ /**
+ Given a set of grouped entries as returned by the charm store "find"
+ method, returns the same data but with the charms converted into data
+ objects that are more amenable to rendering with handlebars.
+
+ @method makeRenderableResults
+ @static
+ @private
+ @param {Array} entries An ordered collection of groups of charms, as
+ returned by the charm store "find" method.
+ @return {Array} An ordered collection of groups of charm data.
+ */
+ makeRenderableResults = function(entries) {
+ return entries.map(
+ function(data) {
+ return {
+ series: data.series,
+ charms: data.charms.map(
+ function(charm) { return charm.getAttrs(); })
+ };
+ });
+ },
+ /**
+ Given an array of interface data as stored in a charm's "required"
+ and "provided" attributes, return an array of interface names.
+
+ @method getInterfaces
+ @static
+ @private
+ @param {Array} data A collection of interfaces as stored in a charm's
+ "required" and "provided" attributes.
+ @return {Array} A collection of interface names extracted from the
+ input.
+ */
+ getInterfaces = function(data) {
+ if (data) {
+ return Y.Array.map(
+ Y.Object.values(data),
+ function(val) { return val['interface']; });
+ }
};
var CharmCollectionView = Y.Base.create('CharmCollectionView', Y.View, [], {
@@ -125,15 +167,7 @@
defaultEntries = this.get('defaultEntries'),
resultEntries = this.get('resultEntries'),
raw_entries = searchText ? resultEntries : defaultEntries,
- entries = raw_entries && raw_entries.map(
- function(data) {
- return {
- series: data.series,
- charms: data.charms.map(
- function(charm) { return charm.getAttrs(); })
- };
- }
- );
+ entries = raw_entries && makeRenderableResults(raw_entries);
container.setHTML(this.template({ charms: entries }));
this._setScroll();
return this;
@@ -154,6 +188,14 @@
scrollContainer.setStyle('height', height + 'px');
}
},
+ /**
+ Fires an event indicating that the charm panel should switch to the
+ "description" for a given charm.
+
+ @method showDetails
+ @param {Object} ev An event object (with a "halt" method).
+ @return {undefined} Sends a signal only.
+ */
showDetails: function(ev) {
ev.halt();
this.fire(
@@ -262,10 +304,12 @@
var CharmDescriptionView = Y.Base.create(
'CharmDescriptionView', Y.View, [views.JujuBaseView], {
template: views.Templates['charm-description'],
+ relatedTemplate: views.Templates['charm-description-related'],
events: {
'.charm-nav-back': {click: 'goBack'},
'.btn': {click: 'deploy'},
- '.charm-section h4': {click: toggleSectionVisibility}
+ '.charm-section h4': {click: toggleSectionVisibility},
+ 'a.charm-detail': {click: 'showDetails'}
},
initializer: function() {
this.bindModelView(this.get('model'));
@@ -280,6 +324,10 @@
el.ancestor('.charm-section').one('div')
.setStyle('height', '0px');
});
+ var slot = container.one('#related-charms');
+ if (slot) {
+ this.getRelatedCharms(charm, slot);
+ }
} else {
container.setHTML(
'<div class="alert">Waiting on charm data...</div>');
@@ -288,6 +336,87 @@
return this;
},
/**
+ Get related charms and render them in the provided node. Typically
+ this is asynchronous, waiting on charm store results.
+
+ @method
+ @param {Object} charm A charm model. Finds charms related to the
+ required and provided interfaces of this charm.
+ @param {Object} slot An YUI node that will contain the results (using
+ setHTML).
+ @return {undefined} Mutates slot only.
+ */
+ getRelatedCharms: function(charm, slot) {
+ var store = this.get('charmStore'),
+ defaultSeries = this.get('defaultSeries'),
+ list = this.get('charms'),
+ self = this,
+ query = {
+ op: 'union',
+ requires: getInterfaces(charm.get('provides')),
+ provides: getInterfaces(charm.get('requires'))
+ };
+ if (query.requires || query.provides) {
+ store.find(
+ query,
+ /**
+ If the charm we searched for is still the same as the view's
+ charm, ask renderRelatedCharms to render the results. If they
+ differ, discard the results, because they are no longer
+ relevant.
+ */
+ { success: function(related) {
+ if (charm === self.get('model')) {
+ self.renderRelatedCharms(related, slot);
+ }
+ },
+ /**
+ If there was a failure, render it to the console and to the
+ notifications section.
+ */
+ failure: function(e) {
+ console.error(e.error);
+ self.get('db').notifications.add(
+ new models.Notification({
+ title: 'Could not retrieve charm data',
+ message: e.error,
+ level: 'error'
+ })
+ );
+ },
+ defaultSeries: defaultSeries,
+ list: list
+ }
+ );
+ } else {
+ slot.setHTML('None');
+ }
+ },
+ /**
+ Given a grouped list of related charms such as those returned by the
+ charm store's "find" method, and a node into which the results should
+ be rendered, renders the results into HTML and sets that into the
+ node.
+
+ @method
+ @param {Array} related A list of grouped charms such as those returned
+ by the charm store's "find" method.
+ @param {Object} slot A node into which the results should be rendered.
+ @return {undefined} Mutates only.
+ */
+ renderRelatedCharms: function(related, slot) {
+ if (related.length) {
+ slot.setHTML(this.relatedTemplate(
+ {charms: makeRenderableResults(related)}));
+ // Make container big enough if it is open.
+ if (slot.get('clientHeight') > 0) {
+ slot.show('sizeIn', {duration: 0.25, width: null});
+ }
+ } else {
+ slot.setHTML('None');
+ }
+ },
+ /**
When the view's "height" attribute is set, adjust the internal
scrollable div to have the appropriate height.
@@ -298,16 +427,48 @@
_setScroll: function() {
setScroll(this.get('container'), this.get('height'));
},
+ /**
+ Fires an event indicating that the charm panel should switch to the
+ "charms" search result view.
+
+ @method goBack
+ @param {Object} ev An event object (with a "halt" method).
+ @return {undefined} Sends a signal only.
+ */
goBack: function(ev) {
ev.halt();
this.fire('changePanel', { name: 'charms' });
},
+ /**
+ Fires an event indicating that the charm panel should switch to the
+ "configuration" panel for the current charm.
+
+ @method deploy
+ @param {Object} ev An event object (with a "halt" method).
+ @return {undefined} Sends a signal only.
+ */
deploy: function(ev) {
ev.halt();
this.fire(
'changePanel',
{ name: 'configuration',
charmId: ev.currentTarget.getData('url')});
+ },
+ /**
+ Fires an event indicating that the charm panel should switch to the
+ same "description" panel but with a new charm. This is used by the
+ "related charms" links.
+
+ @method showDetails
+ @param {Object} ev An event object (with a "halt" method).
+ @return {undefined} Sends a signal only.
+ */
+ showDetails: function(ev) {
+ ev.halt();
+ this.fire(
+ 'changePanel',
+ { name: 'description',
+ charmId: ev.target.getAttribute('href') });
}
});
@@ -487,6 +648,14 @@
}
return;
},
+ /**
+ Fires an event indicating that the charm panel should switch to the
+ "charms" search result view.
+
+ @method goBack
+ @param {Object} ev An event object (with a "halt" method).
+ @return {undefined} Sends a signal only.
+ */
goBack: function(ev) {
ev.halt();
this.fire('changePanel', { name: 'charms' });
@@ -586,7 +755,9 @@
descriptionPanel = new CharmDescriptionView(
{ container: descriptionPanelNode,
env: app.env,
- db: app.db}),
+ db: app.db,
+ charms: charms,
+ charmStore: charmStore }),
configurationPanelNode = Y.Node.create(),
configurationPanel = new CharmConfigurationView(
{ container: configurationPanelNode,
@@ -606,33 +777,31 @@
container.hide();
function setPanel(config) {
- if (config.name !== activePanelName) {
- var newPanel = panels[config.name];
- if (!Y.Lang.isValue(newPanel)) {
- throw 'Developer error: Unknown panel name ' + config.name;
- }
- activePanelName = config.name;
- container.get('children').remove();
- container.append(panels[config.name].get('container'));
- newPanel.set('height', calculatePanelPosition().height);
- if (config.charmId) {
- newPanel.set('model', null); // Clear out the old.
- var charm = charms.getById(config.charmId);
- if (charm.loaded) {
- newPanel.set('model', charm);
- } else {
- charm.load(charmStore, function(err, response) {
- if (err) {
- console.log('error loading charm', response);
- newPanel.fire('changePanel', {name: 'charms'});
- } else {
- newPanel.set('model', charm);
- }
- });
- }
- } else { // This is the search panel.
- newPanel.render();
- }
+ var newPanel = panels[config.name];
+ if (!Y.Lang.isValue(newPanel)) {
+ throw 'Developer error: Unknown panel name ' + config.name;
+ }
+ activePanelName = config.name;
+ container.get('children').remove();
+ container.append(panels[config.name].get('container'));
+ newPanel.set('height', calculatePanelPosition().height);
+ if (config.charmId) {
+ newPanel.set('model', null); // Clear out the old.
+ var charm = charms.getById(config.charmId);
+ if (charm.loaded) {
+ newPanel.set('model', charm);
+ } else {
+ charm.load(charmStore, function(err, response) {
+ if (err) {
+ console.log('error loading charm', response);
+ newPanel.fire('changePanel', {name: 'charms'});
+ } else {
+ newPanel.set('model', charm);
+ }
+ });
+ }
+ } else { // This is the search panel.
+ newPanel.render();
}
}
@@ -761,6 +930,7 @@
node: container,
setDefaultSeries: function(series) {
charmsSearchPanel.set('defaultSeries', series);
+ descriptionPanel.set('defaultSeries', series);
}
};
}
=== modified file 'lib/views/stylesheet.less'
--- lib/views/stylesheet.less 2012-11-05 19:36:26 +0000
+++ lib/views/stylesheet.less 2012-11-06 02:12:19 +0000
@@ -875,6 +875,11 @@
white-space: pre-wrap;
}
}
+ h6 {
+ font-weight: bold;
+ text-transform: uppercase;
+ color: black
+ }
}
&.config-variant {
padding: 1ex 1em;
=== modified file 'test/test_charm_panel.js'
--- test/test_charm_panel.js 2012-11-01 13:21:53 +0000
+++ test/test_charm_panel.js 2012-11-06 02:12:19 +0000
@@ -65,7 +65,7 @@
charm_store: new juju.CharmStore({datasource: {
sendRequest: function(params) {
searchTriggered = true;
- // Mocking the server callback value
+ // Stubbing the server callback value
params.callback.success({
response: {
results: [{
@@ -154,7 +154,7 @@
describe('charm description', function() {
var Y, models, views, juju, conn, env, container, db, app, charm,
- charm_store_data, charm_store;
+ charm_store_data, charm_store, charms;
before(function(done) {
Y = YUI(GlobalConfig).use(
@@ -187,9 +187,10 @@
Y.one('#main').append(container);
db = new models.Database();
charm = db.charms.add({ id: 'cs:precise/mysql-7' });
- charm_store_data = [];
+ charms = new models.CharmList(),
+ charm_store_data = {responseText: '{}'};
charm_store = new juju.CharmStore(
- {datasource: new Y.DataSource.Local({source: charm_store_data})});
+ {datasource: new Y.DataSource.Local({source: [charm_store_data]})});
app = { db: db, env: env, charm_store: charm_store };
});
@@ -208,15 +209,18 @@
it('can render incomplete charm', function() {
var view = new views.CharmDescriptionView(
- { container: container, app: app, model: charm }).render(),
+ { container: container, app: app, model: charm,
+ charmStore: charm_store }).render(),
html = container.one('.charm-description'),
description_div = html.one('.charm-section div'),
interface_div = html.one('div.charm-section:nth-of-type(2)'),
- last_change_div = html.one('div.charm-section:nth-of-type(3)');
+ last_change_div = html.one('div.charm-section:nth-of-type(3)'),
+ related_div = html.one('div.charm-section:nth-of-type(4)');
html.one('h3').get('text').trim().should.equal('mysql');
description_div.getStyle('height').should.not.equal('0px');
var _ = expect(interface_div).to.not.exist;
_ = expect(last_change_div).to.not.exist;
+ _ = expect(related_div).to.not.exist;
});
it('can render fuller charm', function() {
@@ -227,18 +231,28 @@
{ created: 1349797266.032,
committer: 'fred',
message: 'fixed EVERYTHING'}});
+ charm_store_data.responseText = Y.JSON.stringify(
+ { matches: 1,
+ results: [
+ { store_url: 'cs:precise/superthing-7',
+ summary: 'A super thing.'}]});
var view = new views.CharmDescriptionView(
- { container: container, app: app, model: charm }).render(),
+ { container: container, app: app, model: charm, charms: charms,
+ charmStore: charm_store }).render(),
html = container.one('.charm-description'),
description_div = html.one('.charm-section div'),
interface_div = html.one('div.charm-section:nth-of-type(2) div'),
- last_change_div = html.one('div.charm-section:nth-of-type(3) div');
+ last_change_div = html.one('div.charm-section:nth-of-type(3) div'),
+ related_div = html.one('div.charm-section:nth-of-type(4)');
description_div.get('text').should.contain('A DB');
interface_div.getStyle('height').should.equal('0px');
interface_div.get('text').should.contain('munin');
last_change_div.getStyle('height').should.equal('0px');
last_change_div.get('text').should.contain('fixed EVERYTHING');
last_change_div.get('text').should.contain('2012-10-09');
+ related_div.one('a').getAttribute('href').should.equal(
+ 'cs:precise/superthing-7');
+ related_div.one('a').get('text').trim().should.equal('superthing');
});
it('can toggle visibility of subsections', function() {
@@ -250,8 +264,10 @@
committer: 'fred',
message: 'fixed EVERYTHING'}});
var view = new views.CharmDescriptionView(
- { container: container, app: app, model: charm }).render(),
+ { container: container, app: app, model: charm,
+ charmStore: charm_store }).render(),
html = container.one('.charm-description'),
+ // We use the last change div.
section_container = html.one('div.charm-section:nth-of-type(3)');
section_container.one('div').getStyle('height').should.equal('0px');
assert(section_container.one('h4 i').hasClass('icon-chevron-up'));
@@ -265,7 +281,8 @@
it('can respond to the "back" link', function(done) {
var view = new views.CharmDescriptionView(
- { container: container, app: app, model: charm }).render();
+ { container: container, app: app, model: charm,
+ charmStore: charm_store }).render();
view.on('changePanel', function(ev) {
ev.name.should.equal('charms');
done();
=== modified file 'test/test_charm_store.js'
--- test/test_charm_store.js 2012-10-23 19:54:44 +0000
+++ test/test_charm_store.js 2012-11-06 02:12:19 +0000
@@ -100,6 +100,64 @@
'search/json?search_text=' + escape('foo:bar sha:zam'));
});
+ it('sends a proper request for a hash call of array to find', function() {
+ var args;
+ charm_store.set('datasource', {
+ sendRequest: function(params) {
+ args = params;
+ }
+ });
+ charm_store.find({foo: ['bar', 'baz', 'bing'], sha: 'zam'}, {});
+ args.request.should.equal(
+ 'search/json?search_text=' +
+ escape('foo:bar foo:baz foo:bing sha:zam'));
+ });
+
+ it('sends a proper request for a hash union call to find', function() {
+ var args;
+ charm_store.set('datasource', {
+ sendRequest: function(params) {
+ args = params;
+ }
+ });
+ charm_store.find(
+ {foo: ['bar', 'baz', 'bing'], sha: 'zam', op: 'union'}, {});
+ args.request.should.equal(
+ 'search/json?search_text=' +
+ escape('foo:bar OR foo:baz OR foo:bing OR sha:zam'));
+ });
+
+ it('sends a proper request for a hash intersection call to find',
+ function() {
+ var args;
+ charm_store.set('datasource', {
+ sendRequest: function(params) {
+ args = params;
+ }
+ });
+ charm_store.find(
+ {foo: ['bar', 'baz', 'bing'], sha: 'zam', op: 'intersection'}, {});
+ args.request.should.equal(
+ 'search/json?search_text=' +
+ escape('foo:bar foo:baz foo:bing sha:zam'));
+ });
+
+ it('throws an error with unknown operator', function() {
+ var args;
+ charm_store.set('datasource', {
+ sendRequest: function(params) {
+ args = params;
+ }
+ });
+ try {
+ charm_store.find(
+ {foo: ['bar', 'baz', 'bing'], sha: 'zam', op: 'fiddly'}, {});
+ assert.fail('should have thrown an error');
+ } catch (e) {
+ e.should.equal('Developer error: unknown operator fiddly');
+ }
+ });
+
it('processes and orders search text requests properly', function(done) {
// This is data from
// http://jujucharms.com/search/json?search_text=cassandra .
=== modified file 'undocumented'
--- undocumented 2012-11-01 14:35:58 +0000
+++ undocumented 2012-11-06 02:12:19 +0000
@@ -67,9 +67,7 @@
app/views/charm-panel.js _showErrors
app/views/charm-panel.js calculatePanelPosition
app/views/charm-panel.js createInstance
-app/views/charm-panel.js deploy
app/views/charm-panel.js getInstance
-app/views/charm-panel.js goBack
app/views/charm-panel.js hide
app/views/charm-panel.js hideDescription
app/views/charm-panel.js initializer
@@ -88,7 +86,6 @@
app/views/charm-panel.js show
app/views/charm-panel.js showConfiguration
app/views/charm-panel.js showDescription
-app/views/charm-panel.js showDetails
app/views/charm-panel.js toggle
app/views/charm-panel.js updatePanelPosition
app/views/charm.js _deployCallback
Follow ups
