[PATCH 3/4] parser: Add 'X-Patchwork-Action-Required' header
Stephen Finucane
stephen at that.guru
Fri Aug 27 18:32:06 AEST 2021
On Fri, 2021-08-27 at 13:51 +1000, Daniel Axtens wrote:
> 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?
I've no idea. A quick scan through an old Patchwork archive mbox I have locally
suggests no but I can't speak for other lists. That said, part of the issue here
could simply be lack of awareness of the feature as much as anything else.
Perhaps we should try to make this feature more prominent in the docs?
> 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.
Evolution does make it pretty easy [1], as does mutt [1]. We could add these
links to the docs also, if it would help? I'll admit I haven't used either
myself though since I'm happy enough with 'git-pw' for my day-to-day work.
Stephen
[1] https://help.gnome.org/users/evolution/stable//mail-composer-custom-header-lines
[2] http://www.mutt.org/doc/manual/#ex-my-hdr
>
> >
> > 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