parser: fix wrong parsing of diff comments

If the subject of a submission is prefixed by 'Re:', then it can't be a
patch or cover letter.

Signed-off-by: WEN Pingbo <wengpingbo@gmail.com>
Reviewed-by: Stephen Finucane <stephenfinucane@hotmail.com>

diff --git a/patchwork/parser.py b/patchwork/parser.py
index 0e229ba921bf5133cacca3ab534653bcb7ca9891..7c05479b33bc5663488c3ffee1b3937da0b12acd 100644 (file)
--- a/patchwork/parser.py
+++ b/patchwork/parser.py
@@ -353,6 +353,13 @@ def clean_subject(subject, drop_prefixes=None):
     return (subject, prefixes)
 
 
+def subject_check(subject):
+    """Determine if a mail is a reply."""
+    comment_re = re.compile(r'^(re)[:\s]\s*', re.I)
+
+    return comment_re.match(subject)
+
+
 def clean_content(content):
     """Remove cruft from the email message.
 
@@ -619,7 +626,9 @@ def parse_mail(mail, list_id=None):
 
     msgid = mail.get('Message-Id').strip()
     author = find_author(mail)
-    name, prefixes = clean_subject(mail.get('Subject'), [project.linkname])
+    subject = mail.get('Subject')
+    name, prefixes = clean_subject(subject, [project.linkname])
+    is_comment = subject_check(subject)
     x, n = parse_series_marker(prefixes)
     refs = find_references(mail)
     date = find_date(mail)
@@ -628,7 +637,7 @@ def parse_mail(mail, list_id=None):
 
     # build objects
 
-    if diff or pull_url:  # patches or pull requests
+    if not is_comment and (diff or pull_url):  # patches or pull requests
         # we delay the saving until we know we have a patch.
         author.save()
 
@@ -658,17 +667,18 @@ def parse_mail(mail, list_id=None):
         # however, we need to see if a match already exists and, if
         # not, assume that it is indeed a new cover letter
         is_cover_letter = False
-        if not refs == []:
-            try:
-                CoverLetter.objects.all().get(name=name)
-            except CoverLetter.DoesNotExist:
-                # if no match, this is a new cover letter
+        if not is_comment:
+            if not refs == []:
+                try:
+                    CoverLetter.objects.all().get(name=name)
+                except CoverLetter.DoesNotExist:
+                    # if no match, this is a new cover letter
+                    is_cover_letter = True
+                except CoverLetter.MultipleObjectsReturned:
+                    # if multiple cover letters are found, just ignore
+                    pass
+            else:
                 is_cover_letter = True
-            except CoverLetter.MultipleObjectsReturned:
-                # if multiple cover letters are found, just ignore
-                pass
-        else:
-            is_cover_letter = True
 
         if is_cover_letter:
             author.save()
@@ -693,6 +703,9 @@ def parse_mail(mail, list_id=None):
     if not submission:
         return
 
+    if is_comment and diff:
+        message += diff
+
     author.save()
 
     comment = Comment(

diff --git a/patchwork/tests/test_parser.py b/patchwork/tests/test_parser.py
index 9a739e4d16ce980fe77e95edb3e465dd6685315a..496818dc4fd42506eff08419615c47fc8982cf90 100644 (file)
--- a/patchwork/tests/test_parser.py
+++ b/patchwork/tests/test_parser.py
@@ -38,6 +38,7 @@ from patchwork.parser import parse_mail as _parse_mail
 from patchwork.parser import parse_pull_request
 from patchwork.parser import parse_series_marker
 from patchwork.parser import split_prefixes
+from patchwork.parser import subject_check
 from patchwork.tests.utils import create_project
 from patchwork.tests.utils import create_state
 from patchwork.tests.utils import create_user
@@ -706,3 +707,11 @@ class SubjectTest(TestCase):
                          ('[bar] meep', ['bar']))
         self.assertEqual(clean_subject('[FOO] [bar] meep', ['foo']),
                          ('[bar] meep', ['bar']))
+
+    def test_subject_check(self):
+        self.assertIsNotNone(subject_check('RE: meep'))
+        self.assertIsNotNone(subject_check('Re: meep'))
+        self.assertIsNotNone(subject_check('re: meep'))
+        self.assertIsNotNone(subject_check('RE meep'))
+        self.assertIsNotNone(subject_check('Re meep'))
+        self.assertIsNotNone(subject_check('re meep'))