[PATCH v4 6/9] api: add comments detail endpoint
Raxel Gutierrez
raxel at google.com
Fri Aug 20 14:50:27 AEST 2021
Add new endpoint for patch and cover comments at api/.../comments/<comment_id>.
This involves updating the API version to v1.3 to reflect the new
endpoints as a minor change, following the usual semantic versioning
convention.
The endpoint will make it possible to use the REST API to update the new
`addressed` field for individual patch and cover comments with JavaScript
on the client side. In the process of these changes, clean up the use of
the CurrentPatchDefault context so that it exists in base.py and can be
used throughout the API (e.g. Check and Comment REST endpoints).
The tests cover retrieval and update requests and also handle calls from
the various API versions. Also, they cover permissions for update
requests and handle invalid update values for the new `addressed` field.
Signed-off-by: Raxel Gutierrez <raxel at google.com>
---
docs/api/schemas/generate-schemas.py | 4 +-
docs/api/schemas/patchwork.j2 | 165 ++++++++++++
patchwork/api/base.py | 24 +-
patchwork/api/check.py | 20 +-
patchwork/api/comment.py | 146 ++++++++--
patchwork/tests/api/test_comment.py | 388 ++++++++++++++++++++++++---
patchwork/urls.py | 20 +-
7 files changed, 682 insertions(+), 85 deletions(-)
diff --git a/docs/api/schemas/generate-schemas.py b/docs/api/schemas/generate-schemas.py
index a0c1e45f..3a436a16 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/patchwork.j2 b/docs/api/schemas/patchwork.j2
index af20743d..3b4ad2f6 100644
--- a/docs/api/schemas/patchwork.j2
+++ b/docs/api/schemas/patchwork.j2
@@ -324,6 +324,74 @@ paths:
$ref: '#/components/schemas/Error'
tags:
- comments
+{% if version >= (1, 3) %}
+ /api/{{ version_url }}covers/{cover_id}/comments/{comment_id}/:
+ parameters:
+ - in: path
+ name: cover_id
+ description: A unique integer value identifying the parent cover.
+ required: true
+ schema:
+ title: Cover 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 cover comment.
+ operationId: cover_comments_read
+ responses:
+ '200':
+ description: ''
+ content:
+ application/json:
+ schema:
+ $ref: '#/components/schemas/Comment'
+ '404':
+ description: Not found
+ content:
+ application/json:
+ schema:
+ $ref: '#/components/schemas/Error'
+ tags:
+ - comments
+ patch:
+ description: Update a cover comment (partial).
+ operationId: cover_comments_partial_update
+ requestBody:
+ $ref: '#/components/requestBodies/Comment'
+ responses:
+ '200':
+ description: ''
+ content:
+ application/json:
+ schema:
+ $ref: '#/components/schemas/Comment'
+ '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 }}events/:
get:
description: List events.
@@ -656,6 +724,74 @@ paths:
$ref: '#/components/schemas/Error'
tags:
- comments
+{% if version >= (1, 3) %}
+ /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
+ get:
+ description: Show a patch comment.
+ operationId: patch_comments_read
+ responses:
+ '200':
+ description: ''
+ content:
+ application/json:
+ schema:
+ $ref: '#/components/schemas/Comment'
+ '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
+ requestBody:
+ $ref: '#/components/requestBodies/Comment'
+ responses:
+ '200':
+ description: ''
+ content:
+ application/json:
+ schema:
+ $ref: '#/components/schemas/Comment'
+ '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 +1413,14 @@ components:
application/x-www-form-urlencoded:
schema:
$ref: '#/components/schemas/CheckCreate'
+{% if version >= (1, 3) %}
+ Comment:
+ required: true
+ content:
+ application/json:
+ schema:
+ $ref: '#/components/schemas/CommentUpdate'
+{% endif %}
Patch:
required: true
content:
@@ -1586,6 +1730,17 @@ components:
additionalProperties:
type: string
readOnly: true
+{% if version >= (1, 3) %}
+ addressed:
+ title: Addressed
+ type: boolean
+ CommentUpdate:
+ type: object
+ properties:
+ addressed:
+ title: Addressed
+ type: boolean
+{% endif %}
CoverList:
type: object
properties:
@@ -2659,6 +2814,16 @@ components:
items:
type: string
readOnly: true
+{% if version >= (1, 3) %}
+ ErrorCommentUpdate:
+ type: object
+ properties:
+ addressed:
+ title: Addressed
+ type: array
+ items:
+ type: string
+{% endif %}
ErrorPatchUpdate:
type: object
properties:
diff --git a/patchwork/api/base.py b/patchwork/api/base.py
index 89a43114..a98644ee 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, PatchComment
+ and CoverComment 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 a6bf5f8c..2049d2f9 100644
--- a/patchwork/api/check.py
+++ b/patchwork/api/check.py
@@ -6,7 +6,6 @@
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 +16,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 5d7a77a1..334016d7 100644
--- a/patchwork/api/comment.py
+++ b/patchwork/api/comment.py
@@ -4,13 +4,19 @@
# SPDX-License-Identifier: GPL-2.0-or-later
import email.parser
+import rest_framework
+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
@@ -18,6 +24,24 @@ from patchwork.models import Patch
from patchwork.models import PatchComment
+DRF_VERSION = tuple(int(x) for x in rest_framework.__version__.split('.'))
+
+
+if DRF_VERSION > (3, 11):
+ class CurrentCoverDefault(object):
+ requires_context = True
+
+ def __call__(self, serializer_field):
+ return serializer_field.context['request'].cover
+else:
+ class CurrentCoverDefault(object):
+ def set_context(self, serializer_field):
+ self.patch = serializer_field.context['request'].cover
+
+ def __call__(self):
+ return self.cover
+
+
class BaseCommentListSerializer(BaseHyperlinkedModelSerializer):
web_url = SerializerMethodField()
@@ -49,65 +73,133 @@ class BaseCommentListSerializer(BaseHyperlinkedModelSerializer):
class Meta:
fields = ('id', 'web_url', 'msgid', 'list_archive_url', 'date',
- 'subject', 'submitter', 'content', 'headers')
- read_only_fields = fields
+ 'subject', 'submitter', 'content', 'headers', 'addressed')
+ read_only_fields = ('id', 'web_url', 'msgid', 'list_archive_url',
+ 'date', 'subject', 'submitter', 'content',
+ 'headers')
versioned_fields = {
'1.1': ('web_url', ),
'1.2': ('list_archive_url',),
+ '1.3': ('addressed',),
}
-class CoverCommentListSerializer(BaseCommentListSerializer):
+class CoverCommentSerializer(BaseCommentListSerializer):
+
+ cover = HiddenField(default=CurrentCoverDefault())
class Meta:
model = CoverComment
- fields = BaseCommentListSerializer.Meta.fields
- read_only_fields = fields
+ fields = BaseCommentListSerializer.Meta.fields + (
+ 'cover', 'addressed')
+ read_only_fields = BaseCommentListSerializer.Meta.read_only_fields + (
+ 'cover', )
versioned_fields = BaseCommentListSerializer.Meta.versioned_fields
+ extra_kwargs = {
+ 'url': {'view_name': 'api-cover-comment-detail'}
+ }
+
+class CoverCommentMixin(object):
+
+ permission_classes = (PatchworkPermission,)
+ serializer_class = CoverCommentSerializer
-class PatchCommentListSerializer(BaseCommentListSerializer):
+ def get_object(self):
+ queryset = self.filter_queryset(self.get_queryset())
+ comment_id = self.kwargs['comment_id']
+ obj = get_object_or_404(queryset, id=int(comment_id))
+ self.check_object_permissions(self.request, obj)
+ return obj
+
+ def get_queryset(self):
+ cover_id = self.kwargs['cover_id']
+ if not Cover.objects.filter(id=cover_id).exists():
+ raise Http404
+
+ return CoverComment.objects.filter(
+ cover=cover_id
+ ).select_related('submitter')
+
+
+class PatchCommentSerializer(BaseCommentListSerializer):
+
+ patch = HiddenField(default=CurrentPatchDefault())
class Meta:
model = PatchComment
- fields = BaseCommentListSerializer.Meta.fields
- read_only_fields = fields
+ fields = BaseCommentListSerializer.Meta.fields + ('patch', )
+ read_only_fields = BaseCommentListSerializer.Meta.read_only_fields + (
+ 'patch', )
versioned_fields = BaseCommentListSerializer.Meta.versioned_fields
+ extra_kwargs = {
+ 'url': {'view_name': 'api-patch-comment-detail'}
+ }
-class CoverCommentList(ListAPIView):
- """List cover comments"""
+class PatchCommentMixin(object):
permission_classes = (PatchworkPermission,)
- serializer_class = CoverCommentListSerializer
+ serializer_class = PatchCommentSerializer
+
+ def get_object(self):
+ queryset = self.filter_queryset(self.get_queryset())
+ comment_id = self.kwargs['comment_id']
+ obj = get_object_or_404(queryset, id=int(comment_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(CoverCommentMixin, ListAPIView):
+ """List cover comments"""
+
search_fields = ('subject',)
ordering_fields = ('id', 'subject', 'date', 'submitter')
ordering = 'id'
lookup_url_kwarg = 'cover_id'
- def get_queryset(self):
- if not Cover.objects.filter(id=self.kwargs['cover_id']).exists():
- raise Http404
- return CoverComment.objects.filter(
- cover=self.kwargs['cover_id']
- ).select_related('submitter')
+class CoverCommentDetail(CoverCommentMixin, MultipleFieldLookupMixin,
+ RetrieveUpdateAPIView):
+ """
+ get:
+ Show a cover comment.
+ patch:
+ Update a cover comment.
+ put:
+ Update a cover comment.
+ """
+ lookup_url_kwargs = ('cover_id', 'comment_id')
+ lookup_fields = ('cover_id', 'id')
-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 = 'patch_id'
- def get_queryset(self):
- if not Patch.objects.filter(id=self.kwargs['patch_id']).exists():
- raise Http404
- return PatchComment.objects.filter(
- patch=self.kwargs['patch_id']
- ).select_related('submitter')
+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/tests/api/test_comment.py b/patchwork/tests/api/test_comment.py
index 53abf8f0..033a1416 100644
--- a/patchwork/tests/api/test_comment.py
+++ b/patchwork/tests/api/test_comment.py
@@ -9,11 +9,17 @@ from django.conf import settings
from django.urls import NoReverseMatch
from django.urls import reverse
+from patchwork.models import PatchComment
+from patchwork.models import CoverComment
from patchwork.tests.api import utils
from patchwork.tests.utils import create_cover
from patchwork.tests.utils import create_cover_comment
from patchwork.tests.utils import create_patch
from patchwork.tests.utils import create_patch_comment
+from patchwork.tests.utils import create_maintainer
+from patchwork.tests.utils import create_project
+from patchwork.tests.utils import create_person
+from patchwork.tests.utils import create_user
from patchwork.tests.utils import SAMPLE_CONTENT
if settings.ENABLE_REST_API:
@@ -23,18 +29,26 @@ if settings.ENABLE_REST_API:
@unittest.skipUnless(settings.ENABLE_REST_API, 'requires ENABLE_REST_API')
class TestCoverComments(utils.APITestCase):
@staticmethod
- def api_url(cover, version=None):
- kwargs = {}
+ def api_url(cover, version=None, item=None):
+ kwargs = {'cover_id': cover.id}
if version:
kwargs['version'] = version
- kwargs['cover_id'] = cover.id
+ if item is None:
+ return reverse('api-cover-comment-list', kwargs=kwargs)
+ kwargs['comment_id'] = item.id
+ return reverse('api-cover-comment-detail', kwargs=kwargs)
- return reverse('api-cover-comment-list', kwargs=kwargs)
+ def setUp(self):
+ super(TestCoverComments, self).setUp()
+ self.project = create_project()
+ self.user = create_maintainer(self.project)
+ self.cover = create_cover(project=self.project)
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.assertEqual(comment_obj.addressed, comment_json['addressed'])
self.assertIn(SAMPLE_CONTENT, comment_json['content'])
def test_list_empty(self):
@@ -47,10 +61,9 @@ class TestCoverComments(utils.APITestCase):
@utils.store_samples('cover-comment-list')
def test_list(self):
"""List cover letter comments."""
- cover = create_cover()
- comment = create_cover_comment(cover=cover)
+ comment = create_cover_comment(cover=self.cover)
- resp = self.client.get(self.api_url(cover))
+ resp = self.client.get(self.api_url(self.cover))
self.assertEqual(status.HTTP_200_OK, resp.status_code)
self.assertEqual(1, len(resp.data))
self.assertSerialized(comment, resp.data[0])
@@ -58,23 +71,21 @@ class TestCoverComments(utils.APITestCase):
def test_list_version_1_1(self):
"""List cover letter comments using API v1.1."""
- cover = create_cover()
- comment = create_cover_comment(cover=cover)
+ create_cover_comment(cover=self.cover)
- resp = self.client.get(self.api_url(cover, version='1.1'))
+ resp = self.client.get(self.api_url(self.cover, version='1.1'))
self.assertEqual(status.HTTP_200_OK, resp.status_code)
self.assertEqual(1, len(resp.data))
- self.assertSerialized(comment, resp.data[0])
self.assertNotIn('list_archive_url', resp.data[0])
+ self.assertNotIn('addressed', resp.data[0])
def test_list_version_1_0(self):
- """List cover letter comments using API v1.0."""
- cover = create_cover()
- create_cover_comment(cover=cover)
+ """List cover letter comments using API v1.0.
- # check we can't access comments using the old version of the API
+ Ensure we can't access cover comments using the old version of the API.
+ """
with self.assertRaises(NoReverseMatch):
- self.client.get(self.api_url(cover, version='1.0'))
+ self.client.get(self.api_url(self.cover, version='1.0'))
def test_list_invalid_cover(self):
"""Ensure we get a 404 for a non-existent cover letter."""
@@ -82,38 +93,193 @@ class TestCoverComments(utils.APITestCase):
reverse('api-cover-comment-list', kwargs={'cover_id': '99999'}))
self.assertEqual(status.HTTP_404_NOT_FOUND, resp.status_code)
+ @utils.store_samples('cover-comment-detail')
+ def test_detail(self):
+ """Show a cover letter comment."""
+ comment = create_cover_comment(cover=self.cover)
+
+ resp = self.client.get(self.api_url(self.cover, item=comment))
+ self.assertEqual(status.HTTP_200_OK, resp.status_code)
+ self.assertSerialized(comment, resp.data)
+
+ def test_detail_version_1_3(self):
+ """Show a cover letter comment using API v1.3."""
+ comment = create_cover_comment(cover=self.cover)
+
+ resp = self.client.get(
+ self.api_url(self.cover, version='1.3', item=comment))
+ self.assertEqual(status.HTTP_200_OK, resp.status_code)
+ self.assertSerialized(comment, resp.data)
+
+ def test_detail_version_1_2(self):
+ """Show a cover letter comment using API v1.2."""
+ comment = create_cover_comment(cover=self.cover)
+
+ with self.assertRaises(NoReverseMatch):
+ self.client.get(
+ self.api_url(self.cover, version='1.2', item=comment))
+
+ def test_detail_version_1_1(self):
+ """Show a cover letter comment using API v1.1."""
+ comment = create_cover_comment(cover=self.cover)
+
+ with self.assertRaises(NoReverseMatch):
+ self.client.get(
+ self.api_url(self.cover, version='1.1', item=comment))
+
+ def test_detail_version_1_0(self):
+ """Show a cover letter comment using API v1.0."""
+ comment = create_cover_comment(cover=self.cover)
+
+ with self.assertRaises(NoReverseMatch):
+ self.client.get(
+ self.api_url(self.cover, version='1.0', item=comment))
+
+ @utils.store_samples('cover-comment-detail-error-not-found')
+ def test_detail_invalid_cover(self):
+ """Ensure we handle non-existent cover letters."""
+ comment = create_cover_comment()
+ resp = self.client.get(
+ reverse('api-cover-comment-detail', kwargs={
+ 'cover_id': '99999',
+ 'comment_id': comment.id}
+ ),
+ )
+ self.assertEqual(status.HTTP_404_NOT_FOUND, resp.status_code)
+
+ def _test_update(self, person, **kwargs):
+ submitter = kwargs.get('submitter', person)
+ cover = kwargs.get('cover', self.cover)
+ comment = create_cover_comment(submitter=submitter, cover=cover)
+
+ if kwargs.get('authenticate', True):
+ self.client.force_authenticate(user=person.user)
+ return self.client.patch(
+ self.api_url(cover, item=comment),
+ {'addressed': kwargs.get('addressed', True)},
+ validate_request=kwargs.get('validate_request', True)
+ )
+
+ @utils.store_samples('cover-comment-detail-update-authorized')
+ def test_update_authorized(self):
+ """Update an existing cover letter comment as an authorized user.
+
+ To be authorized users must meet at least one of the following:
+ - project maintainer, cover letter submitter, or cover letter
+ comment submitter
+
+ Ensure updates can only be performed by authorized users.
+ """
+ # Update as maintainer
+ person = create_person(user=self.user)
+ resp = self._test_update(person=person)
+ self.assertEqual(1, CoverComment.objects.all().count())
+ self.assertEqual(status.HTTP_200_OK, resp.status_code)
+ self.assertTrue(resp.data['addressed'])
+
+ # Update as cover letter submitter
+ person = create_person(name='cover-submitter', user=create_user())
+ cover = create_cover(submitter=person)
+ resp = self._test_update(person=person, cover=cover)
+ self.assertEqual(2, CoverComment.objects.all().count())
+ self.assertEqual(status.HTTP_200_OK, resp.status_code)
+ self.assertTrue(resp.data['addressed'])
+
+ # Update as cover letter comment submitter
+ person = create_person(name='comment-submitter', user=create_user())
+ cover = create_cover()
+ resp = self._test_update(person=person, cover=cover, submitter=person)
+ self.assertEqual(3, CoverComment.objects.all().count())
+ self.assertEqual(status.HTTP_200_OK, resp.status_code)
+ self.assertTrue(resp.data['addressed'])
+
+ @utils.store_samples('cover-comment-detail-update-not-authorized')
+ def test_update_not_authorized(self):
+ """Update an existing cover letter comment when not signed in and
+ not authorized.
+
+ To be authorized users must meet at least one of the following:
+ - project maintainer, cover letter submitter, or cover letter
+ comment submitter
+
+ Ensure updates can only be performed by authorized users.
+ """
+ person = create_person(user=self.user)
+ resp = self._test_update(person=person, authenticate=False)
+ self.assertEqual(status.HTTP_403_FORBIDDEN, resp.status_code)
+
+ person = create_person() # normal user without edit permissions
+ resp = self._test_update(person=person) # signed-in
+ self.assertEqual(status.HTTP_403_FORBIDDEN, resp.status_code)
+
+ @utils.store_samples('cover-comment-detail-update-error-bad-request')
+ def test_update_invalid_addressed(self):
+ """Update an existing cover letter comment using invalid values.
+
+ Ensure we handle invalid cover letter comment addressed values.
+ """
+ person = create_person(name='cover-submitter', user=create_user())
+ cover = create_cover(submitter=person)
+ resp = self._test_update(person=person,
+ cover=cover,
+ addressed='not-valid',
+ validate_request=False)
+ self.assertEqual(status.HTTP_400_BAD_REQUEST, resp.status_code)
+ self.assertFalse(
+ getattr(CoverComment.objects.all().first(), 'addressed')
+ )
+
+ def test_create_delete(self):
+ """Ensure creates and deletes aren't allowed"""
+ comment = create_cover_comment(cover=self.cover)
+ self.user.is_superuser = True
+ self.user.save()
+ self.client.force_authenticate(user=self.user)
+
+ resp = self.client.post(self.api_url(self.cover, item=comment))
+ self.assertEqual(status.HTTP_405_METHOD_NOT_ALLOWED, resp.status_code)
+
+ resp = self.client.delete(self.api_url(self.cover, item=comment))
+ self.assertEqual(status.HTTP_405_METHOD_NOT_ALLOWED, resp.status_code)
+
@unittest.skipUnless(settings.ENABLE_REST_API, 'requires ENABLE_REST_API')
class TestPatchComments(utils.APITestCase):
@staticmethod
- def api_url(patch, version=None):
- kwargs = {}
+ def api_url(patch, version=None, item=None):
+ kwargs = {'patch_id': patch.id}
if version:
kwargs['version'] = version
- kwargs['patch_id'] = patch.id
+ if item is None:
+ return reverse('api-patch-comment-list', kwargs=kwargs)
+ kwargs['comment_id'] = item.id
+ return reverse('api-patch-comment-detail', kwargs=kwargs)
- return reverse('api-patch-comment-list', kwargs=kwargs)
+ def setUp(self):
+ super(TestPatchComments, self).setUp()
+ self.project = create_project()
+ self.user = create_maintainer(self.project)
+ self.patch = create_patch(project=self.project)
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.assertEqual(comment_obj.addressed, comment_json['addressed'])
self.assertIn(SAMPLE_CONTENT, comment_json['content'])
def test_list_empty(self):
"""List patch comments when none are present."""
- patch = create_patch()
- resp = self.client.get(self.api_url(patch))
+ resp = self.client.get(self.api_url(self.patch))
self.assertEqual(status.HTTP_200_OK, resp.status_code)
self.assertEqual(0, len(resp.data))
@utils.store_samples('patch-comment-list')
def test_list(self):
"""List patch comments."""
- patch = create_patch()
- comment = create_patch_comment(patch=patch)
+ comment = create_patch_comment(patch=self.patch)
- resp = self.client.get(self.api_url(patch))
+ resp = self.client.get(self.api_url(self.patch))
self.assertEqual(status.HTTP_200_OK, resp.status_code)
self.assertEqual(1, len(resp.data))
self.assertSerialized(comment, resp.data[0])
@@ -121,26 +287,180 @@ class TestPatchComments(utils.APITestCase):
def test_list_version_1_1(self):
"""List patch comments using API v1.1."""
- patch = create_patch()
- comment = create_patch_comment(patch=patch)
+ create_patch_comment(patch=self.patch)
- resp = self.client.get(self.api_url(patch, version='1.1'))
+ resp = self.client.get(self.api_url(self.patch, version='1.1'))
self.assertEqual(status.HTTP_200_OK, resp.status_code)
self.assertEqual(1, len(resp.data))
- self.assertSerialized(comment, resp.data[0])
self.assertNotIn('list_archive_url', resp.data[0])
+ self.assertNotIn('addressed', resp.data[0])
def test_list_version_1_0(self):
- """List patch comments using API v1.0."""
- patch = create_patch()
- create_patch_comment(patch=patch)
+ """List patch comments using API v1.0.
- # check we can't access comments using the old version of the API
+ Ensure we can't access comments using the old version of the API.
+ """
with self.assertRaises(NoReverseMatch):
- self.client.get(self.api_url(patch, version='1.0'))
+ self.client.get(self.api_url(self.patch, version='1.0'))
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={'patch_id': '99999'}))
self.assertEqual(status.HTTP_404_NOT_FOUND, resp.status_code)
+
+ @utils.store_samples('patch-comment-detail')
+ def test_detail(self):
+ """Show a patch comment."""
+ comment = create_patch_comment(patch=self.patch)
+
+ resp = self.client.get(self.api_url(self.patch, item=comment))
+ self.assertEqual(status.HTTP_200_OK, resp.status_code)
+ self.assertSerialized(comment, resp.data)
+
+ def test_detail_version_1_3(self):
+ """Show a patch comment using API v1.3."""
+ comment = create_patch_comment(patch=self.patch)
+
+ resp = self.client.get(
+ self.api_url(self.patch, version='1.3', item=comment))
+ self.assertEqual(status.HTTP_200_OK, resp.status_code)
+ self.assertSerialized(comment, resp.data)
+
+ def test_detail_version_1_2(self):
+ """Show a patch comment using API v1.2."""
+ comment = create_patch_comment(patch=self.patch)
+
+ with self.assertRaises(NoReverseMatch):
+ self.client.get(
+ self.api_url(self.patch, version='1.2', item=comment))
+
+ def test_detail_version_1_1(self):
+ """Show a patch comment using API v1.1."""
+ comment = create_patch_comment(patch=self.patch)
+
+ with self.assertRaises(NoReverseMatch):
+ self.client.get(
+ self.api_url(self.patch, version='1.1', item=comment))
+
+ def test_detail_version_1_0(self):
+ """Show a patch comment using API v1.0."""
+ comment = create_patch_comment(patch=self.patch)
+
+ with self.assertRaises(NoReverseMatch):
+ self.client.get(
+ self.api_url(self.patch, version='1.0', item=comment))
+
+ @utils.store_samples('patch-comment-detail-error-not-found')
+ def test_detail_invalid_patch(self):
+ """Ensure we handle non-existent patches."""
+ comment = create_patch_comment()
+ resp = self.client.get(
+ reverse('api-patch-comment-detail', kwargs={
+ 'patch_id': '99999',
+ 'comment_id': comment.id}
+ ),
+ )
+ self.assertEqual(status.HTTP_404_NOT_FOUND, resp.status_code)
+
+ def _test_update(self, person, **kwargs):
+ submitter = kwargs.get('submitter', person)
+ patch = kwargs.get('patch', self.patch)
+ comment = create_patch_comment(submitter=submitter, patch=patch)
+
+ if kwargs.get('authenticate', True):
+ self.client.force_authenticate(user=person.user)
+ return self.client.patch(
+ self.api_url(patch, item=comment),
+ {'addressed': kwargs.get('addressed', True)},
+ validate_request=kwargs.get('validate_request', True)
+ )
+
+ @utils.store_samples('patch-comment-detail-update-authorized')
+ def test_update_authorized(self):
+ """Update an existing patch comment as an authorized user.
+
+ To be authorized users must meet at least one of the following:
+ - project maintainer, patch submitter, patch delegate, or
+ patch comment submitter
+
+ Ensure updates can only be performed by authorized users.
+ """
+ # Update as maintainer
+ person = create_person(user=self.user)
+ resp = self._test_update(person=person)
+ self.assertEqual(1, PatchComment.objects.all().count())
+ self.assertEqual(status.HTTP_200_OK, resp.status_code)
+ self.assertTrue(resp.data['addressed'])
+
+ # Update as patch submitter
+ person = create_person(name='patch-submitter', user=create_user())
+ patch = create_patch(submitter=person)
+ resp = self._test_update(person=person, patch=patch)
+ self.assertEqual(2, PatchComment.objects.all().count())
+ self.assertEqual(status.HTTP_200_OK, resp.status_code)
+ self.assertTrue(resp.data['addressed'])
+
+ # Update as patch delegate
+ person = create_person(name='patch-delegate', user=create_user())
+ patch = create_patch(delegate=person.user)
+ resp = self._test_update(person=person, patch=patch)
+ self.assertEqual(3, PatchComment.objects.all().count())
+ self.assertEqual(status.HTTP_200_OK, resp.status_code)
+ self.assertTrue(resp.data['addressed'])
+
+ # Update as patch comment submitter
+ person = create_person(name='comment-submitter', user=create_user())
+ patch = create_patch()
+ resp = self._test_update(person=person, patch=patch, submitter=person)
+ self.assertEqual(4, PatchComment.objects.all().count())
+ self.assertEqual(status.HTTP_200_OK, resp.status_code)
+ self.assertTrue(resp.data['addressed'])
+
+ @utils.store_samples('patch-comment-detail-update-not-authorized')
+ def test_update_not_authorized(self):
+ """Update an existing patch comment when not signed in and not authorized.
+
+ To be authorized users must meet at least one of the following:
+ - project maintainer, patch submitter, patch delegate, or
+ patch comment submitter
+
+ Ensure updates can only be performed by authorized users.
+ """
+ person = create_person(user=self.user)
+ resp = self._test_update(person=person, authenticate=False)
+ self.assertEqual(status.HTTP_403_FORBIDDEN, resp.status_code)
+
+ person = create_person() # normal user without edit permissions
+ resp = self._test_update(person=person) # signed-in
+ self.assertEqual(status.HTTP_403_FORBIDDEN, resp.status_code)
+
+ @utils.store_samples('patch-comment-detail-update-error-bad-request')
+ def test_update_invalid_addressed(self):
+ """Update an existing patch comment using invalid values.
+
+ Ensure we handle invalid patch comment addressed values.
+ """
+ person = create_person(name='patch-submitter', user=create_user())
+ patch = create_patch(submitter=person)
+ resp = self._test_update(person=person,
+ patch=patch,
+ addressed='not-valid',
+ validate_request=False)
+ self.assertEqual(status.HTTP_400_BAD_REQUEST, resp.status_code)
+ self.assertFalse(
+ getattr(PatchComment.objects.all().first(), 'addressed')
+ )
+
+ def test_create_delete(self):
+ """Ensure creates and deletes aren't allowed"""
+ comment = create_patch_comment(patch=self.patch)
+ self.user.is_superuser = True
+ self.user.save()
+ self.client.force_authenticate(user=self.user)
+
+ resp = self.client.post(self.api_url(self.patch, item=comment))
+ self.assertEqual(status.HTTP_405_METHOD_NOT_ALLOWED, resp.status_code)
+
+ resp = self.client.delete(self.api_url(self.patch, item=comment))
+ self.assertEqual(status.HTTP_405_METHOD_NOT_ALLOWED, resp.status_code)
diff --git a/patchwork/urls.py b/patchwork/urls.py
index 0180e76d..b3f40cbf 100644
--- a/patchwork/urls.py
+++ b/patchwork/urls.py
@@ -343,12 +343,28 @@ 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',
+ ),
+ path(
+ 'covers/<cover_id>/comments/<comment_id>/',
+ api_comment_views.CoverCommentDetail.as_view(),
+ name='api-cover-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(
--
2.33.0.rc2.250.ged5fa647cd-goog
More information about the Patchwork
mailing list