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 <step...@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...@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-aut...@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 _______________________________________________ Patchwork mailing list Patchwork@lists.ozlabs.org https://lists.ozlabs.org/listinfo/patchwork