launchpad-reviewers team mailing list archive

[Curtis Hovey <curtis.hovey@xxxxxxxxxxxxx>]

Curtis Hovey has proposed merging lp:~sinzui/launchpad/non-active-user-bug-mail into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #38162 in Launchpad itself: "People without preferred email addresses shouldn't be able to use the email interface"
  https://bugs.launchpad.net/launchpad/+bug/38162

For more details, see:
https://code.launchpad.net/~sinzui/launchpad/non-active-user-bug-mail/+merge/77004

Send the bug-mail help instructions to non-active users.

    Launchpad bug: https://bugs.launchpad.net/bugs/38162
    Pre-implementation: No one

If a person doesn't have a preferred email address, he can't use
Launchpad via the web UI before setting the address. He can however use
Launchpad via the email interface, which causes some subtle bug.

If a person without a preferred email address sends an email to the
email interface, he should get a message explaining what he has to do
in order to use it.

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

RULES

    * We already have an email that explains how to use bug mail, but the
      block is executed near the end of the loop setup. Move the help
      block to the first step.
    * There are already rules in the send help email block to get the user
      and preferred email address. They can be moved before the block to
      indicate if the help block should be entered.


QA

    * Send an email from a registered address of a user without a preferred
      email address (inactive, deactivated, suspended).
    * Verify that a help email is in the staging inbox to the user.


LINT

    lib/lp/bugs/mail/handler.py
    lib/lp/bugs/mail/tests/test_handler.py


IMPLEMENTATION

Enter the help block when the email is addressed to help or there
is no preferred email address. If the email address is unknown to Lp (there
is no user), the existing rule to drip the email applies. When there is a
user without a preferred email address, help is sent to the the from address.
    lib/lp/bugs/mail/handler.py
    lib/lp/bugs/mail/tests/test_handler.py
-- 
https://code.launchpad.net/~sinzui/launchpad/non-active-user-bug-mail/+merge/77004
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~sinzui/launchpad/non-active-user-bug-mail into lp:launchpad.

=== modified file 'lib/lp/bugs/mail/handler.py'
--- lib/lp/bugs/mail/handler.py	2011-09-18 08:43:52 +0000
+++ lib/lp/bugs/mail/handler.py	2011-09-26 17:59:27 +0000
@@ -19,6 +19,7 @@
 from zope.interface import implements
 
 from canonical.launchpad.helpers import get_email_template
+from canonical.launchpad.interfaces.emailaddress import IEmailAddressSet
 from canonical.launchpad.mailnotification import (
     MailWrapper,
     send_process_error_notification,
@@ -209,10 +210,22 @@
         commands = self.getCommands(signed_msg)
         to_user, to_host = to_addr.split('@')
         add_comment_to_bug = False
+        from_user = getUtility(ILaunchBag).user
+        if to_user.lower() == 'help' or from_user is None:
+            if from_user is not None and from_user.preferredemail is not None:
+                to_address = str(from_user.preferredemail.email)
+            else:
+                to_address = signed_msg['From']
+                address = getUtility(IEmailAddressSet).getByEmail(to_address)
+                if address is None:
+                    to_address = None
+            if to_address is not None:
+                self.sendHelpEmail(to_address)
+            return True, False, None
         # If there are any commands, we must have strong authentication.
         # We send a different failure message for attempts to create a new
         # bug.
-        if to_user.lower() == 'new':
+        elif to_user.lower() == 'new':
             ensure_not_weakly_authenticated(signed_msg, CONTEXT,
                 'unauthenticated-bug-creation.txt',
                 error_templates=error_templates)
@@ -228,14 +241,6 @@
             # the bug.
             add_comment_to_bug = True
             commands.insert(0, BugEmailCommands.get('bug', [to_user]))
-        elif to_user.lower() == 'help':
-            from_user = getUtility(ILaunchBag).user
-            if from_user is not None:
-                preferredemail = from_user.preferredemail
-                if preferredemail is not None:
-                    to_address = str(preferredemail.email)
-                    self.sendHelpEmail(to_address)
-            return True, False, None
         elif to_user.lower() != 'edit':
             # Indicate that we didn't handle the mail.
             return False, False, None

=== modified file 'lib/lp/bugs/mail/tests/test_handler.py'
--- lib/lp/bugs/mail/tests/test_handler.py	2011-08-29 20:04:54 +0000
+++ lib/lp/bugs/mail/tests/test_handler.py	2011-09-26 17:59:27 +0000
@@ -19,6 +19,7 @@
 from canonical.config import config
 from canonical.database.sqlbase import commit
 from canonical.launchpad.ftests import import_secret_test_key
+from canonical.launchpad.interfaces.emailaddress import EmailAddressStatus
 from canonical.launchpad.webapp.authorization import LaunchpadSecurityPolicy
 from canonical.testing.layers import (
     LaunchpadFunctionalLayer,
@@ -111,10 +112,28 @@
             'affects malone',
             ])
 
+    def test_mailToHelpFromNonActiveUser(self):
+        """Mail from people without a preferred email get a help message."""
+        self.factory.makePerson(
+            email='non@xxxxxx',
+            email_address_status=EmailAddressStatus.NEW)
+        message = self.factory.makeSignedMessage(email_address='non@xxxxxx')
+        handler = MaloneHandler()
+        response = handler.extractAndAuthenticateCommands(
+            message, 'help@xxxxxxxxxxxxxxxxxx')
+        mail_handled, add_comment_to_bug, commands = response
+        self.assertEquals(mail_handled, True)
+        emails = self.getSentMail()
+        self.assertEquals(1, len(emails))
+        self.assertEquals(['non@xxxxxx'], emails[0][1])
+        self.assertTrue(
+            'Subject: Launchpad Bug Tracker Email Interface' in emails[0][2])
+
     def test_mailToHelpFromUnknownUser(self):
         """Mail from people of no account to help@ is simply dropped.
         """
-        message = self.factory.makeSignedMessage()
+        message = self.factory.makeSignedMessage(
+            email_address='unregistered@xxxxxx')
         handler = MaloneHandler()
         mail_handled, add_comment_to_bug, commands = \
             handler.extractAndAuthenticateCommands(message,
@@ -124,15 +143,19 @@
 
     def test_mailToHelp(self):
         """Mail to help@ generates a help command."""
-        message = self.factory.makeSignedMessage()
+        user = self.factory.makePerson(email='user@xxxxxx')
+        message = self.factory.makeSignedMessage(email_address='user@xxxxxx')
         handler = MaloneHandler()
-        with person_logged_in(self.factory.makePerson()):
+        with person_logged_in(user):
             mail_handled, add_comment_to_bug, commands = \
                 handler.extractAndAuthenticateCommands(message,
                     'help@xxxxxxxxxxxxxxxxxx')
         self.assertEquals(mail_handled, True)
-        self.assertEquals(len(self.getSentMail()), 1)
-        # TODO: Check the right mail was sent. -- mbp 20100923
+        emails = self.getSentMail()
+        self.assertEquals(1, len(emails))
+        self.assertEquals([message['From']], emails[0][1])
+        self.assertTrue(
+            'Subject: Launchpad Bug Tracker Email Interface' in emails[0][2])
 
     def getSentMail(self):
         # Sending mail is (unfortunately) a side effect of parsing the