launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #05520
[Merge] lp:~abentley/launchpad/next-prev into lp:launchpad
Aaron Bentley has proposed merging lp:~abentley/launchpad/next-prev into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #890745 in Launchpad itself: "dynamic bug listings don't handle next and prev correctly on first and last batch"
https://bugs.launchpad.net/launchpad/+bug/890745
For more details, see:
https://code.launchpad.net/~abentley/launchpad/next-prev/+merge/82308
= Summary =
Fix bug #890745: dynamic bug listings don't handle next and prev correctly on first and last batch
== Proposed fix ==
Use "null" rather than "undefined"
== Pre-implementation notes ==
None
== Implementation details ==
Many tests had to be changed because they relied on next/prev being undefined.
== Tests ==
bin/test -t test_buglisting.html
== Demo and Q/A ==
Go to a +bugs page. Enable firebug. Click "previous". There should be no error in the log. Click "Last", "Next". There should be no error in the log.
= Launchpad lint =
Checking for conflicts and issues in changed files.
Linting changed files:
lib/lp/bugs/javascript/buglisting.js
lib/lp/bugs/javascript/tests/test_buglisting.js
--
https://code.launchpad.net/~abentley/launchpad/next-prev/+merge/82308
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~abentley/launchpad/next-prev into lp:launchpad.
=== modified file 'lib/lp/bugs/javascript/buglisting.js'
--- lib/lp/bugs/javascript/buglisting.js 2011-11-14 16:10:10 +0000
+++ lib/lp/bugs/javascript/buglisting.js 2011-11-15 20:00:32 +0000
@@ -244,7 +244,7 @@
*/
next_batch: function() {
var current_batch = this.get('current_batch');
- if (current_batch.next === undefined){
+ if (current_batch.next === null){
return;
}
this.update({
@@ -260,7 +260,7 @@
*/
prev_batch: function() {
var current_batch = this.get('current_batch');
- if (current_batch.prev === undefined){
+ if (current_batch.prev === null){
return;
}
this.update({
=== modified file 'lib/lp/bugs/javascript/tests/test_buglisting.js'
--- lib/lp/bugs/javascript/tests/test_buglisting.js 2011-11-14 15:54:07 +0000
+++ lib/lp/bugs/javascript/tests/test_buglisting.js 2011-11-15 20:00:32 +0000
@@ -16,7 +16,7 @@
var navigator = new module.ListingNavigator({
current_url: 'http://yahoo.com?foo=bar&start=1&memo=2&;' +
'direction=3&orderby=4',
- cache: {}});
+ cache: {next: null, prev: null}});
Y.lp.testing.assert.assert_equal_structure(
{foo: 'bar'}, navigator.get('search_params'));
},
@@ -26,7 +26,9 @@
current_url: '',
cache: {
field_visibility: {show_item: true},
- mustache_model: {bugtasks: [{show_item: true} ] }
+ mustache_model: {bugtasks: [{show_item: true} ] },
+ next: null,
+ prev: null
}
});
bugtask = navigator.get('current_batch').mustache_model.bugtasks[0];
@@ -39,7 +41,9 @@
var model = {
field_visibility: {show_item: true},
mustache_model: {bugtasks: []},
- memo: 1
+ memo: 1,
+ next: null,
+ prev: null
};
var navigator = new module.ListingNavigator({
current_url: '',
@@ -50,7 +54,9 @@
var batch = {
mustache_model: {
bugtasks: [{show_item: true}]},
- memo: 2
+ memo: 2,
+ next: null,
+ prev: null
};
var query = navigator.get_batch_query(batch);
navigator.update_from_new_model(query, batch);
@@ -216,7 +222,9 @@
mustache_model: {
foo: 'bar',
bugtasks: []
- }
+ },
+ next: null,
+ prev: null
};
var target = Y.Node.create('<div id="client-listing"></div>');
var navigator = new module.ListingNavigator({
@@ -244,7 +252,9 @@
order_by: 'intensity',
start: 0,
forwards: true,
- memo: null
+ memo: null,
+ next: null,
+ prev: null
});
return navigator;
},
@@ -288,7 +298,9 @@
},
mustache_model: {
foo: 'bar'
- }
+ },
+ next: null,
+ prev: null
};
var template = "<ol>" +
"{{#item}}<li>{{name}}</li>{{/item}}</ol>";
@@ -356,7 +368,7 @@
test_get_batch_query_orderby: function() {
var navigator = new module.ListingNavigator({
current_url: '?param=1',
- cache: {}
+ cache: {next: null, prev: null}
});
var query = navigator.get_batch_query({order_by: 'importance'});
Y.Assert.areSame('importance', query.orderby);
@@ -368,7 +380,7 @@
test_get_batch_query_memo: function() {
var navigator = new module.ListingNavigator({
current_url: '?param=foo',
- cache: {}
+ cache: {next: null, prev: null}
});
var query = navigator.get_batch_query({memo: 'pi'});
Y.Assert.areSame('pi', query.memo);
@@ -380,7 +392,7 @@
test_get_batch_null_memo: function() {
var navigator = new module.ListingNavigator({
current_url: '?memo=foo',
- cache: {}
+ cache: {next: null, prev: null}
});
var query = navigator.get_batch_query({memo: null});
Y.Assert.areSame(undefined, query.memo);
@@ -391,7 +403,7 @@
test_get_batch_query_forwards: function() {
var navigator = new module.ListingNavigator({
current_url: '?param=pi&direction=backwards',
- cache: {}
+ cache: {next: null, prev: null}
});
var query = navigator.get_batch_query({forwards: true});
Y.Assert.areSame('pi', query.param);
@@ -403,7 +415,7 @@
test_get_batch_query_backwards: function() {
var navigator = new module.ListingNavigator({
current_url: '?param=pi',
- cache: {}
+ cache: {next: null, prev: null}
});
var query = navigator.get_batch_query({forwards: false});
Y.Assert.areSame('pi', query.param);
@@ -415,7 +427,7 @@
test_get_batch_query_start: function() {
var navigator = new module.ListingNavigator({
current_url: '?start=pi',
- cache: {}
+ cache: {next: null, prev:null}
});
var query = navigator.get_batch_query({});
Y.Assert.areSame(undefined, query.start);
@@ -426,8 +438,11 @@
}
}));
-var get_navigator = function(url) {
+var get_navigator = function(url, config) {
var mock_io = new Y.lp.testing.mockio.MockIo();
+ if (config === undefined){
+ config = {};
+ }
lp_cache = {
context: {
resource_type_link: 'http://foo_type',
@@ -444,6 +459,12 @@
order_by: 'foo',
last_start: 23
};
+ if (config.no_next){
+ lp_cache.next = null;
+ }
+ if (config.no_prev){
+ lp_cache.prev = null;
+ }
var target = Y.Node.create('<div id="client-listing"></div>');
return new module.ListingNavigator({
current_url: url,
@@ -499,8 +520,7 @@
* Calling next_batch when there is none is a no-op.
*/
test_next_batch_missing: function() {
- var navigator = get_navigator('?memo=pi&start=26');
- delete navigator.get('current_batch').next;
+ var navigator = get_navigator('?memo=pi&start=26', {no_next: true});
navigator.next_batch();
Y.Assert.areSame(
null, navigator.get('io_provider').last_request);
@@ -523,8 +543,8 @@
* Calling prev_batch when there is none is a no-op.
*/
test_prev_batch_missing: function() {
- var navigator = get_navigator('?memo=pi&start=26');
- delete navigator.get('current_batch').prev;
+ var navigator = get_navigator(
+ '?memo=pi&start=26', {no_prev: true, no_next: true});
navigator.prev_batch();
Y.Assert.areSame(
null, navigator.get('io_provider').last_request);
@@ -546,7 +566,9 @@
navigator.get('batches')['some-batch-key'] = {
mustache_model: {
bugtasks: []
- }
+ },
+ next: null,
+ prev: null
};
navigator.update_from_cache({foo: 'bar'}, 'some-batch-key');
Y.Assert.areEqual('some-batch-key', e.newVal.batch_key);
@@ -563,7 +585,9 @@
mustache_model: {
bugtasks: [],
foo: 'bar'
- }
+ },
+ next: null,
+ prev: null
};
navigator.set('template', '{{foo}}');
navigator.get('batches')['some-batch-key'] = batch;
@@ -579,7 +603,9 @@
setUp: function() {
window.LP = {
cache: {
- current_batch: {}
+ current_batch: {},
+ next: null,
+ prev: null
}
};
},
