Attachment 118818 Details for Bug 23666 – Bug 23666: Add PATCH /patron/:patron_id/extended_attributes/:extended_attribute_id

[patch] Bug 23666: Add PATCH /patron/:patron_id/extended_attributes/:extended_attribute_id

Bug-23666-Add-PATCH-patronpatronidextendedattribut.patch (text/plain), 10.75 KB, created by Tomás Cohen Arazi (tcohen) on 2021-03-25 14:53:16 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Tomás Cohen Arazi (tcohen)

Created: 2021-03-25 14:53:16 UTC

Size: 10.75 KB

patch

obsolete

>From 07ea2c64a73cb92534bcfced5c14f42ef24f689a Mon Sep 17 00:00:00 2001
>From: Tomas Cohen Arazi <tomascohen@theke.io>
>Date: Thu, 25 Mar 2021 10:25:41 -0300
>Subject: [PATCH] Bug 23666: Add PATCH
> /patron/:patron_id/extended_attributes/:extended_attribute_id
>
>This patch adds the described route. It is designed to use the
>underlying libraries' methods to update an existing attribute.
>
>The tests cover the use cases.
>
>Note: I added handling for two exceptions that can only occur on bad
>data (i.e. not by using our codebase). This are:
>- Koha::Exceptions::Patron::Attribute::InvalidType
>- Koha::Exceptions::Patron::Attribute::NonRepeatable
>
>To test:
>1. Apply this patch
>2. Run:
>   $ kshell
>  k$ prove t/db_dependent/api/v1/patrons_extended_attributes.t
>=> SUCCESS: Tests pass!
>3. PLay with the route
>=> SUCCESS: Expected behavior!
>4. Sign off :-D
>
>Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
>---
> Koha/REST/V1/Patrons/Attributes.pm            |  78 +++++++++++-
> .../paths/patrons_extended_attributes.json    |  93 ++++++++++++++
> .../api/v1/patrons_extended_attributes.t      | 116 +++++++++++++++++-
> 3 files changed, 284 insertions(+), 3 deletions(-)
>
>diff --git a/Koha/REST/V1/Patrons/Attributes.pm b/Koha/REST/V1/Patrons/Attributes.pm
>index 2132ec0a047..75a43aa9c8b 100644
>--- a/Koha/REST/V1/Patrons/Attributes.pm
>+++ b/Koha/REST/V1/Patrons/Attributes.pm
>@@ -186,7 +186,6 @@ sub overwrite {
>                     status  => 400,
>                     openapi => { error => "$_" }
>                 );
>-
>             }
>             elsif (
>                 $_->isa(
>@@ -222,9 +221,84 @@ sub overwrite {
>     };
> }
> 
>+
>+=head3 update
>+
>+Controller method that handles updating a single extended patron attribute.
>+
>+=cut
>+
>+sub update {
>+    my $c = shift->openapi->valid_input or return;
>+
>+    my $patron = Koha::Patrons->find( $c->validation->param('patron_id') );
>+
>+    unless ($patron) {
>+        return $c->render(
>+            status  => 404,
>+            openapi => {
>+                error => 'Patron not found'
>+            }
>+        );
>+    }
>+
>+    return try {
>+        my $attribute = $patron->extended_attributes->find(
>+            $c->validation->param('extended_attribute_id') );
>+
>+        unless ($attribute) {
>+            return $c->render(
>+                status  => 404,
>+                openapi => {
>+                    error => 'Attribute not found'
>+                }
>+            );
>+        }
>+
>+        $attribute->set_from_api( $c->validation->param('body') )->store;
>+        $attribute->discard_changes;
>+
>+        return $c->render(
>+            status  => 200,
>+            openapi => $attribute->to_api
>+        );
>+    }
>+    catch {
>+        if ( blessed $_ ) {
>+            if ( $_->isa('Koha::Exceptions::Patron::Attribute::InvalidType') ) {
>+                return $c->render(
>+                    status  => 400,
>+                    openapi => { error => "$_" }
>+                );
>+            }
>+            elsif (
>+                $_->isa(
>+                    'Koha::Exceptions::Patron::Attribute::UniqueIDConstraint')
>+              )
>+            {
>+                return $c->render(
>+                    status  => 409,
>+                    openapi => { error => "$_" }
>+                );
>+            }
>+            elsif (
>+                $_->isa('Koha::Exceptions::Patron::Attribute::NonRepeatable') )
>+            {
>+                return $c->render(
>+                    status  => 409,
>+                    openapi => { error => "$_" }
>+                );
>+            }
>+        }
>+
>+        $c->unhandled_exception($_);
>+    };
>+
>+}
>+
> =head3 delete
> 
>-Controller method that handling removing an extended patron attribute
>+Controller method that handles removing an extended patron attribute.
> 
> =cut
> 
>diff --git a/api/v1/swagger/paths/patrons_extended_attributes.json b/api/v1/swagger/paths/patrons_extended_attributes.json
>index c721f409c35..0f9201b9ca2 100644
>--- a/api/v1/swagger/paths/patrons_extended_attributes.json
>+++ b/api/v1/swagger/paths/patrons_extended_attributes.json
>@@ -251,6 +251,99 @@
>     }
>   },
>   "/patrons/{patron_id}/extended_attributes/{extended_attribute_id}": {
>+    "patch": {
>+      "x-mojo-to": "Patrons::Attributes#update",
>+      "operationId": "updatePatronAttribute",
>+      "tags": [
>+        "patrons",
>+        "extended_attributes"
>+      ],
>+      "parameters": [
>+        {
>+          "$ref": "../parameters.json#/patron_id_pp"
>+        },
>+        {
>+          "name": "extended_attribute_id",
>+          "in": "path",
>+          "description": "Internal patron extended attribute identifier",
>+          "type": "integer",
>+          "required": true
>+        },
>+        {
>+          "name": "body",
>+          "in": "body",
>+          "description": "An object containing the updated values for the patron extended attribute",
>+          "required": true,
>+          "schema": {
>+            "type": "object",
>+            "properties": {
>+              "value": {
>+                "description": "Extended attribute value",
>+                "type": "string"
>+              }
>+            }
>+          }
>+        }
>+      ],
>+      "produces": [
>+        "application/json"
>+      ],
>+      "responses": {
>+        "200": {
>+          "description": "A successfully updated patron extended attribute",
>+          "schema": {
>+            "$ref": "../definitions.json#/patron_extended_attribute"
>+          }
>+        },
>+        "400": {
>+          "description": "Bad parameter",
>+          "schema": {
>+            "$ref": "../definitions.json#/error"
>+          }
>+        },
>+        "401": {
>+          "description": "Authentication required",
>+          "schema": {
>+            "$ref": "../definitions.json#/error"
>+          }
>+        },
>+        "403": {
>+          "description": "Access forbidden",
>+          "schema": {
>+            "$ref": "../definitions.json#/error"
>+          }
>+        },
>+        "404": {
>+          "description": "Object not found",
>+          "schema": {
>+            "$ref": "../definitions.json#/error"
>+          }
>+        },
>+        "409": {
>+          "description": "Conflict in updating resource",
>+          "schema": {
>+            "$ref": "../definitions.json#/error"
>+          }
>+        },
>+        "500": {
>+          "description": "Internal server error",
>+          "schema": {
>+            "$ref": "../definitions.json#/error"
>+          }
>+        },
>+        "503": {
>+          "description": "Under maintenance",
>+          "schema": {
>+            "$ref": "../definitions.json#/error"
>+          }
>+        }
>+      },
>+      "x-koha-authorization": {
>+        "permissions": {
>+          "borrowers": "edit_borrowers"
>+        }
>+      }
>+    },
>     "delete": {
>       "x-mojo-to": "Patrons::Attributes#delete",
>       "operationId": "deletePatronAttribute",
>diff --git a/t/db_dependent/api/v1/patrons_extended_attributes.t b/t/db_dependent/api/v1/patrons_extended_attributes.t
>index da80c2e23ea..7de141a5d1a 100755
>--- a/t/db_dependent/api/v1/patrons_extended_attributes.t
>+++ b/t/db_dependent/api/v1/patrons_extended_attributes.t
>@@ -17,7 +17,7 @@
> 
> use Modern::Perl;
> 
>-use Test::More tests => 4;
>+use Test::More tests => 5;
> use Test::Mojo;
> 
> use t::lib::TestBuilder;
>@@ -394,3 +394,117 @@ subtest 'delete() tests' => sub {
> 
>     $schema->storage->txn_rollback;
> };
>+
>+subtest 'update() tests' => sub {
>+
>+    plan tests => 12;
>+
>+    $schema->storage->txn_begin;
>+
>+    my $patron = $builder->build_object(
>+        {
>+            class => 'Koha::Patrons',
>+            value => { flags => 2**4 }    # 'borrowers' flag == 4
>+        }
>+    );
>+    my $password = 'thePassword123';
>+    $patron->set_password( { password => $password, skip_validation => 1 } );
>+    my $userid = $patron->userid;
>+
>+    my $repeatable_attr_type = $builder->build_object(
>+        {
>+            class => 'Koha::Patron::Attribute::Types',
>+            value => {
>+                mandatory     => 0,
>+                repeatable    => 1,
>+                unique_id     => 0,
>+                category_code => undef
>+            }
>+        }
>+    );
>+    my $unique_attr_type = $builder->build_object(
>+        {
>+            class => 'Koha::Patron::Attribute::Types',
>+            value => {
>+                mandatory     => 0,
>+                repeatable    => 0,
>+                unique_id     => 1,
>+                category_code => undef
>+            }
>+        }
>+    );
>+
>+    # Add a unique attribute to our patron
>+    my $unique_attribute = $patron->add_extended_attribute(
>+        {
>+            code      => $unique_attr_type->code,
>+            attribute => 'WOW'
>+        }
>+    );
>+
>+    # Let's have an attribute ID we are sure doesn't exist on the DB
>+    my $non_existent_attribute = $patron->add_extended_attribute(
>+        {
>+            code      => $repeatable_attr_type->code,
>+            attribute => 'BOO'
>+        }
>+    );
>+    my $non_existent_attribute_id = $non_existent_attribute->id;
>+    $non_existent_attribute->delete;
>+
>+    my $non_existent_patron =
>+      $builder->build_object( { class => 'Koha::Patrons' } );
>+    my $non_existent_patron_id = $non_existent_patron->id;
>+
>+    # get rid of the patron
>+    $non_existent_patron->delete;
>+
>+    $t->patch_ok( "//$userid:$password@/api/v1/patrons/"
>+          . $non_existent_patron_id
>+          . '/extended_attributes/'
>+          . 123 => json => { value => 'something' } )->status_is(404)
>+      ->json_is( '/error' => 'Patron not found' );
>+
>+    $t->patch_ok( "//$userid:$password@/api/v1/patrons/"
>+          . $patron->id
>+          . '/extended_attributes/'
>+          . $non_existent_attribute_id => json => { value => 'something' } )
>+      ->status_is(404)->json_is( '/error' => 'Attribute not found' );
>+
>+    my $response =
>+      $t->patch_ok( "//$userid:$password@/api/v1/patrons/"
>+          . $patron->id
>+          . '/extended_attributes/'
>+          . $unique_attribute->id => json => { value => 'HEY' } )
>+      ->status_is(200)->tx->res->json;
>+
>+    is_deeply(
>+        Koha::Patron::Attributes->find( $response->{extended_attribute_id} )
>+          ->to_api,
>+        $response,
>+        "The returned object is on the DB"
>+    );
>+
>+    my $unique_value = 'HEHE';
>+
>+    # Add a patron with the unique attribute to test changing to it
>+    $builder->build_object( { class => 'Koha::Patrons' } )
>+      ->add_extended_attribute(
>+        {
>+            code      => $unique_attr_type->code,
>+            attribute => $unique_value
>+        }
>+      );
>+
>+    $t->patch_ok( "//$userid:$password@/api/v1/patrons/"
>+          . $patron->id
>+          . '/extended_attributes/'
>+          . $unique_attribute->id => json => { value => $unique_value } )
>+      ->status_is(409)
>+      ->json_is( '/error' =>
>+            "Your action breaks a unique constraint on the attribute. type="
>+          . $unique_attr_type->code
>+          . " value=$unique_value" );
>+
>+    $schema->storage->txn_rollback;
>+};
>-- 
>2.31.0

Actions: View | Diff | Splinter Review

Attachments on bug 23666: 118508 | 118509 | 118510 | 118550 | 118699 | 118700 | 118701 | 118702 | 118818 | 118854 | 119042 | 119388 | 119389 | 119390 | 119391 | 119392 | 119393