[PATCH v3 00/10] patch-detail: add unaddressed/addressed status to patch comments

Raxel Gutierrez raxel at google.com
Fri Aug 13 15:31:17 AEST 2021


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 <raxel at 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 <raxel at 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



More information about the Patchwork mailing list