[PATCH 2/2] REST: Allow creating, updating, deleting of bundles

Daniel Axtens dja at axtens.net
Wed Oct 2 11:50:41 AEST 2019


Stephen Finucane <stephen at that.guru> writes:

> Allow users to create a new bundle, change the name, public flag and
> patches of an existing bundle, and delete an existing bundle.
>
> Some small nits with existing tests are resolved.
>
> Signed-off-by: Stephen Finucane <stephen at that.guru>
> ---
>  docs/api/schemas/latest/patchwork.yaml        | 170 +++++++++++++++-
>  docs/api/schemas/patchwork.j2                 | 181 +++++++++++++++++-
>  docs/api/schemas/v1.0/patchwork.yaml          |   5 +-
>  docs/api/schemas/v1.1/patchwork.yaml          |   5 +-
>  docs/api/schemas/v1.2/patchwork.yaml          | 170 +++++++++++++++-
>  patchwork/api/bundle.py                       |  84 +++++++-
>  patchwork/models.py                           |  11 ++
>  patchwork/tests/api/test_bundle.py            | 118 +++++++++++-
>  patchwork/tests/api/utils.py                  |  16 +-
>  ...pdate-bundle-via-api-2946d8c4e730d545.yaml |   4 +
>  10 files changed, 737 insertions(+), 27 deletions(-)
>  create mode 100644 releasenotes/notes/update-bundle-via-api-2946d8c4e730d545.yaml
>
> diff --git a/docs/api/schemas/latest/patchwork.yaml b/docs/api/schemas/latest/patchwork.yaml
> index 45a61180..e6c6bb4a 100644
> --- a/docs/api/schemas/latest/patchwork.yaml
> +++ b/docs/api/schemas/latest/patchwork.yaml
> @@ -1,5 +1,6 @@
>  # DO NOT EDIT THIS FILE. It is generated from a template. Changes should be
> -# proposed against the template.
> +# proposed against the template and updated files generated using the
> +# 'generate_schema.py' tool
>  ---
>  openapi: '3.0.0'
>  info:
> @@ -72,6 +73,35 @@ paths:
>                    $ref: '#/components/schemas/Bundle'
>        tags:
>          - bundles
> +    post:
> +      description: Create a bundle.
> +      operationId: bundles_create
> +      security:
> +        - basicAuth: []
> +        - apiKeyAuth: []
> +      requestBody:
> +        $ref: '#/components/requestBodies/Bundle'
> +      responses:
> +        '201':
> +          description: ''
> +          content:
> +            application/json:
> +              schema:
> +                $ref: '#/components/schemas/Bundle'
> +        '400':
> +          description: Invalid Request
> +          content:
> +            application/json:
> +              schema:
> +                $ref: '#/components/schemas/ErrorBundleCreateUpdate'
> +        '403':
> +          description: Forbidden
> +          content:
> +            application/json:
> +              schema:
> +                $ref: '#/components/schemas/Error'
> +      tags:
> +        - bundles
>    /api/bundles/{id}/:
>      get:
>        description: Show a bundle.
> @@ -99,6 +129,92 @@ paths:
>                  $ref: '#/components/schemas/Error'
>        tags:
>          - bundles
> +    patch:
> +      description: Update a bundle (partial).
> +      operationId: bundles_partial_update
> +      security:
> +        - basicAuth: []
> +        - apiKeyAuth: []
> +      parameters:
> +        - in: path
> +          name: id
> +          description: A unique integer value identifying this bundle.
> +          required: true
> +          schema:
> +            title: ID
> +            type: integer
> +      requestBody:
> +        $ref: '#/components/requestBodies/Bundle'
> +      responses:
> +        '200':
> +          description: ''
> +          content:
> +            application/json:
> +              schema:
> +                $ref: '#/components/schemas/Bundle'
> +        '400':
> +          description: Bad request
> +          content:
> +            application/json:
> +              schema:
> +                $ref: '#/components/schemas/ErrorBundleCreateUpdate'
> +        '403':
> +          description: Forbidden
> +          content:
> +            application/json:
> +              schema:
> +                $ref: '#/components/schemas/Error'
> +        '404':
> +          description: Not found
> +          content:
> +            application/json:
> +              schema:
> +                $ref: '#/components/schemas/Error'
> +      tags:
> +        - bundles
> +    put:
> +      description: Update a bundle.
> +      operationId: bundles_update
> +      security:
> +        - basicAuth: []
> +        - apiKeyAuth: []
> +      parameters:
> +        - in: path
> +          name: id
> +          description: A unique integer value identifying this bundle.
> +          required: true
> +          schema:
> +            title: ID
> +            type: integer
> +      requestBody:
> +        $ref: '#/components/requestBodies/Bundle'
> +      responses:
> +        '200':
> +          description: ''
> +          content:
> +            application/json:
> +              schema:
> +                $ref: '#/components/schemas/Bundle'
> +        '400':
> +          description: Bad request
> +          content:
> +            application/json:
> +              schema:
> +                $ref: '#/components/schemas/ErrorBundleCreateUpdate'
> +        '403':
> +          description: Forbidden
> +          content:
> +            application/json:
> +              schema:
> +                $ref: '#/components/schemas/Error'
> +        '404':
> +          description: Not found
> +          content:
> +            application/json:
> +              schema:
> +                $ref: '#/components/schemas/Error'
> +      tags:
> +        - bundles
>    /api/covers/:
>      get:
>        description: List cover letters.
> @@ -1131,6 +1247,18 @@ components:
>        schema:
>          type: string
>    requestBodies:
> +    Bundle:
> +      required: true
> +      content:
> +        application/json:
> +          schema:
> +            $ref: '#/components/schemas/BundleCreateUpdate'
> +        multipart/form-data:
> +          schema:
> +            $ref: '#/components/schemas/BundleCreateUpdate'
> +        application/x-www-form-urlencoded:
> +          schema:
> +            $ref: '#/components/schemas/BundleCreateUpdate'
>      Check:
>        required: true
>        content:
> @@ -1251,10 +1379,10 @@ components:
>            allOf:
>              - $ref: '#/components/schemas/UserEmbedded'
>          patches:
> +          title: Patches
>            type: array
>            items:
>              $ref: '#/components/schemas/PatchEmbedded'
> -          readOnly: true
>            uniqueItems: true
>          public:
>            title: Public
> @@ -1264,6 +1392,25 @@ components:
>            type: string
>            format: uri
>            readOnly: true
> +    BundleCreateUpdate:
> +      type: object
> +      required:
> +        - name
> +      properties:
> +        name:
> +          title: Name
> +          type: string
> +          minLength: 1
> +          maxLength: 50
> +        patches:
> +          title: Patches
> +          type: array
> +          items:
> +            type: integer
> +          uniqueItems: true
> +        public:
> +          title: Public
> +          type: boolean
>      Check:
>        type: object
>        properties:
> @@ -1961,6 +2108,7 @@ components:
>          cover_letter:
>            $ref: '#/components/schemas/CoverLetterEmbedded'
>          patches:
> +          title: Patches
>            type: array
>            items:
>              $ref: '#/components/schemas/PatchEmbedded'
> @@ -2307,6 +2455,24 @@ components:
>            title: Detail
>            type: string
>            readOnly: true
> +    ErrorBundleCreateUpdate:
> +      type: object
> +      properties:
> +        name:
> +          title: Name
> +          type: string
> +          minLength: 1
> +          maxLength: 50
> +        patches:
> +          title: Patches
> +          type: array
> +          items:
> +            $ref: '#/components/schemas/PatchEmbedded'
> +          readOnly: true
> +          uniqueItems: true
> +        public:
> +          title: Public
> +          type: boolean
>      ErrorCheckCreate:
>        type: object
>        properties:
> diff --git a/docs/api/schemas/patchwork.j2 b/docs/api/schemas/patchwork.j2
> index 843981f8..5f7510da 100644
> --- a/docs/api/schemas/patchwork.j2
> +++ b/docs/api/schemas/patchwork.j2
> @@ -1,6 +1,7 @@
>  {# You can obviously ignore the below when editing this template #}
>  # DO NOT EDIT THIS FILE. It is generated from a template. Changes should be
> -# proposed against the template.
> +# proposed against the template and updated files generated using the
> +# 'generate_schema.py' tool
>  ---
>  openapi: '3.0.0'
>  info:
> @@ -73,6 +74,37 @@ paths:
>                    $ref: '#/components/schemas/Bundle'
>        tags:
>          - bundles
> +{% if version >= (1, 2) %}
> +    post:
> +      description: Create a bundle.
> +      operationId: bundles_create
> +      security:
> +        - basicAuth: []
> +        - apiKeyAuth: []
> +      requestBody:
> +        $ref: '#/components/requestBodies/Bundle'
> +      responses:
> +        '201':
> +          description: ''
> +          content:
> +            application/json:
> +              schema:
> +                $ref: '#/components/schemas/Bundle'
> +        '400':
> +          description: Invalid Request
> +          content:
> +            application/json:
> +              schema:
> +                $ref: '#/components/schemas/ErrorBundleCreateUpdate'
> +        '403':
> +          description: Forbidden
> +          content:
> +            application/json:
> +              schema:
> +                $ref: '#/components/schemas/Error'
> +      tags:
> +        - bundles
> +{% endif %}
>    /api/{{ version_url }}bundles/{id}/:
>      get:
>        description: Show a bundle.
> @@ -100,6 +132,94 @@ paths:
>                  $ref: '#/components/schemas/Error'
>        tags:
>          - bundles
> +{% if version >= (1, 2) %}
> +    patch:
> +      description: Update a bundle (partial).
> +      operationId: bundles_partial_update
> +      security:
> +        - basicAuth: []
> +        - apiKeyAuth: []
> +      parameters:
> +        - in: path
> +          name: id
> +          description: A unique integer value identifying this bundle.
> +          required: true
> +          schema:
> +            title: ID
> +            type: integer
> +      requestBody:
> +        $ref: '#/components/requestBodies/Bundle'
> +      responses:
> +        '200':
> +          description: ''
> +          content:
> +            application/json:
> +              schema:
> +                $ref: '#/components/schemas/Bundle'
> +        '400':
> +          description: Bad request
> +          content:
> +            application/json:
> +              schema:
> +                $ref: '#/components/schemas/ErrorBundleCreateUpdate'
> +        '403':
> +          description: Forbidden
> +          content:
> +            application/json:
> +              schema:
> +                $ref: '#/components/schemas/Error'
> +        '404':
> +          description: Not found
> +          content:
> +            application/json:
> +              schema:
> +                $ref: '#/components/schemas/Error'
> +      tags:
> +        - bundles
> +    put:
> +      description: Update a bundle.
> +      operationId: bundles_update
> +      security:
> +        - basicAuth: []
> +        - apiKeyAuth: []
> +      parameters:
> +        - in: path
> +          name: id
> +          description: A unique integer value identifying this bundle.
> +          required: true
> +          schema:
> +            title: ID
> +            type: integer
> +      requestBody:
> +        $ref: '#/components/requestBodies/Bundle'
> +      responses:
> +        '200':
> +          description: ''
> +          content:
> +            application/json:
> +              schema:
> +                $ref: '#/components/schemas/Bundle'
> +        '400':
> +          description: Bad request
> +          content:
> +            application/json:
> +              schema:
> +                $ref: '#/components/schemas/ErrorBundleCreateUpdate'
> +        '403':
> +          description: Forbidden
> +          content:
> +            application/json:
> +              schema:
> +                $ref: '#/components/schemas/Error'
> +        '404':
> +          description: Not found
> +          content:
> +            application/json:
> +              schema:
> +                $ref: '#/components/schemas/Error'
> +      tags:
> +        - bundles
> +{% endif %}
>    /api/{{ version_url }}covers/:
>      get:
>        description: List cover letters.
> @@ -1132,6 +1252,20 @@ components:
>        schema:
>          type: string
>    requestBodies:
> +{% if version >= (1, 2) %}
> +    Bundle:
> +      required: true
> +      content:
> +        application/json:
> +          schema:
> +            $ref: '#/components/schemas/BundleCreateUpdate'
> +        multipart/form-data:
> +          schema:
> +            $ref: '#/components/schemas/BundleCreateUpdate'
> +        application/x-www-form-urlencoded:
> +          schema:
> +            $ref: '#/components/schemas/BundleCreateUpdate'
> +{% endif %}
>      Check:
>        required: true
>        content:
> @@ -1254,10 +1388,13 @@ components:
>            allOf:
>              - $ref: '#/components/schemas/UserEmbedded'
>          patches:
> +          title: Patches
>            type: array
>            items:
>              $ref: '#/components/schemas/PatchEmbedded'
> +{% if version < (1, 2) %}
>            readOnly: true
> +{% endif %}
>            uniqueItems: true
>          public:
>            title: Public
> @@ -1267,6 +1404,27 @@ components:
>            type: string
>            format: uri
>            readOnly: true
> +{% if version >= (1, 2) %}
> +    BundleCreateUpdate:
> +      type: object
> +      required:
> +        - name
> +      properties:
> +        name:
> +          title: Name
> +          type: string
> +          minLength: 1
> +          maxLength: 50
> +        patches:
> +          title: Patches
> +          type: array
> +          items:
> +            type: integer
> +          uniqueItems: true
> +        public:
> +          title: Public
> +          type: boolean
> +{% endif %}
>      Check:
>        type: object
>        properties:
> @@ -1988,6 +2146,7 @@ components:
>          cover_letter:
>            $ref: '#/components/schemas/CoverLetterEmbedded'
>          patches:
> +          title: Patches
>            type: array
>            items:
>              $ref: '#/components/schemas/PatchEmbedded'
> @@ -2346,6 +2505,26 @@ components:
>            title: Detail
>            type: string
>            readOnly: true
> +{% if version >= (1, 2) %}
> +    ErrorBundleCreateUpdate:
> +      type: object
> +      properties:
> +        name:
> +          title: Name
> +          type: string
> +          minLength: 1
> +          maxLength: 50
> +        patches:
> +          title: Patches
> +          type: array
> +          items:
> +            $ref: '#/components/schemas/PatchEmbedded'
> +          readOnly: true
> +          uniqueItems: true
> +        public:
> +          title: Public
> +          type: boolean
> +{% endif %}
>      ErrorCheckCreate:
>        type: object
>        properties:
> diff --git a/docs/api/schemas/v1.0/patchwork.yaml b/docs/api/schemas/v1.0/patchwork.yaml
> index 02f3a156..cafef5ee 100644
> --- a/docs/api/schemas/v1.0/patchwork.yaml
> +++ b/docs/api/schemas/v1.0/patchwork.yaml
> @@ -1,5 +1,6 @@
>  # DO NOT EDIT THIS FILE. It is generated from a template. Changes should be
> -# proposed against the template.
> +# proposed against the template and updated files generated using the
> +# 'generate_schema.py' tool
>  ---
>  openapi: '3.0.0'
>  info:
> @@ -1246,6 +1247,7 @@ components:
>            allOf:
>              - $ref: '#/components/schemas/UserEmbedded'
>          patches:
> +          title: Patches
>            type: array
>            items:
>              $ref: '#/components/schemas/PatchEmbedded'
> @@ -1877,6 +1879,7 @@ components:
>          cover_letter:
>            $ref: '#/components/schemas/CoverLetterEmbedded'
>          patches:
> +          title: Patches
>            type: array
>            items:
>              $ref: '#/components/schemas/PatchEmbedded'
> diff --git a/docs/api/schemas/v1.1/patchwork.yaml b/docs/api/schemas/v1.1/patchwork.yaml
> index 0c086eda..9d45d058 100644
> --- a/docs/api/schemas/v1.1/patchwork.yaml
> +++ b/docs/api/schemas/v1.1/patchwork.yaml
> @@ -1,5 +1,6 @@
>  # DO NOT EDIT THIS FILE. It is generated from a template. Changes should be
> -# proposed against the template.
> +# proposed against the template and updated files generated using the
> +# 'generate_schema.py' tool
>  ---
>  openapi: '3.0.0'
>  info:
> @@ -1251,6 +1252,7 @@ components:
>            allOf:
>              - $ref: '#/components/schemas/UserEmbedded'
>          patches:
> +          title: Patches
>            type: array
>            items:
>              $ref: '#/components/schemas/PatchEmbedded'
> @@ -1928,6 +1930,7 @@ components:
>          cover_letter:
>            $ref: '#/components/schemas/CoverLetterEmbedded'
>          patches:
> +          title: Patches
>            type: array
>            items:
>              $ref: '#/components/schemas/PatchEmbedded'
> diff --git a/docs/api/schemas/v1.2/patchwork.yaml b/docs/api/schemas/v1.2/patchwork.yaml
> index 3a96aa3a..5c03bf1e 100644
> --- a/docs/api/schemas/v1.2/patchwork.yaml
> +++ b/docs/api/schemas/v1.2/patchwork.yaml
> @@ -1,5 +1,6 @@
>  # DO NOT EDIT THIS FILE. It is generated from a template. Changes should be
> -# proposed against the template.
> +# proposed against the template and updated files generated using the
> +# 'generate_schema.py' tool
>  ---
>  openapi: '3.0.0'
>  info:
> @@ -72,6 +73,35 @@ paths:
>                    $ref: '#/components/schemas/Bundle'
>        tags:
>          - bundles
> +    post:
> +      description: Create a bundle.
> +      operationId: bundles_create
> +      security:
> +        - basicAuth: []
> +        - apiKeyAuth: []
> +      requestBody:
> +        $ref: '#/components/requestBodies/Bundle'
> +      responses:
> +        '201':
> +          description: ''
> +          content:
> +            application/json:
> +              schema:
> +                $ref: '#/components/schemas/Bundle'
> +        '400':
> +          description: Invalid Request
> +          content:
> +            application/json:
> +              schema:
> +                $ref: '#/components/schemas/ErrorBundleCreateUpdate'
> +        '403':
> +          description: Forbidden
> +          content:
> +            application/json:
> +              schema:
> +                $ref: '#/components/schemas/Error'
> +      tags:
> +        - bundles
>    /api/1.2/bundles/{id}/:
>      get:
>        description: Show a bundle.
> @@ -99,6 +129,92 @@ paths:
>                  $ref: '#/components/schemas/Error'
>        tags:
>          - bundles
> +    patch:
> +      description: Update a bundle (partial).
> +      operationId: bundles_partial_update
> +      security:
> +        - basicAuth: []
> +        - apiKeyAuth: []
> +      parameters:
> +        - in: path
> +          name: id
> +          description: A unique integer value identifying this bundle.
> +          required: true
> +          schema:
> +            title: ID
> +            type: integer
> +      requestBody:
> +        $ref: '#/components/requestBodies/Bundle'
> +      responses:
> +        '200':
> +          description: ''
> +          content:
> +            application/json:
> +              schema:
> +                $ref: '#/components/schemas/Bundle'
> +        '400':
> +          description: Bad request
> +          content:
> +            application/json:
> +              schema:
> +                $ref: '#/components/schemas/ErrorBundleCreateUpdate'
> +        '403':
> +          description: Forbidden
> +          content:
> +            application/json:
> +              schema:
> +                $ref: '#/components/schemas/Error'
> +        '404':
> +          description: Not found
> +          content:
> +            application/json:
> +              schema:
> +                $ref: '#/components/schemas/Error'
> +      tags:
> +        - bundles
> +    put:
> +      description: Update a bundle.
> +      operationId: bundles_update
> +      security:
> +        - basicAuth: []
> +        - apiKeyAuth: []
> +      parameters:
> +        - in: path
> +          name: id
> +          description: A unique integer value identifying this bundle.
> +          required: true
> +          schema:
> +            title: ID
> +            type: integer
> +      requestBody:
> +        $ref: '#/components/requestBodies/Bundle'
> +      responses:
> +        '200':
> +          description: ''
> +          content:
> +            application/json:
> +              schema:
> +                $ref: '#/components/schemas/Bundle'
> +        '400':
> +          description: Bad request
> +          content:
> +            application/json:
> +              schema:
> +                $ref: '#/components/schemas/ErrorBundleCreateUpdate'
> +        '403':
> +          description: Forbidden
> +          content:
> +            application/json:
> +              schema:
> +                $ref: '#/components/schemas/Error'
> +        '404':
> +          description: Not found
> +          content:
> +            application/json:
> +              schema:
> +                $ref: '#/components/schemas/Error'
> +      tags:
> +        - bundles
>    /api/1.2/covers/:
>      get:
>        description: List cover letters.
> @@ -1131,6 +1247,18 @@ components:
>        schema:
>          type: string
>    requestBodies:
> +    Bundle:
> +      required: true
> +      content:
> +        application/json:
> +          schema:
> +            $ref: '#/components/schemas/BundleCreateUpdate'
> +        multipart/form-data:
> +          schema:
> +            $ref: '#/components/schemas/BundleCreateUpdate'
> +        application/x-www-form-urlencoded:
> +          schema:
> +            $ref: '#/components/schemas/BundleCreateUpdate'
>      Check:
>        required: true
>        content:
> @@ -1251,10 +1379,10 @@ components:
>            allOf:
>              - $ref: '#/components/schemas/UserEmbedded'
>          patches:
> +          title: Patches
>            type: array
>            items:
>              $ref: '#/components/schemas/PatchEmbedded'
> -          readOnly: true
>            uniqueItems: true
>          public:
>            title: Public
> @@ -1264,6 +1392,25 @@ components:
>            type: string
>            format: uri
>            readOnly: true
> +    BundleCreateUpdate:
> +      type: object
> +      required:
> +        - name
> +      properties:
> +        name:
> +          title: Name
> +          type: string
> +          minLength: 1
> +          maxLength: 50
> +        patches:
> +          title: Patches
> +          type: array
> +          items:
> +            type: integer
> +          uniqueItems: true
> +        public:
> +          title: Public
> +          type: boolean
>      Check:
>        type: object
>        properties:
> @@ -1961,6 +2108,7 @@ components:
>          cover_letter:
>            $ref: '#/components/schemas/CoverLetterEmbedded'
>          patches:
> +          title: Patches
>            type: array
>            items:
>              $ref: '#/components/schemas/PatchEmbedded'
> @@ -2307,6 +2455,24 @@ components:
>            title: Detail
>            type: string
>            readOnly: true
> +    ErrorBundleCreateUpdate:
> +      type: object
> +      properties:
> +        name:
> +          title: Name
> +          type: string
> +          minLength: 1
> +          maxLength: 50
> +        patches:
> +          title: Patches
> +          type: array
> +          items:
> +            $ref: '#/components/schemas/PatchEmbedded'
> +          readOnly: true
> +          uniqueItems: true
> +        public:
> +          title: Public
> +          type: boolean
>      ErrorCheckCreate:
>        type: object
>        properties:
> diff --git a/patchwork/api/bundle.py b/patchwork/api/bundle.py
> index 2dec70d1..c5885aae 100644
> --- a/patchwork/api/bundle.py
> +++ b/patchwork/api/bundle.py
> @@ -4,9 +4,12 @@
>  # SPDX-License-Identifier: GPL-2.0-or-later
>  
>  from django.db.models import Q
> -from rest_framework.generics import ListAPIView
> -from rest_framework.generics import RetrieveAPIView
> +from rest_framework import exceptions
> +from rest_framework.generics import ListCreateAPIView
> +from rest_framework.generics import RetrieveUpdateDestroyAPIView
> +from rest_framework import permissions
>  from rest_framework.serializers import SerializerMethodField
> +from rest_framework.serializers import ValidationError
>  
>  from patchwork.api.base import BaseHyperlinkedModelSerializer
>  from patchwork.api.base import PatchworkPermission
> @@ -14,16 +17,52 @@ from patchwork.api.filters import BundleFilterSet
>  from patchwork.api.embedded import PatchSerializer
>  from patchwork.api.embedded import ProjectSerializer
>  from patchwork.api.embedded import UserSerializer
> +from patchwork.api import utils
>  from patchwork.models import Bundle
>  
>  
> +class BundlePermission(permissions.BasePermission):
> +    """Ensure the API version, if configured, is >= v1.2.
> +
> +    Bundle creation/updating was only added in API v1.2 and we don't want to
> +    change behavior in older API versions.
> +    """
> +    def has_permission(self, request, view):
> +        # read-only permission for everything
> +        if request.method in permissions.SAFE_METHODS:
> +            return True
> +
> +        if not utils.has_version(request, '1.2'):
> +            raise exceptions.MethodNotAllowed(request.method)
> +
> +        if request.method == 'POST' and (
> +                not request.user or not request.user.is_authenticated):
> +            return False
> +
> +        # we have more to do but we can't do that until we have an object
> +        return True
> +
> +    def has_object_permission(self, request, view, obj):
> +        if (request.user and
> +                request.user.is_authenticated and
> +                request.user == obj.owner):

Just checking that this doesn't have the same User vs UserProfile thing
we had with delegation? I haven't checked in great detail, I just saw a
user comparison and thought it best to ask!

> +            return True
> +
> +        if not obj.public:
> +            # if the bundle isn't public, we don't want to leak the fact that
> +            # it exists
> +            raise exceptions.NotFound

A++

> +
> +        return request.method in permissions.SAFE_METHODS
> +
> +
>  class BundleSerializer(BaseHyperlinkedModelSerializer):
>  
>      web_url = SerializerMethodField()
>      project = ProjectSerializer(read_only=True)
>      mbox = SerializerMethodField()
>      owner = UserSerializer(read_only=True)
> -    patches = PatchSerializer(many=True, read_only=True)
> +    patches = PatchSerializer(many=True)
>  
>      def get_web_url(self, instance):
>          request = self.context.get('request')
> @@ -33,11 +72,35 @@ class BundleSerializer(BaseHyperlinkedModelSerializer):
>          request = self.context.get('request')
>          return request.build_absolute_uri(instance.get_mbox_url())
>  
> +    def create(self, validated_data):
> +        patches = validated_data.pop('patches')
> +        instance = super(BundleSerializer, self).create(validated_data)
> +        instance.overwrite_patches(patches)
> +        return instance
> +
> +    def update(self, instance, validated_data):
> +        patches = validated_data.pop('patches')
> +        instance = super(BundleSerializer, self).update(
> +            instance, validated_data)
> +        instance.overwrite_patches(patches)
> +        return instance
> +
> +    def validate(self, data):
> +        if not data.get('patches'):
> +            raise ValidationError('Bundles cannot be empty')
> +
> +        if len(set([p.project.id for p in data['patches']])) > 1:
> +            raise ValidationError('Patches must belong to the same project')
> +
> +        data['project'] = data['patches'][0].project
> +
> +        return super(BundleSerializer, self).validate(data)
> +
>      class Meta:
>          model = Bundle
>          fields = ('id', 'url', 'web_url', 'project', 'name', 'owner',
>                    'patches', 'public', 'mbox')
> -        read_only_fields = ('owner', 'patches', 'mbox')
> +        read_only_fields = ('project', 'owner', 'mbox')
>          versioned_fields = {
>              '1.1': ('web_url', ),
>          }
> @@ -48,7 +111,7 @@ class BundleSerializer(BaseHyperlinkedModelSerializer):
>  
>  class BundleMixin(object):
>  
> -    permission_classes = (PatchworkPermission,)
> +    permission_classes = [PatchworkPermission & BundlePermission]
>      serializer_class = BundleSerializer
>  
>      def get_queryset(self):
> @@ -63,16 +126,19 @@ class BundleMixin(object):
>              .select_related('owner', 'project')
>  
>  
> -class BundleList(BundleMixin, ListAPIView):
> -    """List bundles."""
> +class BundleList(BundleMixin, ListCreateAPIView):
> +    """List or create bundles."""
>  
>      filter_class = filterset_class = BundleFilterSet
>      search_fields = ('name',)
>      ordering_fields = ('id', 'name', 'owner')
>      ordering = 'id'
>  
> +    def perform_create(self, serializer):
> +        serializer.save(owner=self.request.user)
> +
Likewise and throughout the patch w/ user types.

Apart from that, I think this is absolutely a Good Idea and I hope to do
some testing of it soon.

Regards,
Daniel

>  
> -class BundleDetail(BundleMixin, RetrieveAPIView):
> -    """Show a bundle."""
> +class BundleDetail(BundleMixin, RetrieveUpdateDestroyAPIView):
> +    """Show, update or delete a bundle."""
>  
>      pass
> diff --git a/patchwork/models.py b/patchwork/models.py
> index 32d1b3c2..631de85d 100644
> --- a/patchwork/models.py
> +++ b/patchwork/models.py
> @@ -788,6 +788,11 @@ class Bundle(models.Model):
>      patches = models.ManyToManyField(Patch, through='BundlePatch')
>      public = models.BooleanField(default=False)
>  
> +    def is_editable(self, user):
> +        if not user.is_authenticated:
> +            return False
> +        return user == self.owner
> +
>      def ordered_patches(self):
>          return self.patches.order_by('bundlepatch__order')
>  
> @@ -806,6 +811,12 @@ class Bundle(models.Model):
>          return BundlePatch.objects.create(bundle=self, patch=patch,
>                                            order=max_order + 1)
>  
> +    def overwrite_patches(self, patches):
> +        BundlePatch.objects.filter(bundle=self).delete()
> +
> +        for patch in patches:
> +            self.append_patch(patch)
> +
>      def get_absolute_url(self):
>          return reverse('bundle-detail', kwargs={
>              'username': self.owner.username,
> diff --git a/patchwork/tests/api/test_bundle.py b/patchwork/tests/api/test_bundle.py
> index 303c500c..a3a0c113 100644
> --- a/patchwork/tests/api/test_bundle.py
> +++ b/patchwork/tests/api/test_bundle.py
> @@ -8,9 +8,11 @@ import unittest
>  from django.conf import settings
>  from django.urls import reverse
>  
> +from patchwork.models import Bundle
>  from patchwork.tests.api import utils
>  from patchwork.tests.utils import create_bundle
>  from patchwork.tests.utils import create_maintainer
> +from patchwork.tests.utils import create_patch
>  from patchwork.tests.utils import create_project
>  from patchwork.tests.utils import create_user
>  
> @@ -42,12 +44,15 @@ class TestBundleAPI(utils.APITestCase):
>  
>          # nested fields
>  
> -        self.assertEqual(bundle_obj.patches.count(),
> -                         len(bundle_json['patches']))
>          self.assertEqual(bundle_obj.owner.id,
>                           bundle_json['owner']['id'])
>          self.assertEqual(bundle_obj.project.id,
>                           bundle_json['project']['id'])
> +        self.assertEqual(bundle_obj.patches.count(),
> +                         len(bundle_json['patches']))
> +        for patch_obj, patch_json in zip(
> +                bundle_obj.patches.all(), bundle_json['patches']):
> +            self.assertEqual(patch_obj.id, patch_json['id'])
>  
>      def test_list_empty(self):
>          """List bundles when none are present."""
> @@ -179,18 +184,117 @@ class TestBundleAPI(utils.APITestCase):
>          self.assertIn('url', resp.data)
>          self.assertNotIn('web_url', resp.data)
>  
> -    def test_create_update_delete(self):
> -        """Ensure creates, updates and deletes aren't allowed"""
> +    def _test_create_update(self, authenticate=True):
> +        user = create_user()
> +        project = create_project()
> +        patch_a = create_patch(project=project)
> +        patch_b = create_patch(project=project)
> +
> +        if authenticate:
> +            self.client.force_authenticate(user=user)
> +
> +        return user, project, patch_a, patch_b
> +
> +    @utils.store_samples('bundle-create-error-forbidden')
> +    def test_create_anonymous(self):
> +        """Create a bundle when not signed in.
> +
> +        Ensure creations can only be performed by signed in users.
> +        """
> +        user, project, patch_a, patch_b = self._test_create_update(
> +            authenticate=False)
> +        bundle = {
> +            'name': 'test-bundle',
> +            'public': True,
> +            'patches': [patch_a.id, patch_b.id],
> +        }
> +
> +        resp = self.client.post(self.api_url(), bundle)
> +        self.assertEqual(status.HTTP_403_FORBIDDEN, resp.status_code)
> +
> +    @utils.store_samples('bundle-create')
> +    def test_create(self):
> +        """Validate we can create a new bundle."""
> +        user, project, patch_a, patch_b = self._test_create_update()
> +        bundle = {
> +            'name': 'test-bundle',
> +            'public': True,
> +            'patches': [patch_a.id, patch_b.id],
> +        }
> +
> +        resp = self.client.post(self.api_url(), bundle)
> +        self.assertEqual(status.HTTP_201_CREATED, resp.status_code)
> +        self.assertEqual(1, Bundle.objects.all().count())
> +        self.assertSerialized(Bundle.objects.first(), resp.data)
> +
> +    @utils.store_samples('bundle-update-not-found')
> +    def test_update_anonymous(self):
> +        """Update an existing bundle when not signed in.
> +
> +        Ensure updates can only be performed by signed in users.
> +        """
> +        user, project, patch_a, patch_b = self._test_create_update(
> +            authenticate=False)
> +        bundle = create_bundle(owner=user, project=project)
> +
> +        resp = self.client.patch(self.api_url(bundle.id), {
> +            'name': 'hello-bundle', 'patches': [patch_a.id, patch_b.id]})
> +        self.assertEqual(status.HTTP_404_NOT_FOUND, resp.status_code)
> +
> +    @utils.store_samples('bundle-update')
> +    def test_update(self):
> +        """Validate we can update an existing bundle."""
> +        user, project, patch_a, patch_b = self._test_create_update()
> +        bundle = create_bundle(owner=user, project=project)
> +
> +        resp = self.client.patch(self.api_url(bundle.id), {
> +            'name': 'hello-bundle', 'patches': [patch_a.id, patch_b.id]})
> +        self.assertEqual(status.HTTP_200_OK, resp.status_code)
> +        self.assertEqual(1, Bundle.objects.all().count())
> +        self.assertEqual(len(resp.data['patches']), 2)
> +        self.assertEqual(resp.data['name'], 'hello-bundle')
> +
> +    @utils.store_samples('bundle-delete-not-found')
> +    def test_delete_anonymous(self):
> +        """Delete a bundle when not signed in.
> +
> +        Ensure deletions can only be performed when signed in.
> +        """
> +        user, project, patch_a, patch_b = self._test_create_update(
> +            authenticate=False)
> +        bundle = create_bundle(owner=user, project=project)
> +
> +        resp = self.client.delete(self.api_url(bundle.id))
> +        self.assertEqual(status.HTTP_404_NOT_FOUND, resp.status_code)
> +
> +    @utils.store_samples('bundle-delete')
> +    def test_delete(self):
> +        """Validate we can delete an existing bundle."""
> +        user = create_user()
> +        bundle = create_bundle(owner=user)
> +
> +        self.client.force_authenticate(user=user)
> +
> +        resp = self.client.delete(self.api_url(bundle.id))
> +        self.assertEqual(status.HTTP_204_NO_CONTENT, resp.status_code)
> +        self.assertEqual(0, Bundle.objects.all().count())
> +
> +    def test_create_update_delete_version_1_1(self):
> +        """Ensure creates, updates and deletes aren't allowed with old API."""
>          user = create_maintainer()
>          user.is_superuser = True
>          user.save()
>          self.client.force_authenticate(user=user)
>  
> -        resp = self.client.post(self.api_url(), {'email': 'foo at f.com'})
> +        resp = self.client.post(self.api_url(version='1.1'), {'name': 'test'},
> +                                validate_schema=False)
>          self.assertEqual(status.HTTP_405_METHOD_NOT_ALLOWED, resp.status_code)
>  
> -        resp = self.client.patch(self.api_url(user.id), {'email': 'foo at f.com'})
> +        resp = self.client.patch(self.api_url(1, version='1.1'),
> +                                 {'name': 'test'},
> +                                 validate_schema=False)
>          self.assertEqual(status.HTTP_405_METHOD_NOT_ALLOWED, resp.status_code)
>  
> -        resp = self.client.delete(self.api_url(1))
> +        resp = self.client.delete(self.api_url(1, version='1.1'),
> +                                  validate_schema=False)
>          self.assertEqual(status.HTTP_405_METHOD_NOT_ALLOWED, resp.status_code)
> diff --git a/patchwork/tests/api/utils.py b/patchwork/tests/api/utils.py
> index 0c232d04..ce83ce2b 100644
> --- a/patchwork/tests/api/utils.py
> +++ b/patchwork/tests/api/utils.py
> @@ -112,44 +112,52 @@ class APIClient(BaseAPIClient):
>          self.factory = APIRequestFactory()
>  
>      def get(self, path, data=None, follow=False, **extra):
> +        validate_schema = extra.pop('validate_schema', True)
>          request = self.factory.get(
>              path, data=data, SERVER_NAME='example.com', **extra)
>          response = super(APIClient, self).get(
>              path, data=data, follow=follow, SERVER_NAME='example.com', **extra)
> -        validator.validate_data(path, request, response)
> +        if validate_schema:
> +            validator.validate_data(path, request, response)
>          return response
>  
>      def post(self, path, data=None, format=None, content_type=None,
>               follow=False, **extra):
> +        validate_schema = extra.pop('validate_schema', True)
>          request = self.factory.post(
>              path, data=data, format='json', content_type=content_type,
>              SERVER_NAME='example.com', **extra)
>          response = super(APIClient, self).post(
>              path, data=data, format='json', content_type=content_type,
>              follow=follow, SERVER_NAME='example.com', **extra)
> -        validator.validate_data(path, request, response)
> +        if validate_schema:
> +            validator.validate_data(path, request, response)
>          return response
>  
>      def put(self, path, data=None, format=None, content_type=None,
>              follow=False, **extra):
> +        validate_schema = extra.pop('validate_schema', True)
>          request = self.factory.put(
>              path, data=data, format='json', content_type=content_type,
>              SERVER_NAME='example.com', **extra)
>          response = super(APIClient, self).put(
>              path, data=data, format='json', content_type=content_type,
>              follow=follow, SERVER_NAME='example.com', **extra)
> -        validator.validate_data(path, request, response)
> +        if validate_schema:
> +            validator.validate_data(path, request, response)
>          return response
>  
>      def patch(self, path, data=None, format=None, content_type=None,
>                follow=False, **extra):
> +        validate_schema = extra.pop('validate_schema', True)
>          request = self.factory.patch(
>              path, data=data, format='json', content_type=content_type,
>              SERVER_NAME='example.com', **extra)
>          response = super(APIClient, self).patch(
>              path, data=data, format='json', content_type=content_type,
>              follow=follow, SERVER_NAME='example.com', **extra)
> -        validator.validate_data(path, request, response)
> +        if validate_schema:
> +            validator.validate_data(path, request, response)
>          return response
>  
>  
> diff --git a/releasenotes/notes/update-bundle-via-api-2946d8c4e730d545.yaml b/releasenotes/notes/update-bundle-via-api-2946d8c4e730d545.yaml
> new file mode 100644
> index 00000000..bfa1ef55
> --- /dev/null
> +++ b/releasenotes/notes/update-bundle-via-api-2946d8c4e730d545.yaml
> @@ -0,0 +1,4 @@
> +---
> +api:
> +  - |
> +    Bundles can now be created, updated and deleted via the REST API.
> -- 
> 2.21.0
>
> _______________________________________________
> Patchwork mailing list
> Patchwork at lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/patchwork


More information about the Patchwork mailing list