launchpad-reviewers team mailing list archive

[Richard Harding <rick.harding@xxxxxxxxxxxxx>]

Richard Harding has proposed merging lp:~rharding/launchpad/sort_buttons_907376 into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #907376 in Launchpad itself: "Order By Bar "buttons" do not appear on page load"
  https://bugs.launchpad.net/launchpad/+bug/907376

For more details, see:
https://code.launchpad.net/~rharding/launchpad/sort_buttons_907376/+merge/86579

= Summary = 
Chrome pops the history which causes an extra event to hit the event monitoring the history for changes so that the sort buttons are updated.

== Implementation ==
When Chrome pops the extra event it's missing all the data we normally pass to the event such as the batch_key and the list of fields and their visibility. If the event is missing those details, we just skip updating the UI. There's nothing we can act upon in the event firing.

== Tests ==
There's no direct tests of the macro that catching this event

== QA ==
Load the bug listing in Chrome and select all of the fields as visible. Then reload the page, all of the sort buttons should appear correctly.

== Lint ==

Linting changed files:
  lib/lp/bugs/templates/bugtask-macros-tableview.pt
-- 
https://code.launchpad.net/~rharding/launchpad/sort_buttons_907376/+merge/86579
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~rharding/launchpad/sort_buttons_907376 into lp:launchpad.

=== modified file 'lib/lp/bugs/templates/bugtask-macros-tableview.pt'
--- lib/lp/bugs/templates/bugtask-macros-tableview.pt	2011-12-15 21:16:41 +0000
+++ lib/lp/bugs/templates/bugtask-macros-tableview.pt	2011-12-21 16:05:26 +0000
@@ -694,9 +694,16 @@
             var model = navigator.get('model');
             model.get('history').after(
                 'change', function(e) {
-                     Y.lp.buglisting_utils.update_sort_button_visibility(
-                         orderby, e.newVal);
-                });
+                    // Only update the sort buttons if we've got a valid batch
+                    // key.
+                    if (Y.Object.has_key(e.newVal, 'batch_key')) {
+                        Y.lp.buglisting_utils.update_sort_button_visibility(
+                            orderby,
+                            e.newVal
+                        );
+                    }
+                 }
+            );
             var config_node = orderby.get('config_node');
             var list_util = new Y.lp.buglisting_utils.BugListingConfigUtil({
                 srcNode: config_node,