launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging ~cjwatson/launchpad:py3-send-direct-contact-email into launchpad:master.

Commit message:
Fix send_direct_contact_email for Python 3

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/+git/launchpad/+merge/398389

On Python 3, `email.header.Header.__str__` returns a Unicode approximation of the header rather than an RFC-2047-encoded form.  Use `email.header.Header.encode` instead.

While we're here, also fix the reasons produced by ContactViaWebNotificationRecipientSet to avoid including embedded newlines, since Python 3's email.header.Header.encode raises an exception on those.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:py3-send-direct-contact-email into launchpad:master.

diff --git a/lib/lp/registry/browser/person.py b/lib/lp/registry/browser/person.py
index c07a1c2..31c29ed 100644
--- a/lib/lp/registry/browser/person.py
+++ b/lib/lp/registry/browser/person.py
@@ -4134,13 +4134,13 @@ class ContactViaWebNotificationRecipientSet:
         """
         if self.primary_reason is self.TO_USER:
             reason = (
-                'using the "Contact this user" link on your profile page\n'
+                'using the "Contact this user" link on your profile page '
                 '(%s)' % canonical_url(person_or_team))
             header = 'ContactViaWeb user'
         elif self.primary_reason is self.TO_ADMINS:
             reason = (
                 'using the "Contact this team\'s admins" link on the '
-                '%s team page\n(%s)' % (
+                '%s team page (%s)' % (
                     person_or_team.displayname,
                     canonical_url(person_or_team)))
             header = 'ContactViaWeb owner (%s team)' % person_or_team.name
@@ -4148,7 +4148,7 @@ class ContactViaWebNotificationRecipientSet:
             # self.primary_reason is self.TO_MEMBERS.
             reason = (
                 'to each member of the %s team using the '
-                '"Contact this team" link on the %s team page\n(%s)' % (
+                '"Contact this team" link on the %s team page (%s)' % (
                     person_or_team.displayname,
                     person_or_team.displayname,
                     canonical_url(person_or_team)))
diff --git a/lib/lp/registry/browser/tests/test_person_contact.py b/lib/lp/registry/browser/tests/test_person_contact.py
index d9f1f8b..85ccfdd 100644
--- a/lib/lp/registry/browser/tests/test_person_contact.py
+++ b/lib/lp/registry/browser/tests/test_person_contact.py
@@ -140,7 +140,7 @@ class ContactViaWebNotificationRecipientSetTestCase(TestCaseWithFactory):
         for email, recipient in recipient_set.getRecipientPersons():
             reason, rationale = recipient_set.getReason(email)
         self.assertEqual(
-            'using the "Contact this user" link on your profile page\n'
+            'using the "Contact this user" link on your profile page '
             '(http://launchpad.test/~pting)',
             reason)
         self.assertEqual('ContactViaWeb user', rationale)
@@ -153,7 +153,7 @@ class ContactViaWebNotificationRecipientSetTestCase(TestCaseWithFactory):
             reason, rationale = recipient_set.getReason(email)
         self.assertEqual(
             'using the "Contact this team\'s admins" link '
-            'on the Pting team page\n'
+            'on the Pting team page '
             '(http://launchpad.test/~pting)',
             reason)
         self.assertEqual('ContactViaWeb owner (pting team)', rationale)
@@ -166,7 +166,7 @@ class ContactViaWebNotificationRecipientSetTestCase(TestCaseWithFactory):
             reason, rationale = recipient_set.getReason(email)
         self.assertEqual(
             'to each member of the Pting team using the '
-            '"Contact this team" link on the Pting team page\n'
+            '"Contact this team" link on the Pting team page '
             '(http://launchpad.test/~pting)',
             reason)
         self.assertEqual('ContactViaWeb member (pting team)', rationale)
diff --git a/lib/lp/registry/mail/notification.py b/lib/lp/registry/mail/notification.py
index e4598b1..cc60a1a 100644
--- a/lib/lp/registry/mail/notification.py
+++ b/lib/lp/registry/mail/notification.py
@@ -140,12 +140,12 @@ def notify_message_held(message_approval, event):
         simple_sendmail(from_address, address, subject, body)
 
 
-def encode(value):
-    """Encode string for transport in a mail header.
+def make_header(value):
+    """Prepare string for transport in a mail header.
 
     :param value: The raw email header value.
     :type value: unicode
-    :return: The encoded header.
+    :return: The header.
     :rtype: `email.header.Header`
     """
     try:
@@ -175,7 +175,7 @@ def send_direct_contact_email(
     """
     # Craft the email message.  Start by checking whether the subject and
     # message bodies are ASCII or not.
-    subject_header = encode(subject)
+    subject_header = make_header(subject)
     try:
         body.encode('us-ascii')
         charset = 'us-ascii'
@@ -185,7 +185,7 @@ def send_direct_contact_email(
     person_set = getUtility(IPersonSet)
     sender = person_set.getByEmail(sender_email)
     assert sender is not None, 'No person for sender %s' % sender_email
-    sender_name = str(encode(sender.displayname))
+    sender_name = make_header(sender.displayname).encode()
     # Do a single authorization/quota check for the sender.  We consume one
     # quota credit per contact, not per recipient.
     authorization = IDirectEmailAuthorization(sender)
@@ -210,9 +210,9 @@ def send_direct_contact_email(
     mailwrapper = MailWrapper(width=72)
     message = None
     for recipient_email, recipient in recipients_set.getRecipientPersons():
-        recipient_name = str(encode(recipient.displayname))
+        recipient_name = make_header(recipient.displayname).encode()
         reason, rationale_header = recipients_set.getReason(recipient_email)
-        reason = str(encode(reason)).replace('\n ', '\n')
+        reason = make_header(reason).encode().replace('\n ', '\n')
         formatted_body = mailwrapper.format(body, force_wrap=True)
         formatted_body += additions % reason
         formatted_body = formatted_body.encode(charset)