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