launchpad-reviewers team mailing list archive

[Abel Deuring <abel.deuring@xxxxxxxxxxxxx>]

Abel Deuring has proposed merging lp:~adeuring/launchpad/bug-901016 into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #901016 in Launchpad itself: ""order by" buttons are in a different order to the displayed columns"
  https://bugs.launchpad.net/launchpad/+bug/901016

For more details, see:
https://code.launchpad.net/~adeuring/launchpad/bug-901016/+merge/85902

This branch fixes bug 901016: "order by" buttons are in a different order to the displayed columns.

The fix is simple: The parameters for the "order by" buttons are defined in the list lp.bugs.browser.bugtask:SORTKEYS; the elemnts of this list just have to be reordered.

When I compared the sort buttons with the displayed data, I noticed that the button "sort by title" was gone. I introduced this bug in an earlier branch (lp:~adeuring/launchpad/history-model), which intended to hide sort buttons for data that is not displyed, and to show sort buttons for data that is displayed. The decision to show or hide a button is based on the value of data_visibility.show_something in the function update_sort_button_visibility() (file buglistings_utils.js) -- and I forgot that there is no flag data_visibility.show_title, because the title of a bug is always displayed.

There was already an exception to the rule "show only sort buttons for data that is displayed": On the "advanced search" page, users can select an ordering hat does not correspond o any data displayed on a bug listing page. If users select such a sort order, the related sort button is always displayed (attribute orderbybar.always_display in update_sort_button_visibility()). I changed this attribute from a string to a list of strings, and 'title' now always apperas in this list.

I could not figure out a good test that the sort buttons are properly ordered... The change of orderbybar.always_display is reflected in a change of test_update_sort_button_visibility_extra_sort_order() in test_buglisting_utils.js:

./bin/test bugs -vvt test_buglisting_utils

I also removed some cruft from an earlier branch.
-- 
https://code.launchpad.net/~adeuring/launchpad/bug-901016/+merge/85902
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~adeuring/launchpad/bug-901016 into lp:launchpad.

=== modified file 'lib/lp/bugs/browser/bugtask.py'
--- lib/lp/bugs/browser/bugtask.py	2011-12-15 14:27:15 +0000
+++ lib/lp/bugs/browser/bugtask.py	2011-12-15 16:13:35 +0000
@@ -2481,18 +2481,18 @@
 # All sort orders supported by BugTaskSet.search() and a title for
 # them.
 SORT_KEYS = [
+    ('importance', 'Importance'),
+    ('status', 'Status'),
     ('id', 'Bug number'),
     ('title', 'Bug title'),
-    ('importance', 'Importance'),
-    ('status', 'Status'),
-    ('heat', 'Bug heat'),
-    ('reporter', 'Reporter'),
-    ('assignee', 'Assignee'),
     ('targetname', 'Package/Project/Series name'),
     ('milestone_name', 'Milestone'),
+    ('date_last_updated', 'Date bug last updated'),
+    ('assignee', 'Assignee'),
+    ('reporter', 'Reporter'),
     ('datecreated', 'Bug age'),
-    ('date_last_updated', 'Date bug last updated'),
     ('tag', 'Bug Tags'),
+    ('heat', 'Bug heat'),
     ('date_closed', 'Date bug closed'),
     ('dateassigned', 'Date when the bug task was assigned'),
     ('number_of_duplicates', 'Number of duplicates'),

=== modified file 'lib/lp/bugs/javascript/buglisting_utils.js'
--- lib/lp/bugs/javascript/buglisting_utils.js	2011-12-14 15:16:11 +0000
+++ lib/lp/bugs/javascript/buglisting_utils.js	2011-12-15 16:13:35 +0000
@@ -340,10 +340,11 @@
         }
         // Never hide the button for the current sort order...
         orderby_visibility[orderbybar.get('active')] = true;
-        // ...and for the sort order that should always be displayed.
-        if (!Y.Lang.isUndefined(orderbybar.always_display)) {
-            orderby_visibility[orderbybar.always_display] = true;
-        }
+        // ...and buttons for sort orders that should always be displayed.
+        Y.each(orderbybar.always_display, function(sort_key) {
+            orderby_visibility[sort_key] = true;
+        });
+        Y.log(data_visibility);
         orderbybar.updateVisibility(orderby_visibility);
     }
 

=== modified file 'lib/lp/bugs/javascript/tests/test_buglisting_utils.js'
--- lib/lp/bugs/javascript/tests/test_buglisting_utils.js	2011-12-14 12:30:53 +0000
+++ lib/lp/bugs/javascript/tests/test_buglisting_utils.js	2011-12-15 16:13:35 +0000
@@ -526,7 +526,7 @@
             sort_order: 'desc'
         });
         orderby.render();
-        orderby.always_display = 'users_affected_count';
+        orderby.always_display = ['users_affected_count'];
         var data_visibility = this.getDefaultDataVisibility();
         Y.lp.buglisting_utils.update_sort_button_visibility(
             orderby, data_visibility);

=== modified file 'lib/lp/bugs/templates/bugtask-macros-tableview.pt'
--- lib/lp/bugs/templates/bugtask-macros-tableview.pt	2011-12-15 14:27:15 +0000
+++ lib/lp/bugs/templates/bugtask-macros-tableview.pt	2011-12-15 16:13:35 +0000
@@ -672,14 +672,6 @@
                 sort_order = 'desc';
             }
             var unknown_sort_key = true;
-            /*xxxxxxxxxxxxxxx
-            for (var i = 0; i < sort_keys.length; i++) {
-                if (sort_keys[i][0] === active_sort_key) {
-                    unknown_sort_key = false;
-                    break;
-                }
-            }
-            */
             Y.each(sort_keys, function(sort_key) {
                 if (sort_key[0] === active_sort_key) {
                     unknown_sort_key = false;
@@ -716,11 +708,12 @@
             });
             var field_visibility =
                 navigator.get('model').get_field_visibility();
+            orderby.always_display = ['title'];
             // The advanced search page contains sort options that have
             // no related data fields we can display. If a user has selected
             // such a sort order, this sort option should always be visible.
             if (field_visibility['show_' + active_sort_key] === undefined) {
-                orderby.always_display = active_sort_key;
+                orderby.always_display.push(active_sort_key);
             }
             Y.lp.buglisting_utils.update_sort_button_visibility(
                  orderby, field_visibility);