From e0d4cd86409c5e76542273b7d5e670d9f2b9efb8 Mon Sep 17 00:00:00 2001 From: Tomas Cohen Arazi 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 Signed-off-by: Martin Renvoize --- 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 2132ec0a04..75a43aa9c8 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 c721f409c3..0f9201b9ca 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 da80c2e23e..7de141a5d1 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.20.1