launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #05298
[Merge] lp:~abentley/launchpad/update-listings into lp:launchpad
Aaron Bentley has proposed merging lp:~abentley/launchpad/update-listings into lp:launchpad with lp:~abentley/launchpad/mustache-bugs as a prerequisite.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~abentley/launchpad/update-listings/+merge/79998
= Summary =
Implement dynamic reordering of bug listings. Part of: https://dev.launchpad.net/Projects/CustomBugListings
== Proposed fix ==
Use the ++model++ to retrieve listings with an updated ordering. Preserve all other query parameters.
== Pre-implementation notes ==
== Implementation details ==
Implemented get_query to retrieve the current query in structured form. Updated load_model to accept query parameters. Implemented update_listing to render a specified ordering.
Changed buglisting-default.pt so that the default rendering is temporarily forced to 'status'. (This is only needed until the ordering widget is implemented.)
== Tests ==
xvfb-run bin/test -t test_buglisting.html
== Demo and Q/A ==
Enable the feature flag. Go to a bug listing. The 'client-side mustache' listing should show the bugs ordered by status, whatever specified order. Regardless of order, it should show the same bugs.
= Launchpad lint =
Checking for conflicts and issues in changed files.
Linting changed files:
lib/lp/bugs/javascript/tests/test_buglisting.html
lib/lp/app/javascript/lp.js
lib/lp/bugs/javascript/buglisting.js
lib/lp/app/javascript/testing/assert.js
lib/lp/app/javascript/client.js
lib/lp/bugs/templates/buglisting-default.pt
lib/lp/bugs/browser/tests/test_bugtask.py
lib/lp/bugs/browser/bugtask.py
lib/lp/bugs/templates/buglisting.mustache
lib/lp/app/javascript/testing/mockio.js
lib/lp/bugs/templates/bugs-listing-table-without-navlinks.pt
lib/lp/bugs/javascript/tests/test_buglisting.js
./lib/lp/app/javascript/lp.js
52: Expected '!==' and instead saw '!='.
145: Don't make functions within a loop.
154: Expected '===' and instead saw '=='.
171: Expected '===' and instead saw '=='.
171: Expected '===' and instead saw '=='.
183: Expected '===' and instead saw '=='.
213: Expected '===' and instead saw '=='.
--
https://code.launchpad.net/~abentley/launchpad/update-listings/+merge/79998
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~abentley/launchpad/update-listings into lp:launchpad.
=== modified file 'lib/lp/app/javascript/client.js'
--- lib/lp/app/javascript/client.js 2011-10-11 17:05:30 +0000
+++ lib/lp/app/javascript/client.js 2011-10-20 20:30:31 +0000
@@ -190,11 +190,17 @@
* @param {Entry} entry
* @param {String} view_name
* @param {String} namespace
+ * @param {String} query (optional) structured query variables to use.
* @return {String} URL
*/
-module.get_view_url = function(entry, view_name, namespace){
+module.get_view_url = function(entry, view_name, namespace, query){
entry_url = Y.lp.get_url_path(entry.get('web_link'));
- return entry_url + '/' + view_name + '/++' + namespace + '++';
+ querystring = Y.QueryString.stringify(query);
+ if (querystring !== '') {
+ querystring = '?' + querystring;
+ }
+ return (
+ entry_url + '/' + view_name + '/++' + namespace + '++' + querystring);
};
@@ -210,8 +216,16 @@
};
-module.load_model = function(entry, view_name, config){
- var url = module.get_view_url(entry, view_name, 'model');
+/**
+ * Load the model for a view.
+ *
+ * @param entry An Entry, i.e. a Lanchpad API object
+ * @param view_name The name of the view to retrieve the model for
+ * @param config An IO config.
+ * @param query (optional) The structured query variables to use.
+ */
+module.load_model = function(entry, view_name, config, query){
+ var url = module.get_view_url(entry, view_name, 'model', query);
var old_on_success = config.on.success;
var on = config.on;
on.success = module.wrap_resource_on_success;
@@ -936,7 +950,8 @@
}, "0.1",
- {"requires":["attribute", "io", "json-parse", "json-stringify", "lp"]});
+ {"requires":["attribute", "io", "querystring", "json-parse",
+ "json-stringify", "lp"]});
YUI.add('lp.client.plugins', function (Y) {
=== modified file 'lib/lp/app/javascript/lp.js'
--- lib/lp/app/javascript/lp.js 2011-07-18 09:23:10 +0000
+++ lib/lp/app/javascript/lp.js 2011-10-20 20:30:31 +0000
@@ -64,10 +64,34 @@
'.collapsible', 'legend', 'legend + *', true);
};
+ /**
+ * Return a hyperlink with the specified URL.
+ */
+ var get_hyperlink = function(url){
+ var link = Y.Node.create('<a>junk</a>');
+ link.set('href', url);
+ return link;
+ };
+
+ /**
+ * Return the path portion of the specified URL.
+ */
Y.lp.get_url_path = function(url) {
- return Y.Node.create('<a>junk</a>').set('href', url).get('pathname');
- };
-}, "0.1", {"requires":["cookie", "lazr.effects", "lp.app.widgets.expander"]});
+ return get_hyperlink(url).get('pathname');
+ };
+
+ /**
+ * Return the query string of the specified URL.
+ */
+ Y.lp.get_url_query = function(url){
+ var link = get_hyperlink(url);
+ var query = link.get('search');
+ if (query.length > 0) {
+ query = query.slice(1);
+ }
+ return query;
+ };
+}, "0.1", {"requires":["cookie", "lp.app.widgets.expander"]});
// Lint-safe scripting URL.
@@ -105,8 +129,9 @@
function setFocusByName(name) {
// Focus the first element matching the given name which can be focused.
var nodes = document.getElementsByName(name);
- for (var i = 0; i < nodes.length; i++) {
- var node = nodes[i];
+ var i, node;
+ for (i = 0; i < nodes.length; i++) {
+ node = nodes[i];
if (node.focus) {
try {
// Trying to focus a hidden element throws an error in IE8.
=== added file 'lib/lp/app/javascript/testing/assert.js'
--- lib/lp/app/javascript/testing/assert.js 1970-01-01 00:00:00 +0000
+++ lib/lp/app/javascript/testing/assert.js 2011-10-20 20:30:31 +0000
@@ -0,0 +1,17 @@
+/* Copyright (c) 2011, Canonical Ltd. All rights reserved. */
+
+YUI.add('lp.testing.assert', function(Y) {
+ /**
+ * A utility module for use in YUI unit-tests with custom asserts
+ *
+ * @module lp.testing.mockio
+ */
+ var namespace = Y.namespace("lp.testing.assert");
+
+ /**
+ * Assert that two structures are equal by comparing their json form.
+ */
+ namespace.assert_equal_structure = function(expected, actual){
+ Y.Assert.areEqual(JSON.stringify(expected), JSON.stringify(actual));
+ };
+}, '0.1', {'requires': []});
=== modified file 'lib/lp/app/javascript/testing/mockio.js'
--- lib/lp/app/javascript/testing/mockio.js 2011-08-24 15:41:23 +0000
+++ lib/lp/app/javascript/testing/mockio.js 2011-10-20 20:30:31 +0000
@@ -82,6 +82,20 @@
}
};
+ /**
+ * Provide a successful JSON response
+ *
+ * @method successJSON
+ * @param {Object} json_data JSON-serializable data to provide as the
+ * response.
+ */
+ MockHttpRequest.prototype.successJSON = function(json_data){
+ this.respond({
+ responseText: Y.JSON.stringify(json_data),
+ responseHeaders: {'Content-Type': 'application/json'}
+ });
+ };
+
namespace.MockHttpRequest = MockHttpRequest;
var MockIo = function() {
=== modified file 'lib/lp/bugs/javascript/buglisting.js'
--- lib/lp/bugs/javascript/buglisting.js 2011-10-20 20:30:31 +0000
+++ lib/lp/bugs/javascript/buglisting.js 2011-10-20 20:30:31 +0000
@@ -11,13 +11,66 @@
var namespace = Y.namespace('lp.bugs.buglisting');
-namespace.rendertable = function(){
+
+/**
+ * Render bug listings via Mustache.
+ *
+ * If model is supplied, it is used as the data for rendering the listings.
+ * Otherwise, LP.cache.mustache_model is used.
+ *
+ * The template is always LP.mustache_listings.
+ */
+namespace.rendertable = function(model){
client_listing = Y.one('#client-listing');
if (client_listing === null){
return;
}
- var txt = Mustache.to_html(LP.mustache_listings, LP.cache.mustache_model);
+ if (!Y.Lang.isValue(model)){
+ model = LP.cache.mustache_model;
+ }
+ var txt = Mustache.to_html(LP.mustache_listings, model);
client_listing.set('innerHTML', txt);
};
-}, "0.1", {"requires": ["node"]});
+/**
+ * A shim to use the data of an LP.cache to render the bug listings.
+ */
+namespace.update_from_model = function(model){
+ namespace.rendertable(model.mustache_model);
+};
+
+
+/**
+ * Update the bug listings.
+ *
+ * order_by is a string specifying the sort order, as it would appear in a
+ * URL.
+ */
+namespace.update_listing = function(order_by){
+ var lp_client = new Y.lp.client.Launchpad();
+ var cache = lp_client.wrap_resource(null, LP.cache);
+ var query = namespace.get_query(window.location);
+ query.orderby = order_by;
+ load_model_config = {
+ on: {
+ success: namespace.update_from_model
+ }
+ };
+ if (Y.Lang.isValue(config)){
+ load_model_config.io_provider = config.io_provider;
+ }
+ Y.lp.client.load_model(
+ cache.context, '+bugs', load_model_config, query);
+};
+
+
+/**
+ * Return the query of the specified URL in structured form.
+ */
+namespace.get_query = function(url){
+ var querystring = Y.lp.get_url_query(url);
+ return Y.QueryString.parse(querystring);
+};
+
+
+}, "0.1", {"requires": ["node", 'lp.client']});
=== modified file 'lib/lp/bugs/javascript/tests/test_buglisting.html'
--- lib/lp/bugs/javascript/tests/test_buglisting.html 2011-10-20 20:30:31 +0000
+++ lib/lp/bugs/javascript/tests/test_buglisting.html 2011-10-20 20:30:31 +0000
@@ -9,6 +9,18 @@
<link rel="stylesheet" href="../../../app/javascript/testing/test.css" />
<script type="text/javascript"
src="../../../app/javascript/testing/testrunner.js"></script>
+ <script type="text/javascript"
+ src="../../../app/javascript/testing/assert.js"></script>
+ <script type="text/javascript"
+ src="../../../app/javascript/testing/mockio.js"></script>
+ <script type="text/javascript"
+ src="../../../app/javascript/effects/effects.js"></script>
+ <script type="text/javascript"
+ src="../../../app/javascript/expander.js"></script>
+ <script type="text/javascript"
+ src="../../../app/javascript/lp.js"></script>
+ <script type="text/javascript"
+ src="../../../app/javascript/client.js"></script>
<script type="text/javascript"
src="../../../contrib/javascript/mustache.js"></script>
=== modified file 'lib/lp/bugs/javascript/tests/test_buglisting.js'
--- lib/lp/bugs/javascript/tests/test_buglisting.js 2011-10-20 20:30:31 +0000
+++ lib/lp/bugs/javascript/tests/test_buglisting.js 2011-10-20 20:30:31 +0000
@@ -1,15 +1,21 @@
YUI({
base: '../../../../canonical/launchpad/icing/yui/',
filter: 'raw', combine: false, fetchCSS: false
- }).use('test', 'console', 'lp.bugs.buglisting', function(Y) {
+ }).use('test', 'console', 'lp.bugs.buglisting', 'lp.testing.mockio',
+ 'lp.testing.assert',
+ function(Y) {
var suite = new Y.Test.Suite("lp.bugs.buglisting Tests");
var module = Y.lp.bugs.buglisting;
/**
- * Test is_notification_level_shown() for a given set of
- * conditions.
+ * Set the HTML fixture to the desired value.
*/
+var set_fixture = function(value){
+ var fixture = Y.one('#fixture');
+ fixture.set('innerHTML', value);
+};
+
suite.add(new Y.Test.Case({
name: 'rendertable',
@@ -20,18 +26,16 @@
tearDown: function() {
delete window.LP;
- this.set_fixture('');
+ set_fixture('');
},
- set_fixture: function(value) {
- var fixture = Y.one('#fixture');
- fixture.set('innerHTML', value);
- },
test_rendertable_no_client_listing: function() {
+ // Rendering should not error with no #client-listing.
module.rendertable();
},
test_rendertable: function() {
- this.set_fixture('<div id="client-listing"></div>');
+ // Rendering should work with #client-listing supplied.
+ set_fixture('<div id="client-listing"></div>');
window.LP.cache = {
mustache_model: {
foo: 'bar'
@@ -43,6 +47,65 @@
}
}));
+suite.add(new Y.Test.Case({
+ name: 'update_listing',
+
+ setUp: function () {
+ this.MY_NAME = "ME";
+ var lp_client = new Y.lp.client.Launchpad();
+ window.LP = { links: { me: "/~" + this.MY_NAME } };
+ set_fixture('<div id="client-listing"></div>');
+ window.LP.cache = {
+ context: {
+ resource_type_link: 'http://foo_type',
+ web_link: 'http://foo/bar'
+ },
+ mustache_model: {
+ foo: 'bar'
+ }
+ };
+ window.LP.mustache_listings = "<ol>" +
+ "{{#item}}<li>{{name}}</li>{{/item}}</ol>";
+ },
+ tearDown: function() {
+ delete window.LP;
+ },
+ test_update_listing: function() {
+ /* update_listing retrieves a listing for the new ordering and
+ * displays it */
+ mock_io = new Y.lp.testing.mockio.MockIo();
+ module.update_listing('intensity', {io_provider: mock_io});
+ Y.Assert.areEqual('',
+ Y.one('#client-listing').get('innerHTML'));
+ Y.Assert.areEqual('/bar/+bugs/++model++?orderby=intensity',
+ mock_io.last_request.url);
+ mock_io.last_request.successJSON({mustache_model:
+ {item: [
+ {name: 'first'},
+ {name: 'second'}
+ ]}
+ });
+ Y.Assert.areEqual('<ol><li>first</li><li>second</li></ol>',
+ Y.one('#client-listing').get('innerHTML'));
+ }
+}));
+
+suite.add(new Y.Test.Case({
+ name: 'get_query',
+
+ setUp: function () {
+ },
+ tearDown: function() {
+ delete window.LP;
+ },
+ test_get_query: function() {
+ // get_query returns the query portion of a URL in structured form.
+ var query = module.get_query('http://yahoo.com?me=you&a=b&a=c');
+ Y.lp.testing.assert.assert_equal_structure(
+ {me: 'you', a: ['b', 'c']}, query);
+ }
+}));
+
var handle_complete = function(data) {
window.status = '::::' + JSON.stringify(data);
};
=== modified file 'lib/lp/bugs/templates/buglisting-default.pt'
--- lib/lp/bugs/templates/buglisting-default.pt 2011-10-20 20:30:31 +0000
+++ lib/lp/bugs/templates/buglisting-default.pt 2011-10-20 20:30:31 +0000
@@ -23,7 +23,7 @@
Y.on('domready', function() {
Y.lp.registry.structural_subscription.setup(
{content_box: "#structural-subscription-content-box"});
- Y.lp.bugs.buglisting.rendertable()
+ Y.lp.bugs.buglisting.update_listing('status');
});
});
</script>
