This series is a revision to the previous version of the patch series.
The series mainly addresses the review comments from the v2 series [1].
In doing so, its patches that deal with refactoring and don�t make
user-facing changes are placed at the beginning of this series to make
it easier to merge them as their complexity is low.
Since v2:
- Patch 1: adds refactoring of <pk> to <patch_id>
- Patch 2: clean up/refactoring of patch detail page
- Patch 3: adds patch-list.js file to modularize script code in template
- Patch 4: change patch meta info toggles from link elements to
buttons
- Patch 5: adds dependency patch that adds the JS Cookie library
to be able to add csrf tokens to requests made client-side using JS
- Patch 6: adds rest.js file to modularize the functionality of making
REST API requests in the client-side
- Patch 7: split from [v2 1/5] to only deal with adding addressed field
to the PatchComment model and changing it�s edit permissions
- Patch 8: adds /comments/<comment_id> endpoint and has its tests now
too. Also, based on review comments:
- fix OpenAPI definition of new `comments/<comment_id> endpoint to
not apply to older versions (< v1.3) of the REST API
- add motivation of adding addressed field to patch comments & reason
behind new endpoint to commit message of patch
- remove `addressed: False` from tests/utils.py creation of patch
comment
- Patch 9: combine styling from [v2 3/5] and functionality from [v2 4/5]
to add addressed status label and buttons with the ability to update
the state manually through the button as before
- Patch 10: remove preclude section and reword features/api sections
[1] https://lists.ozlabs.org/pipermail/patchwork/2021-July/007001.html
[2] https://lists.ozlabs.org/pipermail/patchwork/2021-July/006944.html
Raxel Gutierrez (10):
api: change <pk> parameter to <patch_id> for comments endpoint
patch-detail: clean up patch detail page template
patch-detail: refactor JS code into submission.js
patch-detail: change patch info toggles from links to buttons
static: add JS Cookie library to get csrftoken for client-side
requests
static: add rest.js to handle PATCH requests & respective responses
models: add addressed field and change edit permissions for comments
api: add patch comments detail endpoint and respective tests
patch-detail: add label and button for comment addressed status
docs: add release note for addressed/unaddressed comments
docs/api/schemas/generate-schemas.py | 4 +-
docs/api/schemas/latest/patchwork.yaml | 93 +-
docs/api/schemas/patchwork.j2 | 97 +
docs/api/schemas/v1.3/patchwork.yaml | 2704 +++++++++++++++++
htdocs/README.rst | 16 +
htdocs/css/style.css | 59 +-
htdocs/js/js.cookie.min.js | 2 +
htdocs/js/rest.js | 119 +
htdocs/js/submission.js | 57 +
patchwork/api/base.py | 24 +-
patchwork/api/check.py | 20 +-
patchwork/api/comment.py | 76 +-
patchwork/api/patch.py | 2 +-
.../migrations/0045_patchcomment_addressed.py | 18 +
patchwork/models.py | 5 +-
patchwork/templates/patchwork/submission.html | 123 +-
patchwork/tests/api/test_comment.py | 201 +-
patchwork/urls.py | 17 +-
patchwork/views/patch.py | 1 +
...essed-patch-comments-bfe71689b6f35a22.yaml | 16 +
templates/base.html | 6 +-
21 files changed, 3537 insertions(+), 123 deletions(-)
create mode 100644 docs/api/schemas/v1.3/patchwork.yaml
create mode 100644 htdocs/js/js.cookie.min.js
create mode 100644 htdocs/js/rest.js
create mode 100644 htdocs/js/submission.js
create mode 100644 patchwork/migrations/0045_patchcomment_addressed.py
create mode 100644
releasenotes/notes/comment-detail-endpoint-for-addressed-unaddressed-patch-comments-bfe71689b6f35a22.yaml
Range-diff against v2:
-: ------- > 1: 943864e api: change <pk> parameter to <patch_id> for
comments endpoint
-: ------- > 2: 1c8ffa5 patch-detail: clean up patch detail page template
-: ------- > 3: acfff46 patch-detail: refactor JS code into submission.js
-: ------- > 4: f50f345 patch-detail: change patch info toggles from links
to buttons
-: ------- > 5: 360b545 static: add JS Cookie library to get csrftoken for
client-side requests
-: ------- > 6: abee581 static: add rest.js to handle PATCH requests &
respective responses
-: ------- > 7: a9cf360 models: add addressed field and change edit
permissions for comments
1: 3ec6933 ! 8: 86e01dc api: add addressed field and detail endpoint for
patch comments
@@ Metadata
Author: Raxel Gutierrez <ra...@google.com>
## Commit message ##
- api: add addressed field and detail endpoint for patch comments
+ api: add patch comments detail endpoint and respective tests
- Add addressed boolean field to PatchComment to be able to distinguish
- between them in the patch-detail page. Change PatchComment to have same
- is_editable from the related patch with the addition of being editable
- by the user who submitted the comment.
-
- Add endpoint for patch comments at api/.../comments/<comment_id>.
+ Add new 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.
+ `addressed` field for individual patch comments with JavaScript on the
+ client side. In the process of these changes, clean up 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).
Add the OpenAPI definition of the new endpoint and upgrade API version
- to v1.3 to reflect the minor change. In order for tests to pass, clean
- up test_comment.py to reflect the change from the <pk> to <patch_id>
- parameter for the `api-patch-comment-list` endpoint.
+ to v1.3 to reflect the new endpoint as minor change for semantic
+ versioning.
+
+ Add tests for the new api/.../comments/<comment_id> endpoint that takes
+ GET, PATCH, and PUT requests. The tests cover retrieval and update
+ requests and handle calls from the various API versions. Also, they
+ handle permissions for update requests on the new `addressed` field and
+ invalid update values for the `addressed` field.
- Signed-off-by: Raxel Gutierrez <ra...@google.com>
+ Add `addressed` field to create_patch_comment helper in api tests
+ utils.py.
## docs/api/schemas/generate-schemas.py ##
@@ docs/api/schemas/generate-schemas.py: except ImportError:
@@ docs/api/schemas/latest/patchwork.yaml: info:
paths:
/api/:
get:
-@@ docs/api/schemas/latest/patchwork.yaml: paths:
- $ref: '#/components/schemas/Error'
- tags:
- - patches
-- /api/patches/{id}/comments/:
-+ /api/patches/{patch_id}/comments/:
- parameters:
- - in: path
-- name: id
-+ name: patch_id
- description: A unique integer value identifying the parent patch.
- required: true
- schema:
-- title: ID
-+ title: Patch ID
- type: integer
- get:
- description: List comments
@@ docs/api/schemas/latest/patchwork.yaml: paths:
$ref: '#/components/schemas/Error'
tags:
@@ docs/api/schemas/latest/patchwork.yaml: paths:
+ content:
+ application/json:
+ schema:
-+ $ref: '#/components/schemas/CommentDetail'
++ $ref: '#/components/schemas/Comment'
+ '404':
+ description: Not found
+ content:
@@ docs/api/schemas/latest/patchwork.yaml: paths:
+ patch:
+ description: Update a patch comment (partial).
+ operationId: patch_comments_partial_update
-+# security:
-+# - basicAuth: []
-+# - apiKeyAuth: []
+ requestBody:
+ $ref: '#/components/requestBodies/Comment'
+ responses:
@@ docs/api/schemas/latest/patchwork.yaml: paths:
+ content:
+ application/json:
+ schema:
-+ $ref: '#/components/schemas/CommentDetail'
-+ '400':
-+ description: Invalid Request
-+ content:
-+ application/json:
-+ schema:
-+ $ref: '#/components/schemas/ErrorCommentUpdate'
-+ '403':
-+ description: Forbidden
-+ content:
-+ application/json:
-+ schema:
-+ $ref: '#/components/schemas/Error'
-+ '404':
-+ description: Not found
-+ content:
-+ application/json:
-+ schema:
-+ $ref: '#/components/schemas/Error'
-+ tags:
-+ - comments
-+ put:
-+ description: Update a patch comment.
-+ operationId: patch_comments_update
-+# security:
-+# - basicAuth: []
-+# - apiKeyAuth: []
-+ requestBody:
-+ $ref: '#/components/requestBodies/Comment'
-+ responses:
-+ '200':
-+ description: ''
-+ content:
-+ application/json:
-+ schema:
-+ $ref: '#/components/schemas/CommentDetail'
++ $ref: '#/components/schemas/Comment'
+ '400':
+ description: Invalid Request
+ content:
@@ docs/api/schemas/latest/patchwork.yaml: components:
additionalProperties:
type: string
readOnly: true
-+ CommentDetail:
-+ allOf:
-+ - $ref: '#/components/schemas/Comment'
-+ - properties:
-+ patch:
-+ $ref: '#/components/schemas/PatchEmbedded'
-+ addressed:
-+ title: Addressed
-+ type: boolean
++ addressed:
++ title: Addressed
++ type: boolean
+ CommentUpdate:
+ type: object
+ properties:
@@ docs/api/schemas/latest/patchwork.yaml: components:
+ type: array
+ items:
+ type: string
-+ readOnly: true
ErrorPatchUpdate:
type: object
properties:
## docs/api/schemas/patchwork.j2 ##
-@@ docs/api/schemas/patchwork.j2: paths:
- {% endif %}
- tags:
- - patches
-- /api/{{ version_url }}patches/{id}/comments/:
-+ /api/{{ version_url }}patches/{patch_id}/comments/:
- parameters:
- - in: path
-- name: id
-+ name: patch_id
- description: A unique integer value identifying the parent patch.
- required: true
- schema:
-- title: ID
-+ title: Patch ID
- type: integer
- get:
- description: List comments
@@ docs/api/schemas/patchwork.j2: paths:
$ref: '#/components/schemas/Error'
tags:
- comments
++{% if version >= (1, 3) %}
+ /api/{{ version_url }}patches/{patch_id}/comments/{comment_id}/:
+ parameters:
+ - in: path
@@ docs/api/schemas/patchwork.j2: paths:
+ schema:
+ title: Comment ID
+ type: integer
-+{% if version >= (1, 3) %}
+ get:
+ description: Show a patch comment.
+ operationId: patch_comments_read
@@ docs/api/schemas/patchwork.j2: paths:
+ content:
+ application/json:
+ schema:
-+ $ref: '#/components/schemas/CommentDetail'
++ $ref: '#/components/schemas/Comment'
+ '404':
+ description: Not found
+ content:
@@ docs/api/schemas/patchwork.j2: paths:
+ patch:
+ description: Update a patch comment (partial).
+ operationId: patch_comments_partial_update
-+# security:
-+# - basicAuth: []
-+# - apiKeyAuth: []
-+ requestBody:
-+ $ref: '#/components/requestBodies/Comment'
-+ responses:
-+ '200':
-+ description: ''
-+ content:
-+ application/json:
-+ schema:
-+ $ref: '#/components/schemas/CommentDetail'
-+ '400':
-+ description: Invalid Request
-+ content:
-+ application/json:
-+ schema:
-+ $ref: '#/components/schemas/ErrorCommentUpdate'
-+ '403':
-+ description: Forbidden
-+ content:
-+ application/json:
-+ schema:
-+ $ref: '#/components/schemas/Error'
-+ '404':
-+ description: Not found
-+ content:
-+ application/json:
-+ schema:
-+ $ref: '#/components/schemas/Error'
-+ tags:
-+ - comments
-+ put:
-+ description: Update a patch comment.
-+ operationId: patch_comments_update
-+# security:
-+# - basicAuth: []
-+# - apiKeyAuth: []
+ requestBody:
+ $ref: '#/components/requestBodies/Comment'
+ responses:
@@ docs/api/schemas/patchwork.j2: paths:
+ content:
+ application/json:
+ schema:
-+ $ref: '#/components/schemas/CommentDetail'
++ $ref: '#/components/schemas/Comment'
+ '400':
+ description: Invalid Request
+ content:
@@ docs/api/schemas/patchwork.j2: 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:
@@ docs/api/schemas/patchwork.j2: components:
additionalProperties:
type: string
readOnly: true
-+ CommentDetail:
-+ allOf:
-+ - $ref: '#/components/schemas/Comment'
+{% if version >= (1, 3) %}
-+ - properties:
-+ patch:
-+ $ref: '#/components/schemas/PatchEmbedded'
-+ addressed:
-+ title: Addressed
-+ type: boolean
-+{% endif %}
++ addressed:
++ title: Addressed
++ type: boolean
+ CommentUpdate:
+ type: object
-+{% if version >= (1, 3) %}
+ properties:
+ addressed:
+ title: Addressed
@@ docs/api/schemas/patchwork.j2: components:
items:
type: string
readOnly: true
++{% if version >= (1, 3) %}
+ ErrorCommentUpdate:
+ type: object
-+{% if version >= (1, 3) %}
+ properties:
+ addressed:
+ title: Addressed
+ type: array
+ items:
+ type: string
-+ readOnly: true
+{% endif %}
ErrorPatchUpdate:
type: object
properties:
- ## docs/api/schemas/v1.0/patchwork.yaml ##
-@@ docs/api/schemas/v1.0/patchwork.yaml: paths:
- $ref: '#/components/schemas/Error'
- tags:
- - patches
-- /api/1.0/patches/{id}/comments/:
-+ /api/1.0/patches/{patch_id}/comments/:
- parameters:
- - in: path
-- name: id
-+ name: patch_id
- description: A unique integer value identifying the parent patch.
- required: true
- schema:
-- title: ID
-+ title: Patch ID
- type: integer
- get:
- description: List comments
-@@ docs/api/schemas/v1.0/patchwork.yaml: paths:
- $ref: '#/components/schemas/Error'
- tags:
- - comments
-+ /api/1.0/patches/{patch_id}/comments/{comment_id}/:
-+ parameters:
-+ - in: path
-+ name: patch_id
-+ description: A unique integer value identifying the parent patch.
-+ required: true
-+ schema:
-+ title: Patch ID
-+ type: integer
-+ - in: path
-+ name: comment_id
-+ description: A unique integer value identifying this comment.
-+ required: true
-+ schema:
-+ title: Comment ID
-+ type: integer
- /api/1.0/patches/{patch_id}/checks/:
- parameters:
- - in: path
-@@ docs/api/schemas/v1.0/patchwork.yaml: components:
- application/x-www-form-urlencoded:
- schema:
- $ref: '#/components/schemas/CheckCreate'
-+ Comment:
-+ required: true
-+ content:
-+ application/json:
-+ schema:
-+ $ref: '#/components/schemas/CommentUpdate'
- Patch:
- required: true
- content:
-@@ docs/api/schemas/v1.0/patchwork.yaml: components:
- additionalProperties:
- type: string
- readOnly: true
-+ CommentDetail:
-+ allOf:
-+ - $ref: '#/components/schemas/Comment'
-+ CommentUpdate:
-+ type: object
- CoverList:
- type: object
- properties:
-@@ docs/api/schemas/v1.0/patchwork.yaml: components:
- items:
- type: string
- readOnly: true
-+ ErrorCommentUpdate:
-+ type: object
- ErrorPatchUpdate:
- type: object
- properties:
-
- ## docs/api/schemas/v1.1/patchwork.yaml ##
-@@ docs/api/schemas/v1.1/patchwork.yaml: paths:
- $ref: '#/components/schemas/Error'
- tags:
- - patches
-- /api/1.1/patches/{id}/comments/:
-+ /api/1.1/patches/{patch_id}/comments/:
- parameters:
- - in: path
-- name: id
-+ name: patch_id
- description: A unique integer value identifying the parent patch.
- required: true
- schema:
-- title: ID
-+ title: Patch ID
- type: integer
- get:
- description: List comments
-@@ docs/api/schemas/v1.1/patchwork.yaml: paths:
- $ref: '#/components/schemas/Error'
- tags:
- - comments
-+ /api/1.1/patches/{patch_id}/comments/{comment_id}/:
-+ parameters:
-+ - in: path
-+ name: patch_id
-+ description: A unique integer value identifying the parent patch.
-+ required: true
-+ schema:
-+ title: Patch ID
-+ type: integer
-+ - in: path
-+ name: comment_id
-+ description: A unique integer value identifying this comment.
-+ required: true
-+ schema:
-+ title: Comment ID
-+ type: integer
- /api/1.1/patches/{patch_id}/checks/:
- parameters:
- - in: path
-@@ docs/api/schemas/v1.1/patchwork.yaml: components:
- application/x-www-form-urlencoded:
- schema:
- $ref: '#/components/schemas/CheckCreate'
-+ Comment:
-+ required: true
-+ content:
-+ application/json:
-+ schema:
-+ $ref: '#/components/schemas/CommentUpdate'
- Patch:
- required: true
- content:
-@@ docs/api/schemas/v1.1/patchwork.yaml: components:
- additionalProperties:
- type: string
- readOnly: true
-+ CommentDetail:
-+ allOf:
-+ - $ref: '#/components/schemas/Comment'
-+ CommentUpdate:
-+ type: object
- CoverList:
- type: object
- properties:
-@@ docs/api/schemas/v1.1/patchwork.yaml: components:
- items:
- type: string
- readOnly: true
-+ ErrorCommentUpdate:
-+ type: object
- ErrorPatchUpdate:
- type: object
- properties:
-
- ## docs/api/schemas/v1.2/patchwork.yaml ##
-@@ docs/api/schemas/v1.2/patchwork.yaml: paths:
- $ref: '#/components/schemas/Error'
- tags:
- - patches
-- /api/1.2/patches/{id}/comments/:
-+ /api/1.2/patches/{patch_id}/comments/:
- parameters:
- - in: path
-- name: id
-+ name: patch_id
- description: A unique integer value identifying the parent patch.
- required: true
- schema:
-- title: ID
-+ title: Patch ID
- type: integer
- get:
- description: List comments
-@@ docs/api/schemas/v1.2/patchwork.yaml: paths:
- $ref: '#/components/schemas/Error'
- tags:
- - comments
-+ /api/1.2/patches/{patch_id}/comments/{comment_id}/:
-+ parameters:
-+ - in: path
-+ name: patch_id
-+ description: A unique integer value identifying the parent patch.
-+ required: true
-+ schema:
-+ title: Patch ID
-+ type: integer
-+ - in: path
-+ name: comment_id
-+ description: A unique integer value identifying this comment.
-+ required: true
-+ schema:
-+ title: Comment ID
-+ type: integer
- /api/1.2/patches/{patch_id}/checks/:
- parameters:
- - in: path
-@@ docs/api/schemas/v1.2/patchwork.yaml: components:
- application/x-www-form-urlencoded:
- schema:
- $ref: '#/components/schemas/CheckCreate'
-+ Comment:
-+ required: true
-+ content:
-+ application/json:
-+ schema:
-+ $ref: '#/components/schemas/CommentUpdate'
- Patch:
- required: true
- content:
-@@ docs/api/schemas/v1.2/patchwork.yaml: components:
- additionalProperties:
- type: string
- readOnly: true
-+ CommentDetail:
-+ allOf:
-+ - $ref: '#/components/schemas/Comment'
-+ CommentUpdate:
-+ type: object
- CoverList:
- type: object
- properties:
-@@ docs/api/schemas/v1.2/patchwork.yaml: components:
- items:
- type: string
- readOnly: true
-+ ErrorCommentUpdate:
-+ type: object
- ErrorPatchUpdate:
- type: object
- properties:
-
## docs/api/schemas/v1.3/patchwork.yaml (new) ##
@@
+# DO NOT EDIT THIS FILE. It is generated from a template. Changes should
be
@@ docs/api/schemas/v1.3/patchwork.yaml (new)
+ $ref: '#/components/schemas/Error'
+ tags:
+ - patches
-+ /api/1.3/patches/{patch_id}/comments/:
++ /api/1.3/patches/{id}/comments/:
+ parameters:
+ - in: path
-+ name: patch_id
++ name: id
+ description: A unique integer value identifying the parent patch.
+ required: true
+ schema:
-+ title: Patch ID
++ title: ID
+ type: integer
+ get:
+ description: List comments
@@ docs/api/schemas/v1.3/patchwork.yaml (new)
+ content:
+ application/json:
+ schema:
-+ $ref: '#/components/schemas/CommentDetail'
++ $ref: '#/components/schemas/Comment'
+ '404':
+ description: Not found
+ content:
@@ docs/api/schemas/v1.3/patchwork.yaml (new)
+ patch:
+ description: Update a patch comment (partial).
+ operationId: patch_comments_partial_update
-+# security:
-+# - basicAuth: []
-+# - apiKeyAuth: []
+ requestBody:
+ $ref: '#/components/requestBodies/Comment'
+ responses:
@@ docs/api/schemas/v1.3/patchwork.yaml (new)
+ content:
+ application/json:
+ schema:
-+ $ref: '#/components/schemas/CommentDetail'
-+ '400':
-+ description: Invalid Request
-+ content:
-+ application/json:
-+ schema:
-+ $ref: '#/components/schemas/ErrorCommentUpdate'
-+ '403':
-+ description: Forbidden
-+ content:
-+ application/json:
-+ schema:
-+ $ref: '#/components/schemas/Error'
-+ '404':
-+ description: Not found
-+ content:
-+ application/json:
-+ schema:
-+ $ref: '#/components/schemas/Error'
-+ tags:
-+ - comments
-+ put:
-+ description: Update a patch comment.
-+ operationId: patch_comments_update
-+# security:
-+# - basicAuth: []
-+# - apiKeyAuth: []
-+ requestBody:
-+ $ref: '#/components/requestBodies/Comment'
-+ responses:
-+ '200':
-+ description: ''
-+ content:
-+ application/json:
-+ schema:
-+ $ref: '#/components/schemas/CommentDetail'
++ $ref: '#/components/schemas/Comment'
+ '400':
+ description: Invalid Request
+ content:
@@ docs/api/schemas/v1.3/patchwork.yaml (new)
+ additionalProperties:
+ type: string
+ readOnly: true
-+ CommentDetail:
-+ allOf:
-+ - $ref: '#/components/schemas/Comment'
-+ - properties:
-+ patch:
-+ $ref: '#/components/schemas/PatchEmbedded'
-+ addressed:
-+ title: Addressed
-+ type: boolean
++ addressed:
++ title: Addressed
++ type: boolean
+ CommentUpdate:
+ type: object
+ properties:
@@ docs/api/schemas/v1.3/patchwork.yaml (new)
+ type: array
+ items:
+ type: string
-+ readOnly: true
+ ErrorPatchUpdate:
+ type: object
+ properties:
@@ patchwork/api/check.py
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
@@ patchwork/api/comment.py: class
CoverCommentListSerializer(BaseCommentListSerial
- read_only_fields = fields
+ fields = BaseCommentListSerializer.Meta.fields + (
+ 'patch', 'addressed')
-+ read_only_fields = fields[:-1] # able to write to addressed field
++ read_only_fields = BaseCommentListSerializer.Meta.fields +
('patch', )
+ versioned_fields = {
+ '1.3': ('patch', 'addressed'),
+ }
@@ patchwork/api/comment.py: class CoverCommentList(ListAPIView):
search_fields = ('subject',)
ordering_fields = ('id', 'subject', 'date', 'submitter')
ordering = 'id'
-- lookup_url_kwarg = 'pk'
--
+ lookup_url_kwarg = 'patch_id'
+
- def get_queryset(self):
-- if not Patch.objects.filter(pk=self.kwargs['pk']).exists():
+- if not Patch.objects.filter(id=self.kwargs['patch_id']).exists():
- raise Http404
--
+
- return PatchComment.objects.filter(
-- patch=self.kwargs['pk']
+- patch=self.kwargs['patch_id']
- ).select_related('submitter')
-+ lookup_url_kwarg = 'patch_id'
-+
-+
+class PatchCommentDetail(PatchCommentMixin, MultipleFieldLookupMixin,
+ RetrieveUpdateAPIView):
+ """
@@ patchwork/api/comment.py: class CoverCommentList(ListAPIView):
+ lookup_url_kwargs = ('patch_id', 'comment_id')
+ lookup_fields = ('patch_id', 'id')
- ## patchwork/api/patch.py ##
-@@ patchwork/api/patch.py: class
PatchListSerializer(BaseHyperlinkedModelSerializer):
+ ## patchwork/tests/api/test_comment.py ##
+@@ patchwork/tests/api/test_comment.py: from django.conf import settings
+ from django.urls import NoReverseMatch
+ from django.urls import reverse
- def get_comments(self, patch):
- return self.context.get('request').build_absolute_uri(
-- reverse('api-patch-comment-list', kwargs={'pk': patch.id}))
-+ reverse('api-patch-comment-list', kwargs={'patch_id':
patch.id}))
++from patchwork.models import PatchComment
+ 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
- def get_check(self, instance):
- return instance.combined_check_state
-
- ## patchwork/migrations/0045_patchcomment_addressed.py (new) ##
-@@
-+# 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),
-+ ),
-+ ]
-
- ## patchwork/models.py ##
-@@ patchwork/models.py: class PatchComment(EmailMixin, models.Model):
- related_query_name='comment',
- on_delete=models.CASCADE,
- )
-+ addressed = models.BooleanField(default=False)
+ if settings.ENABLE_REST_API:
+@@ patchwork/tests/api/test_comment.py: class
TestCoverComments(utils.APITestCase):
+ @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)
- @property
- def list_archive_url(self):
-@@ patchwork/models.py: class PatchComment(EmailMixin, models.Model):
- self.patch.refresh_tag_counts()
+- 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 is_editable(self, user):
-- return False
-+ if user == self.submitter.user:
-+ return True
-+ return self.patch.is_editable(user)
+ 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'])
- class Meta:
- ordering = ['date']
-
- ## patchwork/tests/api/test_comment.py ##
-@@ patchwork/tests/api/test_comment.py: class
TestPatchComments(utils.APITestCase):
- kwargs = {}
- if version:
- kwargs['version'] = version
-- kwargs['pk'] = patch.id
-+ kwargs['patch_id'] = patch.id
+ 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))
- return reverse('api-patch-comment-list', kwargs=kwargs)
+ @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])
@@ patchwork/tests/api/test_comment.py: 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)
++ comment = 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])
+
+ 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={'pk': '99999'}))
-+ reverse('api-patch-comment-list', kwargs={'patch_id':
'99999'}))
+ 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)
## patchwork/urls.py ##
@@ patchwork/urls.py: 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',
- ),
-@@ patchwork/urls.py: if settings.ENABLE_REST_API:
),
]
2: 51859c1 < -: ------- tests: add tests for patch comments detail endpoint
3: 8f094e4 < -: ------- patch-detail: add label and button for comment
addressed status
4: fc477ed < -: ------- patch-detail: add functionality for comment status
updates
5: 20c2bd6 < -: ------- docs: add release note for addressed/unaddressed
comments
-: ------- > 9: a4091be patch-detail: add label and button for comment
addressed status
-: ------- > 10: fdb7f20 docs: add release note for addressed/unaddressed
comments
--
2.33.0.rc1.237.g0d66db33f3-goog
_______________________________________________
Patchwork mailing list
Patchwork@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/patchwork
