launchpad-reviewers team mailing list archive

[Francesco Banconi <frankban@xxxxxxxxx>]

Francesco Banconi has proposed merging lp:~frankban/launchpad/whitespace-comments-198058 into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #198058 in Launchpad itself: "Comments made up entirely of whitespace should be discarded"
  https://bugs.launchpad.net/launchpad/+bug/198058

For more details, see:
https://code.launchpad.net/~frankban/launchpad/whitespace-comments-198058/+merge/85344


-- 
https://code.launchpad.net/~frankban/launchpad/whitespace-comments-198058/+merge/85344
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~frankban/launchpad/whitespace-comments-198058 into lp:launchpad.

=== modified file 'lib/lp/bugs/browser/bugmessage.py'
--- lib/lp/bugs/browser/bugmessage.py	2010-11-23 23:22:27 +0000
+++ lib/lp/bugs/browser/bugmessage.py	2011-12-12 14:45:34 +0000
@@ -54,9 +54,9 @@
         # Ensure either a comment or filecontent was provide, but only
         # if no errors have already been noted.
         if len(self.errors) == 0:
-            comment = data.get('comment', None)
+            comment = data.get('comment', None) or u''
             filecontent = data.get('filecontent', None)
-            if not comment and not filecontent:
+            if not comment.strip() and not filecontent:
                 self.addError("Either a comment or attachment "
                               "must be provided.")
 
@@ -135,7 +135,6 @@
             self.request.response.addNotification(
                 "Attachment %s added to bug." % filename)
 
-
     def shouldShowEmailMeWidget(self):
         """Should the subscribe checkbox be shown?"""
         return not self.context.bug.isSubscribed(self.user)

=== modified file 'lib/lp/bugs/browser/tests/test_bug_views.py'
--- lib/lp/bugs/browser/tests/test_bug_views.py	2011-11-28 00:39:07 +0000
+++ lib/lp/bugs/browser/tests/test_bug_views.py	2011-12-12 14:45:34 +0000
@@ -26,7 +26,10 @@
 from canonical.launchpad.webapp.interfaces import IOpenLaunchBag
 from canonical.launchpad.webapp.servers import LaunchpadTestRequest
 from canonical.launchpad.testing.pages import find_tag_by_id
-from canonical.testing.layers import DatabaseFunctionalLayer
+from canonical.testing.layers import (
+    DatabaseFunctionalLayer,
+    LaunchpadFunctionalLayer,
+    )
 
 from lp.registry.interfaces.person import PersonVisibility
 from lp.services.features.testing import FeatureFixture
@@ -491,3 +494,36 @@
                 'link rel=canonical',
                 'link',
                 dict(rel='canonical', href=expected_url))))
+
+
+class TestBugMessageAddFormView(TestCaseWithFactory):
+    """Tests for the add message to bug view."""
+    layer = LaunchpadFunctionalLayer
+
+    def test_whitespaces_message(self):
+        # Ensure that a message only containing whitespaces is not
+        # considered valid.
+        bug = self.factory.makeBug()
+        form = {
+            'field.comment': u' ',
+            'field.actions.save': u'Post Comment',
+            }
+        view = create_initialized_view(
+            bug.default_bugtask, '+addcomment', form=form)
+        expected_error = u'Either a comment or attachment must be provided.'
+        self.assertEquals(view.errors[0], expected_error)
+
+    def test_whitespaces_message_with_attached_file(self):
+        # If the message only contains whitespaces but a file
+        # is attached then the request have to be considered valid.
+        bug = self.factory.makeBug()
+        form = {
+            'field.comment': u' ',
+            'field.actions.save': u'Post Comment',
+            'field.filecontent': self.factory.makeFakeFileUpload(),
+            'field.patch.used': u'',
+            }
+        login_person(self.factory.makePerson())
+        view = create_initialized_view(
+            bug.default_bugtask, '+addcomment', form=form)
+        self.assertFalse(view.errors)

=== modified file 'lib/lp/testing/factory.py'
--- lib/lp/testing/factory.py	2011-12-12 02:45:10 +0000
+++ lib/lp/testing/factory.py	2011-12-12 14:45:34 +0000
@@ -4398,6 +4398,23 @@
         IStore(grant).flush()
         return grant
 
+    def makeFakeFileUpload(self, filename=None, content=None):
+        """Return a zope.publisher.browser.FileUpload like object.
+
+        This can be useful while testing multipart form submission.
+        """
+        if filename is None:
+            filename = self.getUniqueString()
+        if content is None:
+            content = self.getUniqueString()
+        fileupload = StringIO(content)
+        fileupload.filename = filename
+        fileupload.headers = {
+            'Content-Type': 'text/plain; charset=utf-8',
+            'Content-Disposition': 'attachment; filename="%s"' % filename
+            }
+        return fileupload
+
 
 # Some factory methods return simple Python types. We don't add
 # security wrappers for them, as well as for objects created by