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

[Eleanor Berger]

[Eleanor Berger (intellectronica)]

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.

[Deryck Hodge (deryck)]

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)

=== 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 @@
 
     def getBugNotification(self):
         if self.old_value is None:
-            message = '** Attachment added: "%s"\n   %s' % (
-                self.new_value.title, self.new_value.libraryfile.http_url)
+            if self.new_value.is_patch:
+                attachment_str = 'Patch'
+            else:
+                attachment_str = 'Attachment'
+            message = '** %s added: "%s"\n   %s' % (
+                attachment_str, self.new_value.title,
+                self.new_value.libraryfile.http_url)
         else:
-            message = '** Attachment removed: "%s"\n   %s' % (
-                self.old_value.title, self.old_value.libraryfile.http_url)
+            if self.old_value.is_patch:
+                attachment_str = 'Patch'
+            else:
+                attachment_str = 'Attachment'
+            message = '** %s removed: "%s"\n   %s' % (
+                attachment_str, self.old_value.title,
+                self.old_value.libraryfile.http_url)
 
         return {'text': message}
 

=== 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 @@
     >>> bugs[0].id
     1
 
+An easy way to determine whether an attachment is a patch is to read its
+`is_patch` attribute.
+
+    >>> attachment.type = BugAttachmentType.PATCH
+    >>> attachment.is_patch
+    True
+
+    >>> attachment.type = BugAttachmentType.UNSPECIFIED
+    >>> attachment.is_patch
+    False
+
 
 == Deleting attachments ==
 

=== 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 @@
     ...     def __init__(self, url):
     ...         self.http_url = url
     >>> class MockAttachment:
-    ...     def __init__(self, title, libraryfile):
+    ...     def __init__(self, title, libraryfile, is_patch=False):
     ...         self.title = title
     ...         self.libraryfile = libraryfile
+    ...         self.is_patch = is_patch
     >>> attachment = MockAttachment(
     ...     title="A screenshot of the problem",
     ...     libraryfile=MockLibraryFile('http://foo.com/screenshot.png'))
@@ -382,6 +383,42 @@
        http://foo.com/screenshot.png
     -----------------------------
 
+Adding an attachment and marking it as a patch generates a different
+notification.
+
+    >>> attachment = MockAttachment(
+    ...     title="A new icon for the application",
+    ...     libraryfile=MockLibraryFile('http://foo.com/new-icon.png'),
+    ...     is_patch=True)
+    >>> bug_delta = BugDelta(
+    ...     bug=edited_bug,
+    ...     bugurl="http://www.example.com/bugs/2",
+    ...     user=sample_person,
+    ...     attachment={'new' : attachment, 'old': None})
+    >>> for change in get_bug_changes(bug_delta):
+    ...     notification = change.getBugNotification()
+    ...     print notification['text'] #doctest: -NORMALIZE_WHITESPACE
+    ...     print "-----------------------------"
+    ** Patch added: "A new icon for the application"
+       http://foo.com/new-icon.png
+    -----------------------------
+
+Removing a patch also generates a different notification.
+
+    >>> bug_delta = BugDelta(
+    ...     bug=edited_bug,
+    ...     bugurl="http://www.example.com/bugs/2",
+    ...     user=sample_person,
+    ...     attachment={'old' : attachment, 'new': None})
+    >>> for change in get_bug_changes(bug_delta):
+    ...     notification = change.getBugNotification()
+    ...     print notification['text'] #doctest: -NORMALIZE_WHITESPACE
+    ...     print "-----------------------------"
+    ** Patch removed: "A new icon for the application"
+       http://foo.com/new-icon.png
+    -----------------------------
+
+
 = Generation of From: and Reply-To: addresses =
 
 The Reply-To: and From: addresses used to send email are generated in a

=== 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 @@
     message = exported(
         Reference(IMessage, title=_("The message that was created when we "
                                     "added this attachment.")))
+    is_patch = Bool(
+        title=_('Patch?'),
+        description=_('Is this attachment a patch?'),
+        readonly=True)
 
     @call_with(user=REQUEST_USER)
     @export_write_operation()
@@ -94,7 +98,6 @@
         The library file content for this attachment is set to None.
         """
 
-
 # Need to do this here because of circular imports.
 IMessage['bugattachments'].value_type.schema = IBugAttachment
 

=== 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 @@
     message = ForeignKey(
         foreignKey='Message', dbName='message', notNull=True)
 
+    @property
+    def is_patch(self):
+        """See IBugAttachment."""
+        return self.type == BugAttachmentType.PATCH
+
     def removeFromBug(self, user):
         """See IBugAttachment."""
         notify(ObjectDeletedEvent(self, user))