launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #05033
[Merge] lp:~gmb/launchpad/argh-argh-die into lp:launchpad
Graham Binns has proposed merging lp:~gmb/launchpad/argh-argh-die into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #838819 in Launchpad itself: "Async comment loading sometimes loads comments that are already visible"
https://bugs.launchpad.net/launchpad/+bug/838819
For more details, see:
https://code.launchpad.net/~gmb/launchpad/argh-argh-die/+merge/76178
This branch fixes bug 838819 by preventing the +batched-comments view
from returning comments and activity that have already been shown on the
page.
This branch has had a bit of a torturous birth, but we're here now and
I'm happy about that. I'll try not to take your review too hard, whoever
you are (jtv, I'm looking at you here).
There were several problems with the original approach to batched
comment and activity loading, and I've attempted to rectify them all in
one fell swoop.
Firstly, we were slicing the list of activity and comments late, which
meant pulling in all the activity for the bug before slicing. I've now
made the slicing happen earlier by re-jigging the way that
_getEventGroups() works and by turning the `comments` property in to a
wrapper around a getComments() method that _getEventGroups() now calls.
In order to make this work properly, I decided to use the first and last
comments in any particular batch as bookends for the activity that we
were going to return. That way we will return a maximum of batch_size
comments _and_ all the activity inbetween those comments. There is a
risk that we will drop some activity late on in the batching, but it's
not a huge risk unless there's a lot of status flap on a bug, and at
this stage it's a risk I'm willing to take. We may be able to revisit
this later.
I've tweaked the +batched-comments view so that it now only returns
those comments that aren't visible on the initial bug page. I've had to
do a bit of dancing around to avoid some recursion when testing the
view, but nothing major (just pre-populating the activity_and_comments
cachedproperty at initialisation time).
--
https://code.launchpad.net/~gmb/launchpad/argh-argh-die/+merge/76178
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~gmb/launchpad/argh-argh-die into lp:launchpad.
=== modified file 'lib/lp/bugs/browser/bugcomment.py'
--- lib/lp/bugs/browser/bugcomment.py 2011-08-22 16:24:02 +0000
+++ lib/lp/bugs/browser/bugcomment.py 2011-09-20 10:09:29 +0000
@@ -85,8 +85,7 @@
return comments
-def group_comments_with_activity(comments, activities, batch_size=None,
- offset=None):
+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)
@@ -116,13 +115,6 @@
# second, when two events are tied the comment index is used to
# disambiguate.
events = sorted(chain(comments, activity), key=itemgetter(0, 1, 2))
- if batch_size is not None:
- # If we're limiting to a given set of results, we work on just
- # that subset of results from hereon in, which saves on
- # processing time a bit.
- if offset is None:
- offset = 0
- events = events[offset:offset+batch_size]
def gen_event_windows(events):
"""Generate event windows.
=== modified file 'lib/lp/bugs/browser/bugtask.py'
--- lib/lp/bugs/browser/bugtask.py 2011-09-13 05:23:16 +0000
+++ lib/lp/bugs/browser/bugtask.py 2011-09-20 10:09:29 +0000
@@ -679,7 +679,7 @@
cache.objects['total_comments_and_activity'] = (
self.total_comments + self.total_activity)
cache.objects['initial_comment_batch_offset'] = (
- self.visible_initial_comments)
+ self.visible_initial_comments + 1)
cache.objects['first visible_recent_comment'] = (
self.total_comments - self.visible_recent_comments)
@@ -726,14 +726,31 @@
@cachedproperty
def comments(self):
"""Return the bugtask's comments."""
+ return self._getComments()
+
+ def _getComments(self, slice_info=None):
show_spam_controls = check_permission(
'launchpad.Admin', self.context.bug)
- return get_comments_for_bugtask(self.context, truncate=True,
+ return get_comments_for_bugtask(
+ self.context, truncate=True, slice_info=slice_info,
for_display=True, show_spam_controls=show_spam_controls)
@cachedproperty
def interesting_activity(self):
+ return self._getInterestingActivity()
+
+ def _getInterestingActivity(self, earliest_activity_date=None,
+ latest_activity_date=None):
"""A sequence of interesting bug activity."""
+ if (earliest_activity_date is not None and
+ latest_activity_date is not None):
+ # Only get the activity for the date range that we're
+ # interested in to save us from processing too much.
+ activity = self.context.bug.getActivityForDateRange(
+ start_date=earliest_activity_date,
+ end_date=latest_activity_date)
+ else:
+ activity = self.context.bug.activity
bug_change_re = (
'affects|description|security vulnerability|'
'summary|tags|visibility')
@@ -742,10 +759,14 @@
'(assignee|importance|milestone|status)')
interesting_match = re.compile(
"^(%s|%s)$" % (bug_change_re, bugtask_change_re)).match
- return tuple(
+ interesting_activity = tuple(
BugActivityItem(activity)
- for activity in self.context.bug.activity
+ for activity in activity
if interesting_match(activity.whatchanged) is not None)
+ # This is a bit kludgy but it means that interesting_activity is
+ # populated correctly for all subsequent calls.
+ self._interesting_activity_cached_value = interesting_activity
+ return interesting_activity
def _getEventGroups(self, batch_size=None, offset=None):
# Ensure truncation results in < max_length comments as expected
@@ -753,29 +774,42 @@
+ config.malone.comments_list_truncate_newest_to
< config.malone.comments_list_max_length)
- if not self.visible_comments_truncated_for_display:
+ if (not self.visible_comments_truncated_for_display and
+ batch_size is None):
comments = self.comments
+ elif batch_size is not None:
+ # If we're limiting to a given set of comments, we work on
+ # just that subset of comments from hereon in, which saves
+ # on processing time a bit.
+ if offset is None:
+ offset = self.visible_initial_comments
+ comments = self._getComments([
+ slice(offset, offset+batch_size)])
else:
# the comment function takes 0-offset counts where comment 0 is
# the initial description, so we need to add one to the limits
# to adjust.
oldest_count = 1 + self.visible_initial_comments
new_count = 1 + self.total_comments - self.visible_recent_comments
- show_spam_controls = check_permission(
- 'launchpad.Admin', self.context.bug)
- comments = get_comments_for_bugtask(
- self.context, truncate=True, for_display=True,
- slice_info=[
- slice(None, oldest_count), slice(new_count, None)],
- show_spam_controls=show_spam_controls)
+ slice_info = [
+ slice(None, oldest_count), slice(new_count, None)]
+ comments = self._getComments(slice_info)
visible_comments = get_visible_comments(
comments, user=self.user)
+ if len(visible_comments) > 0 and batch_size is not None:
+ first_comment = visible_comments[0]
+ last_comment = visible_comments[-1]
+ interesting_activity = (
+ self._getInterestingActivity(
+ earliest_activity_date=first_comment.datecreated,
+ latest_activity_date=last_comment.datecreated))
+ else:
+ interesting_activity = self.interesting_activity
event_groups = group_comments_with_activity(
comments=visible_comments,
- activities=self.interesting_activity,
- batch_size=batch_size, offset=offset)
+ activities=interesting_activity)
return event_groups
@cachedproperty
@@ -900,7 +934,9 @@
@cachedproperty
def total_activity(self):
"""Return the count of all activity items for the bug."""
- return self.context.bug.activity.count()
+ # Ignore the first activity item, since it relates to the bug's
+ # creation.
+ return self.context.bug.activity.count() - 1
def wasDescriptionModified(self):
"""Return a boolean indicating whether the description was modified"""
@@ -1058,15 +1094,23 @@
# We never truncate comments in this view; there would be no point.
visible_comments_truncated_for_display = False
+ def initialize(self):
+ super(BugTaskBatchedCommentsAndActivityView, self).initialize()
+ # This is here to save us from the pain of recursion,
+ # particularly in tests.
+ activity_and_comments = self.activity_and_comments
+
@property
def offset(self):
try:
return int(self.request.form_ng.getOne('offset'))
except TypeError:
- # We return visible_initial_comments, since otherwise we'd
+ # We return visible_initial_comments + 1, since otherwise we'd
# end up repeating comments that are already visible on the
- # page.
- return self.visible_initial_comments
+ # page. The +1 accounts for the fact that bug comments are
+ # essentially indexed from 1 due to comment 0 being the
+ # initial bug description.
+ return self.visible_initial_comments + 1
@property
def batch_size(self):
@@ -1085,18 +1129,31 @@
def next_offset(self):
return self.offset + self.batch_size
- @cachedproperty
+ @property
def _event_groups(self):
"""See `BugTaskView`."""
+ batch_size = self.batch_size
+ if (batch_size > (self.total_comments) or
+ not self.has_more_comments_and_activity):
+ # If the batch size is big enough to encompass all the
+ # remaining comments and activity, trim it so that we don't
+ # re-show things.
+ if self.offset == self.visible_initial_comments + 1:
+ offset_to_remove = self.visible_initial_comments
+ else:
+ offset_to_remove = self.offset
+ batch_size = (
+ self.total_comments - self.visible_recent_comments -
+ # This last bit is to make sure that _getEventGroups()
+ # doesn't accidentally inflate the batch size later on.
+ offset_to_remove)
return self._getEventGroups(
- batch_size=self.batch_size,
- offset=self.offset)
+ batch_size=batch_size, offset=self.offset)
@cachedproperty
def has_more_comments_and_activity(self):
"""Return True if there are more camments and activity to load."""
return (
- len(self.activity_and_comments) > 0 and
self.next_offset < (self.total_comments + self.total_activity))
=== modified file 'lib/lp/bugs/browser/tests/test_bugtask.py'
--- lib/lp/bugs/browser/tests/test_bugtask.py 2011-09-02 16:40:21 +0000
+++ lib/lp/bugs/browser/tests/test_bugtask.py 2011-09-20 10:09:29 +0000
@@ -1046,19 +1046,20 @@
layer = LaunchpadFunctionalLayer
- def _makeNoisyBug(self, comments_only=False):
+ def _makeNoisyBug(self, comments_only=False, number_of_comments=10,
+ number_of_changes=10):
"""Create and return a bug with a lot of comments and activity."""
bug = self.factory.makeBug(
date_created=datetime.now(UTC) - timedelta(days=30))
with person_logged_in(bug.owner):
if not comments_only:
- for i in range(10):
- task = self.factory.makeBugTask(bug=bug)
+ for i in range(number_of_changes):
change = BugTaskStatusChange(
- task, datetime.now(UTC), task.product.owner, 'status',
+ bug.default_bugtask, datetime.now(UTC),
+ bug.default_bugtask.product.owner, 'status',
BugTaskStatus.NEW, BugTaskStatus.TRIAGED)
bug.addChange(change)
- for i in range (10):
+ for i in range (number_of_comments):
msg = self.factory.makeMessage(
owner=bug.owner, content="Message %i." % i,
datecreated=datetime.now(UTC) - timedelta(days=20-i))
@@ -1068,12 +1069,12 @@
def test_offset(self):
# BugTaskBatchedCommentsAndActivityView.offset returns the
# current offset being used to select a batch of bug comments
- # and activity. If one is not specified, the view's
- # visible_initial_comments count will be returned (so that
- # comments already shown on the page won't appear twice).
+ # and activity. If one is not specified, the offset will be the
+ # view's visible_initial_comments count + 1 (so that comments
+ # already shown on the page won't appear twice).
bug_task = self.factory.makeBugTask()
view = create_initialized_view(bug_task, '+batched-comments')
- self.assertEqual(view.visible_initial_comments, view.offset)
+ self.assertEqual(view.visible_initial_comments+1, view.offset)
view = create_initialized_view(
bug_task, '+batched-comments', form={'offset': 100})
self.assertEqual(100, view.offset)
@@ -1095,12 +1096,27 @@
def test_event_groups_only_returns_batch_size_results(self):
# BugTaskBatchedCommentsAndActivityView._event_groups will
# return only batch_size results.
- bug = self._makeNoisyBug()
+ bug = self._makeNoisyBug(number_of_comments=20)
view = create_initialized_view(
bug.default_bugtask, '+batched-comments',
- form={'batch_size': 10})
+ form={'batch_size': 10, 'offset': 1})
self.assertEqual(10, len([group for group in view._event_groups]))
+ def test_event_groups_excludes_visible_recent_comments(self):
+ # BugTaskBatchedCommentsAndActivityView._event_groups will
+ # not return the last view comments - those covered by the
+ # visible_recent_comments property.
+ bug = self._makeNoisyBug(number_of_comments=20, comments_only=True)
+ view = create_initialized_view(
+ bug.default_bugtask, '+batched-comments',
+ form={'batch_size': 10, 'offset': 10})
+ expected_length = 10 - view.visible_recent_comments
+ actual_length = len([group for group in view._event_groups])
+ self.assertEqual(
+ expected_length, actual_length,
+ "Expected %i comments, got %i." %
+ (expected_length, actual_length))
+
def test_activity_and_comments_matches_unbatched_version(self):
# BugTaskBatchedCommentsAndActivityView extends BugTaskView in
# order to add the batching logic and reduce rendering
@@ -1109,20 +1125,26 @@
# We create a bug with comments only so that we can test the
# contents of activity_and_comments properly. Trying to test it
# with multiply different datatypes is fragile at best.
- bug = self._makeNoisyBug(comments_only=True)
+ bug = self._makeNoisyBug(comments_only=True, number_of_comments=20)
# We create a batched view with an offset of 0 so that all the
# comments are returned.
batched_view = create_initialized_view(
bug.default_bugtask, '+batched-comments',
- form={'offset': 0})
+ {'offset': 5, 'batch_size': 10})
unbatched_view = create_initialized_view(
- bug.default_bugtask, '+index')
- self.assertEqual(
- len(unbatched_view.activity_and_comments),
- len(batched_view.activity_and_comments))
- for i in range(len(unbatched_view.activity_and_comments)):
- unbatched_item = unbatched_view.activity_and_comments[i]
- batched_item = batched_view.activity_and_comments[i]
+ bug.default_bugtask, '+index', form={'comments': 'all'})
+ # It may look slightly confusing, but it's because the unbatched
+ # view's activity_and_comments list is indexed from comment 1,
+ # whereas the batched view indexes from zero for ease-of-coding.
+ # Comment 0 is the original bug description and so is rarely
+ # returned.
+ unbatched_activity = unbatched_view.activity_and_comments[4:14]
+ for index, unbatched_item in enumerate(unbatched_activity):
+ batched_item = batched_view.activity_and_comments[index]
self.assertEqual(
- unbatched_item['comment'].text_for_display,
- batched_item['comment'].text_for_display)
+ unbatched_item['comment'].index,
+ batched_item['comment'].index,
+ "The comments at index %i don't match. Expected to see "
+ "comment %i, got comment %i instead." %
+ (index, unbatched_item['comment'].index,
+ batched_item['comment'].index))
=== modified file 'lib/lp/bugs/configure.zcml'
--- lib/lp/bugs/configure.zcml 2011-09-19 08:35:03 +0000
+++ lib/lp/bugs/configure.zcml 2011-09-20 10:09:29 +0000
@@ -729,6 +729,7 @@
subscriptions
syncUpdate
date_last_updated
+ getActivityForDateRange
getBugTask
getDirectSubscribers
getDirectSubscribersWithDetails
=== modified file 'lib/lp/bugs/interfaces/bug.py'
--- lib/lp/bugs/interfaces/bug.py 2011-09-19 08:35:03 +0000
+++ lib/lp/bugs/interfaces/bug.py 2011-09-20 10:09:29 +0000
@@ -967,6 +967,15 @@
Returns True or False.
"""
+ def getActivityForDateRange(start_date, end_date):
+ """Return all the `IBugActivity` for this bug in a date range.
+
+ :param start_date: The earliest date for which activity can be
+ returned.
+ :param end_date: The latest date for which activity can be
+ returned.
+ """
+
# We are forced to define these now to avoid circular import problems.
IBugAttachment['bug'].schema = IBug
=== modified file 'lib/lp/bugs/model/bug.py'
--- lib/lp/bugs/model/bug.py 2011-09-19 08:35:03 +0000
+++ lib/lp/bugs/model/bug.py 2011-09-20 10:09:29 +0000
@@ -158,6 +158,7 @@
from lp.bugs.interfaces.bugwatch import IBugWatchSet
from lp.bugs.interfaces.cve import ICveSet
from lp.bugs.mail.bugnotificationrecipients import BugNotificationRecipients
+from lp.bugs.model.bugactivity import BugActivity
from lp.bugs.model.bugattachment import BugAttachment
from lp.bugs.model.bugbranch import BugBranch
from lp.bugs.model.bugcve import BugCve
@@ -2089,6 +2090,15 @@
self._attachments_query(),
operator.itemgetter(0))
+ def getActivityForDateRange(self, start_date, end_date):
+ """See `IBug`."""
+ store = Store.of(self)
+ activity_in_range = store.find(
+ BugActivity,
+ BugActivity.bug == self,
+ BugActivity.datechanged >= start_date,
+ BugActivity.datechanged <= end_date)
+ return activity_in_range
@ProxyFactory
def get_also_notified_subscribers(
=== modified file 'lib/lp/bugs/model/tests/test_bug.py'
--- lib/lp/bugs/model/tests/test_bug.py 2011-09-19 08:35:03 +0000
+++ lib/lp/bugs/model/tests/test_bug.py 2011-09-20 10:09:29 +0000
@@ -3,11 +3,18 @@
__metaclass__ = type
+from datetime import (
+ datetime,
+ timedelta,
+ )
+from pytz import UTC
+
from storm.store import Store
from testtools.testcase import ExpectedException
from zope.component import getUtility
from canonical.testing.layers import DatabaseFunctionalLayer
+from lp.bugs.adapters.bugchange import BugTitleChange
from lp.bugs.enum import (
BugNotificationLevel,
BugNotificationStatus,
@@ -485,6 +492,260 @@
self.assertNotIn(private_branch, linked_branches)
+<<<<<<< TREE
+=======
+class TestBugActivityMethods(TestCaseWithFactory):
+
+ layer = DatabaseFunctionalLayer
+
+ def setUp(self):
+ super(TestBugActivityMethods, self).setUp()
+ self.now = datetime.now(UTC)
+
+ def _makeActivityForBug(self, bug):
+ with person_logged_in(bug.owner):
+ earliest_activity = BugTitleChange(
+ when=self.now-timedelta(days=300), person=bug.owner,
+ what_changed='title', old_value='foo', new_value='baz')
+ bug.addChange(earliest_activity)
+ middle_activity = BugTitleChange(
+ when=self.now-timedelta(days=200), person=bug.owner,
+ what_changed='title', old_value='baz', new_value='bar')
+ bug.addChange(middle_activity)
+ oldest_activity = BugTitleChange(
+ when=self.now-timedelta(days=100), person=bug.owner,
+ what_changed='title', old_value='bar', new_value='spam')
+ bug.addChange(oldest_activity)
+ store = Store.of(bug)
+ store.flush()
+
+ def test_getActivityForDateRange_returns_items_between_dates(self):
+ # Bug.getActivityForDateRange() will return the activity for
+ # that bug that falls within a given date range.
+ bug = self.factory.makeBug(
+ date_created=self.now-timedelta(days=365))
+ self._makeActivityForBug(bug)
+ start_date = self.now - timedelta(days=250)
+ end_date = self.now - timedelta(days=150)
+ activity = bug.getActivityForDateRange(
+ start_date=start_date, end_date=end_date)
+ self.assertEqual(1, activity.count())
+
+ def test_getActivityForDateRange_is_inclusive_of_date_limits(self):
+ # Bug.getActivityForDateRange() will return the activity that
+ # falls on the start_ and end_ dates.
+ bug = self.factory.makeBug(
+ date_created=self.now-timedelta(days=365))
+ self._makeActivityForBug(bug)
+ start_date = self.now - timedelta(days=300)
+ end_date = self.now - timedelta(days=100)
+ activity = bug.getActivityForDateRange(
+ start_date=start_date, end_date=end_date)
+ self.assertEqual(3, activity.count())
+
+
+class TestBugPrivateAndSecurityRelatedUpdates(TestCaseWithFactory):
+
+ layer = DatabaseFunctionalLayer
+
+ def test_setPrivate_subscribes_person_who_makes_bug_private(self):
+ # When setPrivate(True) is called on a bug, the person who is
+ # marking the bug private is subscribed to the bug.
+ bug = self.factory.makeBug()
+ person = self.factory.makePerson()
+ with person_logged_in(person):
+ bug.setPrivate(True, person)
+ self.assertTrue(bug.personIsDirectSubscriber(person))
+
+ def test_setPrivate_does_not_subscribe_member_of_subscribed_team(self):
+ # When setPrivate(True) is called on a bug, the person who is
+ # marking the bug private will not be subscribed if they're
+ # already a member of a team which is a direct subscriber.
+ bug = self.factory.makeBug()
+ team = self.factory.makeTeam()
+ person = team.teamowner
+ with person_logged_in(person):
+ bug.subscribe(team, person)
+ bug.setPrivate(True, person)
+ self.assertFalse(bug.personIsDirectSubscriber(person))
+
+ def createBugTasksAndSubscribers(self, private_security_related=False):
+ # Used with the various setPrivateAndSecurityRelated tests to create
+ # a bug and add some initial subscribers.
+ bug_owner = self.factory.makePerson(name='bugowner')
+ bug = self.factory.makeBug(
+ owner=bug_owner,
+ private=private_security_related,
+ security_related=private_security_related)
+ owner_a = self.factory.makePerson(name='ownera')
+ security_contact_a = self.factory.makePerson(name='securitycontacta')
+ bug_supervisor_a = self.factory.makePerson(name='bugsupervisora')
+ driver_a = self.factory.makePerson(name='drivera')
+ product_a = self.factory.makeProduct(
+ owner=owner_a,
+ security_contact=security_contact_a,
+ bug_supervisor=bug_supervisor_a,
+ driver=driver_a)
+ owner_b = self.factory.makePerson(name='ownerb')
+ security_contact_b = self.factory.makePerson(name='securitycontactb')
+ product_b = self.factory.makeProduct(
+ owner=owner_b,
+ security_contact=security_contact_b)
+ bugtask_a = self.factory.makeBugTask(bug=bug, target=product_a)
+ bugtask_b = self.factory.makeBugTask(bug=bug, target=product_b)
+ naked_bugtask_a = removeSecurityProxy(bugtask_a)
+ naked_bugtask_b = removeSecurityProxy(bugtask_b)
+ naked_default_bugtask = removeSecurityProxy(bug.default_bugtask)
+ return (bug, bug_owner, naked_bugtask_a, naked_bugtask_b,
+ naked_default_bugtask)
+
+ def test_setPrivateTrueAndSecurityRelatedTrue(self):
+ # When a bug is marked as private=true and security_related=true, the
+ # only direct subscribers should be:
+ # - the bug reporter
+ # - the bugtask pillar security contacts (if set)
+ # - the person changing the state
+ # - and bug/pillar owners, drivers if they are already subscribed
+
+ (bug, bug_owner, bugtask_a, bugtask_b, default_bugtask) = (
+ self.createBugTasksAndSubscribers())
+ initial_subscribers = set(
+ (self.factory.makePerson(), bugtask_a.owner, bug_owner,
+ bugtask_a.pillar.security_contact, bugtask_a.pillar.driver))
+
+ with person_logged_in(bug_owner):
+ for subscriber in initial_subscribers:
+ bug.subscribe(subscriber, bug_owner)
+ who = self.factory.makePerson()
+ bug.setPrivacyAndSecurityRelated(
+ private=True, security_related=True, who=who)
+ subscribers = bug.getDirectSubscribers()
+ self.assertContentEqual(
+ set((bugtask_a.pillar.security_contact,
+ bugtask_a.pillar.bug_supervisor,
+ bugtask_a.pillar.driver,
+ bugtask_b.pillar.security_contact,
+ bugtask_a.owner,
+ default_bugtask.pillar.owner,
+ bug_owner, bugtask_b.pillar.owner, who)),
+ subscribers
+ )
+
+ def test_setPrivateTrueAndSecurityRelatedFalse(self):
+ # When a bug is marked as private=true and security_related=false, the
+ # only direct subscribers should be:
+ # - the bug reporter
+ # - the bugtask pillar bug supervisors (if set)
+ # - the person changing the state
+ # - and bug/pillar owners, drivers if they are already subscribed
+
+ (bug, bug_owner, bugtask_a, bugtask_b, default_bugtask) = (
+ self.createBugTasksAndSubscribers())
+ initial_subscribers = set(
+ (self.factory.makePerson(), bug_owner,
+ bugtask_a.pillar.security_contact, bugtask_a.pillar.driver))
+
+ with person_logged_in(bug_owner):
+ for subscriber in initial_subscribers:
+ bug.subscribe(subscriber, bug_owner)
+ who = self.factory.makePerson()
+ bug.setPrivacyAndSecurityRelated(
+ private=True, security_related=False, who=who)
+ subscribers = bug.getDirectSubscribers()
+ self.assertContentEqual(
+ set((bugtask_a.pillar.bug_supervisor,
+ bugtask_a.pillar.driver,
+ bugtask_b.pillar.owner,
+ default_bugtask.pillar.owner,
+ bug_owner, who)),
+ subscribers
+ )
+
+ def test_setPrivateFalseAndSecurityRelatedTrue(self):
+ # When a bug is marked as private=false and security_related=true, the
+ # only direct subscribers should be:
+ # - the bug reporter
+ # - the bugtask pillar security contacts (if set)
+ # - and bug/pillar owners, drivers if they are already subscribed
+
+ (bug, bug_owner, bugtask_a, bugtask_b, default_bugtask) = (
+ self.createBugTasksAndSubscribers())
+ initial_subscribers = set(
+ (self.factory.makePerson(), bug_owner,
+ bugtask_a.pillar.security_contact, bugtask_a.pillar.driver,
+ bugtask_a.pillar.bug_supervisor))
+
+ with person_logged_in(bug_owner):
+ for subscriber in initial_subscribers:
+ bug.subscribe(subscriber, bug_owner)
+ who = self.factory.makePerson()
+ bug.setPrivacyAndSecurityRelated(
+ private=False, security_related=True, who=who)
+ subscribers = bug.getDirectSubscribers()
+ self.assertContentEqual(
+ set((bugtask_a.pillar.security_contact,
+ bugtask_a.pillar.driver,
+ bugtask_b.pillar.security_contact,
+ default_bugtask.pillar.owner,
+ bug_owner)),
+ subscribers
+ )
+
+ def test_setPrivateFalseAndSecurityRelatedFalse(self):
+ # When a bug is marked as private=false and security_related=false,
+ # any existing subscriptions are left alone.
+
+ (bug, bug_owner, bugtask_a, bugtask_b, default_bugtask) = (
+ self.createBugTasksAndSubscribers(private_security_related=True))
+ initial_subscribers = set(
+ (self.factory.makePerson(), bug_owner,
+ bugtask_a.pillar.security_contact, bugtask_a.pillar.driver))
+
+ with person_logged_in(bug_owner):
+ for subscriber in initial_subscribers:
+ bug.subscribe(subscriber, bug_owner)
+ who = self.factory.makePerson()
+ expected_direct_subscribers = set(bug.getDirectSubscribers())
+ bug.setPrivacyAndSecurityRelated(
+ private=False, security_related=False, who=who)
+ subscribers = bug.getDirectSubscribers()
+ for subscriber in expected_direct_subscribers:
+ self.assertTrue(subscriber in subscribers)
+
+ def test_setPillarOwnerSubscribedIfNoBugSupervisor(self):
+ # The pillar owner is subscribed if the bug supervisor is not set.
+
+ bug_owner = self.factory.makePerson(name='bugowner')
+ bug = self.factory.makeBug(owner=bug_owner)
+ with person_logged_in(bug_owner):
+ who = self.factory.makePerson()
+ bug.setPrivacyAndSecurityRelated(
+ private=True, security_related=False, who=who)
+ subscribers = bug.getDirectSubscribers()
+ naked_bugtask = removeSecurityProxy(bug.default_bugtask)
+ self.assertContentEqual(
+ set((naked_bugtask.pillar.owner, bug_owner, who)),
+ subscribers
+ )
+
+ def test_setPillarOwnerSubscribedIfNoSecurityContact(self):
+ # The pillar owner is subscribed if the security contact is not set.
+
+ bug_owner = self.factory.makePerson(name='bugowner')
+ bug = self.factory.makeBug(owner=bug_owner)
+ with person_logged_in(bug_owner):
+ who = self.factory.makePerson()
+ bug.setPrivacyAndSecurityRelated(
+ private=False, security_related=True, who=who)
+ subscribers = bug.getDirectSubscribers()
+ naked_bugtask = removeSecurityProxy(bug.default_bugtask)
+ self.assertContentEqual(
+ set((naked_bugtask.pillar.owner, bug_owner)),
+ subscribers
+ )
+
+
+>>>>>>> MERGE-SOURCE
class TestBugAutoConfirmation(TestCaseWithFactory):
"""Tests for auto confirming bugs"""
