launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging ~cjwatson/launchpad:py3-group-bug-comments into launchpad:master.

Commit message:
Fix sorting of bug activities on Python 3

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/+git/launchpad/+merge/402636

In some cases (auto-confirmation of bug tasks) it's possible for a bug to have multiple activities with the same timestamp but different actors, and that caused an OOPS when trying to render such bugs on Python 3.  Fix the tie-breaking condition.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:py3-group-bug-comments into launchpad:master.

diff --git a/lib/lp/bugs/browser/bugcomment.py b/lib/lp/bugs/browser/bugcomment.py
index 2b61552..84be54e 100644
--- a/lib/lp/bugs/browser/bugcomment.py
+++ b/lib/lp/bugs/browser/bugcomment.py
@@ -123,10 +123,15 @@ def group_comments_with_activity(comments, activities):
         (activity.datechanged, max_index,
             activity.person, activity_kind, activity)
         for activity in activities)
+
     # when an action and a comment happen at the same time, the action comes
     # second, when two events are tied the comment index is used to
     # disambiguate.
-    events = sorted(chain(comments, activity), key=itemgetter(0, 1, 2))
+    def event_sort_key(event):
+        date, index, person, _, _ = event
+        return date, index, person.id
+
+    events = sorted(chain(comments, activity), key=event_sort_key)
 
     def gen_event_windows(events):
         """Generate event windows.
diff --git a/lib/lp/bugs/browser/tests/test_bugcomment.py b/lib/lp/bugs/browser/tests/test_bugcomment.py
index c1877a5..f115b83 100644
--- a/lib/lp/bugs/browser/tests/test_bugcomment.py
+++ b/lib/lp/bugs/browser/tests/test_bugcomment.py
@@ -137,6 +137,17 @@ class TestGroupCommentsWithActivities(TestCase):
             [[activity1], comment1, [activity2], comment2],
             self.group(comments=comments, activities=activities))
 
+    def test_activities_identical_timestamp_no_common_actor(self):
+        # When two activities have an identical timestamp but different
+        # actors, no grouping is possible.  (This can happen when a bug task
+        # was auto-confirmed due to multiple affected users; in that case
+        # the status change is ascribed to ~janitor.)
+        activity_time = next(self.time_index)[0]
+        activities = [BugActivityStub(activity_time) for _ in range(2)]
+        self.assertEqual(
+            [[activity] for activity in activities],
+            self.group(comments=[], activities=activities))
+
     def test_comment_then_activity_close_by_common_actor(self):
         # An activity shortly after a comment by the same person is grouped
         # into the comment.