Merge lp:~adeuring/launchpad/bug-903852 into lp:launchpad
Status: | Merged |
---|---|
Approved by: | Aaron Bentley |
Approved revision: | no longer in the source branch. |
Merged at revision: | 14525 |
Proposed branch: | lp:~adeuring/launchpad/bug-903852 |
Merge into: | lp:launchpad |
Diff against target: |
570 lines (+247/-148) 7 files modified
lib/lp/bugs/browser/bugtask.py (+29/-1) lib/lp/bugs/browser/tests/test_bugtask.py (+16/-0) lib/lp/bugs/interfaces/bugtask.py (+3/-7) lib/lp/bugs/javascript/buglisting_utils.js (+5/-1) lib/lp/bugs/javascript/tests/test_buglisting_utils.js (+70/-30) lib/lp/bugs/model/bugtask.py (+93/-92) lib/lp/bugs/templates/bugtask-macros-tableview.pt (+31/-17) |
To merge this branch: | bzr merge lp:~adeuring/launchpad/bug-903852 |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Aaron Bentley (community) | Approve | ||
Review via email: mp+85651@code.launchpad.net |
Commit message
[r=abentley][bug=903852] add JS sort buttons for all sort orders implemented by BugTaskSet.search()
Description of the change
The new JS widgets to change the sort order and to select the displyed data
break at present when a bug listing page is accessed with an "unexpected"
sort order. This can happen for example when a user selects "sort by
numbers of users affected" in the "Advanced search" page. An example URL
for the bug:
https:/
(make sure that the feature bugs.dynamic_
The cause is simple: The JS code in
lp/bugs/
does not define sort buttons for all sort orders implemented by
BugTaskSet.search()
A related problem: Generally, we want to show only sort buttons for data
that is also displayed on the page. A strict implementation of this rule
would be confusing for users: If the data for current sort order is not
displayed, users would have no indication what the sort order is (well,
the< could have a look at the URL, but that's not obvious to all users,
and it can be cumbersome to find the "orderby" parameter in complex
queries).
Hence there is already an exemption to this rule: The button for the
current sort order is never hidden. But the button disappears when
the users selects a different sort order.
This branch adds another exemption: If the data for the sort order
selected during page load can never be displayed, the sort button will
always be displayed during the "Lifetime" of the page view.
Implementation details:
I moved the definition of the set of all sort buttons (parameter
sort_keys for Y.lp.ordering.
lp.browser.bugtask; this data is added in an initialize() call to the
JSON cache.
This makes it possible to write a test that sort_keys (or more precisely,
the new constant lp.browser.
sort options implemented by BugTaskSet.search()
(test_sort_
This required another, mostly mechanical, change: The SQL expressions
for the ORDER BY clause were stored in BugTaskSet.
which is not initialized during module load to avoid circular imports.
Instead, this dictionary was initilalized in the method
BugTaskSet.
cached property BugTaskSet.
test_sort_
tests:
./bin/test bugs -vvt test_bugtask.
./bin/test bugs -vvt test_buglisting
lint:
Checking for conflicts and issues in changed files.
Linting changed files:
lib/lp/
lib/lp/
lib/lp/
lib/lp/
lib/lp/
lib/lp/
lib/lp/
./lib/lp/
674: not well-formed (invalid token)
make: *** [lint] Fehler 1
This is caused by the '<' in
for (var i = 0; i < sort_keys.length; i++)
This complaint could be avoided by
for (var i = 0; sort_keys.length > i; i++)
but at least for me the former variant is easier for "mental parsing".
Nice improvement.
A couple of small tweaks: isUndefined( )" (or isValue) rather than "!== undefined".
- Our style guide recommends "!Y.Lang.
- You should be able to avoid the lint warning, yet write a readable loop using Y.each.