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
1=== modified file 'lib/lp/bugs/adapters/bugchange.py'
2--- lib/lp/bugs/adapters/bugchange.py 2009-07-18 00:05:49 +0000
3+++ lib/lp/bugs/adapters/bugchange.py 2010-01-22 17:25:27 +0000
4@@ -523,11 +523,21 @@
5
6 def getBugNotification(self):
7 if self.old_value is None:
8- message = '** Attachment added: "%s"\n %s' % (
9- self.new_value.title, self.new_value.libraryfile.http_url)
10+ if self.new_value.is_patch:
11+ attachment_str = 'Patch'
12+ else:
13+ attachment_str = 'Attachment'
14+ message = '** %s added: "%s"\n %s' % (
15+ attachment_str, self.new_value.title,
16+ self.new_value.libraryfile.http_url)
17 else:
18- message = '** Attachment removed: "%s"\n %s' % (
19- self.old_value.title, self.old_value.libraryfile.http_url)
20+ if self.old_value.is_patch:
21+ attachment_str = 'Patch'
22+ else:
23+ attachment_str = 'Attachment'
24+ message = '** %s removed: "%s"\n %s' % (
25+ attachment_str, self.old_value.title,
26+ self.old_value.libraryfile.http_url)
27
28 return {'text': message}
29
30
31=== modified file 'lib/lp/bugs/doc/bugattachments.txt'
32--- lib/lp/bugs/doc/bugattachments.txt 2009-12-09 11:14:16 +0000
33+++ lib/lp/bugs/doc/bugattachments.txt 2010-01-22 17:25:27 +0000
34@@ -427,6 +427,17 @@
35 >>> bugs[0].id
36 1
37
38+An easy way to determine whether an attachment is a patch is to read its
39+`is_patch` attribute.
40+
41+ >>> attachment.type = BugAttachmentType.PATCH
42+ >>> attachment.is_patch
43+ True
44+
45+ >>> attachment.type = BugAttachmentType.UNSPECIFIED
46+ >>> attachment.is_patch
47+ False
48+
49
50 == Deleting attachments ==
51
52
53=== modified file 'lib/lp/bugs/doc/bugnotification-email.txt'
54--- lib/lp/bugs/doc/bugnotification-email.txt 2009-07-22 09:12:01 +0000
55+++ lib/lp/bugs/doc/bugnotification-email.txt 2010-01-22 17:25:27 +0000
56@@ -347,9 +347,10 @@
57 ... def __init__(self, url):
58 ... self.http_url = url
59 >>> class MockAttachment:
60- ... def __init__(self, title, libraryfile):
61+ ... def __init__(self, title, libraryfile, is_patch=False):
62 ... self.title = title
63 ... self.libraryfile = libraryfile
64+ ... self.is_patch = is_patch
65 >>> attachment = MockAttachment(
66 ... title="A screenshot of the problem",
67 ... libraryfile=MockLibraryFile('http://foo.com/screenshot.png'))
68@@ -382,6 +383,42 @@
69 http://foo.com/screenshot.png
70 -----------------------------
71
72+Adding an attachment and marking it as a patch generates a different
73+notification.
74+
75+ >>> attachment = MockAttachment(
76+ ... title="A new icon for the application",
77+ ... libraryfile=MockLibraryFile('http://foo.com/new-icon.png'),
78+ ... is_patch=True)
79+ >>> bug_delta = BugDelta(
80+ ... bug=edited_bug,
81+ ... bugurl="http://www.example.com/bugs/2",
82+ ... user=sample_person,
83+ ... attachment={'new' : attachment, 'old': None})
84+ >>> for change in get_bug_changes(bug_delta):
85+ ... notification = change.getBugNotification()
86+ ... print notification['text'] #doctest: -NORMALIZE_WHITESPACE
87+ ... print "-----------------------------"
88+ ** Patch added: "A new icon for the application"
89+ http://foo.com/new-icon.png
90+ -----------------------------
91+
92+Removing a patch also generates a different notification.
93+
94+ >>> bug_delta = BugDelta(
95+ ... bug=edited_bug,
96+ ... bugurl="http://www.example.com/bugs/2",
97+ ... user=sample_person,
98+ ... attachment={'old' : attachment, 'new': None})
99+ >>> for change in get_bug_changes(bug_delta):
100+ ... notification = change.getBugNotification()
101+ ... print notification['text'] #doctest: -NORMALIZE_WHITESPACE
102+ ... print "-----------------------------"
103+ ** Patch removed: "A new icon for the application"
104+ http://foo.com/new-icon.png
105+ -----------------------------
106+
107+
108 = Generation of From: and Reply-To: addresses =
109
110 The Reply-To: and From: addresses used to send email are generated in a
111
112=== modified file 'lib/lp/bugs/interfaces/bugattachment.py'
113--- lib/lp/bugs/interfaces/bugattachment.py 2009-11-26 10:35:21 +0000
114+++ lib/lp/bugs/interfaces/bugattachment.py 2010-01-22 17:25:27 +0000
115@@ -82,6 +82,10 @@
116 message = exported(
117 Reference(IMessage, title=_("The message that was created when we "
118 "added this attachment.")))
119+ is_patch = Bool(
120+ title=_('Patch?'),
121+ description=_('Is this attachment a patch?'),
122+ readonly=True)
123
124 @call_with(user=REQUEST_USER)
125 @export_write_operation()
126@@ -94,7 +98,6 @@
127 The library file content for this attachment is set to None.
128 """
129
130-
131 # Need to do this here because of circular imports.
132 IMessage['bugattachments'].value_type.schema = IBugAttachment
133
134
135=== modified file 'lib/lp/bugs/model/bugattachment.py'
136--- lib/lp/bugs/model/bugattachment.py 2009-11-26 10:35:21 +0000
137+++ lib/lp/bugs/model/bugattachment.py 2010-01-22 17:25:27 +0000
138@@ -41,6 +41,11 @@
139 message = ForeignKey(
140 foreignKey='Message', dbName='message', notNull=True)
141
142+ @property
143+ def is_patch(self):
144+ """See IBugAttachment."""
145+ return self.type == BugAttachmentType.PATCH
146+
147 def removeFromBug(self, user):
148 """See IBugAttachment."""
149 notify(ObjectDeletedEvent(self, user))