[PATCH 3/4] parser: Add 'X-Patchwork-Action-Required' header

Daniel Axtens dja at axtens.net
Fri Aug 27 13:51:42 AEST 2021


Stephen Finucane <stephen at that.guru> writes:

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

Hmm, do we have any evidence that any of our existing mail headers are
used by anything?

Also, I'm not confident I know how to set a header on a comment and I
write my email in emacs, notoriously one of the more configurable
platforms for any given task.

>
> Some minors issues are addressed in the docs while we're here.
>
> Signed-off-by: Stephen Finucane <stephen at that.guru>
> ---
>  docs/usage/headers.rst         | 17 ++++---
>  docs/usage/overview.rst        |  7 +++
>  patchwork/parser.py            | 10 +++-
>  patchwork/tests/test_parser.py | 89 ++++++++++++++++++++++++++++++++--
>  4 files changed, 111 insertions(+), 12 deletions(-)
>
> diff --git docs/usage/headers.rst docs/usage/headers.rst
> index 26e97c0a..b2b4b2d6 100644
> --- docs/usage/headers.rst
> +++ 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 at 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`.
> diff --git docs/usage/overview.rst docs/usage/overview.rst
> index a58acfa5..297569ec 100644
> --- docs/usage/overview.rst
> +++ 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
>  -----------
> diff --git patchwork/parser.py patchwork/parser.py
> index 61a81246..e6e1a7fb 100644
> --- patchwork/parser.py
> +++ 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')
>  
> diff --git patchwork/tests/test_parser.py patchwork/tests/test_parser.py
> index eaf6599c..d0c5c2d7 100644
> --- patchwork/tests/test_parser.py
> +++ patchwork/tests/test_parser.py
> @@ -18,6 +18,7 @@ from django.db.transaction import atomic
>  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 at 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 @@ class DelegateRequestTest(TestCase):
>          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'
> -- 
> 2.31.1


More information about the Patchwork mailing list