launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #02212
[Merge] lp:~allenap/launchpad/bug-activity-grouping-bug-353890 into lp:launchpad
Gavin Panella has proposed merging lp:~allenap/launchpad/bug-activity-grouping-bug-353890 into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
#353890 Make the grouping of interleaved changes less time-sensitive
https://bugs.launchpad.net/bugs/353890
This branch groups together related comments and activity for display
on bug pages.
Related means: a string of 2 or more activities by a single user
happening within a given time window (5 minutes for now). There can be
zero or one comments in any given string of related activities.
Previously only activities happening at exactly the same moment would
be grouped, but this doesn't work so well with the AJAXy stuff.
The steps I took:
- Break out the code that identifies interesting activities into a new
interesting_activity property.
I have added a single test for this property. I am considering
testing this more extensively, but I'm not yet sure if I would be
repeating work done elsewhere.
- Ditch the activity_by_date property.
- New function group_comments_with_activity() that takes a sequence
(or iterable) of comments and another of activity and merges the
two, working out what is related according to the rule defined
earlier.
This is tested exclusively using stubs, so it's very fast to
test. Existing tests (which I have updated) already provide
integration tests.
- The activity_and_comments property was updated to further process
what comes out of group_comments_with_activity().
- A big pile of doctest changes and improvements.
--
https://code.launchpad.net/~allenap/launchpad/bug-activity-grouping-bug-353890/+merge/44247
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~allenap/launchpad/bug-activity-grouping-bug-353890 into lp:launchpad.
=== modified file 'lib/lp/bugs/browser/bugcomment.py'
--- lib/lp/bugs/browser/bugcomment.py 2010-09-24 22:30:48 +0000
+++ lib/lp/bugs/browser/bugcomment.py 2010-12-20 17:14:54 +0000
@@ -6,18 +6,24 @@
__metaclass__ = type
__all__ = [
'BugComment',
+ 'BugCommentBoxExpandedReplyView',
+ 'BugCommentBoxView',
+ 'BugCommentBreadcrumb',
'BugCommentView',
- 'BugCommentBoxView',
- 'BugCommentBoxExpandedReplyView',
'BugCommentXHTMLRepresentation',
- 'BugCommentBreadcrumb',
'build_comments_from_chunks',
+ 'group_comments_with_activity',
]
from datetime import (
datetime,
timedelta,
)
+from itertools import (
+ chain,
+ groupby,
+ )
+from operator import itemgetter
from lazr.restful.interfaces import IWebServiceClientRequest
from pytz import utc
@@ -44,7 +50,9 @@
IBugComment,
IBugMessageSet,
)
-from lp.registry.interfaces.person import IPersonSet
+
+
+COMMENT_ACTIVITY_GROUPING_WINDOW = timedelta(minutes=5)
def build_comments_from_chunks(chunks, bugtask, truncate=False):
@@ -87,6 +95,72 @@
return comments
+def group_comments_with_activity(comments, activities):
+ """Group comments and activity together for human consumption.
+
+ Generates a stream of comment instances (with the activity grouped within)
+ or `list`s of grouped activities.
+
+ :param comments: An iterable of `BugComment` instances.
+ :param activities: An iterable of `BugActivity` instances.
+ """
+ window = COMMENT_ACTIVITY_GROUPING_WINDOW
+
+ comment_kind = "comment"
+ comments = (
+ (comment.datecreated, comment.owner, comment_kind, comment)
+ for comment in comments)
+ activity_kind = "activity"
+ activity = (
+ (activity.datechanged, activity.person, activity_kind, activity)
+ for activity in activities)
+ events = sorted(chain(comments, activity), key=itemgetter(0, 1))
+
+ def gen_event_windows(events):
+ """Generate event windows.
+
+ Yields `(window_index, kind, event)` tuples, where `window_index` is
+ an integer, and is incremented each time the windowing conditions are
+ triggered.
+
+ :param events: An iterable of `(date, actor, kind, event)` tuples in
+ order.
+ """
+ window_comment, window_actor = None, None
+ window_index, window_end = 0, None
+ for date, actor, kind, event in events:
+ window_ended = (
+ # A window may contain only one comment.
+ (window_comment is not None and kind is comment_kind) or
+ # All events must have happened within a given timeframe.
+ (window_end is None or date >= window_end) or
+ # All events within the window must belong to the same actor.
+ (window_actor is None or actor != window_actor))
+ if window_ended:
+ window_comment, window_actor = None, actor
+ window_index, window_end = window_index + 1, date + window
+ if kind is comment_kind:
+ window_comment = event
+ yield window_index, kind, event
+
+ event_windows = gen_event_windows(events)
+ event_windows_grouper = groupby(event_windows, itemgetter(0))
+ for window_index, window_group in event_windows_grouper:
+ window_group = [
+ (kind, event) for (index, kind, event) in window_group]
+ for kind, event in window_group:
+ if kind is comment_kind:
+ window_comment = event
+ window_comment.activity.extend(
+ event for (kind, event) in window_group
+ if kind is activity_kind)
+ yield window_comment
+ # There's only one comment per window.
+ break
+ else:
+ yield [event for (kind, event) in window_group]
+
+
class BugComment:
"""Data structure that holds all data pertaining to a bug comment.
@@ -208,10 +282,17 @@
obfuscated for unauthenticated users.
"""
now = datetime.now(tz=utc)
- # The major factor in how long we can cache a bug comment is
- # the timestamp. The rendering of the timestamp changes every
- # minute for the first hour because we say '7 minutes ago'.
- if self.datecreated > now - timedelta(hours=1):
+
+ # The major factor in how long we can cache a bug comment is the
+ # timestamp. For up to 5 minutes comments and activity can be grouped
+ # together as related, so do not cache.
+ if self.datecreated > now - COMMENT_ACTIVITY_GROUPING_WINDOW:
+ # Don't return 0 because that indicates no time limit.
+ return -1
+
+ # The rendering of the timestamp changes every minute for the first
+ # hour because we say '7 minutes ago'.
+ elif self.datecreated > now - timedelta(hours=1):
return 60
# Don't cache for long if we are waiting for synchronization.
=== modified file 'lib/lp/bugs/browser/bugtask.py'
--- lib/lp/bugs/browser/bugtask.py 2010-11-24 10:10:07 +0000
+++ lib/lp/bugs/browser/bugtask.py 2010-12-20 17:14:54 +0000
@@ -11,12 +11,14 @@
'BugListingPortletInfoView',
'BugListingPortletStatsView',
'BugNominationsView',
+ 'BugsBugTaskSearchListingView',
'bugtarget_renderer',
'BugTargetTraversalMixin',
'BugTargetView',
+ 'bugtask_heat_html',
+ 'BugTaskBreadcrumb',
'BugTaskContextMenu',
'BugTaskCreateQuestionView',
- 'BugTaskBreadcrumb',
'BugTaskEditView',
'BugTaskExpirableListingView',
'BugTaskListingItem',
@@ -25,22 +27,20 @@
'BugTaskPortletView',
'BugTaskPrivacyAdapter',
'BugTaskRemoveQuestionView',
+ 'BugTasksAndNominationsView',
'BugTaskSearchListingView',
'BugTaskSetNavigation',
'BugTaskStatusView',
'BugTaskTableRowView',
'BugTaskTextView',
'BugTaskView',
- 'BugTasksAndNominationsView',
- 'bugtask_heat_html',
- 'BugsBugTaskSearchListingView',
'calculate_heat_display',
- 'NominationsReviewTableBatchNavigatorView',
- 'TextualBugTaskSearchListingView',
'get_buglisting_search_filter_url',
'get_comments_for_bugtask',
'get_sortorder_from_request',
'get_visible_comments',
+ 'NominationsReviewTableBatchNavigatorView',
+ 'TextualBugTaskSearchListingView',
]
import cgi
@@ -48,14 +48,15 @@
datetime,
timedelta,
)
+from itertools import (
+ chain,
+ groupby,
+ )
from math import (
floor,
log,
)
-from operator import (
- attrgetter,
- itemgetter,
- )
+from operator import attrgetter
import re
import urllib
@@ -75,7 +76,7 @@
IWebServiceClientRequest,
)
from lazr.uri import URI
-import pytz
+from pytz import utc
from simplejson import dumps
from z3c.ptcompat import ViewPageTemplateFile
from zope import (
@@ -203,7 +204,10 @@
BugTextView,
BugViewMixin,
)
-from lp.bugs.browser.bugcomment import build_comments_from_chunks
+from lp.bugs.browser.bugcomment import (
+ build_comments_from_chunks,
+ group_comments_with_activity,
+ )
from lp.bugs.interfaces.bug import (
IBug,
IBugSet,
@@ -812,62 +816,22 @@
return comments
@cachedproperty
- def activity_by_date(self):
- """Return a dict of `BugActivityItem`s for the current bug.
-
- The `BugActivityItem`s will be keyed by the date on which they
- occurred.
- """
- activity_by_date = {}
+ def interesting_activity(self):
+ """A sequence of interesting bug activity."""
bugtask_change_re = (
'[a-z0-9][a-z0-9\+\.\-]+( \([A-Za-z0-9\s]+\))?: '
'(assignee|importance|milestone|status)')
- interesting_changes = [
- 'description',
- 'security vulnerability',
- 'summary',
- 'tags',
- 'visibility',
- 'affects',
- bugtask_change_re,
- ]
-
- # Turn the interesting_changes list into a regex so that we can
- # do complex matches.
- interesting_changes_expression = "|".join(interesting_changes)
- interesting_changes_regex = re.compile(
- "^(%s)$" % interesting_changes_expression)
-
- for activity in self.context.bug.activity:
- # If we're not interested in the change, skip it.
- if interesting_changes_regex.match(activity.whatchanged) is None:
- continue
-
- activity = BugActivityItem(activity)
- if activity.datechanged not in activity_by_date:
- activity_by_date[activity.datechanged] = {
- activity.target: [activity]}
- else:
- activity_dict = activity_by_date[activity.datechanged]
- if activity.target in activity_dict:
- activity_dict[activity.target].append(activity)
- else:
- activity_dict[activity.target] = [activity]
-
- # Sort all the lists to ensure that changes are written out in
- # alphabetical order.
- for date, activity_by_target in activity_by_date.items():
- # We convert each {target: activity_list} mapping into a list
- # of {target, activity_list} dicts for the sake of making
- # them usable in templates.
- activity_by_date[date] = [{
- 'target': target,
- 'activity': sorted(
- activity_list, key=attrgetter('attribute')),
- }
- for target, activity_list in activity_by_target.items()]
-
- return activity_by_date
+ interesting_expressions = [
+ 'affects', 'description', 'security vulnerability',
+ 'summary', 'tags', 'visibility', bugtask_change_re]
+ interesting_expression = "|".join(interesting_expressions)
+ interesting_regex = re.compile("^(%s)$" % interesting_expression)
+ p_interesting = lambda activity: (
+ interesting_regex.match(activity.whatchanged) is not None)
+ return tuple(
+ BugActivityItem(activity)
+ for activity in self.context.bug.activity
+ if p_interesting(activity))
@cachedproperty
def activity_and_comments(self):
@@ -883,9 +847,6 @@
comments. The division between the newest and oldest is marked
by an entry in the list with the key 'num_hidden' defined.
"""
- activity_and_comments = []
- activity_log = self.activity_by_date
-
# Ensure truncation results in < max_length comments as expected
assert(config.malone.comments_list_truncate_oldest_to
+ config.malone.comments_list_truncate_newest_to
@@ -894,50 +855,68 @@
newest_comments = self.visible_newest_comments_for_display
oldest_comments = self.visible_oldest_comments_for_display
- # Oldest comments and activities
- for comment in oldest_comments:
- # Move any corresponding activities into the comment
- comment.activity = activity_log.pop(comment.datecreated, [])
- comment.activity.sort(key=itemgetter('target'))
-
- activity_and_comments.append({
- 'comment': comment,
- 'date': comment.datecreated,
- })
-
- # Insert blank if we're showing only a subset of the comment list
+ event_groups = group_comments_with_activity(
+ comments=chain(oldest_comments, newest_comments),
+ activities=self.interesting_activity)
+
+ def group_activities_by_target(activities):
+ activities = sorted(
+ activities, key=attrgetter(
+ "datechanged", "target", "attribute"))
+ return [
+ {"target": target, "activity": list(activity)}
+ for target, activity in groupby(
+ activities, attrgetter("target"))]
+
+ def comment_event_dict(comment):
+ actors = set(activity.person for activity in comment.activity)
+ actors.add(comment.owner)
+ assert len(actors) == 1, actors
+ dates = set(activity.datechanged for activity in comment.activity)
+ dates.add(comment.datecreated)
+ comment.activity = group_activities_by_target(comment.activity)
+ return {
+ "comment": comment,
+ "date": min(dates),
+ "person": actors.pop(),
+ }
+
+ def activity_event_dict(activities):
+ actors = set(activity.person for activity in activities)
+ assert len(actors) == 1, actors
+ dates = set(activity.datechanged for activity in activities)
+ return {
+ "activity": group_activities_by_target(activities),
+ "date": min(dates),
+ "person": actors.pop(),
+ }
+
+ def event_dict(event_group):
+ if isinstance(event_group, list):
+ return activity_event_dict(event_group)
+ else:
+ return comment_event_dict(event_group)
+
+ events = map(event_dict, event_groups)
+
+ # Insert blank if we're showing only a subset of the comment list.
if len(newest_comments) > 0:
- activity_and_comments.append({
- 'num_hidden': (len(self.visible_comments)
- - len(oldest_comments)
- - len(newest_comments)),
- 'date': newest_comments[0].datecreated,
- })
-
- # Most recent comments and activities (if showing a subset)
- for comment in newest_comments:
- # Move any corresponding activities into the comment
- comment.activity = activity_log.pop(comment.datecreated, [])
- comment.activity.sort(key=itemgetter('target'))
-
- activity_and_comments.append({
- 'comment': comment,
- 'date': comment.datecreated,
- })
-
- # Add the remaining activities not associated with any visible
- # comments to the activity_for_comments list. For each
- # activity dict we use the person responsible for the first
- # activity item as the owner of the list of activities.
- for date, activity_dict in activity_log.items():
- activity_and_comments.append({
- 'activity': activity_dict,
- 'date': date,
- 'person': activity_dict[0]['activity'][0].person,
- })
-
- activity_and_comments.sort(key=itemgetter('date'))
- return activity_and_comments
+ # Find first newest comment in the event list.
+ first_newest_comment = newest_comments[0]
+ for index, event in enumerate(events):
+ if event.get("comment") is first_newest_comment:
+ num_hidden = (
+ len(self.visible_comments)
+ - len(oldest_comments)
+ - len(newest_comments))
+ separator = {
+ 'date': first_newest_comment.datecreated,
+ 'num_hidden': num_hidden,
+ }
+ events.insert(index, separator)
+ break
+
+ return events
@cachedproperty
def visible_comments(self):
@@ -1010,7 +989,7 @@
expire_after = timedelta(days=config.malone.days_before_expiration)
expiration_date = self.context.bug.date_last_updated + expire_after
- remaining_time = expiration_date - datetime.now(pytz.timezone('UTC'))
+ remaining_time = expiration_date - datetime.now(utc)
return remaining_time.days
@property
=== modified file 'lib/lp/bugs/browser/tests/bug-views.txt'
--- lib/lp/bugs/browser/tests/bug-views.txt 2010-10-21 01:42:14 +0000
+++ lib/lp/bugs/browser/tests/bug-views.txt 2010-12-20 17:14:54 +0000
@@ -677,7 +677,7 @@
False
>>> for message in bug_edit.view.notifications:
... print message
- The tag "new-tag" hasn't been used by Mozilla Firefox before.
+ The tag "new-tag" hasn't been used by Mozilla Firefox before.
<input ...Create the new tag...>
>>> bug_edit.wasRedirected()
@@ -751,16 +751,6 @@
>>> view = getMultiAdapter(
... (bug.bugtasks[0], request), name="+index")
-The activity_by_date property of BugTaskView returns a dict of
-BugActivityItems grouped by the date on which they occurred. The items
-are returned as a list of dicts containig a list of activity items for a
-given target.
-
- >>> print pretty(view.activity_by_date)
- {datetime.datetime(2009, 3, 26...):
- [{'activity': [<...BugActivityItem...>],
- 'target': None}]}
-
The activity_and_comments property of BugTaskView is a list of comments
and activity on a bug, ordered by the date that they occurred. Each item
is encapsulated in a dict, in the form: {'comment': <BugComment>} or
@@ -784,28 +774,40 @@
... (bug.bugtasks[0], request), name="+index")
>>> view.initialize()
- >>> activity_and_comments = view.activity_and_comments
- >>> for activity_or_comment in activity_and_comments:
- ... if 'activity' in activity_or_comment:
- ... activity_dict = activity_or_comment['activity']
- ... for activity_for_target in activity_dict:
- ... target_name = activity_for_target['target']
- ... activity_items = activity_for_target['activity']
- ... if target_name is not None:
- ... print "Changed in %s:" % target_name
- ... for activity_item in activity_items:
- ... print "%s: %s => %s" % (
- ... activity_item.change_summary,
- ... activity_item.oldvalue,
- ... activity_item.newvalue)
- ...
- ... if 'comment' in activity_or_comment:
- ... comment = activity_or_comment['comment']
- ... print comment.text_for_display
- summary: A bug title => A new bug title
+ >>> def print_activities(activities):
+ ... for activity in activities:
+ ... target_name = activity['target']
+ ... if target_name is None:
+ ... print "Changed:"
+ ... else:
+ ... print "Changed in %s:" % target_name
+ ... activity_items = activity['activity']
+ ... for activity_item in activity_items:
+ ... print "* %s: %s => %s" % (
+ ... activity_item.change_summary,
+ ... activity_item.oldvalue,
+ ... activity_item.newvalue)
+
+ >>> def print_comment(comment):
+ ... print comment.text_for_display
+ ... print_activities(comment.activity)
+
+ >>> def print_activity_and_comments(activity_and_comments):
+ ... for activity_or_comment in activity_and_comments:
+ ... print "-- {person.name} --".format(**activity_or_comment)
+ ... if 'activity' in activity_or_comment:
+ ... print_activities(activity_or_comment["activity"])
+ ... if 'comment' in activity_or_comment:
+ ... print_comment(activity_or_comment["comment"])
+
+ >>> print_activity_and_comments(view.activity_and_comments)
+ -- name16 --
+ Changed:
+ * summary: A bug title => A new bug title
+ -- name16 --
A comment, for the reading of.
Changed in testproduct:
- status: New => Confirmed
+ * status: New => Confirmed
If a comment and a BugActivity item occur at the same time, the activity
item will be returned in the comment's activity property rather than as
@@ -832,36 +834,17 @@
before, plus the new comment, since the changes we just made were
grouped with that comment.
- >>> activity_and_comments = view.activity_and_comments
- >>> for activity_or_comment in activity_and_comments:
- ... if 'activity' in activity_or_comment:
- ... for activity_dict in activity_or_comment['activity']:
- ... for activity in activity_dict['activity']:
- ... print "%s: %s => %s" % (
- ... activity.whatchanged, activity.oldvalue,
- ... activity.newvalue)
- ... if 'comment' in activity_or_comment:
- ... comment = activity_or_comment['comment']
- ... print comment.text_for_display
- summary: A bug title => A new bug title
+ >>> print_activity_and_comments(view.activity_and_comments)
+ -- name16 --
+ Changed:
+ * summary: A bug title => A new bug title
+ -- name16 --
A comment, for the reading of.
+ -- name16 --
I triaged it.
-
-But if we look at the most recent comment's activity property we'll see
-that the activity recorded there reflects the changes that go with the
-comment.
-
- >>> comment = activity_and_comments[-1]['comment']
- >>> for activity_dict in comment.activity:
- ... if activity_dict['target'] is not None:
- ... print "Changed in %s:" % activity_dict['target']
- ... for activity in activity_dict['activity']:
- ... print "%s: %s => %s" % (
- ... activity.attribute, activity.oldvalue,
- ... activity.newvalue)
Changed in testproduct:
- importance: Undecided => High
- status: New => Confirmed
+ * importance: Undecided => High
+ * status: New => Confirmed
== Getting the list of possible duplicates for a new bug ==
=== added file 'lib/lp/bugs/browser/tests/test_bugcomment.py'
--- lib/lp/bugs/browser/tests/test_bugcomment.py 1970-01-01 00:00:00 +0000
+++ lib/lp/bugs/browser/tests/test_bugcomment.py 2010-12-20 17:14:54 +0000
@@ -0,0 +1,178 @@
+# Copyright 2010 Canonical Ltd. This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Tests for the bugcomment module."""
+
+__metaclass__ = type
+
+from datetime import (
+ datetime,
+ timedelta,
+ )
+from itertools import count
+
+from pytz import utc
+
+from lp.bugs.browser.bugcomment import group_comments_with_activity
+from lp.testing import TestCase
+
+
+class BugActivityStub:
+
+ def __init__(self, datechanged, person=None):
+ self.datechanged = datechanged
+ if person is None:
+ person = PersonStub()
+ self.person = person
+
+ def __repr__(self):
+ return "BugActivityStub(%r, %r)" % (
+ self.datechanged.strftime('%Y-%m-%d--%H%M'), self.person)
+
+
+class BugCommentStub:
+
+ def __init__(self, datecreated, owner=None):
+ self.datecreated = datecreated
+ if owner is None:
+ owner = PersonStub()
+ self.owner = owner
+ self.activity = []
+
+ def __repr__(self):
+ return "BugCommentStub(%r, %r)" % (
+ self.datecreated.strftime('%Y-%m-%d--%H%M'), self.owner)
+
+
+class PersonStub:
+
+ ids = count(1)
+
+ def __init__(self):
+ self.id = next(self.ids)
+
+ def __repr__(self):
+ return "PersonStub#%d" % self.id
+
+
+class TestGroupCommentsWithActivities(TestCase):
+ """Tests for `group_comments_with_activities`."""
+
+ def setUp(self):
+ super(TestGroupCommentsWithActivities, self).setUp()
+ self.now = datetime.now(utc)
+ self.timestamps = (
+ self.now + timedelta(minutes=counter)
+ for counter in count(1))
+
+ def group(self, comments, activities):
+ return list(
+ group_comments_with_activity(
+ comments=comments, activities=activities))
+
+ def test_empty(self):
+ # Given no comments or activities the result is also empty.
+ self.assertEqual(
+ [], self.group(comments=[], activities=[]))
+
+ def test_activity_empty_no_common_actor(self):
+ # When no activities is passed in, and the comments passed in don't
+ # have any common actors, no grouping is possible.
+ comments = [
+ BugCommentStub(next(self.timestamps))
+ for number in xrange(5)]
+ self.assertEqual(
+ comments, self.group(comments=comments, activities=[]))
+
+ def test_comments_empty_no_common_actor(self):
+ # When no comments are passed in, and the activities passed in don't
+ # have any common actors, no grouping is possible.
+ activities = [
+ BugActivityStub(next(self.timestamps))
+ for number in xrange(5)]
+ self.assertEqual(
+ [[activity] for activity in activities], self.group(
+ comments=[], activities=activities))
+
+ def test_no_common_actor(self):
+ # When each activities and comment given has a different actor then no
+ # grouping is possible.
+ activity1 = BugActivityStub(next(self.timestamps))
+ comment1 = BugCommentStub(next(self.timestamps))
+ activity2 = BugActivityStub(next(self.timestamps))
+ comment2 = BugCommentStub(next(self.timestamps))
+
+ activities = set([activity1, activity2])
+ comments = set([comment1, comment2])
+
+ self.assertEqual(
+ [[activity1], comment1, [activity2], comment2],
+ self.group(comments=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.
+ actor = PersonStub()
+ comment = BugCommentStub(next(self.timestamps), actor)
+ activity = BugActivityStub(next(self.timestamps), actor)
+ grouped = self.group(comments=[comment], activities=[activity])
+ self.assertEqual([comment], grouped)
+ self.assertEqual([activity], comment.activity)
+
+ def test_activity_then_comment_close_by_common_actor(self):
+ # An activity shortly before a comment by the same person is grouped
+ # into the comment.
+ actor = PersonStub()
+ activity = BugActivityStub(next(self.timestamps), actor)
+ comment = BugCommentStub(next(self.timestamps), actor)
+ grouped = self.group(comments=[comment], activities=[activity])
+ self.assertEqual([comment], grouped)
+ self.assertEqual([activity], comment.activity)
+
+ def test_interleaved_activity_with_comments_by_common_actor(self):
+ # Activities shortly before and after a comment are grouped into the
+ # comment's activity.
+ actor = PersonStub()
+ activity1 = BugActivityStub(next(self.timestamps), actor)
+ comment = BugCommentStub(next(self.timestamps), actor)
+ activity2 = BugActivityStub(next(self.timestamps), actor)
+ grouped = self.group(
+ comments=[comment], activities=[activity1, activity2])
+ self.assertEqual([comment], grouped)
+ self.assertEqual([activity1, activity2], comment.activity)
+
+ def test_common_actor_over_a_prolonged_time(self):
+ # There is a timeframe for grouping events. Anything outside of that
+ # window is considered separate.
+ actor = PersonStub()
+ activities = [
+ BugActivityStub(next(self.timestamps), actor)
+ for count in xrange(8)]
+ grouped = self.group(comments=[], activities=activities)
+ self.assertEqual(2, len(grouped))
+ self.assertEqual(activities[:5], grouped[0])
+ self.assertEqual(activities[5:], grouped[1])
+
+ def test_two_comments_by_common_actor(self):
+ # Only one comment will ever appear in a group.
+ actor = PersonStub()
+ comment1 = BugCommentStub(next(self.timestamps), actor)
+ comment2 = BugCommentStub(next(self.timestamps), actor)
+ grouped = self.group(comments=[comment1, comment2], activities=[])
+ self.assertEqual([comment1, comment2], grouped)
+
+ def test_two_comments_with_activity_by_common_actor(self):
+ # Activity gets associated with earlier comment when all other factors
+ # are unchanging.
+ actor = PersonStub()
+ activity1 = BugActivityStub(next(self.timestamps), actor)
+ comment1 = BugCommentStub(next(self.timestamps), actor)
+ activity2 = BugActivityStub(next(self.timestamps), actor)
+ comment2 = BugCommentStub(next(self.timestamps), actor)
+ activity3 = BugActivityStub(next(self.timestamps), actor)
+ grouped = self.group(
+ comments=[comment1, comment2],
+ activities=[activity1, activity2, activity3])
+ self.assertEqual([comment1, comment2], grouped)
+ self.assertEqual([activity1, activity2], comment1.activity)
+ self.assertEqual([activity3], comment2.activity)
=== modified file 'lib/lp/bugs/browser/tests/test_bugtask.py'
--- lib/lp/bugs/browser/tests/test_bugtask.py 2010-10-26 15:47:24 +0000
+++ lib/lp/bugs/browser/tests/test_bugtask.py 2010-12-20 17:14:54 +0000
@@ -3,13 +3,14 @@
__metaclass__ = type
-
+from datetime import datetime
from doctest import DocTestSuite
import unittest
+from pytz import UTC
from storm.store import Store
from testtools.matchers import LessThan
-
+from zope.component import getUtility
from zope.security.proxy import removeSecurityProxy
from canonical.launchpad.ftests import (
@@ -31,7 +32,9 @@
BugTaskEditView,
BugTasksAndNominationsView,
)
+from lp.bugs.interfaces.bugactivity import IBugActivitySet
from lp.bugs.interfaces.bugtask import BugTaskStatus
+from lp.services.propertycache import get_property_cache
from lp.testing import (
person_logged_in,
TestCaseWithFactory,
@@ -87,6 +90,32 @@
LessThan(count_with_no_teams + 3),
))
+ def test_interesting_activity(self):
+ # The interesting_activity property returns a tuple of interesting
+ # `BugActivityItem`s.
+ bug = self.factory.makeBug()
+ view = create_initialized_view(
+ bug.default_bugtask, name=u'+index', rootsite='bugs')
+
+ def add_activity(what, old=None, new=None, message=None):
+ getUtility(IBugActivitySet).new(
+ bug, datetime.now(UTC), bug.owner, whatchanged=what,
+ oldvalue=old, newvalue=new, message=message)
+ del get_property_cache(view).interesting_activity
+
+ # A fresh bug has no interesting activity.
+ self.assertEqual((), view.interesting_activity)
+
+ # Some activity is not considered interesting.
+ add_activity("boring")
+ self.assertEqual((), view.interesting_activity)
+
+ # A description change is interesting.
+ add_activity("description")
+ self.assertEqual(1, len(view.interesting_activity))
+ [activity] = view.interesting_activity
+ self.assertEqual("description", activity.whatchanged)
+
class TestBugTasksAndNominationsView(TestCaseWithFactory):
=== modified file 'lib/lp/bugs/stories/bugs/xx-bug-activity.txt'
--- lib/lp/bugs/stories/bugs/xx-bug-activity.txt 2010-09-24 22:30:48 +0000
+++ lib/lp/bugs/stories/bugs/xx-bug-activity.txt 2010-12-20 17:14:54 +0000
@@ -48,11 +48,11 @@
Some bug activity is show interleaved with comments on a bug's main
page.
- >>> def print_comments(page):
+ >>> def print_comments(page, subset=slice(-1, None)):
... """Print all the comments on the page."""
... comment_divs = find_tags_by_class(page, 'boardComment')
- ... for div_tag in comment_divs:
- ... print extract_text(div_tag)
+ ... for div_tag in comment_divs[subset]:
+ ... print extract_text(div_tag).replace("&\x238594;", "=>")
... print '-' * 8
>>> user_browser.open(
@@ -60,13 +60,13 @@
>>> user_browser.getControl(name='field.comment').value = (
... "Here's a comment for testing, like.")
>>> user_browser.getControl('Post Comment').click()
- >>> print_comments(user_browser.contents)
+ >>> print_comments(user_browser.contents, slice(None))
In...
Bug Watch Updater
on 2007-12-18
Changed in thunderbird:
status:
- Unknown → New
+ Unknown => New
--------
No Privileges Person
...
@@ -84,7 +84,7 @@
that have been added.
>>> user_browser.open('http://launchpad.dev/bugs/15')
- >>> print_comments(user_browser.contents)
+ >>> print_comments(user_browser.contents, slice(None))
In...
Bug
...
@@ -105,16 +105,19 @@
>>> admin_browser.open('http://launchpad.dev/bugs/15')
>>> print_comments(admin_browser.contents)
- In...
- Bug
- ...
- --------
- Foo Bar ... ago
- visibility: public → private
+ Foo Bar
+ ... ago
+ summary:
+ - Nonsensical bugs are useless
+ + A new title for this bug
+ visibility:
+ public => private
--------
-Changes to a bug's security_related field will be displayed as 'security
-vulnerability' changes, with values of 'yes' or 'no'.
+Changes to a bug's security_related field will be displayed as
+'security vulnerability' changes, with values of 'yes' or 'no'. Note
+that changes are grouped together if they occur at similar times, but
+are still shown in the order the changes were made.
>>> admin_browser.open(
... 'http://bugs.launchpad.dev/redfish/+bug/15/+secrecy')
@@ -124,33 +127,12 @@
>>> admin_browser.open('http://launchpad.dev/bugs/15')
>>> print_comments(admin_browser.contents)
- In...
- Bug
- ...
- --------
- Foo Bar ... ago
- security vulnerability: no → yes
- --------
-
-Changes will be grouped together if they occur at the same time.
-
- >>> admin_browser.open(
- ... 'http://bugs.launchpad.dev/redfish/+bug/15/+secrecy')
- >>> admin_browser.getControl(
- ... "This bug is a security vulnerability").selected = False
- >>> admin_browser.getControl(
- ... "This bug report should be private").selected = False
- >>> admin_browser.getControl("Change").click()
-
- >>> admin_browser.open('http://launchpad.dev/bugs/15')
- >>> print_comments(admin_browser.contents)
- In...
- Bug
- ...
- --------
- Foo Bar ... ago
- security vulnerability: yes → no
- visibility: private → public
+ Foo Bar
+ ... ago
+ summary:
+ ...
+ security vulnerability:
+ no => yes
--------
Changes to the bug's description will simply be displayed as 'description:
@@ -164,12 +146,12 @@
>>> admin_browser.open('http://launchpad.dev/bugs/15')
>>> print_comments(admin_browser.contents)
- In...
- Bug
+ Foo Bar
+ ... ago
+ summary:
...
- --------
- Foo Bar ... ago
- description: updated
+ description:
+ updated
--------
Changes to the bug's tags will be show in the form tags removed or tags
@@ -183,13 +165,17 @@
>>> admin_browser.open('http://launchpad.dev/bugs/15')
>>> print_comments(admin_browser.contents)
- In...
- Bug
+ Foo Bar
+ ... ago
+ summary:
...
- --------
- Foo Bar ... ago
- tags: added: tag1 tag2 tag3
- --------
+ tags:
+ added: tag1 tag2 tag3
+ --------
+
+When two similar activities are grouped into the same comment - like
+two sets of tag changes - they are displayed in the order they were
+made.
>>> admin_browser.open(
... 'http://bugs.launchpad.dev/redfish/+bug/15/+edit')
@@ -199,13 +185,15 @@
>>> admin_browser.open('http://launchpad.dev/bugs/15')
>>> print_comments(admin_browser.contents)
- In...
- Bug
+ Foo Bar
+ ... ago
+ summary:
...
- --------
- Foo Bar ... ago
- tags: added: tag4
- removed: tag3
+ tags:
+ added: tag1 tag2 tag3
+ tags:
+ added: tag4
+ removed: tag3
--------
Changes to a BugTask's attributes will show up listed under the task's
@@ -231,16 +219,19 @@
>>> admin_browser.getControl("Save Changes").click()
>>> print_comments(admin_browser.contents)
- In...
- Bug
+ Foo Bar
+ ... ago
+ summary:
...
- --------
- Foo Bar ... ago
Changed in redfish:
- assignee: nobody → Foo Bar (name16)
- importance: Undecided → High
- milestone: none → foo
- status: New → Confirmed
+ assignee:
+ nobody => Foo Bar (name16)
+ importance:
+ Undecided => High
+ milestone:
+ none => foo
+ status:
+ New => Confirmed
--------
If a change is made to a bug task which is targeted to a distro source
@@ -252,13 +243,11 @@
>>> admin_browser.getControl('Status').value = ['Confirmed']
>>> admin_browser.getControl("Save Changes").click()
>>> print_comments(admin_browser.contents)
- Sample Person
- ...
Foo Bar
- ...
+ ... ago
Changed in mozilla-firefox (Ubuntu):
status:
- New → Confirmed
+ New => Confirmed
--------
If a change has a comment associated with it it will be displayed in the
@@ -273,18 +262,18 @@
>>> admin_browser.getControl('Comment').value = "Lookit, a change!"
>>> admin_browser.getControl("Save Changes").click()
>>> print_comments(admin_browser.contents)
- Sample Person
- ...
Foo Bar
wrote
- ...
+ ... ago:
#2
Lookit, a change!
Changed in mozilla-firefox (Ubuntu):
+ status:
+ New => Confirmed
importance:
- Medium → Low
+ Medium => Low
status:
- Confirmed → New
+ Confirmed => New
--------
If a target of a bug task is changed the old and new value will be shown.
@@ -296,15 +285,11 @@
... 'Package').value = 'linux-source-2.6.15'
>>> admin_browser.getControl("Save Changes").click()
>>> print_comments(admin_browser.contents)
- Sample Person
- ...
Foo Bar
wrote
- ...
- --------
- Foo Bar
+ ... ago:
+ #2
...
affects:
- mozilla-firefox (Ubuntu) → linux-source-2.6.15 (Ubuntu)
+ mozilla-firefox (Ubuntu) => linux-source-2.6.15 (Ubuntu)
--------
-
Follow ups
