issue2551232 - modify in-reply-to threading when multiple matches

if an email is missing an issue designator, in-reply-to threading is
attempted.  In this change if in-reply-to threading matches multiple
issues, fall back to matching on subject. It used to just arbitrairly
choose the first matching issue.

Author: rouilj

CHANGES.txt

@@ -45,6 +45,10 @@ Fixed:
   connection is open. (John Rouillard)
 - fix crash if postgresql native-fts backend is asked to index content
   with null bytes. (John Rouillard)
+- issue2551232 - modify in-reply-to threading when multiple matches
+  Change how in-reply-to threading works in the mailgw. If there is
+  more than one issue with a matching parent message, fall back to
+  subject matching. See upgrading.txt for details. (John Rouillard)
 
 Features:
 

doc/upgrading.txt

@@ -169,6 +169,30 @@ are not used.)
 For details on WAL mode see `<https://www.sqlite.org/wal.html>`_
 and `<https://www.sqlite.org/pragma.html#pragma_journal_mode>`_.
 
+Change in processing of In-Reply_to email header
+------------------------------------------------
+
+Messages received via email usually include a ``[issue23]``
+designator in the subject line. This indicates what issue is
+being updated. If the designator is missing, Roundup tries
+to find the correct issue by using the in-reply-to email
+header.
+
+The former code appends the new message to the first issue
+found with a message matching the in-reply-to
+header. Usually a message is associated with only one
+issue. However nothing in Roundup requires that.
+
+In this release, the in-reply-to matching is disabled if
+there are multiple issues with the same message. In this
+case, subject matching is used to try to find the matching
+issue.
+
+If you don't have messages assigned to multiple issues you
+will see no change. If you do have multi-linked messages
+this will hopefully result in better message->issue
+matching.
+
 .. index:: Upgrading; 2.1.0 to 2.2.0
 
 Migrating from 2.1.0 to 2.2.0

roundup/mailgw.py

@@ -773,29 +773,36 @@ def get_nodeid(self):
             nodeid = self.matches['nodeid']
 
         # try in-reply-to to match the message if there's no nodeid
-        # FIXME: possible crash if message linked to multiple issues
-        #    Use the in-reply-to of the current message to find an id
-        #    for the message being replied to.
-        #    Then search the current class (probably issue) for an issue
-        #    that has the parent_message id in the issue's messages
-        #    property. Then use this id as the node to update. HOWEVER if
-        #    the reply to message is linked to multiple issues, I think
-        #    this blows up.
-        #    Linking a message to multiple issues can be used to group
-        #    issues so that an update on a child issue is also reflected
-        #    on a parent issue. As the parent and child may have different
-        #    nosy/watchers.
-
+        # If there are multiple matches for the in-reply-to, fall back
+        # to title/subject match.
         inreplyto = self.message.get_header('in-reply-to') or ''
         if nodeid is None and inreplyto:
             parent_message = self.db.getclass('msg').stringFind(
                 messageid=inreplyto)
-            # FIXME: if a message is linked to multiple issues, can nodeid
-            # be a list? If so, will this crash??
             if parent_message:
                 nodeid = self.cl.filter(None,
-                                        {'messages': parent_message})[0]
-
+                                        {'messages': parent_message})
+                if len(nodeid) == 1:
+                    nodeid = nodeid[0]
+                elif nodeid:   # len(nodeid) > 1
+                    # This message is responding to a message
+                    # we know about. But there is more than 1 issue
+                    # associated with it.
+                    # Before bouncing it or creating a new issue,
+                    # force it to be treated as a reply even if the Subject
+                    # is missing 'Re:'
+                    # Note that multiple issues may be matched by
+                    #   Subject as well. The code chooses the most
+                    #   recently updated.  Hopefully Subjects have
+                    #   less of a chance of collision. Possible future
+                    #   idea filter ids that match subject by id's
+                    #   that match in-reply-to and choose newest
+                    #   match. Not sure if this would work better in
+                    #   production, so not implementing now.
+                    nodeid = None
+                    # trigger Subject match
+                    self.matches['refwd'] = True
+                
         # but we do need either a title or a nodeid...
         if nodeid is None and not title:
             raise MailUsageError(_("""

test/test_mailgw.py

@@ -4204,6 +4204,60 @@ def testReplytoMatch(self):
         self.assertEqual(nodeid, nodeid2)
         self.assertEqual(nodeid, nodeid3)
 
+
+    def testReplytoMultiMatch(self):
+        """ If an in reply-to header matches more than 1 issue:
+            Try a subject match, if that fails create a new issue.
+        """
+
+        # create two issues with the same initial message/messgage-id.
+        nodeid1 = self.doNewIssue()
+        nodeid2 = self.doNewIssue()
+        
+        # set unique title/subject for second issue.
+        self.db.issue.set("2", title="Testing1...")
+
+        # Send an email that will match both issue1 and issue2 by
+        # in-reply-to. As a result we fall back to Subject match, but
+        # the Subject doesn't match issue1 or 2. So it creates a new
+        # issue.
+        nodeid3 = self._handle_mail('''Content-Type: text/plain;
+  charset="iso-8859-1"
+From: Chef <[email protected]>
+To: [email protected]
+Message-Id: <dummy_test_message_id2>
+In-Reply-To: <dummy_test_message_id>
+Subject: Testing2...
+
+Followup message.
+''')
+        # this will be added to issue3 because of in-reply-to.
+        nodeid4 = self._handle_mail('''Content-Type: text/plain;
+  charset="iso-8859-1"
+From: Chef <[email protected]>
+To: [email protected]
+Message-Id: <dummy_test_message_id3>
+In-Reply-To: <dummy_test_message_id2>
+Subject: Testing...
+
+Yet another message in the same thread/issue.
+''')
+
+        # this message gets added to issue 2 by subject match.
+        nodeid5 = self._handle_mail('''Content-Type: text/plain;
+  charset="iso-8859-1"
+From: Chef <[email protected]>
+To: [email protected]
+Message-Id: <dummy_test_message_id4>
+In-Reply-To: <dummy_test_message_id>
+Subject: Testing1...
+
+Yet another message in the same thread/issue.
+''')
+
+        self.assertEqual(nodeid3, nodeid4)
+        self.assertEqual(nodeid2, nodeid5)
+
     def testHelpSubject(self):
         message = '''Content-Type: text/plain;
   charset="iso-8859-1"