diff mbox series

[v2,1/5] api: add addressed field and detail endpoint for patch comments

Message ID 20210728181718.3613124-2-raxel@google.com
State Changes Requested
Headers show
Series patch-detail: add unaddressed/addressed status to patch comments | expand
Related show

Commit Message

Raxel Gutierrez July 28, 2021, 6:17 p.m. UTC 
Add addressed boolean field to PatchComment to be able to distinguish
between them in the patch-detail page. Change PatchComment to have same
is_editable from the related patch with the addition of being editable
by the user who submitted the comment.

Add endpoint for patch comments at api/.../comments/<comment_id>.
The endpoint will make it possible to use the REST API to update the new
`addressed` field for patch comments with JavaScript on the client side.
In the process of these changes, clean up use of CurrentPatchDefault so
that it exists in base.py and can be used throughout the API.

Add the OpenAPI definition of the new endpoint and upgrade API version
to v1.3 to reflect the minor change. In order for tests to pass, clean
up test_comment.py to reflect the change from the <pk> to <patch_id>
parameter for the `api-patch-comment-list` endpoint.

Signed-off-by: Raxel Gutierrez <raxel@google.com>
---
 docs/api/schemas/generate-schemas.py          |    4 +-
 docs/api/schemas/latest/patchwork.yaml        |  144 +-
 docs/api/schemas/patchwork.j2                 |  148 +-
 docs/api/schemas/v1.0/patchwork.yaml          |   35 +-
 docs/api/schemas/v1.1/patchwork.yaml          |   35 +-
 docs/api/schemas/v1.2/patchwork.yaml          |   35 +-
 docs/api/schemas/v1.3/patchwork.yaml          | 2749 +++++++++++++++++
 patchwork/api/base.py                         |   24 +-
 patchwork/api/check.py                        |   21 +-
 patchwork/api/comment.py                      |   76 +-
 patchwork/api/patch.py                        |    2 +-
 .../migrations/0045_patchcomment_addressed.py |   18 +
 patchwork/models.py                           |    5 +-
 patchwork/tests/api/test_comment.py           |    4 +-
 patchwork/urls.py                             |   17 +-
 15 files changed, 3256 insertions(+), 61 deletions(-)
 create mode 100644 docs/api/schemas/v1.3/patchwork.yaml
 create mode 100644 patchwork/migrations/0045_patchcomment_addressed.py

Comments

Emily Shaffer Aug. 3, 2021, 1:13 a.m. UTC | #1 
On Wed, Jul 28, 2021 at 06:17:14PM +0000, Raxel Gutierrez wrote:
> 
> Add addressed boolean field to PatchComment to be able to distinguish
> between them in the patch-detail page. Change PatchComment to have same
> is_editable from the related patch with the addition of being editable
> by the user who submitted the comment.
> 
> Add endpoint for patch comments at api/.../comments/<comment_id>.
> The endpoint will make it possible to use the REST API to update the new
> `addressed` field for patch comments with JavaScript on the client side.
> In the process of these changes, clean up use of CurrentPatchDefault so
> that it exists in base.py and can be used throughout the API.
> 
> Add the OpenAPI definition of the new endpoint and upgrade API version
> to v1.3 to reflect the minor change. In order for tests to pass, clean
> up test_comment.py to reflect the change from the <pk> to <patch_id>
> parameter for the `api-patch-comment-list` endpoint.

This is a pretty good example of what jrnieder and I mean when we're
talking about using the commit message to describe things about the
patch you cannot learn by examining the diff. In each paragraph of your
commit-msg, you've told us something about the diff - but haven't really
explained why you made that change. This commit message also lacks an
explanation of motivation. Why are you adding this field? What is the
ultimate goal which justifies incrementing the API version? This commit
message doesn't say anywhere that this is setup for tracking state on
comments.

Your cover letter *does* do a good job of that - but your cover letter
won't end up in 'git log'.

> diff --git a/docs/api/schemas/generate-schemas.py b/docs/api/schemas/generate-schemas.py
> index a0c1e45..3a436a1 100755
> --- a/docs/api/schemas/generate-schemas.py
> +++ b/docs/api/schemas/generate-schemas.py
> @@ -14,8 +14,8 @@ except ImportError:
>      yaml = None
>  
>  ROOT_DIR = os.path.dirname(os.path.realpath(__file__))
> -VERSIONS = [(1, 0), (1, 1), (1, 2), None]
> -LATEST_VERSION = (1, 2)
> +VERSIONS = [(1, 0), (1, 1), (1, 2), (1, 3), None]
> +LATEST_VERSION = (1, 3)

Ok, we are incrementing the API version here. It's necessary because
we're adding a new REST endpoint. Sure.

> diff --git a/docs/api/schemas/latest/patchwork.yaml b/docs/api/schemas/latest/patchwork.yaml
> index a8910a7..95329c7 100644
> --- a/docs/api/schemas/latest/patchwork.yaml
> +++ b/docs/api/schemas/latest/patchwork.yaml
> @@ -13,7 +13,7 @@ info:
>    license:
>      name: GPL v2 License
>      url: https://www.gnu.org/licenses/gpl-2.0.html
> -  version: '1.2'
> +  version: '1.3'

...and here as well. Ok.

>  paths:
>    /api/:
>      get:
> @@ -598,14 +598,14 @@ paths:
>                  $ref: '#/components/schemas/Error'
>        tags:
>          - patches
> -  /api/patches/{id}/comments/:
> +  /api/patches/{patch_id}/comments/:
>      parameters:
>        - in: path
> -        name: id
> +        name: patch_id
>          description: A unique integer value identifying the parent patch.
>          required: true
>          schema:
> -          title: ID
> +          title: Patch ID

This bit isn't mentioned in the commit message that I see. But I think
you are changing the field name to disambiguate it from the other
'comment_id'. I actually don't know - does this rename make any
functional difference for API consumers, or are we just doing it to make
our own codebase less confusing?

>            type: integer
>      get:
>        description: List comments
> @@ -635,6 +635,110 @@ paths:
>                  $ref: '#/components/schemas/Error'
>        tags:
>          - comments
> +  /api/patches/{patch_id}/comments/{comment_id}/:
> +    parameters:
> +      - in: path
> +        name: patch_id
> +        description: A unique integer value identifying the parent patch.
> +        required: true
> +        schema:
> +          title: Patch ID
> +          type: integer
> +      - in: path
> +        name: comment_id
> +        description: A unique integer value identifying this comment.
> +        required: true
> +        schema:
> +          title: Comment ID
> +          type: integer
> +    get:
> +      description: Show a patch comment.
> +      operationId: patch_comments_read
> +      responses:
> +        '200':
> +          description: ''
> +          content:
> +            application/json:
> +              schema:
> +                $ref: '#/components/schemas/CommentDetail'
> +        '404':
> +          description: Not found
> +          content:
> +            application/json:
> +              schema:
> +                $ref: '#/components/schemas/Error'
> +      tags:
> +        - comments
> +    patch:
> +      description: Update a patch comment (partial).
> +      operationId: patch_comments_partial_update
> +#      security:
> +#        - basicAuth: []
> +#        - apiKeyAuth: []
> +      requestBody:
> +        $ref: '#/components/requestBodies/Comment'
> +      responses:
> +        '200':
> +          description: ''
> +          content:
> +            application/json:
> +              schema:
> +                $ref: '#/components/schemas/CommentDetail'
> +        '400':
> +          description: Invalid Request
> +          content:
> +            application/json:
> +              schema:
> +                $ref: '#/components/schemas/ErrorCommentUpdate'
> +        '403':
> +          description: Forbidden
> +          content:
> +            application/json:
> +              schema:
> +                $ref: '#/components/schemas/Error'
> +        '404':
> +          description: Not found
> +          content:
> +            application/json:
> +              schema:
> +                $ref: '#/components/schemas/Error'
> +      tags:
> +        - comments
> +    put:
> +      description: Update a patch comment.
> +      operationId: patch_comments_update
> +#      security:
> +#        - basicAuth: []
> +#        - apiKeyAuth: []
> +      requestBody:
> +        $ref: '#/components/requestBodies/Comment'
> +      responses:
> +        '200':
> +          description: ''
> +          content:
> +            application/json:
> +              schema:
> +                $ref: '#/components/schemas/CommentDetail'
> +        '400':
> +          description: Invalid Request
> +          content:
> +            application/json:
> +              schema:
> +                $ref: '#/components/schemas/ErrorCommentUpdate'
> +        '403':
> +          description: Forbidden
> +          content:
> +            application/json:
> +              schema:
> +                $ref: '#/components/schemas/Error'
> +        '404':
> +          description: Not found
> +          content:
> +            application/json:
> +              schema:
> +                $ref: '#/components/schemas/Error'
> +      tags:
> +        - comments

I've not got much to say on this bit but I'll acknowledge that I looked
at it and said "yep, that's YAML". Hopefully someone else has better
comments than me. ;)

>    /api/patches/{patch_id}/checks/:
>      parameters:
>        - in: path
> @@ -1242,6 +1346,12 @@ components:
>          application/x-www-form-urlencoded:
>            schema:
>              $ref: '#/components/schemas/CheckCreate'
> +    Comment:
> +      required: true
> +      content:
> +        application/json:
> +          schema:
> +            $ref: '#/components/schemas/CommentUpdate'

Is this requiring that there must be some comments present in order for
/api/patches/<patch id>/comments/ to work at all?

>      Patch:
>        required: true
>        content:
> @@ -1528,6 +1638,21 @@ components:
>                additionalProperties:
>                  type: string
>            readOnly: true
> +    CommentDetail:
> +      allOf:
> +        - $ref: '#/components/schemas/Comment'
> +        - properties:
> +            patch:
> +              $ref: '#/components/schemas/PatchEmbedded'
> +            addressed:
> +              title: Addressed
> +              type: boolean
> +    CommentUpdate:
> +      type: object
> +      properties:
> +        addressed:
> +          title: Addressed
> +          type: boolean
>      CoverList:
>        type: object
>        properties:
> @@ -1712,9 +1837,11 @@ components:
>                  previous_relation:
>                    title: Previous relation
>                    type: string
> +                  nullable: true
>                  current_relation:
>                    title: Current relation
>                    type: string
> +                  nullable: true

I'm curious why this is being added (and what it's being added to)?

> diff --git a/docs/api/schemas/patchwork.j2 b/docs/api/schemas/patchwork.j2

(I've not got anything useful to add here - this seems like basically
the same change as the file I commented on above.)

> diff --git a/docs/api/schemas/v1.0/patchwork.yaml b/docs/api/schemas/v1.0/patchwork.yaml
> diff --git a/docs/api/schemas/v1.1/patchwork.yaml b/docs/api/schemas/v1.1/patchwork.yaml
> diff --git a/docs/api/schemas/v1.2/patchwork.yaml b/docs/api/schemas/v1.2/patchwork.yaml

All these seem to be a) changing 'id' to 'patch_id' and b) adding the
extra comment stuff. But is it appropriate to do so for older API
versions?

> diff --git a/docs/api/schemas/v1.3/patchwork.yaml b/docs/api/schemas/v1.3/patchwork.yaml
> new file mode 100644
> index 0000000..2b100b4
> --- /dev/null
> +++ b/docs/api/schemas/v1.3/patchwork.yaml
> @@ -0,0 +1,2749 @@
> +# DO NOT EDIT THIS FILE. It is generated from a template. Changes should be
> +# proposed against the template and updated files generated using the
> +# 'generate-schemas.py' tool

...Or, I guess they were updated by the tool. Ok.
> diff --git a/patchwork/api/base.py b/patchwork/api/base.py

[snip]
I guess I see why you wanted to separate the testing from these schema
updates, then. Never mind my comment on the cover letter. :)

> --- a/patchwork/api/base.py
> +++ b/patchwork/api/base.py
> @@ -3,6 +3,7 @@
>  #
>  # SPDX-License-Identifier: GPL-2.0-or-later
>  
> +import rest_framework
>  
>  from django.conf import settings
>  from django.shortcuts import get_object_or_404
> @@ -15,6 +16,24 @@ from rest_framework.serializers import HyperlinkedModelSerializer
>  from patchwork.api import utils
>  
>  
> +DRF_VERSION = tuple(int(x) for x in rest_framework.__version__.split('.'))
> +
> +
> +if DRF_VERSION > (3, 11):
> +    class CurrentPatchDefault(object):
> +        requires_context = True
> +
> +        def __call__(self, serializer_field):
> +            return serializer_field.context['request'].patch
> +else:
> +    class CurrentPatchDefault(object):
> +        def set_context(self, serializer_field):
> +            self.patch = serializer_field.context['request'].patch
> +
> +        def __call__(self):
> +            return self.patch
> +
> +

I'm a little confused why this seems to be moved from check.py to
here... but I think that's because I don't know the codebase well ;)

>  class LinkHeaderPagination(PageNumberPagination):
>      """Provide pagination based on rfc5988.
>  
> @@ -44,7 +63,10 @@ class LinkHeaderPagination(PageNumberPagination):
>  
>  
>  class PatchworkPermission(permissions.BasePermission):
> -    """This permission works for Project and Patch model objects"""
> +    """
> +    This permission works for Project, Patch, and PatchComment
> +    model objects
> +    """
>      def has_object_permission(self, request, view, obj):
>          # read only for everyone
>          if request.method in permissions.SAFE_METHODS:
> diff --git a/patchwork/api/check.py b/patchwork/api/check.py
> index a6bf5f8..fde6b61 100644
> --- a/patchwork/api/check.py
> +++ b/patchwork/api/check.py
> @@ -6,7 +6,7 @@
>  from django.http import Http404
>  from django.http.request import QueryDict
>  from django.shortcuts import get_object_or_404
> -import rest_framework
> +
>  from rest_framework.exceptions import PermissionDenied
>  from rest_framework.generics import ListCreateAPIView
>  from rest_framework.generics import RetrieveAPIView
> @@ -17,30 +17,13 @@ from rest_framework.serializers import ValidationError
>  
>  from patchwork.api.base import CheckHyperlinkedIdentityField
>  from patchwork.api.base import MultipleFieldLookupMixin
> +from patchwork.api.base import CurrentPatchDefault
>  from patchwork.api.embedded import UserSerializer
>  from patchwork.api.filters import CheckFilterSet
>  from patchwork.models import Check
>  from patchwork.models import Patch
>  
>  
> -DRF_VERSION = tuple(int(x) for x in rest_framework.__version__.split('.'))
> -
> -
> -if DRF_VERSION > (3, 11):
> -    class CurrentPatchDefault(object):
> -        requires_context = True
> -
> -        def __call__(self, serializer_field):
> -            return serializer_field.context['request'].patch
> -else:
> -    class CurrentPatchDefault(object):
> -        def set_context(self, serializer_field):
> -            self.patch = serializer_field.context['request'].patch
> -
> -        def __call__(self):
> -            return self.patch
> -
> -
>  class CheckSerializer(HyperlinkedModelSerializer):
>  
>      url = CheckHyperlinkedIdentityField('api-check-detail')
> diff --git a/patchwork/api/comment.py b/patchwork/api/comment.py
> index 43b26c6..50d70d1 100644
> --- a/patchwork/api/comment.py
> +++ b/patchwork/api/comment.py
> @@ -5,12 +5,17 @@
>  
>  import email.parser
>  
> +from django.shortcuts import get_object_or_404
>  from django.http import Http404
>  from rest_framework.generics import ListAPIView
> +from rest_framework.generics import RetrieveUpdateAPIView
> +from rest_framework.serializers import HiddenField
>  from rest_framework.serializers import SerializerMethodField
>  
>  from patchwork.api.base import BaseHyperlinkedModelSerializer
> +from patchwork.api.base import MultipleFieldLookupMixin
>  from patchwork.api.base import PatchworkPermission
> +from patchwork.api.base import CurrentPatchDefault
>  from patchwork.api.embedded import PersonSerializer
>  from patchwork.models import Cover
>  from patchwork.models import CoverComment
> @@ -66,15 +71,50 @@ class CoverCommentListSerializer(BaseCommentListSerializer):
>          versioned_fields = BaseCommentListSerializer.Meta.versioned_fields
>  
>  
> -class PatchCommentListSerializer(BaseCommentListSerializer):
> +class PatchCommentSerializer(BaseCommentListSerializer):
> +
> +    patch = HiddenField(default=CurrentPatchDefault())
>  
>      class Meta:
>          model = PatchComment
> -        fields = BaseCommentListSerializer.Meta.fields
> -        read_only_fields = fields
> +        fields = BaseCommentListSerializer.Meta.fields + (
> +            'patch', 'addressed')
> +        read_only_fields = fields[:-1]  # able to write to addressed field
> +        versioned_fields = {
> +            '1.3': ('patch', 'addressed'),
> +        }
> +        extra_kwargs = {
> +            'url': {'view_name': 'api-patch-comment-detail'}
> +        }
>          versioned_fields = BaseCommentListSerializer.Meta.versioned_fields

Cool, adding the new bit to the comment serializer. Seems fine.

> diff --git a/patchwork/migrations/0045_patchcomment_addressed.py b/patchwork/migrations/0045_patchcomment_addressed.py
> new file mode 100644
> index 0000000..92e6c4e
> --- /dev/null
> +++ b/patchwork/migrations/0045_patchcomment_addressed.py
> @@ -0,0 +1,18 @@
> +# Generated by Django 3.1.12 on 2021-07-16 04:12
> +
> +from django.db import migrations, models
> +
> +
> +class Migration(migrations.Migration):
> +
> +    dependencies = [
> +        ('patchwork', '0044_add_project_linkname_validation'),
> +    ]
> +
> +    operations = [
> +        migrations.AddField(
> +            model_name='patchcomment',
> +            name='addressed',
> +            field=models.BooleanField(default=False),
> +        ),
> +    ]

Oh cool, so this is what migrates existing data for a running instance?

> diff --git a/patchwork/models.py b/patchwork/models.py
> index 00273da..ef52f2c 100644
> --- a/patchwork/models.py
> +++ b/patchwork/models.py
> @@ -693,6 +693,7 @@ class PatchComment(EmailMixin, models.Model):
>          related_query_name='comment',
>          on_delete=models.CASCADE,
>      )
> +    addressed = models.BooleanField(default=False)
>  
>      @property
>      def list_archive_url(self):
> @@ -718,7 +719,9 @@ class PatchComment(EmailMixin, models.Model):
>          self.patch.refresh_tag_counts()
>  
>      def is_editable(self, user):
> -        return False
> +        if user == self.submitter.user:
> +            return True
> +        return self.patch.is_editable(user)

I guess this is what I was asking for in the cover - changeable if it's
my patch or if I have patch edit permissions. I wonder if it's possible
to make it changeable for the author of the comment too? Hrm.

> diff --git a/patchwork/tests/api/test_comment.py b/patchwork/tests/api/test_comment.py
> index 5bbebf2..59450d8 100644
> --- a/patchwork/tests/api/test_comment.py
> +++ b/patchwork/tests/api/test_comment.py
> @@ -90,7 +90,7 @@ class TestPatchComments(utils.APITestCase):
>          kwargs = {}
>          if version:
>              kwargs['version'] = version
> -        kwargs['pk'] = patch.id
> +        kwargs['patch_id'] = patch.id
>  
>          return reverse('api-patch-comment-list', kwargs=kwargs)
>  
> @@ -142,5 +142,5 @@ class TestPatchComments(utils.APITestCase):
>      def test_list_invalid_patch(self):
>          """Ensure we get a 404 for a non-existent patch."""
>          resp = self.client.get(
> -            reverse('api-patch-comment-list', kwargs={'pk': '99999'}))
> +            reverse('api-patch-comment-list', kwargs={'patch_id': '99999'}))
>          self.assertEqual(status.HTTP_404_NOT_FOUND, resp.status_code)

Ok, just teaching the test about the disambiguated name for the id
field, I guess.


 - Emily
Daniel Axtens Aug. 10, 2021, 11:20 p.m. UTC | #2 
Raxel Gutierrez <raxel@google.com> writes:

> Add addressed boolean field to PatchComment to be able to distinguish
> between them in the patch-detail page. Change PatchComment to have same
> is_editable from the related patch with the addition of being editable
> by the user who submitted the comment.
>
> Add endpoint for patch comments at api/.../comments/<comment_id>.
> The endpoint will make it possible to use the REST API to update the new
> `addressed` field for patch comments with JavaScript on the client side.
> In the process of these changes, clean up use of CurrentPatchDefault so
> that it exists in base.py and can be used throughout the API.
>
> Add the OpenAPI definition of the new endpoint and upgrade API version
> to v1.3 to reflect the minor change. In order for tests to pass, clean
> up test_comment.py to reflect the change from the <pk> to <patch_id>
> parameter for the `api-patch-comment-list` endpoint.
>
> Signed-off-by: Raxel Gutierrez <raxel@google.com>
> ---
>  docs/api/schemas/generate-schemas.py          |    4 +-
>  docs/api/schemas/latest/patchwork.yaml        |  144 +-
>  docs/api/schemas/patchwork.j2                 |  148 +-
>  docs/api/schemas/v1.0/patchwork.yaml          |   35 +-
>  docs/api/schemas/v1.1/patchwork.yaml          |   35 +-
>  docs/api/schemas/v1.2/patchwork.yaml          |   35 +-
>  docs/api/schemas/v1.3/patchwork.yaml          | 2749 +++++++++++++++++
>  patchwork/api/base.py                         |   24 +-
>  patchwork/api/check.py                        |   21 +-
>  patchwork/api/comment.py                      |   76 +-
>  patchwork/api/patch.py                        |    2 +-
>  .../migrations/0045_patchcomment_addressed.py |   18 +
>  patchwork/models.py                           |    5 +-
>  patchwork/tests/api/test_comment.py           |    4 +-
>  patchwork/urls.py                             |   17 +-
>  15 files changed, 3256 insertions(+), 61 deletions(-)
>  create mode 100644 docs/api/schemas/v1.3/patchwork.yaml
>  create mode 100644 patchwork/migrations/0045_patchcomment_addressed.py
>
> diff --git a/docs/api/schemas/generate-schemas.py b/docs/api/schemas/generate-schemas.py
> index a0c1e45..3a436a1 100755
> --- a/docs/api/schemas/generate-schemas.py
> +++ b/docs/api/schemas/generate-schemas.py
> @@ -14,8 +14,8 @@ except ImportError:
>      yaml = None
>  
>  ROOT_DIR = os.path.dirname(os.path.realpath(__file__))
> -VERSIONS = [(1, 0), (1, 1), (1, 2), None]
> -LATEST_VERSION = (1, 2)
> +VERSIONS = [(1, 0), (1, 1), (1, 2), (1, 3), None]
> +LATEST_VERSION = (1, 3)
>  
>  
>  def generate_schemas():

[generated schema snipped]

> diff --git a/docs/api/schemas/patchwork.j2 b/docs/api/schemas/patchwork.j2
> index af20743..3600139 100644
> --- a/docs/api/schemas/patchwork.j2
> +++ b/docs/api/schemas/patchwork.j2
> @@ -619,14 +619,14 @@ paths:
>  {% endif %}
>        tags:
>          - patches
> -  /api/{{ version_url }}patches/{id}/comments/:
> +  /api/{{ version_url }}patches/{patch_id}/comments/:
>      parameters:
>        - in: path
> -        name: id
> +        name: patch_id

As Emily suggested, this is probably not something we can do in old
schema versions. I tried running the OpenAPI generator
(https://openapi-generator.tech/) over the new and old v1.2 schema,
generating a Go client for the v1.2 API, and observed different method
signatures and struct field names. I don't know if people use named
parameters in Go so maybe things would keep working by accident but
hopefully you get what I'm going for: if you had written some program
that interacts with the API using code generated from the API definition
file, things might break.

There shouldn't generally be any meaningful changes to older API version
descriptions at this stage.

>          description: A unique integer value identifying the parent patch.
>          required: true
>          schema:
> -          title: ID
> +          title: Patch ID
>            type: integer
>      get:
>        description: List comments
> @@ -656,6 +656,112 @@ paths:
>                  $ref: '#/components/schemas/Error'
>        tags:
>          - comments

> +  /api/{{ version_url }}patches/{patch_id}/comments/{comment_id}/:
I think the {% if ... needs to start above this line - otherwise all the
API versions see this endpoint, just without any methods. I think you've
correctly hidden it from older versions with the api_1_3_patterns part
in urls.py, so we don't want them to see it in the definitions file
either.

> +    parameters:
> +      - in: path
> +        name: patch_id
> +        description: A unique integer value identifying the parent patch.
> +        required: true
> +        schema:
> +          title: Patch ID
> +          type: integer
> +      - in: path
> +        name: comment_id
> +        description: A unique integer value identifying this comment.
> +        required: true
> +        schema:
> +          title: Comment ID
> +          type: integer
> +{% if version >= (1, 3) %}
> +    get:
> +      description: Show a patch comment.
> +      operationId: patch_comments_read
> +      responses:
> +        '200':
> +          description: ''
> +          content:
> +            application/json:
> +              schema:
> +                $ref: '#/components/schemas/CommentDetail'
> +        '404':
> +          description: Not found
> +          content:
> +            application/json:
> +              schema:
> +                $ref: '#/components/schemas/Error'
> +      tags:
> +        - comments
> +    patch:
> +      description: Update a patch comment (partial).
> +      operationId: patch_comments_partial_update
> +#      security:
> +#        - basicAuth: []
> +#        - apiKeyAuth: []

I can see you've copy-pasted this from the other definitions, which is
exactly what I said to do. It feels silly to keep it but also seems
silly to break consistency. Idk. Unless Stephen weighs in feel free to
just do whatever you feel most comfortable with.
[See commit 2047107468a1 ("docs: Resolve issues with 'patches'")]

> +      requestBody:
> +        $ref: '#/components/requestBodies/Comment'
> +      responses:
> +        '200':
> +          description: ''
> +          content:
> +            application/json:
> +              schema:
> +                $ref: '#/components/schemas/CommentDetail'
> +        '400':
> +          description: Invalid Request
> +          content:
> +            application/json:
> +              schema:
> +                $ref: '#/components/schemas/ErrorCommentUpdate'
> +        '403':
> +          description: Forbidden
> +          content:
> +            application/json:
> +              schema:
> +                $ref: '#/components/schemas/Error'
> +        '404':
> +          description: Not found
> +          content:
> +            application/json:
> +              schema:
> +                $ref: '#/components/schemas/Error'
> +      tags:
> +        - comments
> +    put:
> +      description: Update a patch comment.
> +      operationId: patch_comments_update
> +#      security:
> +#        - basicAuth: []
> +#        - apiKeyAuth: []
> +      requestBody:
> +        $ref: '#/components/requestBodies/Comment'
> +      responses:
> +        '200':
> +          description: ''
> +          content:
> +            application/json:
> +              schema:
> +                $ref: '#/components/schemas/CommentDetail'
> +        '400':
> +          description: Invalid Request
> +          content:
> +            application/json:
> +              schema:
> +                $ref: '#/components/schemas/ErrorCommentUpdate'
> +        '403':
> +          description: Forbidden
> +          content:
> +            application/json:
> +              schema:
> +                $ref: '#/components/schemas/Error'
> +        '404':
> +          description: Not found
> +          content:
> +            application/json:
> +              schema:
> +                $ref: '#/components/schemas/Error'
> +      tags:
> +        - comments

Hmm. Do we want a PUT method?

We have a pretty eclectic collection of PUTable objects atm: User,
Bundle, Patch and Project.

If I understand the HTTP methods correctly, I don't think that we're
supposed to allow the PUT method unless you can create the object or
replace its contents via the request. So I think it makes sense to allow
us to PUT a Bundle, but I think we should only allow Users, Patches,
Projects and Comments to be PATCHed: they should all be created in other
ways, and usually we don't allow their contents to be replaced.

I think you only need a PATCH method here, unless Stephen has any other ideas.

> +{% endif %}
>    /api/{{ version_url }}patches/{patch_id}/checks/:
>      parameters:
>        - in: path
> @@ -1277,6 +1383,12 @@ components:
>          application/x-www-form-urlencoded:
>            schema:
>              $ref: '#/components/schemas/CheckCreate'
> +    Comment:
> +      required: true
> +      content:
> +        application/json:
> +          schema:
> +            $ref: '#/components/schemas/CommentUpdate'
>      Patch:
>        required: true
>        content:
> @@ -1586,6 +1698,25 @@ components:
>                additionalProperties:
>                  type: string
>            readOnly: true
> +    CommentDetail:
> +      allOf:
> +        - $ref: '#/components/schemas/Comment'
> +{% if version >= (1, 3) %}
> +        - properties:
> +            patch:
> +              $ref: '#/components/schemas/PatchEmbedded'
> +            addressed:
> +              title: Addressed
> +              type: boolean
> +{% endif %}
> +    CommentUpdate:
> +      type: object
> +{% if version >= (1, 3) %}
> +      properties:
> +        addressed:
> +          title: Addressed
> +          type: boolean
> +{% endif %}
>      CoverList:
>        type: object
>        properties:
> @@ -2659,6 +2790,17 @@ components:
>            items:
>              type: string
>            readOnly: true
> +    ErrorCommentUpdate:
> +      type: object
> +{% if version >= (1, 3) %}
> +      properties:
> +        addressed:
> +          title: Addressed
> +          type: array
> +          items:
> +            type: string
> +          readOnly: true
> +{% endif %}

I think we should move the if condition for these objects too. I think
if the types are only used in v1.3 they should only be defined in v1.3 -
I can't think of a reason we would need to have objects with no
properties in older API versions?

>      ErrorPatchUpdate:
>        type: object
>        properties:
> diff --git a/docs/api/schemas/v1.0/patchwork.yaml b/docs/api/schemas/v1.0/patchwork.yaml
> index 23e8930..3f90b3e 100644

[snip generated versions]

> diff --git a/patchwork/api/base.py b/patchwork/api/base.py
> index 89a4311..856fbd3 100644
> --- a/patchwork/api/base.py
> +++ b/patchwork/api/base.py
> @@ -3,6 +3,7 @@
>  #
>  # SPDX-License-Identifier: GPL-2.0-or-later
>  
> +import rest_framework
>  
>  from django.conf import settings
>  from django.shortcuts import get_object_or_404
> @@ -15,6 +16,24 @@ from rest_framework.serializers import HyperlinkedModelSerializer
>  from patchwork.api import utils
>  
>  
> +DRF_VERSION = tuple(int(x) for x in rest_framework.__version__.split('.'))
> +
> +
> +if DRF_VERSION > (3, 11):
> +    class CurrentPatchDefault(object):
> +        requires_context = True
> +
> +        def __call__(self, serializer_field):
> +            return serializer_field.context['request'].patch
> +else:
> +    class CurrentPatchDefault(object):
> +        def set_context(self, serializer_field):
> +            self.patch = serializer_field.context['request'].patch
> +
> +        def __call__(self):
> +            return self.patch

Hmm. I was going to comment on this but then I realised you just moved
this code. I think we might be able to drop the DRF version condition
entirely but you don't need to be the person to do that (and this series
isn't necessarily the right place).

No action required here.

> +
> +
>  class LinkHeaderPagination(PageNumberPagination):
>      """Provide pagination based on rfc5988.
>  
> @@ -44,7 +63,10 @@ class LinkHeaderPagination(PageNumberPagination):
>  
>  
>  class PatchworkPermission(permissions.BasePermission):
> -    """This permission works for Project and Patch model objects"""
> +    """
> +    This permission works for Project, Patch, and PatchComment
> +    model objects
> +    """
>      def has_object_permission(self, request, view, obj):
>          # read only for everyone
>          if request.method in permissions.SAFE_METHODS:
> diff --git a/patchwork/api/check.py b/patchwork/api/check.py
> index a6bf5f8..fde6b61 100644
> --- a/patchwork/api/check.py
> +++ b/patchwork/api/check.py
> @@ -6,7 +6,7 @@
>  from django.http import Http404
>  from django.http.request import QueryDict
>  from django.shortcuts import get_object_or_404
> -import rest_framework
> +
>  from rest_framework.exceptions import PermissionDenied
>  from rest_framework.generics import ListCreateAPIView
>  from rest_framework.generics import RetrieveAPIView
> @@ -17,30 +17,13 @@ from rest_framework.serializers import ValidationError
>  
>  from patchwork.api.base import CheckHyperlinkedIdentityField
>  from patchwork.api.base import MultipleFieldLookupMixin
> +from patchwork.api.base import CurrentPatchDefault
>  from patchwork.api.embedded import UserSerializer
>  from patchwork.api.filters import CheckFilterSet
>  from patchwork.models import Check
>  from patchwork.models import Patch
>  
>  
> -DRF_VERSION = tuple(int(x) for x in rest_framework.__version__.split('.'))
> -
> -
> -if DRF_VERSION > (3, 11):
> -    class CurrentPatchDefault(object):
> -        requires_context = True
> -
> -        def __call__(self, serializer_field):
> -            return serializer_field.context['request'].patch
> -else:
> -    class CurrentPatchDefault(object):
> -        def set_context(self, serializer_field):
> -            self.patch = serializer_field.context['request'].patch
> -
> -        def __call__(self):
> -            return self.patch
> -
> -
>  class CheckSerializer(HyperlinkedModelSerializer):
>  
>      url = CheckHyperlinkedIdentityField('api-check-detail')
> diff --git a/patchwork/api/comment.py b/patchwork/api/comment.py
> index 43b26c6..50d70d1 100644
> --- a/patchwork/api/comment.py
> +++ b/patchwork/api/comment.py
> @@ -5,12 +5,17 @@
>  
>  import email.parser
>  
> +from django.shortcuts import get_object_or_404
>  from django.http import Http404
>  from rest_framework.generics import ListAPIView
> +from rest_framework.generics import RetrieveUpdateAPIView
> +from rest_framework.serializers import HiddenField
>  from rest_framework.serializers import SerializerMethodField
>  
>  from patchwork.api.base import BaseHyperlinkedModelSerializer
> +from patchwork.api.base import MultipleFieldLookupMixin
>  from patchwork.api.base import PatchworkPermission
> +from patchwork.api.base import CurrentPatchDefault
>  from patchwork.api.embedded import PersonSerializer
>  from patchwork.models import Cover
>  from patchwork.models import CoverComment
> @@ -66,15 +71,50 @@ class CoverCommentListSerializer(BaseCommentListSerializer):
>          versioned_fields = BaseCommentListSerializer.Meta.versioned_fields
>  
>  
> -class PatchCommentListSerializer(BaseCommentListSerializer):
> +class PatchCommentSerializer(BaseCommentListSerializer):
> +
> +    patch = HiddenField(default=CurrentPatchDefault())
>  
>      class Meta:
>          model = PatchComment
> -        fields = BaseCommentListSerializer.Meta.fields
> -        read_only_fields = fields
> +        fields = BaseCommentListSerializer.Meta.fields + (
> +            'patch', 'addressed')
> +        read_only_fields = fields[:-1]  # able to write to addressed field

Please make this explicit, that is
BaseCommentListSerializer.Meta.fields + ('patch', )

> +        versioned_fields = {
> +            '1.3': ('patch', 'addressed'),
> +        }
> +        extra_kwargs = {
> +            'url': {'view_name': 'api-patch-comment-detail'}
> +        }
>          versioned_fields = BaseCommentListSerializer.Meta.versioned_fields
>  
>  
> +class PatchCommentMixin(object):
> +
> +    permission_classes = (PatchworkPermission,)
> +    serializer_class = PatchCommentSerializer
> +
> +    def get_object(self):
> +        queryset = self.filter_queryset(self.get_queryset())
> +        comment_id = self.kwargs['comment_id']
> +        try:
> +            obj = queryset.get(id=int(comment_id))
> +        except (ValueError, PatchComment.DoesNotExist):
> +            obj = get_object_or_404(queryset, linkname=comment_id)
> +        self.kwargs['comment_id'] = obj.id
> +        self.check_object_permissions(self.request, obj)
> +        return obj
> +
> +    def get_queryset(self):
> +        patch_id = self.kwargs['patch_id']
> +        if not Patch.objects.filter(id=patch_id).exists():
> +            raise Http404
> +
> +        return PatchComment.objects.filter(
> +            patch=patch_id
> +        ).select_related('submitter')
> +
> +
>  class CoverCommentList(ListAPIView):
>      """List cover comments"""
>  
> @@ -94,20 +134,24 @@ class CoverCommentList(ListAPIView):
>          ).select_related('submitter')
>  
>  
> -class PatchCommentList(ListAPIView):
> -    """List comments"""
> +class PatchCommentList(PatchCommentMixin, ListAPIView):
> +    """List patch comments"""
>  
> -    permission_classes = (PatchworkPermission,)
> -    serializer_class = PatchCommentListSerializer
>      search_fields = ('subject',)
>      ordering_fields = ('id', 'subject', 'date', 'submitter')
>      ordering = 'id'
> -    lookup_url_kwarg = 'pk'
> -
> -    def get_queryset(self):
> -        if not Patch.objects.filter(pk=self.kwargs['pk']).exists():
> -            raise Http404
> -
> -        return PatchComment.objects.filter(
> -            patch=self.kwargs['pk']
> -        ).select_related('submitter')
> +    lookup_url_kwarg = 'patch_id'
> +
> +
> +class PatchCommentDetail(PatchCommentMixin, MultipleFieldLookupMixin,
> +                         RetrieveUpdateAPIView):
> +    """
> +    get:
> +    Show a patch comment.
> +    patch:
> +    Update a patch comment.
> +    put:
> +    Update a patch comment.
> +    """
> +    lookup_url_kwargs = ('patch_id', 'comment_id')
> +    lookup_fields = ('patch_id', 'id')
> diff --git a/patchwork/api/patch.py b/patchwork/api/patch.py
> index 9d22275..a97a882 100644
> --- a/patchwork/api/patch.py
> +++ b/patchwork/api/patch.py
> @@ -97,7 +97,7 @@ class PatchListSerializer(BaseHyperlinkedModelSerializer):
>  
>      def get_comments(self, patch):
>          return self.context.get('request').build_absolute_uri(
> -            reverse('api-patch-comment-list', kwargs={'pk': patch.id}))
> +            reverse('api-patch-comment-list', kwargs={'patch_id': patch.id}))
>  
>      def get_check(self, instance):
>          return instance.combined_check_state
> diff --git a/patchwork/migrations/0045_patchcomment_addressed.py b/patchwork/migrations/0045_patchcomment_addressed.py
> new file mode 100644
> index 0000000..92e6c4e
> --- /dev/null
> +++ b/patchwork/migrations/0045_patchcomment_addressed.py
> @@ -0,0 +1,18 @@
> +# Generated by Django 3.1.12 on 2021-07-16 04:12
> +
> +from django.db import migrations, models
> +
> +
> +class Migration(migrations.Migration):
> +
> +    dependencies = [
> +        ('patchwork', '0044_add_project_linkname_validation'),
> +    ]
> +
> +    operations = [
> +        migrations.AddField(
> +            model_name='patchcomment',
> +            name='addressed',
> +            field=models.BooleanField(default=False),
> +        ),
> +    ]
> diff --git a/patchwork/models.py b/patchwork/models.py
> index 00273da..ef52f2c 100644
> --- a/patchwork/models.py
> +++ b/patchwork/models.py
> @@ -693,6 +693,7 @@ class PatchComment(EmailMixin, models.Model):
>          related_query_name='comment',
>          on_delete=models.CASCADE,
>      )
> +    addressed = models.BooleanField(default=False)
>  
>      @property
>      def list_archive_url(self):
> @@ -718,7 +719,9 @@ class PatchComment(EmailMixin, models.Model):
>          self.patch.refresh_tag_counts()
>  
>      def is_editable(self, user):
> -        return False
> +        if user == self.submitter.user:
> +            return True
> +        return self.patch.is_editable(user)
>  
>      class Meta:
>          ordering = ['date']
> diff --git a/patchwork/tests/api/test_comment.py b/patchwork/tests/api/test_comment.py
> index 5bbebf2..59450d8 100644
> --- a/patchwork/tests/api/test_comment.py
> +++ b/patchwork/tests/api/test_comment.py
> @@ -90,7 +90,7 @@ class TestPatchComments(utils.APITestCase):
>          kwargs = {}
>          if version:
>              kwargs['version'] = version
> -        kwargs['pk'] = patch.id
> +        kwargs['patch_id'] = patch.id

I think we might be able to get away with renaming the parameter
internally even if we can't rename it in old versions of the API, but
please split the 'pk'->'patch_id' change into a different patch.

Kind regards,
Daniel
Stephen Finucane Aug. 12, 2021, 11:38 a.m. UTC | #3 
On Wed, 2021-08-11 at 09:20 +1000, Daniel Axtens wrote:
> Raxel Gutierrez <raxel@google.com> writes:
> 
> > Add addressed boolean field to PatchComment to be able to distinguish
> > between them in the patch-detail page. Change PatchComment to have same
> > is_editable from the related patch with the addition of being editable
> > by the user who submitted the comment.
> > 
> > Add endpoint for patch comments at api/.../comments/<comment_id>.
> > The endpoint will make it possible to use the REST API to update the new
> > `addressed` field for patch comments with JavaScript on the client side.
> > In the process of these changes, clean up use of CurrentPatchDefault so
> > that it exists in base.py and can be used throughout the API.
> > 
> > Add the OpenAPI definition of the new endpoint and upgrade API version
> > to v1.3 to reflect the minor change. In order for tests to pass, clean
> > up test_comment.py to reflect the change from the <pk> to <patch_id>
> > parameter for the `api-patch-comment-list` endpoint.
> > 
> > Signed-off-by: Raxel Gutierrez <raxel@google.com>
> > ---
> >  docs/api/schemas/generate-schemas.py          |    4 +-
> >  docs/api/schemas/latest/patchwork.yaml        |  144 +-
> >  docs/api/schemas/patchwork.j2                 |  148 +-
> >  docs/api/schemas/v1.0/patchwork.yaml          |   35 +-
> >  docs/api/schemas/v1.1/patchwork.yaml          |   35 +-
> >  docs/api/schemas/v1.2/patchwork.yaml          |   35 +-
> >  docs/api/schemas/v1.3/patchwork.yaml          | 2749 +++++++++++++++++
> >  patchwork/api/base.py                         |   24 +-
> >  patchwork/api/check.py                        |   21 +-
> >  patchwork/api/comment.py                      |   76 +-
> >  patchwork/api/patch.py                        |    2 +-
> >  .../migrations/0045_patchcomment_addressed.py |   18 +
> >  patchwork/models.py                           |    5 +-
> >  patchwork/tests/api/test_comment.py           |    4 +-
> >  patchwork/urls.py                             |   17 +-
> >  15 files changed, 3256 insertions(+), 61 deletions(-)
> >  create mode 100644 docs/api/schemas/v1.3/patchwork.yaml
> >  create mode 100644 patchwork/migrations/0045_patchcomment_addressed.py
> > 
> > diff --git a/docs/api/schemas/generate-schemas.py b/docs/api/schemas/generate-schemas.py
> > index a0c1e45..3a436a1 100755
> > --- a/docs/api/schemas/generate-schemas.py
> > +++ b/docs/api/schemas/generate-schemas.py
> > @@ -14,8 +14,8 @@ except ImportError:
> >      yaml = None
> >  
> >  ROOT_DIR = os.path.dirname(os.path.realpath(__file__))
> > -VERSIONS = [(1, 0), (1, 1), (1, 2), None]
> > -LATEST_VERSION = (1, 2)
> > +VERSIONS = [(1, 0), (1, 1), (1, 2), (1, 3), None]
> > +LATEST_VERSION = (1, 3)
> >  
> >  
> >  def generate_schemas():
> 
> [generated schema snipped]
> 
> > diff --git a/docs/api/schemas/patchwork.j2 b/docs/api/schemas/patchwork.j2
> > index af20743..3600139 100644
> > --- a/docs/api/schemas/patchwork.j2
> > +++ b/docs/api/schemas/patchwork.j2
> > @@ -619,14 +619,14 @@ paths:
> >  {% endif %}
> >        tags:
> >          - patches
> > -  /api/{{ version_url }}patches/{id}/comments/:
> > +  /api/{{ version_url }}patches/{patch_id}/comments/:
> >      parameters:
> >        - in: path
> > -        name: id
> > +        name: patch_id
> 
> As Emily suggested, this is probably not something we can do in old
> schema versions. I tried running the OpenAPI generator
> (https://openapi-generator.tech/) over the new and old v1.2 schema,
> generating a Go client for the v1.2 API, and observed different method
> signatures and struct field names. I don't know if people use named
> parameters in Go so maybe things would keep working by accident but
> hopefully you get what I'm going for: if you had written some program
> that interacts with the API using code generated from the API definition
> file, things might break.
> 
> There shouldn't generally be any meaningful changes to older API version
> descriptions at this stage.

tbh, I doubt it makes much of a difference. Ultimately the API behaves the same
way - all we've changed is the token name. If we break something, I'm sure
someone will yell at us.

I would rather this change happened separately in a precursor patch though.
There's already enough going on here without muddying the water further.

> 
> >          description: A unique integer value identifying the parent patch.
> >          required: true
> >          schema:
> > -          title: ID
> > +          title: Patch ID
> >            type: integer
> >      get:
> >        description: List comments
> > @@ -656,6 +656,112 @@ paths:
> >                  $ref: '#/components/schemas/Error'
> >        tags:
> >          - comments
> 
> > +  /api/{{ version_url }}patches/{patch_id}/comments/{comment_id}/:
> I think the {% if ... needs to start above this line - otherwise all the
> API versions see this endpoint, just without any methods. I think you've
> correctly hidden it from older versions with the api_1_3_patterns part
> in urls.py, so we don't want them to see it in the definitions file
> either.

Correct.

> 
> > +    parameters:
> > +      - in: path
> > +        name: patch_id
> > +        description: A unique integer value identifying the parent patch.
> > +        required: true
> > +        schema:
> > +          title: Patch ID
> > +          type: integer
> > +      - in: path
> > +        name: comment_id
> > +        description: A unique integer value identifying this comment.
> > +        required: true
> > +        schema:
> > +          title: Comment ID
> > +          type: integer
> > +{% if version >= (1, 3) %}
> > +    get:
> > +      description: Show a patch comment.
> > +      operationId: patch_comments_read
> > +      responses:
> > +        '200':
> > +          description: ''
> > +          content:
> > +            application/json:
> > +              schema:
> > +                $ref: '#/components/schemas/CommentDetail'
> > +        '404':
> > +          description: Not found
> > +          content:
> > +            application/json:
> > +              schema:
> > +                $ref: '#/components/schemas/Error'
> > +      tags:
> > +        - comments
> > +    patch:
> > +      description: Update a patch comment (partial).
> > +      operationId: patch_comments_partial_update
> > +#      security:
> > +#        - basicAuth: []
> > +#        - apiKeyAuth: []
> 
> I can see you've copy-pasted this from the other definitions, which is
> exactly what I said to do. It feels silly to keep it but also seems
> silly to break consistency. Idk. Unless Stephen weighs in feel free to
> just do whatever you feel most comfortable with.
> [See commit 2047107468a1 ("docs: Resolve issues with 'patches'")]

Yeah, this sucks. Feel free to either copy-paste as you've done or drop all of
these commented out blobs. I've tried to fix this a few times and been
frustrated every single time.

> 
> > +      requestBody:
> > +        $ref: '#/components/requestBodies/Comment'
> > +      responses:
> > +        '200':
> > +          description: ''
> > +          content:
> > +            application/json:
> > +              schema:
> > +                $ref: '#/components/schemas/CommentDetail'
> > +        '400':
> > +          description: Invalid Request
> > +          content:
> > +            application/json:
> > +              schema:
> > +                $ref: '#/components/schemas/ErrorCommentUpdate'
> > +        '403':
> > +          description: Forbidden
> > +          content:
> > +            application/json:
> > +              schema:
> > +                $ref: '#/components/schemas/Error'
> > +        '404':
> > +          description: Not found
> > +          content:
> > +            application/json:
> > +              schema:
> > +                $ref: '#/components/schemas/Error'
> > +      tags:
> > +        - comments
> > +    put:
> > +      description: Update a patch comment.
> > +      operationId: patch_comments_update
> > +#      security:
> > +#        - basicAuth: []
> > +#        - apiKeyAuth: []
> > +      requestBody:
> > +        $ref: '#/components/requestBodies/Comment'
> > +      responses:
> > +        '200':
> > +          description: ''
> > +          content:
> > +            application/json:
> > +              schema:
> > +                $ref: '#/components/schemas/CommentDetail'
> > +        '400':
> > +          description: Invalid Request
> > +          content:
> > +            application/json:
> > +              schema:
> > +                $ref: '#/components/schemas/ErrorCommentUpdate'
> > +        '403':
> > +          description: Forbidden
> > +          content:
> > +            application/json:
> > +              schema:
> > +                $ref: '#/components/schemas/Error'
> > +        '404':
> > +          description: Not found
> > +          content:
> > +            application/json:
> > +              schema:
> > +                $ref: '#/components/schemas/Error'
> > +      tags:
> > +        - comments
> 
> Hmm. Do we want a PUT method?
> 
> We have a pretty eclectic collection of PUTable objects atm: User,
> Bundle, Patch and Project.
> 
> If I understand the HTTP methods correctly, I don't think that we're
> supposed to allow the PUT method unless you can create the object or
> replace its contents via the request. So I think it makes sense to allow
> us to PUT a Bundle, but I think we should only allow Users, Patches,
> Projects and Comments to be PATCHed: they should all be created in other
> ways, and usually we don't allow their contents to be replaced.
> 
> I think you only need a PATCH method here, unless Stephen has any other ideas.

If we're documenting PUT here then that means we support it at the API layer
too, assuming tests have been fully implemented. I've no issues with not
supporting it but we should remove support at the code level also. IIRC, this
might have implications for the web UI autogenerated by django-rest-framework
but you'd need to investigate this yourself.

> 
> > +{% endif %}
> >    /api/{{ version_url }}patches/{patch_id}/checks/:
> >      parameters:
> >        - in: path
> > @@ -1277,6 +1383,12 @@ components:
> >          application/x-www-form-urlencoded:
> >            schema:
> >              $ref: '#/components/schemas/CheckCreate'
> > +    Comment:
> > +      required: true
> > +      content:
> > +        application/json:
> > +          schema:
> > +            $ref: '#/components/schemas/CommentUpdate'
> >      Patch:
> >        required: true
> >        content:
> > @@ -1586,6 +1698,25 @@ components:
> >                additionalProperties:
> >                  type: string
> >            readOnly: true
> > +    CommentDetail:
> > +      allOf:
> > +        - $ref: '#/components/schemas/Comment'
> > +{% if version >= (1, 3) %}

This is in the wrong place also. The entire resource should not be included if
version < 1.3. You need to move this above 'CommentDetail'.

> > +        - properties:
> > +            patch:
> > +              $ref: '#/components/schemas/PatchEmbedded'
> > +            addressed:
> > +              title: Addressed
> > +              type: boolean
> > +{% endif %}
> > +    CommentUpdate:
> > +      type: object
> > +{% if version >= (1, 3) %}

Ditto.

> > +      properties:
> > +        addressed:
> > +          title: Addressed
> > +          type: boolean
> > +{% endif %}
> >      CoverList:
> >        type: object
> >        properties:
> > @@ -2659,6 +2790,17 @@ components:
> >            items:
> >              type: string
> >            readOnly: true
> > +    ErrorCommentUpdate:
> > +      type: object
> > +{% if version >= (1, 3) %}

Ditto.

> > +      properties:
> > +        addressed:
> > +          title: Addressed
> > +          type: array
> > +          items:
> > +            type: string
> > +          readOnly: true
> > +{% endif %}
> 
> I think we should move the if condition for these objects too. I think
> if the types are only used in v1.3 they should only be defined in v1.3 -
> I can't think of a reason we would need to have objects with no
> properties in older API versions?

+1

> >      ErrorPatchUpdate:
> >        type: object
> >        properties:
> > diff --git a/docs/api/schemas/v1.0/patchwork.yaml b/docs/api/schemas/v1.0/patchwork.yaml
> > index 23e8930..3f90b3e 100644
> 
> [snip generated versions]

I wonder if we want to send the auto-generated version in a separate patch? Long
term we should probably remove those auto-generated versions from tree in favour
of building them via the test suite and a Sphinx extension.

> 
> > diff --git a/patchwork/api/base.py b/patchwork/api/base.py
> > index 89a4311..856fbd3 100644
> > --- a/patchwork/api/base.py
> > +++ b/patchwork/api/base.py
> > @@ -3,6 +3,7 @@
> >  #
> >  # SPDX-License-Identifier: GPL-2.0-or-later
> >  
> > +import rest_framework
> >  
> >  from django.conf import settings
> >  from django.shortcuts import get_object_or_404
> > @@ -15,6 +16,24 @@ from rest_framework.serializers import HyperlinkedModelSerializer
> >  from patchwork.api import utils
> >  
> >  
> > +DRF_VERSION = tuple(int(x) for x in rest_framework.__version__.split('.'))
> > +
> > +
> > +if DRF_VERSION > (3, 11):
> > +    class CurrentPatchDefault(object):
> > +        requires_context = True
> > +
> > +        def __call__(self, serializer_field):
> > +            return serializer_field.context['request'].patch
> > +else:
> > +    class CurrentPatchDefault(object):
> > +        def set_context(self, serializer_field):
> > +            self.patch = serializer_field.context['request'].patch
> > +
> > +        def __call__(self):
> > +            return self.patch
> 
> Hmm. I was going to comment on this but then I realised you just moved
> this code. I think we might be able to drop the DRF version condition
> entirely but you don't need to be the person to do that (and this series
> isn't necessarily the right place).
> 
> No action required here.

An I was going to say you should leave a comment here explaining why you're
doing this. However, this is code moved from 'patchwork/api/check.py' which I
authored so never mind.

@Daniel We can't remove it until we formally drop support for DRF < 3.12, which
we haven't done. I need to evaluate the package situation on Debian before I do
this.

> > +
> > +
> >  class LinkHeaderPagination(PageNumberPagination):
> >      """Provide pagination based on rfc5988.
> >  
> > @@ -44,7 +63,10 @@ class LinkHeaderPagination(PageNumberPagination):
> >  
> >  
> >  class PatchworkPermission(permissions.BasePermission):
> > -    """This permission works for Project and Patch model objects"""
> > +    """
> > +    This permission works for Project, Patch, and PatchComment
> > +    model objects
> > +    """
> >      def has_object_permission(self, request, view, obj):
> >          # read only for everyone
> >          if request.method in permissions.SAFE_METHODS:
> > diff --git a/patchwork/api/check.py b/patchwork/api/check.py
> > index a6bf5f8..fde6b61 100644
> > --- a/patchwork/api/check.py
> > +++ b/patchwork/api/check.py
> > @@ -6,7 +6,7 @@
> >  from django.http import Http404
> >  from django.http.request import QueryDict
> >  from django.shortcuts import get_object_or_404
> > -import rest_framework
> > +

nit: don't add a newline here. Imports should be grouped like so:

  stdlib imports

  third-party package imports

  patchwork imports

> >  from rest_framework.exceptions import PermissionDenied
> >  from rest_framework.generics import ListCreateAPIView
> >  from rest_framework.generics import RetrieveAPIView
> > @@ -17,30 +17,13 @@ from rest_framework.serializers import ValidationError
> >  
> >  from patchwork.api.base import CheckHyperlinkedIdentityField
> >  from patchwork.api.base import MultipleFieldLookupMixin
> > +from patchwork.api.base import CurrentPatchDefault
> >  from patchwork.api.embedded import UserSerializer
> >  from patchwork.api.filters import CheckFilterSet
> >  from patchwork.models import Check
> >  from patchwork.models import Patch
> >  
> >  
> > -DRF_VERSION = tuple(int(x) for x in rest_framework.__version__.split('.'))
> > -
> > -
> > -if DRF_VERSION > (3, 11):
> > -    class CurrentPatchDefault(object):
> > -        requires_context = True
> > -
> > -        def __call__(self, serializer_field):
> > -            return serializer_field.context['request'].patch
> > -else:
> > -    class CurrentPatchDefault(object):
> > -        def set_context(self, serializer_field):
> > -            self.patch = serializer_field.context['request'].patch
> > -
> > -        def __call__(self):
> > -            return self.patch
> > -
> > -
> >  class CheckSerializer(HyperlinkedModelSerializer):
> >  
> >      url = CheckHyperlinkedIdentityField('api-check-detail')
> > diff --git a/patchwork/api/comment.py b/patchwork/api/comment.py
> > index 43b26c6..50d70d1 100644
> > --- a/patchwork/api/comment.py
> > +++ b/patchwork/api/comment.py
> > @@ -5,12 +5,17 @@
> >  
> >  import email.parser
> >  
> > +from django.shortcuts import get_object_or_404
> >  from django.http import Http404
> >  from rest_framework.generics import ListAPIView
> > +from rest_framework.generics import RetrieveUpdateAPIView
> > +from rest_framework.serializers import HiddenField
> >  from rest_framework.serializers import SerializerMethodField
> >  
> >  from patchwork.api.base import BaseHyperlinkedModelSerializer
> > +from patchwork.api.base import MultipleFieldLookupMixin
> >  from patchwork.api.base import PatchworkPermission
> > +from patchwork.api.base import CurrentPatchDefault
> >  from patchwork.api.embedded import PersonSerializer
> >  from patchwork.models import Cover
> >  from patchwork.models import CoverComment
> > @@ -66,15 +71,50 @@ class CoverCommentListSerializer(BaseCommentListSerializer):
> >          versioned_fields = BaseCommentListSerializer.Meta.versioned_fields
> >  
> >  
> > -class PatchCommentListSerializer(BaseCommentListSerializer):
> > +class PatchCommentSerializer(BaseCommentListSerializer):
> > +
> > +    patch = HiddenField(default=CurrentPatchDefault())
> >  
> >      class Meta:
> >          model = PatchComment
> > -        fields = BaseCommentListSerializer.Meta.fields
> > -        read_only_fields = fields
> > +        fields = BaseCommentListSerializer.Meta.fields + (
> > +            'patch', 'addressed')
> > +        read_only_fields = fields[:-1]  # able to write to addressed field
> 
> Please make this explicit, that is
> BaseCommentListSerializer.Meta.fields + ('patch', )

+1

> 
> > +        versioned_fields = {
> > +            '1.3': ('patch', 'addressed'),
> > +        }
> > +        extra_kwargs = {
> > +            'url': {'view_name': 'api-patch-comment-detail'}
> > +        }
> >          versioned_fields = BaseCommentListSerializer.Meta.versioned_fields
> >  
> >  
> > +class PatchCommentMixin(object):
> > +
> > +    permission_classes = (PatchworkPermission,)
> > +    serializer_class = PatchCommentSerializer
> > +
> > +    def get_object(self):
> > +        queryset = self.filter_queryset(self.get_queryset())
> > +        comment_id = self.kwargs['comment_id']
> > +        try:
> > +            obj = queryset.get(id=int(comment_id))
> > +        except (ValueError, PatchComment.DoesNotExist):
> > +            obj = get_object_or_404(queryset, linkname=comment_id)
> > +        self.kwargs['comment_id'] = obj.id
> > +        self.check_object_permissions(self.request, obj)
> > +        return obj
> > +
> > +    def get_queryset(self):
> > +        patch_id = self.kwargs['patch_id']
> > +        if not Patch.objects.filter(id=patch_id).exists():
> > +            raise Http404
> > +
> > +        return PatchComment.objects.filter(
> > +            patch=patch_id
> > +        ).select_related('submitter')
> > +
> > +
> >  class CoverCommentList(ListAPIView):
> >      """List cover comments"""
> >  
> > @@ -94,20 +134,24 @@ class CoverCommentList(ListAPIView):
> >          ).select_related('submitter')
> >  
> >  
> > -class PatchCommentList(ListAPIView):
> > -    """List comments"""
> > +class PatchCommentList(PatchCommentMixin, ListAPIView):
> > +    """List patch comments"""
> >  
> > -    permission_classes = (PatchworkPermission,)
> > -    serializer_class = PatchCommentListSerializer
> >      search_fields = ('subject',)
> >      ordering_fields = ('id', 'subject', 'date', 'submitter')
> >      ordering = 'id'
> > -    lookup_url_kwarg = 'pk'
> > -
> > -    def get_queryset(self):
> > -        if not Patch.objects.filter(pk=self.kwargs['pk']).exists():
> > -            raise Http404
> > -
> > -        return PatchComment.objects.filter(
> > -            patch=self.kwargs['pk']
> > -        ).select_related('submitter')
> > +    lookup_url_kwarg = 'patch_id'
> > +
> > +
> > +class PatchCommentDetail(PatchCommentMixin, MultipleFieldLookupMixin,
> > +                         RetrieveUpdateAPIView):
> > +    """
> > +    get:
> > +    Show a patch comment.
> > +    patch:
> > +    Update a patch comment.
> > +    put:
> > +    Update a patch comment.
> > +    """
> > +    lookup_url_kwargs = ('patch_id', 'comment_id')
> > +    lookup_fields = ('patch_id', 'id')
> > diff --git a/patchwork/api/patch.py b/patchwork/api/patch.py
> > index 9d22275..a97a882 100644
> > --- a/patchwork/api/patch.py
> > +++ b/patchwork/api/patch.py
> > @@ -97,7 +97,7 @@ class PatchListSerializer(BaseHyperlinkedModelSerializer):
> >  
> >      def get_comments(self, patch):
> >          return self.context.get('request').build_absolute_uri(
> > -            reverse('api-patch-comment-list', kwargs={'pk': patch.id}))
> > +            reverse('api-patch-comment-list', kwargs={'patch_id': patch.id}))
> >  
> >      def get_check(self, instance):
> >          return instance.combined_check_state
> > diff --git a/patchwork/migrations/0045_patchcomment_addressed.py b/patchwork/migrations/0045_patchcomment_addressed.py
> > new file mode 100644
> > index 0000000..92e6c4e
> > --- /dev/null
> > +++ b/patchwork/migrations/0045_patchcomment_addressed.py
> > @@ -0,0 +1,18 @@
> > +# Generated by Django 3.1.12 on 2021-07-16 04:12
> > +
> > +from django.db import migrations, models
> > +
> > +
> > +class Migration(migrations.Migration):
> > +
> > +    dependencies = [
> > +        ('patchwork', '0044_add_project_linkname_validation'),
> > +    ]
> > +
> > +    operations = [
> > +        migrations.AddField(
> > +            model_name='patchcomment',
> > +            name='addressed',
> > +            field=models.BooleanField(default=False),

Thinking out loud: I suspect this will be an expensive migration since it will
add a new non-nullable field to all patchcomment rows. Maybe that's something we
can live with, but making this nullable might make more sense, particularly if
we want to make this feature enableable/disableable on a project-by-project
basis (I don't know if we do).

> > +        ),
> > +    ]
> > diff --git a/patchwork/models.py b/patchwork/models.py
> > index 00273da..ef52f2c 100644
> > --- a/patchwork/models.py
> > +++ b/patchwork/models.py
> > @@ -693,6 +693,7 @@ class PatchComment(EmailMixin, models.Model):
> >          related_query_name='comment',
> >          on_delete=models.CASCADE,
> >      )
> > +    addressed = models.BooleanField(default=False)
> >  
> >      @property
> >      def list_archive_url(self):
> > @@ -718,7 +719,9 @@ class PatchComment(EmailMixin, models.Model):
> >          self.patch.refresh_tag_counts()
> >  
> >      def is_editable(self, user):
> > -        return False
> > +        if user == self.submitter.user:
> > +            return True
> > +        return self.patch.is_editable(user)
> >  
> >      class Meta:
> >          ordering = ['date']
> > diff --git a/patchwork/tests/api/test_comment.py b/patchwork/tests/api/test_comment.py
> > index 5bbebf2..59450d8 100644
> > --- a/patchwork/tests/api/test_comment.py
> > +++ b/patchwork/tests/api/test_comment.py
> > @@ -90,7 +90,7 @@ class TestPatchComments(utils.APITestCase):
> >          kwargs = {}
> >          if version:
> >              kwargs['version'] = version
> > -        kwargs['pk'] = patch.id
> > +        kwargs['patch_id'] = patch.id
> 
> I think we might be able to get away with renaming the parameter
> internally even if we can't rename it in old versions of the API, but
> please split the 'pk'->'patch_id' change into a different patch.
> 
> Kind regards,
> Daniel

Cheers,
Stephen
Daniel Axtens Aug. 16, 2021, 2:24 p.m. UTC | #4 
> Thinking out loud: I suspect this will be an expensive migration since it will
> add a new non-nullable field to all patchcomment rows. Maybe that's something we
> can live with, but making this nullable might make more sense, particularly if
> we want to make this feature enableable/disableable on a project-by-project
> basis (I don't know if we do).

There's probably some context here that hopefully you've caught up with
Raxel and co about.

But anyway, the addressed/unaddressed thing just appears in the header
bar of each comment along with the commenter, date and the
permalink. Maintainers/submitters/commenters can chose whether they want
to look at addressed/unaddressed or they want to ignore it. I suspect
the main value will be for submitters to make sure they've addressed the
comments before they send out their next version so in that sense it
isn't even all that dependent on the whims of maintainers. So I was
thinking of just making the whole thing on all the time rather than
introducing the complexity of a per-project feature-flag.

My vibe from the kernel lists I hang out on is that we don't tend to get
quite as many comments as patches, but it could be a pretty noisy
distribution so I might have underestimated it. Anyway, I'll do a test
with and without nullable on my largish data set tomorrow, hopefully.

Kind regards,
Daniel

>> > +        ),
>> > +    ]
>> > diff --git a/patchwork/models.py b/patchwork/models.py
>> > index 00273da..ef52f2c 100644
>> > --- a/patchwork/models.py
>> > +++ b/patchwork/models.py
>> > @@ -693,6 +693,7 @@ class PatchComment(EmailMixin, models.Model):
>> >          related_query_name='comment',
>> >          on_delete=models.CASCADE,
>> >      )
>> > +    addressed = models.BooleanField(default=False)
>> >  
>> >      @property
>> >      def list_archive_url(self):
>> > @@ -718,7 +719,9 @@ class PatchComment(EmailMixin, models.Model):
>> >          self.patch.refresh_tag_counts()
>> >  
>> >      def is_editable(self, user):
>> > -        return False
>> > +        if user == self.submitter.user:
>> > +            return True
>> > +        return self.patch.is_editable(user)
>> >  
>> >      class Meta:
>> >          ordering = ['date']
>> > diff --git a/patchwork/tests/api/test_comment.py b/patchwork/tests/api/test_comment.py
>> > index 5bbebf2..59450d8 100644
>> > --- a/patchwork/tests/api/test_comment.py
>> > +++ b/patchwork/tests/api/test_comment.py
>> > @@ -90,7 +90,7 @@ class TestPatchComments(utils.APITestCase):
>> >          kwargs = {}
>> >          if version:
>> >              kwargs['version'] = version
>> > -        kwargs['pk'] = patch.id
>> > +        kwargs['patch_id'] = patch.id
>> 
>> I think we might be able to get away with renaming the parameter
>> internally even if we can't rename it in old versions of the API, but
>> please split the 'pk'->'patch_id' change into a different patch.
>> 
>> Kind regards,
>> Daniel
>
> Cheers,
> Stephen
Daniel Axtens Aug. 19, 2021, 5:50 a.m. UTC | #5 
>> > +    operations = [
>> > +        migrations.AddField(
>> > +            model_name='patchcomment',
>> > +            name='addressed',
>> > +            field=models.BooleanField(default=False),
>
> Thinking out loud: I suspect this will be an expensive migration since it will
> add a new non-nullable field to all patchcomment rows. Maybe that's something we
> can live with, but making this nullable might make more sense, particularly if
> we want to make this feature enableable/disableable on a project-by-project
> basis (I don't know if we do).

I tested this on MariaDB 10.4 and it was ~instantaneous even with my
large data set.

I also tested it with Postgres and it was also less than a second.

I think we're fine as is.

Kind regards,
Daniel
diff mbox series

Patch

diff --git a/docs/api/schemas/generate-schemas.py b/docs/api/schemas/generate-schemas.py
index a0c1e45..3a436a1 100755
--- a/docs/api/schemas/generate-schemas.py
+++ b/docs/api/schemas/generate-schemas.py
@@ -14,8 +14,8 @@  except ImportError:
     yaml = None
 
 ROOT_DIR = os.path.dirname(os.path.realpath(__file__))
-VERSIONS = [(1, 0), (1, 1), (1, 2), None]
-LATEST_VERSION = (1, 2)
+VERSIONS = [(1, 0), (1, 1), (1, 2), (1, 3), None]
+LATEST_VERSION = (1, 3)
 
 
 def generate_schemas():
diff --git a/docs/api/schemas/latest/patchwork.yaml b/docs/api/schemas/latest/patchwork.yaml
index a8910a7..95329c7 100644
--- a/docs/api/schemas/latest/patchwork.yaml
+++ b/docs/api/schemas/latest/patchwork.yaml
@@ -13,7 +13,7 @@  info:
   license:
     name: GPL v2 License
     url: https://www.gnu.org/licenses/gpl-2.0.html
-  version: '1.2'
+  version: '1.3'
 paths:
   /api/:
     get:
@@ -598,14 +598,14 @@  paths:
                 $ref: '#/components/schemas/Error'
       tags:
         - patches
-  /api/patches/{id}/comments/:
+  /api/patches/{patch_id}/comments/:
     parameters:
       - in: path
-        name: id
+        name: patch_id
         description: A unique integer value identifying the parent patch.
         required: true
         schema:
-          title: ID
+          title: Patch ID
           type: integer
     get:
       description: List comments
@@ -635,6 +635,110 @@  paths:
                 $ref: '#/components/schemas/Error'
       tags:
         - comments
+  /api/patches/{patch_id}/comments/{comment_id}/:
+    parameters:
+      - in: path
+        name: patch_id
+        description: A unique integer value identifying the parent patch.
+        required: true
+        schema:
+          title: Patch ID
+          type: integer
+      - in: path
+        name: comment_id
+        description: A unique integer value identifying this comment.
+        required: true
+        schema:
+          title: Comment ID
+          type: integer
+    get:
+      description: Show a patch comment.
+      operationId: patch_comments_read
+      responses:
+        '200':
+          description: ''
+          content:
+            application/json:
+              schema:
+                $ref: '#/components/schemas/CommentDetail'
+        '404':
+          description: Not found
+          content:
+            application/json:
+              schema:
+                $ref: '#/components/schemas/Error'
+      tags:
+        - comments
+    patch:
+      description: Update a patch comment (partial).
+      operationId: patch_comments_partial_update
+#      security:
+#        - basicAuth: []
+#        - apiKeyAuth: []
+      requestBody:
+        $ref: '#/components/requestBodies/Comment'
+      responses:
+        '200':
+          description: ''
+          content:
+            application/json:
+              schema:
+                $ref: '#/components/schemas/CommentDetail'
+        '400':
+          description: Invalid Request
+          content:
+            application/json:
+              schema:
+                $ref: '#/components/schemas/ErrorCommentUpdate'
+        '403':
+          description: Forbidden
+          content:
+            application/json:
+              schema:
+                $ref: '#/components/schemas/Error'
+        '404':
+          description: Not found
+          content:
+            application/json:
+              schema:
+                $ref: '#/components/schemas/Error'
+      tags:
+        - comments
+    put:
+      description: Update a patch comment.
+      operationId: patch_comments_update
+#      security:
+#        - basicAuth: []
+#        - apiKeyAuth: []
+      requestBody:
+        $ref: '#/components/requestBodies/Comment'
+      responses:
+        '200':
+          description: ''
+          content:
+            application/json:
+              schema:
+                $ref: '#/components/schemas/CommentDetail'
+        '400':
+          description: Invalid Request
+          content:
+            application/json:
+              schema:
+                $ref: '#/components/schemas/ErrorCommentUpdate'
+        '403':
+          description: Forbidden
+          content:
+            application/json:
+              schema:
+                $ref: '#/components/schemas/Error'
+        '404':
+          description: Not found
+          content:
+            application/json:
+              schema:
+                $ref: '#/components/schemas/Error'
+      tags:
+        - comments
   /api/patches/{patch_id}/checks/:
     parameters:
       - in: path
@@ -1242,6 +1346,12 @@  components:
         application/x-www-form-urlencoded:
           schema:
             $ref: '#/components/schemas/CheckCreate'
+    Comment:
+      required: true
+      content:
+        application/json:
+          schema:
+            $ref: '#/components/schemas/CommentUpdate'
     Patch:
       required: true
       content:
@@ -1528,6 +1638,21 @@  components:
               additionalProperties:
                 type: string
           readOnly: true
+    CommentDetail:
+      allOf:
+        - $ref: '#/components/schemas/Comment'
+        - properties:
+            patch:
+              $ref: '#/components/schemas/PatchEmbedded'
+            addressed:
+              title: Addressed
+              type: boolean
+    CommentUpdate:
+      type: object
+      properties:
+        addressed:
+          title: Addressed
+          type: boolean
     CoverList:
       type: object
       properties:
@@ -1712,9 +1837,11 @@  components:
                 previous_relation:
                   title: Previous relation
                   type: string
+                  nullable: true
                 current_relation:
                   title: Current relation
                   type: string
+                  nullable: true
     EventPatchDelegated:
       allOf:
         - $ref: '#/components/schemas/EventBase'
@@ -2555,6 +2682,15 @@  components:
           items:
             type: string
           readOnly: true
+    ErrorCommentUpdate:
+      type: object
+      properties:
+        addressed:
+          title: Addressed
+          type: array
+          items:
+            type: string
+          readOnly: true
     ErrorPatchUpdate:
       type: object
       properties:
diff --git a/docs/api/schemas/patchwork.j2 b/docs/api/schemas/patchwork.j2
index af20743..3600139 100644
--- a/docs/api/schemas/patchwork.j2
+++ b/docs/api/schemas/patchwork.j2
@@ -619,14 +619,14 @@  paths:
 {% endif %}
       tags:
         - patches
-  /api/{{ version_url }}patches/{id}/comments/:
+  /api/{{ version_url }}patches/{patch_id}/comments/:
     parameters:
       - in: path
-        name: id
+        name: patch_id
         description: A unique integer value identifying the parent patch.
         required: true
         schema:
-          title: ID
+          title: Patch ID
           type: integer
     get:
       description: List comments
@@ -656,6 +656,112 @@  paths:
                 $ref: '#/components/schemas/Error'
       tags:
         - comments
+  /api/{{ version_url }}patches/{patch_id}/comments/{comment_id}/:
+    parameters:
+      - in: path
+        name: patch_id
+        description: A unique integer value identifying the parent patch.
+        required: true
+        schema:
+          title: Patch ID
+          type: integer
+      - in: path
+        name: comment_id
+        description: A unique integer value identifying this comment.
+        required: true
+        schema:
+          title: Comment ID
+          type: integer
+{% if version >= (1, 3) %}
+    get:
+      description: Show a patch comment.
+      operationId: patch_comments_read
+      responses:
+        '200':
+          description: ''
+          content:
+            application/json:
+              schema:
+                $ref: '#/components/schemas/CommentDetail'
+        '404':
+          description: Not found
+          content:
+            application/json:
+              schema:
+                $ref: '#/components/schemas/Error'
+      tags:
+        - comments
+    patch:
+      description: Update a patch comment (partial).
+      operationId: patch_comments_partial_update
+#      security:
+#        - basicAuth: []
+#        - apiKeyAuth: []
+      requestBody:
+        $ref: '#/components/requestBodies/Comment'
+      responses:
+        '200':
+          description: ''
+          content:
+            application/json:
+              schema:
+                $ref: '#/components/schemas/CommentDetail'
+        '400':
+          description: Invalid Request
+          content:
+            application/json:
+              schema:
+                $ref: '#/components/schemas/ErrorCommentUpdate'
+        '403':
+          description: Forbidden
+          content:
+            application/json:
+              schema:
+                $ref: '#/components/schemas/Error'
+        '404':
+          description: Not found
+          content:
+            application/json:
+              schema:
+                $ref: '#/components/schemas/Error'
+      tags:
+        - comments
+    put:
+      description: Update a patch comment.
+      operationId: patch_comments_update
+#      security:
+#        - basicAuth: []
+#        - apiKeyAuth: []
+      requestBody:
+        $ref: '#/components/requestBodies/Comment'
+      responses:
+        '200':
+          description: ''
+          content:
+            application/json:
+              schema:
+                $ref: '#/components/schemas/CommentDetail'
+        '400':
+          description: Invalid Request
+          content:
+            application/json:
+              schema:
+                $ref: '#/components/schemas/ErrorCommentUpdate'
+        '403':
+          description: Forbidden
+          content:
+            application/json:
+              schema:
+                $ref: '#/components/schemas/Error'
+        '404':
+          description: Not found
+          content:
+            application/json:
+              schema:
+                $ref: '#/components/schemas/Error'
+      tags:
+        - comments
+{% endif %}
   /api/{{ version_url }}patches/{patch_id}/checks/:
     parameters:
       - in: path
@@ -1277,6 +1383,12 @@  components:
         application/x-www-form-urlencoded:
           schema:
             $ref: '#/components/schemas/CheckCreate'
+    Comment:
+      required: true
+      content:
+        application/json:
+          schema:
+            $ref: '#/components/schemas/CommentUpdate'
     Patch:
       required: true
       content:
@@ -1586,6 +1698,25 @@  components:
               additionalProperties:
                 type: string
           readOnly: true
+    CommentDetail:
+      allOf:
+        - $ref: '#/components/schemas/Comment'
+{% if version >= (1, 3) %}
+        - properties:
+            patch:
+              $ref: '#/components/schemas/PatchEmbedded'
+            addressed:
+              title: Addressed
+              type: boolean
+{% endif %}
+    CommentUpdate:
+      type: object
+{% if version >= (1, 3) %}
+      properties:
+        addressed:
+          title: Addressed
+          type: boolean
+{% endif %}
     CoverList:
       type: object
       properties:
@@ -2659,6 +2790,17 @@  components:
           items:
             type: string
           readOnly: true
+    ErrorCommentUpdate:
+      type: object
+{% if version >= (1, 3) %}
+      properties:
+        addressed:
+          title: Addressed
+          type: array
+          items:
+            type: string
+          readOnly: true
+{% endif %}
     ErrorPatchUpdate:
       type: object
       properties:
diff --git a/docs/api/schemas/v1.0/patchwork.yaml b/docs/api/schemas/v1.0/patchwork.yaml
index 23e8930..3f90b3e 100644
--- a/docs/api/schemas/v1.0/patchwork.yaml
+++ b/docs/api/schemas/v1.0/patchwork.yaml
@@ -459,14 +459,14 @@  paths:
                 $ref: '#/components/schemas/Error'
       tags:
         - patches
-  /api/1.0/patches/{id}/comments/:
+  /api/1.0/patches/{patch_id}/comments/:
     parameters:
       - in: path
-        name: id
+        name: patch_id
         description: A unique integer value identifying the parent patch.
         required: true
         schema:
-          title: ID
+          title: Patch ID
           type: integer
     get:
       description: List comments
@@ -496,6 +496,22 @@  paths:
                 $ref: '#/components/schemas/Error'
       tags:
         - comments
+  /api/1.0/patches/{patch_id}/comments/{comment_id}/:
+    parameters:
+      - in: path
+        name: patch_id
+        description: A unique integer value identifying the parent patch.
+        required: true
+        schema:
+          title: Patch ID
+          type: integer
+      - in: path
+        name: comment_id
+        description: A unique integer value identifying this comment.
+        required: true
+        schema:
+          title: Comment ID
+          type: integer
   /api/1.0/patches/{patch_id}/checks/:
     parameters:
       - in: path
@@ -1091,6 +1107,12 @@  components:
         application/x-www-form-urlencoded:
           schema:
             $ref: '#/components/schemas/CheckCreate'
+    Comment:
+      required: true
+      content:
+        application/json:
+          schema:
+            $ref: '#/components/schemas/CommentUpdate'
     Patch:
       required: true
       content:
@@ -1344,6 +1366,11 @@  components:
               additionalProperties:
                 type: string
           readOnly: true
+    CommentDetail:
+      allOf:
+        - $ref: '#/components/schemas/Comment'
+    CommentUpdate:
+      type: object
     CoverList:
       type: object
       properties:
@@ -2175,6 +2202,8 @@  components:
           items:
             type: string
           readOnly: true
+    ErrorCommentUpdate:
+      type: object
     ErrorPatchUpdate:
       type: object
       properties:
diff --git a/docs/api/schemas/v1.1/patchwork.yaml b/docs/api/schemas/v1.1/patchwork.yaml
index 3b75c54..5ca168a 100644
--- a/docs/api/schemas/v1.1/patchwork.yaml
+++ b/docs/api/schemas/v1.1/patchwork.yaml
@@ -459,14 +459,14 @@  paths:
                 $ref: '#/components/schemas/Error'
       tags:
         - patches
-  /api/1.1/patches/{id}/comments/:
+  /api/1.1/patches/{patch_id}/comments/:
     parameters:
       - in: path
-        name: id
+        name: patch_id
         description: A unique integer value identifying the parent patch.
         required: true
         schema:
-          title: ID
+          title: Patch ID
           type: integer
     get:
       description: List comments
@@ -496,6 +496,22 @@  paths:
                 $ref: '#/components/schemas/Error'
       tags:
         - comments
+  /api/1.1/patches/{patch_id}/comments/{comment_id}/:
+    parameters:
+      - in: path
+        name: patch_id
+        description: A unique integer value identifying the parent patch.
+        required: true
+        schema:
+          title: Patch ID
+          type: integer
+      - in: path
+        name: comment_id
+        description: A unique integer value identifying this comment.
+        required: true
+        schema:
+          title: Comment ID
+          type: integer
   /api/1.1/patches/{patch_id}/checks/:
     parameters:
       - in: path
@@ -1091,6 +1107,12 @@  components:
         application/x-www-form-urlencoded:
           schema:
             $ref: '#/components/schemas/CheckCreate'
+    Comment:
+      required: true
+      content:
+        application/json:
+          schema:
+            $ref: '#/components/schemas/CommentUpdate'
     Patch:
       required: true
       content:
@@ -1354,6 +1376,11 @@  components:
               additionalProperties:
                 type: string
           readOnly: true
+    CommentDetail:
+      allOf:
+        - $ref: '#/components/schemas/Comment'
+    CommentUpdate:
+      type: object
     CoverList:
       type: object
       properties:
@@ -2241,6 +2268,8 @@  components:
           items:
             type: string
           readOnly: true
+    ErrorCommentUpdate:
+      type: object
     ErrorPatchUpdate:
       type: object
       properties:
diff --git a/docs/api/schemas/v1.2/patchwork.yaml b/docs/api/schemas/v1.2/patchwork.yaml
index 17d948a..289d747 100644
--- a/docs/api/schemas/v1.2/patchwork.yaml
+++ b/docs/api/schemas/v1.2/patchwork.yaml
@@ -598,14 +598,14 @@  paths:
                 $ref: '#/components/schemas/Error'
       tags:
         - patches
-  /api/1.2/patches/{id}/comments/:
+  /api/1.2/patches/{patch_id}/comments/:
     parameters:
       - in: path
-        name: id
+        name: patch_id
         description: A unique integer value identifying the parent patch.
         required: true
         schema:
-          title: ID
+          title: Patch ID
           type: integer
     get:
       description: List comments
@@ -635,6 +635,22 @@  paths:
                 $ref: '#/components/schemas/Error'
       tags:
         - comments
+  /api/1.2/patches/{patch_id}/comments/{comment_id}/:
+    parameters:
+      - in: path
+        name: patch_id
+        description: A unique integer value identifying the parent patch.
+        required: true
+        schema:
+          title: Patch ID
+          type: integer
+      - in: path
+        name: comment_id
+        description: A unique integer value identifying this comment.
+        required: true
+        schema:
+          title: Comment ID
+          type: integer
   /api/1.2/patches/{patch_id}/checks/:
     parameters:
       - in: path
@@ -1242,6 +1258,12 @@  components:
         application/x-www-form-urlencoded:
           schema:
             $ref: '#/components/schemas/CheckCreate'
+    Comment:
+      required: true
+      content:
+        application/json:
+          schema:
+            $ref: '#/components/schemas/CommentUpdate'
     Patch:
       required: true
       content:
@@ -1528,6 +1550,11 @@  components:
               additionalProperties:
                 type: string
           readOnly: true
+    CommentDetail:
+      allOf:
+        - $ref: '#/components/schemas/Comment'
+    CommentUpdate:
+      type: object
     CoverList:
       type: object
       properties:
@@ -2557,6 +2584,8 @@  components:
           items:
             type: string
           readOnly: true
+    ErrorCommentUpdate:
+      type: object
     ErrorPatchUpdate:
       type: object
       properties:
diff --git a/docs/api/schemas/v1.3/patchwork.yaml b/docs/api/schemas/v1.3/patchwork.yaml
new file mode 100644
index 0000000..2b100b4
--- /dev/null
+++ b/docs/api/schemas/v1.3/patchwork.yaml
@@ -0,0 +1,2749 @@ 
+# DO NOT EDIT THIS FILE. It is generated from a template. Changes should be
+# proposed against the template and updated files generated using the
+# 'generate-schemas.py' tool
+---
+openapi: '3.0.0'
+info:
+  title: Patchwork API
+  description: >
+    Patchwork is a web-based patch tracking system designed to facilitate the
+    contribution and management of contributions to an open-source project.
+  contact:
+    email: patchwork@lists.ozlabs.org
+  license:
+    name: GPL v2 License
+    url: https://www.gnu.org/licenses/gpl-2.0.html
+  version: '1.3'
+paths:
+  /api/1.3/:
+    get:
+      description: List API resources.
+      operationId: api_list
+      parameters: []
+      responses:
+        '200':
+          description: ''
+          content:
+            application/json:
+              schema:
+                $ref: '#/components/schemas/Index'
+      tags:
+        - api
+  /api/1.3/bundles/:
+    get:
+      description: List bundles.
+      operationId: bundles_list
+      parameters:
+        - $ref: '#/components/parameters/Page'
+        - $ref: '#/components/parameters/PageSize'
+        - $ref: '#/components/parameters/Order'
+        - $ref: '#/components/parameters/Search'
+        - in: query
+          name: project
+          description: An ID or linkname of a project to filter bundles by.
+          schema:
+            title: ''
+            type: string
+        - in: query
+          name: owner
+          description: An ID or username of a user to filter bundles by.
+          schema:
+            title: ''
+            type: string
+        - in: query
+          name: public
+          description: Show only public (`true`) or private (`false`) bundles.
+          schema:
+            title: ''
+            type: string
+            enum:
+              - 'true'
+              - 'false'
+      responses:
+        '200':
+          description: ''
+          headers:
+            Link:
+              $ref: '#/components/headers/Link'
+          content:
+            application/json:
+              schema:
+                type: array
+                items:
+                  $ref: '#/components/schemas/Bundle'
+      tags:
+        - bundles
+    post:
+      description: Create a bundle.
+      operationId: bundles_create
+#      security:
+#        - basicAuth: []
+#        - apiKeyAuth: []
+      requestBody:
+        $ref: '#/components/requestBodies/Bundle'
+      responses:
+        '201':
+          description: ''
+          content:
+            application/json:
+              schema:
+                $ref: '#/components/schemas/Bundle'
+        '400':
+          description: Invalid Request
+          content:
+            application/json:
+              schema:
+                $ref: '#/components/schemas/ErrorBundleCreateUpdate'
+        '403':
+          description: Forbidden
+          content:
+            application/json:
+              schema:
+                $ref: '#/components/schemas/Error'
+      tags:
+        - bundles
+  /api/1.3/bundles/{id}/:
+    parameters:
+      - in: path
+        name: id
+        required: true
+        description: A unique integer value identifying this bundle.
+        schema:
+          title: ID
+          type: integer
+    get:
+      description: Show a bundle.
+      operationId: bundles_read
+      responses:
+        '200':
+          description: ''
+          content:
+            application/json:
+              schema:
+                $ref: '#/components/schemas/Bundle'
+        '404':
+          description: Not found
+          content:
+            application/json:
+              schema:
+                $ref: '#/components/schemas/Error'
+      tags:
+        - bundles
+    patch:
+      description: Update a bundle (partial).
+      operationId: bundles_partial_update
+#      security:
+#        - basicAuth: []
+#        - apiKeyAuth: []
+      requestBody:
+        $ref: '#/components/requestBodies/Bundle'
+      responses:
+        '200':
+          description: ''
+          content:
+            application/json:
+              schema:
+                $ref: '#/components/schemas/Bundle'
+        '400':
+          description: Bad request
+          content:
+            application/json:
+              schema:
+                $ref: '#/components/schemas/ErrorBundleCreateUpdate'
+        '403':
+          description: Forbidden
+          content:
+            application/json:
+              schema:
+                $ref: '#/components/schemas/Error'
+        '404':
+          description: Not found
+          content:
+            application/json:
+              schema:
+                $ref: '#/components/schemas/Error'
+      tags:
+        - bundles
+    put:
+      description: Update a bundle.
+      operationId: bundles_update
+#      security:
+#        - basicAuth: []
+#        - apiKeyAuth: []
+      requestBody:
+        $ref: '#/components/requestBodies/Bundle'
+      responses:
+        '200':
+          description: ''
+          content:
+            application/json:
+              schema:
+                $ref: '#/components/schemas/Bundle'
+        '400':
+          description: Bad request
+          content:
+            application/json:
+              schema:
+                $ref: '#/components/schemas/ErrorBundleCreateUpdate'
+        '403':
+          description: Forbidden
+          content:
+            application/json:
+              schema:
+                $ref: '#/components/schemas/Error'
+        '404':
+          description: Not found
+          content:
+            application/json:
+              schema:
+                $ref: '#/components/schemas/Error'
+      tags:
+        - bundles
+  /api/1.3/covers/:
+    get:
+      description: List cover letters.
+      operationId: covers_list
+      parameters:
+        - $ref: '#/components/parameters/Page'
+        - $ref: '#/components/parameters/PageSize'
+        - $ref: '#/components/parameters/Order'
+        - $ref: '#/components/parameters/Search'
+        - $ref: '#/components/parameters/BeforeFilter'
+        - $ref: '#/components/parameters/SinceFilter'
+        - in: query
+          name: project
+          description: >
+            An ID or linkname of a project to filter cover letters by.
+          schema:
+            title: ''
+            type: string
+        - in: query
+          name: series
+          description: An ID of a series to filter cover letters by.
+          schema:
+            title: ''
+            type: string
+        - in: query
+          name: submitter
+          description: >
+            An ID or email address of a person to filter cover letters by.
+          schema:
+            title: ''
+            type: string
+        - in: query
+          name: msgid
+          description: >
+            The cover message-id as a case-sensitive string, without leading or
+            trailing angle brackets, to filter by.
+          schema:
+            title: ''
+            type: string
+      responses:
+        '200':
+          description: ''
+          headers:
+            Link:
+              $ref: '#/components/headers/Link'
+          content:
+            application/json:
+              schema:
+                type: array
+                items:
+                  $ref: '#/components/schemas/CoverList'
+      tags:
+        - covers
+  /api/1.3/covers/{id}/:
+    parameters:
+      - in: path
+        name: id
+        description: A unique integer value identifying this cover letter.
+        required: true
+        schema:
+          title: ID
+          type: integer
+    get:
+      description: Show a cover letter.
+      operationId: covers_read
+      responses:
+        '200':
+          description: ''
+          content:
+            application/json:
+              schema:
+                $ref: '#/components/schemas/CoverDetail'
+        '404':
+          description: Not found
+          content:
+            application/json:
+              schema:
+                $ref: '#/components/schemas/Error'
+      tags:
+        - covers
+  /api/1.3/covers/{id}/comments/:
+    parameters:
+      - in: path
+        name: id
+        description: >
+          A unique integer value identifying the parent cover letter.
+        required: true
+        schema:
+          title: ID
+          type: integer
+    get:
+      description: List comments
+      operationId: cover_comments_list
+      parameters:
+        - $ref: '#/components/parameters/Page'
+        - $ref: '#/components/parameters/PageSize'
+        - $ref: '#/components/parameters/Order'
+        - $ref: '#/components/parameters/Search'
+      responses:
+        '200':
+          description: ''
+          headers:
+            Link:
+              $ref: '#/components/headers/Link'
+          content:
+            application/json:
+              schema:
+                type: array
+                items:
+                  $ref: '#/components/schemas/Comment'
+        '404':
+          description: Not found
+          content:
+            application/json:
+              schema:
+                $ref: '#/components/schemas/Error'
+      tags:
+        - comments
+  /api/1.3/events/:
+    get:
+      description: List events.
+      operationId: events_list
+      parameters:
+        - $ref: '#/components/parameters/Page'
+        - $ref: '#/components/parameters/PageSize'
+        - $ref: '#/components/parameters/Order'
+        - $ref: '#/components/parameters/Search'
+        - $ref: '#/components/parameters/BeforeFilter'
+        - $ref: '#/components/parameters/SinceFilter'
+        - in: query
+          name: project
+          description: An ID or linkname of a project to filter events by.
+          schema:
+            title: ''
+            type: string
+        - in: query
+          name: category
+          description: An event category to filter events by.
+          schema:
+            title: ''
+            type: string
+            enum:
+              - cover-created
+              - patch-created
+              - patch-completed
+              - patch-state-changed
+              - patch-relation-changed
+              - patch-delegated
+              - check-created
+              - series-created
+              - series-completed
+        - in: query
+          name: series
+          description: An ID of a series to filter events by.
+          schema:
+            title: ''
+            type: integer
+        - in: query
+          name: patch
+          description: An ID of a patch to filter events by.
+          schema:
+            title: ''
+            type: integer
+        - in: query
+          name: cover
+          description: An ID of a cover letter to filter events by.
+          schema:
+            title: ''
+            type: integer
+      responses:
+        '200':
+          description: ''
+          headers:
+            Link:
+              $ref: '#/components/headers/Link'
+          content:
+            application/json:
+              schema:
+                type: array
+                items:
+                  anyOf:
+                    - $ref: '#/components/schemas/EventCoverCreated'
+                    - $ref: '#/components/schemas/EventPatchCreated'
+                    - $ref: '#/components/schemas/EventPatchCompleted'
+                    - $ref: '#/components/schemas/EventPatchStateChanged'
+                    - $ref: '#/components/schemas/EventPatchRelationChanged'
+                    - $ref: '#/components/schemas/EventPatchDelegated'
+                    - $ref: '#/components/schemas/EventCheckCreated'
+                    - $ref: '#/components/schemas/EventSeriesCreated'
+                    - $ref: '#/components/schemas/EventSeriesCompleted'
+                  discriminator:
+                    propertyName: category
+                    mapping:
+                      cover-created: '#/components/schemas/EventCoverCreated'
+                      patch-created: '#/components/schemas/EventPatchCreated'
+                      patch-completed: >
+                        '#/components/schemas/EventPatchCompleted'
+                      patch-state-changed: >
+                        '#/components/schemas/EventPatchStateChanged'
+                      patch-relation-changed: >
+                        '#/components/schemas/EventPatchRelationChanged'
+                      patch-delegated: >
+                        '#/components/schemas/EventPatchDelegated'
+                      check-created: '#/components/schemas/EventCheckCreated'
+                      series-created: '#/components/schemas/EventSeriesCreated'
+                      series-completed: >
+                        '#/components/schemas/EventSeriesCompleted'
+      tags:
+        - events
+  /api/1.3/patches/:
+    get:
+      description: List patches.
+      operationId: patches_list
+      parameters:
+        - $ref: '#/components/parameters/Page'
+        - $ref: '#/components/parameters/PageSize'
+        - $ref: '#/components/parameters/Order'
+        - $ref: '#/components/parameters/Search'
+        - $ref: '#/components/parameters/BeforeFilter'
+        - $ref: '#/components/parameters/SinceFilter'
+        - in: query
+          name: project
+          description: An ID or linkname of a project to filter patches by.
+          schema:
+            title: ''
+            type: string
+        - in: query
+          name: series
+          description: An ID of a series to filter patches by.
+          schema:
+            title: ''
+            type: integer
+        - in: query
+          name: submitter
+          description: >
+            An ID or email address of a person to filter patches by.
+          schema:
+            title: ''
+            type: string
+        - in: query
+          name: delegate
+          description: An ID or username of a user to filter patches by.
+          schema:
+            title: ''
+            type: string
+        - in: query
+          name: state
+          description: A slug representation of a state to filter patches by.
+          schema:
+            title: ''
+            type: string
+        - in: query
+          name: archived
+          description: >
+            Show only archived (`true`) or non-archived (`false`) patches.
+          schema:
+            title: ''
+            type: string
+            enum:
+              - 'true'
+              - 'false'
+        - in: query
+          name: hash
+          description: >
+            The patch hash as a case-insensitive hexadecimal string, to filter by.
+          schema:
+            title: ''
+            type: string
+        - in: query
+          name: msgid
+          description: >
+            The patch message-id as a case-sensitive string, without leading or
+            trailing angle brackets, to filter by.
+          schema:
+            title: ''
+            type: string
+      responses:
+        '200':
+          description: ''
+          headers:
+            Link:
+              $ref: '#/components/headers/Link'
+          content:
+            application/json:
+              schema:
+                type: array
+                items:
+                  $ref: '#/components/schemas/PatchList'
+      tags:
+        - patches
+  /api/1.3/patches/{id}/:
+    parameters:
+      - in: path
+        name: id
+        description: A unique integer value identifying this patch.
+        required: true
+        schema:
+          title: ID
+          type: integer
+    get:
+      description: Show a patch.
+      operationId: patches_read
+      responses:
+        '200':
+          description: ''
+          content:
+            application/json:
+              schema:
+                $ref: '#/components/schemas/PatchDetail'
+        '404':
+          description: Not found
+          content:
+            application/json:
+              schema:
+                $ref: '#/components/schemas/Error'
+      tags:
+        - patches
+    patch:
+      description: Update a patch (partial).
+      operationId: patches_partial_update
+#      security:
+#        - basicAuth: []
+#        - apiKeyAuth: []
+      requestBody:
+        $ref: '#/components/requestBodies/Patch'
+      responses:
+        '200':
+          description: ''
+          content:
+            application/json:
+              schema:
+                $ref: '#/components/schemas/PatchDetail'
+        '400':
+          description: Invalid Request
+          content:
+            application/json:
+              schema:
+                $ref: '#/components/schemas/ErrorPatchUpdate'
+        '403':
+          description: Forbidden
+          content:
+            application/json:
+              schema:
+                $ref: '#/components/schemas/Error'
+        '404':
+          description: Not found
+          content:
+            application/json:
+              schema:
+                $ref: '#/components/schemas/Error'
+        '409':
+          description: Conflict
+          content:
+            application/json:
+              schema:
+                $ref: '#/components/schemas/Error'
+      tags:
+        - patches
+    put:
+      description: Update a patch.
+      operationId: patches_update
+#      security:
+#        - basicAuth: []
+#        - apiKeyAuth: []
+      requestBody:
+        $ref: '#/components/requestBodies/Patch'
+      responses:
+        '200':
+          description: ''
+          content:
+            application/json:
+              schema:
+                $ref: '#/components/schemas/PatchDetail'
+        '400':
+          description: Invalid Request
+          content:
+            application/json:
+              schema:
+                $ref: '#/components/schemas/ErrorPatchUpdate'
+        '403':
+          description: Forbidden
+          content:
+            application/json:
+              schema:
+                $ref: '#/components/schemas/Error'
+        '404':
+          description: Not found
+          content:
+            application/json:
+              schema:
+                $ref: '#/components/schemas/Error'
+        '409':
+          description: Conflict
+          content:
+            application/json:
+              schema:
+                $ref: '#/components/schemas/Error'
+      tags:
+        - patches
+  /api/1.3/patches/{patch_id}/comments/:
+    parameters:
+      - in: path
+        name: patch_id
+        description: A unique integer value identifying the parent patch.
+        required: true
+        schema:
+          title: Patch ID
+          type: integer
+    get:
+      description: List comments
+      operationId: patch_comments_list
+      parameters:
+        - $ref: '#/components/parameters/Page'
+        - $ref: '#/components/parameters/PageSize'
+        - $ref: '#/components/parameters/Order'
+        - $ref: '#/components/parameters/Search'
+      responses:
+        '200':
+          description: ''
+          headers:
+            Link:
+              $ref: '#/components/headers/Link'
+          content:
+            application/json:
+              schema:
+                type: array
+                items:
+                  $ref: '#/components/schemas/Comment'
+        '404':
+          description: Not found
+          content:
+            application/json:
+              schema:
+                $ref: '#/components/schemas/Error'
+      tags:
+        - comments
+  /api/1.3/patches/{patch_id}/comments/{comment_id}/:
+    parameters:
+      - in: path
+        name: patch_id
+        description: A unique integer value identifying the parent patch.
+        required: true
+        schema:
+          title: Patch ID
+          type: integer
+      - in: path
+        name: comment_id
+        description: A unique integer value identifying this comment.
+        required: true
+        schema:
+          title: Comment ID
+          type: integer
+    get:
+      description: Show a patch comment.
+      operationId: patch_comments_read
+      responses:
+        '200':
+          description: ''
+          content:
+            application/json:
+              schema:
+                $ref: '#/components/schemas/CommentDetail'
+        '404':
+          description: Not found
+          content:
+            application/json:
+              schema:
+                $ref: '#/components/schemas/Error'
+      tags:
+        - comments
+    patch:
+      description: Update a patch comment (partial).
+      operationId: patch_comments_partial_update
+#      security:
+#        - basicAuth: []
+#        - apiKeyAuth: []
+      requestBody:
+        $ref: '#/components/requestBodies/Comment'
+      responses:
+        '200':
+          description: ''
+          content:
+            application/json:
+              schema:
+                $ref: '#/components/schemas/CommentDetail'
+        '400':
+          description: Invalid Request
+          content:
+            application/json:
+              schema:
+                $ref: '#/components/schemas/ErrorCommentUpdate'
+        '403':
+          description: Forbidden
+          content:
+            application/json:
+              schema:
+                $ref: '#/components/schemas/Error'
+        '404':
+          description: Not found
+          content:
+            application/json:
+              schema:
+                $ref: '#/components/schemas/Error'
+      tags:
+        - comments
+    put:
+      description: Update a patch comment.
+      operationId: patch_comments_update
+#      security:
+#        - basicAuth: []
+#        - apiKeyAuth: []
+      requestBody:
+        $ref: '#/components/requestBodies/Comment'
+      responses:
+        '200':
+          description: ''
+          content:
+            application/json:
+              schema:
+                $ref: '#/components/schemas/CommentDetail'
+        '400':
+          description: Invalid Request
+          content:
+            application/json:
+              schema:
+                $ref: '#/components/schemas/ErrorCommentUpdate'
+        '403':
+          description: Forbidden
+          content:
+            application/json:
+              schema:
+                $ref: '#/components/schemas/Error'
+        '404':
+          description: Not found
+          content:
+            application/json:
+              schema:
+                $ref: '#/components/schemas/Error'
+      tags:
+        - comments
+  /api/1.3/patches/{patch_id}/checks/:
+    parameters:
+      - in: path
+        name: patch_id
+        description: A unique integer value identifying the parent patch.
+        required: true
+        schema:
+          title: Patch ID
+          type: integer
+    get:
+      description: List checks.
+      operationId: checks_list
+      parameters:
+        - $ref: '#/components/parameters/Page'
+        - $ref: '#/components/parameters/PageSize'
+        - $ref: '#/components/parameters/Order'
+        - $ref: '#/components/parameters/Search'
+        - $ref: '#/components/parameters/BeforeFilter'
+        - $ref: '#/components/parameters/SinceFilter'
+        - in: query
+          name: user
+          description: An ID or username of a user to filter checks by.
+          schema:
+            title: ''
+            type: string
+        - in: query
+          name: state
+          description: A check state to filter checks by.
+          schema:
+            title: ''
+            type: string
+            enum:
+              - pending
+              - success
+              - warning
+              - fail
+        - in: query
+          name: context
+          description: A check context to filter checks by.
+          schema:
+            title: ''
+            type: string
+      responses:
+        '200':
+          description: ''
+          headers:
+            Link:
+              $ref: '#/components/headers/Link'
+          content:
+            application/json:
+              schema:
+                type: array
+                items:
+                  $ref: '#/components/schemas/Check'
+        '404':
+          description: Not found
+          content:
+            application/json:
+              schema:
+                $ref: '#/components/schemas/Error'
+      tags:
+        - checks
+    post:
+      description: Create a check.
+      operationId: checks_create
+#      security:
+#        - basicAuth: []
+#        - apiKeyAuth: []
+      requestBody:
+        $ref: '#/components/requestBodies/Check'
+      responses:
+        '201':
+          description: ''
+          content:
+            application/json:
+              schema:
+                $ref: '#/components/schemas/Check'
+        '400':
+          description: Invalid Request
+          content:
+            application/json:
+              schema:
+                $ref: '#/components/schemas/ErrorCheckCreate'
+        '403':
+          description: Forbidden
+          content:
+            application/json:
+              schema:
+                $ref: '#/components/schemas/Error'
+        '404':
+          description: Not found
+          content:
+            application/json:
+              schema:
+                $ref: '#/components/schemas/Error'
+      tags:
+        - checks
+  /api/1.3/patches/{patch_id}/checks/{check_id}/:
+    parameters:
+      - in: path
+        name: patch_id
+        description: A unique integer value identifying the parent patch.
+        required: true
+        schema:
+          title: Patch ID
+          type: integer
+      - in: path
+        name: check_id
+        description: A unique integer value identifying this check.
+        required: true
+        schema:
+          title: Check ID
+          type: integer
+    get:
+      description: Show a check.
+      operationId: checks_read
+      responses:
+        '200':
+          description: ''
+          content:
+            application/json:
+              schema:
+                $ref: '#/components/schemas/Check'
+        '404':
+          description: Not found
+          content:
+            application/json:
+              schema:
+                $ref: '#/components/schemas/Error'
+      tags:
+        - checks
+  /api/1.3/people/:
+    get:
+      description: List people.
+      operationId: people_list
+#      security:
+#        - basicAuth: []
+#        - apiKeyAuth: []
+      parameters:
+        - $ref: '#/components/parameters/Page'
+        - $ref: '#/components/parameters/PageSize'
+        - $ref: '#/components/parameters/Order'
+        - $ref: '#/components/parameters/Search'
+      responses:
+        '200':
+          description: ''
+          headers:
+            Link:
+              $ref: '#/components/headers/Link'
+          content:
+            application/json:
+              schema:
+                type: array
+                items:
+                  $ref: '#/components/schemas/Person'
+        '403':
+          description: Forbidden
+          content:
+            application/json:
+              schema:
+                $ref: '#/components/schemas/Error'
+      tags:
+        - people
+  /api/1.3/people/{id}/:
+    parameters:
+      - in: path
+        name: id
+        description: A unique integer value identifying this person.
+        required: true
+        schema:
+          title: ID
+          type: integer
+    get:
+      description: Show a person.
+      operationId: people_read
+#      security:
+#        - basicAuth: []
+#        - apiKeyAuth: []
+      responses:
+        '200':
+          description: ''
+          content:
+            application/json:
+              schema:
+                $ref: '#/components/schemas/Person'
+        '403':
+          description: Forbidden
+          content:
+            application/json:
+              schema:
+                $ref: '#/components/schemas/Error'
+        '404':
+          description: Not found
+          content:
+            application/json:
+              schema:
+                $ref: '#/components/schemas/Error'
+      tags:
+        - people
+  /api/1.3/projects/:
+    get:
+      description: List projects.
+      operationId: projects_list
+      parameters:
+        - $ref: '#/components/parameters/Page'
+        - $ref: '#/components/parameters/PageSize'
+        - $ref: '#/components/parameters/Order'
+        - $ref: '#/components/parameters/Search'
+      responses:
+        '200':
+          description: ''
+          headers:
+            Link:
+              $ref: '#/components/headers/Link'
+          content:
+            application/json:
+              schema:
+                type: array
+                items:
+                  $ref: '#/components/schemas/Project'
+      tags:
+        - projects
+  /api/1.3/projects/{id}/:
+    parameters:
+      - in: path
+        name: id
+        description: A unique integer value identifying this project.
+        required: true
+        schema:
+          title: ID
+          # TODO: Add regex?
+          type: string
+    get:
+      description: Show a project.
+      operationId: projects_read
+      responses:
+        '200':
+          description: ''
+          content:
+            application/json:
+              schema:
+                $ref: '#/components/schemas/Project'
+        '404':
+          description: Not found
+          content:
+            application/json:
+              schema:
+                $ref: '#/components/schemas/Error'
+      tags:
+        - projects
+    patch:
+      description: Update a project (partial).
+      operationId: projects_partial_update
+#      security:
+#        - basicAuth: []
+#        - apiKeyAuth: []
+      requestBody:
+        $ref: '#/components/requestBodies/Project'
+      responses:
+        '200':
+          description: ''
+          content:
+            application/json:
+              schema:
+                $ref: '#/components/schemas/Project'
+        '400':
+          description: Bad request
+          content:
+            application/json:
+              schema:
+                $ref: '#/components/schemas/ErrorProjectUpdate'
+        '403':
+          description: Forbidden
+          content:
+            application/json:
+              schema:
+                $ref: '#/components/schemas/Error'
+        '404':
+          description: Not found
+          content:
+            application/json:
+              schema:
+                $ref: '#/components/schemas/Error'
+      tags:
+        - projects
+    put:
+      description: Update a project.
+      operationId: projects_update
+#      security:
+#        - basicAuth: []
+#        - apiKeyAuth: []
+      requestBody:
+        $ref: '#/components/requestBodies/Project'
+      responses:
+        '200':
+          description: ''
+          content:
+            application/json:
+              schema:
+                $ref: '#/components/schemas/Project'
+        '400':
+          description: Bad request
+          content:
+            application/json:
+              schema:
+                $ref: '#/components/schemas/ErrorProjectUpdate'
+        '403':
+          description: Forbidden
+          content:
+            application/json:
+              schema:
+                $ref: '#/components/schemas/Error'
+        '404':
+          description: Not found
+          content:
+            application/json:
+              schema:
+                $ref: '#/components/schemas/Error'
+      tags:
+        - projects
+  /api/1.3/series/:
+    get:
+      description: List series.
+      operationId: series_list
+      parameters:
+        - $ref: '#/components/parameters/Page'
+        - $ref: '#/components/parameters/PageSize'
+        - $ref: '#/components/parameters/Order'
+        - $ref: '#/components/parameters/Search'
+        - $ref: '#/components/parameters/BeforeFilter'
+        - $ref: '#/components/parameters/SinceFilter'
+        - in: query
+          name: submitter
+          description: An ID or email address of a person to filter series by.
+          schema:
+            title: ''
+            type: string
+        - in: query
+          name: project
+          description: An ID or linkname of a project to filter series by.
+          schema:
+            title: ''
+            type: string
+      responses:
+        '200':
+          description: ''
+          headers:
+            Link:
+              $ref: '#/components/headers/Link'
+          content:
+            application/json:
+              schema:
+                type: array
+                items:
+                  $ref: '#/components/schemas/Series'
+      tags:
+        - series
+  /api/1.3/series/{id}/:
+    parameters:
+      - in: path
+        name: id
+        description: A unique integer value identifying this series.
+        required: true
+        schema:
+          title: ID
+          type: integer
+    get:
+      description: Show a series.
+      operationId: series_read
+      responses:
+        '200':
+          description: ''
+          content:
+            application/json:
+              schema:
+                $ref: '#/components/schemas/Series'
+        '404':
+          description: Not found
+          content:
+            application/json:
+              schema:
+                $ref: '#/components/schemas/Error'
+      tags:
+        - series
+  /api/1.3/users/:
+    get:
+      description: List users.
+      operationId: users_list
+#      security:
+#        - basicAuth: []
+#        - apiKeyAuth: []
+      parameters:
+        - $ref: '#/components/parameters/Page'
+        - $ref: '#/components/parameters/PageSize'
+        - $ref: '#/components/parameters/Order'
+        - $ref: '#/components/parameters/Search'
+      responses:
+        '200':
+          description: ''
+          headers:
+            Link:
+              $ref: '#/components/headers/Link'
+          content:
+            application/json:
+              schema:
+                type: array
+                items:
+                  $ref: '#/components/schemas/User'
+        '403':
+          description: Forbidden
+          content:
+            application/json:
+              schema:
+                $ref: '#/components/schemas/Error'
+      tags:
+        - users
+  /api/1.3/users/{id}/:
+    parameters:
+      - in: path
+        name: id
+        description: A unique integer value identifying this user.
+        required: true
+        schema:
+          title: ID
+          type: integer
+    get:
+      description: Show a user.
+      operationId: users_read
+#      security:
+#        - basicAuth: []
+#        - apiKeyAuth: []
+      responses:
+        '200':
+          description: ''
+          content:
+            application/json:
+              schema:
+                $ref: '#/components/schemas/UserDetail'
+        '403':
+          description: Forbidden
+          content:
+            application/json:
+              schema:
+                $ref: '#/components/schemas/Error'
+        '404':
+          description: Not found
+          content:
+            application/json:
+              schema:
+                $ref: '#/components/schemas/Error'
+      tags:
+        - users
+    patch:
+      description: Update a user (partial).
+      operationId: users_partial_update
+#      security:
+#        - basicAuth: []
+#        - apiKeyAuth: []
+      requestBody:
+        $ref: '#/components/requestBodies/User'
+      responses:
+        '200':
+          description: ''
+          content:
+            application/json:
+              schema:
+                $ref: '#/components/schemas/UserDetail'
+        '400':
+          description: Bad request
+          content:
+            application/json:
+              schema:
+                $ref: '#/components/schemas/ErrorUserUpdate'
+        '403':
+          description: Forbidden
+          content:
+            application/json:
+              schema:
+                $ref: '#/components/schemas/Error'
+        '404':
+          description: Not found
+          content:
+            application/json:
+              schema:
+                $ref: '#/components/schemas/Error'
+      tags:
+        - users
+    put:
+      description: Update a user.
+      operationId: users_update
+#      security:
+#        - basicAuth: []
+#        - apiKeyAuth: []
+      requestBody:
+        $ref: '#/components/requestBodies/User'
+      responses:
+        '200':
+          description: ''
+          content:
+            application/json:
+              schema:
+                $ref: '#/components/schemas/UserDetail'
+        '400':
+          description: Bad request
+          content:
+            application/json:
+              schema:
+                $ref: '#/components/schemas/ErrorUserUpdate'
+        '403':
+          description: Forbidden
+          content:
+            application/json:
+              schema:
+                $ref: '#/components/schemas/Error'
+        '404':
+          description: Not found
+          content:
+            application/json:
+              schema:
+                $ref: '#/components/schemas/Error'
+      tags:
+        - users
+components:
+  securitySchemes:
+    basicAuth:
+      type: http
+      scheme: basic
+    apiKeyAuth:
+      type: http
+      scheme: bearer
+  parameters:
+    Page:
+      in: query
+      name: page
+      description: A page number within the paginated result set.
+      schema:
+        title: Page
+        type: integer
+    PageSize:
+      in: query
+      name: per_page
+      description: Number of results to return per page.
+      schema:
+        title: Page size
+        type: integer
+    Order:
+      in: query
+      name: order
+      description: Which field to use when ordering the results.
+      schema:
+        title: Ordering
+        type: string
+    Search:
+      in: query
+      name: q
+      description: A search term.
+      schema:
+        title: Search
+        type: string
+    BeforeFilter:
+      in: query
+      name: before
+      description: Latest date-time to retrieve results for.
+      schema:
+        title: ''
+        type: string
+    SinceFilter:
+      in: query
+      name: since
+      description: Earliest date-time to retrieve results for.
+      schema:
+        title: ''
+        type: string
+  headers:
+    Link:
+      description: >
+        Links to related resources, in the format defined by
+        [RFC 5988](https://tools.ietf.org/html/rfc5988#section-5).
+        This will include a link with relation type `next` to the
+        next page, if there is a next page.
+      schema:
+        type: string
+  requestBodies:
+    Bundle:
+      required: true
+      content:
+        application/json:
+          schema:
+            $ref: '#/components/schemas/BundleCreateUpdate'
+        multipart/form-data:
+          schema:
+            $ref: '#/components/schemas/BundleCreateUpdate'
+        application/x-www-form-urlencoded:
+          schema:
+            $ref: '#/components/schemas/BundleCreateUpdate'
+    Check:
+      required: true
+      content:
+        application/json:
+          schema:
+            $ref: '#/components/schemas/CheckCreate'
+        multipart/form-data:
+          schema:
+            $ref: '#/components/schemas/CheckCreate'
+        application/x-www-form-urlencoded:
+          schema:
+            $ref: '#/components/schemas/CheckCreate'
+    Comment:
+      required: true
+      content:
+        application/json:
+          schema:
+            $ref: '#/components/schemas/CommentUpdate'
+    Patch:
+      required: true
+      content:
+        application/json:
+          schema:
+            $ref: '#/components/schemas/PatchUpdate'
+        multipart/form-data:
+          schema:
+            $ref: '#/components/schemas/PatchUpdate'
+        application/x-www-form-urlencoded:
+          schema:
+            $ref: '#/components/schemas/PatchUpdate'
+    Project:
+      required: true
+      content:
+        application/json:
+          schema:
+            $ref: '#/components/schemas/Project'
+        multipart/form-data:
+          schema:
+            $ref: '#/components/schemas/Project'
+        application/x-www-form-urlencoded:
+          schema:
+            $ref: '#/components/schemas/Project'
+    User:
+      required: true
+      content:
+        application/json:
+          schema:
+            $ref: '#/components/schemas/UserDetail'
+        multipart/form-data:
+          schema:
+            $ref: '#/components/schemas/UserDetail'
+        application/x-www-form-urlencoded:
+          schema:
+            $ref: '#/components/schemas/UserDetail'
+  schemas:
+    Index:
+      type: object
+      properties:
+        bundles:
+          title: Bundles URL
+          type: string
+          format: uri
+          readOnly: true
+        covers:
+          title: Covers URL
+          type: string
+          format: uri
+          readOnly: true
+        events:
+          title: Events URL
+          type: string
+          format: uri
+          readOnly: true
+        patches:
+          title: Patches URL
+          type: string
+          format: uri
+          readOnly: true
+        people:
+          title: People URL
+          type: string
+          format: uri
+          readOnly: true
+        projects:
+          title: Projects URL
+          type: string
+          format: uri
+          readOnly: true
+        users:
+          title: Users URL
+          type: string
+          format: uri
+          readOnly: true
+        series:
+          title: Series URL
+          type: string
+          format: uri
+          readOnly: true
+    Bundle:
+      required:
+        - name
+      type: object
+      properties:
+        id:
+          title: ID
+          type: integer
+          readOnly: true
+        url:
+          title: URL
+          type: string
+          format: uri
+          readOnly: true
+        web_url:
+          title: Web URL
+          type: string
+          format: uri
+          readOnly: true
+        project:
+          $ref: '#/components/schemas/ProjectEmbedded'
+        name:
+          title: Name
+          type: string
+          minLength: 1
+          maxLength: 50
+        owner:
+          type: object
+          title: Owner
+          readOnly: true
+          nullable: false
+          allOf:
+            - $ref: '#/components/schemas/UserEmbedded'
+        patches:
+          title: Patches
+          type: array
+          items:
+            $ref: '#/components/schemas/PatchEmbedded'
+          uniqueItems: true
+        public:
+          title: Public
+          type: boolean
+        mbox:
+          title: Mbox
+          type: string
+          format: uri
+          readOnly: true
+    BundleCreateUpdate:
+      type: object
+      required:
+        - name
+      properties:
+        name:
+          title: Name
+          type: string
+          minLength: 1
+          maxLength: 50
+        patches:
+          title: Patches
+          type: array
+          items:
+            type: integer
+          uniqueItems: true
+        public:
+          title: Public
+          type: boolean
+    Check:
+      type: object
+      properties:
+        id:
+          title: ID
+          type: integer
+          readOnly: true
+        url:
+          title: Url
+          type: string
+          format: uri
+          readOnly: true
+        user:
+          $ref: '#/components/schemas/UserEmbedded'
+        date:
+          title: Date
+          type: string
+          format: iso8601
+          readOnly: true
+        state:
+          title: State
+          description: The state of the check.
+          type: string
+          enum:
+            - pending
+            - success
+            - warning
+            - fail
+        target_url:
+          title: Target URL
+          description: >
+            The target URL to associate with this check. This should be
+            specific to the patch.
+          type: string
+          format: uri
+          maxLength: 200
+          nullable: true
+        context:
+          title: Context
+          description: >
+            A label to discern check from checks of other testing systems.
+          type: string
+          pattern: ^[-a-zA-Z0-9_]+$
+          minLength: 1
+          maxLength: 255
+        description:
+          title: Description
+          description: A brief description of the check.
+          type: string
+          nullable: true
+    CheckCreate:
+      type: object
+      required:
+       - state
+      properties:
+        state:
+          title: State
+          description: The state of the check.
+          type: string
+          enum:
+            - pending
+            - success
+            - warning
+            - fail
+        target_url:
+          title: Target URL
+          description:
+            The target URL to associate with this check. This should be
+            specific to the patch.
+          type: string
+          format: uri
+          maxLength: 200
+          nullable: true
+        context:
+          title: Context
+          description: >
+            A label to discern check from checks of other testing systems.
+          type: string
+          pattern: ^[-a-zA-Z0-9_]+$
+          minLength: 1
+          maxLength: 255
+        description:
+          title: Description
+          description: A brief description of the check.
+          type: string
+          nullable: true
+    Comment:
+      type: object
+      properties:
+        id:
+          title: ID
+          type: integer
+          readOnly: true
+        web_url:
+          title: Web URL
+          type: string
+          format: uri
+          readOnly: true
+        msgid:
+          title: Message ID
+          type: string
+          readOnly: true
+          minLength: 1
+          maxLength: 255
+        list_archive_url:
+          title: List archive URL
+          type: string
+          readOnly: true
+          nullable: true
+        date:
+          title: Date
+          type: string
+          format: iso8601
+          readOnly: true
+        subject:
+          title: Subject
+          type: string
+          readOnly: true
+        submitter:
+          type: object
+          title: Submitter
+          allOf:
+            - $ref: '#/components/schemas/PersonEmbedded'
+        content:
+          title: Content
+          type: string
+          readOnly: true
+          minLength: 1
+        headers:
+          title: Headers
+          anyOf:
+            - type: object
+              additionalProperties:
+                type: array
+                items:
+                  type: string
+            - type: object
+              additionalProperties:
+                type: string
+          readOnly: true
+    CommentDetail:
+      allOf:
+        - $ref: '#/components/schemas/Comment'
+        - properties:
+            patch:
+              $ref: '#/components/schemas/PatchEmbedded'
+            addressed:
+              title: Addressed
+              type: boolean
+    CommentUpdate:
+      type: object
+      properties:
+        addressed:
+          title: Addressed
+          type: boolean
+    CoverList:
+      type: object
+      properties:
+        id:
+          title: ID
+          type: integer
+          readOnly: true
+        url:
+          title: URL
+          type: string
+          format: uri
+          readOnly: true
+        web_url:
+          title: Web URL
+          type: string
+          format: uri
+          readOnly: true
+        project:
+          $ref: '#/components/schemas/ProjectEmbedded'
+        msgid:
+          title: Message ID
+          type: string
+          readOnly: true
+          minLength: 1
+          maxLength: 255
+        list_archive_url:
+          title: List archive URL
+          type: string
+          readOnly: true
+          nullable: true
+        date:
+          title: Date
+          type: string
+          format: iso8601
+          readOnly: true
+        name:
+          title: Name
+          type: string
+          readOnly: true
+          minLength: 1
+          maxLength: 255
+        submitter:
+          type: object
+          title: Submitter
+          readOnly: true
+          allOf:
+            - $ref: '#/components/schemas/PersonEmbedded'
+        mbox:
+          title: Mbox
+          type: string
+          format: uri
+          readOnly: true
+        series:
+          type: array
+          items:
+            $ref: '#/components/schemas/SeriesEmbedded'
+          readOnly: true
+        comments:
+          title: Comments
+          type: string
+          format: uri
+          readOnly: true
+    CoverDetail:
+      allOf:
+        - $ref: '#/components/schemas/CoverList'
+        - properties:
+            headers:
+              title: Headers
+              anyOf:
+                - type: object
+                  additionalProperties:
+                    type: array
+                    items:
+                      type: string
+                - type: object
+                  additionalProperties:
+                    type: string
+              readOnly: true
+            content:
+              title: Content
+              type: string
+              readOnly: true
+              minLength: 1
+    EventBase:
+      type: object
+      properties:
+        id:
+          title: ID
+          type: integer
+          readOnly: true
+        category:
+          title: Category
+          description: The category of the event.
+          type: string
+          readOnly: true
+        project:
+          $ref: '#/components/schemas/ProjectEmbedded'
+        date:
+          title: Date
+          description: The time this event was created.
+          type: string
+          format: iso8601
+          readOnly: true
+        actor:
+          type: object
+          title: Actor
+          description: The user that caused/created this event.
+          readOnly: true
+          nullable: true
+          allOf:
+            - $ref: '#/components/schemas/UserEmbedded'
+        payload:
+          type: object
+    EventCoverCreated:
+      allOf:
+        - $ref: '#/components/schemas/EventBase'
+        - type: object
+          properties:
+            category:
+              enum:
+                - cover-created
+            payload:
+              properties:
+                cover:
+                  $ref: '#/components/schemas/CoverEmbedded'
+    EventPatchCreated:
+      allOf:
+        - $ref: '#/components/schemas/EventBase'
+        - type: object
+          properties:
+            category:
+              enum:
+                - patch-created
+            payload:
+              properties:
+                patch:
+                  $ref: '#/components/schemas/PatchEmbedded'
+    EventPatchCompleted:
+      allOf:
+        - $ref: '#/components/schemas/EventBase'
+        - type: object
+          properties:
+            category:
+              enum:
+                - patch-completed
+            payload:
+              properties:
+                patch:
+                  $ref: '#/components/schemas/PatchEmbedded'
+                series:
+                  $ref: '#/components/schemas/SeriesEmbedded'
+    EventPatchStateChanged:
+      allOf:
+        - $ref: '#/components/schemas/EventBase'
+        - type: object
+          properties:
+            category:
+              enum:
+                - patch-state-changed
+            payload:
+              properties:
+                patch:
+                  $ref: '#/components/schemas/PatchEmbedded'
+                previous_state:
+                  title: Previous state
+                  type: string
+                current_state:
+                  title: Current state
+                  type: string
+    EventPatchRelationChanged:
+      allOf:
+        - $ref: '#/components/schemas/EventBase'
+        - type: object
+          properties:
+            category:
+              enum:
+                - patch-relation-changed
+            payload:
+              properties:
+                patch:
+                  $ref: '#/components/schemas/PatchEmbedded'
+                previous_relation:
+                  title: Previous relation
+                  type: string
+                  nullable: true
+                current_relation:
+                  title: Current relation
+                  type: string
+                  nullable: true
+    EventPatchDelegated:
+      allOf:
+        - $ref: '#/components/schemas/EventBase'
+        - type: object
+          properties:
+            category:
+              enum:
+                - patch-delegated
+            payload:
+              properties:
+                patch:
+                  $ref: '#/components/schemas/PatchEmbedded'
+                previous_delegate:
+                  $ref: '#/components/schemas/UserEmbedded'
+                current_delegate:
+                  $ref: '#/components/schemas/UserEmbedded'
+    EventCheckCreated:
+      allOf:
+        - $ref: '#/components/schemas/EventBase'
+        - type: object
+          properties:
+            category:
+              enum:
+                - check-created
+            payload:
+              properties:
+                patch:
+                  $ref: '#/components/schemas/PatchEmbedded'
+                check:
+                  $ref: '#/components/schemas/CheckEmbedded'
+    EventSeriesCreated:
+      allOf:
+        - $ref: '#/components/schemas/EventBase'
+        - type: object
+          properties:
+            category:
+              enum:
+                - series-created
+            payload:
+              properties:
+                series:
+                  $ref: '#/components/schemas/SeriesEmbedded'
+    EventSeriesCompleted:
+      allOf:
+        - $ref: '#/components/schemas/EventBase'
+        - type: object
+          properties:
+            category:
+              enum:
+                - series-completed
+            payload:
+              properties:
+                series:
+                  $ref: '#/components/schemas/SeriesEmbedded'
+    PatchList:
+      required:
+        - state
+        - delegate
+      type: object
+      properties:
+        id:
+          title: ID
+          type: integer
+          readOnly: true
+        url:
+          title: URL
+          type: string
+          format: uri
+          readOnly: true
+        web_url:
+          title: Web URL
+          type: string
+          format: uri
+          readOnly: true
+        project:
+          $ref: '#/components/schemas/ProjectEmbedded'
+        msgid:
+          title: Message ID
+          type: string
+          readOnly: true
+          minLength: 1
+          maxLength: 255
+        list_archive_url:
+          title: List archive URL
+          type: string
+          readOnly: true
+          nullable: true
+        date:
+          title: Date
+          type: string
+          format: iso8601
+          readOnly: true
+        name:
+          title: Name
+          type: string
+          readOnly: true
+          minLength: 1
+          maxLength: 255
+        commit_ref:
+          title: Commit ref
+          type: string
+          maxLength: 255
+          nullable: true
+        pull_url:
+          title: Pull URL
+          type: string
+          format: uri
+          maxLength: 255
+          nullable: true
+        state:
+          title: State
+          type: string
+        archived:
+          title: Archived
+          type: boolean
+        hash:
+          title: Hash
+          type: string
+          readOnly: true
+          minLength: 1
+        submitter:
+          type: object
+          title: Submitter
+          readOnly: true
+          allOf:
+            - $ref: '#/components/schemas/PersonEmbedded'
+        delegate:
+          type: object
+          title: Delegate
+          nullable: true
+          readOnly: true
+          allOf:
+            - $ref: '#/components/schemas/UserEmbedded'
+        mbox:
+          title: Mbox
+          type: string
+          format: uri
+          readOnly: true
+        series:
+          type: array
+          items:
+            $ref: '#/components/schemas/SeriesEmbedded'
+          readOnly: true
+        comments:
+          title: Comments
+          type: string
+          format: uri
+          readOnly: true
+        check:
+          title: Check
+          type: string
+          readOnly: true
+          enum:
+            - pending
+            - success
+            - warning
+            - fail
+        checks:
+          title: Checks
+          type: string
+          format: uri
+          readOnly: true
+        tags:
+          title: Tags
+          type: object
+          additionalProperties:
+            type: string
+          readOnly: true
+        related:
+          title: Relations
+          type: array
+          items:
+            $ref: '#/components/schemas/PatchEmbedded'
+    PatchDetail:
+      allOf:
+        - $ref: '#/components/schemas/PatchList'
+        - properties:
+            headers:
+              title: Headers
+              anyOf:
+                - type: object
+                  additionalProperties:
+                    type: array
+                    items:
+                      type: string
+                - type: object
+                  additionalProperties:
+                    type: string
+              readOnly: true
+            content:
+              title: Content
+              type: string
+              readOnly: true
+              minLength: 1
+            diff:
+              title: Diff
+              type: string
+              readOnly: true
+              minLength: 1
+            prefixes:
+              title: Prefixes
+              type: array
+              items:
+                type: string
+              readOnly: true
+    PatchUpdate:
+      type: object
+      properties:
+        commit_ref:
+          title: Commit ref
+          type: string
+          maxLength: 255
+          nullable: true
+        pull_url:
+          title: Pull URL
+          type: string
+          format: uri
+          maxLength: 255
+          nullable: true
+        state:
+          title: State
+          type: string
+        archived:
+          title: Archived
+          type: boolean
+        delegate:
+          title: Delegate
+          type: integer
+          nullable: true
+        related:
+          title: Relations
+          type: array
+          items:
+            type: integer
+    Person:
+      type: object
+      properties:
+        id:
+          title: ID
+          type: integer
+          readOnly: true
+        url:
+          title: URL
+          type: string
+          format: uri
+          readOnly: true
+        name:
+          title: Name
+          type: string
+          readOnly: true
+          minLength: 1
+          maxLength: 255
+        email:
+          title: Email
+          type: string
+          format: email
+          readOnly: true
+          minLength: 1
+          maxLength: 255
+        user:
+          type: object
+          title: User
+          nullable: true
+          readOnly: true
+          allOf:
+            - $ref: '#/components/schemas/UserEmbedded'
+    Project:
+      type: object
+      properties:
+        id:
+          title: ID
+          type: integer
+          readOnly: true
+        url:
+          title: URL
+          type: string
+          format: uri
+          readOnly: true
+        name:
+          title: Name
+          type: string
+          readOnly: true
+          minLength: 1
+          maxLength: 255
+        link_name:
+          title: Link name
+          type: string
+          readOnly: true
+          minLength: 1
+          maxLength: 255
+        list_id:
+          title: List ID
+          type: string
+          readOnly: true
+          minLength: 1
+          maxLength: 255
+        list_email:
+          title: List email
+          type: string
+          format: email
+          readOnly: true
+          minLength: 1
+          maxLength: 200
+        web_url:
+          title: Web URL
+          type: string
+          format: uri
+          maxLength: 2000
+        scm_url:
+          title: SCM URL
+          type: string
+          format: uri
+          maxLength: 2000
+        webscm_url:
+          title: Web SCM URL
+          type: string
+          format: uri
+          maxLength: 2000
+        maintainers:
+          type: array
+          items:
+            $ref: '#/components/schemas/UserEmbedded'
+          readOnly: true
+          uniqueItems: true
+        subject_match:
+          title: Subject match
+          description: >
+            Regex to match the subject against if only part of emails sent to
+            the list belongs to this project. Will be used with IGNORECASE and
+            MULTILINE flags. If rules for more projects match the first one
+            returned from DB is chosen; empty field serves as a default for
+            every email which has no other match.
+          type: string
+          readOnly: true
+          maxLength: 64
+        list_archive_url:
+          title: List archive URL
+          type: string
+          format: uri
+          maxLength: 2000
+          nullable: true
+        list_archive_url_format:
+          title: List archive URL format
+          type: string
+          format: uri
+          maxLength: 2000
+          nullable: true
+          description: >
+            URL format for the list archive's Message-ID redirector. {} will be
+            replaced by the Message-ID.
+        commit_url_format:
+          title: Web SCM URL format for a particular commit
+          type: string
+    Series:
+      type: object
+      properties:
+        id:
+          title: ID
+          type: integer
+          readOnly: true
+        url:
+          title: URL
+          type: string
+          format: uri
+          readOnly: true
+        web_url:
+          title: Web URL
+          type: string
+          format: uri
+          readOnly: true
+        project:
+          $ref: '#/components/schemas/ProjectEmbedded'
+        name:
+          title: Name
+          description: >
+            An optional name to associate with the series, e.g. "John's PCI
+            series".
+          type: string
+          maxLength: 255
+          nullable: true
+        date:
+          title: Date
+          type: string
+          format: iso8601
+          readOnly: true
+        submitter:
+          type: object
+          title: Submitter
+          readOnly: true
+          allOf:
+            - $ref: '#/components/schemas/PersonEmbedded'
+        version:
+          title: Version
+          description: >
+            Version of series as indicated by the subject prefix(es).
+          type: integer
+        total:
+          title: Total
+          description: >
+            Number of patches in series as indicated by the subject prefix(es).
+          type: integer
+          readOnly: true
+        received_total:
+          title: Received total
+          type: integer
+          readOnly: true
+        received_all:
+          title: Received all
+          type: boolean
+          readOnly: true
+        mbox:
+          title: Mbox
+          type: string
+          format: uri
+          readOnly: true
+        cover_letter:
+          $ref: '#/components/schemas/CoverEmbedded'
+        patches:
+          title: Patches
+          type: array
+          items:
+            $ref: '#/components/schemas/PatchEmbedded'
+          readOnly: true
+          uniqueItems: true
+    User:
+      type: object
+      properties:
+        id:
+          title: ID
+          type: integer
+          readOnly: true
+        url:
+          title: URL
+          type: string
+          format: uri
+          readOnly: true
+        username:
+          title: Username
+          type: string
+          readOnly: true
+          minLength: 1
+          maxLength: 150
+        first_name:
+          title: First name
+          type: string
+          maxLength: 30
+        last_name:
+          title: Last name
+          type: string
+          maxLength: 150
+        email:
+          title: Email address
+          type: string
+          format: email
+          readOnly: true
+          minLength: 1
+    UserDetail:
+      type: object
+      allOf:
+        - $ref: '#/components/schemas/User'
+        - type: object
+          properties:
+            settings:
+              type: object
+              properties:
+                send_email:
+                  title: Send email
+                  description: >
+                    Whether Patchwork should send email on your behalf.
+                    Only present and configurable for your account.
+                  type: boolean
+                items_per_page:
+                  title: Items per page
+                  description: >
+                    Number of items to display per page (web UI).
+                    Only present and configurable for your account.
+                  type: integer
+                show_ids:
+                  title: Show IDs
+                  description:
+                    Show click-to-copy IDs in the list view (web UI).
+                    Only present and configurable for your account.
+                  type: boolean
+    CheckEmbedded:
+      type: object
+      properties:
+        id:
+          title: ID
+          type: integer
+          readOnly: true
+        url:
+          title: Url
+          type: string
+          format: uri
+          readOnly: true
+        date:
+          title: Date
+          type: string
+          format: iso8601
+          readOnly: true
+        state:
+          title: State
+          description: The state of the check.
+          type: string
+          readOnly: true
+          enum:
+            - pending
+            - success
+            - warning
+            - fail
+        target_url:
+          title: Target url
+          description: >
+            The target URL to associate with this check. This should be specific
+            to the patch.
+          type: string
+          format: uri
+          maxLength: 200
+          nullable: true
+          readOnly: true
+        context:
+          title: Context
+          description: >
+            A label to discern check from checks of other testing systems.
+          type: string
+          pattern: ^[-a-zA-Z0-9_]+$
+          maxLength: 255
+          minLength: 1
+          readOnly: true
+    CoverEmbedded:
+      type: object
+      properties:
+        id:
+          title: ID
+          type: integer
+          readOnly: true
+        url:
+          title: URL
+          type: string
+          format: uri
+          readOnly: true
+        web_url:
+          title: Web URL
+          type: string
+          format: uri
+          readOnly: true
+        msgid:
+          title: Message ID
+          type: string
+          readOnly: true
+          minLength: 1
+        list_archive_url:
+          title: List archive URL
+          type: string
+          readOnly: true
+          nullable: true
+        date:
+          title: Date
+          type: string
+          format: iso8601
+          readOnly: true
+        name:
+          title: Name
+          type: string
+          readOnly: true
+          minLength: 1
+        mbox:
+          title: Mbox
+          type: string
+          format: uri
+          readOnly: true
+    PatchEmbedded:
+      type: object
+      properties:
+        id:
+          title: ID
+          type: integer
+          readOnly: true
+        url:
+          title: URL
+          type: string
+          format: uri
+          readOnly: true
+        web_url:
+          title: Web URL
+          type: string
+          format: uri
+          readOnly: true
+        msgid:
+          title: Message ID
+          type: string
+          readOnly: true
+          minLength: 1
+        list_archive_url:
+          title: List archive URL
+          type: string
+          readOnly: true
+          nullable: true
+        date:
+          title: Date
+          type: string
+          format: iso8601
+          readOnly: true
+        name:
+          title: Name
+          type: string
+          readOnly: true
+          minLength: 1
+        mbox:
+          title: Mbox
+          type: string
+          format: uri
+          readOnly: true
+    PersonEmbedded:
+      type: object
+      properties:
+        id:
+          title: ID
+          type: integer
+          readOnly: true
+        url:
+          title: URL
+          type: string
+          format: uri
+          readOnly: true
+        name:
+          title: Name
+          type: string
+          readOnly: true
+          minLength: 1
+        email:
+          title: Email
+          type: string
+          format: email
+          readOnly: true
+          minLength: 1
+    ProjectEmbedded:
+      type: object
+      properties:
+        id:
+          title: ID
+          type: integer
+          readOnly: true
+        url:
+          title: URL
+          type: string
+          format: uri
+          readOnly: true
+        name:
+          title: Name
+          type: string
+          readOnly: true
+          minLength: 1
+        link_name:
+          title: Link name
+          type: string
+          readOnly: true
+          maxLength: 255
+          minLength: 1
+        list_id:
+          title: List ID
+          type: string
+          readOnly: true
+          maxLength: 255
+          minLength: 1
+        list_email:
+          title: List email
+          type: string
+          format: email
+          readOnly: true
+          maxLength: 200
+          minLength: 1
+        web_url:
+          title: Web URL
+          type: string
+          format: uri
+          readOnly: true
+          maxLength: 2000
+        scm_url:
+          title: SCM URL
+          type: string
+          format: uri
+          readOnly: true
+          maxLength: 2000
+        webscm_url:
+          title: WebSCM URL
+          type: string
+          format: uri
+          readOnly: true
+          maxLength: 2000
+        list_archive_url:
+          title: List archive URL
+          type: string
+          format: uri
+          maxLength: 2000
+          nullable: true
+        list_archive_url_format:
+          title: List archive URL format
+          type: string
+          format: uri
+          maxLength: 2000
+          nullable: true
+          description: >
+            URL format for the list archive's Message-ID redirector. {} will be
+            replaced by the Message-ID.
+        commit_url_format:
+          title: Web SCM URL format for a particular commit
+          type: string
+          readOnly: true
+    SeriesEmbedded:
+      type: object
+      properties:
+        id:
+          title: ID
+          type: integer
+          readOnly: true
+        url:
+          title: URL
+          type: string
+          format: uri
+          readOnly: true
+        web_url:
+          title: Web URL
+          type: string
+          format: uri
+          readOnly: true
+        name:
+          title: Name
+          description: >
+            An optional name to associate with the series, e.g. "John's PCI
+            series".
+          type: string
+          readOnly: true
+          maxLength: 255
+          nullable: true
+        date:
+          title: Date
+          type: string
+          format: iso8601
+          readOnly: true
+        version:
+          title: Version
+          description: >
+            Version of series as indicated by the subject prefix(es).
+          type: integer
+          readOnly: true
+        mbox:
+          title: Mbox
+          type: string
+          format: uri
+          readOnly: true
+    UserEmbedded:
+      type: object
+      nullable: true
+      properties:
+        id:
+          title: ID
+          type: integer
+          readOnly: true
+        url:
+          title: URL
+          type: string
+          format: uri
+          readOnly: true
+        username:
+          title: Username
+          type: string
+          readOnly: true
+          minLength: 1
+          maxLength: 150
+        first_name:
+          title: First name
+          type: string
+          maxLength: 30
+          readOnly: true
+        last_name:
+          title: Last name
+          type: string
+          maxLength: 150
+          readOnly: true
+        email:
+          title: Email address
+          type: string
+          format: email
+          readOnly: true
+          minLength: 1
+    Error:
+      type: object
+      properties:
+        detail:
+          title: Detail
+          type: string
+          readOnly: true
+    ErrorBundleCreateUpdate:
+      type: object
+      properties:
+        name:
+          title: Name
+          type: array
+          items:
+            type: string
+          readOnly: true
+        patches:
+          title: Patches
+          type: array
+          items:
+            type: string
+          readOnly: true
+        public:
+          title: Public
+          type: array
+          items:
+            type: string
+    ErrorCheckCreate:
+      type: object
+      properties:
+        state:
+          title: State
+          type: array
+          items:
+            type: string
+          readOnly: true
+        target_url:
+          title: Target URL
+          type: array
+          items:
+            type: string
+          readOnly: true
+        context:
+          title: Context
+          type: array
+          items:
+            type: string
+          readOnly: true
+        description:
+          title: Description
+          type: array
+          items:
+            type: string
+          readOnly: true
+    ErrorCommentUpdate:
+      type: object
+      properties:
+        addressed:
+          title: Addressed
+          type: array
+          items:
+            type: string
+          readOnly: true
+    ErrorPatchUpdate:
+      type: object
+      properties:
+        state:
+          title: State
+          type: array
+          items:
+            type: string
+          readOnly: true
+        delegate:
+          title: Delegate
+          type: array
+          items:
+            type: string
+          readOnly: true
+        commit_ref:
+          title: Commit ref
+          type: array
+          items:
+            type: string
+          readOnly: true
+        archived:
+          title: Archived
+          type: array
+          items:
+            type: string
+          readOnly: true
+    ErrorProjectUpdate:
+      type: object
+      properties:
+        web_url:
+          title: Web URL
+          type: string
+          format: uri
+          readOnly: true
+        scm_url:
+          title: SCM URL
+          type: string
+          format: uri
+          readOnly: true
+        webscm_url:
+          title: Web SCM URL
+          type: string
+          format: uri
+          readOnly: true
+    ErrorUserUpdate:
+      type: object
+      properties:
+        first_name:
+          title: First name
+          type: string
+          readOnly: true
+        last_name:
+          title: First name
+          type: string
+          readOnly: true
diff --git a/patchwork/api/base.py b/patchwork/api/base.py
index 89a4311..856fbd3 100644
--- a/patchwork/api/base.py
+++ b/patchwork/api/base.py
@@ -3,6 +3,7 @@ 
 #
 # SPDX-License-Identifier: GPL-2.0-or-later
 
+import rest_framework
 
 from django.conf import settings
 from django.shortcuts import get_object_or_404
@@ -15,6 +16,24 @@  from rest_framework.serializers import HyperlinkedModelSerializer
 from patchwork.api import utils
 
 
+DRF_VERSION = tuple(int(x) for x in rest_framework.__version__.split('.'))
+
+
+if DRF_VERSION > (3, 11):
+    class CurrentPatchDefault(object):
+        requires_context = True
+
+        def __call__(self, serializer_field):
+            return serializer_field.context['request'].patch
+else:
+    class CurrentPatchDefault(object):
+        def set_context(self, serializer_field):
+            self.patch = serializer_field.context['request'].patch
+
+        def __call__(self):
+            return self.patch
+
+
 class LinkHeaderPagination(PageNumberPagination):
     """Provide pagination based on rfc5988.
 
@@ -44,7 +63,10 @@  class LinkHeaderPagination(PageNumberPagination):
 
 
 class PatchworkPermission(permissions.BasePermission):
-    """This permission works for Project and Patch model objects"""
+    """
+    This permission works for Project, Patch, and PatchComment
+    model objects
+    """
     def has_object_permission(self, request, view, obj):
         # read only for everyone
         if request.method in permissions.SAFE_METHODS:
diff --git a/patchwork/api/check.py b/patchwork/api/check.py
index a6bf5f8..fde6b61 100644
--- a/patchwork/api/check.py
+++ b/patchwork/api/check.py
@@ -6,7 +6,7 @@ 
 from django.http import Http404
 from django.http.request import QueryDict
 from django.shortcuts import get_object_or_404
-import rest_framework
+
 from rest_framework.exceptions import PermissionDenied
 from rest_framework.generics import ListCreateAPIView
 from rest_framework.generics import RetrieveAPIView
@@ -17,30 +17,13 @@  from rest_framework.serializers import ValidationError
 
 from patchwork.api.base import CheckHyperlinkedIdentityField
 from patchwork.api.base import MultipleFieldLookupMixin
+from patchwork.api.base import CurrentPatchDefault
 from patchwork.api.embedded import UserSerializer
 from patchwork.api.filters import CheckFilterSet
 from patchwork.models import Check
 from patchwork.models import Patch
 
 
-DRF_VERSION = tuple(int(x) for x in rest_framework.__version__.split('.'))
-
-
-if DRF_VERSION > (3, 11):
-    class CurrentPatchDefault(object):
-        requires_context = True
-
-        def __call__(self, serializer_field):
-            return serializer_field.context['request'].patch
-else:
-    class CurrentPatchDefault(object):
-        def set_context(self, serializer_field):
-            self.patch = serializer_field.context['request'].patch
-
-        def __call__(self):
-            return self.patch
-
-
 class CheckSerializer(HyperlinkedModelSerializer):
 
     url = CheckHyperlinkedIdentityField('api-check-detail')
diff --git a/patchwork/api/comment.py b/patchwork/api/comment.py
index 43b26c6..50d70d1 100644
--- a/patchwork/api/comment.py
+++ b/patchwork/api/comment.py
@@ -5,12 +5,17 @@ 
 
 import email.parser
 
+from django.shortcuts import get_object_or_404
 from django.http import Http404
 from rest_framework.generics import ListAPIView
+from rest_framework.generics import RetrieveUpdateAPIView
+from rest_framework.serializers import HiddenField
 from rest_framework.serializers import SerializerMethodField
 
 from patchwork.api.base import BaseHyperlinkedModelSerializer
+from patchwork.api.base import MultipleFieldLookupMixin
 from patchwork.api.base import PatchworkPermission
+from patchwork.api.base import CurrentPatchDefault
 from patchwork.api.embedded import PersonSerializer
 from patchwork.models import Cover
 from patchwork.models import CoverComment
@@ -66,15 +71,50 @@  class CoverCommentListSerializer(BaseCommentListSerializer):
         versioned_fields = BaseCommentListSerializer.Meta.versioned_fields
 
 
-class PatchCommentListSerializer(BaseCommentListSerializer):
+class PatchCommentSerializer(BaseCommentListSerializer):
+
+    patch = HiddenField(default=CurrentPatchDefault())
 
     class Meta:
         model = PatchComment
-        fields = BaseCommentListSerializer.Meta.fields
-        read_only_fields = fields
+        fields = BaseCommentListSerializer.Meta.fields + (
+            'patch', 'addressed')
+        read_only_fields = fields[:-1]  # able to write to addressed field
+        versioned_fields = {
+            '1.3': ('patch', 'addressed'),
+        }
+        extra_kwargs = {
+            'url': {'view_name': 'api-patch-comment-detail'}
+        }
         versioned_fields = BaseCommentListSerializer.Meta.versioned_fields
 
 
+class PatchCommentMixin(object):
+
+    permission_classes = (PatchworkPermission,)
+    serializer_class = PatchCommentSerializer
+
+    def get_object(self):
+        queryset = self.filter_queryset(self.get_queryset())
+        comment_id = self.kwargs['comment_id']
+        try:
+            obj = queryset.get(id=int(comment_id))
+        except (ValueError, PatchComment.DoesNotExist):
+            obj = get_object_or_404(queryset, linkname=comment_id)
+        self.kwargs['comment_id'] = obj.id
+        self.check_object_permissions(self.request, obj)
+        return obj
+
+    def get_queryset(self):
+        patch_id = self.kwargs['patch_id']
+        if not Patch.objects.filter(id=patch_id).exists():
+            raise Http404
+
+        return PatchComment.objects.filter(
+            patch=patch_id
+        ).select_related('submitter')
+
+
 class CoverCommentList(ListAPIView):
     """List cover comments"""
 
@@ -94,20 +134,24 @@  class CoverCommentList(ListAPIView):
         ).select_related('submitter')
 
 
-class PatchCommentList(ListAPIView):
-    """List comments"""
+class PatchCommentList(PatchCommentMixin, ListAPIView):
+    """List patch comments"""
 
-    permission_classes = (PatchworkPermission,)
-    serializer_class = PatchCommentListSerializer
     search_fields = ('subject',)
     ordering_fields = ('id', 'subject', 'date', 'submitter')
     ordering = 'id'
-    lookup_url_kwarg = 'pk'
-
-    def get_queryset(self):
-        if not Patch.objects.filter(pk=self.kwargs['pk']).exists():
-            raise Http404
-
-        return PatchComment.objects.filter(
-            patch=self.kwargs['pk']
-        ).select_related('submitter')
+    lookup_url_kwarg = 'patch_id'
+
+
+class PatchCommentDetail(PatchCommentMixin, MultipleFieldLookupMixin,
+                         RetrieveUpdateAPIView):
+    """
+    get:
+    Show a patch comment.
+    patch:
+    Update a patch comment.
+    put:
+    Update a patch comment.
+    """
+    lookup_url_kwargs = ('patch_id', 'comment_id')
+    lookup_fields = ('patch_id', 'id')
diff --git a/patchwork/api/patch.py b/patchwork/api/patch.py
index 9d22275..a97a882 100644
--- a/patchwork/api/patch.py
+++ b/patchwork/api/patch.py
@@ -97,7 +97,7 @@  class PatchListSerializer(BaseHyperlinkedModelSerializer):
 
     def get_comments(self, patch):
         return self.context.get('request').build_absolute_uri(
-            reverse('api-patch-comment-list', kwargs={'pk': patch.id}))
+            reverse('api-patch-comment-list', kwargs={'patch_id': patch.id}))
 
     def get_check(self, instance):
         return instance.combined_check_state
diff --git a/patchwork/migrations/0045_patchcomment_addressed.py b/patchwork/migrations/0045_patchcomment_addressed.py
new file mode 100644
index 0000000..92e6c4e
--- /dev/null
+++ b/patchwork/migrations/0045_patchcomment_addressed.py
@@ -0,0 +1,18 @@ 
+# Generated by Django 3.1.12 on 2021-07-16 04:12
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('patchwork', '0044_add_project_linkname_validation'),
+    ]
+
+    operations = [
+        migrations.AddField(
+            model_name='patchcomment',
+            name='addressed',
+            field=models.BooleanField(default=False),
+        ),
+    ]
diff --git a/patchwork/models.py b/patchwork/models.py
index 00273da..ef52f2c 100644
--- a/patchwork/models.py
+++ b/patchwork/models.py
@@ -693,6 +693,7 @@  class PatchComment(EmailMixin, models.Model):
         related_query_name='comment',
         on_delete=models.CASCADE,
     )
+    addressed = models.BooleanField(default=False)
 
     @property
     def list_archive_url(self):
@@ -718,7 +719,9 @@  class PatchComment(EmailMixin, models.Model):
         self.patch.refresh_tag_counts()
 
     def is_editable(self, user):
-        return False
+        if user == self.submitter.user:
+            return True
+        return self.patch.is_editable(user)
 
     class Meta:
         ordering = ['date']
diff --git a/patchwork/tests/api/test_comment.py b/patchwork/tests/api/test_comment.py
index 5bbebf2..59450d8 100644
--- a/patchwork/tests/api/test_comment.py
+++ b/patchwork/tests/api/test_comment.py
@@ -90,7 +90,7 @@  class TestPatchComments(utils.APITestCase):
         kwargs = {}
         if version:
             kwargs['version'] = version
-        kwargs['pk'] = patch.id
+        kwargs['patch_id'] = patch.id
 
         return reverse('api-patch-comment-list', kwargs=kwargs)
 
@@ -142,5 +142,5 @@  class TestPatchComments(utils.APITestCase):
     def test_list_invalid_patch(self):
         """Ensure we get a 404 for a non-existent patch."""
         resp = self.client.get(
-            reverse('api-patch-comment-list', kwargs={'pk': '99999'}))
+            reverse('api-patch-comment-list', kwargs={'patch_id': '99999'}))
         self.assertEqual(status.HTTP_404_NOT_FOUND, resp.status_code)
diff --git a/patchwork/urls.py b/patchwork/urls.py
index 6ac9b81..0c48727 100644
--- a/patchwork/urls.py
+++ b/patchwork/urls.py
@@ -332,7 +332,7 @@  if settings.ENABLE_REST_API:
 
     api_1_1_patterns = [
         path(
-            'patches/<pk>/comments/',
+            'patches/<patch_id>/comments/',
             api_comment_views.PatchCommentList.as_view(),
             name='api-patch-comment-list',
         ),
@@ -343,12 +343,23 @@  if settings.ENABLE_REST_API:
         ),
     ]
 
+    api_1_3_patterns = [
+        path(
+            'patches/<patch_id>/comments/<comment_id>/',
+            api_comment_views.PatchCommentDetail.as_view(),
+            name='api-patch-comment-detail',
+        ),
+    ]
+
     urlpatterns += [
         re_path(
-            r'^api/(?:(?P<version>(1.0|1.1|1.2))/)?', include(api_patterns)
+            r'^api/(?:(?P<version>(1.0|1.1|1.2|1.3))/)?', include(api_patterns)
+        ),
+        re_path(
+            r'^api/(?:(?P<version>(1.1|1.2|1.3))/)?', include(api_1_1_patterns)
         ),
         re_path(
-            r'^api/(?:(?P<version>(1.1|1.2))/)?', include(api_1_1_patterns)
+            r'^api/(?:(?P<version>(1.3))/)?', include(api_1_3_patterns)
         ),
         # token change
         path(