From ccf0bfeb1096ce27786e3a5d755c3c6382ff866b Mon Sep 17 00:00:00 2001 From: Johanna Raisa Date: Tue, 9 Jul 2019 11:53:07 +0300 Subject: [PATCH] Bug 23285: Add patrons PATCH route Implementation for patrons PATCH route. Test plan - Apply this patch - Run prove t/db_dependent/api/v1/patrons.t - Create user with borrowers permissions - Authenticate with the user created above and get a CGISESSION id. - Use the CGISESSION to patch patrons via the API - PATCH /patrons/{patron_id} with some parameters in the body Sponsored-by: Koha-Suomi Oy --- Koha/REST/V1/Patrons.pm | 2 +- api/v1/swagger/definitions.json | 3 + api/v1/swagger/definitions/patron_patch.json | 283 +++++++++++++++++++ api/v1/swagger/paths/patrons.json | 78 +++++ t/db_dependent/api/v1/patrons.t | 66 ++++- 5 files changed, 430 insertions(+), 2 deletions(-) create mode 100644 api/v1/swagger/definitions/patron_patch.json diff --git a/Koha/REST/V1/Patrons.pm b/Koha/REST/V1/Patrons.pm index 5e8e398eac..71b67171a7 100644 --- a/Koha/REST/V1/Patrons.pm +++ b/Koha/REST/V1/Patrons.pm @@ -198,7 +198,7 @@ sub update { $patron->set($body)->store; $patron->discard_changes; - return $c->render( status => 200, openapi => $patron ); + return $c->render( status => 200, openapi => _to_api( $patron->TO_JSON ) ); } catch { unless ( blessed $_ && $_->can('rethrow') ) { diff --git a/api/v1/swagger/definitions.json b/api/v1/swagger/definitions.json index e3001b5e60..4a7b6a8ab8 100644 --- a/api/v1/swagger/definitions.json +++ b/api/v1/swagger/definitions.json @@ -32,6 +32,9 @@ "patron_balance": { "$ref": "definitions/patron_balance.json" }, + "patron_patch": { + "$ref": "definitions/patron_patch.json" + }, "allows_renewal": { "$ref": "definitions/allows_renewal.json" }, diff --git a/api/v1/swagger/definitions/patron_patch.json b/api/v1/swagger/definitions/patron_patch.json new file mode 100644 index 0000000000..2f55493d12 --- /dev/null +++ b/api/v1/swagger/definitions/patron_patch.json @@ -0,0 +1,283 @@ +{ + "type": "object", + "properties": { + "patron_id": { + "$ref": "../x-primitives.json#/patron_id" + }, + "cardnumber": { + "$ref": "../x-primitives.json#/cardnumber" + }, + "surname": { + "$ref": "../x-primitives.json#/surname" + }, + "firstname": { + "$ref": "../x-primitives.json#/firstname" + }, + "title": { + "type": ["string", "null"], + "description": "patron's title" + }, + "other_name": { + "type": ["string", "null"], + "description": "any other names associated with the patron" + }, + "initials": { + "type": ["string", "null"], + "description": "initials of the patron" + }, + "street_number": { + "type": ["string", "null"], + "description": "street number of patron's primary address" + }, + "street_type": { + "type": ["string", "null"], + "description": "street type of patron's primary address" + }, + "address": { + "type": ["string", "null"], + "description": "first address line of patron's primary address" + }, + "address2": { + "type": ["string", "null"], + "description": "second address line of patron's primary address" + }, + "city": { + "type": ["string", "null"], + "description": "city or town of patron's primary address" + }, + "state": { + "type": ["string", "null"], + "description": "state or province of patron's primary address" + }, + "postal_code": { + "type": ["string", "null"], + "description": "zip or postal code of patron's primary address" + }, + "country": { + "type": ["string", "null"], + "description": "country of patron's primary address" + }, + "email": { + "$ref": "../x-primitives.json#/email" + }, + "phone": { + "$ref": "../x-primitives.json#/phone" + }, + "mobile": { + "type": ["string", "null"], + "description": "the other phone number for patron's primary address" + }, + "fax": { + "type": ["string", "null"], + "description": "fax number for patron's primary address" + }, + "secondary_email": { + "type": ["string", "null"], + "description": "secondary email address for patron's primary address" + }, + "secondary_phone": { + "type": ["string", "null"], + "description": "secondary phone number for patron's primary address" + }, + "altaddress_street_number": { + "type": ["string", "null"], + "description": "street number of patron's alternate address" + }, + "altaddress_street_type": { + "type": ["string", "null"], + "description": "street type of patron's alternate address" + }, + "altaddress_address": { + "type": ["string", "null"], + "description": "first address line of patron's alternate address" + }, + "altaddress_address2": { + "type": ["string", "null"], + "description": "second address line of patron's alternate address" + }, + "altaddress_city": { + "type": ["string", "null"], + "description": "city or town of patron's alternate address" + }, + "altaddress_state": { + "type": ["string", "null"], + "description": "state or province of patron's alternate address" + }, + "altaddress_postal_code": { + "type": ["string", "null"], + "description": "zip or postal code of patron's alternate address" + }, + "altaddress_country": { + "type": ["string", "null"], + "description": "country of patron's alternate address" + }, + "altaddress_email": { + "type": ["string", "null"], + "description": "email address for patron's alternate address" + }, + "altaddress_phone": { + "type": ["string", "null"], + "description": "phone number for patron's alternate address" + }, + "date_of_birth": { + "type": ["string", "null"], + "format": "date", + "description": "patron's date of birth" + }, + "library_id": { + "type": "string", + "description": "Internal identifier for the patron's home library" + }, + "category_id": { + "type": "string", + "description": "Internal identifier for the patron's category" + }, + "date_enrolled": { + "type": ["string", "null"], + "format": "date", + "description": "date the patron was added to Koha" + }, + "expiry_date": { + "type": ["string", "null"], + "format": "date", + "description": "date the patron's card is set to expire" + }, + "date_renewed": { + "type": ["string", "null"], + "description": "date the patron's card was last renewed" + }, + "incorrect_address": { + "type": ["boolean", "null"], + "description": "set to 1 if library marked this patron as having an unconfirmed address" + }, + "patron_card_lost": { + "type": ["boolean", "null"], + "description": "set to 1 if library marked this patron as having lost his card" + }, + "restricted": { + "type": "boolean", + "readOnly": true, + "description": "If any restriction applies to the patron" + }, + "guarantor_id": { + "type": ["integer", "null"], + "description": "patron_id used for children or professionals to link them to guarantor or organizations" + }, + "staff_notes": { + "type": ["string", "null"], + "description": "a note on the patron's account" + }, + "relationship_type": { + "type": ["string", "null"], + "description": "used for children to include the relationship to their guarantor" + }, + "gender": { + "type": ["string", "null"], + "description": "patron's gender" + }, + "userid": { + "type": ["string", "null"], + "description": "patron's login" + }, + "opac_notes": { + "type": ["string", "null"], + "description": "a note on the patron's account visible in OPAC and staff client" + }, + "altaddress_notes": { + "type": ["string", "null"], + "description": "a note related to patron's alternate address" + }, + "statistics_1": { + "type": ["string", "null"], + "description": "a field that can be used for any information unique to the library" + }, + "statistics_2": { + "type": ["string", "null"], + "description": "a field that can be used for any information unique to the library" + }, + "altcontact_firstname": { + "type": ["string", "null"], + "description": "first name of alternate contact for the patron" + }, + "altcontact_surname": { + "type": ["string", "null"], + "description": "surname or last name of the alternate contact for the patron" + }, + "altcontact_address": { + "type": ["string", "null"], + "description": "the first address line for the alternate contact for the patron" + }, + "altcontact_address2": { + "type": ["string", "null"], + "description": "the second address line for the alternate contact for the patron" + }, + "altcontact_city": { + "type": ["string", "null"], + "description": "the city for the alternate contact for the patron" + }, + "altcontact_state": { + "type": ["string", "null"], + "description": "the state for the alternate contact for the patron" + }, + "altcontact_postal_code": { + "type": ["string", "null"], + "description": "the zipcode for the alternate contact for the patron" + }, + "altcontact_country": { + "type": ["string", "null"], + "description": "the country for the alternate contact for the patron" + }, + "altcontact_phone": { + "type": ["string", "null"], + "description": "the phone number for the alternate contact for the patron" + }, + "sms_number": { + "type": ["string", "null"], + "description": "the mobile phone number where the patron would like to receive notices (if SMS turned on)" + }, + "sms_provider_id": { + "type": ["integer", "null"], + "description": "the provider of the mobile phone number defined in smsalertnumber" + }, + "privacy": { + "type": "integer", + "description": "patron's privacy settings related to their reading history" + }, + "privacy_guarantor_checkouts": { + "type": "integer", + "description": "controls if relatives can see this patron's checkouts" + }, + "check_previous_checkout": { + "type": "string", + "description": "produce a warning for this patron if this item has previously been checked out to this patron if 'yes', not if 'no', defer to category setting if 'inherit'" + }, + "updated_on": { + "type": "string", + "format": "date-time", + "description": "time of last change could be useful for synchronization with external systems (among others)" + }, + "last_seen": { + "type": ["string", "null"], + "format": "date-time", + "description": "last time a patron has been seen (connected at the OPAC or staff interface)" + }, + "lang": { + "type": "string", + "description": "lang to use to send notices to this patron" + }, + "login_attempts": { + "type": ["integer", "null"], + "description": "number of failed login attemps" + }, + "overdrive_auth_token": { + "type": ["string", "null"], + "description": "persist OverDrive auth token" + }, + "anonymized": { + "type": "boolean", + "readOnly": true, + "description": "If the patron has been anonymized" + } + }, + "additionalProperties": false +} diff --git a/api/v1/swagger/paths/patrons.json b/api/v1/swagger/paths/patrons.json index 27bd08f69f..ed522eec66 100644 --- a/api/v1/swagger/paths/patrons.json +++ b/api/v1/swagger/paths/patrons.json @@ -654,6 +654,84 @@ } } }, + "patch": { + "x-mojo-to": "Patrons#update", + "operationId": "patchPatron", + "tags": ["patrons"], + "parameters": [ + { + "$ref": "../parameters.json#/patron_id_pp" + }, + { + "name": "body", + "in": "body", + "description": "A JSON object containing new information about existing patron", + "required": true, + "schema": { + "$ref": "../definitions.json#/patron_patch" + } + } + ], + "consumes": ["application/json"], + "produces": ["application/json"], + "responses": { + "200": { + "description": "A successfully updated patron", + "schema": { + "items": { + "$ref": "../definitions.json#/patron" + } + } + }, + "202": { + "description": "Accepted and waiting for librarian verification", + "schema": { + "type": "object" + } + }, + "204": { + "description": "No Content", + "schema": { + "type": "object" + } + }, + "400": { + "description": "Bad parameter", + "schema": { + "$ref": "../definitions.json#/error" + } + }, + "403": { + "description": "Access forbidden", + "schema": { + "$ref": "../definitions.json#/error" + } + }, + "404": { + "description": "Resource 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" + } + } + }, + "x-koha-authorization": { + "permissions": { + "borrowers": "1" + } + } + }, "delete": { "x-mojo-to": "Patrons#delete", "operationId": "deletePatron", diff --git a/t/db_dependent/api/v1/patrons.t b/t/db_dependent/api/v1/patrons.t index 4f96a12430..5e66783bae 100644 --- a/t/db_dependent/api/v1/patrons.t +++ b/t/db_dependent/api/v1/patrons.t @@ -17,7 +17,7 @@ use Modern::Perl; -use Test::More tests => 5; +use Test::More tests => 6; use Test::Mojo; use Test::Warn; @@ -313,6 +313,70 @@ subtest 'update() tests' => sub { }; }; +subtest 'patch() tests' => sub { + plan tests => 2; + + $schema->storage->txn_begin; + unauthorized_access_tests('PATCH', 123, {email => 'nobody@example.com'}); + $schema->storage->txn_rollback; + + subtest 'librarian access tests' => sub { + plan tests => 15; + + $schema->storage->txn_begin; + + t::lib::Mocks::mock_preference('minPasswordLength', 1); + my ( $patron_id_1, $session_id ) = create_user_and_session({ authorized => 1 }); + my ( $patron_id_2, undef ) = create_user_and_session({ authorized => 0 }); + + my $patron_1 = Koha::Patrons->find($patron_id_1); + my $patron_2 = Koha::Patrons->find($patron_id_2); + my $newpatron = {firstname => "myfirstname"}; + + my $tx = $t->ua->build_tx(PATCH => "/api/v1/patrons/-1" => json => $newpatron ); + $tx->req->cookies({name => 'CGISESSID', value => $session_id}); + $t->request_ok($tx) + ->status_is(404) + ->json_has('/error', 'Fails when trying to PATCH nonexistent patron'); + + $newpatron->{falseproperty} = "Non existent property"; + $tx = $t->ua->build_tx(PATCH => "/api/v1/patrons/" . $patron_2->id => json => $newpatron ); + $tx->req->cookies({name => 'CGISESSID', value => $session_id}); + $t->request_ok($tx) + ->status_is(400) + ->json_is('/errors/0/message' => + 'Properties not allowed: falseproperty.'); + delete $newpatron->{falseproperty}; + + # Set both cardnumber and userid to already existing values + $newpatron->{cardnumber} = $patron_1->cardnumber; + $newpatron->{userid} = $patron_1->userid; + + $tx = $t->ua->build_tx( PATCH => "/api/v1/patrons/" . $patron_2->id => json => $newpatron ); + $tx->req->cookies({ name => 'CGISESSID', value => $session_id }); + warning_like { + $t->request_ok($tx) + ->status_is(409) + ->json_has( '/error' => "Fails when trying to update to an existing cardnumber or userid") + ->json_is( '/conflict', 'cardnumber' ); } + qr/^DBD::mysql::st execute failed: Duplicate entry '(.*?)' for key 'cardnumber'/; + + delete $newpatron->{cardnumber}; + delete $newpatron->{userid}; + + #Update only firstname field + $tx = $t->ua->build_tx(PATCH => "/api/v1/patrons/" . $patron_2->id => json => $newpatron); + $tx->req->cookies({name => 'CGISESSID', value => $session_id}); + $t->request_ok($tx) + ->status_is(200, 'Patron updated successfully') + ->json_has($newpatron); + is(Koha::Patrons->find( $patron_2->id )->firstname, + $newpatron->{ firstname }, 'Patron is really updated!'); + + $schema->storage->txn_rollback; + }; +}; + subtest 'delete() tests' => sub { plan tests => 2; -- 2.17.1