launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #10300
[Merge] lp:~stevenk/launchpad/structsub-private-bugs-redux into lp:launchpad
Steve Kowalik has proposed merging lp:~stevenk/launchpad/structsub-private-bugs-redux into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~stevenk/launchpad/structsub-private-bugs-redux/+merge/116798
Filter indirect, structural and duplicate subscriptions by those who have an AAG or APG. Remove include_master_dupe_subscribers for being pointless and unused.
I have removed a whole swatch of tests for being completely pointless with this change.
--
https://code.launchpad.net/~stevenk/launchpad/structsub-private-bugs-redux/+merge/116798
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~stevenk/launchpad/structsub-private-bugs-redux into lp:launchpad.
=== modified file 'database/schema/security.cfg'
--- database/schema/security.cfg 2012-07-23 11:25:26 +0000
+++ database/schema/security.cfg 2012-07-26 06:08:29 +0000
@@ -1512,8 +1512,10 @@
[bugnotification]
groups=script
public.accessartifact = SELECT, INSERT, DELETE
+public.accessartifactgrant = SELECT
public.accesspolicy = SELECT
public.accesspolicyartifact = SELECT, INSERT, DELETE
+public.accesspolicygrant = SELECT
public.account = SELECT
public.answercontact = SELECT
public.archive = SELECT
=== modified file 'lib/lp/bugs/browser/tests/bugtarget-filebug-views.txt'
--- lib/lp/bugs/browser/tests/bugtarget-filebug-views.txt 2012-07-19 04:40:03 +0000
+++ lib/lp/bugs/browser/tests/bugtarget-filebug-views.txt 2012-07-26 06:08:29 +0000
@@ -387,17 +387,6 @@
>>> filebug_view.added_bug.information_type
<DBItem InformationType.USERDATA, (4) Private>
-Since the bug was marked private before it was filed, only the bug
-reporter has been subscribed to the bug and there should be no indirect
-subscribers.
-
- >>> for subscriber in filebug_view.added_bug.getDirectSubscribers():
- ... print subscriber.displayname
- No Privileges Person
-
- >>> filebug_view.added_bug.getIndirectSubscribers()
- []
-
The user will be notified that the bug has been marked as private.
>>> print filebug_view.request.response.notifications[2].message
@@ -595,11 +584,6 @@
Mark Shuttleworth
No Privileges Person
-Since the bug is private, there should be no indirect subscribers.
-
- >>> filebug_view.added_bug.getIndirectSubscribers()
- []
-
The user will be notified that Mark Shuttleworth has been subscribed to
this bug and that the bug has been marked as private.
=== modified file 'lib/lp/bugs/doc/bug.txt'
--- lib/lp/bugs/doc/bug.txt 2012-07-19 04:40:03 +0000
+++ lib/lp/bugs/doc/bug.txt 2012-07-26 06:08:29 +0000
@@ -423,11 +423,6 @@
>>> [subscriber.name for subscriber in private_bug.getDirectSubscribers()]
[u'name16']
-Since it's private, there are no indirect subscribers.
-
- >>> private_bug.getIndirectSubscribers()
- []
-
It's up to the submitter to subscribe the maintainer, if they so choose.
This works similarly for distributions; in this case the
@@ -449,8 +444,6 @@
>>> private_bug = bugset.get(added_bug.id)
>>> [subscriber.name for subscriber in private_bug.getDirectSubscribers()]
[u'name16']
- >>> private_bug.getIndirectSubscribers()
- []
Prevent reporter from being subscribed to filed bugs
=== modified file 'lib/lp/bugs/doc/bugsubscription.txt'
--- lib/lp/bugs/doc/bugsubscription.txt 2012-07-19 04:40:03 +0000
+++ lib/lp/bugs/doc/bugsubscription.txt 2012-07-26 06:08:29 +0000
@@ -405,18 +405,6 @@
Mark Shuttleworth
Robert Collins
-A private bug never has indirect subscribers. Let's add an indirect subscriber
-to show that they still aren't included in the indirect subscriptions.
-
- >>> linux_source_bug.bugtasks[0].transitionToAssignee(
- ... personset.getByName("martin-pitt"))
-
- >>> linux_source_bug.getIndirectSubscribers()
- []
-
- >>> linux_source_bug.getSubscribersFromDuplicates()
- ()
-
Direct subscriptions always take precedence over indirect subscriptions.
So, if we unmark the above bug as private, indirect_subscribers will include
any subscribers not converted to direct subscribers.
@@ -430,14 +418,12 @@
Robert Collins
>>> print_displayname(linux_source_bug.getIndirectSubscribers())
- Martin Pitt
No Privileges Person
Sample Person
Scott James Remnant
Ubuntu Team
>>> print_displayname(linux_source_bug.getAlsoNotifiedSubscribers())
- Martin Pitt
No Privileges Person
Sample Person
Scott James Remnant
@@ -781,20 +767,6 @@
robertc@xxxxxxxxxxxxxxxxx
test@xxxxxxxxxxxxx
-The upstream maintainer will be subscribed to security-related private
-bugs, because upstream has no security contact, in this case.
-
- >>> firefox.security_contact is None
- True
-
- >>> params = CreateBugParams(
- ... title="a test bug", comment="a test description",
- ... owner=foobar, information_type=InformationType.PRIVATESECURITY)
- >>> new_bug = firefox.createBug(params)
-
- >>> getSubscribers(new_bug)
- ['foo.bar@xxxxxxxxxxxxx', 'test@xxxxxxxxxxxxx']
-
Now let's create a bug on a specific package, which has no package bug
contacts:
=== modified file 'lib/lp/bugs/doc/security-teams.txt'
--- lib/lp/bugs/doc/security-teams.txt 2012-07-19 04:40:03 +0000
+++ lib/lp/bugs/doc/security-teams.txt 2012-07-26 06:08:29 +0000
@@ -67,9 +67,6 @@
... bug.getIndirectSubscribers())
... return sorted(subscriber.name for subscriber in subscribers)
- >>> subscriber_names(bug)
- [u'mark', u'name16']
-
If the bug were not reported as security-related, only Foo Bar would
have been subscribed:
@@ -104,9 +101,6 @@
>>> bug.private
True
- >>> subscriber_names(bug)
- [u'name16', u'ubuntu-team']
-
Again, if the bug were not reported as security-related, the security
contact, the Ubuntu Team, would not have been subscribed:
=== modified file 'lib/lp/bugs/interfaces/bug.py'
--- lib/lp/bugs/interfaces/bug.py 2012-07-25 04:52:04 +0000
+++ lib/lp/bugs/interfaces/bug.py 2012-07-26 06:08:29 +0000
@@ -518,8 +518,7 @@
`BugSubscriptionLevel.LIFECYCLE` if unspecified.
"""
- def getBugNotificationRecipients(duplicateof=None, old_bug=None,
- include_master_dupe_subscribers=False):
+ def getBugNotificationRecipients(duplicateof=None, old_bug=None):
"""Return a complete INotificationRecipientSet instance.
The INotificationRecipientSet instance will contain details of
@@ -527,8 +526,6 @@
includes email addresses and textual and header-ready
rationales. See `BugNotificationRecipients` for
details of this implementation.
- If this bug is a dupe, set include_master_dupe_subscribers to
- True to include the master bug's subscribers as recipients.
"""
def canBeAQuestion():
=== modified file 'lib/lp/bugs/mail/bugnotificationrecipients.py'
--- lib/lp/bugs/mail/bugnotificationrecipients.py 2012-07-19 04:40:03 +0000
+++ lib/lp/bugs/mail/bugnotificationrecipients.py 2012-07-26 06:08:29 +0000
@@ -56,7 +56,7 @@
duplicateof parameter above and the addDupeSubscriber method.
Don't confuse them!
"""
- NotificationRecipientSet.__init__(self)
+ super(BugNotificationRecipients, self).__init__()
self.duplicateof = duplicateof
self.subscription_filters = set()
=== modified file 'lib/lp/bugs/model/bug.py'
--- lib/lp/bugs/model/bug.py 2012-07-25 22:25:27 +0000
+++ lib/lp/bugs/model/bug.py 2012-07-26 06:08:29 +0000
@@ -1123,27 +1123,12 @@
return get_also_notified_subscribers(self, recipients, level)
def getBugNotificationRecipients(self, duplicateof=None, old_bug=None,
- level=None,
- include_master_dupe_subscribers=False):
+ level=None):
"""See `IBug`."""
recipients = BugNotificationRecipients(duplicateof=duplicateof)
self.getDirectSubscribers(recipients, level=level)
- if self.private:
- assert self.getIndirectSubscribers() == [], (
- "Indirect subscribers found on private bug. "
- "A private bug should never have implicit subscribers!")
- else:
- self.getIndirectSubscribers(recipients, level=level)
- if include_master_dupe_subscribers and self.duplicateof:
- # This bug is a public duplicate of another bug, so include
- # the dupe target's subscribers in the recipient list. Note
- # that we only do this for duplicate bugs that are public;
- # changes in private bugs are not broadcast to their dupe
- # targets.
- dupe_recipients = (
- self.duplicateof.getBugNotificationRecipients(
- duplicateof=self.duplicateof, level=level))
- recipients.update(dupe_recipients)
+ self.getIndirectSubscribers(recipients, level=level)
+
# XXX Tom Berger 2008-03-18:
# We want to look up the recipients for `old_bug` too,
# but for this to work, this code has to move out of the
@@ -2249,9 +2234,6 @@
else:
raise ValueError('First argument must be bug or bugtask')
- if bug.private:
- return []
-
# Subscribers to exclude.
exclude_subscribers = frozenset().union(
info.direct_subscribers_at_all_levels, info.muted_subscribers)
@@ -2466,6 +2448,19 @@
muted_people = Select(BugMute.person_id, BugMute.bug == self.bug)
return load_people(Person.id.is_in(muted_people))
+ def forbidden_recipients_filter(self, column=None):
+ # Circular fail :(
+ from lp.bugs.model.bugtaskflat import BugTaskFlat
+ from lp.bugs.model.bugtasksearch import get_bug_privacy_filter_terms
+
+ if not column:
+ column = BugSubscription.person_id
+ if self.bug.private:
+ return [Or(*get_bug_privacy_filter_terms(column)),
+ BugTaskFlat.bug_id == self.bug.id]
+ else:
+ return []
+
@cachedproperty
@freeze(BugSubscriptionSet)
def direct_subscriptions(self):
@@ -2478,7 +2473,7 @@
BugSubscription.bug_notification_level >= self.level,
BugSubscription.bug == self.bug,
Not(In(BugSubscription.person_id,
- Select(BugMute.person_id, BugMute.bug_id == self.bug.id))))
+ Select(BugMute.person_id, BugMute.bug_id == self.bug.id))))
@property
def direct_subscribers(self):
@@ -2510,21 +2505,19 @@
def duplicate_subscriptions(self):
"""Subscriptions to duplicates of the bug.
- Excludes muted subscriptions.
+ Excludes muted subscriptions, and subscribers who can not see the
+ master bug.
"""
- if self.bug.private:
- return ()
- else:
- return IStore(BugSubscription).find(
- BugSubscription,
- BugSubscription.bug_notification_level >= self.level,
- BugSubscription.bug_id == Bug.id,
- Bug.duplicateof == self.bug,
- Not(In(
- BugSubscription.person_id,
- Select(
- BugMute.person_id, BugMute.bug_id == Bug.id,
- tables=[BugMute]))))
+ filters = [BugSubscription.bug_notification_level >= self.level,
+ BugSubscription.bug_id == Bug.id,
+ Bug.duplicateof == self.bug,
+ Not(In(
+ BugSubscription.person_id,
+ Select(
+ BugMute.person_id, BugMute.bug_id == Bug.id,
+ tables=[BugMute])))]
+ filters.extend(self.forbidden_recipients_filter())
+ return IStore(BugSubscription).find(BugSubscription, *filters)
@property
def duplicate_subscribers(self):
@@ -2585,10 +2578,16 @@
*Does not* exclude muted subscribers.
"""
if self.bugtask is None:
- assignees = Select(BugTask.assigneeID, BugTask.bug == self.bug)
- return load_people(Person.id.is_in(assignees))
- else:
- return load_people(Person.id == self.bugtask.assigneeID)
+ all = Select(BugTask.assigneeID, BugTask.bug == self.bug)
+ assignees = load_people(Person.id.is_in(all))
+ else:
+ assignees = load_people(Person.id == self.bugtask.assigneeID)
+ if self.bug.private:
+ return IStore(Person).find(Person,
+ Person.id.is_in([a.id for a in assignees]),
+ self.forbidden_recipients_filter(column=Person.id))
+ else:
+ return assignees
@cachedproperty
@freeze(BugSubscriberSet)
@@ -2612,16 +2611,13 @@
@cachedproperty
def also_notified_subscribers(self):
"""All subscribers except direct, dupe, and muted subscribers."""
- if self.bug.private:
- return BugSubscriberSet()
- else:
- subscribers = BugSubscriberSet().union(
- self.structural_subscribers,
- self.all_pillar_owners_without_bug_supervisors,
- self.all_assignees)
- return subscribers.difference(
- self.direct_subscribers_at_all_levels,
- self.muted_subscribers)
+ subscribers = BugSubscriberSet().union(
+ self.structural_subscribers,
+ self.all_pillar_owners_without_bug_supervisors,
+ self.all_assignees)
+ return subscribers.difference(
+ self.direct_subscribers_at_all_levels,
+ self.muted_subscribers)
@cachedproperty
def indirect_subscribers(self):
=== modified file 'lib/lp/bugs/model/structuralsubscription.py'
--- lib/lp/bugs/model/structuralsubscription.py 2012-06-14 05:18:22 +0000
+++ lib/lp/bugs/model/structuralsubscription.py 2012-07-26 06:08:29 +0000
@@ -702,6 +702,9 @@
:param direct_subscribers: a collection of Person objects who are
directly subscribed to the bug.
"""
+ # Circular. :-(
+ from lp.bugs.model.bugtaskflat import BugTaskFlat
+ from lp.bugs.model.bugtasksearch import get_bug_privacy_filter_terms
# We get the ids because we need to use group by in order to
# look at the filters' tags in aggregate. Once we have the ids,
# we can get the full set of what we need in subsuming or
@@ -738,6 +741,11 @@
Not(In(StructuralSubscription.subscriberID,
Select(BugSubscription.person_id,
BugSubscription.bug == bug))))
+ if bug.private:
+ filters.extend([
+ Or(*get_bug_privacy_filter_terms(
+ StructuralSubscription.subscriberID)),
+ BugTaskFlat.bug == bug])
candidates = list(_get_structural_subscriptions(
StructuralSubscription.id, query_arguments, *filters))
if not candidates:
=== modified file 'lib/lp/bugs/model/tests/test_bug.py'
--- lib/lp/bugs/model/tests/test_bug.py 2012-07-19 04:40:03 +0000
+++ lib/lp/bugs/model/tests/test_bug.py 2012-07-26 06:08:29 +0000
@@ -9,7 +9,6 @@
)
from pytz import UTC
-from storm.expr import Join
from storm.store import Store
from testtools.testcase import ExpectedException
from zope.component import getUtility
@@ -30,7 +29,6 @@
BugNotification,
BugSubscriptionInfo,
)
-from lp.bugs.model.bugnotification import BugNotificationRecipient
from lp.registry.enums import InformationType
from lp.registry.interfaces.accesspolicy import (
IAccessArtifactSource,
@@ -787,23 +785,6 @@
set((naked_bugtask.pillar.owner, bug_owner, who)),
subscribers)
- def test_structural_bug_supervisor_becomes_direct_on_private(self):
- # If a bug supervisor has a structural subscription to the bug, and
- # the bug is marked as private, the supervisor should get a direct
- # subscription. Otherwise they should be removed, per other tests.
- bug_supervisor = self.factory.makePerson()
- product = self.factory.makeProduct(bug_supervisor=bug_supervisor)
- bug_owner = self.factory.makePerson()
- bug = self.factory.makeBug(owner=bug_owner, product=product)
- with person_logged_in(product.owner):
- product.addSubscription(bug_supervisor, bug_supervisor)
-
- self.assertFalse(bug_supervisor in bug.getDirectSubscribers())
- with person_logged_in(bug_owner):
- who = self.factory.makePerson(name="who")
- bug.transitionToInformationType(InformationType.USERDATA, who)
- self.assertTrue(bug_supervisor in bug.getDirectSubscribers())
-
class TestBugPrivacy(TestCaseWithFactory):
@@ -891,25 +872,6 @@
bug.setPrivate(True, bug.owner)
self.assertEqual(InformationType.USERDATA, bug.information_type)
- def test_information_type_does_not_leak(self):
- # Make sure that bug notifications for private bugs do not leak to
- # people with a subscription on the product.
- product = self.factory.makeProduct()
- with person_logged_in(product.owner):
- product.addSubscription(product.owner, product.owner)
- reporter = self.factory.makePerson()
- bug = self.factory.makeBug(
- information_type=InformationType.USERDATA, product=product,
- owner=reporter)
- recipients = Store.of(bug).using(
- BugNotificationRecipient,
- Join(BugNotification, BugNotification.bugID == bug.id)).find(
- BugNotificationRecipient,
- BugNotificationRecipient.bug_notificationID ==
- BugNotification.id)
- self.assertEqual(
- [reporter], [recipient.person for recipient in recipients])
-
def test__reconcileAccess_handles_all_targets(self):
# _reconcileAccess gets the pillar from any task
# type.
=== modified file 'lib/lp/bugs/model/tests/test_bugsubscriptioninfo.py'
--- lib/lp/bugs/model/tests/test_bugsubscriptioninfo.py 2012-07-19 04:40:03 +0000
+++ lib/lp/bugs/model/tests/test_bugsubscriptioninfo.py 2012-07-26 06:08:29 +0000
@@ -273,18 +273,6 @@
self.assertContentEqual(
[duplicate_bug_subscription], found_subscriptions)
- def test_duplicate_for_private_bug(self):
- # The set of subscribers from duplicate bugs is always empty when the
- # master bug is private.
- duplicate_bug = self.factory.makeBug(product=self.target)
- with person_logged_in(duplicate_bug.owner):
- duplicate_bug.markAsDuplicate(self.bug)
- with person_logged_in(self.bug.owner):
- self.bug.setPrivate(True, self.bug.owner)
- found_subscriptions = self.getInfo().duplicate_subscriptions
- self.assertContentEqual([], found_subscriptions)
- self.assertContentEqual([], found_subscriptions.subscribers)
-
def test_duplicate_only(self):
# The set of duplicate subscriptions where the subscriber has no other
# subscriptions.
@@ -491,33 +479,6 @@
found_subscribers = self.getInfo().also_notified_subscribers
self.assertContentEqual([], found_subscribers)
- def test_also_notified_subscribers_for_private_bug(self):
- # The set of also notified subscribers is always empty when the master
- # bug is private.
- assignee = self.factory.makePerson()
- bugtask = self.bug.default_bugtask
- with person_logged_in(bugtask.pillar.bug_supervisor):
- bugtask.transitionToAssignee(assignee)
- with person_logged_in(self.bug.owner):
- self.bug.setPrivate(True, self.bug.owner)
- found_subscribers = self.getInfo().also_notified_subscribers
- self.assertContentEqual([], found_subscribers)
-
- def test_indirect_subscribers(self):
- # The set of indirect subscribers is the union of also notified
- # subscribers and subscribers to duplicates.
- assignee = self.factory.makePerson()
- bugtask = self.bug.default_bugtask
- with person_logged_in(bugtask.pillar.bug_supervisor):
- bugtask.transitionToAssignee(assignee)
- duplicate_bug = self.factory.makeBug(product=self.target)
- with person_logged_in(duplicate_bug.owner):
- duplicate_bug.markAsDuplicate(self.bug)
- found_subscribers = self.getInfo().indirect_subscribers
- self.assertContentEqual(
- [assignee, duplicate_bug.owner],
- found_subscribers)
-
class TestBugSubscriptionInfoPermissions(TestCaseWithFactory):
@@ -645,7 +606,7 @@
self.make_duplicate_bug()
with person_logged_in(self.bug.owner):
self.bug.setPrivate(True, self.bug.owner)
- with self.exactly_x_queries(0):
+ with self.exactly_x_queries(1):
self.info.duplicate_subscriptions
with self.exactly_x_queries(0):
self.info.duplicate_subscriptions.subscribers
=== modified file 'lib/lp/bugs/scripts/bugnotification.py'
--- lib/lp/bugs/scripts/bugnotification.py 2012-05-08 17:04:43 +0000
+++ lib/lp/bugs/scripts/bugnotification.py 2012-07-26 06:08:29 +0000
@@ -313,8 +313,7 @@
# Construct the real notification with recipients.
bug = notification.bug
recipients = bug.getBugNotificationRecipients(
- level=BugNotificationLevel.LIFECYCLE,
- include_master_dupe_subscribers=False)
+ level=BugNotificationLevel.LIFECYCLE)
message = notification.message
is_comment = notification.is_comment
activity = notification.activity
=== modified file 'lib/lp/bugs/subscribers/bug.py'
--- lib/lp/bugs/subscribers/bug.py 2012-05-08 01:06:31 +0000
+++ lib/lp/bugs/subscribers/bug.py 2012-07-26 06:08:29 +0000
@@ -156,12 +156,11 @@
level=BugNotificationLevel.METADATA)
recipients.update(old_bugtask_recipients)
for change in changes:
+ bug = bug_delta.bug
if isinstance(change, BugDuplicateChange):
- no_dupe_master_recipients = (
- bug_delta.bug.getBugNotificationRecipients(
- old_bug=bug_delta.bug_before_modification,
- level=change.change_level,
- include_master_dupe_subscribers=False))
+ no_dupe_master_recipients = bug.getBugNotificationRecipients(
+ old_bug=bug_delta.bug_before_modification,
+ level=change.change_level)
bug_delta.bug.addChange(
change, recipients=no_dupe_master_recipients)
elif (isinstance(change, BugTaskAssigneeChange) and
@@ -173,11 +172,9 @@
bug_delta.bug.addChange(change, recipients=recipients)
else:
if change.change_level == BugNotificationLevel.LIFECYCLE:
- change_recipients = (
- bug_delta.bug.getBugNotificationRecipients(
- old_bug=bug_delta.bug_before_modification,
- level=change.change_level,
- include_master_dupe_subscribers=False))
+ change_recipients = bug.getBugNotificationRecipients(
+ old_bug=bug_delta.bug_before_modification,
+ level=change.change_level)
recipients.update(change_recipients)
# Additionally, if we are re-targetting a bugtask for a private
# bug, we need to ensure the new bug supervisor and maintainer are
=== modified file 'lib/lp/bugs/tests/test_bug_notification_recipients.py'
--- lib/lp/bugs/tests/test_bug_notification_recipients.py 2012-07-24 05:43:46 +0000
+++ lib/lp/bugs/tests/test_bug_notification_recipients.py 2012-07-26 06:08:29 +0000
@@ -5,7 +5,10 @@
__metaclass__ = type
+from zope.component import getUtility
+
from lp.registry.enums import InformationType
+from lp.registry.interfaces.accesspolicy import IAccessPolicySource
from lp.testing import (
person_logged_in,
TestCaseWithFactory,
@@ -59,6 +62,7 @@
bug.getBugNotificationRecipients())
def test_private_bug(self):
+ # Only the owner is notified about a private bug.
owner = self.factory.makePerson()
bug = self.factory.makeBug(
owner=owner, information_type=InformationType.USERDATA)
@@ -67,6 +71,7 @@
[owner], bug.getBugNotificationRecipients())
def test_private_bug_with_subscriber(self):
+ # Subscribing a user to a bug grants access, so they will be notified.
owner = self.factory.makePerson()
subscriber = self.factory.makePerson()
bug = self.factory.makeBug(
@@ -77,6 +82,8 @@
[owner, subscriber], bug.getBugNotificationRecipients())
def test_private_bug_with_structural_subscriber(self):
+ # A structural subscriber without access does not get notified about
+ # a private bug.
owner = self.factory.makePerson()
subscriber = self.factory.makePerson()
product = self.factory.makeProduct()
@@ -89,7 +96,26 @@
self.assertContentEqual(
[owner], bug.getBugNotificationRecipients())
+ def test_private_bug_with_structural_subscriber_with_access(self):
+ # When a structural subscriber has access to a private bug, they are
+ # notified.
+ owner = self.factory.makePerson()
+ subscriber = self.factory.makePerson()
+ product = self.factory.makeProduct()
+ with person_logged_in(subscriber):
+ product.addBugSubscription(subscriber, subscriber)
+ policy = getUtility(IAccessPolicySource).find(
+ [(product, InformationType.USERDATA)]).one()
+ self.factory.makeAccessPolicyGrant(policy=policy, grantee=subscriber)
+ bug = self.factory.makeBug(
+ product=product, owner=owner,
+ information_type=InformationType.USERDATA)
+ with person_logged_in(owner):
+ self.assertContentEqual(
+ [owner, subscriber], bug.getBugNotificationRecipients())
+
def test_private_bug_assignee(self):
+ # Assigning a user to a private bug does not give them visibility.
owner = self.factory.makePerson()
assignee = self.factory.makePerson()
bug = self.factory.makeBug(
@@ -99,7 +125,22 @@
self.assertContentEqual(
[owner], bug.getBugNotificationRecipients())
+ def test_private_bug_assignee_with_access(self):
+ # An assignee with access will get notified.
+ owner = self.factory.makePerson()
+ assignee = self.factory.makePerson()
+ bug = self.factory.makeBug(
+ owner=owner, information_type=InformationType.USERDATA)
+ artifact = self.factory.makeAccessArtifact(concrete=bug)
+ self.factory.makeAccessArtifactGrant(
+ artifact=artifact, grantee=assignee)
+ with person_logged_in(owner):
+ bug.default_bugtask.transitionToAssignee(assignee)
+ self.assertContentEqual(
+ [owner, assignee], bug.getBugNotificationRecipients())
+
def test_private_bug_with_duplicate_subscriber(self):
+ # A subscriber to a duplicate of a private bug will not be notified.
owner = self.factory.makePerson()
subscriber = self.factory.makePerson()
bug = self.factory.makeBug(
@@ -110,3 +151,20 @@
dupe.markAsDuplicate(bug)
self.assertContentEqual(
[owner], bug.getBugNotificationRecipients())
+
+ def test_private_bug_with_duplicate_subscriber_with_access(self):
+ # A subscriber to a duplicate of a private bug will be notified, if
+ # they have access.
+ owner = self.factory.makePerson()
+ subscriber = self.factory.makePerson()
+ bug = self.factory.makeBug(
+ owner=owner, information_type=InformationType.USERDATA)
+ artifact = self.factory.makeAccessArtifact(concrete=bug)
+ self.factory.makeAccessArtifactGrant(
+ artifact=artifact, grantee=subscriber)
+ dupe = self.factory.makeBug(owner=owner)
+ with person_logged_in(owner):
+ dupe.subscribe(subscriber, owner)
+ dupe.markAsDuplicate(bug)
+ self.assertContentEqual(
+ [owner, subscriber], bug.getBugNotificationRecipients())
=== modified file 'lib/lp/bugs/tests/test_bugchanges.py'
--- lib/lp/bugs/tests/test_bugchanges.py 2012-07-19 04:40:03 +0000
+++ lib/lp/bugs/tests/test_bugchanges.py 2012-07-26 06:08:29 +0000
@@ -31,7 +31,6 @@
from lp.services.webapp.publisher import canonical_url
from lp.testing import (
api_url,
- celebrity_logged_in,
launchpadlib_for,
login_person,
person_logged_in,
@@ -1046,83 +1045,6 @@
expected_activity=expected_activity,
expected_notification=expected_notification)
- def _test_retarget_private_security_bug_to_product(self,
- bug, maintainer,
- bug_supervisor=None):
- # When a private security related bug has a bugtask retargetted to a
- # different product, a notification is sent to the new bug supervisor
- # and maintainer. If they are the same person, only one notification
- # is sent. They only get notifications if they can see the bug.
-
- # Create the private bug.
- bug_task = bug.bugtasks[0]
- new_target = self.factory.makeProduct(
- owner=maintainer, bug_supervisor=bug_supervisor)
- self.saveOldChanges(bug)
-
- bug_task_before_modification = Snapshot(
- bug_task, providing=providedBy(bug_task))
- bug_task.transitionToTarget(new_target, self.user)
- notify(ObjectModifiedEvent(
- bug_task, bug_task_before_modification,
- ['target', 'product'], user=self.user))
-
- expected_activity = {
- 'person': self.user,
- 'whatchanged': 'affects',
- 'oldvalue': bug_task_before_modification.bugtargetname,
- 'newvalue': bug_task.bugtargetname,
- }
-
- expected_recipients = [self.user]
- expected_reasons = ['Subscriber']
- if bug.userCanView(maintainer):
- expected_recipients.append(maintainer)
- expected_reasons.append('Maintainer')
- if (bug_supervisor and not bug_supervisor.inTeam(maintainer)
- and bug.userCanView(bug_supervisor)):
- expected_recipients.append(bug_supervisor)
- expected_reasons.append('Bug Supervisor')
- expected_notification = {
- 'text': u"** Project changed: %s => %s" % (
- bug_task_before_modification.bugtargetname,
- bug_task.bugtargetname),
- 'person': self.user,
- 'recipients': expected_recipients,
- 'recipient_reasons': expected_reasons
- }
- self.assertRecordedChange(
- expected_activity=expected_activity,
- expected_notification=expected_notification, bug=bug)
-
- def test_retarget_private_security_bug_to_product(self):
- # A series of tests for re-targetting a private bug task.
- bug = self.factory.makeBug(
- product=self.product, owner=self.user,
- information_type=InformationType.USERDATA)
- maintainer = self.factory.makePerson()
- bug_supervisor = self.factory.makePerson()
-
- # Test with no bug supervisor
- self._test_retarget_private_security_bug_to_product(bug, maintainer)
- # Test with bug supervisor = maintainer.
- self._test_retarget_private_security_bug_to_product(
- bug, maintainer, maintainer)
- # Test with different bug supervisor
- self._test_retarget_private_security_bug_to_product(
- bug, maintainer, bug_supervisor)
-
- # Now make the bug visible to the bug supervisor and re-test.
- with celebrity_logged_in('admin'):
- bug.default_bugtask.transitionToAssignee(bug_supervisor)
-
- # Test with bug supervisor = maintainer.
- self._test_retarget_private_security_bug_to_product(
- bug, maintainer, maintainer)
- # Test with different bug supervisor
- self._test_retarget_private_security_bug_to_product(
- bug, maintainer, bug_supervisor)
-
def test_target_bugtask_to_sourcepackage(self):
# When a bugtask's target is changed, BugActivity and
# BugNotification get updated.
@@ -1174,21 +1096,6 @@
expected_notification=expected_notification,
bug=source_package_bug)
- def test_private_bug_target_change_doesnt_add_everyone(self):
- # Retargeting a private bug doesn't add all subscribers for the
- # target.
- old_product = self.factory.makeProduct()
- new_product = self.factory.makeProduct()
- subscriber = self.factory.makePerson()
- new_product.addBugSubscription(subscriber, subscriber)
- owner = self.factory.makePerson()
- bug = self.factory.makeBug(
- product=old_product, owner=owner,
- information_type=InformationType.USERDATA)
- bug.default_bugtask.transitionToTarget(new_product, owner)
- self.assertNotIn(subscriber, bug.getDirectSubscribers())
- self.assertNotIn(subscriber, bug.getIndirectSubscribers())
-
def test_add_bugwatch_to_bugtask(self):
# Adding a BugWatch to a bug task records an entry in
# BugActivity and BugNotification.
=== modified file 'lib/lp/registry/tests/test_sharingjob.py'
--- lib/lp/registry/tests/test_sharingjob.py 2012-07-19 04:40:03 +0000
+++ lib/lp/registry/tests/test_sharingjob.py 2012-07-26 06:08:29 +0000
@@ -19,7 +19,6 @@
from lp.registry.enums import InformationType
from lp.registry.interfaces.accesspolicy import (
IAccessArtifactGrantSource,
- IAccessArtifactSource,
IAccessPolicySource,
)
from lp.registry.interfaces.person import TeamSubscriptionPolicy
@@ -182,9 +181,9 @@
job, job_type = create_job(distro, bug, grantee, owner)
# Subscribing grantee has created an artifact grant so we need to
# revoke that to test the job.
+ artifact = self.factory.makeAccessArtifact(concrete=bug)
getUtility(IAccessArtifactGrantSource).revokeByArtifact(
- getUtility(IAccessArtifactSource).find(
- [bug]), [grantee])
+ [artifact], [grantee])
transaction.commit()
out, err, exit_code = run_script(
@@ -295,9 +294,9 @@
CodeReviewNotificationLevel.NOEMAIL, owner)
# Subscribing policy_team_grantee has created an artifact grant so we
# need to revoke that to test the job.
+ artifact = self.factory.makeAccessArtifact(concrete=bug)
getUtility(IAccessArtifactGrantSource).revokeByArtifact(
- getUtility(IAccessArtifactSource).find(
- [bug]), [policy_team_grantee])
+ [artifact], [policy_team_grantee])
# policy grantees are subscribed because the job has not been run yet.
subscribers = removeSecurityProxy(bug).getDirectSubscribers()
@@ -367,9 +366,9 @@
bug.subscribe(artifact_indirect_grantee, owner)
# Subscribing policy_team_grantee has created an artifact grant so we
# need to revoke that to test the job.
+ artifact = self.factory.makeAccessArtifact(concrete=branch)
getUtility(IAccessArtifactGrantSource).revokeByArtifact(
- getUtility(IAccessArtifactSource).find(
- [branch]), [policy_team_grantee])
+ [artifact], [policy_team_grantee])
# policy grantees are subscribed because the job has not been run yet.
#subscribers = removeSecurityProxy(branch).subscribers
@@ -432,10 +431,9 @@
bug.subscribe(grantee, owner)
# Subscribing grantee to bug creates an access grant so we need to
# revoke that for our test.
- accessartifact_source = getUtility(IAccessArtifactSource)
- accessartifact_grant_source = getUtility(IAccessArtifactGrantSource)
- accessartifact_grant_source.revokeByArtifact(
- accessartifact_source.find([bug]), [grantee])
+ artifact = self.factory.makeAccessArtifact(concrete=bug)
+ getUtility(IAccessArtifactGrantSource).revokeByArtifact(
+ [artifact], [grantee])
return bug, owner
Follow ups
