parser: Add 'X-Patchwork-Action-Required' header

Allow submitters to indicate that their comment is something that needs
to be addressed.

Some minors issues are addressed in the docs while we're here.

Signed-off-by: Stephen Finucane <stephen@that.guru>

docs/usage/headers.rst

@@ -5,8 +5,7 @@ Patchwork provides a number of special email headers to control how a patch is
 handled when it is received. The examples provided below use `git-send-email`,
 but custom headers can also be set when using tools like `mutt`.
 
-`X-Patchwork-Hint`
-
+``X-Patchwork-Hint``
   Valid values: ignore
 
   When set, this header will ensure the provided email is not parsed
@@ -16,8 +15,7 @@ but custom headers can also be set when using tools like `mutt`.
 
      $ git send-email --add-header="X-Patchwork-Hint: ignore" master
 
-`X-Patchwork-Delegate`
-
+``X-Patchwork-Delegate``
   Valid values: An email address associated with a Patchwork user
 
   If set and valid, the user corresponding to the provided email address will
@@ -28,8 +26,7 @@ but custom headers can also be set when using tools like `mutt`.
 
      $ git send-email --add-header="X-Patchwork-Delegate: a@example.com" master
 
-`X-Patchwork-State`
-
+``X-Patchwork-State``
   Valid values: Varies between deployments. This can usually be one of
   "Accepted", "Rejected", "RFC" or "Awaiting Upstream", among others.
 
@@ -39,3 +36,11 @@ but custom headers can also be set when using tools like `mutt`.
   .. code-block:: shell
 
      $ git send-email --add-header="X-Patchwork-State: RFC" master
+
+``X-Patchwork-Action-Required``
+  Valid values: <none, value is ignored>
+
+  When set on a reply to an existing cover letter or patch, this header will
+  mark the comment as "unaddressed". The comment can then be addressed using
+  the API or web UI. For more details, refer to
+  :ref:`overview-comment-metadata`.

docs/usage/overview.rst

@@ -181,6 +181,8 @@ system to test patches. Checks have a number of fields associated with them:
    to Patchwork.
 
 
+.. _overview-comment-metadata:
+
 Comment Metadata
 ----------------
 
@@ -198,6 +200,11 @@ the cover letters. Once the submitter has provided the required information,
 either the submitter or a maintainer can mark the comment as "addressed". This
 provides a more granular way of tracking work items than patch states.
 
+.. note::
+
+   Users can indicate that a comment requires an action using a custom mail
+   header. For more information, refer to :doc:`/usage/headers`.
+
 
 Collections
 -----------

patchwork/parser.py

@@ -1019,6 +1019,12 @@ def find_delegate_by_header(mail):
     return None
 
 
+def find_comment_addressed_by_header(mail):
+    """Determine whether a comment is actionable or not."""
+    # we dispose of the value - it's irrelevant
+    return False if 'X-Patchwork-Action-Required' in mail else None
+
+
 def parse_mail(mail, list_id=None):
     """Parse a mail and add to the database.
 
@@ -1278,6 +1284,7 @@ def parse_mail(mail, list_id=None):
     patch = find_patch_for_comment(project, refs)
     if patch:
         author = get_or_create_author(mail, project)
+        addressed = find_comment_addressed_by_header(mail)
 
         with transaction.atomic():
             if PatchComment.objects.filter(patch=patch, msgid=msgid):
@@ -1288,7 +1295,8 @@ def parse_mail(mail, list_id=None):
                 date=date,
                 headers=headers,
                 submitter=author,
-                content=message)
+                content=message,
+                addressed=addressed)
 
         logger.debug('Comment saved')
 

patchwork/tests/test_parser.py

@@ -18,6 +18,7 @@
 from django.db import connection
 
 from patchwork.models import Cover
+from patchwork.models import CoverComment
 from patchwork.models import Patch
 from patchwork.models import PatchComment
 from patchwork.models import Person
@@ -68,22 +69,42 @@ def read_mail(filename, project=None):
     return mail
 
 
-def _create_email(msg, msgid=None, sender=None, listid=None, in_reply_to=None):
+def _create_email(
+    msg,
+    msgid=None,
+    subject=None,
+    sender=None,
+    listid=None,
+    in_reply_to=None,
+    headers=None,
+):
     msg['Message-Id'] = msgid or make_msgid()
-    msg['Subject'] = 'Test subject'
+    msg['Subject'] = subject or 'Test subject'
     msg['From'] = sender or 'Test Author <test-author@example.com>'
     msg['List-Id'] = listid or 'test.example.com'
+
     if in_reply_to:
         msg['In-Reply-To'] = in_reply_to
 
+    for header in headers or {}:
+        msg[header] = headers[header]
+
     return msg
 
 
-def create_email(content, msgid=None, sender=None, listid=None,
-                 in_reply_to=None):
+def create_email(
+    content,
+    msgid=None,
+    subject=None,
+    sender=None,
+    listid=None,
+    in_reply_to=None,
+    headers=None,
+):
     msg = MIMEText(content, _charset='us-ascii')
 
-    return _create_email(msg, msgid, sender, listid, in_reply_to)
+    return _create_email(
+        msg, msgid, subject, sender, listid, in_reply_to, headers)
 
 
 def parse_mail(*args, **kwargs):
@@ -821,6 +842,64 @@ def test_invalid_delegate(self):
         self.assertDelegate(None)
 
 
+class CommentActionRequiredTest(TestCase):
+
+    fixtures = ['default_tags']
+
+    def setUp(self):
+        self.project = create_project(listid='test.example.com')
+
+    def _create_submission_and_comments(self, submission_email):
+        comment_a_email = create_email(
+            'test comment\n',
+            in_reply_to=submission_email['Message-Id'],
+            listid=self.project.listid,
+            headers={},
+        )
+        comment_b_email = create_email(
+            'another test comment\n',
+            in_reply_to=submission_email['Message-Id'],
+            listid=self.project.listid,
+            headers={'X-Patchwork-Action-Required': ''},
+        )
+        parse_mail(submission_email)
+        parse_mail(comment_a_email)
+        parse_mail(comment_b_email)
+
+        comment_a_msgid = comment_a_email.get('Message-ID')
+        comment_b_msgid = comment_b_email.get('Message-ID')
+
+        return comment_a_msgid, comment_b_msgid
+
+    def test_patch_comment(self):
+        body = read_patch('0001-add-line.patch')
+        patch_email = create_email(body, listid=self.project.listid)
+        comment_a_msgid, comment_b_msgid = \
+            self._create_submission_and_comments(patch_email)
+
+        self.assertEqual(1, Patch.objects.count())
+        self.assertEqual(2, PatchComment.objects.count())
+        comment_a = PatchComment.objects.get(msgid=comment_a_msgid)
+        self.assertIsNone(comment_a.addressed)
+        comment_b = PatchComment.objects.get(msgid=comment_b_msgid)
+        self.assertFalse(comment_b.addressed)
+
+    def test_cover_comment(self):
+        cover_email = create_email(
+            'test cover letter',
+            subject='[0/2] A cover letter',
+            listid=self.project.listid)
+        comment_a_msgid, comment_b_msgid = \
+            self._create_submission_and_comments(cover_email)
+
+        self.assertEqual(1, Cover.objects.count())
+        self.assertEqual(2, CoverComment.objects.count())
+        comment_a = CoverComment.objects.get(msgid=comment_a_msgid)
+        self.assertIsNone(comment_a.addressed)
+        comment_b = CoverComment.objects.get(msgid=comment_b_msgid)
+        self.assertFalse(comment_b.addressed)
+
+
 class InitialPatchStateTest(TestCase):
 
     patch_filename = '0001-add-line.patch'