Merge lp:~kfogel/launchpad/538219-debdiff-is-patch-too into lp:launchpad

[Karl Fogel]

When a plaintext file with a name ending in ".diff", ".debdiff", or ".patch" is offered by the user as a patch attachment, have Launchpad trust the user and not prompt for confirmation.

[Abel Deuring (adeuring)]

nice work!

And thanks for fixing all the typos I made in earlier branches :)

review: Approve (code)

=== modified file 'lib/lp/bugs/browser/bugattachment.py'
--- lib/lp/bugs/browser/bugattachment.py	2010-01-28 11:07:50 +0000
+++ lib/lp/bugs/browser/bugattachment.py	2010-03-30 21:00:57 +0000
@@ -40,9 +40,19 @@
     """
 
     def guessContentType(self, filename, file_content):
-        """Guess the content type a file with the given anme and content."""
+        """Guess the content type a file with the given name and content."""
         guessed_type, encoding = guess_content_type(
             name=filename, body=file_content)
+        # Zope's guess_content_type() doesn't consider all the factors
+        # we want considered.  So after we get its answer, we probe a
+        # little further.  But we still don't look at the encoding nor
+        # the file content, because we'd like to avoid reimplementing
+        # 'patch'.  See bug #538219 for more.
+        if (guessed_type == 'text/plain'
+            and (filename.endswith('.diff')
+                 or filename.endswith('.debdiff')
+                 or filename.endswith('.patch'))):
+            guessed_type = 'text/x-diff'
         return guessed_type
 
     def attachmentTypeConsistentWithContentType(

=== modified file 'lib/lp/bugs/browser/bugmessage.py'
--- lib/lp/bugs/browser/bugmessage.py	2010-01-28 11:07:50 +0000
+++ lib/lp/bugs/browser/bugmessage.py	2010-03-30 21:00:57 +0000
@@ -107,7 +107,7 @@
             # If the patch flag is not consistent with the result of
             # the guess made in attachmentTypeConsistentWithContentType(),
             # we use the guessed type and lead the user to a page
-            # where he can override the flag value, if Luanchpad's
+            # where he can override the flag value, if Launchpad's
             # guess is wrong.
             patch_flag_consistent = (
                 self.attachmentTypeConsistentWithContentType(

=== modified file 'lib/lp/bugs/stories/bugattachments/10-add-bug-attachment.txt'
--- lib/lp/bugs/stories/bugattachments/10-add-bug-attachment.txt	2010-01-28 11:07:50 +0000
+++ lib/lp/bugs/stories/bugattachments/10-add-bug-attachment.txt	2010-03-30 21:00:57 +0000
@@ -204,3 +204,52 @@
   ...   print li_tag.a.renderContents()
   A fix for this bug.
   A better icon for foo
+
+We expect Launchpad to believe us (that is, not ask for confirmation)
+when we tell it that plain text files whose names end in ".diff",
+".debdiff", or ".patch" are patch attachments:
+
+  >>> user_browser.open(
+  ...     'http://bugs.launchpad.dev/firefox/+bug/1/+addcomment')
+  >>> foo_file.seek(0)
+  >>> user_browser.getControl('Attachment').add_file(
+  ...   foo_file, 'text/plain', 'foo3.diff')
+  >>> user_browser.getControl('Description').value = 'the foo3 patch'
+  >>> patch_control = user_browser.getControl(
+  ...     'This attachment contains a solution (patch) for this bug')
+  >>> patch_control.selected = True
+  >>> user_browser.getControl(
+  ...     name="field.comment").value = 'Add foo3.diff as a patch.'
+  >>> user_browser.getControl('Post Comment').click()
+  >>> user_browser.url
+  'http://bugs.launchpad.dev/firefox/+bug/1'
+
+  >>> user_browser.open(
+  ...     'http://bugs.launchpad.dev/firefox/+bug/1/+addcomment')
+  >>> foo_file.seek(0)
+  >>> user_browser.getControl('Attachment').add_file(
+  ...   foo_file, 'text/plain', 'foo4.debdiff')
+  >>> user_browser.getControl('Description').value = 'the foo4 patch'
+  >>> patch_control = user_browser.getControl(
+  ...     'This attachment contains a solution (patch) for this bug')
+  >>> patch_control.selected = True
+  >>> user_browser.getControl(
+  ...     name="field.comment").value = 'Add foo4.debdiff as a patch.'
+  >>> user_browser.getControl('Post Comment').click()
+  >>> user_browser.url
+  'http://bugs.launchpad.dev/firefox/+bug/1'
+
+  >>> user_browser.open(
+  ...     'http://bugs.launchpad.dev/firefox/+bug/1/+addcomment')
+  >>> foo_file.seek(0)
+  >>> user_browser.getControl('Attachment').add_file(
+  ...   foo_file, 'text/plain', 'foo5.patch')
+  >>> user_browser.getControl('Description').value = 'the foo5 patch'
+  >>> patch_control = user_browser.getControl(
+  ...     'This attachment contains a solution (patch) for this bug')
+  >>> patch_control.selected = True
+  >>> user_browser.getControl(
+  ...     name="field.comment").value = 'Add foo5.patch as a patch.'
+  >>> user_browser.getControl('Post Comment').click()
+  >>> user_browser.url
+  'http://bugs.launchpad.dev/firefox/+bug/1'

=== modified file 'lib/lp/bugs/stories/bugattachments/20-edit-bug-attachment.txt'
--- lib/lp/bugs/stories/bugattachments/20-edit-bug-attachment.txt	2010-01-28 11:07:50 +0000
+++ lib/lp/bugs/stories/bugattachments/20-edit-bug-attachment.txt	2010-03-30 21:00:57 +0000
@@ -76,6 +76,9 @@
     Another title
     A fix for this bug.
     A better icon for foo
+    the foo3 patch
+    the foo4 patch
+    the foo5 patch
 
 ...while it is gone from the portlet "Bug attachments".