Merge lp:~sinzui/launchpad/empty-attchments-message into lp:launchpad

Proposed by Curtis Hovey
Status: Merged
Approved by: j.c.sackett
Approved revision: no longer in the source branch.
Merged at revision: 16091
Proposed branch: lp:~sinzui/launchpad/empty-attchments-message
Merge into: lp:launchpad
Diff against target: 920 lines (+245/-548)
5 files modified
lib/lp/bugs/browser/bugtarget.py (+2/-1)
lib/lp/bugs/browser/tests/bugtarget-filebug-views.txt (+0/-540)
lib/lp/bugs/browser/tests/test_bugtarget_filebug.py (+221/-5)
lib/lp/bugs/model/bug.py (+3/-2)
lib/lp/bugs/model/tests/test_bug.py (+19/-0)
To merge this branch: bzr merge lp:~sinzui/launchpad/empty-attchments-message
Reviewer Review Type Date Requested Status
j.c.sackett (community) Approve
Review via email: mp+127869@code.launchpad.net

Commit message

Do not send empty messages when attachments are reported with bugs.

Description of the change

Users get empty emails when extra_data has attachments. The attachment
notifications are suppressed because they are commonly added by apport
when reporting the bug. But an extra comment is still being created for
the attachments, and since it has no text, just the empty message is
queued

--------------------------------------------------------------------

RULES

    Pre-implementation: wgrant
    * The attachment notifications are suppressed because they cause
      timeouts and are rarely interesting when sent via mail.
    * We do not want queue an email, but creating an empty bug comment
      for the extra attachments always queues a message because notify
      is called by Bug.newMessage().
      * Update Bug.newMessage() to accept a send_notifications=True
        arg that submit_bug_action() can pass False to. Only
        call notify if send_notifications is True
      * Update FileBugViewBase.submit_bug_action() to create the
        attachment_comment message using send_notifications=False.

QA

    * Visit https://qastaging.launchpad.net/~/+edit
      and make sure you receive notifications for bugs you change.
    * Visit https://bugs.qastaging.launchpad.net/gdp/+filebug
      ans report a bug with an attachment.
    * Verify that the first comment shows the attachment.
    * Check the staging inbox and verify that there is only a message
      about the new bug. There is not another message with no content.

LoC:

    This branch replaces a several doctest parts with a new test case
    to reduce lines.

LINT

    lib/lp/bugs/browser/bugtarget.py
    lib/lp/bugs/browser/tests/bugtarget-filebug-views.txt
    lib/lp/bugs/browser/tests/test_bugtarget_filebug.py
    lib/lp/bugs/model/bug.py
    lib/lp/bugs/model/tests/test_bug.py

TEST

    ./bin/test -vvc -t newMessage lp.bugs.model.tests.test_bug
    ./bin/test -vvc -t ExtraData lp.bugs.browser.tests.test_bugtarget_filebug
    ./bin/test -vvc -t filebug-views.txt lp.bugs.browser.tests

IMPLEMENTATION

I added send_notifications=True to newMessage(), then only call notify
for the new message if send_notifications is True.
    lib/lp/bugs/model/bug.py
    lib/lp/bugs/model/tests/test_bug.py

I replaced many parts of a doctest with unit tests. I needed a helper to
process the blob/extra_data that FileBugViewBase uses in the most common
case of reporting a bug with attachments. I decided to convert the
old doctest instead of just copying the technique.
    lib/lp/bugs/browser/tests/bugtarget-filebug-views.txt
    lib/lp/bugs/browser/tests/test_bugtarget_filebug.py

I updated FileBugViewBase to pass send_notifications=False for the
attachment comment. I updated the attachment test to use the event
recorder to show that there is no event for the new message.
    lib/lp/bugs/browser/bugtarget.py
    lib/lp/bugs/browser/tests/test_bugtarget_filebug.py

To post a comment you must log in.
Revision history for this message
j.c.sackett (jcsackett) wrote :

This looks good to me. As a minor point, you can probably drop several places where you're asserting the length of an array, as you then assert the contents equal a particular list. The length assertion is implicit in comparison of contents.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/lp/bugs/browser/bugtarget.py'
2--- lib/lp/bugs/browser/bugtarget.py 2012-09-21 00:28:49 +0000
3+++ lib/lp/bugs/browser/bugtarget.py 2012-10-04 15:15:46 +0000
4@@ -579,7 +579,8 @@
5 if attachment or extra_data.attachments:
6 # Attach all the comments to a single empty comment.
7 attachment_comment = bug.newMessage(
8- owner=self.user, subject=bug.followup_subject(), content=None)
9+ owner=self.user, subject=bug.followup_subject(), content=None,
10+ send_notifications=False)
11
12 # Deal with attachments added in the filebug form.
13 if attachment:
14
15=== modified file 'lib/lp/bugs/browser/tests/bugtarget-filebug-views.txt'
16--- lib/lp/bugs/browser/tests/bugtarget-filebug-views.txt 2012-08-16 05:18:54 +0000
17+++ lib/lp/bugs/browser/tests/bugtarget-filebug-views.txt 2012-10-04 15:15:46 +0000
18@@ -80,546 +80,6 @@
19 http://launchpad.dev/firefox/+filebug-show-similar
20
21
22-Adding extra info to filed bugs
23--------------------------------
24-
25-It's possible for bug reporting tools to upload a file with debug
26-information to Launchpad, and pass that information to the filebug page.
27-When the data is uploaded a token is returned, which is appended to the
28-+filebug URL, resulting in a URL like '/.../+filebug/12345abcde'. The
29-+filebug view's publishTraverse method looks up the correct data from
30-using the token.
31-
32-The uploaded debug information should be MIME multipart message, where
33-the Content-Disposition header tells Launchpad what to do with the
34-different parts.
35-
36-
37-First inline part
38-.................
39-
40-The first inline part will be appended to the bug description.
41-
42- >>> debug_data = """MIME-Version: 1.0
43- ... Content-type: multipart/mixed; boundary=boundary
44- ...
45- ... --boundary
46- ... Content-disposition: inline
47- ... Content-type: text/plain; charset=utf-8
48- ...
49- ... This should be added to the description.
50- ...
51- ... --boundary--
52- ... """
53- >>> import transaction
54- >>> from lp.services.temporaryblobstorage.interfaces import (
55- ... ITemporaryStorageManager)
56- >>> token = getUtility(ITemporaryStorageManager).new(debug_data)
57-
58-We need to commit the transaction since the data will be stored in the
59-Librarian.
60-
61- >>> transaction.commit()
62-
63-The data is processed by a ProcessApportBlobJob, which the filebug view
64-can then access to get the parsed data. We'll define a helper method for
65-processing the blob.
66-
67- >>> from zope.component import getUtility
68- >>> from lp.bugs.interfaces.apportjob import IProcessApportBlobJobSource
69-
70- >>> def process_blob(token):
71- ... blob = getUtility(ITemporaryStorageManager).fetch(token)
72- ... job = getUtility(IProcessApportBlobJobSource).create(blob)
73- ... job.job.start()
74- ... job.run()
75- ... job.job.complete()
76- >>> process_blob(token)
77-
78-Now, if we pass the token to the filebug view, the extra_data attribute
79-will be set with the actual data.
80-
81- >>> filebug_view = create_initialized_view(ubuntu_firefox, '+filebug')
82- >>> filebug_view.publishTraverse(
83- ... filebug_view.request, token) is filebug_view
84- True
85-
86- >>> filebug_view.extra_data.extra_description
87- u'This should be added to the description.'
88-
89- >>> filebug_view.validate(bug_data) is None
90- True
91-
92- >>> filebug_view.submit_bug_action.success(bug_data)
93- >>> filebug_view.added_bug.title
94- u'Test Title'
95-
96- >>> description = filebug_view.added_bug.description
97- >>> print description #doctest: -NORMALIZE_WHITESPACE
98- Test description.
99- <BLANKLINE>
100- This should be added to the description.
101-
102-A notification was added to inform the user about what happened.
103-
104- >>> for notification in filebug_view.request.response.notifications:
105- ... print notification.message
106- <p class="last">Thank you for your bug report.</p>
107- Additional information was added to the bug description.
108-
109-
110-Other inline parts
111-..................
112-
113-If there are more than one inline part, those will be added as comments
114-to the bug.
115-
116- >>> debug_data = """MIME-Version: 1.0
117- ... Content-type: multipart/mixed; boundary=boundary
118- ...
119- ... --boundary
120- ... Content-disposition: inline
121- ... Content-type: text/plain; charset=utf-8
122- ...
123- ... This should be added to the description.
124- ...
125- ... --boundary
126- ... Content-disposition: inline
127- ... Content-type: text/plain; charset=utf-8
128- ...
129- ... This should be added as a comment.
130- ...
131- ... --boundary
132- ... Content-disposition: inline
133- ... Content-type: text/plain; charset=utf-8
134- ...
135- ... This should be added as another comment.
136- ...
137- ... --boundary--
138- ... """
139- >>> token = getUtility(ITemporaryStorageManager).new(debug_data)
140- >>> transaction.commit()
141-
142- >>> process_blob(token)
143- >>> filebug_view = create_initialized_view(ubuntu_firefox, '+filebug')
144- >>> filebug_view.publishTraverse(filebug_view.request,
145- ... token) is filebug_view
146- True
147-
148- >>> filebug_view.validate(bug_data) is None
149- True
150-
151- >>> filebug_view.submit_bug_action.success(bug_data)
152-
153- >>> filebug_view.added_bug.title
154- u'Test Title'
155-
156- >>> description = filebug_view.added_bug.description
157- >>> print description #doctest: -NORMALIZE_WHITESPACE
158- Test description.
159- <BLANKLINE>
160- This should be added to the description.
161-
162- >>> for comment in filebug_view.added_bug.messages[1:]:
163- ... print "Comment by %s: %s" % (
164- ... comment.owner.displayname, comment.text_contents)
165- Comment by No Privileges Person: This should be added as a comment.
166- Comment by No Privileges Person: This should be added as another comment.
167-
168-Notifications were added to inform the user about what happened.
169-
170- >>> for notification in filebug_view.request.response.notifications:
171- ... print notification.message
172- <p class="last">Thank you for your bug report.</p>
173- Additional information was added to the bug description.
174- A comment with additional information was added to the bug report.
175- A comment with additional information was added to the bug report.
176-
177-
178-Attachments
179-...........
180-
181-All the parts that have a 'Content-disposition: attachment' header will
182-get added as attachments to the bug. The attachment description can be
183-specified using a Content-description header, but it's not required.
184-
185- >>> debug_data = """MIME-Version: 1.0
186- ... Content-type: multipart/mixed; boundary=boundary
187- ...
188- ... --boundary
189- ... Content-disposition: attachment; filename='attachment1'
190- ... Content-type: text/plain; charset=utf-8
191- ...
192- ... This is an attachment.
193- ...
194- ... --boundary
195- ... Content-disposition: attachment; filename='attachment2'
196- ... Content-description: Attachment description.
197- ... Content-type: text/plain; charset=ISO-8859-1
198- ...
199- ... This is another attachment, with a description.
200- ...
201- ... --boundary--
202- ... """
203- >>> token = getUtility(ITemporaryStorageManager).new(debug_data)
204- >>> transaction.commit()
205-
206- >>> process_blob(token)
207- >>> filebug_view = create_initialized_view(ubuntu_firefox, '+filebug')
208- >>> filebug_view.publishTraverse(filebug_view.request,
209- ... token) is filebug_view
210- True
211-
212- >>> filebug_view.validate(bug_data) is None
213- True
214-
215- >>> filebug_view.submit_bug_action.success(bug_data)
216-
217-Since the attachments are stored in the Librarian, we need to commit the
218-transaction in order to access them.
219-
220- >>> transaction.commit()
221-
222- >>> filebug_view.added_bug.title
223- u'Test Title'
224-
225- >>> print filebug_view.added_bug.description
226- Test description.
227-
228-The attachments got added, with the charsets preserved, and the one that
229-didn't specify a description got an autogenerated one.
230-
231- >>> for attachment in filebug_view.added_bug.attachments_unpopulated:
232- ... print "Filename: %s" % attachment.libraryfile.filename
233- ... print "Content type: %s" % attachment.libraryfile.mimetype
234- ... print "Description: %s" % attachment.title
235- ... print "Contents:\n%s" % attachment.libraryfile.read()
236- ... print
237- Filename: attachment1
238- Content type: text/plain; charset=utf-8
239- Description: attachment1
240- Contents:
241- This is an attachment.
242- <BLANKLINE>
243- Filename: attachment2
244- Content type: text/plain; charset=ISO-8859-1
245- Description: Attachment description.
246- Contents:
247- This is another attachment, with a description.
248- <BLANKLINE>
249-
250-Notifications were added to inform the user about what happened.
251-
252- >>> for notification in filebug_view.request.response.notifications:
253- ... print notification.message
254- <p class="last">Thank you for your bug report.</p>
255- The file "attachment1" was attached to the bug report.
256- The file "attachment2" was attached to the bug report.
257-
258-The attachments are all added to the same comment.
259-
260- >>> for comment in filebug_view.added_bug.messages[1:]:
261- ... print "Comment by %s: %s attachment(s)" % (
262- ... comment.owner.displayname, len(comment.bugattachments))
263- Comment by No Privileges Person: 2 attachment(s)
264-
265-
266-Private Bugs
267-............
268-
269-We can specify whether a bug is private by providing Private field in
270-the message.
271-
272- >>> debug_data = """MIME-Version: 1.0
273- ... Content-type: multipart/mixed; boundary=boundary
274- ... Private: yes
275- ...
276- ... --boundary
277- ... Content-disposition: inline
278- ... Content-type: text/plain; charset=utf-8
279- ...
280- ... This bug should be private.
281- ...
282- ... --boundary--
283- ... """
284- >>> token = getUtility(ITemporaryStorageManager).new(debug_data)
285- >>> transaction.commit()
286-
287- >>> process_blob(token)
288- >>> filebug_view = create_initialized_view(ubuntu_firefox, '+filebug')
289- >>> filebug_view.publishTraverse(filebug_view.request,
290- ... token) is filebug_view
291- True
292-
293- >>> filebug_view.extra_data.extra_description
294- u'This bug should be private.'
295-
296- >>> filebug_view.extra_data.private
297- True
298-
299- >>> filebug_view.validate(bug_data) is None
300- True
301-
302- >>> filebug_view.submit_bug_action.success(bug_data)
303- >>> filebug_view.added_bug.title
304- u'Test Title'
305-
306- >>> description = filebug_view.added_bug.description
307- >>> print description #doctest: -NORMALIZE_WHITESPACE
308- Test description.
309- <BLANKLINE>
310- This bug should be private.
311-
312- >>> filebug_view.added_bug.information_type
313- <DBItem InformationType.USERDATA, (4) Private>
314-
315-The user will be notified that the bug has been marked as private.
316-
317- >>> print filebug_view.request.response.notifications[2].message
318- This bug report has been marked private...
319-
320-We can also specify that a bug is public via the same field.
321-
322- >>> debug_data = """MIME-Version: 1.0
323- ... Content-type: multipart/mixed; boundary=boundary
324- ... Private: no
325- ...
326- ... --boundary
327- ... Content-disposition: inline
328- ... Content-type: text/plain; charset=utf-8
329- ...
330- ... This bug should be public.
331- ...
332- ... --boundary--
333- ... """
334- >>> token = getUtility(ITemporaryStorageManager).new(debug_data)
335- >>> transaction.commit()
336-
337- >>> process_blob(token)
338- >>> filebug_view = create_initialized_view(ubuntu_firefox, '+filebug')
339- >>> filebug_view.publishTraverse(filebug_view.request,
340- ... token) is filebug_view
341- True
342-
343- >>> filebug_view.extra_data.extra_description
344- u'This bug should be public.'
345-
346- >>> filebug_view.extra_data.private
347- False
348-
349- >>> filebug_view.validate(bug_data) is None
350- True
351-
352- >>> filebug_view.submit_bug_action.success(bug_data)
353- >>> filebug_view.added_bug.title
354- u'Test Title'
355-
356- >>> description = filebug_view.added_bug.description
357- >>> print description #doctest: -NORMALIZE_WHITESPACE
358- Test description.
359- <BLANKLINE>
360- This bug should be public.
361-
362- >>> filebug_view.added_bug.private
363- False
364-
365-Since this bug is public, both the reporter and the bug supervisor have
366-been subscribed.
367-
368- >>> for subscriber in filebug_view.added_bug.getDirectSubscribers():
369- ... print subscriber.displayname
370- No Privileges Person
371-
372- >>> for subscriber in filebug_view.added_bug.getIndirectSubscribers():
373- ... print subscriber.displayname
374- Foo Bar
375-
376-
377-Subscriptions
378-.............
379-
380-We can also subscribe someone to this bug when we file it by using a
381-Subscribe field in the message. Multiple people can be specified and
382-they can be identified by their Launchpad name or their e-mail address.
383-
384- >>> debug_data = """MIME-Version: 1.0
385- ... Content-type: multipart/mixed; boundary=boundary
386- ... Subscribers: ddaa test@canonical.com
387- ...
388- ... --boundary
389- ... Content-disposition: inline
390- ... Content-type: text/plain; charset=utf-8
391- ...
392- ... Other people are interested in this bug.
393- ...
394- ... --boundary--
395- ... """
396- >>> token = getUtility(ITemporaryStorageManager).new(debug_data)
397- >>> transaction.commit()
398-
399- >>> process_blob(token)
400- >>> filebug_view = create_initialized_view(ubuntu_firefox, '+filebug')
401- >>> filebug_view.publishTraverse(filebug_view.request,
402- ... token) is filebug_view
403- True
404-
405- >>> filebug_view.extra_data.extra_description
406- u'Other people are interested in this bug.'
407-
408- >>> for subscriber in filebug_view.extra_data.subscribers:
409- ... print subscriber
410- ddaa
411- test@canonical.com
412-
413- >>> filebug_view.validate(bug_data) is None
414- True
415-
416- >>> filebug_view.submit_bug_action.success(bug_data)
417- >>> filebug_view.added_bug.title
418- u'Test Title'
419-
420- >>> description = filebug_view.added_bug.description
421- >>> print description #doctest: -NORMALIZE_WHITESPACE
422- Test description.
423- <BLANKLINE>
424- Other people are interested in this bug.
425-
426-As well as the reporter, both Sample Person and David Allouche have been
427-subscribed to the bug.
428-
429- >>> for subscriber in filebug_view.added_bug.getDirectSubscribers():
430- ... print subscriber.displayname
431- David Allouche
432- No Privileges Person
433- Sample Person
434-
435-The user will be notified that Sample Person and David Allouche has been
436-subscribed to this bug.
437-
438- >>> for notification in filebug_view.request.response.notifications:
439- ... print notification.message
440- <p class="last">Thank you for your bug report.</p>
441- Additional information was added to the bug description.
442- David Allouche has been subscribed to this bug.
443- Sample Person has been subscribed to this bug.
444-
445-
446-Subscribers to Private bugs
447-...........................
448-
449-The Private and Subscriber fields are intended to be used together to
450-subscribe certain people and teams to bugs when they are filed.
451-
452- >>> debug_data = """MIME-Version: 1.0
453- ... Content-type: multipart/mixed; boundary=boundary
454- ... Private: yes
455- ... Subscribers: mark
456- ...
457- ... --boundary
458- ... Content-disposition: inline
459- ... Content-type: text/plain; charset=utf-8
460- ...
461- ... This bug should be private, and Mark Shuttleworth subscribed.
462- ...
463- ... --boundary--
464- ... """
465- >>> token = getUtility(ITemporaryStorageManager).new(debug_data)
466- >>> transaction.commit()
467-
468- >>> process_blob(token)
469- >>> filebug_view = create_initialized_view(ubuntu_firefox, '+filebug')
470- >>> filebug_view.publishTraverse(filebug_view.request,
471- ... token) is filebug_view
472- True
473-
474- >>> filebug_view.extra_data.extra_description
475- u'This bug should be private, and Mark Shuttleworth subscribed.'
476-
477- >>> filebug_view.extra_data.private
478- True
479-
480- >>> for subscriber in filebug_view.extra_data.subscribers:
481- ... print subscriber
482- mark
483-
484- >>> filebug_view.validate(bug_data) is None
485- True
486-
487- >>> filebug_view.submit_bug_action.success(bug_data)
488- >>> filebug_view.added_bug.title
489- u'Test Title'
490-
491- >>> description = filebug_view.added_bug.description
492- >>> print description #doctest: -NORMALIZE_WHITESPACE
493- Test description.
494- <BLANKLINE>
495- This bug should be private, and Mark Shuttleworth subscribed.
496-
497- >>> filebug_view.added_bug.private
498- True
499-
500- >>> filebug_view.added_bug.security_related
501- False
502-
503-As well as the reporter, Mark Shuttleworth should have been subscribed
504-to the bug.
505-
506- >>> for subscriber in filebug_view.added_bug.getDirectSubscribers():
507- ... print subscriber.displayname
508- Mark Shuttleworth
509- No Privileges Person
510-
511-The user will be notified that Mark Shuttleworth has been subscribed to
512-this bug and that the bug has been marked as private.
513-
514- >>> for notification in filebug_view.request.response.notifications:
515- ... print notification.message
516- <p class="last">Thank you for your bug report.</p>
517- Additional information was added to the bug description.
518- Mark Shuttleworth has been subscribed to this bug.
519- This bug report has been marked private...
520-
521-
522-publishTraverse()
523------------------
524-
525-As already seen above, it's the FileBugViewBase's publishTraverse that
526-finds the right blob to use.
527-
528- >>> filebug_view = create_initialized_view(ubuntu_firefox, '+filebug')
529- >>> filebug_view.publishTraverse(filebug_view.request,
530- ... token) is filebug_view
531- True
532-
533- >>> filebug_view.extra_data_token == token
534- True
535-
536- >>> filebug_view.extra_data is not None
537- True
538-
539-Since the view itself is returned, it will handle further traversals as
540-well, so if we call the method again, it represents a URL like
541-'.../+filebug/token/foo', which should raise a NotFound error.
542-
543- >>> filebug_view.publishTraverse(filebug_view.request, token)
544- Traceback (most recent call last):
545- ...
546- NotFound:...
547-
548-
549-Not found tokens
550-................
551-
552-If publishTraverse is called with a token that can't be found, a
553-NotFound error is raised.
554-
555- >>> filebug_view = create_initialized_view(ubuntu_firefox, '+filebug')
556- >>> filebug_view.publishTraverse(filebug_view.request, 'no-such-token')
557- Traceback (most recent call last):
558- ...
559- NotFound:...
560-
561-
562 Adding tags to filed bugs
563 -------------------------
564
565
566=== modified file 'lib/lp/bugs/browser/tests/test_bugtarget_filebug.py'
567--- lib/lp/bugs/browser/tests/test_bugtarget_filebug.py 2012-09-21 00:28:49 +0000
568+++ lib/lp/bugs/browser/tests/test_bugtarget_filebug.py 2012-10-04 15:15:46 +0000
569@@ -4,10 +4,12 @@
570 __metaclass__ = type
571
572
573+from textwrap import dedent
574 from BeautifulSoup import BeautifulSoup
575 from lazr.restful.interfaces import IJSONRequestCache
576 import transaction
577 from zope.component import getUtility
578+from zope.publisher.interfaces import NotFound
579 from zope.schema.interfaces import (
580 TooLong,
581 TooShort,
582@@ -19,6 +21,7 @@
583 PUBLIC_INFORMATION_TYPES,
584 )
585 from lp.bugs.browser.bugtarget import FileBugViewBase
586+from lp.bugs.interfaces.apportjob import IProcessApportBlobJobSource
587 from lp.bugs.interfaces.bug import (
588 IBugAddForm,
589 IBugSet,
590@@ -30,14 +33,20 @@
591 from lp.bugs.publisher import BugsLayer
592 from lp.registry.enums import BugSharingPolicy
593 from lp.registry.interfaces.projectgroup import IProjectGroup
594+from lp.services.temporaryblobstorage.interfaces import (
595+ ITemporaryStorageManager)
596 from lp.services.webapp.servers import LaunchpadTestRequest
597 from lp.testing import (
598+ EventRecorder,
599 login,
600 login_person,
601 person_logged_in,
602 TestCaseWithFactory,
603 )
604-from lp.testing.layers import DatabaseFunctionalLayer
605+from lp.testing.layers import (
606+ DatabaseFunctionalLayer,
607+ LaunchpadFunctionalLayer,
608+ )
609 from lp.testing.pages import (
610 find_main_content,
611 find_tag_by_id,
612@@ -275,9 +284,8 @@
613 self.assertIs(None, find_tag_by_id(html, 'filebug-search-form'))
614
615
616-class TestFileBugViewBase(TestCaseWithFactory):
617-
618- layer = DatabaseFunctionalLayer
619+class FileBugViewMixin:
620+ """Provide a FileBugView subclass that is easy to test."""
621
622 class FileBugTestView(FileBugViewBase):
623 """A simple subclass."""
624@@ -288,7 +296,7 @@
625 pass
626
627 def setUp(self):
628- super(TestFileBugViewBase, self).setUp()
629+ super(FileBugViewMixin, self).setUp()
630 self.target = self.factory.makeProduct()
631 transaction.commit()
632 login_person(self.target.owner)
633@@ -308,6 +316,11 @@
634 view.initialize()
635 return view
636
637+
638+class TestFileBugViewBase(FileBugViewMixin, TestCaseWithFactory):
639+
640+ layer = DatabaseFunctionalLayer
641+
642 def test_submit_comment_empty_error(self):
643 # The comment cannot be an empty string.
644 form = self.get_form(comment='')
645@@ -491,6 +504,209 @@
646 self.assertIn("This can be fixed by changing", html)
647
648
649+class FileBugViewBaseExtraDataTestCase(FileBugViewMixin, TestCaseWithFactory):
650+ """FileBugView handles extra data from blobs created by apport."""
651+
652+ layer = LaunchpadFunctionalLayer
653+
654+ @staticmethod
655+ def get_form():
656+ return {
657+ 'title': 'test title',
658+ 'comment': 'test description',
659+ }
660+
661+ @staticmethod
662+ def process_extra_data(raw_data=None, command=''):
663+ if raw_data is None:
664+ raw_data = """\
665+ MIME-Version: 1.0
666+ Content-type: multipart/mixed; boundary=boundary
667+ %s
668+
669+ --boundary
670+ Content-disposition: inline
671+ Content-type: text/plain; charset=utf-8
672+
673+ Added to the description.
674+
675+ --boundary--
676+ """ % command
677+ extra_data = dedent(raw_data)
678+ temp_storage_manager = getUtility(ITemporaryStorageManager)
679+ token = temp_storage_manager.new(extra_data)
680+ transaction.commit()
681+ blob = temp_storage_manager.fetch(token)
682+ job = getUtility(IProcessApportBlobJobSource).create(blob)
683+ job.job.start()
684+ job.run()
685+ job.job.complete()
686+ return token
687+
688+ def test_publish_traverse_token(self):
689+ # The publish_traverse() method uses a token to lookup the extra_data.
690+ token = self.process_extra_data(command="not-requred: ignore")
691+ view = self.create_initialized_view()
692+ self.assertIs(view, view.publishTraverse(view.request, token))
693+
694+ def test_publish_traverse_token_error(self):
695+ # The publish_traverse() method uses a token to lookup the extra_data.
696+ view = self.create_initialized_view()
697+ self.assertRaises(
698+ NotFound, view.publishTraverse, view.request, 'no-such-token')
699+
700+ def test_description_and_comments(self):
701+ # The first extra text part is added to the desciption, all other
702+ # extra parts become additional bug messages.
703+ token = self.process_extra_data("""\
704+ MIME-Version: 1.0
705+ Content-type: multipart/mixed; boundary=boundary
706+
707+ --boundary
708+ Content-disposition: inline
709+ Content-type: text/plain; charset=utf-8
710+
711+ Added to the description.
712+
713+ --boundary
714+ Content-disposition: inline
715+ Content-type: text/plain; charset=utf-8
716+
717+ A bug comment.
718+
719+ --boundary--
720+ """)
721+ view = self.create_initialized_view()
722+ view.publishTraverse(view.request, token)
723+ self.assertEqual(
724+ 'Added to the description.', view.extra_data.extra_description)
725+ with EventRecorder() as recorder:
726+ view.submit_bug_action.success(self.get_form())
727+ # Subscribers are only notified about the new bug event;
728+ # The extra comment for the attchments was silent.
729+ self.assertEqual(2, len(recorder.events))
730+ bug_event, message_event = recorder.events
731+ transaction.commit()
732+ bug = view.added_bug
733+ self.assertEqual(bug, bug_event.object)
734+ self.assertEqual(
735+ 'test description\n\n'
736+ 'Added to the description.',
737+ bug.description)
738+ self.assertEqual(2, bug.messages.count())
739+ self.assertEqual(bug.bug_messages[-1], message_event.object)
740+ notifications = [
741+ no.message for no in view.request.response.notifications]
742+ self.assertContentEqual(
743+ ['<p class="last">Thank you for your bug report.</p>',
744+ 'Additional information was added to the bug description.',
745+ 'A comment with additional information was added to the'
746+ ' bug report.'],
747+ notifications)
748+
749+ def test_private_yes(self):
750+ # The extra data can specify the bug is private.
751+ token = self.process_extra_data(command='Private: yes')
752+ view = self.create_initialized_view()
753+ view.publishTraverse(view.request, token)
754+ view.submit_bug_action.success(self.get_form())
755+ transaction.commit()
756+ bug = view.added_bug
757+ self.assertIs(True, bug.private)
758+ self.assertEqual(InformationType.USERDATA, bug.information_type)
759+
760+ def test_private_no(self):
761+ # The extra data can specify the bug is public.
762+ token = self.process_extra_data(command='Private: no')
763+ view = self.create_initialized_view()
764+ view.publishTraverse(view.request, token)
765+ view.submit_bug_action.success(self.get_form())
766+ transaction.commit()
767+ bug = view.added_bug
768+ self.assertIs(False, bug.private)
769+ self.assertEqual(InformationType.PUBLIC, bug.information_type)
770+
771+ def test_subscribers_with_email_address(self):
772+ # The extra data can add bug subscribers via email address.
773+ subscriber_1 = self.factory.makePerson(email='me@eg.dom')
774+ subscriber_2 = self.factory.makePerson(email='him@eg.dom')
775+ token = self.process_extra_data(
776+ command='Subscribers: me@eg.dom him@eg.dom')
777+ view = self.create_initialized_view()
778+ view.publishTraverse(view.request, token)
779+ self.assertContentEqual(
780+ ['me@eg.dom', 'him@eg.dom'], view.extra_data.subscribers)
781+ view.submit_bug_action.success(self.get_form())
782+ transaction.commit()
783+ bug = view.added_bug
784+ subscribers = [subscriber_1, subscriber_2, bug.owner]
785+ self.assertContentEqual(subscribers, bug.getDirectSubscribers())
786+
787+ def test_subscribers_with_name(self):
788+ # The extra data can add bug subscribers via Launchpad Id..
789+ subscriber_1 = self.factory.makePerson(name='me')
790+ subscriber_2 = self.factory.makePerson(name='him')
791+ token = self.process_extra_data(command='Subscribers: me him')
792+ view = self.create_initialized_view()
793+ view.publishTraverse(view.request, token)
794+ self.assertContentEqual(['me', 'him'], view.extra_data.subscribers)
795+ view.submit_bug_action.success(self.get_form())
796+ transaction.commit()
797+ bug = view.added_bug
798+ subscribers = [subscriber_1, subscriber_2, bug.owner]
799+ self.assertContentEqual(subscribers, bug.getDirectSubscribers())
800+
801+ def test_attachments(self):
802+ # The attachment comment has no content and it does not notify.
803+ token = self.process_extra_data("""\
804+ MIME-Version: 1.0
805+ Content-type: multipart/mixed; boundary=boundary
806+
807+ --boundary
808+ Content-disposition: attachment; filename='attachment1'
809+ Content-type: text/plain; charset=utf-8
810+
811+ This is an attachment.
812+
813+ --boundary
814+ Content-disposition: attachment; filename='attachment2'
815+ Content-description: Attachment description.
816+ Content-type: text/plain; charset=ISO-8859-1
817+
818+ This is another attachment, with a description.
819+
820+ --boundary--
821+ """)
822+ view = self.create_initialized_view()
823+ view.publishTraverse(view.request, token)
824+ with EventRecorder() as recorder:
825+ view.submit_bug_action.success(self.get_form())
826+ # Subscribers are only notified about the new bug event;
827+ # The extra comment for the attchments was silent.
828+ self.assertEqual(1, len(recorder.events))
829+ self.assertEqual(view.added_bug, recorder.events[0].object)
830+ transaction.commit()
831+ bug = view.added_bug
832+ attachments = [at for at in bug.attachments_unpopulated]
833+ self.assertEqual(2, len(attachments))
834+ attachment = attachments[0]
835+ self.assertEqual('attachment1', attachment.title)
836+ self.assertEqual('attachment1', attachment.libraryfile.filename)
837+ self.assertEqual(
838+ 'text/plain; charset=utf-8', attachment.libraryfile.mimetype)
839+ self.assertEqual(
840+ 'This is an attachment.\n\n', attachment.libraryfile.read())
841+ self.assertEqual(2, bug.messages.count())
842+ self.assertEqual(2, len(bug.messages[1].bugattachments))
843+ notifications = [
844+ no.message for no in view.request.response.notifications]
845+ self.assertContentEqual(
846+ ['<p class="last">Thank you for your bug report.</p>',
847+ 'The file "attachment1" was attached to the bug report.',
848+ 'The file "attachment2" was attached to the bug report.'],
849+ notifications)
850+
851+
852 class TestFileBugForNonBugSupervisors(TestCaseWithFactory):
853
854 layer = DatabaseFunctionalLayer
855
856=== modified file 'lib/lp/bugs/model/bug.py'
857--- lib/lp/bugs/model/bug.py 2012-10-02 19:12:26 +0000
858+++ lib/lp/bugs/model/bug.py 2012-10-04 15:15:46 +0000
859@@ -1166,7 +1166,7 @@
860
861 def newMessage(self, owner=None, subject=None,
862 content=None, parent=None, bugwatch=None,
863- remote_comment_id=None):
864+ remote_comment_id=None, send_notifications=True):
865 """Create a new Message and link it to this bug."""
866 if subject is None:
867 subject = self.followup_subject()
868@@ -1180,7 +1180,8 @@
869 if not bugmsg:
870 return
871
872- notify(ObjectCreatedEvent(bugmsg, user=owner))
873+ if send_notifications:
874+ notify(ObjectCreatedEvent(bugmsg, user=owner))
875
876 return bugmsg.message
877
878
879=== modified file 'lib/lp/bugs/model/tests/test_bug.py'
880--- lib/lp/bugs/model/tests/test_bug.py 2012-09-21 00:28:49 +0000
881+++ lib/lp/bugs/model/tests/test_bug.py 2012-10-04 15:15:46 +0000
882@@ -13,6 +13,7 @@
883 from testtools.testcase import ExpectedException
884 from zope.component import getUtility
885 from zope.security.proxy import removeSecurityProxy
886+from lazr.lifecycle.event import ObjectCreatedEvent
887
888 from lp.app.enums import InformationType
889 from lp.app.interfaces.launchpad import ILaunchpadCelebrities
890@@ -39,6 +40,7 @@
891 from lp.registry.tests.test_accesspolicy import get_policies_for_artifact
892 from lp.testing import (
893 admin_logged_in,
894+ EventRecorder,
895 feature_flags,
896 login_person,
897 person_logged_in,
898@@ -566,6 +568,23 @@
899 self.assertThat(
900 recorder2, HasQueryCount(Equals(recorder1.count)))
901
902+ def test_newMessage_default(self):
903+ # Adding a bug message notifies that is was created.
904+ bug = self.factory.makeBug()
905+ login_person(bug.owner)
906+ with EventRecorder() as recorder:
907+ bug.newMessage(owner=bug.owner)
908+ self.assertEqual(1, len(recorder.events))
909+ self.assertIsInstance(recorder.events[0], ObjectCreatedEvent)
910+
911+ def test_newMessage_send_notification_false(self):
912+ # Notifications about new messages can be supressed.
913+ bug = self.factory.makeBug()
914+ login_person(bug.owner)
915+ with EventRecorder() as recorder:
916+ bug.newMessage(owner=bug.owner, send_notifications=False)
917+ self.assertEqual(0, len(recorder.events))
918+
919
920 class TestBugPrivateAndSecurityRelatedUpdatesMixin:
921