Launchpad itself

Merge lp:~intellectronica/launchpad/patch-in-mailnotification into lp:launchpad

  1. patch-in-mailnotification
  2. Merge into devel
Proposed by Eleanor Berger
Status: Merged
Merged at revision: not available
Proposed branch: lp:~intellectronica/launchpad/patch-in-mailnotification
Merge into: lp:launchpad
Diff against target: 149 lines (+72/-6)
5 files modified
lib/lp/bugs/adapters/bugchange.py (+14/-4)
lib/lp/bugs/doc/bugattachments.txt (+11/-0)
lib/lp/bugs/doc/bugnotification-email.txt (+38/-1)
lib/lp/bugs/interfaces/bugattachment.py (+4/-1)
lib/lp/bugs/model/bugattachment.py (+5/-0)
To merge this branch: bzr merge lp:~intellectronica/launchpad/patch-in-mailnotification
Reviewer Review Type Date Requested Status
Deryck Hodge (community) code Approve
Review via email: mp+17761@code.launchpad.net
To post a comment you must log in.
Revision history for this message
Eleanor Berger (intellectronica) wrote :

This branch changes the the bug notification for added patches, so that it reads 'Patch added' (or removed). To make testing the attachment type easier we added a helper property, is_patch, so that we don't have to import the enumeration and compare. Both changes are tested.

Revision history for this message
Deryck Hodge (deryck) wrote :

As discussed IRL, I think we should avoid the conditional expression until we ask about it at the reviewers meeting. Otherwise, r=me. Thanks!

review: Approve (code)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'lib/lp/bugs/adapters/bugchange.py'
--- lib/lp/bugs/adapters/bugchange.py 2009-07-18 00:05:49 +0000
+++ lib/lp/bugs/adapters/bugchange.py 2010-01-22 17:25:27 +0000
@@ -523,11 +523,21 @@
523523
524 def getBugNotification(self):524 def getBugNotification(self):
525 if self.old_value is None:525 if self.old_value is None:
526 message = '** Attachment added: "%s"\n %s' % (526 if self.new_value.is_patch:
527 self.new_value.title, self.new_value.libraryfile.http_url)527 attachment_str = 'Patch'
528 else:
529 attachment_str = 'Attachment'
530 message = '** %s added: "%s"\n %s' % (
531 attachment_str, self.new_value.title,
532 self.new_value.libraryfile.http_url)
528 else:533 else:
529 message = '** Attachment removed: "%s"\n %s' % (534 if self.old_value.is_patch:
530 self.old_value.title, self.old_value.libraryfile.http_url)535 attachment_str = 'Patch'
536 else:
537 attachment_str = 'Attachment'
538 message = '** %s removed: "%s"\n %s' % (
539 attachment_str, self.old_value.title,
540 self.old_value.libraryfile.http_url)
531541
532 return {'text': message}542 return {'text': message}
533543
534544
=== modified file 'lib/lp/bugs/doc/bugattachments.txt'
--- lib/lp/bugs/doc/bugattachments.txt 2009-12-09 11:14:16 +0000
+++ lib/lp/bugs/doc/bugattachments.txt 2010-01-22 17:25:27 +0000
@@ -427,6 +427,17 @@
427 >>> bugs[0].id427 >>> bugs[0].id
428 1428 1
429429
430An easy way to determine whether an attachment is a patch is to read its
431`is_patch` attribute.
432
433 >>> attachment.type = BugAttachmentType.PATCH
434 >>> attachment.is_patch
435 True
436
437 >>> attachment.type = BugAttachmentType.UNSPECIFIED
438 >>> attachment.is_patch
439 False
440
430441
431== Deleting attachments ==442== Deleting attachments ==
432443
433444
=== modified file 'lib/lp/bugs/doc/bugnotification-email.txt'
--- lib/lp/bugs/doc/bugnotification-email.txt 2009-07-22 09:12:01 +0000
+++ lib/lp/bugs/doc/bugnotification-email.txt 2010-01-22 17:25:27 +0000
@@ -347,9 +347,10 @@
347 ... def __init__(self, url):347 ... def __init__(self, url):
348 ... self.http_url = url348 ... self.http_url = url
349 >>> class MockAttachment:349 >>> class MockAttachment:
350 ... def __init__(self, title, libraryfile):350 ... def __init__(self, title, libraryfile, is_patch=False):
351 ... self.title = title351 ... self.title = title
352 ... self.libraryfile = libraryfile352 ... self.libraryfile = libraryfile
353 ... self.is_patch = is_patch
353 >>> attachment = MockAttachment(354 >>> attachment = MockAttachment(
354 ... title="A screenshot of the problem",355 ... title="A screenshot of the problem",
355 ... libraryfile=MockLibraryFile('http://foo.com/screenshot.png'))356 ... libraryfile=MockLibraryFile('http://foo.com/screenshot.png'))
@@ -382,6 +383,42 @@
382 http://foo.com/screenshot.png383 http://foo.com/screenshot.png
383 -----------------------------384 -----------------------------
384385
386Adding an attachment and marking it as a patch generates a different
387notification.
388
389 >>> attachment = MockAttachment(
390 ... title="A new icon for the application",
391 ... libraryfile=MockLibraryFile('http://foo.com/new-icon.png'),
392 ... is_patch=True)
393 >>> bug_delta = BugDelta(
394 ... bug=edited_bug,
395 ... bugurl="http://www.example.com/bugs/2",
396 ... user=sample_person,
397 ... attachment={'new' : attachment, 'old': None})
398 >>> for change in get_bug_changes(bug_delta):
399 ... notification = change.getBugNotification()
400 ... print notification['text'] #doctest: -NORMALIZE_WHITESPACE
401 ... print "-----------------------------"
402 ** Patch added: "A new icon for the application"
403 http://foo.com/new-icon.png
404 -----------------------------
405
406Removing a patch also generates a different notification.
407
408 >>> bug_delta = BugDelta(
409 ... bug=edited_bug,
410 ... bugurl="http://www.example.com/bugs/2",
411 ... user=sample_person,
412 ... attachment={'old' : attachment, 'new': None})
413 >>> for change in get_bug_changes(bug_delta):
414 ... notification = change.getBugNotification()
415 ... print notification['text'] #doctest: -NORMALIZE_WHITESPACE
416 ... print "-----------------------------"
417 ** Patch removed: "A new icon for the application"
418 http://foo.com/new-icon.png
419 -----------------------------
420
421
385= Generation of From: and Reply-To: addresses =422= Generation of From: and Reply-To: addresses =
386423
387The Reply-To: and From: addresses used to send email are generated in a424The Reply-To: and From: addresses used to send email are generated in a
388425
=== modified file 'lib/lp/bugs/interfaces/bugattachment.py'
--- lib/lp/bugs/interfaces/bugattachment.py 2009-11-26 10:35:21 +0000
+++ lib/lp/bugs/interfaces/bugattachment.py 2010-01-22 17:25:27 +0000
@@ -82,6 +82,10 @@
82 message = exported(82 message = exported(
83 Reference(IMessage, title=_("The message that was created when we "83 Reference(IMessage, title=_("The message that was created when we "
84 "added this attachment.")))84 "added this attachment.")))
85 is_patch = Bool(
86 title=_('Patch?'),
87 description=_('Is this attachment a patch?'),
88 readonly=True)
8589
86 @call_with(user=REQUEST_USER)90 @call_with(user=REQUEST_USER)
87 @export_write_operation()91 @export_write_operation()
@@ -94,7 +98,6 @@
94 The library file content for this attachment is set to None.98 The library file content for this attachment is set to None.
95 """99 """
96100
97
98# Need to do this here because of circular imports.101# Need to do this here because of circular imports.
99IMessage['bugattachments'].value_type.schema = IBugAttachment102IMessage['bugattachments'].value_type.schema = IBugAttachment
100103
101104
=== modified file 'lib/lp/bugs/model/bugattachment.py'
--- lib/lp/bugs/model/bugattachment.py 2009-11-26 10:35:21 +0000
+++ lib/lp/bugs/model/bugattachment.py 2010-01-22 17:25:27 +0000
@@ -41,6 +41,11 @@
41 message = ForeignKey(41 message = ForeignKey(
42 foreignKey='Message', dbName='message', notNull=True)42 foreignKey='Message', dbName='message', notNull=True)
4343
44 @property
45 def is_patch(self):
46 """See IBugAttachment."""
47 return self.type == BugAttachmentType.PATCH
48
44 def removeFromBug(self, user):49 def removeFromBug(self, user):
45 """See IBugAttachment."""50 """See IBugAttachment."""
46 notify(ObjectDeletedEvent(self, user))51 notify(ObjectDeletedEvent(self, user))