Merge lp:~gary/launchpad/bug164196-2 into lp:launchpad/db-devel

Proposed by Gary Poster
Status: Merged
Approved by: Данило Шеган
Approved revision: no longer in the source branch.
Merged at revision: 10215
Proposed branch: lp:~gary/launchpad/bug164196-2
Merge into: lp:launchpad/db-devel
Prerequisite: lp:~gary/launchpad/bug164196-1
Diff against target: 1212 lines (+691/-80)
8 files modified
lib/lp/bugs/adapters/bugchange.py (+38/-12)
lib/lp/bugs/browser/bugtask.py (+13/-6)
lib/lp/bugs/doc/bugactivity.txt (+8/-3)
lib/lp/bugs/doc/bugnotification-sending.txt (+23/-21)
lib/lp/bugs/model/bugactivity.py (+35/-3)
lib/lp/bugs/scripts/bugnotification.py (+64/-18)
lib/lp/bugs/scripts/tests/test_bugnotification.py (+426/-16)
lib/lp/bugs/tests/test_bugchanges.py (+84/-1)
To merge this branch: bzr merge lp:~gary/launchpad/bug164196-2
Reviewer Review Type Date Requested Status
Данило Шеган (community) Approve
Review via email: mp+49977@code.launchpad.net

Commit message

use the activity attributes on bug notifications to implement the logic to silence emails for actions that are done and then undone in rapid succession.

Description of the change

This is the second of three branches that address bug 164196. The other two are lp:~gary/launchpad/bug164196-1 and lp:~gary/launchpad/bug164196-3. My pre-implementation call for these changes was with Graham Binns.

This branch uses the activity attributes on bug notifications introduced in the first branch to implement the logic to silence notifications for actions that are done and then undone in rapid succession (that is, to do what the bug requests). Undone actions are omitted from emails, and if the emails then have no content, they are not sent.

The only thing left to do is to make sure that the omitted notification objects are marked as processed so that they are not perpetually considered for subsequent notification cronscript runs. This is tackled in the last branch of the series.

The implementation is all in lib/lp/bugs/scripts/bugnotification.py. Within construct_email_notifications, there were already two loops through the notification objects, so I rearranged them so I could hook into one of them to determine what could be omitted. The changes are a bit smaller than what a quick scan of the diff suggests, because I reordered the two loops.

`get_key` probably holds the subtlest code here: we normalize the bug activity's data to give us a key that will work for items that are linked/added and unlinked/removed. For instance, if a change adds one branch but then removes another, both of these should be reported. We can only squelch the notification if the same branch is added and removed. A less subtle but important part of get_key is that it normalizes the "duplicate" names. Unlike other changes, the activity text changes depending on what happens, so we need to normalize that here.

I renamed the local variable "person_causing_change" to "actor" as an easy solution to some long lines.

The bulk of the branch is the tests for these relatively small changes. I changed the doctest enough for it to pass (and mention the behavior in passing), and updated some test infrastructure in lib/lp/bugs/scripts/tests/test_bugnotification.py (by adding the "activity" attribute to the mock bug notifications), but the main changes are brand new tests at the bottom of lib/lp/bugs/scripts/tests/test_bugnotification.py . I don't test all change objects, just the ones that were unusual. In this case, it means I omitted tests for non-collection bug attributes other than title (representative) and duplicate (exceptional), expecting the rest to work out fine; and I omitted tests for non-collection bugtask attributes other than status. I can add them relatively easily--with fun test subclasses--if you like.

Speaking of subclasses, I don't love what I've done to assemble these tests from subclasses and mixins, but it certainly has precedence in our codebase, and the alternative, with lots of repetition, is even less attractive.

I removed the "def test_suite():" boilerplate because, AIUI, we have now 'fessed up to the fact that it didn't accomplish anything practical, and are actively encouraging removal.

Thank you

To post a comment you must log in.
Данило Шеган (danilo) wrote :

Conclusions from IRC.

To increase trust in what's going on in parsing whatchanged, we should:
 1) add docstring and unittest for get_key
 2) move duplicateof code to target computed attribute code, and add test
 3) change get_key to use target/attribute

As for the rest, I really like the test coverage: great job there!

And a side note for s/person_causing_change/actor/: wordy name was introduced instead of 'person' to help make code easier to understand/read. It's up to you to decide which you want to keep as long as you take code readability into account :)

Considering we have agreed on a few changes, I am marking this as 'approved', though I'd be happy to take a look at incremental diff if it's a significant change.

review: Approve
Gary Poster (gary) wrote :

I'll wait for Данило's review of my changes, because they ended up being fairly extensive.

Данило Шеган (danilo) wrote :

Gary, thanks for the improvements.

This looks much, much, nicer — sorry for putting you through the
trouble, but I think it was worth it. :)

  review approve
  merge approve

Just one minor tidbit below:

>=== modified file 'lib/lp/bugs/scripts/tests/test_bugnotification.py'
...
>+
>+class MockBugActivity:
>+ """A mock BugActivity user for testing."""

A typo most likely: s/user/used/.

Cheers,
Danilo

review: Approve
Robert Collins (lifeless) wrote :

BTW, there are mocks for bugactivity etc in the bugcomment tests
today; may want to snarf those and reused (if you haven't already, I
haven't read your diff).

Cheers,
Rob

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 2011-02-04 01:15:13 +0000
3+++ lib/lp/bugs/adapters/bugchange.py 2011-02-17 17:46:51 +0000
4@@ -5,8 +5,20 @@
5
6 __metaclass__ = type
7 __all__ = [
8+ 'ATTACHMENT_ADDED',
9+ 'ATTACHMENT_REMOVED',
10+ 'BRANCH_LINKED',
11+ 'BRANCH_UNLINKED',
12+ 'BUG_WATCH_ADDED',
13+ 'BUG_WATCH_REMOVED',
14+ 'CHANGED_DUPLICATE_MARKER',
15+ 'CVE_LINKED',
16+ 'CVE_UNLINKED',
17+ 'MARKED_AS_DUPLICATE',
18+ 'REMOVED_DUPLICATE_MARKER',
19 'BranchLinkedToBug',
20 'BranchUnlinkedFromBug',
21+ 'BugAttachmentChange',
22 'BugConvertedToQuestion',
23 'BugDescriptionChange',
24 'BugDuplicateChange',
25@@ -40,13 +52,27 @@
26 from lp.bugs.enum import BugNotificationLevel
27 from lp.bugs.interfaces.bugchange import IBugChange
28 from lp.bugs.interfaces.bugtask import (
29- BugTaskStatus,
30 IBugTask,
31 RESOLVED_BUGTASK_STATUSES,
32 UNRESOLVED_BUGTASK_STATUSES,
33 )
34 from lp.registry.interfaces.product import IProduct
35
36+# These are used lp.bugs.model.bugactivity.BugActivity.attribute to normalize
37+# the output from these change objects into the attribute that actually
38+# changed. It is fragile, but a reasonable incremental step.
39+ATTACHMENT_ADDED = "attachment added"
40+ATTACHMENT_REMOVED = "attachment removed"
41+BRANCH_LINKED = 'branch linked'
42+BRANCH_UNLINKED = 'branch unlinked'
43+BUG_WATCH_ADDED = 'bug watch added'
44+BUG_WATCH_REMOVED = 'bug watch removed'
45+CHANGED_DUPLICATE_MARKER = 'changed duplicate marker'
46+CVE_LINKED = 'cve linked'
47+CVE_UNLINKED = 'cve unlinked'
48+MARKED_AS_DUPLICATE = 'marked as duplicate'
49+REMOVED_DUPLICATE_MARKER = 'removed duplicate marker'
50+
51
52 class NoBugChangeFoundError(Exception):
53 """Raised when a BugChange class can't be found for an object."""
54@@ -254,7 +280,7 @@
55 def getBugActivity(self):
56 """See `IBugChange`."""
57 return dict(
58- whatchanged='bug watch added',
59+ whatchanged=BUG_WATCH_ADDED,
60 newvalue=self.bug_watch.url)
61
62 def getBugNotification(self):
63@@ -278,7 +304,7 @@
64 def getBugActivity(self):
65 """See `IBugChange`."""
66 return dict(
67- whatchanged='bug watch removed',
68+ whatchanged=BUG_WATCH_REMOVED,
69 oldvalue=self.bug_watch.url)
70
71 def getBugNotification(self):
72@@ -305,7 +331,7 @@
73 if self.branch.private:
74 return None
75 return dict(
76- whatchanged='branch linked',
77+ whatchanged=BRANCH_LINKED,
78 newvalue=self.branch.bzr_identity)
79
80 def getBugNotification(self):
81@@ -328,7 +354,7 @@
82 if self.branch.private:
83 return None
84 return dict(
85- whatchanged='branch unlinked',
86+ whatchanged=BRANCH_UNLINKED,
87 oldvalue=self.branch.bzr_identity)
88
89 def getBugNotification(self):
90@@ -397,18 +423,18 @@
91 def getBugActivity(self):
92 if self.old_value is not None and self.new_value is not None:
93 return {
94- 'whatchanged': 'changed duplicate marker',
95+ 'whatchanged': CHANGED_DUPLICATE_MARKER,
96 'oldvalue': str(self.old_value.id),
97 'newvalue': str(self.new_value.id),
98 }
99 elif self.old_value is None:
100 return {
101- 'whatchanged': 'marked as duplicate',
102+ 'whatchanged': MARKED_AS_DUPLICATE,
103 'newvalue': str(self.new_value.id),
104 }
105 elif self.new_value is None:
106 return {
107- 'whatchanged': 'removed duplicate marker',
108+ 'whatchanged': REMOVED_DUPLICATE_MARKER,
109 'oldvalue': str(self.old_value.id),
110 }
111 else:
112@@ -605,13 +631,13 @@
113
114 def getBugActivity(self):
115 if self.old_value is None:
116- what_changed = "attachment added"
117+ what_changed = ATTACHMENT_ADDED
118 old_value = None
119 new_value = "%s %s" % (
120 self.new_value.title,
121 download_url_of_bugattachment(self.new_value))
122 else:
123- what_changed = "attachment removed"
124+ what_changed = ATTACHMENT_REMOVED
125 attachment = self.new_value
126 old_value = "%s %s" % (
127 self.old_value.title,
128@@ -656,7 +682,7 @@
129 """See `IBugChange`."""
130 return dict(
131 newvalue=self.cve.sequence,
132- whatchanged='cve linked')
133+ whatchanged=CVE_LINKED)
134
135 def getBugNotification(self):
136 """See `IBugChange`."""
137@@ -674,7 +700,7 @@
138 """See `IBugChange`."""
139 return dict(
140 oldvalue=self.cve.sequence,
141- whatchanged='cve unlinked')
142+ whatchanged=CVE_UNLINKED)
143
144 def getBugNotification(self):
145 """See `IBugChange`."""
146
147=== modified file 'lib/lp/bugs/browser/bugtask.py'
148--- lib/lp/bugs/browser/bugtask.py 2011-02-16 02:55:17 +0000
149+++ lib/lp/bugs/browser/bugtask.py 2011-02-17 17:46:51 +0000
150@@ -3814,7 +3814,13 @@
151 @property
152 def change_summary(self):
153 """Return a formatted summary of the change."""
154- return self.attribute
155+ if self.target is not None:
156+ # This is a bug task. We want the attribute, as filtered out.
157+ return self.attribute
158+ else:
159+ # Otherwise, the attribute is more normalized than what we want.
160+ # Use "whatchanged," which sometimes is more descriptive.
161+ return self.whatchanged
162
163 @property
164 def _formatted_tags_change(self):
165@@ -3858,30 +3864,31 @@
166 'old_value': self.oldvalue,
167 'new_value': self.newvalue,
168 }
169- if self.attribute == 'summary':
170+ attribute = self.attribute
171+ if attribute == 'title':
172 # We display summary changes as a unified diff, replacing
173 # \ns with <br />s so that the lines are separated properly.
174 diff = cgi.escape(
175 get_unified_diff(self.oldvalue, self.newvalue, 72), True)
176 return diff.replace("\n", "<br />")
177
178- elif self.attribute == 'description':
179+ elif attribute == 'description':
180 # Description changes can be quite long, so we just return
181 # 'updated' rather than returning the whole new description
182 # or a diff.
183 return 'updated'
184
185- elif self.attribute == 'tags':
186+ elif attribute == 'tags':
187 # We special-case tags because we can work out what's been
188 # added and what's been removed.
189 return self._formatted_tags_change.replace('\n', '<br />')
190
191- elif self.attribute == 'assignee':
192+ elif attribute == 'assignee':
193 for key in return_dict:
194 if return_dict[key] is None:
195 return_dict[key] = 'nobody'
196
197- elif self.attribute == 'milestone':
198+ elif attribute == 'milestone':
199 for key in return_dict:
200 if return_dict[key] is None:
201 return_dict[key] = 'none'
202
203=== modified file 'lib/lp/bugs/doc/bugactivity.txt'
204--- lib/lp/bugs/doc/bugactivity.txt 2011-02-11 18:33:04 +0000
205+++ lib/lp/bugs/doc/bugactivity.txt 2011-02-17 17:46:51 +0000
206@@ -113,13 +113,18 @@
207 u'status'
208
209 If the activity is not for a bug task, `target` is None, and `attribute` is
210-the same as `whatchanged`. For instance, look at the attributes on the
211-previous activity.
212+typically the same as `whatchanged`. However, in some cases (ideally,
213+whenever necessary), it is normalized to show the actual attribute name.
214+For instance, look at the attributes on the previous activity.
215
216 >>> print bug.activity[-2].target
217 None
218+ >>> bug.activity[-2].whatchanged
219+ u'summary'
220 >>> bug.activity[-2].attribute
221- u'summary'
222+ 'title'
223+
224+(This is covered more comprehensively in tests/test_bugchanges.py).
225
226 Upstream product assignment edited
227 ==================================
228
229=== modified file 'lib/lp/bugs/doc/bugnotification-sending.txt'
230--- lib/lp/bugs/doc/bugnotification-sending.txt 2011-02-17 15:20:36 +0000
231+++ lib/lp/bugs/doc/bugnotification-sending.txt 2011-02-17 17:46:51 +0000
232@@ -293,14 +293,10 @@
233 - Old summary
234 + New summary
235 <BLANKLINE>
236- ** Visibility changed to: Private
237- <BLANKLINE>
238 ** Summary changed:
239 - New summary
240 + Another summary
241 <BLANKLINE>
242- ** Visibility changed to: Public
243- <BLANKLINE>
244 --
245 You received this bug notification because you are a member of Ubuntu
246 Team, which is the registrant for Ubuntu.
247@@ -312,6 +308,29 @@
248 To: test@canonical.com
249 ...
250
251+If you look carefully, there's a surprise in that output: the visibility
252+changes are not reported. This is because they are done and then undone
253+within the same notification. Undone changes like that are omitted.
254+moreover, if the email only would have reported done/undone changes, it
255+is not sent at all. This is tested elsewhere (see
256+lp/bugs/tests/test_bugnotification.py), and not demonstrated here.
257+
258+Another thing worth noting is that there's a blank line before the
259+signature, and the signature marker has a trailing space.
260+
261+ >>> message.get_payload(decode=True).splitlines()
262+ [...,
263+ '',
264+ '-- ',
265+ 'You received this bug notification because you are a direct subscriber',
266+ 'of the bug.',
267+ 'http://bugs.launchpad.dev/bugs/1',
268+ '',
269+ 'Title:',
270+ ' Firefox does not support SVG'...]
271+
272+ >>> flush_notifications()
273+
274 We send the notification only if the user hasn't done any other changes
275 for the last 5 minutes:
276
277@@ -328,22 +347,6 @@
278
279 >>> flush_notifications()
280
281-There's a blank line before the signature, and the signature marker has
282-a trailing space.
283-
284- >>> message.get_payload(decode=True).splitlines()
285- [...,
286- '',
287- '-- ',
288- 'You received this bug notification because you are a direct subscriber',
289- 'of the bug.',
290- 'http://bugs.launchpad.dev/bugs/1',
291- '',
292- 'Title:',
293- ' Firefox does not support SVG'...]
294-
295- >>> flush_notifications()
296-
297 If a team without a contact address is subscribed to the bug, the
298 notification will be sent to all members individually.
299
300@@ -379,7 +382,6 @@
301
302 >>> flush_notifications()
303
304-
305 Duplicates
306 ----------
307
308
309=== modified file 'lib/lp/bugs/model/bugactivity.py'
310--- lib/lp/bugs/model/bugactivity.py 2011-02-11 18:15:28 +0000
311+++ lib/lp/bugs/model/bugactivity.py 2011-02-17 17:46:51 +0000
312@@ -17,6 +17,19 @@
313
314 from canonical.database.datetimecol import UtcDateTimeCol
315 from canonical.database.sqlbase import SQLBase
316+from lp.bugs.adapters.bugchange import (
317+ ATTACHMENT_ADDED,
318+ ATTACHMENT_REMOVED,
319+ BRANCH_LINKED,
320+ BRANCH_UNLINKED,
321+ BUG_WATCH_ADDED,
322+ BUG_WATCH_REMOVED,
323+ CHANGED_DUPLICATE_MARKER,
324+ CVE_LINKED,
325+ CVE_UNLINKED,
326+ MARKED_AS_DUPLICATE,
327+ REMOVED_DUPLICATE_MARKER,
328+ )
329 from lp.bugs.interfaces.bugactivity import (
330 IBugActivity,
331 IBugActivitySet,
332@@ -68,12 +81,31 @@
333 `attribute` is determined based on the `whatchanged` string.
334
335 :return: The attribute name of the item if `whatchanged` is of
336- the form <target_name>: <attribute>. Otherwise, return the
337- original `whatchanged` string.
338+ the form <target_name>: <attribute>. If we know how to determine
339+ the attribute by normalizing whatchanged, we return that.
340+ Otherwise, return the original `whatchanged` string.
341 """
342 match = self.bugtask_change_re.match(self.whatchanged)
343 if match is None:
344- return self.whatchanged
345+ result = self.whatchanged
346+ # Now we normalize names, as necessary. This is fragile, but
347+ # a reasonable incremental step. These are consumed in
348+ # lp.bugs.scripts.bugnotification.get_activity_key.
349+ if result in (CHANGED_DUPLICATE_MARKER,
350+ MARKED_AS_DUPLICATE,
351+ REMOVED_DUPLICATE_MARKER):
352+ result = 'duplicateof'
353+ elif result in (ATTACHMENT_ADDED, ATTACHMENT_REMOVED):
354+ result = 'attachments'
355+ elif result in (BRANCH_LINKED, BRANCH_UNLINKED):
356+ result = 'linked_branches'
357+ elif result in (BUG_WATCH_ADDED, BUG_WATCH_REMOVED):
358+ result = 'watches'
359+ elif result in (CVE_LINKED, CVE_UNLINKED):
360+ result = 'cves'
361+ elif result == 'summary':
362+ result = 'title'
363+ return result
364 else:
365 return match.groupdict()['attribute']
366
367
368=== modified file 'lib/lp/bugs/scripts/bugnotification.py'
369--- lib/lp/bugs/scripts/bugnotification.py 2011-02-16 14:11:46 +0000
370+++ lib/lp/bugs/scripts/bugnotification.py 2011-02-17 17:46:51 +0000
371@@ -31,6 +31,39 @@
372 from lp.services.mail.mailwrapper import MailWrapper
373
374
375+def get_activity_key(notification):
376+ """Given a notification, return a key for the activity if it exists.
377+
378+ The key will be used to determine whether changes for the activity are
379+ undone within the same batch of notifications (which are supposed to
380+ be all for the same bug when they get to this function). Therefore,
381+ the activity's attribute is a good start for the key.
382+
383+ If the activity was on a bugtask, we will also want to distinguish
384+ by bugtask, because, for instance, changing a status from INPROGRESS
385+ to FIXCOMMITED on one bug task is not undone if the status changes
386+ from FIXCOMMITTED to INPROGRESS on another bugtask.
387+
388+ Similarly, if the activity is about adding or removing something
389+ that we can have multiple of, like a branch or an attachment, the
390+ key should include information on that value, because adding one
391+ attachment is not undone by removing another one.
392+ """
393+ activity = notification.activity
394+ if activity is not None:
395+ key = activity.attribute
396+ if activity.target is not None:
397+ key = ':'.join((activity.target, key))
398+ if key in ('attachments', 'watches', 'cves', 'linked_branches'):
399+ # We are intentionally leaving bug task bugwatches out of this
400+ # list, so we use the key rather than the activity.attribute.
401+ if activity.oldvalue is not None:
402+ key = ':'.join((key, activity.oldvalue))
403+ elif activity.newvalue is not None:
404+ key = ':'.join((key, activity.newvalue))
405+ return key
406+
407+
408 def construct_email_notifications(bug_notifications):
409 """Construct an email from a list of related bug notifications.
410
411@@ -39,31 +72,45 @@
412 """
413 first_notification = bug_notifications[0]
414 bug = first_notification.bug
415- person_causing_change = first_notification.message.owner
416+ actor = first_notification.message.owner
417 subject = first_notification.message.subject
418
419 comment = None
420 references = []
421 text_notifications = []
422-
423- recipients = {}
424- for notification in bug_notifications:
425- for recipient in notification.recipients:
426- email_people = emailPeople(recipient.person)
427- if (not person_causing_change.selfgenerated_bugnotifications and
428- person_causing_change in email_people):
429- email_people.remove(person_causing_change)
430- for email_person in email_people:
431- recipients[email_person] = recipient
432+ old_values = {}
433+ new_values = {}
434
435 for notification in bug_notifications:
436 assert notification.bug == bug, bug.id
437- assert notification.message.owner == person_causing_change, (
438- person_causing_change.id)
439+ assert notification.message.owner == actor, actor.id
440 if notification.is_comment:
441 assert comment is None, (
442 "Only one of the notifications is allowed to be a comment.")
443 comment = notification.message
444+ else:
445+ key = get_activity_key(notification)
446+ if key is not None:
447+ if key not in old_values:
448+ old_values[key] = notification.activity.oldvalue
449+ new_values[key] = notification.activity.newvalue
450+
451+ recipients = {}
452+ filtered_notifications = []
453+ for notification in bug_notifications:
454+ key = get_activity_key(notification)
455+ if (notification.is_comment or
456+ key is None or
457+ old_values[key] != new_values[key]):
458+ # We will report this notification.
459+ filtered_notifications.append(notification)
460+ for recipient in notification.recipients:
461+ email_people = emailPeople(recipient.person)
462+ if (not actor.selfgenerated_bugnotifications and
463+ actor in email_people):
464+ email_people.remove(actor)
465+ for email_person in email_people:
466+ recipients[email_person] = recipient
467
468 if bug.duplicateof is not None:
469 text_notifications.append(
470@@ -88,7 +135,7 @@
471 msgid = first_notification.message.rfc822msgid
472 email_date = first_notification.message.datecreated
473
474- for notification in bug_notifications:
475+ for notification in filtered_notifications:
476 if notification.message == comment:
477 # Comments were just handled in the previous if block.
478 continue
479@@ -104,9 +151,8 @@
480 messages = []
481 mail_wrapper = MailWrapper(width=72)
482 content = '\n\n'.join(text_notifications)
483- from_address = get_bugmail_from_address(person_causing_change, bug)
484- bug_notification_builder = BugNotificationBuilder(
485- bug, person_causing_change)
486+ from_address = get_bugmail_from_address(actor, bug)
487+ bug_notification_builder = BugNotificationBuilder(bug, actor)
488 sorted_recipients = sorted(
489 recipients.items(), key=lambda t: t[0].preferredemail.email)
490 for email_person, recipient in sorted_recipients:
491@@ -157,7 +203,7 @@
492 rationale, references, msgid)
493 messages.append(msg)
494
495- return bug_notifications, messages
496+ return filtered_notifications, messages
497
498
499 def notification_comment_batches(notifications):
500
501=== modified file 'lib/lp/bugs/scripts/tests/test_bugnotification.py'
502--- lib/lp/bugs/scripts/tests/test_bugnotification.py 2011-02-17 09:17:00 +0000
503+++ lib/lp/bugs/scripts/tests/test_bugnotification.py 2011-02-17 17:46:51 +0000
504@@ -4,31 +4,57 @@
505
506 __metaclass__ = type
507
508-from datetime import datetime
509+from datetime import datetime, timedelta
510 import unittest
511
512 import pytz
513+from testtools.matchers import Not
514+from transaction import commit
515 from zope.component import getUtility
516 from zope.interface import implements
517
518 from canonical.config import config
519-from canonical.database.sqlbase import commit
520-from lp.bugs.model.bugtask import BugTask
521+from canonical.database.sqlbase import flush_database_updates
522+from canonical.launchpad.ftests import login
523 from canonical.launchpad.helpers import get_contact_email_addresses
524 from canonical.launchpad.interfaces.message import IMessageSet
525 from canonical.testing.layers import LaunchpadZopelessLayer
526+from lp.bugs.adapters.bugchange import (
527+ BranchLinkedToBug,
528+ BranchUnlinkedFromBug,
529+ BugAttachmentChange,
530+ BugDuplicateChange,
531+ BugTagsChange,
532+ BugTaskStatusChange,
533+ BugTitleChange,
534+ BugVisibilityChange,
535+ BugWatchAdded,
536+ BugWatchRemoved,
537+ CveLinkedToBug,
538+ CveUnlinkedFromBug,
539+ )
540 from lp.bugs.interfaces.bug import (
541 IBug,
542 IBugSet,
543 )
544+from lp.bugs.interfaces.bugnotification import IBugNotificationSet
545+from lp.bugs.interfaces.bugtask import BugTaskStatus
546 from lp.bugs.mail.bugnotificationrecipients import BugNotificationRecipients
547+from lp.bugs.model.bugtask import BugTask
548 from lp.bugs.scripts.bugnotification import (
549 get_email_notifications,
550+ get_activity_key,
551 notification_batches,
552 notification_comment_batches,
553 )
554 from lp.registry.interfaces.person import IPersonSet
555 from lp.registry.interfaces.product import IProductSet
556+from lp.services.propertycache import cachedproperty
557+from lp.testing import (
558+ TestCase,
559+ TestCaseWithFactory)
560+from lp.testing.dbuser import lp_dbuser
561+from lp.testing.matchers import Contains
562
563
564 class MockBug:
565@@ -105,6 +131,67 @@
566 self.is_comment = is_comment
567 self.date_emailed = date_emailed
568 self.recipients = [MockBugNotificationRecipient()]
569+ self.activity = None
570+
571+
572+class FakeNotification:
573+ """An even simpler fake notification.
574+
575+ Used by TestGetActivityKey, TestNotificationCommentBatches and
576+ TestNotificationBatches."""
577+
578+ class Message(object):
579+ pass
580+
581+ def __init__(self, is_comment=False, bug=None, owner=None):
582+ self.is_comment = is_comment
583+ self.bug = bug
584+ self.message = self.Message()
585+ self.message.owner = owner
586+ self.activity = None
587+
588+
589+class MockBugActivity:
590+ """A mock BugActivity used for testing."""
591+ def __init__(self, target=None, attribute=None,
592+ oldvalue=None, newvalue=None):
593+ self.target = target
594+ self.attribute = attribute
595+ self.oldvalue = oldvalue
596+ self.newvalue = newvalue
597+
598+
599+class TestGetActivityKey(TestCase):
600+ """Tests for get_activity_key()."""
601+
602+ def test_no_activity(self):
603+ self.assertEqual(get_activity_key(FakeNotification()), None)
604+
605+ def test_normal_bug_attribute_activity(self):
606+ notification = FakeNotification()
607+ notification.activity = MockBugActivity(attribute='title')
608+ self.assertEqual(get_activity_key(notification), 'title')
609+
610+ def test_collection_bug_attribute_added_activity(self):
611+ notification = FakeNotification()
612+ notification.activity = MockBugActivity(
613+ attribute='cves', newvalue='some cve identifier')
614+ self.assertEqual(get_activity_key(notification),
615+ 'cves:some cve identifier')
616+
617+ def test_collection_bug_attribute_removed_activity(self):
618+ notification = FakeNotification()
619+ notification.activity = MockBugActivity(
620+ attribute='cves', oldvalue='some cve identifier')
621+ self.assertEqual(get_activity_key(notification),
622+ 'cves:some cve identifier')
623+
624+ def test_bugtask_attribute_activity(self):
625+ notification = FakeNotification()
626+ notification.activity = MockBugActivity(
627+ attribute='status', target='some bug task identifier')
628+ self.assertEqual(get_activity_key(notification),
629+ 'some bug task identifier:status')
630
631
632 class TestGetEmailNotifications(unittest.TestCase):
633@@ -240,19 +327,6 @@
634 self.assertEqual(bug_four.id, 4)
635
636
637-class FakeNotification:
638- """Used by TestNotificationCommentBatches and TestNotificationBatches."""
639-
640- class Message(object):
641- pass
642-
643- def __init__(self, is_comment=False, bug=None, owner=None):
644- self.is_comment = is_comment
645- self.bug = bug
646- self.message = self.Message()
647- self.message.owner = owner
648-
649-
650 class TestNotificationCommentBatches(unittest.TestCase):
651 """Tests of `notification_comment_batches`."""
652
653@@ -456,3 +530,339 @@
654 expected = [notifications[0:3], notifications[3:4]]
655 observed = list(notification_batches(notifications))
656 self.assertEquals(expected, observed)
657+
658+
659+class EmailNotificationTestBase(TestCaseWithFactory):
660+
661+ layer = LaunchpadZopelessLayer
662+
663+ def setUp(self):
664+ super(EmailNotificationTestBase, self).setUp()
665+ login('foo.bar@canonical.com')
666+ self.product_owner = self.factory.makePerson(name="product-owner")
667+ self.person = self.factory.makePerson(name="sample-person")
668+ self.product = self.factory.makeProduct(owner=self.product_owner)
669+ self.product_subscriber = self.factory.makePerson(
670+ name="product-subscriber")
671+ self.product.addBugSubscription(
672+ self.product_subscriber, self.product_subscriber)
673+ self.bug_subscriber = self.factory.makePerson(name="bug-subscriber")
674+ self.bug_owner = self.factory.makePerson(name="bug-owner")
675+ self.bug = self.factory.makeBug(
676+ product=self.product, private=False, owner=self.bug_owner)
677+ self.reporter = self.bug.owner
678+ self.bug.subscribe(self.bug_subscriber, self.reporter)
679+ [self.product_bugtask] = self.bug.bugtasks
680+ commit()
681+ login('test@canonical.com')
682+ self.layer.switchDbUser(config.malone.bugnotification_dbuser)
683+ self.now = datetime.now(pytz.timezone('UTC'))
684+ self.ten_minutes_ago = self.now - timedelta(minutes=10)
685+ self.notification_set = getUtility(IBugNotificationSet)
686+ for notification in self.notification_set.getNotificationsToSend():
687+ notification.date_emailed = self.now
688+ flush_database_updates()
689+
690+ def tearDown(self):
691+ for notification in self.notification_set.getNotificationsToSend():
692+ notification.date_emailed = self.now
693+ flush_database_updates()
694+ super(EmailNotificationTestBase, self).tearDown()
695+
696+ def get_messages(self):
697+ notifications = self.notification_set.getNotificationsToSend()
698+ email_notifications = get_email_notifications(notifications)
699+ for bug_notifications, messages in email_notifications:
700+ for message in messages:
701+ yield message, message.get_payload(decode=True)
702+
703+
704+class EmailNotificationsBugMixin:
705+
706+ change_class = change_name = old = new = alt = unexpected_text = None
707+
708+ def change(self, old, new):
709+ self.bug.addChange(
710+ self.change_class(
711+ self.ten_minutes_ago, self.person, self.change_name,
712+ old, new))
713+
714+ def change_other(self):
715+ self.bug.addChange(
716+ BugVisibilityChange(
717+ self.ten_minutes_ago, self.person, "private",
718+ False, True))
719+
720+ def test_change_seen(self):
721+ # A smoketest.
722+ self.change(self.old, self.new)
723+ message, body = self.get_messages().next()
724+ self.assertThat(body, Contains(self.unexpected_text))
725+
726+ def test_undone_change_sends_no_emails(self):
727+ self.change(self.old, self.new)
728+ self.change(self.new, self.old)
729+ self.assertEqual(list(self.get_messages()), [])
730+
731+ def test_undone_change_is_not_included(self):
732+ self.change(self.old, self.new)
733+ self.change(self.new, self.old)
734+ self.change_other()
735+ message, body = self.get_messages().next()
736+ self.assertThat(body, Not(Contains(self.unexpected_text)))
737+
738+ def test_multiple_undone_changes_sends_no_emails(self):
739+ self.change(self.old, self.new)
740+ self.change(self.new, self.alt)
741+ self.change(self.alt, self.old)
742+ self.assertEqual(list(self.get_messages()), [])
743+
744+
745+class EmailNotificationsBugNotRequiredMixin(EmailNotificationsBugMixin):
746+ # This test collection is for attributes that can be None.
747+ def test_added_removed_sends_no_emails(self):
748+ self.change(None, self.old)
749+ self.change(self.old, None)
750+ self.assertEqual(list(self.get_messages()), [])
751+
752+ def test_removed_added_sends_no_emails(self):
753+ self.change(self.old, None)
754+ self.change(None, self.old)
755+ self.assertEqual(list(self.get_messages()), [])
756+
757+ def test_duplicate_marked_changed_removed_sends_no_emails(self):
758+ self.change(None, self.old)
759+ self.change(self.old, self.new)
760+ self.change(self.new, None)
761+ self.assertEqual(list(self.get_messages()), [])
762+
763+
764+class EmailNotificationsBugTaskMixin(EmailNotificationsBugMixin):
765+
766+ def change(self, old, new, index=0):
767+ self.bug.addChange(
768+ self.change_class(
769+ self.bug.bugtasks[index], self.ten_minutes_ago,
770+ self.person, self.change_name, old, new))
771+
772+ def test_changing_on_different_bugtasks_is_not_undoing(self):
773+ with lp_dbuser():
774+ product2 = self.factory.makeProduct(owner=self.product_owner)
775+ self.bug.addTask(self.product_owner, product2)
776+ self.change(self.old, self.new, index=0)
777+ self.change(self.new, self.old, index=1)
778+ message, body = self.get_messages().next()
779+ self.assertThat(body, Contains(self.unexpected_text))
780+
781+
782+class EmailNotificationsAddedRemovedMixin:
783+
784+ old = new = added_message = removed_message = None
785+
786+ def add(self, item):
787+ raise NotImplementedError
788+ remove = add
789+
790+ def test_added_seen(self):
791+ self.add(self.old)
792+ message, body = self.get_messages().next()
793+ self.assertThat(body, Contains(self.added_message))
794+
795+ def test_added_removed_sends_no_emails(self):
796+ self.add(self.old)
797+ self.remove(self.old)
798+ self.assertEqual(list(self.get_messages()), [])
799+
800+ def test_removed_added_sends_no_emails(self):
801+ self.remove(self.old)
802+ self.add(self.old)
803+ self.assertEqual(list(self.get_messages()), [])
804+
805+ def test_added_another_removed_sends_emails(self):
806+ self.add(self.old)
807+ self.remove(self.new)
808+ message, body = self.get_messages().next()
809+ self.assertThat(body, Contains(self.added_message))
810+ self.assertThat(body, Contains(self.removed_message))
811+
812+
813+class TestEmailNotificationsBugTitle(
814+ EmailNotificationsBugMixin, EmailNotificationTestBase):
815+
816+ change_class = BugTitleChange
817+ change_name = "title"
818+ old = "Old summary"
819+ new = "New summary"
820+ alt = "Another summary"
821+ unexpected_text = '** Summary changed:'
822+
823+
824+class TestEmailNotificationsBugTags(
825+ EmailNotificationsBugMixin, EmailNotificationTestBase):
826+
827+ change_class = BugTagsChange
828+ change_name = "tags"
829+ old = ['foo', 'bar', 'baz']
830+ new = ['foo', 'bar']
831+ alt = ['bing', 'shazam']
832+ unexpected_text = '** Tags'
833+
834+ def test_undone_ordered_set_sends_no_email(self):
835+ # Tags use ordered sets to generate change descriptions, which we
836+ # demonstrate here.
837+ self.change(['foo', 'bar', 'baz'], ['foo', 'bar'])
838+ self.change(['foo', 'bar'], ['baz', 'bar', 'foo', 'bar'])
839+ self.assertEqual(list(self.get_messages()), [])
840+
841+
842+class TestEmailNotificationsBugDuplicate(
843+ EmailNotificationsBugNotRequiredMixin, EmailNotificationTestBase):
844+
845+ change_class = BugDuplicateChange
846+ change_name = "duplicateof"
847+ unexpected_text = 'duplicate'
848+
849+ def _bug(self):
850+ with lp_dbuser():
851+ return self.factory.makeBug()
852+
853+ old = cachedproperty('old')(_bug)
854+ new = cachedproperty('new')(_bug)
855+ alt = cachedproperty('alt')(_bug)
856+
857+
858+class TestEmailNotificationsBugTaskStatus(
859+ EmailNotificationsBugTaskMixin, EmailNotificationTestBase):
860+
861+ change_class = BugTaskStatusChange
862+ change_name = "status"
863+ old = BugTaskStatus.TRIAGED
864+ new = BugTaskStatus.INPROGRESS
865+ alt = BugTaskStatus.INVALID
866+ unexpected_text = 'Status: '
867+
868+
869+class TestEmailNotificationsBugWatch(
870+ EmailNotificationsAddedRemovedMixin, EmailNotificationTestBase):
871+
872+ # Note that this is for bugwatches added to bugs. Bugwatches added
873+ # to bugtasks are separate animals AIUI, and we don't try to combine
874+ # them here for notifications. Bugtasks have only zero or one
875+ # bugwatch, so they can be handled just as a simple bugtask attribute
876+ # change, like status.
877+
878+ added_message = '** Bug watch added:'
879+ removed_message = '** Bug watch removed:'
880+
881+ @cachedproperty
882+ def tracker(self):
883+ with lp_dbuser():
884+ return self.factory.makeBugTracker()
885+
886+ def _watch(self, identifier='123'):
887+ with lp_dbuser():
888+ # This actually creates a notification all by itself. However,
889+ # it won't be sent out for another five minutes. Therefore,
890+ # we send out separate change notifications.
891+ return self.bug.addWatch(
892+ self.tracker, identifier, self.product_owner)
893+
894+ old = cachedproperty('old')(_watch)
895+ new = cachedproperty('new')(lambda self: self._watch('456'))
896+
897+ def add(self, item):
898+ with lp_dbuser():
899+ self.bug.addChange(
900+ BugWatchAdded(
901+ self.ten_minutes_ago, self.product_owner, item))
902+
903+ def remove(self, item):
904+ with lp_dbuser():
905+ self.bug.addChange(
906+ BugWatchRemoved(
907+ self.ten_minutes_ago, self.product_owner, item))
908+
909+
910+class TestEmailNotificationsBranch(
911+ EmailNotificationsAddedRemovedMixin, EmailNotificationTestBase):
912+
913+ added_message = '** Branch linked:'
914+ removed_message = '** Branch unlinked:'
915+
916+ def _branch(self):
917+ with lp_dbuser():
918+ return self.factory.makeBranch()
919+
920+ old = cachedproperty('old')(_branch)
921+ new = cachedproperty('new')(_branch)
922+
923+ def add(self, item):
924+ with lp_dbuser():
925+ self.bug.addChange(
926+ BranchLinkedToBug(
927+ self.ten_minutes_ago, self.person, item, self.bug))
928+
929+ def remove(self, item):
930+ with lp_dbuser():
931+ self.bug.addChange(
932+ BranchUnlinkedFromBug(
933+ self.ten_minutes_ago, self.person, item, self.bug))
934+
935+
936+class TestEmailNotificationsCVE(
937+ EmailNotificationsAddedRemovedMixin, EmailNotificationTestBase):
938+
939+ added_message = '** CVE added:'
940+ removed_message = '** CVE removed:'
941+
942+ def _cve(self, sequence):
943+ with lp_dbuser():
944+ return self.factory.makeCVE(sequence)
945+
946+ old = cachedproperty('old')(lambda self: self._cve('2020-1234'))
947+ new = cachedproperty('new')(lambda self: self._cve('2020-5678'))
948+
949+ def add(self, item):
950+ with lp_dbuser():
951+ self.bug.addChange(
952+ CveLinkedToBug(
953+ self.ten_minutes_ago, self.person, item))
954+
955+ def remove(self, item):
956+ with lp_dbuser():
957+ self.bug.addChange(
958+ CveUnlinkedFromBug(
959+ self.ten_minutes_ago, self.person, item))
960+
961+
962+class TestEmailNotificationsAttachments(
963+ EmailNotificationsAddedRemovedMixin, EmailNotificationTestBase):
964+
965+ added_message = '** Attachment added:'
966+ removed_message = '** Attachment removed:'
967+
968+ def _attachment(self):
969+ with lp_dbuser():
970+ # This actually creates a notification all by itself, via an
971+ # event subscriber. However, it won't be sent out for
972+ # another five minutes. Therefore, we send out separate
973+ # change notifications.
974+ return self.bug.addAttachment(
975+ self.person, 'content', 'a comment', 'stuff.txt')
976+
977+ old = cachedproperty('old')(_attachment)
978+ new = cachedproperty('new')(_attachment)
979+
980+ def add(self, item):
981+ with lp_dbuser():
982+ self.bug.addChange(
983+ BugAttachmentChange(
984+ self.ten_minutes_ago, self.person, 'attachment',
985+ None, item))
986+
987+ def remove(self, item):
988+ with lp_dbuser():
989+ self.bug.addChange(
990+ BugAttachmentChange(
991+ self.ten_minutes_ago, self.person, 'attachment',
992+ item, None))
993
994=== modified file 'lib/lp/bugs/tests/test_bugchanges.py'
995--- lib/lp/bugs/tests/test_bugchanges.py 2011-02-16 12:05:44 +0000
996+++ lib/lp/bugs/tests/test_bugchanges.py 2011-02-17 17:46:51 +0000
997@@ -8,12 +8,12 @@
998 ObjectModifiedEvent,
999 )
1000 from lazr.lifecycle.snapshot import Snapshot
1001+from testtools.matchers import StartsWith
1002 from zope.component import getUtility
1003 from zope.event import notify
1004 from zope.interface import providedBy
1005
1006 from canonical.launchpad.browser.librarian import ProxiedLibraryFileAlias
1007-from lp.bugs.model.bugnotification import BugNotification
1008 from canonical.launchpad.webapp.interfaces import ILaunchBag
1009 from canonical.launchpad.webapp.publisher import canonical_url
1010 from canonical.testing.layers import LaunchpadFunctionalLayer
1011@@ -24,6 +24,7 @@
1012 BugTaskStatus,
1013 )
1014 from lp.bugs.interfaces.cve import ICveSet
1015+from lp.bugs.model.bugnotification import BugNotification
1016 from lp.bugs.scripts.bugnotification import construct_email_notifications
1017 from lp.testing import (
1018 person_logged_in,
1019@@ -220,6 +221,11 @@
1020 # the Bug's notifications.
1021 old_title = self.changeAttribute(self.bug, 'title', '42')
1022
1023+ # This checks the activity's attribute and target attributes.
1024+ activity = self.bug.activity[-1]
1025+ self.assertEqual(activity.attribute, 'title')
1026+ self.assertEqual(activity.target, None)
1027+
1028 title_change_activity = {
1029 'whatchanged': 'summary',
1030 'oldvalue': old_title,
1031@@ -270,6 +276,11 @@
1032 bugtracker = self.factory.makeBugTracker()
1033 bug_watch = self.bug.addWatch(bugtracker, '42', self.user)
1034
1035+ # This checks the activity's attribute and target attributes.
1036+ activity = self.bug.activity[-1]
1037+ self.assertEqual(activity.attribute, 'watches')
1038+ self.assertEqual(activity.target, None)
1039+
1040 bugwatch_activity = {
1041 'person': self.user,
1042 'whatchanged': 'bug watch added',
1043@@ -337,6 +348,11 @@
1044 self.saveOldChanges()
1045 self.bug.removeWatch(bug_watch, self.user)
1046
1047+ # This checks the activity's attribute and target attributes.
1048+ activity = self.bug.activity[-1]
1049+ self.assertEqual(activity.attribute, 'watches')
1050+ self.assertEqual(activity.target, None)
1051+
1052 bugwatch_activity = {
1053 'person': self.user,
1054 'whatchanged': 'bug watch removed',
1055@@ -413,6 +429,12 @@
1056 # sends an e-mail notification.
1057 branch = self.factory.makeBranch()
1058 self.bug.linkBranch(branch, self.user)
1059+
1060+ # This checks the activity's attribute and target attributes.
1061+ activity = self.bug.activity[-1]
1062+ self.assertEqual(activity.attribute, 'linked_branches')
1063+ self.assertEqual(activity.target, None)
1064+
1065 added_activity = {
1066 'person': self.user,
1067 'whatchanged': 'branch linked',
1068@@ -459,6 +481,12 @@
1069 self.bug.linkBranch(branch, self.user)
1070 self.saveOldChanges()
1071 self.bug.unlinkBranch(branch, self.user)
1072+
1073+ # This checks the activity's attribute and target attributes.
1074+ activity = self.bug.activity[-1]
1075+ self.assertEqual(activity.attribute, 'linked_branches')
1076+ self.assertEqual(activity.target, None)
1077+
1078 added_activity = {
1079 'person': self.user,
1080 'whatchanged': 'branch unlinked',
1081@@ -654,6 +682,11 @@
1082 cve = getUtility(ICveSet)['1999-8979']
1083 self.bug.linkCVE(cve, self.user)
1084
1085+ # This checks the activity's attribute and target attributes.
1086+ activity = self.bug.activity[-1]
1087+ self.assertEqual(activity.attribute, 'cves')
1088+ self.assertEqual(activity.target, None)
1089+
1090 cve_linked_activity = {
1091 'person': self.user,
1092 'whatchanged': 'cve linked',
1093@@ -680,6 +713,11 @@
1094 self.saveOldChanges()
1095 self.bug.unlinkCVE(cve, self.user)
1096
1097+ # This checks the activity's attribute and target attributes.
1098+ activity = self.bug.activity[-1]
1099+ self.assertEqual(activity.attribute, 'cves')
1100+ self.assertEqual(activity.target, None)
1101+
1102 cve_unlinked_activity = {
1103 'person': self.user,
1104 'whatchanged': 'cve unlinked',
1105@@ -708,6 +746,11 @@
1106 attachment = self.factory.makeBugAttachment(
1107 bug=self.bug, owner=self.user, comment=message)
1108
1109+ # This checks the activity's attribute and target attributes.
1110+ activity = self.bug.activity[-1]
1111+ self.assertEqual(activity.attribute, 'attachments')
1112+ self.assertEqual(activity.target, None)
1113+
1114 attachment_added_activity = {
1115 'person': self.user,
1116 'whatchanged': 'attachment added',
1117@@ -741,6 +784,11 @@
1118
1119 attachment.removeFromBug(user=self.user)
1120
1121+ # This checks the activity's attribute and target attributes.
1122+ activity = self.bug.activity[-1]
1123+ self.assertEqual(activity.attribute, 'attachments')
1124+ self.assertEqual(activity.target, None)
1125+
1126 attachment_removed_activity = {
1127 'person': self.user,
1128 'whatchanged': 'attachment removed',
1129@@ -858,6 +906,11 @@
1130 self.bug_task, bug_task_before_modification,
1131 ['importance'], user=self.user))
1132
1133+ # This checks the activity's attribute and target attributes.
1134+ activity = self.bug.activity[-1]
1135+ self.assertEqual(activity.attribute, 'importance')
1136+ self.assertThat(activity.target, StartsWith(u'product-name'))
1137+
1138 expected_activity = {
1139 'person': self.user,
1140 'whatchanged': '%s: importance' % self.bug_task.bugtargetname,
1141@@ -1304,6 +1357,11 @@
1142 level=BugNotificationLevel.METADATA).getRecipients()
1143 self.changeAttribute(duplicate_bug, 'duplicateof', self.bug)
1144
1145+ # This checks the activity's attribute and target attributes.
1146+ activity = duplicate_bug.activity[-1]
1147+ self.assertEqual(activity.attribute, 'duplicateof')
1148+ self.assertEqual(activity.target, None)
1149+
1150 expected_activity = {
1151 'person': self.user,
1152 'whatchanged': 'marked as duplicate',
1153@@ -1351,6 +1409,11 @@
1154 self.saveOldChanges(duplicate_bug)
1155 self.changeAttribute(duplicate_bug, 'duplicateof', None)
1156
1157+ # This checks the activity's attribute and target attributes.
1158+ activity = duplicate_bug.activity[-1]
1159+ self.assertEqual(activity.attribute, 'duplicateof')
1160+ self.assertEqual(activity.target, None)
1161+
1162 expected_activity = {
1163 'person': self.user,
1164 'whatchanged': 'removed duplicate marker',
1165@@ -1382,6 +1445,11 @@
1166 self.saveOldChanges()
1167 self.changeAttribute(self.bug, 'duplicateof', bug_two)
1168
1169+ # This checks the activity's attribute and target attributes.
1170+ activity = self.bug.activity[-1]
1171+ self.assertEqual(activity.attribute, 'duplicateof')
1172+ self.assertEqual(activity.target, None)
1173+
1174 expected_activity = {
1175 'person': self.user,
1176 'whatchanged': 'changed duplicate marker',
1177@@ -1429,6 +1497,11 @@
1178 level=BugNotificationLevel.METADATA).getRecipients()
1179 self.changeAttribute(public_bug, 'duplicateof', private_bug)
1180
1181+ # This checks the activity's attribute and target attributes.
1182+ activity = public_bug.activity[-1]
1183+ self.assertEqual(activity.attribute, 'duplicateof')
1184+ self.assertEqual(activity.target, None)
1185+
1186 expected_activity = {
1187 'person': self.user,
1188 'whatchanged': 'marked as duplicate',
1189@@ -1468,6 +1541,11 @@
1190
1191 self.changeAttribute(public_bug, 'duplicateof', None)
1192
1193+ # This checks the activity's attribute and target attributes.
1194+ activity = public_bug.activity[-1]
1195+ self.assertEqual(activity.attribute, 'duplicateof')
1196+ self.assertEqual(activity.target, None)
1197+
1198 expected_activity = {
1199 'person': self.user,
1200 'whatchanged': 'removed duplicate marker',
1201@@ -1504,6 +1582,11 @@
1202
1203 self.changeAttribute(duplicate_bug, 'duplicateof', public_bug)
1204
1205+ # This checks the activity's attribute and target attributes.
1206+ activity = duplicate_bug.activity[-1]
1207+ self.assertEqual(activity.attribute, 'duplicateof')
1208+ self.assertEqual(activity.target, None)
1209+
1210 expected_activity = {
1211 'person': self.user,
1212 'whatchanged': 'changed duplicate marker',

Subscribers

People subscribed via source and target branches

to status/vote changes: