launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging lp:~cjwatson/launchpad/refactor-bugtasksearch-orderby-expression into lp:launchpad.

Commit message:
Stop importing lp.bugs.model.bugtasksearch into lp.bugs.browser.buglisting.

The browser code already defines SORT_KEYS, which must be kept in sync with
orderby_expression in any event and will do just as well for this check.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/refactor-bugtasksearch-orderby-expression/+merge/325619
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/refactor-bugtasksearch-orderby-expression into lp:launchpad.

=== modified file 'lib/lp/bugs/browser/buglisting.py'
--- lib/lp/bugs/browser/buglisting.py	2015-09-29 18:02:29 +0000
+++ lib/lp/bugs/browser/buglisting.py	2017-06-14 08:04:29 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2014 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2017 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """IBugTask-related browser views."""
@@ -107,7 +107,6 @@
     )
 from lp.bugs.interfaces.bugtracker import IHasExternalBugTracker
 from lp.bugs.interfaces.malone import IMaloneApplication
-from lp.bugs.model.bugtasksearch import orderby_expression
 from lp.layers import FeedsLayer
 from lp.registry.interfaces.distribution import IDistribution
 from lp.registry.interfaces.distributionsourcepackage import (
@@ -863,7 +862,7 @@
 
 
 # All sort orders supported by BugTaskSet.search() and a title for
-# them.
+# them.  Keep in sync with lp.bugs.model.bugtasksearch.orderby_expression.
 SORT_KEYS = [
     ('importance', 'Importance', 'desc'),
     ('status', 'Status', 'asc'),
@@ -1155,14 +1154,13 @@
                     "are out of date or you changed the URL by hand?" %
                     field_name)
 
+        sort_column_names = set(sort_key[0] for sort_key in SORT_KEYS)
         orderby = get_sortorder_from_request(self.request)
         for orderby_col in orderby:
             if orderby_col.startswith("-"):
                 orderby_col = orderby_col[1:]
 
-            try:
-                orderby_expression[orderby_col]
-            except KeyError:
+            if orderby_col not in sort_column_names:
                 raise UnexpectedFormData(
                     "Unknown sort column '%s'" % orderby_col)
 

=== modified file 'lib/lp/bugs/model/bugtasksearch.py'
--- lib/lp/bugs/model/bugtasksearch.py	2016-07-28 10:34:52 +0000
+++ lib/lp/bugs/model/bugtasksearch.py	2017-06-14 08:04:29 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2016 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2017 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 __metaclass__ = type
@@ -111,6 +111,7 @@
 bug_join = (Bug, Join(Bug, Bug.id == BugTaskFlat.bug_id))
 bugtask_join = (
     BugTask, Join(BugTask, BugTask.id == BugTaskFlat.bugtask_id))
+# Keep in sync with lp.bugs.browser.buglisting.SORT_KEYS.
 orderby_expression = {
     "task": (BugTaskFlat.bugtask_id, []),
     "id": (BugTaskFlat.bug_id, []),