Merge lp:~gmb/launchpad/prompt-user-to-subscribe-to-master-bug-485627 into lp:launchpad

Proposed by Graham Binns
Status: Merged
Approved by: Edwin Grubbs
Approved revision: no longer in the source branch.
Merged at revision: 11488
Proposed branch: lp:~gmb/launchpad/prompt-user-to-subscribe-to-master-bug-485627
Merge into: lp:launchpad
Diff against target: 370 lines (+116/-51)
4 files modified
lib/lp/bugs/adapters/bugchange.py (+25/-9)
lib/lp/bugs/doc/bug-change.txt (+1/-0)
lib/lp/bugs/doc/bugnotification-sending.txt (+50/-31)
lib/lp/bugs/tests/test_bugchanges.py (+40/-11)
To merge this branch: bzr merge lp:~gmb/launchpad/prompt-user-to-subscribe-to-master-bug-485627
Reviewer Review Type Date Requested Status
Edwin Grubbs (community) code Approve
Review via email: mp+34302@code.launchpad.net

Commit message

Users will now be prompted to subscribe to the master bug when they are notified that one bug has been marked as a duplicate of another.

Description of the change

This branch adds a link to notifications about bugs being marked as a
duplicate, prompting the user to subscribe to the master bug.

Changes made:

== lib/lp/bugs/adapters/bugchange.py ==

 - I've updated the BugDuplicateChange class to add the 'you can
   subscribe by following this link' footer to duplication
   notifications.
 - I've cleaned up some lint.

== lib/lp/bugs/doc/bugnotification-sending.txt ==

 - Weirdly, this test failed after I'd made my change, but in a code
   path not related to my changes. Adding the switch_db_user calls to
   the point of failure fixed it; AFAICT this test should never have
   passed in the first place.
 - I've taken the opportunity to move this test to ReST format and to
   clean up some lint.

== lib/lp/bugs/tests/test_bugchanges.py ==

 - I've updated the tests for bug duplicate notifications to reflect the
   changes made to bugchange.py above.

To post a comment you must log in.
Revision history for this message
Edwin Grubbs (edwin-grubbs) wrote :

Hi Graham,

This branch looks good.

-Edwin

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/adapters/bugchange.py'
2--- lib/lp/bugs/adapters/bugchange.py 2010-08-20 20:31:18 +0000
3+++ lib/lp/bugs/adapters/bugchange.py 2010-09-02 10:32:47 +0000
4@@ -213,7 +213,7 @@
5 lines.append(u"%13s: %s" % (
6 u"Status", self.bug_task.status.title))
7 return {
8- 'text': '\n'.join(lines)
9+ 'text': '\n'.join(lines),
10 }
11
12
13@@ -381,8 +381,16 @@
14 "%d" % self.new_value.id)
15 else:
16 new_value_text = (
17- "** This bug has been marked a duplicate of bug %d\n"
18- " %s" % (self.new_value.id, self.new_value.title))
19+ "** This bug has been marked a duplicate of bug "
20+ "%(bug_id)d\n %(bug_title)s\n"
21+ " * You can subscribe to bug %(bug_id)d by following "
22+ "this link: %(subscribe_link)s" % {
23+ 'bug_id': self.new_value.id,
24+ 'bug_title': self.new_value.title,
25+ 'subscribe_link': canonical_url(
26+ self.new_value.default_bugtask,
27+ view_name='+subscribe'),
28+ })
29
30 text = "\n".join((old_value_text, new_value_text))
31
32@@ -393,8 +401,16 @@
33 "%d" % self.new_value.id)
34 else:
35 text = (
36- "** This bug has been marked a duplicate of bug %d\n"
37- " %s" % (self.new_value.id, self.new_value.title))
38+ "** This bug has been marked a duplicate of bug "
39+ "%(bug_id)d\n %(bug_title)s\n"
40+ " * You can subscribe to bug %(bug_id)d by following "
41+ "this link: %(subscribe_link)s" % {
42+ 'bug_id': self.new_value.id,
43+ 'bug_title': self.new_value.title,
44+ 'subscribe_link': canonical_url(
45+ self.new_value.default_bugtask,
46+ view_name='+subscribe'),
47+ })
48
49 elif self.new_value is None:
50 if self.old_value.private:
51@@ -492,7 +508,7 @@
52 def getBugNotification(self):
53 return {
54 'text': self.notification_mapping[
55- (self.old_value, self.new_value)]
56+ (self.old_value, self.new_value)],
57 }
58
59
60@@ -685,9 +701,9 @@
61 u"** Changed in: %(bug_target_name)s\n"
62 "%(label)13s: %(oldval)s => %(newval)s\n" % {
63 'bug_target_name': self.bug_task.bugtargetname,
64- 'label' : self.display_notification_label,
65- 'oldval' : self.display_old_value,
66- 'newval' : self.display_new_value,
67+ 'label': self.display_notification_label,
68+ 'oldval': self.display_old_value,
69+ 'newval': self.display_new_value,
70 })
71
72 return {'text': text.rstrip()}
73
74=== modified file 'lib/lp/bugs/doc/bug-change.txt'
75--- lib/lp/bugs/doc/bug-change.txt 2010-08-16 13:50:28 +0000
76+++ lib/lp/bugs/doc/bug-change.txt 2010-09-02 10:32:47 +0000
77@@ -300,6 +300,7 @@
78 >>> print bug_duplicate_change.getBugNotification()['text']
79 ** This bug has been marked a duplicate of bug ...
80 Fish can't walk
81+ * You can subscribe to bug ... by following this link:...
82
83 BugDuplicateChange overrides getBugActivity() to customize all the
84 returned fields.
85
86=== modified file 'lib/lp/bugs/doc/bugnotification-sending.txt'
87--- lib/lp/bugs/doc/bugnotification-sending.txt 2010-08-23 09:28:02 +0000
88+++ lib/lp/bugs/doc/bugnotification-sending.txt 2010-09-02 10:32:47 +0000
89@@ -1,4 +1,5 @@
90-= Sending the Bug Notifications =
91+Sending the Bug Notifications
92+=============================
93
94 As explained in bugnotifications.txt, a change to a bug causes a bug
95 notification to be added. These notifications should be assembled into
96@@ -29,6 +30,21 @@
97 ... print email_notification.get_payload(decode=True)
98 ... print "-" * 70
99
100+We'll also define some helper functions to help us with database users.
101+
102+ >>> from canonical.config import config
103+ >>> from canonical.database.sqlbase import commit
104+ >>> from canonical.testing import LaunchpadZopelessLayer
105+
106+ >>> def switch_db_to_launchpad():
107+ ... commit()
108+ ... LaunchpadZopelessLayer.switchDbUser('launchpad')
109+
110+ >>> def switch_db_to_bugnotification():
111+ ... commit()
112+ ... LaunchpadZopelessLayer.switchDbUser(
113+ ... config.malone.bugnotification_dbuser)
114+
115 You'll note that we are printing out an X-Launchpad-Message-Rationale
116 header. This header is a simple string that allows people to filter
117 bugmail according to the reason they are getting emailed. For instance,
118@@ -46,7 +62,8 @@
119 ... datecreated=ten_minutes_ago)
120 >>> bug_one.addCommentNotification(comment)
121
122- >>> notifications = getUtility(IBugNotificationSet).getNotificationsToSend()
123+ >>> notifications = getUtility(
124+ ... IBugNotificationSet).getNotificationsToSend()
125 >>> len(notifications)
126 1
127
128@@ -323,6 +340,7 @@
129 ... print member.preferredemail.email
130 marilize@hbd.com
131
132+ >>> switch_db_to_launchpad()
133 >>> bug_one.subscribe(shipit_admins, shipit_admins)
134 <...>
135
136@@ -330,6 +348,8 @@
137 ... 'subject', 'a comment.', sample_person,
138 ... datecreated=ten_minutes_ago)
139 >>> bug_one.addCommentNotification(comment)
140+
141+ >>> switch_db_to_bugnotification()
142 >>> pending_notifications = getUtility(
143 ... IBugNotificationSet).getNotificationsToSend()
144 >>> len(pending_notifications)
145@@ -348,22 +368,8 @@
146 >>> flush_notifications()
147
148
149-== Duplicates ==
150-
151-We will need some helper functions.
152-
153- >>> from canonical.config import config
154- >>> from canonical.database.sqlbase import commit
155- >>> from canonical.testing import LaunchpadZopelessLayer
156-
157- >>> def switch_db_to_launchpad():
158- ... commit()
159- ... LaunchpadZopelessLayer.switchDbUser('launchpad')
160-
161- >>> def switch_db_to_bugnotification():
162- ... commit()
163- ... LaunchpadZopelessLayer.switchDbUser(
164- ... config.malone.bugnotification_dbuser)
165+Duplicates
166+----------
167
168 We will also need a fresh new bug.
169
170@@ -391,11 +397,13 @@
171 ... 'subject', 'a comment.', sample_person,
172 ... datecreated=ten_minutes_ago)
173 >>> new_bug.addCommentNotification(comment)
174- >>> notifications = getUtility(IBugNotificationSet).getNotificationsToSend()
175+ >>> notifications = getUtility(
176+ ... IBugNotificationSet).getNotificationsToSend()
177 >>> len(notifications)
178 1
179
180- >>> for bug_notifications, messages in get_email_notifications(notifications):
181+ >>> for bug_notifications, messages in (
182+ ... get_email_notifications(notifications)):
183 ... for message in messages:
184 ... print_notification(message)
185 To: support@ubuntu.com
186@@ -436,7 +444,8 @@
187 >>> flush_notifications()
188
189
190-== Security Vulnerabilities ==
191+Security Vulnerabilities
192+------------------------
193
194 When a new security related bug is filed, a small notification is
195 inserted at the top of the message body.
196@@ -499,7 +508,8 @@
197 >>> flush_notifications()
198
199
200-== The cronscript ==
201+The cronscript
202+--------------
203
204 There's a cronsript which does the sending of the email. Let's add a
205 few notifications to show that it works.
206@@ -528,7 +538,8 @@
207 ... '** Summary changed to: New summary.', sample_person,
208 ... when=ten_minutes_ago)
209
210- >>> notifications = getUtility(IBugNotificationSet).getNotificationsToSend()
211+ >>> notifications = getUtility(
212+ ... IBugNotificationSet).getNotificationsToSend()
213 >>> len(notifications)
214 6
215
216@@ -620,7 +631,8 @@
217 >>> flush_notifications()
218
219
220-== The X-Launchpad-Bug header ==
221+The X-Launchpad-Bug header
222+--------------------------
223
224 When a notification is sent out about a bug, the X-Launchpad-Bug header is
225 filled with data about that bug:
226@@ -658,7 +670,8 @@
227 False
228
229
230-== The X-Launchpad-Bug-Tags header ==
231+The X-Launchpad-Bug-Tags header
232+-------------------------------
233
234 First, a helper function that triggers notifications by adding a
235 comment to a given bug, another that returns a sorted list of new
236@@ -723,7 +736,8 @@
237 ... message.get_all('X-Launchpad-Bug-Tags')
238
239
240-== The X-Launchpad-Bug-Private header ==
241+The X-Launchpad-Bug-Private header
242+----------------------------------
243
244 When a notification is sent out about a bug, the
245 X-Launchpad-Bug-Private header shows if the bug is marked as
246@@ -748,7 +762,8 @@
247 mark@example.com ['yes']
248
249
250-== The X-Launchpad-Bug-Security-Vulnerability header ==
251+The X-Launchpad-Bug-Security-Vulnerability header
252+-------------------------------------------------
253
254 When a notification is sent out about a bug, the
255 X-Launchpad-Bug-Security-Vulnerability header records if the bug is a
256@@ -776,7 +791,8 @@
257 mark@example.com ['yes']
258
259
260-== The X-Launchpad-Bug-Commenters header ==
261+The X-Launchpad-Bug-Commenters header
262+-------------------------------------
263
264 The X-Launchpad-Bug-Recipient-Commented header lists all user IDs of
265 people who have ever commented on the bug. It's a space-separated
266@@ -809,7 +825,8 @@
267 name12 name16
268
269
270-== The X-Launchpad-Bug-Reporter header ==
271+The X-Launchpad-Bug-Reporter header
272+-----------------------------------
273
274 The X-Launchpad-Bug-Reporter header contains information about the Launchpad
275 user who originally reported the bug and opened the bug's first bug task.
276@@ -819,7 +836,8 @@
277 Foo Bar (name16)
278
279
280-== Verbose bug notifications ==
281+Verbose bug notifications
282+-------------------------
283
284 It is possible for users to have all the bug notifications which they
285 receive include the bug description and status. This helps in those
286@@ -1016,7 +1034,8 @@
287 ----------------------------------------------------------------------
288
289
290-== Notification Recipients ==
291+Notification Recipients
292+-----------------------
293
294 Bug notifications are sent to direct subscribers of a bug as well as to
295 structural subscribers. Structural subcribers can select the
296
297=== modified file 'lib/lp/bugs/tests/test_bugchanges.py'
298--- lib/lp/bugs/tests/test_bugchanges.py 2010-08-20 20:31:18 +0000
299+++ lib/lp/bugs/tests/test_bugchanges.py 2010-09-02 10:32:47 +0000
300@@ -1292,8 +1292,17 @@
301
302 expected_notification = {
303 'person': self.user,
304- 'text': ("** This bug has been marked a duplicate of bug %d\n"
305- " %s" % (self.bug.id, self.bug.title)),
306+ 'text': (
307+ "** This bug has been marked a duplicate of bug "
308+ "%(bug_id)d\n %(bug_title)s\n"
309+ " * You can subscribe to bug %(bug_id)d by following "
310+ "this link: %(subscribe_link)s" % {
311+ 'bug_id': self.bug.id,
312+ 'bug_title': self.bug.title,
313+ 'subscribe_link': canonical_url(
314+ self.bug.default_bugtask,
315+ view_name='+subscribe'),
316+ }),
317 'recipients': duplicate_bug_recipients,
318 }
319
320@@ -1361,11 +1370,21 @@
321
322 expected_notification = {
323 'person': self.user,
324- 'text': ("** This bug is no longer a duplicate of bug %d\n"
325- " %s\n"
326- "** This bug has been marked a duplicate of bug %d\n"
327- " %s" % (bug_one.id, bug_one.title,
328- bug_two.id, bug_two.title)),
329+ 'text': (
330+ "** This bug is no longer a duplicate of bug "
331+ "%(bug_one_id)d\n %(bug_one_title)s\n"
332+ "** This bug has been marked a duplicate of bug "
333+ "%(bug_two_id)d\n %(bug_two_title)s\n"
334+ " * You can subscribe to bug %(bug_two_id)d by following "
335+ "this link: %(subscribe_link)s" % {
336+ 'bug_one_id': bug_one.id,
337+ 'bug_one_title': bug_one.title,
338+ 'bug_two_id': bug_two.id,
339+ 'bug_two_title': bug_two.title,
340+ 'subscribe_link': canonical_url(
341+ bug_two.default_bugtask,
342+ view_name='+subscribe'),
343+ }),
344 'recipients': bug_recipients,
345 }
346
347@@ -1474,10 +1493,20 @@
348 expected_notification = {
349 'person': self.user,
350 'text': (
351- "** This bug is no longer a duplicate of private bug %d\n"
352- "** This bug has been marked a duplicate of bug %d\n"
353- " %s" % (private_bug.id, public_bug.id,
354- public_bug.title)),
355+ "** This bug is no longer a duplicate of private bug "
356+ "%(private_bug_id)d\n"
357+ "** This bug has been marked a duplicate of bug "
358+ "%(public_bug_id)d\n %(public_bug_title)s\n"
359+ " * You can subscribe to bug %(public_bug_id)d by following "
360+ "this link: %(subscribe_link)s" % {
361+ 'private_bug_id': private_bug.id,
362+ 'private_bug_title': private_bug.title,
363+ 'public_bug_id': public_bug.id,
364+ 'public_bug_title': public_bug.title,
365+ 'subscribe_link': canonical_url(
366+ public_bug.default_bugtask,
367+ view_name='+subscribe'),
368+ }),
369 'recipients': bug_recipients,
370 }
371