Merge lp:~cjwatson/launchpad/bug-footer into lp:launchpad
- bug-footer
- Merge into devel
Proposed by
Colin Watson
Status: | Merged | ||||
---|---|---|---|---|---|
Merged at revision: | 17667 | ||||
Proposed branch: | lp:~cjwatson/launchpad/bug-footer | ||||
Merge into: | lp:launchpad | ||||
Diff against target: |
301 lines (+77/-41) 5 files modified
lib/lp/bugs/doc/bugnotification-email.txt (+7/-7) lib/lp/bugs/mail/bugnotificationbuilder.py (+36/-18) lib/lp/bugs/mail/tests/test_bugnotificationbuilder.py (+24/-5) lib/lp/bugs/scripts/bugnotification.py (+2/-3) lib/lp/bugs/subscribers/bug.py (+8/-8) |
||||
To merge this branch: | bzr merge lp:~cjwatson/launchpad/bug-footer | ||||
Related bugs: |
|
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
William Grant | code | Approve | |
Review via email: mp+266577@code.launchpad.net |
Commit message
Make bug notifications honour PersonSettings.
Description of the change
Make bug notifications honour PersonSettings.
I tried to convert bug notifications to BaseMailer, and that would still ultimately be better, but it was Hard Work. This only took me half an hour or so and the duplication isn't all that bad.
To post a comment you must log in.
Revision history for this message
William Grant (wgrant) : | # |
review:
Approve
(code)
Preview Diff
[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1 | === modified file 'lib/lp/bugs/doc/bugnotification-email.txt' | |||
2 | --- lib/lp/bugs/doc/bugnotification-email.txt 2015-07-21 09:04:01 +0000 | |||
3 | +++ lib/lp/bugs/doc/bugnotification-email.txt 2015-07-31 14:49:19 +0000 | |||
4 | @@ -535,18 +535,19 @@ | |||
5 | 535 | 535 | ||
6 | 536 | The build() method of a builder accepts a number of parameters and returns | 536 | The build() method of a builder accepts a number of parameters and returns |
7 | 537 | an instance of email.mime.text.MIMEText. The most basic invocation of this | 537 | an instance of email.mime.text.MIMEText. The most basic invocation of this |
10 | 538 | method requires a from address, a to address, a body, a subject and a | 538 | method requires a from address, a to person, a body, a subject and a sending |
11 | 539 | sending date for the mail. | 539 | date for the mail. |
12 | 540 | 540 | ||
13 | 541 | >>> from datetime import datetime | 541 | >>> from datetime import datetime |
14 | 542 | >>> import pytz | 542 | >>> import pytz |
15 | 543 | 543 | ||
16 | 544 | >>> from_address = get_bugmail_from_address(lp_janitor, bug_four) | 544 | >>> from_address = get_bugmail_from_address(lp_janitor, bug_four) |
17 | 545 | >>> to_person = getUtility(IPersonSet).getByEmail('foo.bar@canonical.com') | ||
18 | 545 | >>> sending_date = pytz.timezone('Europe/Prague').localize( | 546 | >>> sending_date = pytz.timezone('Europe/Prague').localize( |
19 | 546 | ... datetime(2008, 5, 20, 11, 5, 47)) | 547 | ... datetime(2008, 5, 20, 11, 5, 47)) |
20 | 547 | 548 | ||
21 | 548 | >>> notification_email = bug_four_notification_builder.build( | 549 | >>> notification_email = bug_four_notification_builder.build( |
23 | 549 | ... from_address, 'foo.bar@canonical.com', | 550 | ... from_address, to_person, |
24 | 550 | ... "A test body.", "A test subject.", sending_date) | 551 | ... "A test body.", "A test subject.", sending_date) |
25 | 551 | 552 | ||
26 | 552 | The fields of the generated notification email will be set according to | 553 | The fields of the generated notification email will be set according to |
27 | @@ -572,7 +573,7 @@ | |||
28 | 572 | references and message_id. | 573 | references and message_id. |
29 | 573 | 574 | ||
30 | 574 | >>> notification_email = bug_four_notification_builder.build( | 575 | >>> notification_email = bug_four_notification_builder.build( |
32 | 575 | ... from_address, 'foo.bar@canonical.com', | 576 | ... from_address, to_person, |
33 | 576 | ... "A test body.", "A test subject.", sending_date, | 577 | ... "A test body.", "A test subject.", sending_date, |
34 | 577 | ... rationale='Because-I-said-so', | 578 | ... rationale='Because-I-said-so', |
35 | 578 | ... references=['<12345@launchpad.net>'], | 579 | ... references=['<12345@launchpad.net>'], |
36 | @@ -598,7 +599,7 @@ | |||
37 | 598 | The message subject will always have [Bug <bug_id>] prepended to it. | 599 | The message subject will always have [Bug <bug_id>] prepended to it. |
38 | 599 | 600 | ||
39 | 600 | >>> notification_email = bug_four_notification_builder.build( | 601 | >>> notification_email = bug_four_notification_builder.build( |
41 | 601 | ... from_address, 'foo.bar@canonical.com', | 602 | ... from_address, to_person, |
42 | 602 | ... "A test body.", "Yet another message", sending_date) | 603 | ... "A test body.", "Yet another message", sending_date) |
43 | 603 | 604 | ||
44 | 604 | >>> print notification_email['Subject'] | 605 | >>> print notification_email['Subject'] |
45 | @@ -608,8 +609,7 @@ | |||
46 | 608 | <bug_id>]. | 609 | <bug_id>]. |
47 | 609 | 610 | ||
48 | 610 | >>> notification_email = bug_four_notification_builder.build( | 611 | >>> notification_email = bug_four_notification_builder.build( |
51 | 611 | ... from_address, 'foo.bar@canonical.com', | 612 | ... from_address, to_person, "A test body.", None, sending_date) |
50 | 612 | ... "A test body.", None, sending_date) | ||
52 | 613 | 613 | ||
53 | 614 | >>> print notification_email['Subject'] | 614 | >>> print notification_email['Subject'] |
54 | 615 | [Bug 4] | 615 | [Bug 4] |
55 | 616 | 616 | ||
56 | === modified file 'lib/lp/bugs/mail/bugnotificationbuilder.py' | |||
57 | --- lib/lp/bugs/mail/bugnotificationbuilder.py 2015-07-13 16:14:46 +0000 | |||
58 | +++ lib/lp/bugs/mail/bugnotificationbuilder.py 2015-07-31 14:49:19 +0000 | |||
59 | @@ -16,6 +16,7 @@ | |||
60 | 16 | import rfc822 | 16 | import rfc822 |
61 | 17 | 17 | ||
62 | 18 | from zope.component import getUtility | 18 | from zope.component import getUtility |
63 | 19 | from zope.security.proxy import removeSecurityProxy | ||
64 | 19 | 20 | ||
65 | 20 | from lp.app.enums import ( | 21 | from lp.app.enums import ( |
66 | 21 | PRIVATE_INFORMATION_TYPES, | 22 | PRIVATE_INFORMATION_TYPES, |
67 | @@ -25,7 +26,10 @@ | |||
68 | 25 | from lp.services.config import config | 26 | from lp.services.config import config |
69 | 26 | from lp.services.helpers import shortlist | 27 | from lp.services.helpers import shortlist |
70 | 27 | from lp.services.identity.interfaces.emailaddress import IEmailAddressSet | 28 | from lp.services.identity.interfaces.emailaddress import IEmailAddressSet |
72 | 28 | from lp.services.mail.sendmail import format_address | 29 | from lp.services.mail.sendmail import ( |
73 | 30 | append_footer, | ||
74 | 31 | format_address, | ||
75 | 32 | ) | ||
76 | 29 | 33 | ||
77 | 30 | 34 | ||
78 | 31 | def format_rfc2822_date(date): | 35 | def format_rfc2822_date(date): |
79 | @@ -161,12 +165,13 @@ | |||
80 | 161 | self.common_headers.append( | 165 | self.common_headers.append( |
81 | 162 | ('X-Launchpad-Bug-Duplicate', str(bug.duplicateof.id))) | 166 | ('X-Launchpad-Bug-Duplicate', str(bug.duplicateof.id))) |
82 | 163 | 167 | ||
84 | 164 | def build(self, from_address, to_address, body, subject, email_date, | 168 | def build(self, from_address, to_person, body, subject, email_date, |
85 | 165 | rationale=None, references=None, message_id=None, filters=None): | 169 | rationale=None, references=None, message_id=None, filters=None): |
86 | 166 | """Construct the notification. | 170 | """Construct the notification. |
87 | 167 | 171 | ||
88 | 168 | :param from_address: The From address of the notification. | 172 | :param from_address: The From address of the notification. |
90 | 169 | :param to_address: The To address for the notification. | 173 | :param to_person: The `IPerson` to use as the To address for the |
91 | 174 | notification. | ||
92 | 170 | :param body: The body text of the notification. | 175 | :param body: The body text of the notification. |
93 | 171 | :type body: unicode | 176 | :type body: unicode |
94 | 172 | :param subject: The Subject of the notification. | 177 | :param subject: The Subject of the notification. |
95 | @@ -178,34 +183,47 @@ | |||
96 | 178 | 183 | ||
97 | 179 | :return: An `email.mime.text.MIMEText` object. | 184 | :return: An `email.mime.text.MIMEText` object. |
98 | 180 | """ | 185 | """ |
103 | 181 | message = MIMEText(body.encode('utf8'), 'plain', 'utf8') | 186 | headers = [ |
104 | 182 | message['Date'] = format_rfc2822_date(email_date) | 187 | ('Date', format_rfc2822_date(email_date)), |
105 | 183 | message['From'] = from_address | 188 | ('From', from_address), |
106 | 184 | message['To'] = to_address | 189 | ('To', str(removeSecurityProxy(to_person).preferredemail.email)), |
107 | 190 | ] | ||
108 | 185 | 191 | ||
109 | 186 | # Add the common headers. | 192 | # Add the common headers. |
112 | 187 | for header in self.common_headers: | 193 | headers.extend(self.common_headers) |
111 | 188 | message.add_header(*header) | ||
113 | 189 | 194 | ||
114 | 190 | if references: | 195 | if references: |
116 | 191 | message['References'] = ' '.join(references) | 196 | headers.append(('References', ' '.join(references))) |
117 | 192 | if message_id is not None: | 197 | if message_id is not None: |
119 | 193 | message['Message-Id'] = message_id | 198 | headers.append(('Message-Id', message_id)) |
120 | 194 | 199 | ||
121 | 195 | subject_prefix = "[Bug %d]" % self.bug.id | 200 | subject_prefix = "[Bug %d]" % self.bug.id |
122 | 196 | if subject is None: | 201 | if subject is None: |
124 | 197 | message['Subject'] = subject_prefix | 202 | headers.append(('Subject', subject_prefix)) |
125 | 198 | elif subject_prefix in subject: | 203 | elif subject_prefix in subject: |
127 | 199 | message['Subject'] = subject | 204 | headers.append(('Subject', subject)) |
128 | 200 | else: | 205 | else: |
130 | 201 | message['Subject'] = "%s %s" % (subject_prefix, subject) | 206 | headers.append(('Subject', "%s %s" % (subject_prefix, subject))) |
131 | 202 | 207 | ||
132 | 203 | if rationale is not None: | 208 | if rationale is not None: |
134 | 204 | message.add_header('X-Launchpad-Message-Rationale', rationale) | 209 | headers.append(('X-Launchpad-Message-Rationale', rationale)) |
135 | 205 | 210 | ||
136 | 206 | if filters is not None: | 211 | if filters is not None: |
137 | 207 | for filter in filters: | 212 | for filter in filters: |
141 | 208 | message.add_header( | 213 | headers.append(('X-Launchpad-Subscription', filter)) |
142 | 209 | 'X-Launchpad-Subscription', filter) | 214 | |
143 | 210 | 215 | # XXX cjwatson 2015-07-31: This is cloned-and-hacked from | |
144 | 216 | # BaseMailer; it would ultimately be better to convert bug | ||
145 | 217 | # notifications to that framework. | ||
146 | 218 | if to_person.expanded_notification_footers: | ||
147 | 219 | lines = [] | ||
148 | 220 | for key, value in headers: | ||
149 | 221 | if key.startswith('X-Launchpad-'): | ||
150 | 222 | lines.append('%s: %s\n' % (key[2:], value)) | ||
151 | 223 | if lines: | ||
152 | 224 | body = append_footer(body, ''.join(lines)) | ||
153 | 225 | |||
154 | 226 | message = MIMEText(body.encode('utf8'), 'plain', 'utf8') | ||
155 | 227 | for header in headers: | ||
156 | 228 | message.add_header(*header) | ||
157 | 211 | return message | 229 | return message |
158 | 212 | 230 | ||
159 | === modified file 'lib/lp/bugs/mail/tests/test_bugnotificationbuilder.py' | |||
160 | --- lib/lp/bugs/mail/tests/test_bugnotificationbuilder.py 2015-01-26 01:31:33 +0000 | |||
161 | +++ lib/lp/bugs/mail/tests/test_bugnotificationbuilder.py 2015-07-31 14:49:19 +0000 | |||
162 | @@ -1,4 +1,4 @@ | |||
164 | 1 | # Copyright 2011 Canonical Ltd. This software is licensed under the | 1 | # Copyright 2011-2015 Canonical Ltd. This software is licensed under the |
165 | 2 | # GNU Affero General Public License version 3 (see the file LICENSE). | 2 | # GNU Affero General Public License version 3 (see the file LICENSE). |
166 | 3 | 3 | ||
167 | 4 | """Tests for BugNotificationBuilder email construction.""" | 4 | """Tests for BugNotificationBuilder email construction.""" |
168 | @@ -27,7 +27,7 @@ | |||
169 | 27 | def test_build_filters_empty(self): | 27 | def test_build_filters_empty(self): |
170 | 28 | """Filters are added.""" | 28 | """Filters are added.""" |
171 | 29 | utc_now = datetime.now(pytz.UTC) | 29 | utc_now = datetime.now(pytz.UTC) |
173 | 30 | message = self.builder.build('from', 'to', 'body', 'subject', | 30 | message = self.builder.build('from', self.bug.owner, 'body', 'subject', |
174 | 31 | utc_now, filters=[]) | 31 | utc_now, filters=[]) |
175 | 32 | self.assertIs(None, | 32 | self.assertIs(None, |
176 | 33 | message.get("X-Launchpad-Subscription", None)) | 33 | message.get("X-Launchpad-Subscription", None)) |
177 | @@ -35,7 +35,7 @@ | |||
178 | 35 | def test_build_filters_single(self): | 35 | def test_build_filters_single(self): |
179 | 36 | """Filters are added.""" | 36 | """Filters are added.""" |
180 | 37 | utc_now = datetime.now(pytz.UTC) | 37 | utc_now = datetime.now(pytz.UTC) |
182 | 38 | message = self.builder.build('from', 'to', 'body', 'subject', | 38 | message = self.builder.build('from', self.bug.owner, 'body', 'subject', |
183 | 39 | utc_now, filters=[u"Testing filter"]) | 39 | utc_now, filters=[u"Testing filter"]) |
184 | 40 | self.assertContentEqual( | 40 | self.assertContentEqual( |
185 | 41 | [u"Testing filter"], | 41 | [u"Testing filter"], |
186 | @@ -45,7 +45,7 @@ | |||
187 | 45 | """Filters are added.""" | 45 | """Filters are added.""" |
188 | 46 | utc_now = datetime.now(pytz.UTC) | 46 | utc_now = datetime.now(pytz.UTC) |
189 | 47 | message = self.builder.build( | 47 | message = self.builder.build( |
191 | 48 | 'from', 'to', 'body', 'subject', utc_now, | 48 | 'from', self.bug.owner, 'body', 'subject', utc_now, |
192 | 49 | filters=[u"Testing filter", u"Second testing filter"]) | 49 | filters=[u"Testing filter", u"Second testing filter"]) |
193 | 50 | self.assertContentEqual( | 50 | self.assertContentEqual( |
194 | 51 | [u"Testing filter", u"Second testing filter"], | 51 | [u"Testing filter", u"Second testing filter"], |
195 | @@ -54,7 +54,26 @@ | |||
196 | 54 | def test_mails_contain_notification_type_header(self): | 54 | def test_mails_contain_notification_type_header(self): |
197 | 55 | utc_now = datetime.now(pytz.UTC) | 55 | utc_now = datetime.now(pytz.UTC) |
198 | 56 | message = self.builder.build( | 56 | message = self.builder.build( |
200 | 57 | 'from', 'to', 'body', 'subject', utc_now, filters=[]) | 57 | 'from', self.bug.owner, 'body', 'subject', utc_now, filters=[]) |
201 | 58 | self.assertEqual( | 58 | self.assertEqual( |
202 | 59 | "bug", message.get("X-Launchpad-Notification-Type", None)) | 59 | "bug", message.get("X-Launchpad-Notification-Type", None)) |
203 | 60 | 60 | ||
204 | 61 | def test_mails_no_expanded_footer(self): | ||
205 | 62 | # Recipients without expanded_notification_footers do not receive an | ||
206 | 63 | # expanded footer on messages. | ||
207 | 64 | utc_now = datetime.now(pytz.UTC) | ||
208 | 65 | message = self.builder.build( | ||
209 | 66 | 'from', self.bug.owner, 'body', 'subject', utc_now, filters=[]) | ||
210 | 67 | self.assertNotIn( | ||
211 | 68 | "Launchpad-Notification-Type", message.get_payload(decode=True)) | ||
212 | 69 | |||
213 | 70 | def test_mails_append_expanded_footer(self): | ||
214 | 71 | # Recipients with expanded_notification_footers receive an expanded | ||
215 | 72 | # footer on messages. | ||
216 | 73 | utc_now = datetime.now(pytz.UTC) | ||
217 | 74 | self.bug.owner.expanded_notification_footers = True | ||
218 | 75 | message = self.builder.build( | ||
219 | 76 | 'from', self.bug.owner, 'body', 'subject', utc_now, filters=[]) | ||
220 | 77 | self.assertIn( | ||
221 | 78 | "\n-- \nLaunchpad-Notification-Type: bug\n", | ||
222 | 79 | message.get_payload(decode=True)) | ||
223 | 61 | 80 | ||
224 | === modified file 'lib/lp/bugs/scripts/bugnotification.py' | |||
225 | --- lib/lp/bugs/scripts/bugnotification.py 2015-07-06 17:27:09 +0000 | |||
226 | +++ lib/lp/bugs/scripts/bugnotification.py 2015-07-31 14:49:19 +0000 | |||
227 | @@ -1,4 +1,4 @@ | |||
229 | 1 | # Copyright 2009 Canonical Ltd. This software is licensed under the | 1 | # Copyright 2009-2015 Canonical Ltd. This software is licensed under the |
230 | 2 | # GNU Affero General Public License version 3 (see the file LICENSE). | 2 | # GNU Affero General Public License version 3 (see the file LICENSE). |
231 | 3 | 3 | ||
232 | 4 | """Functions related to sending bug notifications.""" | 4 | """Functions related to sending bug notifications.""" |
233 | @@ -182,7 +182,6 @@ | |||
234 | 182 | recipients.items(), key=lambda t: t[0].preferredemail.email) | 182 | recipients.items(), key=lambda t: t[0].preferredemail.email) |
235 | 183 | 183 | ||
236 | 184 | for email_person, data in sorted_recipients: | 184 | for email_person, data in sorted_recipients: |
237 | 185 | address = str(email_person.preferredemail.email) | ||
238 | 186 | # Choosing the first source is a bit arbitrary, but it | 185 | # Choosing the first source is a bit arbitrary, but it |
239 | 187 | # is simple for the user to understand. We may want to reconsider | 186 | # is simple for the user to understand. We may want to reconsider |
240 | 188 | # this in the future. | 187 | # this in the future. |
241 | @@ -240,7 +239,7 @@ | |||
242 | 240 | body_template = get_email_template(email_template, 'bugs') | 239 | body_template = get_email_template(email_template, 'bugs') |
243 | 241 | body = (body_template % body_data).strip() | 240 | body = (body_template % body_data).strip() |
244 | 242 | msg = bug_notification_builder.build( | 241 | msg = bug_notification_builder.build( |
246 | 243 | from_address, address, body, subject, email_date, | 242 | from_address, email_person, body, subject, email_date, |
247 | 244 | rationale, references, msgid, filters=data['filter descriptions']) | 243 | rationale, references, msgid, filters=data['filter descriptions']) |
248 | 245 | messages.append(msg) | 244 | messages.append(msg) |
249 | 246 | 245 | ||
250 | 247 | 246 | ||
251 | === modified file 'lib/lp/bugs/subscribers/bug.py' | |||
252 | --- lib/lp/bugs/subscribers/bug.py 2013-11-29 14:12:13 +0000 | |||
253 | +++ lib/lp/bugs/subscribers/bug.py 2015-07-31 14:49:19 +0000 | |||
254 | @@ -1,4 +1,4 @@ | |||
256 | 1 | # Copyright 2009-2012 Canonical Ltd. This software is licensed under the | 1 | # Copyright 2009-2015 Canonical Ltd. This software is licensed under the |
257 | 2 | # GNU Affero General Public License version 3 (see the file LICENSE). | 2 | # GNU Affero General Public License version 3 (see the file LICENSE). |
258 | 3 | 3 | ||
259 | 4 | __metaclass__ = type | 4 | __metaclass__ = type |
260 | @@ -28,9 +28,9 @@ | |||
261 | 28 | from lp.bugs.mail.newbug import generate_bug_add_email | 28 | from lp.bugs.mail.newbug import generate_bug_add_email |
262 | 29 | from lp.bugs.model.bug import get_also_notified_subscribers | 29 | from lp.bugs.model.bug import get_also_notified_subscribers |
263 | 30 | from lp.registry.interfaces.person import IPerson | 30 | from lp.registry.interfaces.person import IPerson |
264 | 31 | from lp.registry.model.person import get_recipients | ||
265 | 31 | from lp.services.config import config | 32 | from lp.services.config import config |
266 | 32 | from lp.services.database.sqlbase import block_implicit_flushes | 33 | from lp.services.database.sqlbase import block_implicit_flushes |
267 | 33 | from lp.services.mail.helpers import get_contact_email_addresses | ||
268 | 34 | from lp.services.mail.sendmail import ( | 34 | from lp.services.mail.sendmail import ( |
269 | 35 | format_address, | 35 | format_address, |
270 | 36 | sendmail, | 36 | sendmail, |
271 | @@ -203,11 +203,11 @@ | |||
272 | 203 | and not event_creator.selfgenerated_bugnotifications): | 203 | and not event_creator.selfgenerated_bugnotifications): |
273 | 204 | new_subs.discard(event_creator) | 204 | new_subs.discard(event_creator) |
274 | 205 | 205 | ||
276 | 206 | to_addrs = set() | 206 | to_persons = set() |
277 | 207 | for new_sub in new_subs: | 207 | for new_sub in new_subs: |
279 | 208 | to_addrs.update(get_contact_email_addresses(new_sub)) | 208 | to_persons.update(get_recipients(new_sub)) |
280 | 209 | 209 | ||
282 | 210 | if not to_addrs: | 210 | if not to_persons: |
283 | 211 | return False | 211 | return False |
284 | 212 | 212 | ||
285 | 213 | from_addr = format_address( | 213 | from_addr = format_address( |
286 | @@ -225,13 +225,13 @@ | |||
287 | 225 | recipients = bug.getBugNotificationRecipients() | 225 | recipients = bug.getBugNotificationRecipients() |
288 | 226 | 226 | ||
289 | 227 | bug_notification_builder = BugNotificationBuilder(bug, event_creator) | 227 | bug_notification_builder = BugNotificationBuilder(bug, event_creator) |
292 | 228 | for to_addr in sorted(to_addrs): | 228 | for to_person in sorted(to_persons): |
293 | 229 | reason, rationale = recipients.getReason(to_addr) | 229 | reason, rationale = recipients.getReason(to_person) |
294 | 230 | subject, contents = generate_bug_add_email( | 230 | subject, contents = generate_bug_add_email( |
295 | 231 | bug, new_recipients=True, subscribed_by=subscribed_by, | 231 | bug, new_recipients=True, subscribed_by=subscribed_by, |
296 | 232 | reason=reason, event_creator=event_creator) | 232 | reason=reason, event_creator=event_creator) |
297 | 233 | msg = bug_notification_builder.build( | 233 | msg = bug_notification_builder.build( |
299 | 234 | from_addr, to_addr, contents, subject, email_date, | 234 | from_addr, to_person, contents, subject, email_date, |
300 | 235 | rationale=rationale, references=references) | 235 | rationale=rationale, references=references) |
301 | 236 | sendmail(msg) | 236 | sendmail(msg) |
302 | 237 | 237 |