launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging lp:~cjwatson/launchpad/bug-self-references into lp:launchpad.

Commit message:
Don't include a References header in notifications for a bug's initial message.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #320034 in Launchpad itself: "Launchpad mail notifications may use References header referencing the same email : confuses Thunderbird threading in < 3.0"
  https://bugs.launchpad.net/launchpad/+bug/320034

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/bug-self-references/+merge/263944

Don't include a References header in notifications for a bug's initial message.  This violates RFC2822 3.6.4 (or at least any sensible reading of it), is silly, and has been known to confuse some MUAs in the past.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/bug-self-references into lp:launchpad.

=== modified file 'lib/lp/bugs/doc/bugnotification-threading.txt'
--- lib/lp/bugs/doc/bugnotification-threading.txt	2012-10-15 03:31:26 +0000
+++ lib/lp/bugs/doc/bugnotification-threading.txt	2015-07-06 17:30:20 +0000
@@ -123,3 +123,33 @@
     >>> comment.rfc822msgid in references
     True
 
+Create a new bug, fetching the notification manually since it will not yet
+be ready to send.  The notification sent for this should not have any
+References header.
+
+    >>> from lp.bugs.interfaces.bug import CreateBugParams
+    >>> from lp.bugs.model.bugnotification import BugNotification
+    >>> from lp.services.database.interfaces import IStore
+
+    >>> for notification in notifications:
+    ...     notification.date_emailed = datetime.now(pytz.timezone('UTC'))
+    >>> flush_database_updates()
+
+    >>> params = CreateBugParams(
+    ...     owner=sample_person, title="New bug", comment="New bug.",
+    ...     target=bug_one.default_bugtask.target)
+    >>> bug = getUtility(IBugSet).createBug(params)
+    >>> notifications = IStore(BugNotification).find(
+    ...     BugNotification, BugNotification.bug == bug)
+    >>> messages = [emails for notifications, omitted, emails in
+    ...     get_email_notifications(notifications)]
+    >>> len(messages)
+    1
+    >>> emails = messages[0]
+    >>> len(emails)
+    1
+    >>> notification = emails[0]
+    >>> notification['Message-Id'] == bug.initial_message.rfc822msgid
+    True
+    >>> 'References' in notification
+    False

=== modified file 'lib/lp/bugs/mail/bugnotificationbuilder.py'
--- lib/lp/bugs/mail/bugnotificationbuilder.py	2015-03-13 19:05:50 +0000
+++ lib/lp/bugs/mail/bugnotificationbuilder.py	2015-07-06 17:30:20 +0000
@@ -183,7 +183,7 @@
         for header in self.common_headers:
             message.add_header(*header)
 
-        if references is not None:
+        if references:
             message['References'] = ' '.join(references)
         if message_id is not None:
             message['Message-Id'] = message_id

=== modified file 'lib/lp/bugs/scripts/bugnotification.py'
--- lib/lp/bugs/scripts/bugnotification.py	2013-01-07 02:40:55 +0000
+++ lib/lp/bugs/scripts/bugnotification.py	2015-07-06 17:30:20 +0000
@@ -164,7 +164,8 @@
         text = notification.message.text_contents.rstrip()
         text_notifications.append(text)
 
-    if bug.initial_message.rfc822msgid not in references:
+    if (bug.initial_message.rfc822msgid not in references and
+            msgid != bug.initial_message.rfc822msgid):
         # Ensure that references contain the initial message ID
         references.insert(0, bug.initial_message.rfc822msgid)