launchpad-reviewers team mailing list archive

[Abel Deuring <abel.deuring@xxxxxxxxxxxxx>]

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

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~adeuring/launchpad/bug-sorting-2/+merge/82298

This branch adds sorting of bug tasks by bugtask assignee and bug reporter.

For both sort options, we need to join the table Person. Since we allow to sort by more than one column, aliases are used so that a sort order (assignee, reporter) does not lead to conflicts.

Deryck noted in previous review that it is ugly to call the isinstance() in _processOrderBy(), just to check if the data returned by getOrderByColumnDBName() is a tuple or a "simple" property column. I changed getOrderByColumnDBName() so that it now always returns a tuple (sort_column, list_of_required_joins).

Ah, and the second element of the tuple was before another tuple (joined_table, join_expression), but one join is not enough to sort by bug reporter, so I changed the second element of the tuple into a list of tuples (joined_table, join_expression).

I also added a small test for sorting by title. I think this was not tested before.

test: ./bin/test bugs -vvt test_bugtask_search.*test_sort_by

no lint

-- 
https://code.launchpad.net/~adeuring/launchpad/bug-sorting-2/+merge/82298
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~adeuring/launchpad/bug-sorting-2 into lp:launchpad.

=== modified file 'lib/lp/bugs/model/bugtask.py'
--- lib/lp/bugs/model/bugtask.py	2011-11-14 19:27:16 +0000
+++ lib/lp/bugs/model/bugtask.py	2011-11-15 17:50:35 +0000
@@ -3161,34 +3161,50 @@
 
     def getOrderByColumnDBName(self, col_name):
         """See `IBugTaskSet`."""
-        # Avoid circular imports.
-        from lp.registry.model.milestone import Milestone
         if BugTaskSet._ORDERBY_COLUMN is None:
-            # Local import of Bug to avoid import loop.
+            # Avoid circular imports.
             from lp.bugs.model.bug import Bug
+            from lp.registry.model.milestone import Milestone
+            from lp.registry.model.person import Person
+            Assignee = ClassAlias(Person)
+            Reporter = ClassAlias(Person)
             BugTaskSet._ORDERBY_COLUMN = {
-                "task": BugTask.id,
-                "id": BugTask.bugID,
-                "importance": BugTask.importance,
+                "task": (BugTask.id, []),
+                "id": (BugTask.bugID, []),
+                "importance": (BugTask.importance, []),
                 # TODO: sort by their name?
-                "assignee": BugTask.assigneeID,
-                "targetname": BugTask.targetnamecache,
-                "status": BugTask._status,
-                "title": Bug.title,
-                "milestone": BugTask.milestoneID,
-                "dateassigned": BugTask.date_assigned,
-                "datecreated": BugTask.datecreated,
-                "date_last_updated": Bug.date_last_updated,
-                "date_closed": BugTask.date_closed,
-                "number_of_duplicates": Bug.number_of_duplicates,
-                "message_count": Bug.message_count,
-                "users_affected_count": Bug.users_affected_count,
-                "heat": BugTask.heat,
-                "latest_patch_uploaded": Bug.latest_patch_uploaded,
+                "assignee": (
+                    Assignee.name,
+                    [
+                        (Assignee,
+                         LeftJoin(Assignee, BugTask.assignee == Assignee.id))
+                        ]),
+                "targetname": (BugTask.targetnamecache, []),
+                "status": (BugTask._status, []),
+                "title": (Bug.title, []),
+                "milestone": (BugTask.milestoneID, []),
+                "dateassigned": (BugTask.date_assigned, []),
+                "datecreated": (BugTask.datecreated, []),
+                "date_last_updated": (Bug.date_last_updated, []),
+                "date_closed": (BugTask.date_closed, []),
+                "number_of_duplicates": (Bug.number_of_duplicates, []),
+                "message_count": (Bug.message_count, []),
+                "users_affected_count": (Bug.users_affected_count, []),
+                "heat": (BugTask.heat, []),
+                "latest_patch_uploaded": (Bug.latest_patch_uploaded, []),
                 "milestone_name": (
                     Milestone.name,
-                    (Milestone,
-                     LeftJoin(Milestone, BugTask.milestone == Milestone.id))),
+                    [
+                        (Milestone,
+                         LeftJoin(Milestone,
+                                  BugTask.milestone == Milestone.id))
+                        ]),
+                "reporter": (
+                    Reporter.name,
+                    [
+                        (Bug, Join(Bug, BugTask.bug == Bug.id)),
+                        (Reporter, Join(Reporter, Bug.owner == Reporter.id))
+                        ]),
                 }
         return BugTaskSet._ORDERBY_COLUMN[col_name]
 
@@ -3258,16 +3274,12 @@
                 orderby_arg.append(orderby_col)
                 continue
             if orderby_col.startswith("-"):
-                col = self.getOrderByColumnDBName(orderby_col[1:])
-                if isinstance(col, tuple):
-                    extra_joins.append(col[1])
-                    col = col[0]
+                col, sort_joins = self.getOrderByColumnDBName(orderby_col[1:])
+                extra_joins.extend(sort_joins)
                 order_clause = Desc(col)
             else:
-                col = self.getOrderByColumnDBName(orderby_col)
-                if isinstance(col, tuple):
-                    extra_joins.append(col[1])
-                    col = col[0]
+                col, sort_joins = self.getOrderByColumnDBName(orderby_col)
+                extra_joins.extend(sort_joins)
                 order_clause = col
             if col in unambiguous_cols:
                 ambiguous = False

=== modified file 'lib/lp/bugs/tests/test_bugtask_search.py'
--- lib/lp/bugs/tests/test_bugtask_search.py	2011-11-14 19:27:16 +0000
+++ lib/lp/bugs/tests/test_bugtask_search.py	2011-11-15 17:50:35 +0000
@@ -610,6 +610,35 @@
             user=None, orderby='-milestone_name')
         self.assertSearchFinds(params, expected)
 
+    def test_sort_by_bug_reporter(self):
+        params = self.getBugTaskSearchParams(user=None, orderby='reporter')
+        expected = sorted(self.bugtasks, key=lambda task: task.bug.owner.name)
+        self.assertSearchFinds(params, expected)
+        expected.reverse()
+        params = self.getBugTaskSearchParams(user=None, orderby='-reporter')
+        self.assertSearchFinds(params, expected)
+
+    def test_sort_by_bug_assignee(self):
+        with person_logged_in(self.owner):
+            self.bugtasks[2].transitionToAssignee(
+                self.factory.makePerson(name="assignee-1"))
+            self.bugtasks[1].transitionToAssignee(
+                self.factory.makePerson(name="assignee-2"))
+        expected = [self.bugtasks[2], self.bugtasks[1], self.bugtasks[0]]
+        params = self.getBugTaskSearchParams(user=None, orderby='assignee')
+        self.assertSearchFinds(params, expected)
+        expected.reverse()
+        params = self.getBugTaskSearchParams(user=None, orderby='-assignee')
+        self.assertSearchFinds(params, expected)
+
+    def test_sort_by_bug_title(self):
+        params = self.getBugTaskSearchParams(user=None, orderby='title')
+        expected = sorted(self.bugtasks, key=lambda task: task.bug.title)
+        self.assertSearchFinds(params, expected)
+        expected.reverse()
+        params = self.getBugTaskSearchParams(user=None, orderby='-title')
+        self.assertSearchFinds(params, expected)
+
 
 class DeactivatedProductBugTaskTestCase(TestCaseWithFactory):