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
1=== modified file 'database/schema/security.cfg'
2--- database/schema/security.cfg 2011-04-21 14:01:20 +0000
3+++ database/schema/security.cfg 2011-04-28 21:06:30 +0000
4@@ -1705,6 +1705,7 @@
5 public.language = SELECT
6 public.person = SELECT, UPDATE
7 public.personlanguage = SELECT
8+public.personsettings = SELECT
9 public.teammembership = SELECT
10 public.teamparticipation = SELECT
11 public.validpersoncache = SELECT
12
13=== modified file 'lib/lp/bugs/subscribers/bug.py'
14--- lib/lp/bugs/subscribers/bug.py 2011-03-07 17:39:20 +0000
15+++ lib/lp/bugs/subscribers/bug.py 2011-04-28 21:06:30 +0000
16@@ -190,17 +190,23 @@
17 This function is designed to handle situations where bugtasks get
18 reassigned to new products or sourcepackages, and the new bug subscribers
19 need to be notified of the bug.
20+
21+ A boolean is returned indicating whether any emails were sent.
22 """
23 prev_subs_set = set(previous_subscribers)
24 cur_subs_set = set(current_subscribers)
25 new_subs = cur_subs_set.difference(prev_subs_set)
26
27+ if (event_creator is not None
28+ and not event_creator.selfgenerated_bugnotifications):
29+ new_subs.discard(event_creator)
30+
31 to_addrs = set()
32 for new_sub in new_subs:
33 to_addrs.update(get_contact_email_addresses(new_sub))
34
35 if not to_addrs:
36- return
37+ return False
38
39 from_addr = format_address(
40 'Launchpad Bug Tracker',
41@@ -226,3 +232,5 @@
42 from_addr, to_addr, contents, subject, email_date,
43 rationale=rationale, references=references)
44 sendmail(msg)
45+
46+ return True
47
48=== modified file 'lib/lp/bugs/subscribers/tests/test_bug.py'
49--- lib/lp/bugs/subscribers/tests/test_bug.py 2011-02-04 01:15:13 +0000
50+++ lib/lp/bugs/subscribers/tests/test_bug.py 2011-04-28 21:06:30 +0000
51@@ -1,6 +1,8 @@
52 # Copyright 2011 Canonical Ltd. This software is licensed under the
53 # GNU Affero General Public License version 3 (see the file LICENSE).
54
55+__metaclass__ = type
56+
57 from storm.store import Store
58
59 from canonical.launchpad.webapp.publisher import canonical_url
60@@ -13,9 +15,17 @@
61 BugNotification,
62 BugNotificationRecipient,
63 )
64-from lp.bugs.subscribers.bug import add_bug_change_notifications
65+from lp.bugs.subscribers.bug import (
66+ add_bug_change_notifications,
67+ send_bug_details_to_new_bug_subscribers,
68+ )
69 from lp.registry.model.person import Person
70-from lp.testing import TestCaseWithFactory
71+from lp.testing import (
72+ TestCase,
73+ TestCaseWithFactory,
74+ )
75+
76+from testtools.matchers import Is
77
78
79 class BugSubscriberTestCase(TestCaseWithFactory):
80@@ -130,3 +140,20 @@
81 # Only METADATA subscribers get notified.
82 self.assertContentEqual(
83 [self.metadata_subscriber], self.getNotifiedPersons())
84+
85+
86+class FauxPerson:
87+ selfgenerated_bugnotifications = False
88+
89+
90+class NewSubscribers(TestCase):
91+
92+ def test_self_notification_preference_respected(self):
93+ # If the person modifying the bug does not want to be notified about
94+ # their own changes, they will not be.
95+ actor = FauxPerson()
96+ any_sent = send_bug_details_to_new_bug_subscribers(
97+ None, [], [actor], event_creator=actor)
98+ # Since the creator of the event was the only person to be notified
99+ # and they don't want self-notifications, no messages were sent.
100+ self.assertThat(any_sent, Is(False))