[v3] api: Add comments to patch and cover endpoints

Message ID 20180417105032.1258-1-vkabatov@redhat.com
State Changes Requested
Headers show
Series
  • [v3] api: Add comments to patch and cover endpoints
Related show

Commit Message

Veronika Kabatova April 17, 2018, 10:50 a.m.
From: Veronika Kabatova <vkabatov@redhat.com>

Signed-off-by: Veronika Kabatova <vkabatov@redhat.com>
---
New changes: move things under /patches|covers/ID/comments
---
 patchwork/api/comment.py                           |  80 ++++++++++++++
 patchwork/api/cover.py                             |  13 ++-
 patchwork/api/patch.py                             |  16 ++-
 patchwork/models.py                                |   3 +
 patchwork/tests/api/test_comment.py                | 115 +++++++++++++++++++++
 patchwork/tests/api/test_cover.py                  |  11 +-
 patchwork/tests/api/test_patch.py                  |  19 +++-
 patchwork/urls.py                                  |  11 ++
 .../notes/comments-api-b7dff6ee4ce04c9b.yaml       |   8 ++
 9 files changed, 267 insertions(+), 9 deletions(-)
 create mode 100644 patchwork/api/comment.py
 create mode 100644 patchwork/tests/api/test_comment.py
 create mode 100644 releasenotes/notes/comments-api-b7dff6ee4ce04c9b.yaml

Comments

Stephen Finucane April 25, 2018, 8:53 a.m. | #1
On Tue, 2018-04-17 at 12:50 +0200, vkabatov@redhat.com wrote:
> From: Veronika Kabatova <vkabatov@redhat.com>
> 
> Signed-off-by: Veronika Kabatova <vkabatov@redhat.com>
> ---
> New changes: move things under /patches|covers/ID/comments

Almost there - just some queries to clean up.

> ---
>  patchwork/api/comment.py                           |  80 ++++++++++++++
>  patchwork/api/cover.py                             |  13 ++-
>  patchwork/api/patch.py                             |  16 ++-
>  patchwork/models.py                                |   3 +
>  patchwork/tests/api/test_comment.py                | 115 +++++++++++++++++++++
>  patchwork/tests/api/test_cover.py                  |  11 +-
>  patchwork/tests/api/test_patch.py                  |  19 +++-
>  patchwork/urls.py                                  |  11 ++
>  .../notes/comments-api-b7dff6ee4ce04c9b.yaml       |   8 ++
>  9 files changed, 267 insertions(+), 9 deletions(-)
>  create mode 100644 patchwork/api/comment.py
>  create mode 100644 patchwork/tests/api/test_comment.py
>  create mode 100644 releasenotes/notes/comments-api-b7dff6ee4ce04c9b.yaml
> 
> diff --git a/patchwork/api/comment.py b/patchwork/api/comment.py
> new file mode 100644
> index 0000000..a650168
> --- /dev/null
> +++ b/patchwork/api/comment.py
> @@ -0,0 +1,80 @@
> +# Patchwork - automated patch tracking system
> +# Copyright (C) 2018 Red Hat
> +#
> +# This file is part of the Patchwork package.
> +#
> +# Patchwork is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 2 of the License, or
> +# (at your option) any later version.
> +#
> +# Patchwork is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with Patchwork; if not, write to the Free Software
> +# Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
> +
> +import email.parser
> +
> +from rest_framework.generics import ListAPIView
> +from rest_framework.serializers import SerializerMethodField
> +
> +from patchwork.api.base import BaseHyperlinkedModelSerializer
> +from patchwork.api.base import PatchworkPermission
> +from patchwork.api.embedded import PersonSerializer
> +from patchwork.api.embedded import ProjectSerializer
> +from patchwork.models import Comment
> +
> +
> +class CommentListSerializer(BaseHyperlinkedModelSerializer):
> +
> +    project = ProjectSerializer(source='submission.project', read_only=True)

This results in N * 2 addition queries for N comments because of the
extra reads to 'submission' and 'project'. You can resolve this
somewhat, as noted below, but I wonder if this is even necessary?
Couldn't we infer it from the parent patch?

> +    tags = SerializerMethodField()
> +    subject = SerializerMethodField()
> +    headers = SerializerMethodField()
> +    submitter = PersonSerializer(read_only=True)
> +
> +    def get_tags(self, comment):
> +        # TODO implement after we get support for tags on comments
> +        return {}
> +
> +    def get_subject(self, comment):
> +        return email.parser.Parser().parsestr(comment.headers,
> +                                              True).get('Subject', '')
> +
> +    def get_headers(self, comment):
> +        headers = {}
> +
> +        if comment.headers:
> +            parsed = email.parser.Parser().parsestr(comment.headers, True)
> +            for key in parsed.keys():
> +                headers[key] = parsed.get_all(key)
> +                # Let's return a single string instead of a list if only one
> +                # header with this key is present
> +                if len(headers[key]) == 1:
> +                    headers[key] = headers[key][0]
> +
> +        return headers
> +
> +    class Meta:
> +        model = Comment
> +        fields = ('id', 'msgid', 'date', 'subject', 'project', 'submitter',
> +                  'tags', 'content', 'headers')

Personally I'd drop the 'tags' field until such a time as we _do_ have
support for this. The only reason they currently exist for '/patches'
is because API v1.0 was out in the wild before I realized there were
performance issues.

> +        read_only_fields = fields
> +
> +
> +class CommentList(ListAPIView):
> +    """List comments"""
> +
> +    permission_classes = (PatchworkPermission,)
> +    serializer_class = CommentListSerializer
> +    search_fields = ('subject',)
> +    ordering_fields = ('id', 'subject', 'date', 'submitter')
> +    ordering = 'id'
> +    lookup_url_kwarg = 'pk'
> +
> +    def get_queryset(self):
> +        return Comment.objects.filter(submission=self.kwargs['pk'])

To resolve the issue above, you'll need to modify this like so:

    return Comment.objects\
        .filter(submission=self.kwargs['pk'])\
        .select_related('submitter', 'submission__project')

However, as noted above, I'm not sure if we even need/want to include
'project'. Assuming we don't, you just need the 'submitter' bit.

> diff --git a/patchwork/api/cover.py b/patchwork/api/cover.py
> index fc7ae97..19f0f65 100644
> --- a/patchwork/api/cover.py
> +++ b/patchwork/api/cover.py
> @@ -21,6 +21,7 @@ import email.parser
>  
>  from rest_framework.generics import ListAPIView
>  from rest_framework.generics import RetrieveAPIView
> +from rest_framework.reverse import reverse
>  from rest_framework.serializers import SerializerMethodField
>  
>  from patchwork.api.base import BaseHyperlinkedModelSerializer
> @@ -58,6 +59,11 @@ class CoverLetterListSerializer(BaseHyperlinkedModelSerializer):
>  class CoverLetterDetailSerializer(CoverLetterListSerializer):
>  
>      headers = SerializerMethodField()
> +    comments = SerializerMethodField()
> +
> +    def get_comments(self, cover):
> +        return self.context.get('request').build_absolute_uri(
> +            reverse('api-comment-list', kwargs={'pk': cover.id}))
>  
>      def get_headers(self, instance):
>          if instance.headers:
> @@ -65,9 +71,14 @@ class CoverLetterDetailSerializer(CoverLetterListSerializer):
>  
>      class Meta:
>          model = CoverLetter
> -        fields = CoverLetterListSerializer.Meta.fields + ('headers', 'content')
> +        fields = CoverLetterListSerializer.Meta.fields + ('headers',
> +                                                          'content',
> +                                                          'comments')
>          read_only_fields = fields
>          extra_kwargs = CoverLetterListSerializer.Meta.extra_kwargs
> +        versioned_fields = {
> +            '1.1': ('mbox', 'comments'),
> +        }

Good spot (the 'mbox' field).

>  
>  
>  class CoverLetterList(ListAPIView):
> diff --git a/patchwork/api/patch.py b/patchwork/api/patch.py
> index 115feff..feb7d80 100644
> --- a/patchwork/api/patch.py
> +++ b/patchwork/api/patch.py
> @@ -24,9 +24,9 @@ from rest_framework.generics import ListAPIView
>  from rest_framework.generics import RetrieveUpdateAPIView
>  from rest_framework.relations import RelatedField
>  from rest_framework.reverse import reverse
> -from rest_framework.serializers import HyperlinkedModelSerializer
>  from rest_framework.serializers import SerializerMethodField
>  
> +from patchwork.api.base import BaseHyperlinkedModelSerializer
>  from patchwork.api.base import PatchworkPermission
>  from patchwork.api.filters import PatchFilter
>  from patchwork.api.embedded import PersonSerializer
> @@ -75,7 +75,7 @@ class StateField(RelatedField):
>          return State.objects.all()
>  
>  
> -class PatchListSerializer(HyperlinkedModelSerializer):
> +class PatchListSerializer(BaseHyperlinkedModelSerializer):
>  
>      project = ProjectSerializer(read_only=True)
>      state = StateField()
> @@ -121,6 +121,11 @@ class PatchDetailSerializer(PatchListSerializer):
>  
>      headers = SerializerMethodField()
>      prefixes = SerializerMethodField()
> +    comments = SerializerMethodField()
> +
> +    def get_comments(self, patch):
> +        return self.context.get('request').build_absolute_uri(
> +            reverse('api-comment-list', kwargs={'pk': patch.id}))
>  
>      def get_headers(self, patch):
>          if patch.headers:
> @@ -132,10 +137,13 @@ class PatchDetailSerializer(PatchListSerializer):
>      class Meta:
>          model = Patch
>          fields = PatchListSerializer.Meta.fields + (
> -            'headers', 'content', 'diff', 'prefixes')
> +            'headers', 'content', 'diff', 'prefixes', 'comments')
>          read_only_fields = PatchListSerializer.Meta.read_only_fields + (
> -            'headers', 'content', 'diff', 'prefixes')
> +            'headers', 'content', 'diff', 'prefixes', 'comments')
>          extra_kwargs = PatchListSerializer.Meta.extra_kwargs
> +        versioned_fields = {
> +            '1.1': ('comments', ),
> +        }
>  
>  
>  class PatchList(ListAPIView):
> diff --git a/patchwork/models.py b/patchwork/models.py
> index f91b994..67c2d3a 100644
> --- a/patchwork/models.py
> +++ b/patchwork/models.py
> @@ -613,6 +613,9 @@ class Comment(EmailMixin, models.Model):
>          if hasattr(self.submission, 'patch'):
>              self.submission.patch.refresh_tag_counts()
>  
> +    def is_editable(self, user):
> +        return False
> +
>      class Meta:
>          ordering = ['date']
>          unique_together = [('msgid', 'submission')]
> diff --git a/patchwork/tests/api/test_comment.py b/patchwork/tests/api/test_comment.py
> new file mode 100644
> index 0000000..b283bfe
> --- /dev/null
> +++ b/patchwork/tests/api/test_comment.py
> @@ -0,0 +1,115 @@
> +# Patchwork - automated patch tracking system
> +# Copyright (C) 2018 Red Hat
> +#
> +# This file is part of the Patchwork package.
> +#
> +# Patchwork is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 2 of the License, or
> +# (at your option) any later version.
> +#
> +# Patchwork is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with Patchwork; if not, write to the Free Software
> +# Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
> +
> +import unittest
> +
> +from django.conf import settings
> +
> +from patchwork.compat import NoReverseMatch
> +from patchwork.compat import reverse
> +from patchwork.tests.utils import create_comment
> +from patchwork.tests.utils import create_cover
> +from patchwork.tests.utils import create_patch
> +from patchwork.tests.utils import SAMPLE_CONTENT
> +
> +if settings.ENABLE_REST_API:
> +    from rest_framework import status
> +    from rest_framework.test import APITestCase
> +else:
> +    # stub out APITestCase
> +    from django.test import TestCase
> +    APITestCase = TestCase  # noqa
> +
> +
> +@unittest.skipUnless(settings.ENABLE_REST_API, 'requires ENABLE_REST_API')
> +class TestCoverComments(APITestCase):
> +    @staticmethod
> +    def api_url(cover, version=None):
> +        kwargs = {}
> +        if version:
> +            kwargs['version'] = version
> +        kwargs['pk'] = cover.id
> +
> +        return reverse('api-comment-list', kwargs=kwargs)
> +
> +    def assertSerialized(self, comment_obj, comment_json):
> +        self.assertEqual(comment_obj.id, comment_json['id'])
> +        self.assertEqual(comment_obj.submitter.id,
> +                         comment_json['submitter']['id'])
> +        self.assertIn(SAMPLE_CONTENT, comment_json['content'])
> +
> +    def test_list(self):
> +        cover_obj = create_cover()
> +        resp = self.client.get(self.api_url(cover_obj))
> +        self.assertEqual(status.HTTP_200_OK, resp.status_code)
> +        self.assertEqual(0, len(resp.data))
> +
> +        comment_obj = create_comment(submission=cover_obj)
> +        resp = self.client.get(self.api_url(cover_obj))
> +        self.assertEqual(status.HTTP_200_OK, resp.status_code)
> +        self.assertEqual(1, len(resp.data))
> +        self.assertSerialized(comment_obj, resp.data[0])
> +
> +        another_comment = create_comment(submission=cover_obj)
> +        resp = self.client.get(self.api_url(cover_obj))
> +        self.assertEqual(status.HTTP_200_OK, resp.status_code)
> +        self.assertEqual(2, len(resp.data))
> +
> +        # check we can't access comments using the old version of the API
> +        with self.assertRaises(NoReverseMatch):
> +            self.client.get(self.api_url(cover_obj, version='1.0'))
> +
> +
> +@unittest.skipUnless(settings.ENABLE_REST_API, 'requires ENABLE_REST_API')
> +class TestPatchComments(APITestCase):
> +    @staticmethod
> +    def api_url(patch, version=None):
> +        kwargs = {}
> +        if version:
> +            kwargs['version'] = version
> +        kwargs['pk'] = patch.id
> +
> +        return reverse('api-comment-list', kwargs=kwargs)
> +
> +    def assertSerialized(self, comment_obj, comment_json):
> +        self.assertEqual(comment_obj.id, comment_json['id'])
> +        self.assertEqual(comment_obj.submitter.id,
> +                         comment_json['submitter']['id'])
> +        self.assertIn(SAMPLE_CONTENT, comment_json['content'])
> +
> +    def test_list(self):
> +        patch_obj = create_patch()
> +        resp = self.client.get(self.api_url(patch_obj))
> +        self.assertEqual(status.HTTP_200_OK, resp.status_code)
> +        self.assertEqual(0, len(resp.data))
> +
> +        comment_obj = create_comment(submission=patch_obj)
> +        resp = self.client.get(self.api_url(patch_obj))
> +        self.assertEqual(status.HTTP_200_OK, resp.status_code)
> +        self.assertEqual(1, len(resp.data))
> +        self.assertSerialized(comment_obj, resp.data[0])
> +
> +        another_comment = create_comment(submission=patch_obj)
> +        resp = self.client.get(self.api_url(patch_obj))
> +        self.assertEqual(status.HTTP_200_OK, resp.status_code)
> +        self.assertEqual(2, len(resp.data))
> +
> +        # check we can't access comments using the old version of the API
> +        with self.assertRaises(NoReverseMatch):
> +            self.client.get(self.api_url(patch_obj, version='1.0'))
> diff --git a/patchwork/tests/api/test_cover.py b/patchwork/tests/api/test_cover.py
> index 3135b7e..9a0fbbb 100644
> --- a/patchwork/tests/api/test_cover.py
> +++ b/patchwork/tests/api/test_cover.py
> @@ -49,7 +49,8 @@ class TestCoverLetterAPI(APITestCase):
>  
>          if item is None:
>              return reverse('api-cover-list', kwargs=kwargs)
> -        return reverse('api-cover-detail', args=[item], kwargs=kwargs)
> +        kwargs['pk'] = item
> +        return reverse('api-cover-detail', kwargs=kwargs)
>  
>      def assertSerialized(self, cover_obj, cover_json):
>          self.assertEqual(cover_obj.id, cover_json['id'])
> @@ -115,6 +116,14 @@ class TestCoverLetterAPI(APITestCase):
>          self.assertEqual(status.HTTP_200_OK, resp.status_code)
>          self.assertSerialized(cover_obj, resp.data)
>  
> +        # test comments
> +        resp = self.client.get(self.api_url(cover_obj.id))
> +        self.assertIn('comments', resp.data)
> +
> +        # test old version of API
> +        resp = self.client.get(self.api_url(cover_obj.id, version='1.0'))
> +        self.assertNotIn('comments', resp.data)
> +
>      def test_create_update_delete(self):
>          user = create_maintainer()
>          user.is_superuser = True
> diff --git a/patchwork/tests/api/test_patch.py b/patchwork/tests/api/test_patch.py
> index 909c1eb..d9d44d3 100644
> --- a/patchwork/tests/api/test_patch.py
> +++ b/patchwork/tests/api/test_patch.py
> @@ -45,10 +45,15 @@ class TestPatchAPI(APITestCase):
>      fixtures = ['default_tags']
>  
>      @staticmethod
> -    def api_url(item=None):
> +    def api_url(item=None, version=None):
> +        kwargs = {}
> +        if version:
> +            kwargs['version'] = version
> +
>          if item is None:
> -            return reverse('api-patch-list')
> -        return reverse('api-patch-detail', args=[item])
> +            return reverse('api-patch-list', kwargs=kwargs)
> +        kwargs['pk'] = item
> +        return reverse('api-patch-detail', kwargs=kwargs)
>  
>      def assertSerialized(self, patch_obj, patch_json):
>          self.assertEqual(patch_obj.id, patch_json['id'])
> @@ -130,6 +135,14 @@ class TestPatchAPI(APITestCase):
>          self.assertEqual(patch.diff, resp.data['diff'])
>          self.assertEqual(0, len(resp.data['tags']))
>  
> +        # test comments
> +        resp = self.client.get(self.api_url(patch.id))
> +        self.assertIn('comments', resp.data)
> +
> +        # test old version of API
> +        resp = self.client.get(self.api_url(item=patch.id, version='1.0'))
> +        self.assertNotIn('comments', resp.data)
> +
>      def test_create(self):
>          """Ensure creations are rejected."""
>          project = create_project()
> diff --git a/patchwork/urls.py b/patchwork/urls.py
> index 0893fe2..1dc4ffc 100644
> --- a/patchwork/urls.py
> +++ b/patchwork/urls.py
> @@ -213,6 +213,7 @@ if settings.ENABLE_REST_API:
>  
>      from patchwork.api import bundle as api_bundle_views  # noqa
>      from patchwork.api import check as api_check_views  # noqa
> +    from patchwork.api import comment as api_comment_views  # noqa
>      from patchwork.api import cover as api_cover_views  # noqa
>      from patchwork.api import event as api_event_views  # noqa
>      from patchwork.api import index as api_index_views  # noqa
> @@ -279,8 +280,18 @@ if settings.ENABLE_REST_API:
>              name='api-event-list'),
>      ]
>  
> +    api_1_1_patterns = [
> +        url(r'^patches/(?P<pk>[^/]+)/comments/$',
> +            api_comment_views.CommentList.as_view(),
> +            name='api-comment-list'),
> +        url(r'^covers/(?P<pk>[^/]+)/comments/$',
> +            api_comment_views.CommentList.as_view(),
> +            name='api-comment-list'),
> +    ]
> +

I was wondering if you'd remember this. Nicely done :)

>      urlpatterns += [
>          url(r'^api/(?:(?P<version>(1.0|1.1))/)?', include(api_patterns)),
> +        url(r'^api/(?:(?P<version>1.1)/)?', include(api_1_1_patterns)),
>  
>          # token change
>          url(r'^user/generate-token/$', user_views.generate_token,
> diff --git a/releasenotes/notes/comments-api-b7dff6ee4ce04c9b.yaml b/releasenotes/notes/comments-api-b7dff6ee4ce04c9b.yaml
> new file mode 100644
> index 0000000..5bc4773
> --- /dev/null
> +++ b/releasenotes/notes/comments-api-b7dff6ee4ce04c9b.yaml
> @@ -0,0 +1,8 @@
> +---
> +api:
> +  - |
> +    Links to related comments are now exposed when checking patch and cover
> +    letter details. The comments themselves are then available via
> +    ``/patches/{patchID}/comments`` and ``/covers/{coverID}/comments``
> +    endpoints. Please note that comments are available only since API
> +    version 1.1
Veronika Kabatova April 25, 2018, 10:22 a.m. | #2
----- Original Message -----
> From: "Stephen Finucane" <stephen@that.guru>
> To: vkabatov@redhat.com, patchwork@lists.ozlabs.org
> Sent: Wednesday, April 25, 2018 10:53:14 AM
> Subject: Re: [PATCH v3] api: Add comments to patch and cover endpoints
> 
> On Tue, 2018-04-17 at 12:50 +0200, vkabatov@redhat.com wrote:
> > From: Veronika Kabatova <vkabatov@redhat.com>
> > 
> > Signed-off-by: Veronika Kabatova <vkabatov@redhat.com>
> > ---
> > New changes: move things under /patches|covers/ID/comments
> 
> Almost there - just some queries to clean up.
> 
> > ---
> >  patchwork/api/comment.py                           |  80 ++++++++++++++
> >  patchwork/api/cover.py                             |  13 ++-
> >  patchwork/api/patch.py                             |  16 ++-
> >  patchwork/models.py                                |   3 +
> >  patchwork/tests/api/test_comment.py                | 115
> >  +++++++++++++++++++++
> >  patchwork/tests/api/test_cover.py                  |  11 +-
> >  patchwork/tests/api/test_patch.py                  |  19 +++-
> >  patchwork/urls.py                                  |  11 ++
> >  .../notes/comments-api-b7dff6ee4ce04c9b.yaml       |   8 ++
> >  9 files changed, 267 insertions(+), 9 deletions(-)
> >  create mode 100644 patchwork/api/comment.py
> >  create mode 100644 patchwork/tests/api/test_comment.py
> >  create mode 100644 releasenotes/notes/comments-api-b7dff6ee4ce04c9b.yaml
> > 
> > diff --git a/patchwork/api/comment.py b/patchwork/api/comment.py
> > new file mode 100644
> > index 0000000..a650168
> > --- /dev/null
> > +++ b/patchwork/api/comment.py
> > @@ -0,0 +1,80 @@
> > +# Patchwork - automated patch tracking system
> > +# Copyright (C) 2018 Red Hat
> > +#
> > +# This file is part of the Patchwork package.
> > +#
> > +# Patchwork is free software; you can redistribute it and/or modify
> > +# it under the terms of the GNU General Public License as published by
> > +# the Free Software Foundation; either version 2 of the License, or
> > +# (at your option) any later version.
> > +#
> > +# Patchwork is distributed in the hope that it will be useful,
> > +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> > +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> > +# GNU General Public License for more details.
> > +#
> > +# You should have received a copy of the GNU General Public License
> > +# along with Patchwork; if not, write to the Free Software
> > +# Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307
> > USA
> > +
> > +import email.parser
> > +
> > +from rest_framework.generics import ListAPIView
> > +from rest_framework.serializers import SerializerMethodField
> > +
> > +from patchwork.api.base import BaseHyperlinkedModelSerializer
> > +from patchwork.api.base import PatchworkPermission
> > +from patchwork.api.embedded import PersonSerializer
> > +from patchwork.api.embedded import ProjectSerializer
> > +from patchwork.models import Comment
> > +
> > +
> > +class CommentListSerializer(BaseHyperlinkedModelSerializer):
> > +
> > +    project = ProjectSerializer(source='submission.project',
> > read_only=True)
> 
> This results in N * 2 addition queries for N comments because of the
> extra reads to 'submission' and 'project'. You can resolve this
> somewhat, as noted below, but I wonder if this is even necessary?
> Couldn't we infer it from the parent patch?
> 

Right, the users can just work with the project mentioned in the patch
detail since they need to get there anyways to get to the comments. I'll
remove that.

> > +    tags = SerializerMethodField()
> > +    subject = SerializerMethodField()
> > +    headers = SerializerMethodField()
> > +    submitter = PersonSerializer(read_only=True)
> > +
> > +    def get_tags(self, comment):
> > +        # TODO implement after we get support for tags on comments
> > +        return {}
> > +
> > +    def get_subject(self, comment):
> > +        return email.parser.Parser().parsestr(comment.headers,
> > +                                              True).get('Subject', '')
> > +
> > +    def get_headers(self, comment):
> > +        headers = {}
> > +
> > +        if comment.headers:
> > +            parsed = email.parser.Parser().parsestr(comment.headers, True)
> > +            for key in parsed.keys():
> > +                headers[key] = parsed.get_all(key)
> > +                # Let's return a single string instead of a list if only
> > one
> > +                # header with this key is present
> > +                if len(headers[key]) == 1:
> > +                    headers[key] = headers[key][0]
> > +
> > +        return headers
> > +
> > +    class Meta:
> > +        model = Comment
> > +        fields = ('id', 'msgid', 'date', 'subject', 'project',
> > 'submitter',
> > +                  'tags', 'content', 'headers')
> 
> Personally I'd drop the 'tags' field until such a time as we _do_ have
> support for this. The only reason they currently exist for '/patches'
> is because API v1.0 was out in the wild before I realized there were
> performance issues.
> 

Okay.

> > +        read_only_fields = fields
> > +
> > +
> > +class CommentList(ListAPIView):
> > +    """List comments"""
> > +
> > +    permission_classes = (PatchworkPermission,)
> > +    serializer_class = CommentListSerializer
> > +    search_fields = ('subject',)
> > +    ordering_fields = ('id', 'subject', 'date', 'submitter')
> > +    ordering = 'id'
> > +    lookup_url_kwarg = 'pk'
> > +
> > +    def get_queryset(self):
> > +        return Comment.objects.filter(submission=self.kwargs['pk'])
> 
> To resolve the issue above, you'll need to modify this like so:
> 
>     return Comment.objects\
>         .filter(submission=self.kwargs['pk'])\
>         .select_related('submitter', 'submission__project')
> 
> However, as noted above, I'm not sure if we even need/want to include
> 'project'. Assuming we don't, you just need the 'submitter' bit.
> 
> > diff --git a/patchwork/api/cover.py b/patchwork/api/cover.py
> > index fc7ae97..19f0f65 100644
> > --- a/patchwork/api/cover.py
> > +++ b/patchwork/api/cover.py
> > @@ -21,6 +21,7 @@ import email.parser
> >  
> >  from rest_framework.generics import ListAPIView
> >  from rest_framework.generics import RetrieveAPIView
> > +from rest_framework.reverse import reverse
> >  from rest_framework.serializers import SerializerMethodField
> >  
> >  from patchwork.api.base import BaseHyperlinkedModelSerializer
> > @@ -58,6 +59,11 @@ class
> > CoverLetterListSerializer(BaseHyperlinkedModelSerializer):
> >  class CoverLetterDetailSerializer(CoverLetterListSerializer):
> >  
> >      headers = SerializerMethodField()
> > +    comments = SerializerMethodField()
> > +
> > +    def get_comments(self, cover):
> > +        return self.context.get('request').build_absolute_uri(
> > +            reverse('api-comment-list', kwargs={'pk': cover.id}))
> >  
> >      def get_headers(self, instance):
> >          if instance.headers:
> > @@ -65,9 +71,14 @@ class
> > CoverLetterDetailSerializer(CoverLetterListSerializer):
> >  
> >      class Meta:
> >          model = CoverLetter
> > -        fields = CoverLetterListSerializer.Meta.fields + ('headers',
> > 'content')
> > +        fields = CoverLetterListSerializer.Meta.fields + ('headers',
> > +                                                          'content',
> > +                                                          'comments')
> >          read_only_fields = fields
> >          extra_kwargs = CoverLetterListSerializer.Meta.extra_kwargs
> > +        versioned_fields = {
> > +            '1.1': ('mbox', 'comments'),
> > +        }
> 
> Good spot (the 'mbox' field).
> 
> >  
> >  
> >  class CoverLetterList(ListAPIView):
> > diff --git a/patchwork/api/patch.py b/patchwork/api/patch.py
> > index 115feff..feb7d80 100644
> > --- a/patchwork/api/patch.py
> > +++ b/patchwork/api/patch.py
> > @@ -24,9 +24,9 @@ from rest_framework.generics import ListAPIView
> >  from rest_framework.generics import RetrieveUpdateAPIView
> >  from rest_framework.relations import RelatedField
> >  from rest_framework.reverse import reverse
> > -from rest_framework.serializers import HyperlinkedModelSerializer
> >  from rest_framework.serializers import SerializerMethodField
> >  
> > +from patchwork.api.base import BaseHyperlinkedModelSerializer
> >  from patchwork.api.base import PatchworkPermission
> >  from patchwork.api.filters import PatchFilter
> >  from patchwork.api.embedded import PersonSerializer
> > @@ -75,7 +75,7 @@ class StateField(RelatedField):
> >          return State.objects.all()
> >  
> >  
> > -class PatchListSerializer(HyperlinkedModelSerializer):
> > +class PatchListSerializer(BaseHyperlinkedModelSerializer):
> >  
> >      project = ProjectSerializer(read_only=True)
> >      state = StateField()
> > @@ -121,6 +121,11 @@ class PatchDetailSerializer(PatchListSerializer):
> >  
> >      headers = SerializerMethodField()
> >      prefixes = SerializerMethodField()
> > +    comments = SerializerMethodField()
> > +
> > +    def get_comments(self, patch):
> > +        return self.context.get('request').build_absolute_uri(
> > +            reverse('api-comment-list', kwargs={'pk': patch.id}))
> >  
> >      def get_headers(self, patch):
> >          if patch.headers:
> > @@ -132,10 +137,13 @@ class PatchDetailSerializer(PatchListSerializer):
> >      class Meta:
> >          model = Patch
> >          fields = PatchListSerializer.Meta.fields + (
> > -            'headers', 'content', 'diff', 'prefixes')
> > +            'headers', 'content', 'diff', 'prefixes', 'comments')
> >          read_only_fields = PatchListSerializer.Meta.read_only_fields + (
> > -            'headers', 'content', 'diff', 'prefixes')
> > +            'headers', 'content', 'diff', 'prefixes', 'comments')
> >          extra_kwargs = PatchListSerializer.Meta.extra_kwargs
> > +        versioned_fields = {
> > +            '1.1': ('comments', ),
> > +        }
> >  
> >  
> >  class PatchList(ListAPIView):
> > diff --git a/patchwork/models.py b/patchwork/models.py
> > index f91b994..67c2d3a 100644
> > --- a/patchwork/models.py
> > +++ b/patchwork/models.py
> > @@ -613,6 +613,9 @@ class Comment(EmailMixin, models.Model):
> >          if hasattr(self.submission, 'patch'):
> >              self.submission.patch.refresh_tag_counts()
> >  
> > +    def is_editable(self, user):
> > +        return False
> > +
> >      class Meta:
> >          ordering = ['date']
> >          unique_together = [('msgid', 'submission')]
> > diff --git a/patchwork/tests/api/test_comment.py
> > b/patchwork/tests/api/test_comment.py
> > new file mode 100644
> > index 0000000..b283bfe
> > --- /dev/null
> > +++ b/patchwork/tests/api/test_comment.py
> > @@ -0,0 +1,115 @@
> > +# Patchwork - automated patch tracking system
> > +# Copyright (C) 2018 Red Hat
> > +#
> > +# This file is part of the Patchwork package.
> > +#
> > +# Patchwork is free software; you can redistribute it and/or modify
> > +# it under the terms of the GNU General Public License as published by
> > +# the Free Software Foundation; either version 2 of the License, or
> > +# (at your option) any later version.
> > +#
> > +# Patchwork is distributed in the hope that it will be useful,
> > +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> > +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> > +# GNU General Public License for more details.
> > +#
> > +# You should have received a copy of the GNU General Public License
> > +# along with Patchwork; if not, write to the Free Software
> > +# Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307
> > USA
> > +
> > +import unittest
> > +
> > +from django.conf import settings
> > +
> > +from patchwork.compat import NoReverseMatch
> > +from patchwork.compat import reverse
> > +from patchwork.tests.utils import create_comment
> > +from patchwork.tests.utils import create_cover
> > +from patchwork.tests.utils import create_patch
> > +from patchwork.tests.utils import SAMPLE_CONTENT
> > +
> > +if settings.ENABLE_REST_API:
> > +    from rest_framework import status
> > +    from rest_framework.test import APITestCase
> > +else:
> > +    # stub out APITestCase
> > +    from django.test import TestCase
> > +    APITestCase = TestCase  # noqa
> > +
> > +
> > +@unittest.skipUnless(settings.ENABLE_REST_API, 'requires ENABLE_REST_API')
> > +class TestCoverComments(APITestCase):
> > +    @staticmethod
> > +    def api_url(cover, version=None):
> > +        kwargs = {}
> > +        if version:
> > +            kwargs['version'] = version
> > +        kwargs['pk'] = cover.id
> > +
> > +        return reverse('api-comment-list', kwargs=kwargs)
> > +
> > +    def assertSerialized(self, comment_obj, comment_json):
> > +        self.assertEqual(comment_obj.id, comment_json['id'])
> > +        self.assertEqual(comment_obj.submitter.id,
> > +                         comment_json['submitter']['id'])
> > +        self.assertIn(SAMPLE_CONTENT, comment_json['content'])
> > +
> > +    def test_list(self):
> > +        cover_obj = create_cover()
> > +        resp = self.client.get(self.api_url(cover_obj))
> > +        self.assertEqual(status.HTTP_200_OK, resp.status_code)
> > +        self.assertEqual(0, len(resp.data))
> > +
> > +        comment_obj = create_comment(submission=cover_obj)
> > +        resp = self.client.get(self.api_url(cover_obj))
> > +        self.assertEqual(status.HTTP_200_OK, resp.status_code)
> > +        self.assertEqual(1, len(resp.data))
> > +        self.assertSerialized(comment_obj, resp.data[0])
> > +
> > +        another_comment = create_comment(submission=cover_obj)
> > +        resp = self.client.get(self.api_url(cover_obj))
> > +        self.assertEqual(status.HTTP_200_OK, resp.status_code)
> > +        self.assertEqual(2, len(resp.data))
> > +
> > +        # check we can't access comments using the old version of the API
> > +        with self.assertRaises(NoReverseMatch):
> > +            self.client.get(self.api_url(cover_obj, version='1.0'))
> > +
> > +
> > +@unittest.skipUnless(settings.ENABLE_REST_API, 'requires ENABLE_REST_API')
> > +class TestPatchComments(APITestCase):
> > +    @staticmethod
> > +    def api_url(patch, version=None):
> > +        kwargs = {}
> > +        if version:
> > +            kwargs['version'] = version
> > +        kwargs['pk'] = patch.id
> > +
> > +        return reverse('api-comment-list', kwargs=kwargs)
> > +
> > +    def assertSerialized(self, comment_obj, comment_json):
> > +        self.assertEqual(comment_obj.id, comment_json['id'])
> > +        self.assertEqual(comment_obj.submitter.id,
> > +                         comment_json['submitter']['id'])
> > +        self.assertIn(SAMPLE_CONTENT, comment_json['content'])
> > +
> > +    def test_list(self):
> > +        patch_obj = create_patch()
> > +        resp = self.client.get(self.api_url(patch_obj))
> > +        self.assertEqual(status.HTTP_200_OK, resp.status_code)
> > +        self.assertEqual(0, len(resp.data))
> > +
> > +        comment_obj = create_comment(submission=patch_obj)
> > +        resp = self.client.get(self.api_url(patch_obj))
> > +        self.assertEqual(status.HTTP_200_OK, resp.status_code)
> > +        self.assertEqual(1, len(resp.data))
> > +        self.assertSerialized(comment_obj, resp.data[0])
> > +
> > +        another_comment = create_comment(submission=patch_obj)
> > +        resp = self.client.get(self.api_url(patch_obj))
> > +        self.assertEqual(status.HTTP_200_OK, resp.status_code)
> > +        self.assertEqual(2, len(resp.data))
> > +
> > +        # check we can't access comments using the old version of the API
> > +        with self.assertRaises(NoReverseMatch):
> > +            self.client.get(self.api_url(patch_obj, version='1.0'))
> > diff --git a/patchwork/tests/api/test_cover.py
> > b/patchwork/tests/api/test_cover.py
> > index 3135b7e..9a0fbbb 100644
> > --- a/patchwork/tests/api/test_cover.py
> > +++ b/patchwork/tests/api/test_cover.py
> > @@ -49,7 +49,8 @@ class TestCoverLetterAPI(APITestCase):
> >  
> >          if item is None:
> >              return reverse('api-cover-list', kwargs=kwargs)
> > -        return reverse('api-cover-detail', args=[item], kwargs=kwargs)
> > +        kwargs['pk'] = item
> > +        return reverse('api-cover-detail', kwargs=kwargs)
> >  
> >      def assertSerialized(self, cover_obj, cover_json):
> >          self.assertEqual(cover_obj.id, cover_json['id'])
> > @@ -115,6 +116,14 @@ class TestCoverLetterAPI(APITestCase):
> >          self.assertEqual(status.HTTP_200_OK, resp.status_code)
> >          self.assertSerialized(cover_obj, resp.data)
> >  
> > +        # test comments
> > +        resp = self.client.get(self.api_url(cover_obj.id))
> > +        self.assertIn('comments', resp.data)
> > +
> > +        # test old version of API
> > +        resp = self.client.get(self.api_url(cover_obj.id, version='1.0'))
> > +        self.assertNotIn('comments', resp.data)
> > +
> >      def test_create_update_delete(self):
> >          user = create_maintainer()
> >          user.is_superuser = True
> > diff --git a/patchwork/tests/api/test_patch.py
> > b/patchwork/tests/api/test_patch.py
> > index 909c1eb..d9d44d3 100644
> > --- a/patchwork/tests/api/test_patch.py
> > +++ b/patchwork/tests/api/test_patch.py
> > @@ -45,10 +45,15 @@ class TestPatchAPI(APITestCase):
> >      fixtures = ['default_tags']
> >  
> >      @staticmethod
> > -    def api_url(item=None):
> > +    def api_url(item=None, version=None):
> > +        kwargs = {}
> > +        if version:
> > +            kwargs['version'] = version
> > +
> >          if item is None:
> > -            return reverse('api-patch-list')
> > -        return reverse('api-patch-detail', args=[item])
> > +            return reverse('api-patch-list', kwargs=kwargs)
> > +        kwargs['pk'] = item
> > +        return reverse('api-patch-detail', kwargs=kwargs)
> >  
> >      def assertSerialized(self, patch_obj, patch_json):
> >          self.assertEqual(patch_obj.id, patch_json['id'])
> > @@ -130,6 +135,14 @@ class TestPatchAPI(APITestCase):
> >          self.assertEqual(patch.diff, resp.data['diff'])
> >          self.assertEqual(0, len(resp.data['tags']))
> >  
> > +        # test comments
> > +        resp = self.client.get(self.api_url(patch.id))
> > +        self.assertIn('comments', resp.data)
> > +
> > +        # test old version of API
> > +        resp = self.client.get(self.api_url(item=patch.id, version='1.0'))
> > +        self.assertNotIn('comments', resp.data)
> > +
> >      def test_create(self):
> >          """Ensure creations are rejected."""
> >          project = create_project()
> > diff --git a/patchwork/urls.py b/patchwork/urls.py
> > index 0893fe2..1dc4ffc 100644
> > --- a/patchwork/urls.py
> > +++ b/patchwork/urls.py
> > @@ -213,6 +213,7 @@ if settings.ENABLE_REST_API:
> >  
> >      from patchwork.api import bundle as api_bundle_views  # noqa
> >      from patchwork.api import check as api_check_views  # noqa
> > +    from patchwork.api import comment as api_comment_views  # noqa
> >      from patchwork.api import cover as api_cover_views  # noqa
> >      from patchwork.api import event as api_event_views  # noqa
> >      from patchwork.api import index as api_index_views  # noqa
> > @@ -279,8 +280,18 @@ if settings.ENABLE_REST_API:
> >              name='api-event-list'),
> >      ]
> >  
> > +    api_1_1_patterns = [
> > +        url(r'^patches/(?P<pk>[^/]+)/comments/$',
> > +            api_comment_views.CommentList.as_view(),
> > +            name='api-comment-list'),
> > +        url(r'^covers/(?P<pk>[^/]+)/comments/$',
> > +            api_comment_views.CommentList.as_view(),
> > +            name='api-comment-list'),
> > +    ]
> > +
> 
> I was wondering if you'd remember this. Nicely done :)
> 
> >      urlpatterns += [
> >          url(r'^api/(?:(?P<version>(1.0|1.1))/)?', include(api_patterns)),
> > +        url(r'^api/(?:(?P<version>1.1)/)?', include(api_1_1_patterns)),
> >  
> >          # token change
> >          url(r'^user/generate-token/$', user_views.generate_token,
> > diff --git a/releasenotes/notes/comments-api-b7dff6ee4ce04c9b.yaml
> > b/releasenotes/notes/comments-api-b7dff6ee4ce04c9b.yaml
> > new file mode 100644
> > index 0000000..5bc4773
> > --- /dev/null
> > +++ b/releasenotes/notes/comments-api-b7dff6ee4ce04c9b.yaml
> > @@ -0,0 +1,8 @@
> > +---
> > +api:
> > +  - |
> > +    Links to related comments are now exposed when checking patch and
> > cover
> > +    letter details. The comments themselves are then available via
> > +    ``/patches/{patchID}/comments`` and ``/covers/{coverID}/comments``
> > +    endpoints. Please note that comments are available only since API
> > +    version 1.1
> 
> 

I'll clean up those two things and resend, hopefully today.

Thanks,

Veronika

Patch

diff --git a/patchwork/api/comment.py b/patchwork/api/comment.py
new file mode 100644
index 0000000..a650168
--- /dev/null
+++ b/patchwork/api/comment.py
@@ -0,0 +1,80 @@ 
+# Patchwork - automated patch tracking system
+# Copyright (C) 2018 Red Hat
+#
+# This file is part of the Patchwork package.
+#
+# Patchwork is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# Patchwork is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with Patchwork; if not, write to the Free Software
+# Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
+
+import email.parser
+
+from rest_framework.generics import ListAPIView
+from rest_framework.serializers import SerializerMethodField
+
+from patchwork.api.base import BaseHyperlinkedModelSerializer
+from patchwork.api.base import PatchworkPermission
+from patchwork.api.embedded import PersonSerializer
+from patchwork.api.embedded import ProjectSerializer
+from patchwork.models import Comment
+
+
+class CommentListSerializer(BaseHyperlinkedModelSerializer):
+
+    project = ProjectSerializer(source='submission.project', read_only=True)
+    tags = SerializerMethodField()
+    subject = SerializerMethodField()
+    headers = SerializerMethodField()
+    submitter = PersonSerializer(read_only=True)
+
+    def get_tags(self, comment):
+        # TODO implement after we get support for tags on comments
+        return {}
+
+    def get_subject(self, comment):
+        return email.parser.Parser().parsestr(comment.headers,
+                                              True).get('Subject', '')
+
+    def get_headers(self, comment):
+        headers = {}
+
+        if comment.headers:
+            parsed = email.parser.Parser().parsestr(comment.headers, True)
+            for key in parsed.keys():
+                headers[key] = parsed.get_all(key)
+                # Let's return a single string instead of a list if only one
+                # header with this key is present
+                if len(headers[key]) == 1:
+                    headers[key] = headers[key][0]
+
+        return headers
+
+    class Meta:
+        model = Comment
+        fields = ('id', 'msgid', 'date', 'subject', 'project', 'submitter',
+                  'tags', 'content', 'headers')
+        read_only_fields = fields
+
+
+class CommentList(ListAPIView):
+    """List comments"""
+
+    permission_classes = (PatchworkPermission,)
+    serializer_class = CommentListSerializer
+    search_fields = ('subject',)
+    ordering_fields = ('id', 'subject', 'date', 'submitter')
+    ordering = 'id'
+    lookup_url_kwarg = 'pk'
+
+    def get_queryset(self):
+        return Comment.objects.filter(submission=self.kwargs['pk'])
diff --git a/patchwork/api/cover.py b/patchwork/api/cover.py
index fc7ae97..19f0f65 100644
--- a/patchwork/api/cover.py
+++ b/patchwork/api/cover.py
@@ -21,6 +21,7 @@  import email.parser
 
 from rest_framework.generics import ListAPIView
 from rest_framework.generics import RetrieveAPIView
+from rest_framework.reverse import reverse
 from rest_framework.serializers import SerializerMethodField
 
 from patchwork.api.base import BaseHyperlinkedModelSerializer
@@ -58,6 +59,11 @@  class CoverLetterListSerializer(BaseHyperlinkedModelSerializer):
 class CoverLetterDetailSerializer(CoverLetterListSerializer):
 
     headers = SerializerMethodField()
+    comments = SerializerMethodField()
+
+    def get_comments(self, cover):
+        return self.context.get('request').build_absolute_uri(
+            reverse('api-comment-list', kwargs={'pk': cover.id}))
 
     def get_headers(self, instance):
         if instance.headers:
@@ -65,9 +71,14 @@  class CoverLetterDetailSerializer(CoverLetterListSerializer):
 
     class Meta:
         model = CoverLetter
-        fields = CoverLetterListSerializer.Meta.fields + ('headers', 'content')
+        fields = CoverLetterListSerializer.Meta.fields + ('headers',
+                                                          'content',
+                                                          'comments')
         read_only_fields = fields
         extra_kwargs = CoverLetterListSerializer.Meta.extra_kwargs
+        versioned_fields = {
+            '1.1': ('mbox', 'comments'),
+        }
 
 
 class CoverLetterList(ListAPIView):
diff --git a/patchwork/api/patch.py b/patchwork/api/patch.py
index 115feff..feb7d80 100644
--- a/patchwork/api/patch.py
+++ b/patchwork/api/patch.py
@@ -24,9 +24,9 @@  from rest_framework.generics import ListAPIView
 from rest_framework.generics import RetrieveUpdateAPIView
 from rest_framework.relations import RelatedField
 from rest_framework.reverse import reverse
-from rest_framework.serializers import HyperlinkedModelSerializer
 from rest_framework.serializers import SerializerMethodField
 
+from patchwork.api.base import BaseHyperlinkedModelSerializer
 from patchwork.api.base import PatchworkPermission
 from patchwork.api.filters import PatchFilter
 from patchwork.api.embedded import PersonSerializer
@@ -75,7 +75,7 @@  class StateField(RelatedField):
         return State.objects.all()
 
 
-class PatchListSerializer(HyperlinkedModelSerializer):
+class PatchListSerializer(BaseHyperlinkedModelSerializer):
 
     project = ProjectSerializer(read_only=True)
     state = StateField()
@@ -121,6 +121,11 @@  class PatchDetailSerializer(PatchListSerializer):
 
     headers = SerializerMethodField()
     prefixes = SerializerMethodField()
+    comments = SerializerMethodField()
+
+    def get_comments(self, patch):
+        return self.context.get('request').build_absolute_uri(
+            reverse('api-comment-list', kwargs={'pk': patch.id}))
 
     def get_headers(self, patch):
         if patch.headers:
@@ -132,10 +137,13 @@  class PatchDetailSerializer(PatchListSerializer):
     class Meta:
         model = Patch
         fields = PatchListSerializer.Meta.fields + (
-            'headers', 'content', 'diff', 'prefixes')
+            'headers', 'content', 'diff', 'prefixes', 'comments')
         read_only_fields = PatchListSerializer.Meta.read_only_fields + (
-            'headers', 'content', 'diff', 'prefixes')
+            'headers', 'content', 'diff', 'prefixes', 'comments')
         extra_kwargs = PatchListSerializer.Meta.extra_kwargs
+        versioned_fields = {
+            '1.1': ('comments', ),
+        }
 
 
 class PatchList(ListAPIView):
diff --git a/patchwork/models.py b/patchwork/models.py
index f91b994..67c2d3a 100644
--- a/patchwork/models.py
+++ b/patchwork/models.py
@@ -613,6 +613,9 @@  class Comment(EmailMixin, models.Model):
         if hasattr(self.submission, 'patch'):
             self.submission.patch.refresh_tag_counts()
 
+    def is_editable(self, user):
+        return False
+
     class Meta:
         ordering = ['date']
         unique_together = [('msgid', 'submission')]
diff --git a/patchwork/tests/api/test_comment.py b/patchwork/tests/api/test_comment.py
new file mode 100644
index 0000000..b283bfe
--- /dev/null
+++ b/patchwork/tests/api/test_comment.py
@@ -0,0 +1,115 @@ 
+# Patchwork - automated patch tracking system
+# Copyright (C) 2018 Red Hat
+#
+# This file is part of the Patchwork package.
+#
+# Patchwork is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# Patchwork is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with Patchwork; if not, write to the Free Software
+# Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
+
+import unittest
+
+from django.conf import settings
+
+from patchwork.compat import NoReverseMatch
+from patchwork.compat import reverse
+from patchwork.tests.utils import create_comment
+from patchwork.tests.utils import create_cover
+from patchwork.tests.utils import create_patch
+from patchwork.tests.utils import SAMPLE_CONTENT
+
+if settings.ENABLE_REST_API:
+    from rest_framework import status
+    from rest_framework.test import APITestCase
+else:
+    # stub out APITestCase
+    from django.test import TestCase
+    APITestCase = TestCase  # noqa
+
+
+@unittest.skipUnless(settings.ENABLE_REST_API, 'requires ENABLE_REST_API')
+class TestCoverComments(APITestCase):
+    @staticmethod
+    def api_url(cover, version=None):
+        kwargs = {}
+        if version:
+            kwargs['version'] = version
+        kwargs['pk'] = cover.id
+
+        return reverse('api-comment-list', kwargs=kwargs)
+
+    def assertSerialized(self, comment_obj, comment_json):
+        self.assertEqual(comment_obj.id, comment_json['id'])
+        self.assertEqual(comment_obj.submitter.id,
+                         comment_json['submitter']['id'])
+        self.assertIn(SAMPLE_CONTENT, comment_json['content'])
+
+    def test_list(self):
+        cover_obj = create_cover()
+        resp = self.client.get(self.api_url(cover_obj))
+        self.assertEqual(status.HTTP_200_OK, resp.status_code)
+        self.assertEqual(0, len(resp.data))
+
+        comment_obj = create_comment(submission=cover_obj)
+        resp = self.client.get(self.api_url(cover_obj))
+        self.assertEqual(status.HTTP_200_OK, resp.status_code)
+        self.assertEqual(1, len(resp.data))
+        self.assertSerialized(comment_obj, resp.data[0])
+
+        another_comment = create_comment(submission=cover_obj)
+        resp = self.client.get(self.api_url(cover_obj))
+        self.assertEqual(status.HTTP_200_OK, resp.status_code)
+        self.assertEqual(2, len(resp.data))
+
+        # check we can't access comments using the old version of the API
+        with self.assertRaises(NoReverseMatch):
+            self.client.get(self.api_url(cover_obj, version='1.0'))
+
+
+@unittest.skipUnless(settings.ENABLE_REST_API, 'requires ENABLE_REST_API')
+class TestPatchComments(APITestCase):
+    @staticmethod
+    def api_url(patch, version=None):
+        kwargs = {}
+        if version:
+            kwargs['version'] = version
+        kwargs['pk'] = patch.id
+
+        return reverse('api-comment-list', kwargs=kwargs)
+
+    def assertSerialized(self, comment_obj, comment_json):
+        self.assertEqual(comment_obj.id, comment_json['id'])
+        self.assertEqual(comment_obj.submitter.id,
+                         comment_json['submitter']['id'])
+        self.assertIn(SAMPLE_CONTENT, comment_json['content'])
+
+    def test_list(self):
+        patch_obj = create_patch()
+        resp = self.client.get(self.api_url(patch_obj))
+        self.assertEqual(status.HTTP_200_OK, resp.status_code)
+        self.assertEqual(0, len(resp.data))
+
+        comment_obj = create_comment(submission=patch_obj)
+        resp = self.client.get(self.api_url(patch_obj))
+        self.assertEqual(status.HTTP_200_OK, resp.status_code)
+        self.assertEqual(1, len(resp.data))
+        self.assertSerialized(comment_obj, resp.data[0])
+
+        another_comment = create_comment(submission=patch_obj)
+        resp = self.client.get(self.api_url(patch_obj))
+        self.assertEqual(status.HTTP_200_OK, resp.status_code)
+        self.assertEqual(2, len(resp.data))
+
+        # check we can't access comments using the old version of the API
+        with self.assertRaises(NoReverseMatch):
+            self.client.get(self.api_url(patch_obj, version='1.0'))
diff --git a/patchwork/tests/api/test_cover.py b/patchwork/tests/api/test_cover.py
index 3135b7e..9a0fbbb 100644
--- a/patchwork/tests/api/test_cover.py
+++ b/patchwork/tests/api/test_cover.py
@@ -49,7 +49,8 @@  class TestCoverLetterAPI(APITestCase):
 
         if item is None:
             return reverse('api-cover-list', kwargs=kwargs)
-        return reverse('api-cover-detail', args=[item], kwargs=kwargs)
+        kwargs['pk'] = item
+        return reverse('api-cover-detail', kwargs=kwargs)
 
     def assertSerialized(self, cover_obj, cover_json):
         self.assertEqual(cover_obj.id, cover_json['id'])
@@ -115,6 +116,14 @@  class TestCoverLetterAPI(APITestCase):
         self.assertEqual(status.HTTP_200_OK, resp.status_code)
         self.assertSerialized(cover_obj, resp.data)
 
+        # test comments
+        resp = self.client.get(self.api_url(cover_obj.id))
+        self.assertIn('comments', resp.data)
+
+        # test old version of API
+        resp = self.client.get(self.api_url(cover_obj.id, version='1.0'))
+        self.assertNotIn('comments', resp.data)
+
     def test_create_update_delete(self):
         user = create_maintainer()
         user.is_superuser = True
diff --git a/patchwork/tests/api/test_patch.py b/patchwork/tests/api/test_patch.py
index 909c1eb..d9d44d3 100644
--- a/patchwork/tests/api/test_patch.py
+++ b/patchwork/tests/api/test_patch.py
@@ -45,10 +45,15 @@  class TestPatchAPI(APITestCase):
     fixtures = ['default_tags']
 
     @staticmethod
-    def api_url(item=None):
+    def api_url(item=None, version=None):
+        kwargs = {}
+        if version:
+            kwargs['version'] = version
+
         if item is None:
-            return reverse('api-patch-list')
-        return reverse('api-patch-detail', args=[item])
+            return reverse('api-patch-list', kwargs=kwargs)
+        kwargs['pk'] = item
+        return reverse('api-patch-detail', kwargs=kwargs)
 
     def assertSerialized(self, patch_obj, patch_json):
         self.assertEqual(patch_obj.id, patch_json['id'])
@@ -130,6 +135,14 @@  class TestPatchAPI(APITestCase):
         self.assertEqual(patch.diff, resp.data['diff'])
         self.assertEqual(0, len(resp.data['tags']))
 
+        # test comments
+        resp = self.client.get(self.api_url(patch.id))
+        self.assertIn('comments', resp.data)
+
+        # test old version of API
+        resp = self.client.get(self.api_url(item=patch.id, version='1.0'))
+        self.assertNotIn('comments', resp.data)
+
     def test_create(self):
         """Ensure creations are rejected."""
         project = create_project()
diff --git a/patchwork/urls.py b/patchwork/urls.py
index 0893fe2..1dc4ffc 100644
--- a/patchwork/urls.py
+++ b/patchwork/urls.py
@@ -213,6 +213,7 @@  if settings.ENABLE_REST_API:
 
     from patchwork.api import bundle as api_bundle_views  # noqa
     from patchwork.api import check as api_check_views  # noqa
+    from patchwork.api import comment as api_comment_views  # noqa
     from patchwork.api import cover as api_cover_views  # noqa
     from patchwork.api import event as api_event_views  # noqa
     from patchwork.api import index as api_index_views  # noqa
@@ -279,8 +280,18 @@  if settings.ENABLE_REST_API:
             name='api-event-list'),
     ]
 
+    api_1_1_patterns = [
+        url(r'^patches/(?P<pk>[^/]+)/comments/$',
+            api_comment_views.CommentList.as_view(),
+            name='api-comment-list'),
+        url(r'^covers/(?P<pk>[^/]+)/comments/$',
+            api_comment_views.CommentList.as_view(),
+            name='api-comment-list'),
+    ]
+
     urlpatterns += [
         url(r'^api/(?:(?P<version>(1.0|1.1))/)?', include(api_patterns)),
+        url(r'^api/(?:(?P<version>1.1)/)?', include(api_1_1_patterns)),
 
         # token change
         url(r'^user/generate-token/$', user_views.generate_token,
diff --git a/releasenotes/notes/comments-api-b7dff6ee4ce04c9b.yaml b/releasenotes/notes/comments-api-b7dff6ee4ce04c9b.yaml
new file mode 100644
index 0000000..5bc4773
--- /dev/null
+++ b/releasenotes/notes/comments-api-b7dff6ee4ce04c9b.yaml
@@ -0,0 +1,8 @@ 
+---
+api:
+  - |
+    Links to related comments are now exposed when checking patch and cover
+    letter details. The comments themselves are then available via
+    ``/patches/{patchID}/comments`` and ``/covers/{coverID}/comments``
+    endpoints. Please note that comments are available only since API
+    version 1.1