launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #05048
[Merge] lp:~wallyworld/launchpad/private-bug-subscriptions-854405 into lp:launchpad
Ian Booth has proposed merging lp:~wallyworld/launchpad/private-bug-subscriptions-854405 into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #854405 in Launchpad itself: "Private bug subscriptions"
https://bugs.launchpad.net/launchpad/+bug/854405
For more details, see:
https://code.launchpad.net/~wallyworld/launchpad/private-bug-subscriptions-854405/+merge/76409
This mp is a re-implementation of the work done for bug 475775 in lp:~wallyworld/launchpad/bug-subscribers-after-private-475775
The original was rolled back due to disagreement about what was implemented. The change over the original functionality in this branch is:
Business rule:
When a bug is made private any direct subscribers should be unsubscribed unless they are in a small set of authorised users, including pillar owner, bug supervisor etc.
Amendment:
Only unsubscribe unauthorised direct subscribers if the project is flagged as having private bugs by default
NB I've implemented the above rule as meaning: private_bugs=true for the default_bugtask
== Implementation ==
As well as tweaking the original implementation, another issue was found. The BugSecrecyEditView action handler was where the ObjectModifiedEvent was published when the bug's privacy/security status was changed. There was a listener to this event which subscribed the bug security contact if the bug was changed to security related. However, the view is not the correct place to publish such events - this should be done in the model. And having the view do it means that when the API is used, the expected processing does not occur.
== Tests ==
Unit tests were created to test the subscription behaviour for bugs with and without a private project for the default bug task.
TestBugPrivateAndSecurityRelatedUpdatesPrivateProject
TestBugPrivateAndSecurityRelatedUpdatesPublicProject
Additionally, code in test_bugchanges which used to have to manually publish ObjectModifiedEvent was removed, since these events are now published when setPrivate() and/or setSecurityRelated() are called on the bug model object.
Otherwise, tests are the same as for the previous version of this work.
== Lint ==
Checking for conflicts and issues in changed files.
Linting changed files:
lib/lp/bugs/configure.zcml
lib/lp/bugs/browser/bug.py
lib/lp/bugs/browser/tests/test_bug_views.py
lib/lp/bugs/browser/tests/test_bugattachment_edit_view.py
lib/lp/bugs/doc/bug-heat.txt
lib/lp/bugs/doc/bug.txt
lib/lp/bugs/doc/bugnotification-email.txt
lib/lp/bugs/doc/bugnotification-sending.txt
lib/lp/bugs/doc/bugsubscription.txt
lib/lp/bugs/doc/initial-bug-contacts.txt
lib/lp/bugs/doc/security-teams.txt
lib/lp/bugs/interfaces/bug.py
lib/lp/bugs/mail/commands.py
lib/lp/bugs/model/bug.py
lib/lp/bugs/model/tests/test_bug.py
lib/lp/bugs/model/tests/test_bugsummary.py
lib/lp/bugs/scripts/bugimport.py
lib/lp/bugs/stories/bug-privacy/xx-bug-privacy.txt
lib/lp/bugs/subscribers/bug.py
lib/lp/bugs/tests/test_bugchanges.py
lib/lp/testing/factory.py
./lib/lp/bugs/doc/bug.txt
457: source exceeds 78 characters.
1129: narrative exceeds 78 characters.
1164: want exceeds 78 characters.
1165: want exceeds 78 characters.
1166: want exceeds 78 characters.
./lib/lp/bugs/doc/bugnotification-email.txt
96: source exceeds 78 characters.
224: source exceeds 78 characters.
250: source exceeds 78 characters.
./lib/lp/bugs/doc/bugnotification-sending.txt
286: want exceeds 78 characters.
./lib/lp/bugs/scripts/bugimport.py
29: redefinition of unused 'ET' from line 27
./lib/lp/bugs/stories/bug-privacy/xx-bug-privacy.txt
27: source has bad indentation.
30: source exceeds 78 characters.
38: source has bad indentation.
46: source has bad indentation.
51: source has bad indentation.
66: source exceeds 78 characters.
122: source exceeds 78 characters.
--
https://code.launchpad.net/~wallyworld/launchpad/private-bug-subscriptions-854405/+merge/76409
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wallyworld/launchpad/private-bug-subscriptions-854405 into lp:launchpad.
=== modified file 'lib/lp/bugs/browser/bug.py'
--- lib/lp/bugs/browser/bug.py 2011-09-19 08:35:03 +0000
+++ lib/lp/bugs/browser/bug.py 2011-09-22 02:45:32 +0000
@@ -849,27 +849,19 @@
data = dict(data)
# We handle privacy changes by hand instead of leaving it to
- # the usual machinery because we must use bug.setPrivate() to
- # ensure auditing information is recorded.
+ # the usual machinery because we must use
+ # bug.setPrivacyAndSecurityRelated() to ensure auditing information is
+ # recorded.
bug = self.context.bug
- bug_before_modification = Snapshot(
- bug, providing=providedBy(bug))
private = data.pop('private')
user_will_be_subscribed = (
private and bug.getSubscribersForPerson(self.user).is_empty())
security_related = data.pop('security_related')
- private_changed = bug.setPrivate(
- private, getUtility(ILaunchBag).user)
- security_related_changed = bug.setSecurityRelated(security_related)
- if private_changed or security_related_changed:
- changed_fields = []
- if private_changed:
- changed_fields.append('private')
- self._handlePrivacyChanged(user_will_be_subscribed)
- if security_related_changed:
- changed_fields.append('security_related')
- notify(ObjectModifiedEvent(
- bug, bug_before_modification, changed_fields))
+ user = getUtility(ILaunchBag).user
+ (private_changed, security_related_changed) = (
+ bug.setPrivacyAndSecurityRelated(private, security_related, user))
+ if private_changed:
+ self._handlePrivacyChanged(user_will_be_subscribed)
if self.request.is_ajax:
if private_changed or security_related_changed:
return self._getSubscriptionDetails()
=== modified file 'lib/lp/bugs/browser/tests/test_bug_views.py'
--- lib/lp/bugs/browser/tests/test_bug_views.py 2011-09-19 08:35:03 +0000
+++ lib/lp/bugs/browser/tests/test_bug_views.py 2011-09-22 02:45:32 +0000
@@ -217,7 +217,7 @@
layer = DatabaseFunctionalLayer
def createInitializedSecrecyView(self, person=None, bug=None,
- request=None):
+ request=None, security_related=False):
"""Create and return an initialized BugSecrecyView."""
if person is None:
person = self.factory.makePerson()
@@ -227,7 +227,8 @@
view = create_initialized_view(
bug.default_bugtask, name='+secrecy', form={
'field.private': 'on',
- 'field.security_related': '',
+ 'field.security_related':
+ 'on' if security_related else 'off',
'field.actions.change': 'Change',
},
request=request)
@@ -276,7 +277,7 @@
# privacy as well as information used to populate the updated
# subscribers list.
person = self.factory.makePerson()
- bug = self.factory.makeBug()
+ bug = self.factory.makeBug(owner=person)
with person_logged_in(person):
bug.subscribe(person, person)
@@ -285,7 +286,7 @@
method='POST', form={
'field.actions.change': 'Change',
'field.private': 'on',
- 'field.security_related': 'ff'},
+ 'field.security_related': 'off'},
**extra)
view = self.createInitializedSecrecyView(person, bug, request)
result_data = simplejson.loads(view.render())
@@ -301,3 +302,18 @@
subscription_data['person_link'])
self.assertEqual(
'Discussion', subscription_data['bug_notification_level'])
+
+ [subscriber_data] = result_data['subscription_data']
+ subscriber = removeSecurityProxy(bug.default_bugtask).pillar.owner
+ self.assertEqual(
+ subscriber.name, subscriber_data['subscriber']['name'])
+ self.assertEqual('Discussion', subscriber_data['subscription_level'])
+
+ def test_set_security_related(self):
+ # Test that the bug attribute 'security_related' can be updated
+ # using the view.
+ owner = self.factory.makePerson()
+ bug = self.factory.makeBug(owner=owner)
+ self.createInitializedSecrecyView(bug=bug, security_related=True)
+ with person_logged_in(owner):
+ self.assertTrue(bug.security_related)
=== modified file 'lib/lp/bugs/browser/tests/test_bugattachment_edit_view.py'
--- lib/lp/bugs/browser/tests/test_bugattachment_edit_view.py 2011-09-19 08:35:03 +0000
+++ lib/lp/bugs/browser/tests/test_bugattachment_edit_view.py 2011-09-22 02:45:32 +0000
@@ -9,6 +9,7 @@
from canonical.testing import LaunchpadFunctionalLayer
from lp.testing import (
login_person,
+ person_logged_in,
TestCaseWithFactory,
)
from lp.testing.views import create_initialized_view
@@ -54,8 +55,9 @@
def test_change_action_private_bug(self):
# Subscribers of a private bug can edit attachments.
user = self.factory.makePerson()
- self.bug.subscribe(user, self.bug_owner)
self.bug.setPrivate(True, self.bug_owner)
+ with person_logged_in(self.bug_owner):
+ self.bug.subscribe(user, self.bug_owner)
transaction.commit()
login_person(user)
create_initialized_view(
@@ -90,8 +92,9 @@
def test_delete_action_private_bug(self):
# Subscribers of a private bug can delete attachments.
user = self.factory.makePerson()
- self.bug.subscribe(user, self.bug_owner)
self.bug.setPrivate(True, self.bug_owner)
+ with person_logged_in(self.bug_owner):
+ self.bug.subscribe(user, self.bug_owner)
login_person(user)
create_initialized_view(
self.bugattachment, name='+edit', form=self.DELETE_FORM_DATA)
=== 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-22 02:45:32 +0000
@@ -844,6 +844,7 @@
removeWatch
setPrivate
setSecurityRelated
+ setPrivacyAndSecurityRelated
setStatus
subscribe
unlinkBranch
=== modified file 'lib/lp/bugs/doc/bug-heat.txt'
--- lib/lp/bugs/doc/bug-heat.txt 2011-09-19 13:43:07 +0000
+++ lib/lp/bugs/doc/bug-heat.txt 2011-09-22 02:45:32 +0000
@@ -80,52 +80,52 @@
>>> changed = bug.setPrivate(True, bug_owner)
>>> bug.heat
- 156
+ 158
Setting the bug as security related adds another 250 heat points.
- >>> changed = bug.setSecurityRelated(True)
+ >>> changed = bug.setSecurityRelated(True, bug_owner)
>>> bug.heat
- 406
+ 408
Marking the bug public removes 150 heat points.
>>> changed = bug.setPrivate(False, bug_owner)
>>> bug.heat
- 256
+ 258
And marking it not security-related removes 250 points.
- >>> changed = bug.setSecurityRelated(False)
+ >>> changed = bug.setSecurityRelated(False, bug_owner)
>>> bug.heat
- 6
+ 8
Adding a subscriber to the bug increases its heat by 2 points.
>>> new_subscriber = factory.makePerson()
>>> subscription = bug.subscribe(new_subscriber, new_subscriber)
>>> bug.heat
- 8
+ 10
When a user unsubscribes, the bug loses 2 points of heat.
>>> bug.unsubscribe(new_subscriber, new_subscriber)
>>> bug.heat
- 6
+ 8
Should a user mark themselves as affected by the bug, it will gain 4
points of heat.
>>> bug.markUserAffected(new_subscriber)
>>> bug.heat
- 10
+ 12
If a user who was previously affected marks themself as not affected,
the bug loses 4 points of heat.
>>> bug.markUserAffected(new_subscriber, False)
>>> bug.heat
- 6
+ 8
If a user who wasn't affected by the bug marks themselve as explicitly
unaffected, the bug's heat doesn't change.
@@ -133,7 +133,7 @@
>>> unaffected_person = factory.makePerson()
>>> bug.markUserAffected(unaffected_person, False)
>>> bug.heat
- 6
+ 8
Marking the bug as a duplicate will set its heat to zero, whilst also
adding 10 points of heat to the bug it duplicates, 6 points for the
@@ -149,14 +149,14 @@
0
>>> duplicated_bug.heat
- 14
+ 16
Unmarking the bug as a duplicate restores its heat and updates the
duplicated bug's heat.
>>> bug.markAsDuplicate(None)
>>> bug.heat
- 6
+ 8
>>> duplicated_bug.heat
6
@@ -185,7 +185,7 @@
... old_value=bug.description, new_value='Some text')
>>> bug.addChange(change)
>>> bug.heat
- 6
+ 8
Getting bugs whose heat is outdated
=== modified file 'lib/lp/bugs/doc/bug.txt'
--- lib/lp/bugs/doc/bug.txt 2011-09-19 08:35:03 +0000
+++ lib/lp/bugs/doc/bug.txt 2011-09-22 02:45:32 +0000
@@ -750,7 +750,8 @@
>>> firefox_bug.security_related
False
- >>> changed = firefox_bug.setSecurityRelated(True)
+ >>> changed = firefox_bug.setSecurityRelated(True,
+ ... getUtility(ILaunchBag).user)
>>> bug_security_changed = ObjectModifiedEvent(
... firefox_bug, bug_before_modification, ["security_related"])
=== modified file 'lib/lp/bugs/doc/bugnotification-email.txt'
--- lib/lp/bugs/doc/bugnotification-email.txt 2011-09-19 13:43:07 +0000
+++ lib/lp/bugs/doc/bugnotification-email.txt 2011-09-22 02:45:32 +0000
@@ -93,7 +93,7 @@
New security related bugs are sent with a prominent warning:
- >>> changed = bug_four.setSecurityRelated(True)
+ >>> changed = bug_four.setSecurityRelated(True, getUtility(ILaunchBag).user)
>>> subject, body = generate_bug_add_email(bug_four)
>>> subject
@@ -210,7 +210,7 @@
+ edit notification email generator knows how to indent and wrap
+ descriptions, so this will appear quite nice in the actual email that
+ gets sent.
- +
+ +
+ It's also smart enough to preserve whitespace, finally!
-----------------------------
@@ -221,8 +221,7 @@
>>> edited_bug.setPrivate(True, getUtility(ILaunchBag).user)
True
-
- >>> changed = edited_bug.setSecurityRelated(True)
+ >>> changed = edited_bug.setSecurityRelated(True, getUtility(ILaunchBag).user)
>>> bug_delta = BugDelta(
... bug=edited_bug,
... bugurl="http://www.example.com/bugs/2";,
@@ -248,8 +247,7 @@
>>> edited_bug.setPrivate(False, getUtility(ILaunchBag).user)
True
-
- >>> changed = edited_bug.setSecurityRelated(False)
+ >>> changed = edited_bug.setSecurityRelated(False, getUtility(ILaunchBag).user)
>>> bug_delta = BugDelta(
... bug=edited_bug,
... bugurl="http://www.example.com/bugs/2";,
=== modified file 'lib/lp/bugs/doc/bugnotification-sending.txt'
--- lib/lp/bugs/doc/bugnotification-sending.txt 2011-09-19 08:35:03 +0000
+++ lib/lp/bugs/doc/bugnotification-sending.txt 2011-09-22 02:45:32 +0000
@@ -768,8 +768,13 @@
>>> bug_three.private
True
+When the bug is made private, the bugtask pillar owners and bug supervisors
+are subscribed in addition any other direct subscribers which already exist.
+
>>> for message in trigger_and_get_email_messages(bug_three):
... print message['To'], message.get_all('X-Launchpad-Bug-Private')
+ foo.bar@xxxxxxxxxxxxx ['yes']
+ mark@xxxxxxxxxxx ['yes']
test@xxxxxxxxxxxxx ['yes']
@@ -786,13 +791,15 @@
>>> for message in trigger_and_get_email_messages(bug_three):
... print message['To'], (
... message.get_all('X-Launchpad-Bug-Security-Vulnerability'))
+ foo.bar@xxxxxxxxxxxxx ['no']
+ mark@xxxxxxxxxxx ['no']
test@xxxxxxxxxxxxx ['no']
The presence of the security flag on a bug is, surprise, denoted by a
simple "yes":
>>> with lp_dbuser():
- ... bug_three.setSecurityRelated(True)
+ ... bug_three.setSecurityRelated(True, getUtility(ILaunchBag).user)
True
>>> bug_three.security_related
True
@@ -800,6 +807,8 @@
>>> for message in trigger_and_get_email_messages(bug_three):
... print message['To'], (
... message.get_all('X-Launchpad-Bug-Security-Vulnerability'))
+ foo.bar@xxxxxxxxxxxxx ['yes']
+ mark@xxxxxxxxxxx ['yes']
test@xxxxxxxxxxxxx ['yes']
@@ -810,8 +819,8 @@
people who have ever commented on the bug. It's a space-separated
list.
- >>> for message in trigger_and_get_email_messages(bug_three):
- ... print message.get('X-Launchpad-Bug-Commenters')
+ >>> message = trigger_and_get_email_messages(bug_three)[0]
+ >>> print message.get('X-Launchpad-Bug-Commenters')
name12
>>> from lp.registry.interfaces.person import IPersonSet
@@ -822,8 +831,8 @@
... ignored = getUtility(IBugMessageSet).createMessage(
... 'Hungry', bug_three, foo_bar, "Make me a sandwich.")
- >>> for message in trigger_and_get_email_messages(bug_three):
- ... print message.get('X-Launchpad-Bug-Commenters')
+ >>> message = trigger_and_get_email_messages(bug_three)[0]
+ >>> print message.get('X-Launchpad-Bug-Commenters')
name12 name16
It only lists each user once, no matter how many comments they've
@@ -833,8 +842,8 @@
... ignored = getUtility(IBugMessageSet).createMessage(
... 'Hungry', bug_three, foo_bar, "Make me a sandwich.")
- >>> for message in trigger_and_get_email_messages(bug_three):
- ... print message.get('X-Launchpad-Bug-Commenters')
+ >>> message = trigger_and_get_email_messages(bug_three)[0]
+ >>> print message.get('X-Launchpad-Bug-Commenters')
name12 name16
@@ -844,8 +853,8 @@
The X-Launchpad-Bug-Reporter header contains information about the Launchpad
user who originally reported the bug and opened the bug's first bug task.
- >>> for message in trigger_and_get_email_messages(bug_three):
- ... print message.get('X-Launchpad-Bug-Reporter')
+ >>> message = trigger_and_get_email_messages(bug_three)[0]
+ >>> print message.get('X-Launchpad-Bug-Reporter')
Foo Bar (name16)
@@ -1176,7 +1185,7 @@
----------------------------------------------------------------------
To: owner@xxxxxxxxxxx
...
- You received this bug notification because you are a member of
+ You received this bug notification because you are a member of
Addressless Team, which is subscribed to the bug report.
...
----------------------------------------------------------------------
@@ -1240,7 +1249,7 @@
----------------------------------------------------------------------
To: owner@xxxxxxxxxxx
...
- You received this bug notification because you are a member of
+ You received this bug notification because you are a member of
Addressless Team, which is subscribed to the bug report.
...
----------------------------------------------------------------------
@@ -1293,7 +1302,7 @@
no comment for no-priv.
<BLANKLINE>
--
- You received this bug notification because you are a member of
+ You received this bug notification because you are a member of
Addressless Team, which is subscribed to the bug report.
...
----------------------------------------------------------------------
@@ -1357,7 +1366,7 @@
----------------------------------------------------------------------
To: owner@xxxxxxxxxxx
...
- You received this bug notification because you are a member of
+ You received this bug notification because you are a member of
Addressless Team, which is subscribed to the bug report.
...
----------------------------------------------------------------------
@@ -1412,7 +1421,7 @@
+ Whatever else
<BLANKLINE>
--
- You received this bug notification because you are a member of
+ You received this bug notification because you are a member of
Addressless Team, which is subscribed to the bug report.
http://bugs.launchpad.dev/bugs/1
...
@@ -1479,7 +1488,7 @@
----------------------------------------------------------------------
To: owner@xxxxxxxxxxx
...
- You received this bug notification because you are a member of
+ You received this bug notification because you are a member of
Addressless Team, which is subscribed to the bug report.
...
----------------------------------------------------------------------
=== modified file 'lib/lp/bugs/doc/bugsubscription.txt'
--- lib/lp/bugs/doc/bugsubscription.txt 2011-09-19 08:35:03 +0000
+++ lib/lp/bugs/doc/bugsubscription.txt 2011-09-22 02:45:32 +0000
@@ -379,8 +379,9 @@
>>> linux_source_bug.getSubscribersFromDuplicates()
()
-When a bug is marked private, all its indirect subscribers become direct
-subscribers.
+When a bug is marked private, specific people like the bugtask bug supervisors
+will be automatically subscribed, and only specifically allowed existing
+direct subscribers (eg bugtask pillar maintainers) will remain subscribed.
>>> from zope.event import notify
@@ -403,16 +404,11 @@
>>> print_displayname(linux_source_bug.getDirectSubscribers())
Foo Bar
Mark Shuttleworth
- No Privileges Person
Robert Collins
- Sample Person
- Scott James Remnant
Ubuntu Team
-A private bug never has indirect subscribers. Since all our indirect
-subscribers have been made into direct subscribers, let's add another
-indirect subscriber to show that they still aren't included in the
-indirect subscriptions.
+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"))
@@ -423,9 +419,9 @@
>>> 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 only martin-pitt.
+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.
>>> linux_source_bug.setPrivate(False, getUtility(ILaunchBag).user)
True
@@ -434,17 +430,20 @@
>>> print_displayname(linux_source_bug.getDirectSubscribers())
Foo Bar
Mark Shuttleworth
- No Privileges Person
Robert Collins
- Sample Person
- Scott James Remnant
Ubuntu Team
>>> print_displayname(linux_source_bug.getIndirectSubscribers())
Martin Pitt
+ No Privileges Person
+ Sample Person
+ Scott James Remnant
>>> print_displayname(linux_source_bug.getAlsoNotifiedSubscribers())
Martin Pitt
+ No Privileges Person
+ Sample Person
+ Scott James Remnant
To find out which email addresses should receive a notification email on
a bug, and why, IBug.getBugNotificationRecipients() assembles an
@@ -458,18 +457,14 @@
>>> [(address, recipients.getReason(address)[1]) for address in addresses]
[('foo.bar@xxxxxxxxxxxxx', 'Subscriber'),
('mark@xxxxxxxxxxx', 'Subscriber'),
- ('no-priv@xxxxxxxxxxxxx', 'Subscriber'),
+ ('no-priv@xxxxxxxxxxxxx', u'Subscriber (linux-source-2.6.15 in Ubuntu)'),
('robertc@xxxxxxxxxxxxxxxxx', 'Subscriber'),
('support@xxxxxxxxxx', u'Subscriber @ubuntu-team'),
- ('test@xxxxxxxxxxxxx', 'Subscriber')]
+ ('test@xxxxxxxxxxxxx', 'Assignee')]
If IBug.getBugNotificationRecipients() is passed a BugNotificationLevel
in its `level` parameter, only structural subscribers with that
-notification level or higher will be returned. When the linux_source_bug
-was temporarily set to "private", the structural subscriber Sample Person
-was directly subscribed, thus he is returned by
-getBugNotificationRecipients() even if the parameter level is larger than
-his structural subscription setting.
+notification level or higher will be returned.
>>> recipients = linux_source_bug.getBugNotificationRecipients(
... level=BugNotificationLevel.COMMENTS)
@@ -477,10 +472,9 @@
>>> [(address, recipients.getReason(address)[1]) for address in addresses]
[('foo.bar@xxxxxxxxxxxxx', 'Subscriber'),
('mark@xxxxxxxxxxx', 'Subscriber'),
- ('no-priv@xxxxxxxxxxxxx', 'Subscriber'),
('robertc@xxxxxxxxxxxxxxxxx', 'Subscriber'),
('support@xxxxxxxxxx', u'Subscriber @ubuntu-team'),
- ('test@xxxxxxxxxxxxx', 'Subscriber')]
+ ('test@xxxxxxxxxxxxx', 'Assignee')]
When Sample Person is unsubscribed from linux_source_bug, he is no
longer included in the result of getBugNotificationRecipients() for
@@ -495,7 +489,7 @@
('mark@xxxxxxxxxxx', 'Subscriber'),
('robertc@xxxxxxxxxxxxxxxxx', 'Subscriber'),
('support@xxxxxxxxxx', u'Subscriber @ubuntu-team'),
- ('test@xxxxxxxxxxxxx', 'Subscriber')]
+ ('test@xxxxxxxxxxxxx', 'Assignee')]
...but remains included for the level LIFECYCLE.
@@ -509,14 +503,15 @@
('no-priv@xxxxxxxxxxxxx', u'Subscriber (linux-source-2.6.15 in Ubuntu)'),
('robertc@xxxxxxxxxxxxxxxxx', 'Subscriber'),
('support@xxxxxxxxxx', u'Subscriber @ubuntu-team'),
- ('test@xxxxxxxxxxxxx', 'Subscriber')]
+ ('test@xxxxxxxxxxxxx', 'Assignee')]
To find out if someone is already directly subscribed to a bug, call
IBug.isSubscribed, passing in an IPerson:
>>> linux_source_bug.isSubscribed(personset.getByName("debonzi"))
False
- >>> linux_source_bug.isSubscribed(sample_person)
+ >>> name16 = personset.getByName("name16")
+ >>> linux_source_bug.isSubscribed(name16)
True
Call isSubscribedToDupes to see if a user is directly subscribed to
=== modified file 'lib/lp/bugs/doc/initial-bug-contacts.txt'
--- lib/lp/bugs/doc/initial-bug-contacts.txt 2011-09-19 13:43:07 +0000
+++ lib/lp/bugs/doc/initial-bug-contacts.txt 2011-09-22 02:45:32 +0000
@@ -316,12 +316,11 @@
>>> transaction.commit()
>>> stub.test_emails = []
-the subscriptions remain unchanged:
+the unallowed subscribers are removed:
>>> subscriber_names(bug_one_in_ubuntu_firefox.bug)
- [u'Dafydd Harries', u'Foo Bar', u'Mark Shuttleworth',
- u'Sample Person', u'Steve Alexander', u'Ubuntu Gnome Team',
- u'Ubuntu Team']
+ [u'Foo Bar', u'Mark Shuttleworth', u'Sample Person',
+ u'Steve Alexander', u'Ubuntu Team']
Product Bug Supervisors and Bug Tasks
=== modified file 'lib/lp/bugs/doc/security-teams.txt'
--- lib/lp/bugs/doc/security-teams.txt 2011-09-19 08:35:03 +0000
+++ lib/lp/bugs/doc/security-teams.txt 2011-09-22 02:45:32 +0000
@@ -265,7 +265,7 @@
>>> subscriber_names(bug)
[u'name12', u'name16']
- >>> bug.setSecurityRelated(False)
+ >>> bug.setSecurityRelated(False, getUtility(ILaunchBag).user)
True
>>> bug.security_related
@@ -296,7 +296,7 @@
>>> bug.addTask(owner=reporter, target=distribution)
<BugTask ...>
>>> old_state = Snapshot(bug, providing=IBug)
- >>> bug.setSecurityRelated(True)
+ >>> bug.setSecurityRelated(True, getUtility(ILaunchBag).user)
True
>>> notify(ObjectModifiedEvent(bug, old_state, ['security_related']))
>>> for subscriber_name in sorted(
=== 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-22 02:45:32 +0000
@@ -850,11 +850,13 @@
@mutator_for(security_related)
@operation_parameters(security_related=copy_field(security_related))
+ @call_with(who=REQUEST_USER)
@export_write_operation()
- def setSecurityRelated(security_related):
+ def setSecurityRelated(security_related, who):
"""Set bug security.
:security_related: True/False.
+ :who: The IPerson who is making the change.
This may also cause the security contact to be subscribed
if one is registered and if the bug is not private.
@@ -862,6 +864,23 @@
Return True if a change is made, False otherwise.
"""
+ @operation_parameters(
+ private=copy_field(private),
+ security_related=copy_field(security_related),
+ )
+ @call_with(who=REQUEST_USER)
+ @export_write_operation()
+ @operation_for_version("devel")
+ def setPrivacyAndSecurityRelated(private, security_related, who):
+ """Set bug privacy and security .
+
+ :private: True/False.
+ :security_related: True/False.
+ :who: The IPerson who is making the change.
+
+ Return (private_changed, security_related_changed) tuple.
+ """
+
def getBugTask(target):
"""Return the bugtask with the specified target.
=== modified file 'lib/lp/bugs/mail/commands.py'
--- lib/lp/bugs/mail/commands.py 2011-09-19 08:35:03 +0000
+++ lib/lp/bugs/mail/commands.py 2011-09-22 02:45:32 +0000
@@ -256,13 +256,13 @@
context, providing=providedBy(context))
# Apply requested changes.
+ user = getUtility(ILaunchBag).user
if security_related:
- user = getUtility(ILaunchBag).user
if context.setPrivate(True, user):
edited = True
edited_fields.add('private')
if context.security_related != security_related:
- context.setSecurityRelated(security_related)
+ context.setSecurityRelated(security_related, user)
edited = True
edited_fields.add('security_related')
=== 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-22 02:45:32 +0000
@@ -811,7 +811,7 @@
self.updateHeat()
return sub
- def unsubscribe(self, person, unsubscribed_by):
+ def unsubscribe(self, person, unsubscribed_by, **kwargs):
"""See `IBug`."""
# Drop cached subscription info.
clear_property_cache(self)
@@ -821,9 +821,11 @@
if person is None:
person = unsubscribed_by
+ ignore_permissions = kwargs.get('ignore_permissions', False)
for sub in self.subscriptions:
if sub.person.id == person.id:
- if not sub.canBeUnsubscribedByUser(unsubscribed_by):
+ if (not ignore_permissions
+ and not sub.canBeUnsubscribedByUser(unsubscribed_by)):
raise UserCannotUnsubscribePerson(
'%s does not have permission to unsubscribe %s.' % (
unsubscribed_by.displayname,
@@ -1644,28 +1646,20 @@
return bugtask
- def setPrivate(self, private, who):
- """See `IBug`.
-
- We also record who made the change and when the change took
- place.
- """
+ def setPrivacyAndSecurityRelated(self, private, security_related, who):
+ """ See `IBug`."""
+ private_changed = False
+ security_related_changed = False
+ bug_before_modification = Snapshot(self, providing=providedBy(self))
+
+ # Before we update the privacy or security_related status, we need to
+ # reconcile the subscribers to avoid leaking private information.
+ if (self.private != private
+ or self.security_related != security_related):
+ self.reconcileSubscribers(private, security_related, who)
+
if self.private != private:
- if private:
- # Change indirect subscribers into direct subscribers
- # *before* setting private because
- # getIndirectSubscribers() behaves differently when
- # the bug is private.
- for person in self.getIndirectSubscribers():
- self.subscribe(person, who)
- subscribers_for_who = self.getSubscribersForPerson(who)
- if subscribers_for_who.is_empty():
- # We also add `who` as a subscriber, if they're not
- # already directly subscribed or part of a team
- # that's directly subscribed, so that they can
- # see the bug they've just marked private.
- self.subscribe(who, who)
-
+ private_changed = True
self.private = private
if private:
@@ -1680,25 +1674,116 @@
for attachment in self.attachments_unpopulated:
attachment.libraryfile.restricted = private
- # Correct the heat for the bug immediately, so that we don't have
- # to wait for the next calculation job for the adjusted heat.
- self.updateHeat()
- return True # Changed.
- else:
- return False # Not changed.
-
- def setSecurityRelated(self, security_related):
- """Setter for the `security_related` property."""
if self.security_related != security_related:
+ security_related_changed = True
self.security_related = security_related
+ if private_changed or security_related_changed:
# Correct the heat for the bug immediately, so that we don't have
# to wait for the next calculation job for the adjusted heat.
self.updateHeat()
- return True # Changed
- else:
- return False # Unchanged
+ if private_changed or security_related_changed:
+ changed_fields = []
+ if private_changed:
+ changed_fields.append('private')
+ if security_related_changed:
+ changed_fields.append('security_related')
+ notify(ObjectModifiedEvent(
+ self, bug_before_modification, changed_fields, user=who))
+
+ return private_changed, security_related_changed
+
+ def setPrivate(self, private, who):
+ """See `IBug`.
+
+ We also record who made the change and when the change took
+ place.
+ """
+ return self.setPrivacyAndSecurityRelated(
+ private, self.security_related, who)[0]
+
+ def setSecurityRelated(self, security_related, who):
+ """Setter for the `security_related` property."""
+ return self.setPrivacyAndSecurityRelated(
+ self.private, security_related, who)[1]
+
+ def getRequiredSubscribers(self, for_private, for_security_related, who):
+ """Return the mandatory subscribers for a bug with given attributes.
+
+ When a bug is marked as private or security related, it is required
+ that certain people be subscribed so they can access details about the
+ bug. The rules are:
+ security=true, private=true/false ->
+ subscribers = the reporter + security contact for each task
+ security=false, private=true ->
+ subscribers = the reporter + bug supervisor for each task
+ security=false, private=false ->
+ subscribers = ()
+
+ If bug supervisor or security contact is unset, fallback to bugtask
+ reporter/owner.
+ """
+ if not for_private and not for_security_related:
+ return set()
+ result = set()
+ result.add(self.owner)
+ for bugtask in self.bugtasks:
+ maintainer = bugtask.pillar.owner
+ if for_security_related:
+ result.add(bugtask.pillar.security_contact or maintainer)
+ if for_private:
+ result.add(bugtask.pillar.bug_supervisor or maintainer)
+ if for_private:
+ subscribers_for_who = self.getSubscribersForPerson(who)
+ if subscribers_for_who.is_empty():
+ result.add(who)
+ return result
+
+ def reconcileSubscribers(self, for_private, for_security_related, who):
+ """ Ensure only appropriate people are subscribed to private bugs.
+
+ When a bug is marked as either private = True or security_related =
+ True, we need to ensure that only people who are authorised to know
+ about the privileged contents of the bug remain directly subscribed
+ to it. So we:
+ 1. Get the required subscribers depending on the bug status.
+ 2. Get the auto removed subscribers depending on the bug status.
+ eg security contacts when a bug is updated to security related =
+ false.
+ 3. Get the allowed subscribers = required subscribers
+ + bugtask owners
+ 4. Remove any current direct subscribers who are not allowed or are
+ to be auto removed.
+ 5. Add any subscribers who are required.
+ """
+ current_direct_subscribers = (
+ self.getSubscriptionInfo().direct_subscribers)
+ required_subscribers = self.getRequiredSubscribers(
+ for_private, for_security_related, who)
+ allowed_subscribers = set()
+ allowed_subscribers.add(self.owner)
+ for bugtask in self.bugtasks:
+ allowed_subscribers.add(bugtask.owner)
+ allowed_subscribers.add(bugtask.pillar.owner)
+ allowed_subscribers.update(set(bugtask.pillar.drivers))
+ allowed_subscribers = required_subscribers.union(allowed_subscribers)
+
+ # If this bug is for a project that is marked as having private bugs
+ # by default, and the bug is private or security related, we will
+ # unsubscribe any unauthorised direct subscribers.
+ pillar = self.default_bugtask.pillar
+ private_project = IProduct.providedBy(pillar) and pillar.private_bugs
+ if private_project and (for_private or for_security_related):
+ subscribers_to_remove = (
+ current_direct_subscribers.difference(allowed_subscribers))
+ for subscriber in subscribers_to_remove:
+ self.unsubscribe(subscriber, who, ignore_permissions=True)
+
+ subscribers_to_add = (
+ required_subscribers.difference(current_direct_subscribers))
+ for subscriber in subscribers_to_add:
+ self.subscribe(subscriber, who)
def getBugTask(self, target):
"""See `IBug`."""
=== 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-22 02:45:32 +0000
@@ -1,11 +1,14 @@
# Copyright 2010-2011 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
+from canonical.launchpad.interfaces.lpstorm import IStore
+from lp.bugs.model.bugnotification import BugNotificationRecipient
__metaclass__ = type
from storm.store import Store
from testtools.testcase import ExpectedException
from zope.component import getUtility
+from zope.security.proxy import removeSecurityProxy
from canonical.testing.layers import DatabaseFunctionalLayer
from lp.bugs.enum import (
@@ -434,27 +437,6 @@
info = bug.getSubscriptionInfo(BugNotificationLevel.METADATA)
self.assertEqual(BugNotificationLevel.METADATA, info.level)
- 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 test_getVisibleLinkedBranches_doesnt_rtn_inaccessible_branches(self):
# If a Bug has branches linked to it that the current user
# cannot access, those branches will not be returned in its
@@ -485,6 +467,253 @@
self.assertNotIn(private_branch, linked_branches)
+class TestBugPrivateAndSecurityRelatedUpdatesMixin:
+
+ 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_supervisor = self.factory.makePerson(name='bugsupervisor')
+ product_owner = self.factory.makePerson(name='productowner')
+ bug_product = self.factory.makeProduct(
+ owner=product_owner, bug_supervisor=bug_supervisor)
+ if self.private_project:
+ removeSecurityProxy(bug_product).private_bugs = True
+ bug = self.factory.makeBug(
+ owner=bug_owner,
+ product=bug_product,
+ 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
+ # direct subscribers should include:
+ # - 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
+ # If the bug is for a private project, then other direct subscribers
+ # should be unsubscribed.
+
+ (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()
+ expected_subscribers = 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,
+ default_bugtask.pillar.bug_supervisor,
+ bug_owner, bugtask_b.pillar.owner, who))
+ if not self.private_project:
+ expected_subscribers.update(initial_subscribers)
+ self.assertContentEqual(expected_subscribers, subscribers)
+
+ def test_setPrivateTrueAndSecurityRelatedFalse(self):
+ # When a bug is marked as private=true and security_related=false, the
+ # direct subscribers should include:
+ # - 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
+ # If the bug is for a private project, then other direct subscribers
+ # should be unsubscribed.
+
+ (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()
+ bug.setPrivacyAndSecurityRelated(
+ private=True, security_related=False, who=who)
+ subscribers = bug.getDirectSubscribers()
+ expected_subscribers = set((
+ bugtask_a.pillar.bug_supervisor,
+ bugtask_a.pillar.driver,
+ bugtask_b.pillar.owner,
+ default_bugtask.pillar.owner,
+ default_bugtask.pillar.bug_supervisor,
+ bug_owner, who))
+ if not self.private_project:
+ expected_subscribers.update(initial_subscribers)
+ expected_subscribers.remove(bugtask_a.pillar.security_contact)
+ self.assertContentEqual(expected_subscribers, subscribers)
+
+ def test_setPrivateFalseAndSecurityRelatedTrue(self):
+ # When a bug is marked as private=false and security_related=true, the
+ # direct subscribers should include:
+ # - the bug reporter
+ # - the bugtask pillar security contacts (if set)
+ # - and bug/pillar owners, drivers if they are already subscribed
+ # If the bug is for a private project, then other direct subscribers
+ # should be unsubscribed.
+
+ (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,
+ 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()
+ expected_subscribers = set((
+ bugtask_a.pillar.security_contact,
+ bugtask_a.pillar.driver,
+ bugtask_b.pillar.security_contact,
+ default_bugtask.pillar.owner,
+ bug_owner))
+ if not self.private_project:
+ expected_subscribers.update(initial_subscribers)
+ expected_subscribers.remove(bugtask_a.pillar.bug_supervisor)
+ self.assertContentEqual(expected_subscribers, 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 = set(bug.getDirectSubscribers())
+ expected_direct_subscribers.remove(bugtask_a.pillar.security_contact)
+ self.assertContentEqual(expected_direct_subscribers, 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
+ )
+
+ def _fetch_notifications(self, bug, header, body):
+ return IStore(BugNotification).find(
+ BugNotification,
+ BugNotification.id == BugNotificationRecipient.bug_notificationID,
+ BugNotificationRecipient.reason_header == header,
+ BugNotificationRecipient.reason_body == body,
+ BugNotification.bug == bug)
+
+
+class TestBugPrivateAndSecurityRelatedUpdatesPrivateProject(
+ TestBugPrivateAndSecurityRelatedUpdatesMixin, TestCaseWithFactory):
+
+ def setUp(self):
+ s = super(TestBugPrivateAndSecurityRelatedUpdatesPrivateProject, self)
+ s.setUp()
+ self.private_project = True
+
+
+class TestBugPrivateAndSecurityRelatedUpdatesPublicProject(
+ TestBugPrivateAndSecurityRelatedUpdatesMixin, TestCaseWithFactory):
+
+ def setUp(self):
+ s = super(TestBugPrivateAndSecurityRelatedUpdatesPublicProject, self)
+ s.setUp()
+ self.private_project = False
+
+
class TestBugAutoConfirmation(TestCaseWithFactory):
"""Tests for auto confirming bugs"""
=== modified file 'lib/lp/bugs/model/tests/test_bugsummary.py'
--- lib/lp/bugs/model/tests/test_bugsummary.py 2011-09-19 08:35:03 +0000
+++ lib/lp/bugs/model/tests/test_bugsummary.py 2011-09-22 02:45:32 +0000
@@ -245,11 +245,11 @@
3 - count)
def test_makePrivate(self):
- product = self.factory.makeProduct()
- bug = self.factory.makeBug(product=product)
-
person_a = self.factory.makePerson()
person_b = self.factory.makePerson()
+ product = self.factory.makeProduct()
+ bug = self.factory.makeBug(product=product, owner=person_b)
+
bug.subscribe(person=person_a, subscribed_by=person_a)
# Make the bug private. We have to use the Python API to ensure
@@ -266,7 +266,7 @@
1)
self.assertEqual(
self.getCount(person_b, BugSummary.product == product),
- 0)
+ 1)
# Confirm implicit subscriptions work too.
self.assertEqual(
self.getCount(bug.owner, BugSummary.product == product),
=== modified file 'lib/lp/bugs/scripts/bugimport.py'
--- lib/lp/bugs/scripts/bugimport.py 2011-09-19 08:35:03 +0000
+++ lib/lp/bugs/scripts/bugimport.py 2011-09-22 02:45:32 +0000
@@ -328,9 +328,9 @@
self.createAttachments(bug, msg, commentnode)
# set up bug
- bug.setPrivate(get_value(bugnode, 'private') == 'True', owner)
- bug.setSecurityRelated(
- get_value(bugnode, 'security_related') == 'True')
+ private = get_value(bugnode, 'private') == 'True'
+ security_related = get_value(bugnode, 'security_related') == 'True'
+ bug.setPrivacyAndSecurityRelated(private, security_related, owner)
bug.name = get_value(bugnode, 'nickname')
description = get_value(bugnode, 'description')
if description:
=== modified file 'lib/lp/bugs/stories/bug-privacy/xx-bug-privacy.txt'
--- lib/lp/bugs/stories/bug-privacy/xx-bug-privacy.txt 2011-09-19 08:35:03 +0000
+++ lib/lp/bugs/stories/bug-privacy/xx-bug-privacy.txt 2011-09-22 02:45:32 +0000
@@ -33,11 +33,12 @@
http://bugs.launchpad.dev/debian/+source/mozilla-firefox/+bug/2
-All the implicit subscribers have been made explicit.
+Subscribers have been updated according to the privacy rules.
>>> browser.open(
... "http://launchpad.dev/bugs/2/+bug-portlet-subscribers-details";)
>>> print_direct_subscribers(browser.contents)
+ Mark Shuttleworth (Unsubscribe)
Sample Person (Unsubscribe)
Steve Alexander (Unsubscribe)
Ubuntu Team (Unsubscribe)
=== modified file 'lib/lp/bugs/subscribers/bug.py'
--- lib/lp/bugs/subscribers/bug.py 2011-08-15 13:59:05 +0000
+++ lib/lp/bugs/subscribers/bug.py 2011-09-22 02:45:32 +0000
@@ -45,14 +45,6 @@
Subscribe the security contacts for a bug when it becomes
security-related, and add notifications for the changes.
"""
- if (event.object.security_related and
- not event.object_before_modification.security_related):
- # The bug turned out to be security-related, subscribe the security
- # contact.
- for pillar in bug.affected_pillars:
- if pillar.security_contact is not None:
- bug.subscribe(pillar.security_contact, IPerson(event.user))
-
bug_delta = get_bug_delta(
old_bug=event.object_before_modification,
new_bug=event.object, user=IPerson(event.user))
=== modified file 'lib/lp/bugs/tests/test_bugchanges.py'
--- lib/lp/bugs/tests/test_bugchanges.py 2011-09-19 08:35:03 +0000
+++ lib/lp/bugs/tests/test_bugchanges.py 2011-09-22 02:45:32 +0000
@@ -18,7 +18,6 @@
from canonical.launchpad.webapp.publisher import canonical_url
from canonical.testing.layers import LaunchpadFunctionalLayer
from lp.bugs.enum import BugNotificationLevel
-from lp.bugs.interfaces.bug import IBug
from lp.bugs.interfaces.bugtask import (
BugTaskImportance,
BugTaskStatus,
@@ -549,10 +548,7 @@
def test_make_private(self):
# Marking a bug as private adds items to the bug's activity log
# and notifications.
- bug_before_modification = Snapshot(self.bug, providing=IBug)
self.bug.setPrivate(True, self.user)
- notify(ObjectModifiedEvent(
- self.bug, bug_before_modification, ['private'], self.user))
visibility_change_activity = {
'person': self.user,
@@ -577,10 +573,7 @@
self.saveOldChanges(private_bug)
self.assertTrue(private_bug.private)
- bug_before_modification = Snapshot(private_bug, providing=IBug)
private_bug.setPrivate(False, self.user)
- notify(ObjectModifiedEvent(
- private_bug, bug_before_modification, ['private'], self.user))
visibility_change_activity = {
'person': self.user,
@@ -648,7 +641,7 @@
def test_mark_as_security_vulnerability(self):
# Marking a bug as a security vulnerability adds to the bug's
# activity log and sends a notification.
- self.bug.setSecurityRelated(False)
+ self.bug.setSecurityRelated(False, self.user)
self.changeAttribute(self.bug, 'security_related', True)
security_change_activity = {
@@ -672,7 +665,8 @@
def test_unmark_as_security_vulnerability(self):
# Unmarking a bug as a security vulnerability adds to the
# bug's activity log and sends a notification.
- self.bug.setSecurityRelated(True)
+ self.bug.setSecurityRelated(True, self.user)
+ self.saveOldChanges()
self.changeAttribute(self.bug, 'security_related', False)
security_change_activity = {
=== modified file 'lib/lp/testing/factory.py'
--- lib/lp/testing/factory.py 2011-09-21 19:10:33 +0000
+++ lib/lp/testing/factory.py 2011-09-22 02:45:32 +0000
@@ -1627,8 +1627,8 @@
return branch.createBranchRevision(sequence, revision)
def makeBug(self, product=None, owner=None, bug_watch_url=None,
- private=False, date_closed=None, title=None,
- date_created=None, description=None, comment=None,
+ private=False, security_related=False, date_closed=None,
+ title=None, date_created=None, description=None, comment=None,
status=None, distribution=None, milestone=None, series=None,
tags=None, sourcepackagename=None):
"""Create and return a new, arbitrary Bug.
@@ -1679,6 +1679,7 @@
sourcepackagename=sourcepackagename)
create_bug_params = CreateBugParams(
owner, title, comment=comment, private=private,
+ security_related=security_related,
datecreated=date_created, description=description,
status=status, tags=tags)
create_bug_params.setBugTarget(
