launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #05833
[Merge] lp:~abentley/launchpad/fix-spinner-bugs into lp:launchpad
Aaron Bentley has proposed merging lp:~abentley/launchpad/fix-spinner-bugs into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #900480 in Launchpad itself: "+bugs 'hanging' when no bugs found"
https://bugs.launchpad.net/launchpad/+bug/900480
Bug #900773 in Launchpad itself: "Spinner does not display on bug listings"
https://bugs.launchpad.net/launchpad/+bug/900773
For more details, see:
https://code.launchpad.net/~abentley/launchpad/fix-spinner-bugs/+merge/84633
= Summary =
Fix bug #900773: Spinner does not display on bug listings and bug #900480: +bugs 'hanging' when no bugs found
== Proposed fix ==
Use the clearProgressUI callback to clear the spinner on error, instead of calling indicator.error even when no error has occurred.
Do not attempt to initialize the ListingNavigator or any of its cohort if #client-listing is not present.
== Pre-implementation notes ==
== Implementation details ==
As well as fixing these bugs, this branch also restores the testing of the spinner, and fixes issues with pre-fetching. When update is called with the fetch-only option, the spinner should be ignored, because the progress of the fetch is not related to whether the user is waiting for something.
== Tests ==
xvfb-run bin/test -t test_buglisting.html
== Demo and Q/A ==
Go to https://bugs.qastaging.launchpad.net/launchpad and click on "Bug heat". A spinner should show while the new batch is being fetched.
https://bugs.qastaging.launchpad.net/unity/+bugs?field.tag=madnesss should not get "hung" showing the "Loading..." in the top-right corner (next to AJAX Log)
= Launchpad lint =
Checking for conflicts and issues in changed files.
Linting changed files:
lib/lp/bugs/templates/bugtask-macros-tableview.pt
lib/lp/app/javascript/indicator/indicator.js
lib/lp/bugs/javascript/buglisting.js
lib/lp/bugs/javascript/tests/test_buglisting.js
--
https://code.launchpad.net/~abentley/launchpad/fix-spinner-bugs/+merge/84633
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~abentley/launchpad/fix-spinner-bugs into lp:launchpad.
=== modified file 'lib/lp/app/javascript/indicator/indicator.js'
--- lib/lp/app/javascript/indicator/indicator.js 2011-12-02 21:47:29 +0000
+++ lib/lp/app/javascript/indicator/indicator.js 2011-12-06 16:48:38 +0000
@@ -1,7 +1,7 @@
/* Copyright 2011 Canonical Ltd. This software is licensed under the
* GNU Affero General Public License version 3 (see the file LICENSE).
*
- * Client-side rendering of bug listings.
+ * Large indicator of pending operations.
*
* @module lp.indicator
*
@@ -87,7 +87,7 @@
/**
* To prevent having to force call sites to pass in
- * parentNode, we must overridge YUI's built-in _renderUI
+ * parentNode, we must override YUI's built-in _renderUI
* method.
*
* This is a copy of the YUI method, except for using our
=== modified file 'lib/lp/bugs/javascript/buglisting.js'
--- lib/lp/bugs/javascript/buglisting.js 2011-12-01 21:58:34 +0000
+++ lib/lp/bugs/javascript/buglisting.js 2011-12-06 16:48:38 +0000
@@ -162,11 +162,15 @@
'change', this.history_changed, this);
},
- get_failure_handler: function(){
+ get_failure_handler: function(fetch_only){
var error_handler = new Y.lp.client.ErrorHandler();
error_handler.showError = Y.bind(
Y.lp.app.errors.display_error, window, null);
- this.indicator.error();
+ if (!fetch_only){
+ error_handler.clearProgressUI = Y.bind(
+ this.indicator.error, this.indicator
+ );
+ }
return error_handler.getFailureHandler();
},
@@ -333,7 +337,9 @@
* will be retrieved and cached upon retrieval.
*/
update: function(config) {
- this.indicator.setBusy();
+ if (!config.fetch_only){
+ this.indicator.setBusy();
+ }
var key = this.constructor.get_batch_key(config);
var cached_batch = this.get('batches')[key];
@@ -451,7 +457,7 @@
on: {
success: Y.bind(
this.update_from_new_model, this, query, fetch_only),
- failure: this.get_failure_handler()
+ failure: this.get_failure_handler(fetch_only)
}
};
var context = this.get_current_batch().context;
@@ -496,6 +502,9 @@
*/
namespace.ListingNavigator.from_page = function() {
var target = Y.one('#client-listing');
+ if (Y.Lang.isNull(target)){
+ return null;
+ }
var navigation_indices = Y.all('.batch-navigation-index');
var pre_fetch = get_feature_flag('bugs.dynamic_bug_listings.pre_fetch');
namespace.linkify_navigation();
=== modified file 'lib/lp/bugs/javascript/tests/test_buglisting.js'
--- lib/lp/bugs/javascript/tests/test_buglisting.js 2011-12-02 14:53:55 +0000
+++ lib/lp/bugs/javascript/tests/test_buglisting.js 2011-12-06 16:48:38 +0000
@@ -495,6 +495,18 @@
var get_navigator = function(url, config) {
var mock_io = new Y.lp.testing.mockio.MockIo();
+ if (Y.Lang.isUndefined(url)){
+ url = '';
+ }
+ if (Y.Lang.isUndefined(config)){
+ config = {};
+ }
+ var target = config.target;
+ if (!Y.Lang.isValue(target)){
+ var target_parent = Y.Node.create('<div></div>');
+ target = Y.Node.create('<div "id=#client-listing"></div>');
+ target_parent.appendChild(target);
+ }
lp_cache = {
context: {
resource_type_link: 'http://foo_type',
@@ -525,12 +537,9 @@
cache: lp_cache,
io_provider: mock_io,
pre_fetch: config.pre_fetch,
- target: config.target,
+ target: target,
template: ''
};
- if (Y.Lang.isValue(config.spinners)){
- navigator_config.spinners = config.spinners;
- }
return new module.ListingNavigator(navigator_config);
};
@@ -718,6 +727,11 @@
module.ListingNavigator.from_page();
Y.Assert.areNotSame('http://example.org/', this.getPreviousLink());
},
+ test_from_page_with_no_client: function() {
+ Y.one('#fixture').setContent('');
+ var navigator = module.ListingNavigator.from_page();
+ Y.Assert.isNull(navigator);
+ },
tearDown: function() {
Y.one('#fixture').setContent("");
delete window.LP;
@@ -844,6 +858,69 @@
}
}));
+suite.add(new Y.Test.Case({
+ name: "Test indicators",
+
+ /**
+ * Update starts showing the pending indicator
+ */
+ test_show_on_update: function(){
+ var navigator = get_navigator();
+ navigator.update({});
+ Y.Assert.isTrue(navigator.indicator.get('visible'));
+ },
+ /**
+ * A fetch-only update starts ignores the pending indicator
+ */
+ test_ignore_on_fetch_only_update: function(){
+ var navigator = get_navigator();
+ navigator.update({fetch_only: true});
+ Y.Assert.isFalse(navigator.indicator.get('visible'));
+ },
+ /**
+ * A successful IO operation clears the pending indicator.
+ */
+ test_hide_on_success: function(){
+ var navigator = get_navigator();
+ navigator.update({});
+ navigator.get('io_provider').last_request.successJSON({
+ mustache_model: {bugtasks: []}
+ });
+ Y.Assert.isFalse(navigator.indicator.get('visible'));
+ },
+ /**
+ * A successful fetch-only IO operation ignores the pending indicator.
+ */
+ test_no_hide_on_fetch_only_success: function(){
+ var navigator = get_navigator();
+ navigator.indicator.setBusy();
+ navigator.update({fetch_only: true});
+ navigator.get('io_provider').last_request.successJSON({
+ mustache_model: {bugtasks: []}
+ });
+ Y.Assert.isTrue(navigator.indicator.get('visible'));
+ },
+ /**
+ * A failed IO operation hides the pending indicator.
+ */
+ test_hide_on_failure: function(){
+ var navigator = get_navigator();
+ navigator.update({});
+ navigator.get('io_provider').failure();
+ Y.Assert.isFalse(navigator.indicator.get('visible'));
+ },
+ /**
+ * A failed fetch-only IO operation does not hide the pending indicator.
+ */
+ test_no_hide_on_fetch_only_failure: function(){
+ var navigator = get_navigator();
+ navigator.indicator.setBusy();
+ navigator.update({fetch_only: true});
+ navigator.get('io_provider').failure();
+ Y.Assert.isTrue(navigator.indicator.get('visible'));
+ }
+}));
+
var handle_complete = function(data) {
window.status = '::::' + JSON.stringify(data);
};
=== modified file 'lib/lp/bugs/templates/bugtask-macros-tableview.pt'
--- lib/lp/bugs/templates/bugtask-macros-tableview.pt 2011-12-05 16:00:27 +0000
+++ lib/lp/bugs/templates/bugtask-macros-tableview.pt 2011-12-06 16:48:38 +0000
@@ -659,6 +659,9 @@
function(Y) {
Y.on('domready', function() {
var navigator = Y.lp.bugs.buglisting.ListingNavigator.from_page();
+ if (Y.Lang.isNull(navigator)){
+ return;
+ }
var orderby = new Y.lp.ordering.OrderByBar({
srcNode: Y.one('#bugs-orderby'),
sort_keys: [
