launchpad-reviewers team mailing list archive

["j.c.sackett" <jonathan.sackett@xxxxxxxxxxxxx>]

j.c.sackett has proposed merging lp:~jcsackett/launchpad/affects-before-bug-email into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #83426 in Launchpad itself: "Issuing the 'affects' command before a 'bug' command while editing multiple bugs using the email interface shouldn't crash."
  https://bugs.launchpad.net/launchpad/+bug/83426

For more details, see:
https://code.launchpad.net/~jcsackett/launchpad/affects-before-bug-email/+merge/55184

Summary
=======
Emails sent to edit@ without specifying a bug cause an oops; this branch adds apporpriate error handling to the handler for that instance.

Proposed Fix
============
Capture emails that specify commands without a bug; halt processing and send a message to the user explaining the problem with their email.

Implementation
==============
lib/canonical/launchpad/mail/commands.py
lib/canonical/launchpad/mail/errortemplates/command-with-no-bug.txt
-------------------------------------------------------------------
Added error handling for commands issued with no associated bug.

lib/lp/bugs/tests/bug-emailinterface.txt
----------------------------------------
Added example case for testing.

Tests
=====
bin/test -vvct bug-emailinterface.txt

Demo & QA
=========
Send an email that invokes "affects" or "summary" without specifying a bug. No OOPs should result, and you should receive an email that explains you screwed up and how to fix it.

Lint
====
make lint output:

= Launchpad lint =

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/canonical/launchpad/mail/commands.py
  lib/canonical/launchpad/mail/errortemplates/command-with-no-bug.txt
  lib/lp/bugs/tests/bugs-emailinterface.txt

-- 
https://code.launchpad.net/~jcsackett/launchpad/affects-before-bug-email/+merge/55184
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jcsackett/launchpad/affects-before-bug-email into lp:launchpad.

=== modified file 'lib/canonical/launchpad/mail/commands.py'
--- lib/canonical/launchpad/mail/commands.py	2011-03-23 16:28:51 +0000
+++ lib/canonical/launchpad/mail/commands.py	2011-03-28 17:34:49 +0000
@@ -423,6 +423,11 @@
 
     def execute(self, bug, current_event):
         """See IEmailCommand."""
+        if bug is None:
+            raise EmailProcessingError(
+                get_error_message('command-with-no-bug.txt'),
+                stop_processing=True)
+
         # Do a manual control of the number of arguments, in order to
         # provide a better error message than the default one.
         if len(self.string_args) > 1:
@@ -612,6 +617,11 @@
 
     def execute(self, bug):
         """See IEmailCommand."""
+        if bug is None:
+            raise EmailProcessingError(
+                get_error_message('command-with-no-bug.txt'),
+                stop_processing=True)
+
         string_args = list(self.string_args)
         try:
             path = string_args.pop(0)

=== added file 'lib/canonical/launchpad/mail/errortemplates/command-with-no-bug.txt'
--- lib/canonical/launchpad/mail/errortemplates/command-with-no-bug.txt	1970-01-01 00:00:00 +0000
+++ lib/canonical/launchpad/mail/errortemplates/command-with-no-bug.txt	2011-03-28 17:34:49 +0000
@@ -0,0 +1,8 @@
+The message you sent included commands to modify a bug,
+but no bug was specified. Please supply a bug before the command
+to modify it.
+
+For example:
+
+    bug 4
+    summary "This is a new summary"

=== modified file 'lib/lp/bugs/tests/bugs-emailinterface.txt'
--- lib/lp/bugs/tests/bugs-emailinterface.txt	2011-03-23 16:28:51 +0000
+++ lib/lp/bugs/tests/bugs-emailinterface.txt	2011-03-28 17:34:49 +0000
@@ -331,6 +331,52 @@
     >>> print bug_five.title
     Nicer summary
 
+The email handler requires that a bug be specified to be changed. If no
+bug is specified, no edits occur and a message is sent to the user telling
+them what happened.
+
+    >>> edit_mail = """From: test@xxxxxxxxxxxxx
+    ... To: edit@malone-domain
+    ... Date: Fri Jun 17 10:10:23 BST 2005
+    ... Subject: Not important
+    ...
+    ...     summary "Even nicer summary" 
+    ... """
+
+    >>> process_email(edit_mail)
+    >>> commit()
+
+This time, neither bug four or five were updated.
+
+    >>> print bug_four.title
+    Changed summary
+    >>> print bug_five.title
+    Nicer summary
+
+And the person sending the email has received an error message.
+
+    >>> def print_latest_email():
+    ...     commit()
+    ...     if not stub.test_emails:
+    ...         raise AssertionError("No emails queued!")
+    ...     from_addr, to_addrs, raw_message = stub.test_emails[-1]
+    ...     sent_msg = email.message_from_string(raw_message)
+    ...     error_mail, original_mail = sent_msg.get_payload()
+    ...     print "Subject: %s" % sent_msg['Subject']
+    ...     print "To: %s" % ', '.join(to_addrs)
+    ...     print
+    ...     print error_mail.get_payload(decode=True)
+
+    >>> print_latest_email()
+    Subject: Submit Request Failure
+    To: test@xxxxxxxxxxxxx
+    <BLANKLINE>
+    ...
+    The message you sent included commands to modify a bug,
+    but no bug was specified. Please supply a bug before the command
+    to modify it.
+    <BLANKLINE>
+    ...
 
 GPG signing and adding comments
 -------------------------------
@@ -397,17 +443,6 @@
 And an error message was sent to the Sample Person, telling him what's
 wrong.
 
-    >>> def print_latest_email():
-    ...     commit()
-    ...     if not stub.test_emails:
-    ...         raise AssertionError("No emails queued!")
-    ...     from_addr, to_addrs, raw_message = stub.test_emails[-1]
-    ...     sent_msg = email.message_from_string(raw_message)
-    ...     error_mail, original_mail = sent_msg.get_payload()
-    ...     print "Subject: %s" % sent_msg['Subject']
-    ...     print "To: %s" % ', '.join(to_addrs)
-    ...     print
-    ...     print error_mail.get_payload(decode=True)
     >>> print_latest_email()
     Subject: Submit Request Failure
     To: test@xxxxxxxxxxxxx