Merge lp:~brian-murray/launchpad/bug-97633 into lp:launchpad

Proposed by Brian Murray
Status: Merged
Approved by: Graham Binns
Approved revision: no longer in the source branch.
Merged at revision: 11046
Proposed branch: lp:~brian-murray/launchpad/bug-97633
Merge into: lp:launchpad
Diff against target: 435 lines (+205/-24)
12 files modified
lib/canonical/launchpad/doc/notification-recipient-set.txt (+14/-1)
lib/canonical/launchpad/interfaces/launchpad.py (+8/-1)
lib/canonical/launchpad/mailnotification.py (+34/-11)
lib/lp/bugs/doc/bugnotification-email.txt (+1/-1)
lib/lp/bugs/doc/bugnotification-sending.txt (+2/-2)
lib/lp/bugs/doc/bugnotifications.txt (+1/-1)
lib/lp/bugs/doc/bugtask.txt (+2/-2)
lib/lp/bugs/mail/tests/test_bug_task_assignment.py (+120/-0)
lib/lp/bugs/model/bugtask.py (+1/-1)
lib/lp/bugs/tests/test_bugchanges.py (+2/-2)
lib/lp/bugs/tests/test_bugtask.py (+1/-1)
lib/lp/services/mail/notificationrecipientset.py (+19/-1)
To merge this branch: bzr merge lp:~brian-murray/launchpad/bug-97633
Reviewer Review Type Date Requested Status
Graham Binns (community) code Approve
Review via email: mp+28109@code.launchpad.net

Commit message

Modify new bug subscription message to identify assignment and assigner instead of treating it like a subscription.

Description of the change

= Summary =

Bug 97633 describes three issues two of which are addressed in this bug report
while the third was put into a separate bug report (bug 596974).

1) Email notification says you have been subscribed when you were assigned
2) Receive 2 emails one about the bug subscription and one about bug
transition
3) Email notification does identify self-assignment (bug 596974)

== Proposed fix ==

1) This is caused by generate_bug_add_email in mailnotification.py not
checking if the new_recipient was an assignee. Additionally, there is no
tracking of whom the assigner was.

2) This is caused by notify_bugtask_edited calling both
add_bug_change_notifications and _send_bug_details_to_new_subscribers for
task assignment changes.

== Implementation details ==

While determining how to prevent the assignee from receiving a notification
about the bug task assignee changing I discovered that
INotificationRecipientSet did not have a method for removing a recipient from
the recipient list and had to create one. Additionally, there were no tests
for testing the email notifications sent to bug assignees so I wrote them in
test_bug_task_assignment.py.

When I was reading all the code to understand how the notification system
worked I also fixed some typos.

lib/canonical/launchpad/doc/notification-recipient-set.txt
 * Created a test for removing recipients

lib/canonical/launchpad/interfaces/launchpad.py
 * documented remove method

lib/canonical/launchpad/mailnotification.py
 * added in event_creator tracking for who assigned the task
 * added in "You have been assigned" to email contents
 * added new_subscribers parament to add_bug_change_notifications

lib/lp/bugs/mail/tests/test_bug_task_assignment.py
 * new file with tests for changes

lib/lp/services/mail/notificationrecipientset.py
 * added in remove method

lib/lp/bugs/doc/bugnotification-email.txt
lib/lp/bugs/doc/bugnotification-sending.txt
lib/lp/bugs/doc/bugnotifications.txt
lib/lp/bugs/doc/bugtask.txt
lib/lp/bugs/model/bugtask.py
lib/lp/bugs/tests/test_bugtask.py
 * Fixed some typos

== Tests ==

bin/test -vv -t test_bug_task_assignment -t notification-recipient-set

Test bug subscription email messages passes too:

bin/test -vv -t test_bug_subscribe

To post a comment you must log in.
Revision history for this message
Graham Binns (gmb) :
review: Approve (code)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'lib/canonical/launchpad/doc/notification-recipient-set.txt'
--- lib/canonical/launchpad/doc/notification-recipient-set.txt 2010-03-18 18:55:02 +0000
+++ lib/canonical/launchpad/doc/notification-recipient-set.txt 2010-06-23 13:33:39 +0000
@@ -202,7 +202,20 @@
202 >>> recipients.getReason(sample_person)202 >>> recipients.getReason(sample_person)
203 ('Notified for fun.', 'Fun')203 ('Notified for fun.', 'Fun')
204204
205== A person first impression stick ==205== Removing recipients ==
206
207It is also possible to remove a person from the NotificationRecipientSet():
208
209 >>> recipients = NotificationRecipientSet()
210 >>> recipients.add(
211 ... [sample_person, no_priv, cprov], 'Notified for fun.', 'Fun')
212 >>> [person.displayname for person in recipients.getRecipients()]
213 [u'Celso Providelo', u'No Privileges Person', u'Sample Person']
214 >>> recipients.remove([sample_person, cprov])
215 >>> [person.displayname for person in recipients.getRecipients()]
216 [u'No Privileges Person']
217
218== A person's first impression sticks ==
206219
207In general, the most specific rationale is used for a given email.220In general, the most specific rationale is used for a given email.
208A rationale given for a person is considered more221A rationale given for a person is considered more
209222
=== modified file 'lib/canonical/launchpad/interfaces/launchpad.py'
--- lib/canonical/launchpad/interfaces/launchpad.py 2010-06-14 20:12:43 +0000
+++ lib/canonical/launchpad/interfaces/launchpad.py 2010-06-23 13:33:39 +0000
@@ -594,7 +594,7 @@
594 """594 """
595595
596 def add(person, reason, header):596 def add(person, reason, header):
597 """Add a person or sequence of person to the recipients list.597 """Add a person or a sequence of persons to the recipients list.
598598
599 When the added person is a team without an email address, all its599 When the added person is a team without an email address, all its
600 members emails will be added. If the person is already in the600 members emails will be added. If the person is already in the
@@ -608,6 +608,13 @@
608 X-Launchpad-Message-Rationale header.608 X-Launchpad-Message-Rationale header.
609 """609 """
610610
611 def remove(person):
612 """Remove a person or a list of persons from the recipients list.
613
614 :param person: The `IPerson` or a sequence of `IPerson`
615 that will removed from the recipients list.
616 """
617
611 def update(recipient_set):618 def update(recipient_set):
612 """Updates this instance's reasons with reasons from another set.619 """Updates this instance's reasons with reasons from another set.
613620
614621
=== modified file 'lib/canonical/launchpad/mailnotification.py'
--- lib/canonical/launchpad/mailnotification.py 2010-06-16 08:36:27 +0000
+++ lib/canonical/launchpad/mailnotification.py 2010-06-23 13:33:39 +0000
@@ -27,7 +27,8 @@
27from canonical.config import config27from canonical.config import config
28from canonical.database.sqlbase import block_implicit_flushes28from canonical.database.sqlbase import block_implicit_flushes
29from lp.bugs.adapters.bugdelta import BugDelta29from lp.bugs.adapters.bugdelta import BugDelta
30from lp.bugs.adapters.bugchange import BugDuplicateChange, get_bug_changes30from lp.bugs.adapters.bugchange import (
31 BugDuplicateChange, get_bug_changes, BugTaskAssigneeChange)
31from canonical.launchpad.helpers import (32from canonical.launchpad.helpers import (
32 get_contact_email_addresses, get_email_template, shortlist)33 get_contact_email_addresses, get_email_template, shortlist)
33from canonical.launchpad.interfaces import (34from canonical.launchpad.interfaces import (
@@ -161,7 +162,8 @@
161162
162163
163def _send_bug_details_to_new_bug_subscribers(164def _send_bug_details_to_new_bug_subscribers(
164 bug, previous_subscribers, current_subscribers, subscribed_by=None):165 bug, previous_subscribers, current_subscribers, subscribed_by=None,
166 event_creator=None):
165 """Send an email containing full bug details to new bug subscribers.167 """Send an email containing full bug details to new bug subscribers.
166168
167 This function is designed to handle situations where bugtasks get169 This function is designed to handle situations where bugtasks get
@@ -198,7 +200,7 @@
198 reason, rationale = recipients.getReason(to_addr)200 reason, rationale = recipients.getReason(to_addr)
199 subject, contents = generate_bug_add_email(201 subject, contents = generate_bug_add_email(
200 bug, new_recipients=True, subscribed_by=subscribed_by,202 bug, new_recipients=True, subscribed_by=subscribed_by,
201 reason=reason)203 reason=reason, event_creator=event_creator)
202 msg = bug_notification_builder.build(204 msg = bug_notification_builder.build(
203 from_addr, to_addr, contents, subject, email_date,205 from_addr, to_addr, contents, subject, email_date,
204 rationale=rationale, references=references)206 rationale=rationale, references=references)
@@ -339,9 +341,8 @@
339 template % {'url': file_alias_url, 'error_msg': message},341 template % {'url': file_alias_url, 'error_msg': message},
340 headers={'X-Launchpad-Unhandled-Email': message})342 headers={'X-Launchpad-Unhandled-Email': message})
341343
342
343def generate_bug_add_email(bug, new_recipients=False, reason=None,344def generate_bug_add_email(bug, new_recipients=False, reason=None,
344 subscribed_by=None):345 subscribed_by=None, event_creator=None):
345 """Generate a new bug notification from the given IBug.346 """Generate a new bug notification from the given IBug.
346347
347 If new_recipients is supplied we generate a notification explaining348 If new_recipients is supplied we generate a notification explaining
@@ -389,7 +390,14 @@
389 }390 }
390391
391 if new_recipients:392 if new_recipients:
392 contents += "You have been subscribed to a %(visibility)s bug"393 if "assignee" in reason:
394 contents += "You have been assigned a bug task for a %(visibility)s bug"
395 if event_creator is not None:
396 contents += " by %(assigner)s"
397 content_substitutions['assigner'] = (
398 event_creator.unique_displayname)
399 else:
400 contents += "You have been subscribed to a %(visibility)s bug"
393 if subscribed_by is not None:401 if subscribed_by is not None:
394 contents += " by %(subscribed_by)s"402 contents += " by %(subscribed_by)s"
395 content_substitutions['subscribed_by'] = (403 content_substitutions['subscribed_by'] = (
@@ -578,7 +586,8 @@
578 key=operator.attrgetter('displayname'))586 key=operator.attrgetter('displayname'))
579587
580588
581def add_bug_change_notifications(bug_delta, old_bugtask=None):589def add_bug_change_notifications(bug_delta, old_bugtask=None,
590 new_subscribers=None):
582 """Generate bug notifications and add them to the bug."""591 """Generate bug notifications and add them to the bug."""
583 changes = get_bug_changes(bug_delta)592 changes = get_bug_changes(bug_delta)
584 recipients = bug_delta.bug.getBugNotificationRecipients(593 recipients = bug_delta.bug.getBugNotificationRecipients(
@@ -603,6 +612,13 @@
603 include_master_dupe_subscribers=False))612 include_master_dupe_subscribers=False))
604 bug_delta.bug.addChange(613 bug_delta.bug.addChange(
605 change, recipients=no_dupe_master_recipients)614 change, recipients=no_dupe_master_recipients)
615 elif (isinstance(change, BugTaskAssigneeChange) and
616 new_subscribers is not None):
617 for person in new_subscribers:
618 reason, rationale = recipients.getReason(person)
619 if 'Assignee' in rationale:
620 recipients.remove(person)
621 bug_delta.bug.addChange(change, recipients=recipients)
606 else:622 else:
607 bug_delta.bug.addChange(change, recipients=recipients)623 bug_delta.bug.addChange(change, recipients=recipients)
608 else:624 else:
@@ -625,13 +641,20 @@
625 bugtask_deltas=bugtask_delta,641 bugtask_deltas=bugtask_delta,
626 user=IPerson(event.user))642 user=IPerson(event.user))
627643
628 add_bug_change_notifications(644 event_creator = IPerson(event.user)
629 bug_delta, old_bugtask=event.object_before_modification)
630
631 previous_subscribers = event.object_before_modification.bug_subscribers645 previous_subscribers = event.object_before_modification.bug_subscribers
632 current_subscribers = event.object.bug_subscribers646 current_subscribers = event.object.bug_subscribers
647 prev_subs_set = set(previous_subscribers)
648 cur_subs_set = set(current_subscribers)
649 new_subs = cur_subs_set.difference(prev_subs_set)
650
651 add_bug_change_notifications(
652 bug_delta, old_bugtask=event.object_before_modification,
653 new_subscribers=new_subs)
654
633 _send_bug_details_to_new_bug_subscribers(655 _send_bug_details_to_new_bug_subscribers(
634 event.object.bug, previous_subscribers, current_subscribers)656 event.object.bug, previous_subscribers, current_subscribers,
657 event_creator=event_creator)
635 update_security_contact_subscriptions(modified_bugtask, event)658 update_security_contact_subscriptions(modified_bugtask, event)
636659
637660
638661
=== modified file 'lib/lp/bugs/doc/bugnotification-email.txt'
--- lib/lp/bugs/doc/bugnotification-email.txt 2010-06-10 10:54:51 +0000
+++ lib/lp/bugs/doc/bugnotification-email.txt 2010-06-23 13:33:39 +0000
@@ -507,7 +507,7 @@
507 X-Launchpad-Bug-Commenters: name12507 X-Launchpad-Bug-Commenters: name12
508508
509The build() method of a builder accepts a number of parameters and509The build() method of a builder accepts a number of parameters and
510returns an instance of email.MIMEText. The most basic invokation of510returns an instance of email.MIMEText. The most basic invocation of
511this method requires a from address, a to address, a body, a subject511this method requires a from address, a to address, a body, a subject
512and a sending date for the mail.512and a sending date for the mail.
513513
514514
=== modified file 'lib/lp/bugs/doc/bugnotification-sending.txt'
--- lib/lp/bugs/doc/bugnotification-sending.txt 2010-06-11 13:26:07 +0000
+++ lib/lp/bugs/doc/bugnotification-sending.txt 2010-06-23 13:33:39 +0000
@@ -153,7 +153,7 @@
153 ... flush_database_updates()153 ... flush_database_updates()
154154
155155
156To every message that get sent out, [Bug $bugid] is prefixed to the156To every message that gets sent out, [Bug $bugid] is prefixed to the
157subject. It gets prefixed only if it's not already present in the157subject. It gets prefixed only if it's not already present in the
158subject, though, which is often the case when someone replies via email.158subject, though, which is often the case when someone replies via email.
159159
@@ -435,7 +435,7 @@
435435
436 >>> flush_notifications()436 >>> flush_notifications()
437437
438If Sample Person does a few changes, and Foo Bar steps in a does a438If Sample Person does a few changes, and Foo Bar steps in and does a
439change in between, three notifications will be sent:439change in between, three notifications will be sent:
440440
441 >>> from canonical.launchpad.interfaces import IPersonSet441 >>> from canonical.launchpad.interfaces import IPersonSet
442442
=== modified file 'lib/lp/bugs/doc/bugnotifications.txt'
--- lib/lp/bugs/doc/bugnotifications.txt 2009-06-12 16:36:02 +0000
+++ lib/lp/bugs/doc/bugnotifications.txt 2010-06-23 13:33:39 +0000
@@ -17,7 +17,7 @@
1717
18 >>> login("test@canonical.com")18 >>> login("test@canonical.com")
1919
20Here are the things that cause bug notifications to be add, and what20Here are the things that cause bug notifications to be added and what
21those notifications look like.21those notifications look like.
2222
2323
2424
=== modified file 'lib/lp/bugs/doc/bugtask.txt'
--- lib/lp/bugs/doc/bugtask.txt 2010-05-25 16:45:26 +0000
+++ lib/lp/bugs/doc/bugtask.txt 2010-06-23 13:33:39 +0000
@@ -571,7 +571,7 @@
571 Traceback (most recent call last):571 Traceback (most recent call last):
572 ...572 ...
573 UserCannotEditBugTaskAssignee: Regular users can assign and unassign573 UserCannotEditBugTaskAssignee: Regular users can assign and unassign
574 only themselves and their teams. Only project onwers, bug supervisors,574 only themselves and their teams. Only project owners, bug supervisors,
575 drivers and release managers can assign others.575 drivers and release managers can assign others.
576576
577 >>> devel_focus_alsa_utils_task.transitionToAssignee(sample_person)577 >>> devel_focus_alsa_utils_task.transitionToAssignee(sample_person)
@@ -592,7 +592,7 @@
592 Traceback (most recent call last):592 Traceback (most recent call last):
593 ...593 ...
594 UserCannotEditBugTaskAssignee: Regular users can assign and unassign594 UserCannotEditBugTaskAssignee: Regular users can assign and unassign
595 only themselves and their teams. Only project onwers, bug supervisors,595 only themselves and their teams. Only project owners, bug supervisors,
596 drivers and release managers can assign others.596 drivers and release managers can assign others.
597597
598 >>> login('test@canonical.com')598 >>> login('test@canonical.com')
599599
=== added file 'lib/lp/bugs/mail/tests/test_bug_task_assignment.py'
--- lib/lp/bugs/mail/tests/test_bug_task_assignment.py 1970-01-01 00:00:00 +0000
+++ lib/lp/bugs/mail/tests/test_bug_task_assignment.py 2010-06-23 13:33:39 +0000
@@ -0,0 +1,120 @@
1# Copyright 2010 Canonical Ltd. This software is licensed under the
2# GNU Affero General Public License version 3 (see the file LICENSE).
3
4"""Tests for Bug task assignment-related email tests."""
5
6from unittest import TestLoader
7
8import transaction
9
10from zope.component import getUtility
11from zope.interface import providedBy
12from zope.event import notify
13
14from canonical.testing import DatabaseFunctionalLayer
15from canonical.launchpad.database import BugNotification
16from canonical.launchpad.webapp.interfaces import ILaunchBag
17
18from lp.services.mail import stub
19from lp.bugs.scripts.bugnotification import construct_email_notifications
20from lp.testing import TestCaseWithFactory
21
22from lazr.lifecycle.event import (ObjectModifiedEvent)
23from lazr.lifecycle.snapshot import Snapshot
24
25
26class TestAssignmentNotification(TestCaseWithFactory):
27 """Test emails sent when assigned a bug report."""
28
29 layer = DatabaseFunctionalLayer
30
31 def setUp(self):
32 # Run the tests as a logged-in user.
33 super(TestAssignmentNotification, self).setUp(
34 user='test@canonical.com')
35 self.user = getUtility(ILaunchBag).user
36 self.product = self.factory.makeProduct(owner=self.user,
37 name='rebirth')
38 self.bug = self.factory.makeBug(product=self.product)
39 self.bug_task = self.bug.getBugTask(self.product)
40 self.bug_task_before_modification = Snapshot(self.bug_task,
41 providing=providedBy(self.bug_task))
42 self.person_assigned_email = 'stever@example.com'
43 self.person_assigned = self.factory.makePerson(
44 name='assigned', displayname='Steve Rogers',
45 email=self.person_assigned_email)
46 self.team_member_email = 'hankp@example.com'
47 self.team_member = self.factory.makePerson(
48 name='giantman', displayname='Hank Pym',
49 email=self.team_member_email)
50 self.team_assigned = self.factory.makeTeam(
51 name='avengers', owner=self.user)
52 self.team_assigned.addMember(self.team_member, self.user)
53 # adding people to a team generates email
54 transaction.commit()
55 del stub.test_emails[:]
56
57 def test_assignee_notification_message(self):
58 """Test notification string when a person is assigned a task by
59 someone else."""
60 self.assertEqual(len(stub.test_emails), 0, 'emails in queue')
61 self.bug_task.transitionToAssignee(self.person_assigned)
62 notify(ObjectModifiedEvent(
63 self.bug_task, self.bug_task_before_modification,
64 ['assignee'], user=self.user))
65 transaction.commit()
66 self.assertEqual(len(stub.test_emails), 1, 'email not sent')
67 rationale = 'You have been assigned a bug task for a public bug by Sample Person'
68 msg = stub.test_emails[-1][2]
69 self.assertTrue(rationale in msg,
70 '%s not in\n%s\n' % (rationale, msg))
71
72 def test_assignee_not_a_subscriber(self):
73 """Test that a new recipient being assigned a bug task does send
74 a NEW message."""
75 self.assertEqual(len(stub.test_emails), 0, 'emails in queue')
76 self.bug_task.transitionToAssignee(self.person_assigned)
77 notify(ObjectModifiedEvent(
78 self.bug_task, self.bug_task_before_modification,
79 ['assignee'], user=self.user))
80 transaction.commit()
81 self.assertEqual(len(stub.test_emails), 1, 'email not sent')
82 new_message = '[NEW]'
83 msg = stub.test_emails[-1][2]
84 self.assertTrue(new_message in msg,
85 '%s not in \n%s\n' % (new_message, msg))
86
87 def test_assignee_new_subscriber(self):
88 """Build a list of people who will receive e-mails about the bug
89 task changes and ensure the assignee is not one."""
90 self.bug_task.transitionToAssignee(self.person_assigned)
91 notify(ObjectModifiedEvent(
92 self.bug_task, self.bug_task_before_modification,
93 ['assignee'], user=self.user))
94 latest_notification = BugNotification.selectFirst(orderBy='-id')
95 notifications, messages = construct_email_notifications([latest_notification])
96 self.assertEqual(len(notifications), 1,
97 'email notication not created')
98 receivers = [message['To'] for message in messages]
99 self.assertFalse(self.person_assigned_email in receivers,
100 'Assignee was emailed about the bug task change')
101
102 def test_team_assigned_new_subscriber(self):
103 """Assign a team, who is not subscribed to a bug, a bug task and
104 ensure that team members do not receive an e-mail about the bug
105 task changes."""
106 self.bug_task.transitionToAssignee(self.team_assigned)
107 notify(ObjectModifiedEvent(
108 self.bug_task, self.bug_task_before_modification,
109 ['assignee'], user=self.user))
110 latest_notification = BugNotification.selectFirst(orderBy='-id')
111 notifications, messages = construct_email_notifications([latest_notification])
112 self.assertEqual(len(notifications), 1,
113 'email notification not created')
114 receivers = [message['To'] for message in messages]
115 self.assertFalse(self.team_member_email in receivers,
116 'Team member was emailed about the bug task change')
117
118
119def test_suite():
120 return TestLoader().loadTestsFromName(__name__)
0121
=== modified file 'lib/lp/bugs/model/bugtask.py'
--- lib/lp/bugs/model/bugtask.py 2010-06-16 16:46:06 +0000
+++ lib/lp/bugs/model/bugtask.py 2010-06-23 13:33:39 +0000
@@ -992,7 +992,7 @@
992 if not self.canTransitionToAssignee(assignee):992 if not self.canTransitionToAssignee(assignee):
993 raise UserCannotEditBugTaskAssignee(993 raise UserCannotEditBugTaskAssignee(
994 'Regular users can assign and unassign only themselves and '994 'Regular users can assign and unassign only themselves and '
995 'their teams. Only project onwers, bug supervisors, drivers '995 'their teams. Only project owners, bug supervisors, drivers '
996 'and release managers can assign others.')996 'and release managers can assign others.')
997997
998 now = datetime.datetime.now(pytz.UTC)998 now = datetime.datetime.now(pytz.UTC)
999999
=== modified file 'lib/lp/bugs/tests/test_bugchanges.py'
--- lib/lp/bugs/tests/test_bugchanges.py 2010-06-09 09:03:51 +0000
+++ lib/lp/bugs/tests/test_bugchanges.py 2010-06-23 13:33:39 +0000
@@ -1044,8 +1044,8 @@
1044 expected_notification=expected_notification)1044 expected_notification=expected_notification)
10451045
1046 def test_unassign_bugtask(self):1046 def test_unassign_bugtask(self):
1047 # Unassigning a bug task to someone adds entries to the bug1047 # Unassigning a bug task assigned to someone adds entries to the
1048 # activity and notifications sets.1048 # bug activity and notifications sets.
1049 old_assignee = self.factory.makePerson()1049 old_assignee = self.factory.makePerson()
1050 self.bug_task.transitionToAssignee(old_assignee)1050 self.bug_task.transitionToAssignee(old_assignee)
1051 self.saveOldChanges()1051 self.saveOldChanges()
10521052
=== modified file 'lib/lp/bugs/tests/test_bugtask.py'
--- lib/lp/bugs/tests/test_bugtask.py 2010-05-27 13:51:06 +0000
+++ lib/lp/bugs/tests/test_bugtask.py 2010-06-23 13:33:39 +0000
@@ -506,7 +506,7 @@
506 layer = LaunchpadFunctionalLayer506 layer = LaunchpadFunctionalLayer
507507
508 def setUp(self):508 def setUp(self):
509 """Crate the test setup.509 """Create the test setup.
510510
511 We need511 We need
512 - bug task targets (a product and a product series, or512 - bug task targets (a product and a product series, or
513513
=== modified file 'lib/lp/services/mail/notificationrecipientset.py'
--- lib/lp/services/mail/notificationrecipientset.py 2010-02-16 20:36:48 +0000
+++ lib/lp/services/mail/notificationrecipientset.py 2010-06-23 13:33:39 +0000
@@ -91,7 +91,7 @@
9191
92 for person in persons:92 for person in persons:
93 assert IPerson.providedBy(person), (93 assert IPerson.providedBy(person), (
94 'You can only add() IPerson: %r' % person)94 'You can only add() an IPerson: %r' % person)
95 # If the person already has a rationale, keep the first one.95 # If the person already has a rationale, keep the first one.
96 if person in self._personToRationale:96 if person in self._personToRationale:
97 continue97 continue
@@ -111,6 +111,24 @@
111 or (old_person.isTeam() and not person.isTeam())):111 or (old_person.isTeam() and not person.isTeam())):
112 self._emailToPerson[email] = person112 self._emailToPerson[email] = person
113113
114 def remove(self, persons):
115 """See `INotificationRecipientSet`."""
116 from zope.security.proxy import removeSecurityProxy
117 if IPerson.providedBy(persons):
118 persons = [persons]
119 for person in persons:
120 assert IPerson.providedBy(person), (
121 'You can only remove() an IPerson: %r' % person)
122 if person in self._personToRationale:
123 del self._personToRationale[person]
124 for removed_person in emailPeople(person):
125 # Bypass zope's security because IEmailAddress.email is
126 # not public.
127 preferred_email = removeSecurityProxy(
128 removed_person.preferredemail)
129 email = str(preferred_email.email)
130 self._receiving_people.discard((email, removed_person))
131
114 def update(self, recipient_set):132 def update(self, recipient_set):
115 """See `INotificationRecipientSet`."""133 """See `INotificationRecipientSet`."""
116 for person in recipient_set:134 for person in recipient_set: