On Fri, 2018-03-23 at 13:33 +0100, vkaba...@redhat.com wrote:
> From: Veronika Kabatova <vkaba...@redhat.com>
> 
> Signed-off-by: Veronika Kabatova <vkaba...@redhat.com>

Looks pretty good. As you note, it does need some tests. I'm also
curious about the idea of nesting this under the patches endpoint. Let
me know what you think.

Stephen

> ---
>  docs/api/rest.rst                                  |   6 +-
>  patchwork/api/comment.py                           | 117 
> +++++++++++++++++++++
>  patchwork/api/filters.py                           |  16 +++
>  patchwork/api/index.py                             |   1 +
>  patchwork/models.py                                |   3 +
>  patchwork/urls.py                                  |  10 ++
>  .../notes/comments-api-b7dff6ee4ce04c9b.yaml       |  10 ++
>  7 files changed, 161 insertions(+), 2 deletions(-)
>  create mode 100644 patchwork/api/comment.py
>  create mode 100644 releasenotes/notes/comments-api-b7dff6ee4ce04c9b.yaml
> 
> diff --git a/docs/api/rest.rst b/docs/api/rest.rst
> index d526b27..e33aefc 100644
> --- a/docs/api/rest.rst
> +++ b/docs/api/rest.rst
> @@ -48,7 +48,8 @@ Patchwork instance hosted at `patchwork.example.com`, run:
>          "people": "https://patchwork.example.com/api/1.0/people/";,
>          "projects": "https://patchwork.example.com/api/1.0/projects/";,
>          "series": "https://patchwork.example.com/api/1.0/series/";,
> -        "users": "https://patchwork.example.com/api/1.0/users/";
> +        "users": "https://patchwork.example.com/api/1.0/users/";,
> +        "comments": "https://patchwork.example.com/api/1.0/comments/";
>      }
>  
>  
> @@ -71,7 +72,8 @@ well-supported. To repeat the above example using 
> `requests`:, run
>          "people": "https://patchwork.example.com/api/1.0/people/";,
>          "projects": "https://patchwork.example.com/api/1.0/projects/";,
>          "series": "https://patchwork.example.com/api/1.0/series/";,
> -        "users": "https://patchwork.example.com/api/1.0/users/";
> +        "users": "https://patchwork.example.com/api/1.0/users/";,
> +        "comments": "https://patchwork.example.com/api/1.0/comments/";
>      }
>  
>  Tools like `curl` and libraries like `requests` can be used to build anything
> diff --git a/patchwork/api/comment.py b/patchwork/api/comment.py
> new file mode 100644
> index 0000000..4252ecd
> --- /dev/null
> +++ b/patchwork/api/comment.py
> @@ -0,0 +1,117 @@
> +# 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.generics import RetrieveUpdateAPIView
> +from rest_framework.reverse import reverse
> +from rest_framework.serializers import HyperlinkedModelSerializer
> +from rest_framework.serializers import SerializerMethodField
> +
> +from patchwork.api.base import PatchworkPermission
> +from patchwork.api.filters import CommentFilter
> +from patchwork.api.embedded import PersonSerializer
> +from patchwork.api.embedded import ProjectSerializer
> +from patchwork.models import Comment
> +
> +
> +class CommentListSerializer(HyperlinkedModelSerializer):
> +
> +    submitter = PersonSerializer(read_only=True)
> +    tags = SerializerMethodField()
> +    subject = SerializerMethodField()
> +    parent = SerializerMethodField(source='submission')
> +
> +    def get_parent(self, instance):
> +        attrs = {'subject': instance.submission.name,
> +                 'msgid': instance.submission.msgid,
> +                 'date': instance.submission.date}
> +
> +        if hasattr(instance.submission, 'patch'):
> +            attrs['url'] = self.context.get('request').build_absolute_uri(
> +                reverse('api-patch-detail', args=[instance.submission.id]))
> +        else:
> +            attrs['url'] = self.context.get('request').build_absolute_uri(
> +                reverse('api-cover-detail', args=[instance.submission.id]))
> +
> +        return attrs
> +
> +    def get_subject(self, instance):
> +        return email.parser.Parser().parsestr(instance.headers,
> +                                              True).get('Subject', '')
> +
> +    def get_tags(self, instance):
> +        # TODO implement after we get support for tags on comments
> +        return {}
> +
> +    class Meta:
> +        model = Comment
> +        fields = ('id', 'url', 'msgid', 'date', 'subject', 'submitter',
> +                  'parent', 'tags')
> +        read_only_fields = fields
> +        extra_kwargs = {
> +            'url': {'view_name': 'api-comment-detail'},
> +        }
> +
> +
> +class CommentDetailSerializer(CommentListSerializer):
> +
> +    headers = SerializerMethodField()
> +    project = ProjectSerializer(source='submission.project', read_only=True)
> +
> +    def get_headers(self, comment):
> +        if comment.headers:
> +            return email.parser.Parser().parsestr(comment.headers, True)
> +
> +    class Meta:
> +        model = Comment
> +        fields = CommentListSerializer.Meta.fields + (
> +            'content', 'headers', 'project'
> +        )
> +        read_only_fields = fields
> +        extra_kwargs = CommentListSerializer.Meta.extra_kwargs
> +
> +
> +class CommentList(ListAPIView):
> +    """List comments"""
> +
> +    permission_classes = (PatchworkPermission,)
> +    serializer_class = CommentListSerializer
> +    filter_class = CommentFilter
> +    search_fields = ('subject',)
> +    ordering_fields = ('id', 'subject', 'date', 'submitter')
> +    ordering = 'id'
> +
> +    def get_queryset(self):
> +        return Comment.objects.all().select_related(
> +            'submission'
> +        ).prefetch_related('related_tags').defer('content')
> +
> +
> +class CommentDetail(RetrieveUpdateAPIView):
> +    """Show a comment"""
> +
> +    permission_classes = (PatchworkPermission,)
> +    serializer_class = CommentDetailSerializer
> +
> +    def get_queryset(self):
> +        return Comment.objects.all().select_related(
> +            'submission'
> +        ).prefetch_related('related_tags')
> diff --git a/patchwork/api/filters.py b/patchwork/api/filters.py
> index d207b2b..8256d8c 100644
> --- a/patchwork/api/filters.py
> +++ b/patchwork/api/filters.py
> @@ -26,6 +26,7 @@ from django.forms import ModelChoiceField
>  
>  from patchwork.models import Bundle
>  from patchwork.models import Check
> +from patchwork.models import Comment
>  from patchwork.models import CoverLetter
>  from patchwork.models import Event
>  from patchwork.models import Patch
> @@ -33,6 +34,7 @@ from patchwork.models import Person
>  from patchwork.models import Project
>  from patchwork.models import Series
>  from patchwork.models import State
> +from patchwork.models import Submission
>  
>  
>  class TimestampMixin(FilterSet):
> @@ -186,3 +188,17 @@ class BundleFilter(ProjectMixin, FilterSet):
>      class Meta:
>          model = Bundle
>          fields = ('project', 'owner', 'public')
> +
> +
> +class CommentFilter(ProjectMixin, TimestampMixin, FilterSet):
> +
> +    submitter = PersonFilter(queryset=Person.objects.all())
> +    parent = ModelChoiceFilter(name='submission',
> +                               queryset=Submission.objects.all())

Assuming we go this route, can you call this 'submission'?

> +    project = ProjectFilter(to_field_name='linkname',
> +                            name='submission__project',
> +                            queryset=Project.objects.all())
> +
> +    class Meta:
> +        model = Comment
> +        fields = ('project', 'parent', 'submitter')
> diff --git a/patchwork/api/index.py b/patchwork/api/index.py
> index 53494db..ebc8dbf 100644
> --- a/patchwork/api/index.py
> +++ b/patchwork/api/index.py
> @@ -34,4 +34,5 @@ class IndexView(APIView):
>              'series': reverse('api-series-list', request=request),
>              'events': reverse('api-event-list', request=request),
>              'bundles': reverse('api-bundle-list', request=request),
> +            'comments': reverse('api-comment-list', request=request),
>          })
> 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/urls.py b/patchwork/urls.py
> index 7193472..7ae4425 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
> @@ -238,6 +239,12 @@ if settings.ENABLE_REST_API:
>          url(r'^people/(?P<pk>[^/]+)/$',
>              api_person_views.PersonDetail.as_view(),
>              name='api-person-detail'),
> +        url(r'^comments/$',
> +            api_comment_views.CommentList.as_view(),
> +            name='api-comment-list'),
> +        url(r'^comments/(?P<pk>[^/]+)/$',
> +            api_comment_views.CommentDetail.as_view(),
> +            name='api-comment-detail'),

Comments sound like something that's very much a child thing. What
about nesting these underneath patch/cover?

    /patches/{patchID}/comments

Would solve a couple of the filtering problems above too; however, it
would prevent you from listing every comment for a given project. I'm
not sure if the latter issue is something you'd really be bothered
about?

>          url(r'^covers/$',
>              api_cover_views.CoverLetterList.as_view(),
>              name='api-cover-list'),
> @@ -277,6 +284,9 @@ if settings.ENABLE_REST_API:
>          url(r'^events/$',
>              api_event_views.EventList.as_view(),
>              name='api-event-list'),
> +        url(r'^comments/$',
> +            api_comment_views.CommentList.as_view(),
> +            name='api-comment-list'),

Looks like you've registered this twice.

>      ]
>  
>      urlpatterns += [
> diff --git a/releasenotes/notes/comments-api-b7dff6ee4ce04c9b.yaml 
> b/releasenotes/notes/comments-api-b7dff6ee4ce04c9b.yaml
> new file mode 100644
> index 0000000..72b3339
> --- /dev/null
> +++ b/releasenotes/notes/comments-api-b7dff6ee4ce04c9b.yaml
> @@ -0,0 +1,10 @@
> +---
> +api:
> +  - |
> +    Added /comments endpoint. For comment list, fields

``/comments`` (or ``/patches/{patchID}/comments``)

> +        'id', 'url', 'msgid', 'date', 'subject', 'submitter', 'parent', 
> 'tags'
> +    are available. For comment detail (/comments/29), comment's
> +        'content', 'headers', 'project'
> +    are added as well. Filtering is possible based on
> +        'project', 'parent', 'submitter'
> +    fields.

You also need to version this, per recent changes. Basically you want
this entire API to 404 if using version 1.0 and drop 'comments' from
the root response for same. See the recent changes to the 'api'
directory for examples of the latter.

Stephen
_______________________________________________
Patchwork mailing list
Patchwork@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/patchwork

Reply via email to