[RFC/PATCH 1/3] api: add addressed field and detail endpoint for patch comments
Raxel Gutierrez
raxel at google.com
Sat Jul 24 00:27:30 AEST 2021
Hi,
I'm sure this might be asked but I used the
RetrieveUpdateDestroyAPIView [1] which handles delete requests. I
changed this to RetrieveUpdateAPIView because patch comments should
not be removable. Currently working on tests for the endpoint :)
[1] https://www.django-rest-framework.org/api-guide/generic-views/#retrieveupdatedestroyapiview
On Thu, Jul 22, 2021 at 5:36 PM Raxel Gutierrez <raxel at google.com> 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.
>
> 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.
>
> Signed-off-by: Raxel Gutierrez <raxel at google.com>
> ---
> patchwork/api/base.py | 24 +++++-
> patchwork/api/check.py | 21 +----
> patchwork/api/comment.py | 76 +++++++++++++++----
> .../migrations/0045_patchcomment_addressed.py | 18 +++++
> patchwork/models.py | 3 +-
> patchwork/urls.py | 7 +-
> 6 files changed, 111 insertions(+), 38 deletions(-)
> create mode 100644 patchwork/migrations/0045_patchcomment_addressed.py
>
> 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..7f1e401 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 RetrieveUpdateDestroyAPIView
> +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
> @@ -55,6 +60,9 @@ class BaseCommentListSerializer(BaseHyperlinkedModelSerializer):
> '1.1': ('web_url', ),
> '1.2': ('list_archive_url',),
> }
> + extra_kwargs = {
> + 'url': {'view_name': 'api-patch-comment-detail'},
> + }
>
>
> class CoverCommentListSerializer(BaseCommentListSerializer):
> @@ -66,15 +74,47 @@ 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
> + 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,
> + RetrieveUpdateDestroyAPIView):
> + """
> + 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/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..0a77b23 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,7 @@ class PatchComment(EmailMixin, models.Model):
> self.patch.refresh_tag_counts()
>
> def is_editable(self, user):
> - return False
> + return self.patch.is_editable(user)
>
> class Meta:
> ordering = ['date']
> diff --git a/patchwork/urls.py b/patchwork/urls.py
> index 6ac9b81..81db982 100644
> --- a/patchwork/urls.py
> +++ b/patchwork/urls.py
> @@ -332,10 +332,15 @@ 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',
> ),
> + path(
> + 'patches/<patch_id>/comments/<comment_id>/',
> + api_comment_views.PatchCommentDetail.as_view(),
> + name='api-patch-comment-detail',
> + ),
> path(
> 'covers/<pk>/comments/',
> api_comment_views.CoverCommentList.as_view(),
> --
> 2.32.0.432.gabb21c7263-goog
>
More information about the Patchwork
mailing list