Launchpad itself

Merge lp:~benji/launchpad/bug-739141 into lp:launchpad

  1. bug-739141
  2. Merge into devel
Proposed by Benji York
Status: Merged
Approved by: Gary Poster
Approved revision: no longer in the source branch.
Merged at revision: 12952
Proposed branch: lp:~benji/launchpad/bug-739141
Merge into: lp:launchpad
Diff against target: 100 lines (+39/-3)
3 files modified
database/schema/security.cfg (+1/-0)
lib/lp/bugs/subscribers/bug.py (+9/-1)
lib/lp/bugs/subscribers/tests/test_bug.py (+29/-2)
To merge this branch: bzr merge lp:~benji/launchpad/bug-739141
Reviewer Review Type Date Requested Status
Gary Poster (community) Approve
Review via email: mp+59260@code.launchpad.net

Commit message

[r=gary][bug=739141] make send_bug_details_to_new_bug_subscribers respect Person.selfgenerated_bugnotifications

Description of the change

Bug 739141 describes a bug notification message being sent to a user about an action that they performed despite the fact that they have the "Send me bug notifications for changes I make" option disabled.

It turns out that in the given situation the user was a new subscriber to the bug. Those notifications are handled by send_bug_details_to_new_bug_subscribers which did not respect the selfgenerated_bugnotifications flag.

The make lint report is clean.

To run the test:

bin/test -c -t test_self_notification_preference_respected -m lp.bugs.subscribers.tests

To post a comment you must log in.
Revision history for this message
Gary Poster (gary) wrote :

It was a good hunt, but the killing blow was easy. :-)

Thank you!

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'database/schema/security.cfg'
--- database/schema/security.cfg 2011-04-21 14:01:20 +0000
+++ database/schema/security.cfg 2011-04-28 21:06:30 +0000
@@ -1705,6 +1705,7 @@
1705public.language = SELECT1705public.language = SELECT
1706public.person = SELECT, UPDATE1706public.person = SELECT, UPDATE
1707public.personlanguage = SELECT1707public.personlanguage = SELECT
1708public.personsettings = SELECT
1708public.teammembership = SELECT1709public.teammembership = SELECT
1709public.teamparticipation = SELECT1710public.teamparticipation = SELECT
1710public.validpersoncache = SELECT1711public.validpersoncache = SELECT
17111712
=== modified file 'lib/lp/bugs/subscribers/bug.py'
--- lib/lp/bugs/subscribers/bug.py 2011-03-07 17:39:20 +0000
+++ lib/lp/bugs/subscribers/bug.py 2011-04-28 21:06:30 +0000
@@ -190,17 +190,23 @@
190 This function is designed to handle situations where bugtasks get190 This function is designed to handle situations where bugtasks get
191 reassigned to new products or sourcepackages, and the new bug subscribers191 reassigned to new products or sourcepackages, and the new bug subscribers
192 need to be notified of the bug.192 need to be notified of the bug.
193
194 A boolean is returned indicating whether any emails were sent.
193 """195 """
194 prev_subs_set = set(previous_subscribers)196 prev_subs_set = set(previous_subscribers)
195 cur_subs_set = set(current_subscribers)197 cur_subs_set = set(current_subscribers)
196 new_subs = cur_subs_set.difference(prev_subs_set)198 new_subs = cur_subs_set.difference(prev_subs_set)
197199
200 if (event_creator is not None
201 and not event_creator.selfgenerated_bugnotifications):
202 new_subs.discard(event_creator)
203
198 to_addrs = set()204 to_addrs = set()
199 for new_sub in new_subs:205 for new_sub in new_subs:
200 to_addrs.update(get_contact_email_addresses(new_sub))206 to_addrs.update(get_contact_email_addresses(new_sub))
201207
202 if not to_addrs:208 if not to_addrs:
203 return209 return False
204210
205 from_addr = format_address(211 from_addr = format_address(
206 'Launchpad Bug Tracker',212 'Launchpad Bug Tracker',
@@ -226,3 +232,5 @@
226 from_addr, to_addr, contents, subject, email_date,232 from_addr, to_addr, contents, subject, email_date,
227 rationale=rationale, references=references)233 rationale=rationale, references=references)
228 sendmail(msg)234 sendmail(msg)
235
236 return True
229237
=== modified file 'lib/lp/bugs/subscribers/tests/test_bug.py'
--- lib/lp/bugs/subscribers/tests/test_bug.py 2011-02-04 01:15:13 +0000
+++ lib/lp/bugs/subscribers/tests/test_bug.py 2011-04-28 21:06:30 +0000
@@ -1,6 +1,8 @@
1# Copyright 2011 Canonical Ltd. This software is licensed under the1# Copyright 2011 Canonical Ltd. This software is licensed under the
2# GNU Affero General Public License version 3 (see the file LICENSE).2# GNU Affero General Public License version 3 (see the file LICENSE).
33
4__metaclass__ = type
5
4from storm.store import Store6from storm.store import Store
57
6from canonical.launchpad.webapp.publisher import canonical_url8from canonical.launchpad.webapp.publisher import canonical_url
@@ -13,9 +15,17 @@
13 BugNotification,15 BugNotification,
14 BugNotificationRecipient,16 BugNotificationRecipient,
15 )17 )
16from lp.bugs.subscribers.bug import add_bug_change_notifications18from lp.bugs.subscribers.bug import (
19 add_bug_change_notifications,
20 send_bug_details_to_new_bug_subscribers,
21 )
17from lp.registry.model.person import Person22from lp.registry.model.person import Person
18from lp.testing import TestCaseWithFactory23from lp.testing import (
24 TestCase,
25 TestCaseWithFactory,
26 )
27
28from testtools.matchers import Is
1929
2030
21class BugSubscriberTestCase(TestCaseWithFactory):31class BugSubscriberTestCase(TestCaseWithFactory):
@@ -130,3 +140,20 @@
130 # Only METADATA subscribers get notified.140 # Only METADATA subscribers get notified.
131 self.assertContentEqual(141 self.assertContentEqual(
132 [self.metadata_subscriber], self.getNotifiedPersons())142 [self.metadata_subscriber], self.getNotifiedPersons())
143
144
145class FauxPerson:
146 selfgenerated_bugnotifications = False
147
148
149class NewSubscribers(TestCase):
150
151 def test_self_notification_preference_respected(self):
152 # If the person modifying the bug does not want to be notified about
153 # their own changes, they will not be.
154 actor = FauxPerson()
155 any_sent = send_bug_details_to_new_bug_subscribers(
156 None, [], [actor], event_creator=actor)
157 # Since the creator of the event was the only person to be notified
158 # and they don't want self-notifications, no messages were sent.
159 self.assertThat(any_sent, Is(False))