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: [

_______________________________________________
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to     : [email protected]
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help   : https://help.launchpad.net/ListHelp

Reply via email to