← Back to team overview

launchpad-reviewers team mailing list archive

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

 

Curtis Hovey has proposed merging lp:~sinzui/launchpad/empty-attchments-message into lp:launchpad.

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

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #667604 in Launchpad itself: "new bug reports with attachments added generate blank email instead of including links to attachments"
  https://bugs.launchpad.net/launchpad/+bug/667604

For more details, see:
https://code.launchpad.net/~sinzui/launchpad/empty-attchments-message/+merge/127869

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
-- 
https://code.launchpad.net/~sinzui/launchpad/empty-attchments-message/+merge/127869
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~sinzui/launchpad/empty-attchments-message into lp:launchpad.
=== modified file 'lib/lp/bugs/browser/bugtarget.py'
--- lib/lp/bugs/browser/bugtarget.py	2012-09-21 00:28:49 +0000
+++ lib/lp/bugs/browser/bugtarget.py	2012-10-03 20:28:38 +0000
@@ -579,7 +579,8 @@
         if attachment or extra_data.attachments:
             # Attach all the comments to a single empty comment.
             attachment_comment = bug.newMessage(
-                owner=self.user, subject=bug.followup_subject(), content=None)
+                owner=self.user, subject=bug.followup_subject(), content=None,
+                send_notifications=False)
 
             # Deal with attachments added in the filebug form.
             if attachment:

=== modified file 'lib/lp/bugs/browser/tests/bugtarget-filebug-views.txt'
--- lib/lp/bugs/browser/tests/bugtarget-filebug-views.txt	2012-08-16 05:18:54 +0000
+++ lib/lp/bugs/browser/tests/bugtarget-filebug-views.txt	2012-10-03 20:28:38 +0000
@@ -80,546 +80,6 @@
     http://launchpad.dev/firefox/+filebug-show-similar
 
 
-Adding extra info to filed bugs
--------------------------------
-
-It's possible for bug reporting tools to upload a file with debug
-information to Launchpad, and pass that information to the filebug page.
-When the data is uploaded a token is returned, which is appended to the
-+filebug URL, resulting in a URL like '/.../+filebug/12345abcde'. The
-+filebug view's publishTraverse method looks up the correct data from
-using the token.
-
-The uploaded debug information should be MIME multipart message, where
-the Content-Disposition header tells Launchpad what to do with the
-different parts.
-
-
-First inline part
-.................
-
-The first inline part will be appended to the bug description.
-
-    >>> debug_data = """MIME-Version: 1.0
-    ... Content-type: multipart/mixed; boundary=boundary
-    ...
-    ... --boundary
-    ... Content-disposition: inline
-    ... Content-type: text/plain; charset=utf-8
-    ... 
-    ... This should be added to the description.
-    ...
-    ... --boundary--
-    ... """
-    >>> import transaction
-    >>> from lp.services.temporaryblobstorage.interfaces import (
-    ...     ITemporaryStorageManager)
-    >>> token = getUtility(ITemporaryStorageManager).new(debug_data)
-
-We need to commit the transaction since the data will be stored in the
-Librarian.
-
-    >>> transaction.commit()
-
-The data is processed by a ProcessApportBlobJob, which the filebug view
-can then access to get the parsed data. We'll define a helper method for
-processing the blob.
-
-    >>> from zope.component import getUtility
-    >>> from lp.bugs.interfaces.apportjob import IProcessApportBlobJobSource
-
-    >>> def process_blob(token):
-    ...     blob = getUtility(ITemporaryStorageManager).fetch(token)
-    ...     job = getUtility(IProcessApportBlobJobSource).create(blob)
-    ...     job.job.start()
-    ...     job.run()
-    ...     job.job.complete()
-    >>> process_blob(token)
-
-Now, if we pass the token to the filebug view, the extra_data attribute
-will be set with the actual data.
-
-    >>> filebug_view = create_initialized_view(ubuntu_firefox, '+filebug')
-    >>> filebug_view.publishTraverse(
-    ...     filebug_view.request, token) is filebug_view
-    True
-
-    >>> filebug_view.extra_data.extra_description
-    u'This should be added to the description.'
-
-    >>> filebug_view.validate(bug_data) is None
-    True
-
-    >>> filebug_view.submit_bug_action.success(bug_data)
-    >>> filebug_view.added_bug.title
-    u'Test Title'
-
-    >>> description = filebug_view.added_bug.description
-    >>> print description #doctest: -NORMALIZE_WHITESPACE
-    Test description.
-    <BLANKLINE>
-    This should be added to the description.
-
-A notification was added to inform the user about what happened.
-
-    >>> for notification in filebug_view.request.response.notifications:
-    ...     print notification.message
-    <p class="last">Thank you for your bug report.</p>
-    Additional information was added to the bug description.
-
-
-Other inline parts
-..................
-
-If there are more than one inline part, those will be added as comments
-to the bug.
-
-    >>> debug_data = """MIME-Version: 1.0
-    ... Content-type: multipart/mixed; boundary=boundary
-    ...
-    ... --boundary
-    ... Content-disposition: inline
-    ... Content-type: text/plain; charset=utf-8
-    ... 
-    ... This should be added to the description.
-    ...
-    ... --boundary
-    ... Content-disposition: inline
-    ... Content-type: text/plain; charset=utf-8
-    ... 
-    ... This should be added as a comment.
-    ...
-    ... --boundary
-    ... Content-disposition: inline
-    ... Content-type: text/plain; charset=utf-8
-    ... 
-    ... This should be added as another comment.
-    ...
-    ... --boundary--
-    ... """
-    >>> token = getUtility(ITemporaryStorageManager).new(debug_data)
-    >>> transaction.commit()
-
-    >>> process_blob(token)
-    >>> filebug_view = create_initialized_view(ubuntu_firefox, '+filebug')
-    >>> filebug_view.publishTraverse(filebug_view.request,
-    ...     token) is filebug_view
-    True
-
-    >>> filebug_view.validate(bug_data) is None
-    True
-
-    >>> filebug_view.submit_bug_action.success(bug_data)
-
-    >>> filebug_view.added_bug.title
-    u'Test Title'
-
-    >>> description = filebug_view.added_bug.description
-    >>> print description #doctest: -NORMALIZE_WHITESPACE
-    Test description.
-    <BLANKLINE>
-    This should be added to the description.
-
-    >>> for comment in filebug_view.added_bug.messages[1:]:
-    ...     print "Comment by %s: %s" % (
-    ...         comment.owner.displayname, comment.text_contents)
-    Comment by No Privileges Person: This should be added as a comment.
-    Comment by No Privileges Person: This should be added as another comment.
-
-Notifications were added to inform the user about what happened.
-
-    >>> for notification in filebug_view.request.response.notifications:
-    ...     print notification.message
-    <p class="last">Thank you for your bug report.</p>
-    Additional information was added to the bug description.
-    A comment with additional information was added to the bug report.
-    A comment with additional information was added to the bug report.
-
-
-Attachments
-...........
-
-All the parts that have a 'Content-disposition: attachment' header will
-get added as attachments to the bug. The attachment description can be
-specified using a Content-description header, but it's not required.
-
-    >>> debug_data = """MIME-Version: 1.0
-    ... Content-type: multipart/mixed; boundary=boundary
-    ...
-    ... --boundary
-    ... Content-disposition: attachment; filename='attachment1'
-    ... Content-type: text/plain; charset=utf-8
-    ... 
-    ... This is an attachment.
-    ...
-    ... --boundary
-    ... Content-disposition: attachment; filename='attachment2'
-    ... Content-description: Attachment description.
-    ... Content-type: text/plain; charset=ISO-8859-1
-    ... 
-    ... This is another attachment, with a description.
-    ...
-    ... --boundary--
-    ... """
-    >>> token = getUtility(ITemporaryStorageManager).new(debug_data)
-    >>> transaction.commit()
-
-    >>> process_blob(token)
-    >>> filebug_view = create_initialized_view(ubuntu_firefox, '+filebug')
-    >>> filebug_view.publishTraverse(filebug_view.request,
-    ...     token) is filebug_view
-    True
-
-    >>> filebug_view.validate(bug_data) is None
-    True
-
-    >>> filebug_view.submit_bug_action.success(bug_data)
-
-Since the attachments are stored in the Librarian, we need to commit the
-transaction in order to access them.
-
-    >>> transaction.commit()
-
-    >>> filebug_view.added_bug.title
-    u'Test Title'
-
-    >>> print filebug_view.added_bug.description
-    Test description.
-
-The attachments got added, with the charsets preserved, and the one that
-didn't specify a description got an autogenerated one.
-
-    >>> for attachment in filebug_view.added_bug.attachments_unpopulated:
-    ...     print "Filename: %s" % attachment.libraryfile.filename
-    ...     print "Content type: %s" % attachment.libraryfile.mimetype
-    ...     print "Description: %s" % attachment.title
-    ...     print "Contents:\n%s" % attachment.libraryfile.read()
-    ...     print
-    Filename: attachment1
-    Content type: text/plain; charset=utf-8
-    Description: attachment1
-    Contents:
-    This is an attachment.
-    <BLANKLINE>
-    Filename: attachment2
-    Content type: text/plain; charset=ISO-8859-1
-    Description: Attachment description.
-    Contents:
-    This is another attachment, with a description.
-    <BLANKLINE>
-
-Notifications were added to inform the user about what happened.
-
-    >>> for notification in filebug_view.request.response.notifications:
-    ...     print notification.message
-    <p class="last">Thank you for your bug report.</p>
-    The file "attachment1" was attached to the bug report.
-    The file "attachment2" was attached to the bug report.
-
-The attachments are all added to the same comment.
-
-    >>> for comment in filebug_view.added_bug.messages[1:]:
-    ...     print "Comment by %s: %s attachment(s)" % (
-    ...         comment.owner.displayname, len(comment.bugattachments))
-    Comment by No Privileges Person: 2 attachment(s)
-
-
-Private Bugs
-............
-
-We can specify whether a bug is private by providing Private field in
-the message.
-
-    >>> debug_data = """MIME-Version: 1.0
-    ... Content-type: multipart/mixed; boundary=boundary
-    ... Private: yes
-    ...
-    ... --boundary
-    ... Content-disposition: inline
-    ... Content-type: text/plain; charset=utf-8
-    ... 
-    ... This bug should be private.
-    ...
-    ... --boundary--
-    ... """
-    >>> token = getUtility(ITemporaryStorageManager).new(debug_data)
-    >>> transaction.commit()
-
-    >>> process_blob(token)
-    >>> filebug_view = create_initialized_view(ubuntu_firefox, '+filebug')
-    >>> filebug_view.publishTraverse(filebug_view.request,
-    ...     token) is filebug_view
-    True
-
-    >>> filebug_view.extra_data.extra_description
-    u'This bug should be private.'
-
-    >>> filebug_view.extra_data.private
-    True
-
-    >>> filebug_view.validate(bug_data) is None
-    True
-
-    >>> filebug_view.submit_bug_action.success(bug_data)
-    >>> filebug_view.added_bug.title
-    u'Test Title'
-
-    >>> description = filebug_view.added_bug.description
-    >>> print description #doctest: -NORMALIZE_WHITESPACE
-    Test description.
-    <BLANKLINE>
-    This bug should be private.
-
-    >>> filebug_view.added_bug.information_type
-    <DBItem InformationType.USERDATA, (4) Private>
-
-The user will be notified that the bug has been marked as private.
-
-    >>> print filebug_view.request.response.notifications[2].message
-    This bug report has been marked private...
-
-We can also specify that a bug is public via the same field.
-
-    >>> debug_data = """MIME-Version: 1.0
-    ... Content-type: multipart/mixed; boundary=boundary
-    ... Private: no
-    ...
-    ... --boundary
-    ... Content-disposition: inline
-    ... Content-type: text/plain; charset=utf-8
-    ... 
-    ... This bug should be public.
-    ...
-    ... --boundary--
-    ... """
-    >>> token = getUtility(ITemporaryStorageManager).new(debug_data)
-    >>> transaction.commit()
-
-    >>> process_blob(token)
-    >>> filebug_view = create_initialized_view(ubuntu_firefox, '+filebug')
-    >>> filebug_view.publishTraverse(filebug_view.request,
-    ...     token) is filebug_view
-    True
-
-    >>> filebug_view.extra_data.extra_description
-    u'This bug should be public.'
-
-    >>> filebug_view.extra_data.private
-    False
-
-    >>> filebug_view.validate(bug_data) is None
-    True
-
-    >>> filebug_view.submit_bug_action.success(bug_data)
-    >>> filebug_view.added_bug.title
-    u'Test Title'
-
-    >>> description = filebug_view.added_bug.description
-    >>> print description #doctest: -NORMALIZE_WHITESPACE
-    Test description.
-    <BLANKLINE>
-    This bug should be public.
-
-    >>> filebug_view.added_bug.private
-    False
-
-Since this bug is public, both the reporter and the bug supervisor have
-been subscribed.
-
-    >>> for subscriber in filebug_view.added_bug.getDirectSubscribers():
-    ...     print subscriber.displayname
-    No Privileges Person
-
-    >>> for subscriber in filebug_view.added_bug.getIndirectSubscribers():
-    ...     print subscriber.displayname
-    Foo Bar
-
-
-Subscriptions
-.............
-
-We can also subscribe someone to this bug when we file it by using a
-Subscribe field in the message. Multiple people can be specified and
-they can be identified by their Launchpad name or their e-mail address.
-
-    >>> debug_data = """MIME-Version: 1.0
-    ... Content-type: multipart/mixed; boundary=boundary
-    ... Subscribers: ddaa test@xxxxxxxxxxxxx
-    ...
-    ... --boundary
-    ... Content-disposition: inline
-    ... Content-type: text/plain; charset=utf-8
-    ... 
-    ... Other people are interested in this bug.
-    ...
-    ... --boundary--
-    ... """
-    >>> token = getUtility(ITemporaryStorageManager).new(debug_data)
-    >>> transaction.commit()
-
-    >>> process_blob(token)
-    >>> filebug_view = create_initialized_view(ubuntu_firefox, '+filebug')
-    >>> filebug_view.publishTraverse(filebug_view.request,
-    ...     token) is filebug_view
-    True
-
-    >>> filebug_view.extra_data.extra_description
-    u'Other people are interested in this bug.'
-
-    >>> for subscriber in filebug_view.extra_data.subscribers:
-    ...     print subscriber
-    ddaa
-    test@xxxxxxxxxxxxx
-
-    >>> filebug_view.validate(bug_data) is None
-    True
-
-    >>> filebug_view.submit_bug_action.success(bug_data)
-    >>> filebug_view.added_bug.title
-    u'Test Title'
-
-    >>> description = filebug_view.added_bug.description
-    >>> print description #doctest: -NORMALIZE_WHITESPACE
-    Test description.
-    <BLANKLINE>
-    Other people are interested in this bug.
-
-As well as the reporter, both Sample Person and David Allouche have been
-subscribed to the bug.
-
-    >>> for subscriber in filebug_view.added_bug.getDirectSubscribers():
-    ...     print subscriber.displayname
-    David Allouche
-    No Privileges Person
-    Sample Person
-
-The user will be notified that Sample Person and David Allouche has been
-subscribed to this bug.
-
-    >>> for notification in filebug_view.request.response.notifications:
-    ...     print notification.message
-    <p class="last">Thank you for your bug report.</p>
-    Additional information was added to the bug description.
-    David Allouche has been subscribed to this bug.
-    Sample Person has been subscribed to this bug.
-
-
-Subscribers to Private bugs
-...........................
-
-The Private and Subscriber fields are intended to be used together to
-subscribe certain people and teams to bugs when they are filed.
-
-    >>> debug_data = """MIME-Version: 1.0
-    ... Content-type: multipart/mixed; boundary=boundary
-    ... Private: yes
-    ... Subscribers: mark
-    ...
-    ... --boundary
-    ... Content-disposition: inline
-    ... Content-type: text/plain; charset=utf-8
-    ... 
-    ... This bug should be private, and Mark Shuttleworth subscribed.
-    ...
-    ... --boundary--
-    ... """
-    >>> token = getUtility(ITemporaryStorageManager).new(debug_data)
-    >>> transaction.commit()
-
-    >>> process_blob(token)
-    >>> filebug_view = create_initialized_view(ubuntu_firefox, '+filebug')
-    >>> filebug_view.publishTraverse(filebug_view.request,
-    ...     token) is filebug_view
-    True
-
-    >>> filebug_view.extra_data.extra_description
-    u'This bug should be private, and Mark Shuttleworth subscribed.'
-
-    >>> filebug_view.extra_data.private
-    True
-
-    >>> for subscriber in filebug_view.extra_data.subscribers:
-    ...     print subscriber
-    mark
-
-    >>> filebug_view.validate(bug_data) is None
-    True
-
-    >>> filebug_view.submit_bug_action.success(bug_data)
-    >>> filebug_view.added_bug.title
-    u'Test Title'
-
-    >>> description = filebug_view.added_bug.description
-    >>> print description #doctest: -NORMALIZE_WHITESPACE
-    Test description.
-    <BLANKLINE>
-    This bug should be private, and Mark Shuttleworth subscribed.
-
-    >>> filebug_view.added_bug.private
-    True
-
-    >>> filebug_view.added_bug.security_related
-    False
-
-As well as the reporter, Mark Shuttleworth should have been subscribed
-to the bug.
-
-    >>> for subscriber in filebug_view.added_bug.getDirectSubscribers():
-    ...     print subscriber.displayname
-    Mark Shuttleworth
-    No Privileges Person
-
-The user will be notified that Mark Shuttleworth has been subscribed to
-this bug and that the bug has been marked as private.
-
-    >>> for notification in filebug_view.request.response.notifications:
-    ...     print notification.message
-    <p class="last">Thank you for your bug report.</p>
-    Additional information was added to the bug description.
-    Mark Shuttleworth has been subscribed to this bug.
-    This bug report has been marked private...
-
-
-publishTraverse()
------------------
-
-As already seen above, it's the FileBugViewBase's publishTraverse that
-finds the right blob to use.
-
-    >>> filebug_view = create_initialized_view(ubuntu_firefox, '+filebug')
-    >>> filebug_view.publishTraverse(filebug_view.request,
-    ...     token) is filebug_view
-    True
-
-    >>> filebug_view.extra_data_token == token
-    True
-
-    >>> filebug_view.extra_data is not None
-    True
-
-Since the view itself is returned, it will handle further traversals as
-well, so if we call the method again, it represents a URL like
-'.../+filebug/token/foo', which should raise a NotFound error.
-
-    >>> filebug_view.publishTraverse(filebug_view.request, token)
-    Traceback (most recent call last):
-    ...
-    NotFound:...
-
-
-Not found tokens
-................
-
-If publishTraverse is called with a token that can't be found, a
-NotFound error is raised.
-
-    >>> filebug_view = create_initialized_view(ubuntu_firefox, '+filebug')
-    >>> filebug_view.publishTraverse(filebug_view.request, 'no-such-token')
-    Traceback (most recent call last):
-    ...
-    NotFound:...
-
-
 Adding tags to filed bugs
 -------------------------
 

=== modified file 'lib/lp/bugs/browser/tests/test_bugtarget_filebug.py'
--- lib/lp/bugs/browser/tests/test_bugtarget_filebug.py	2012-09-21 00:28:49 +0000
+++ lib/lp/bugs/browser/tests/test_bugtarget_filebug.py	2012-10-03 20:28:38 +0000
@@ -4,10 +4,12 @@
 __metaclass__ = type
 
 
+from textwrap import dedent
 from BeautifulSoup import BeautifulSoup
 from lazr.restful.interfaces import IJSONRequestCache
 import transaction
 from zope.component import getUtility
+from zope.publisher.interfaces import NotFound
 from zope.schema.interfaces import (
     TooLong,
     TooShort,
@@ -19,6 +21,7 @@
     PUBLIC_INFORMATION_TYPES,
     )
 from lp.bugs.browser.bugtarget import FileBugViewBase
+from lp.bugs.interfaces.apportjob import IProcessApportBlobJobSource
 from lp.bugs.interfaces.bug import (
     IBugAddForm,
     IBugSet,
@@ -30,14 +33,20 @@
 from lp.bugs.publisher import BugsLayer
 from lp.registry.enums import BugSharingPolicy
 from lp.registry.interfaces.projectgroup import IProjectGroup
+from lp.services.temporaryblobstorage.interfaces import (
+    ITemporaryStorageManager)
 from lp.services.webapp.servers import LaunchpadTestRequest
 from lp.testing import (
+    EventRecorder,
     login,
     login_person,
     person_logged_in,
     TestCaseWithFactory,
     )
-from lp.testing.layers import DatabaseFunctionalLayer
+from lp.testing.layers import (
+    DatabaseFunctionalLayer,
+    LaunchpadFunctionalLayer,
+    )
 from lp.testing.pages import (
     find_main_content,
     find_tag_by_id,
@@ -275,9 +284,8 @@
         self.assertIs(None, find_tag_by_id(html, 'filebug-search-form'))
 
 
-class TestFileBugViewBase(TestCaseWithFactory):
-
-    layer = DatabaseFunctionalLayer
+class FileBugViewMixin:
+    """Provide a FileBugView subclass that is easy to test."""
 
     class FileBugTestView(FileBugViewBase):
         """A simple subclass."""
@@ -288,7 +296,7 @@
             pass
 
     def setUp(self):
-        super(TestFileBugViewBase, self).setUp()
+        super(FileBugViewMixin, self).setUp()
         self.target = self.factory.makeProduct()
         transaction.commit()
         login_person(self.target.owner)
@@ -308,6 +316,11 @@
         view.initialize()
         return view
 
+
+class TestFileBugViewBase(FileBugViewMixin, TestCaseWithFactory):
+
+    layer = DatabaseFunctionalLayer
+
     def test_submit_comment_empty_error(self):
         # The comment cannot be an empty string.
         form = self.get_form(comment='')
@@ -491,6 +504,219 @@
         self.assertIn("This can be fixed by changing", html)
 
 
+class FileBugViewBaseExtraDataTestCase(FileBugViewMixin, TestCaseWithFactory):
+    """FileBugView handles extra data from blobs created by apport."""
+
+    layer = LaunchpadFunctionalLayer
+
+    @staticmethod
+    def get_form():
+        return {
+            'title': 'test title',
+            'comment': 'test description',
+            }
+
+    @staticmethod
+    def process_extra_data(raw_data=None, command=''):
+        if raw_data is None:
+            raw_data = """\
+            MIME-Version: 1.0
+            Content-type: multipart/mixed; boundary=boundary
+            %s
+
+            --boundary
+            Content-disposition: inline
+            Content-type: text/plain; charset=utf-8
+
+            Added to the description.
+
+            --boundary--
+            """ % command
+        extra_data = dedent(raw_data)
+        temp_storage_manager = getUtility(ITemporaryStorageManager)
+        token = temp_storage_manager.new(extra_data)
+        transaction.commit()
+        blob = temp_storage_manager.fetch(token)
+        job = getUtility(IProcessApportBlobJobSource).create(blob)
+        job.job.start()
+        job.run()
+        job.job.complete()
+        return token
+
+    def test_publish_traverse_token(self):
+        # The publish_traverse() method uses a token to lookup the extra_data.
+        token = self.process_extra_data(command="not-requred: ignore")
+        view = self.create_initialized_view()
+        self.assertIs(view, view.publishTraverse(view.request, token))
+
+    def test_publish_traverse_token_error(self):
+        # The publish_traverse() method uses a token to lookup the extra_data.
+        view = self.create_initialized_view()
+        self.assertRaises(
+            NotFound, view.publishTraverse, view.request, 'no-such-token')
+
+    def test_description_and_comments(self):
+        # The first extra text part is added to the desciption, all other
+        # extra parts become additional bug messages.
+        token = self.process_extra_data("""\
+            MIME-Version: 1.0
+            Content-type: multipart/mixed; boundary=boundary
+
+            --boundary
+            Content-disposition: inline
+            Content-type: text/plain; charset=utf-8
+
+            Added to the description.
+
+            --boundary
+            Content-disposition: inline
+            Content-type: text/plain; charset=utf-8
+
+            A bug comment.
+
+            --boundary--
+            """)
+        view = self.create_initialized_view()
+        view.publishTraverse(view.request, token)
+        self.assertEqual(
+            'Added to the description.', view.extra_data.extra_description)
+        with EventRecorder() as recorder:
+            view.submit_bug_action.success(self.get_form())
+            # Subscribers are only notified about the new bug event;
+            # The extra comment for the attchments was silent.
+            self.assertEqual(2, len(recorder.events))
+            bug_event, message_event = recorder.events
+        transaction.commit()
+        bug = view.added_bug
+        self.assertEqual(bug, bug_event.object)
+        self.assertEqual(
+            'test description\n\n'
+            'Added to the description.',
+            bug.description)
+        self.assertEqual(2, bug.messages.count())
+        self.assertEqual(bug.bug_messages[-1], message_event.object)
+        notifications = view.request.response.notifications
+        self.assertEqual(3, len(notifications))
+        self.assertEqual(
+            '<p class="last">Thank you for your bug report.</p>',
+            notifications[0].message)
+        self.assertEqual(
+            'Additional information was added to the bug description.',
+            notifications[1].message)
+        self.assertEqual(
+            'A comment with additional information was added '
+            'to the bug report.',
+            notifications[2].message)
+
+    def test_private_yes(self):
+        # The extra data can specify the bug is private.
+        token = self.process_extra_data(command='Private: yes')
+        view = self.create_initialized_view()
+        view.publishTraverse(view.request, token)
+        view.submit_bug_action.success(self.get_form())
+        transaction.commit()
+        bug = view.added_bug
+        self.assertIs(True, bug.private)
+        self.assertEqual(InformationType.USERDATA, bug.information_type)
+
+    def test_private_no(self):
+        # The extra data can specify the bug is public.
+        token = self.process_extra_data(command='Private: no')
+        view = self.create_initialized_view()
+        view.publishTraverse(view.request, token)
+        view.submit_bug_action.success(self.get_form())
+        transaction.commit()
+        bug = view.added_bug
+        self.assertIs(False, bug.private)
+        self.assertEqual(InformationType.PUBLIC, bug.information_type)
+
+    def test_subscribers_with_email_address(self):
+        # The extra data can add bug subscribers via email address.
+        subscriber_1 = self.factory.makePerson(email='me@xxxxxx')
+        subscriber_2 = self.factory.makePerson(email='him@xxxxxx')
+        token = self.process_extra_data(
+            command='Subscribers: me@xxxxxx him@xxxxxx')
+        view = self.create_initialized_view()
+        view.publishTraverse(view.request, token)
+        self.assertEqual(2, len(view.extra_data.subscribers))
+        self.assertContentEqual(
+            ['me@xxxxxx', 'him@xxxxxx'], view.extra_data.subscribers)
+        view.submit_bug_action.success(self.get_form())
+        transaction.commit()
+        bug = view.added_bug
+        subscribers = [subscriber_1, subscriber_2, bug.owner]
+        self.assertContentEqual(subscribers, bug.getDirectSubscribers())
+
+    def test_subscribers_with_name(self):
+        # The extra data can add bug subscribers via Launchpad Id..
+        subscriber_1 = self.factory.makePerson(name='me')
+        subscriber_2 = self.factory.makePerson(name='him')
+        token = self.process_extra_data(command='Subscribers: me him')
+        view = self.create_initialized_view()
+        view.publishTraverse(view.request, token)
+        self.assertEqual(2, len(view.extra_data.subscribers))
+        self.assertContentEqual(['me', 'him'], view.extra_data.subscribers)
+        view.submit_bug_action.success(self.get_form())
+        transaction.commit()
+        bug = view.added_bug
+        subscribers = [subscriber_1, subscriber_2, bug.owner]
+        self.assertContentEqual(subscribers, bug.getDirectSubscribers())
+
+    def test_attachments(self):
+        # The attachment comment has no content and it does not notify.
+        token = self.process_extra_data("""\
+            MIME-Version: 1.0
+            Content-type: multipart/mixed; boundary=boundary
+
+            --boundary
+            Content-disposition: attachment; filename='attachment1'
+            Content-type: text/plain; charset=utf-8
+
+            This is an attachment.
+
+            --boundary
+            Content-disposition: attachment; filename='attachment2'
+            Content-description: Attachment description.
+            Content-type: text/plain; charset=ISO-8859-1
+
+            This is another attachment, with a description.
+
+            --boundary--
+            """)
+        view = self.create_initialized_view()
+        view.publishTraverse(view.request, token)
+        with EventRecorder() as recorder:
+            view.submit_bug_action.success(self.get_form())
+            # Subscribers are only notified about the new bug event;
+            # The extra comment for the attchments was silent.
+            self.assertEqual(1, len(recorder.events))
+            self.assertEqual(view.added_bug, recorder.events[0].object)
+        transaction.commit()
+        bug = view.added_bug
+        attachments = [at for at in bug.attachments_unpopulated]
+        self.assertEqual(2, len(attachments))
+        attachment = attachments[0]
+        self.assertEqual('attachment1', attachment.title)
+        self.assertEqual('attachment1', attachment.libraryfile.filename)
+        self.assertEqual(
+            'text/plain; charset=utf-8', attachment.libraryfile.mimetype)
+        self.assertEqual(
+            'This is an attachment.\n\n', attachment.libraryfile.read())
+        self.assertEqual(2, bug.messages.count())
+        self.assertEqual(2, len(bug.messages[1].bugattachments))
+        notifications = view.request.response.notifications
+        self.assertEqual(3, len(notifications))
+        self.assertEqual(
+            '<p class="last">Thank you for your bug report.</p>',
+            notifications[0].message)
+        self.assertEqual(
+            'The file "attachment1" was attached to the bug report.',
+            notifications[1].message)
+        self.assertEqual(
+            'The file "attachment2" was attached to the bug report.',
+            notifications[2].message)
+
+
 class TestFileBugForNonBugSupervisors(TestCaseWithFactory):
 
     layer = DatabaseFunctionalLayer

=== modified file 'lib/lp/bugs/model/bug.py'
--- lib/lp/bugs/model/bug.py	2012-10-02 19:12:26 +0000
+++ lib/lp/bugs/model/bug.py	2012-10-03 20:28:38 +0000
@@ -1166,7 +1166,7 @@
 
     def newMessage(self, owner=None, subject=None,
                    content=None, parent=None, bugwatch=None,
-                   remote_comment_id=None):
+                   remote_comment_id=None, send_notifications=True):
         """Create a new Message and link it to this bug."""
         if subject is None:
             subject = self.followup_subject()
@@ -1180,7 +1180,8 @@
         if not bugmsg:
             return
 
-        notify(ObjectCreatedEvent(bugmsg, user=owner))
+        if send_notifications:
+            notify(ObjectCreatedEvent(bugmsg, user=owner))
 
         return bugmsg.message
 

=== modified file 'lib/lp/bugs/model/tests/test_bug.py'
--- lib/lp/bugs/model/tests/test_bug.py	2012-09-21 00:28:49 +0000
+++ lib/lp/bugs/model/tests/test_bug.py	2012-10-03 20:28:38 +0000
@@ -13,6 +13,7 @@
 from testtools.testcase import ExpectedException
 from zope.component import getUtility
 from zope.security.proxy import removeSecurityProxy
+from lazr.lifecycle.event import ObjectCreatedEvent
 
 from lp.app.enums import InformationType
 from lp.app.interfaces.launchpad import ILaunchpadCelebrities
@@ -39,6 +40,7 @@
 from lp.registry.tests.test_accesspolicy import get_policies_for_artifact
 from lp.testing import (
     admin_logged_in,
+    EventRecorder,
     feature_flags,
     login_person,
     person_logged_in,
@@ -566,6 +568,23 @@
         self.assertThat(
             recorder2, HasQueryCount(Equals(recorder1.count)))
 
+    def test_newMessage_default(self):
+        # Adding a bug message notifies that is was created.
+        bug = self.factory.makeBug()
+        login_person(bug.owner)
+        with EventRecorder() as recorder:
+            bug.newMessage(owner=bug.owner)
+            self.assertEqual(1, len(recorder.events))
+            self.assertIsInstance(recorder.events[0], ObjectCreatedEvent)
+
+    def test_newMessage_send_notification_false(self):
+        # Notifications about new messages can be supressed.
+        bug = self.factory.makeBug()
+        login_person(bug.owner)
+        with EventRecorder() as recorder:
+            bug.newMessage(owner=bug.owner, send_notifications=False)
+            self.assertEqual(0, len(recorder.events))
+
 
 class TestBugPrivateAndSecurityRelatedUpdatesMixin:
 


Follow ups