From 5e511b3f085cc9ca494d156ef7978159980d445f Mon Sep 17 00:00:00 2001 From: Benjamin Rokseth Date: Wed, 27 Apr 2016 13:47:04 +0000 Subject: [PATCH] Bug 16330 - REST API: add routes to add, update and delete patrons This patch adds support for add, edit and delete patrons via REST API. POST /api/v1/patrons Create a new patron PUT /api/v1/patrons/ Update data about patron DEL /api/v1/patrons/ Delete a patron Test plan: 1) Apply this patch (on top of dependent bug 15126) 2) Run tests perl t/db_dependent/api/v1/patrons.t 3) Add a user with proper rights to use the REST API 4) play with your favourite REST client (curl/httpie, etc.): Authenticate with the user created above and get a CGISESSION id. Use the CGISESSION to add, edit and delete patrons via the API. Please note there is no validation of body input in PUT/POST other than branchcode,category,userid,cardnumber. --- Koha/REST/V1/Patron.pm | 125 ++++++++++++++++++++++++++++++++++++ api/v1/swagger.json | 137 ++++++++++++++++++++++++++++++++++++++++ t/db_dependent/api/v1/patrons.t | 134 +++++++++++++++++++++++++++++++++++---- 3 files changed, 382 insertions(+), 14 deletions(-) diff --git a/Koha/REST/V1/Patron.pm b/Koha/REST/V1/Patron.pm index 2851308..83d3957 100644 --- a/Koha/REST/V1/Patron.pm +++ b/Koha/REST/V1/Patron.pm @@ -16,11 +16,15 @@ package Koha::REST::V1::Patron; # 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. use Modern::Perl; +use Switch; use Mojo::Base 'Mojolicious::Controller'; use C4::Auth qw( haspermission ); +use Koha::AuthUtils qw(hash_password); use Koha::Patrons; +use Koha::Patron::Categories; +use Koha::Libraries; sub list { my ($c, $args, $cb) = @_; @@ -55,4 +59,125 @@ sub get { return $c->$cb($patron->unblessed, 200); } +sub add { + my ($c, $args, $cb) = @_; + + my $user = $c->stash('koha.user'); + + unless ( $user && haspermission($user->userid, {borrowers => 1}) ) { + return $c->$cb({error => "You don't have the required permission"}, 403); + } + + my $body = $c->req->json; + + # patron cardnumber and/or userid unique? + if ($body->{cardnumber} || $body->{userid}) { + my $patron = Koha::Patrons->find({cardnumber => $body->{cardnumber}, userid => $body->{userid} }); + if ($patron) { + return $c->$cb({ + error => "Patron cardnumber and userid must be unique", + conflict => { cardnumber => $patron->cardnumber, userid => $patron->userid } + }, 409); + } + } + + my $branch = Koha::Libraries->find({branchcode => $body->{branchcode} }); + unless ($branch) { + return $c->$cb({error => "Library with branchcode \"" . $body->{branchcode} . "\" does not exist"}, 404); + } + my $category = Koha::Patron::Categories->find({ categorycode => $body->{categorycode} }); + unless ($category) { + return $c->$cb({error => "Patron category \"" . $body->{categorycode} . "\" does not exist"}, 404); + } + # All OK - save new patron + + if ($body->{password}) { $body->{password} = hash_password($body->{password}) }; # bcrypt password if given + my $patron = Koha::Patron->new($body)->store; + + unless ($patron) { + return $c->$cb({error => "Something went wrong"}, 404); + } + + return $c->$cb($patron->unblessed, 201); +} + +sub edit { + my ($c, $args, $cb) = @_; + + my $user = $c->stash('koha.user'); + + unless ( $user && haspermission($user->userid, {borrowers => 1}) ) { + return $c->$cb({error => "You don't have the required permission"}, 403); + } + + my $patron = Koha::Patrons->find($args->{borrowernumber}); + + unless ($patron) { + return $c->$cb({error => "Patron not found"}, 404); + } + + my $body = $c->req->json; + + # Can we change userid and/or cardnumber? in that case check that they are altered first + if ($body->{cardnumber} || $body->{userid}) { + if ( ($body->{cardnumber} && $body->{cardnumber} ne $patron->cardnumber) || ($body->{userid} && $body->{userid} ne $patron->userid) ) { + my $conflictingPatron = Koha::Patrons->find({cardnumber => $body->{cardnumber}, userid => $body->{userid} }); + if ($conflictingPatron) { + return $c->$cb({ + error => "Patron cardnumber and userid must be unique", + conflict => { cardnumber => $conflictingPatron->cardnumber, userid => $conflictingPatron->userid } + }, 409); + } + } + } + + if ($body->{branchcode}) { + my $branch = Koha::Libraries->find({branchcode => $body->{branchcode} }); + unless ($branch) { + return $c->$cb({error => "Library with branchcode \"" . $body->{branchcode} . "\" does not exist"}, 404); + } + } + + if ($body->{categorycode}) { + my $category = Koha::Patron::Categories->find({ categorycode => $body->{categorycode} }); + unless ($category) { + return $c->$cb({error => "Patron category \"" . $body->{categorycode} . "\" does not exist"}, 404); + } + } + # ALL OK - Update patron + # Perhaps limit/validate what should be updated here? flags, et.al. + if ($body->{password}) { $body->{password} = hash_password($body->{password}) }; # bcrypt password if given + my $updatedpatron = $patron->set($body); + if ($updatedpatron->is_changed) { + my $res = $updatedpatron->store; + return $c->$cb($res->unblessed, 200); + } else { + return $c->$cb({}, 204); # No Content = No changes made + } +} + +sub delete { + my ($c, $args, $cb) = @_; + my $user = $c->stash('koha.user'); + + unless ( $user && haspermission($user->userid, {borrowers => 1}) ) { + return $c->$cb({error => "You don't have the required permission"}, 403); + } + + my $patron = Koha::Patrons->find($args->{borrowernumber}); + + unless ($patron) { + return $c->$cb({error => "Patron not found"}, 404); + } + + # check if loans, reservations, debarrment, etc. before deletion! + my $res = $patron->delete; + switch ($res) { + case '1' { return $c->$cb({}, 200); } + case '0' { return $c->$cb({}, 400); } + case '-1' { return $c->$cb({}, 404); } + else { return $c->$cb({}, 400); } + } +} + 1; diff --git a/api/v1/swagger.json b/api/v1/swagger.json index e821c44..48dd229 100644 --- a/api/v1/swagger.json +++ b/api/v1/swagger.json @@ -38,6 +38,55 @@ } } } + }, + "post": { + "operationId": "addPatron", + "tags": ["patrons"], + "parameters": [{ + "name": "body", + "in": "body", + "description": "A JSON object containing information about the new patron", + "required": true, + "schema": { + "$ref": "#/definitions/patron" + } + }], + "consumes": ["application/json"], + "produces": ["application/json"], + "responses": { + "201": { + "description": "A successfully created patron", + "schema": { + "items": { + "$ref": "#/definitions/patron" + } + } + }, + "403": { + "description": "Access forbidden", + "schema": { + "$ref": "#/definitions/error" + } + }, + "404": { + "description": "Resource not found", + "schema": { + "$ref": "#/definitions/error" + } + }, + "409": { + "description": "Conflict in creating resource", + "schema": { + "$ref": "#/definitions/error" + } + }, + "500": { + "description": "Internal error", + "schema": { + "$ref": "#/definitions/error" + } + } + } } }, "/patrons/{borrowernumber}": { @@ -72,6 +121,94 @@ } } } + }, + "put": { + "operationId": "editPatron", + "tags": ["patrons"], + "parameters": [ + { "$ref": "#/parameters/borrowernumberPathParam" }, + { + "name": "body", + "in": "body", + "description": "A JSON object containing new information about existing patron", + "required": true, + "schema": { + "$ref": "#/definitions/patron" + } + } + ], + "consumes": ["application/json"], + "produces": ["application/json"], + "responses": { + "200": { + "description": "A successfully updated patron", + "schema": { + "items": { + "$ref": "#/definitions/patron" + } + } + }, + "204": { + "description": "No Content", + "schema": { + "type": "object" + } + }, + "403": { + "description": "Access forbidden", + "schema": { + "$ref": "#/definitions/error" + } + }, + "404": { + "description": "Resource not found", + "schema": { + "$ref": "#/definitions/error" + } + }, + "409": { + "description": "Conflict in updating resource", + "schema": { + "$ref": "#/definitions/error" + } + }, + "500": { + "description": "Internal error", + "schema": { + "$ref": "#/definitions/error" + } + } + } + }, + "delete": { + "operationId": "deletePatron", + "tags": ["patrons"], + "parameters": [ + { "$ref": "#/parameters/borrowernumberPathParam" } + ], + "produces": ["application/json"], + "responses": { + "200": { + "description": "Patron deleted successfully", + "schema": { + "type": "object" + } + }, + "400": { + "description": "Patron deletion failed", + "schema": { "$ref": "#/definitions/error" } + }, + "403": { + "description": "Access forbidden", + "schema": { + "$ref": "#/definitions/error" + } + }, + "404": { + "description": "Patron not found", + "schema": { "$ref": "#/definitions/error" } + } + } } } }, diff --git a/t/db_dependent/api/v1/patrons.t b/t/db_dependent/api/v1/patrons.t index 6bd4e1a..c2fb311 100644 --- a/t/db_dependent/api/v1/patrons.t +++ b/t/db_dependent/api/v1/patrons.t @@ -17,28 +17,32 @@ use Modern::Perl; -use Test::More tests => 10; -use Test::Mojo; use t::lib::TestBuilder; +use Test::More tests => 50; +use Test::Mojo; + use C4::Auth; use C4::Context; - use Koha::Database; -use Koha::Patron; -my $builder = t::lib::TestBuilder->new(); +BEGIN { + use_ok('Koha::Object'); + use_ok('Koha::Patron'); +} -my $dbh = C4::Context->dbh; -$dbh->{AutoCommit} = 0; -$dbh->{RaiseError} = 1; +my $schema = Koha::Database->schema; +my $dbh = C4::Context->dbh; +my $builder = t::lib::TestBuilder->new; $ENV{REMOTE_ADDR} = '127.0.0.1'; my $t = Test::Mojo->new('Koha::REST::V1'); +$schema->storage->txn_begin; + my $categorycode = $builder->build({ source => 'Category' })->{ categorycode }; my $branchcode = $builder->build({ source => 'Branch' })->{ branchcode }; -my $borrower = $builder->build({ +my $patron = $builder->build({ source => 'Borrower', value => { branchcode => $branchcode, @@ -49,7 +53,7 @@ my $borrower = $builder->build({ $t->get_ok('/api/v1/patrons') ->status_is(403); -$t->get_ok("/api/v1/patrons/" . $borrower->{ borrowernumber }) +$t->get_ok("/api/v1/patrons/" . $patron->{ borrowernumber }) ->status_is(403); my $loggedinuser = $builder->build({ @@ -74,11 +78,113 @@ $tx->req->env({REMOTE_ADDR => '127.0.0.1'}); $t->request_ok($tx) ->status_is(200); -$tx = $t->ua->build_tx(GET => "/api/v1/patrons/" . $borrower->{ borrowernumber }); +$tx = $t->ua->build_tx(GET => "/api/v1/patrons/" . $patron->{ borrowernumber }); $tx->req->cookies({name => 'CGISESSID', value => $session->id}); $t->request_ok($tx) ->status_is(200) - ->json_is('/borrowernumber' => $borrower->{ borrowernumber }) - ->json_is('/surname' => $borrower->{ surname }); + ->json_is('/borrowernumber' => $patron->{ borrowernumber }) + ->json_is('/surname' => $patron->{ surname }); + +### POST /api/v1/patrons + +my $newpatron = { + branchcode => $branchcode, + categorycode => $categorycode, + surname => "TestUser", + cardnumber => "123456", + userid => "testuser" +}; + +$newpatron->{ branchcode } = "nonexistent"; # Test invalid branchcode +$tx = $t->ua->build_tx(POST => "/api/v1/patrons" => json => $newpatron); +$tx->req->cookies({name => 'CGISESSID', value => $session->id}); +$t->request_ok($tx) + ->status_is(404) + ->json_is('/error' => "Library with branchcode \"nonexistent\" does not exist"); + +$newpatron->{ branchcode } = $branchcode; +$newpatron->{ categorycode } = "nonexistent"; # Test invalid patron category +$tx = $t->ua->build_tx(POST => "/api/v1/patrons" => json => $newpatron); +$tx->req->cookies({name => 'CGISESSID', value => $session->id}); +$t->request_ok($tx) + ->status_is(404) + ->json_is('/error' => "Patron category \"nonexistent\" does not exist"); + +$newpatron->{ categorycode } = $categorycode; +$tx = $t->ua->build_tx(POST => "/api/v1/patrons" => json => $newpatron); +$tx->req->cookies({name => 'CGISESSID', value => $session->id}); +$t->request_ok($tx) + ->status_is(201, 'Patron created successfully') + ->json_has('/borrowernumber', 'got a borrowernumber') + ->json_is('/cardnumber', $newpatron->{ cardnumber }) + ->json_is('/surname' => $newpatron->{ surname }) + ->json_is('/firstname' => $newpatron->{ firstname }); +$newpatron->{borrowernumber} = $tx->res->json->{borrowernumber}; + +$tx = $t->ua->build_tx(POST => "/api/v1/patrons" => json => $newpatron); +$tx->req->cookies({name => 'CGISESSID', value => $session->id}); +$t->request_ok($tx) + ->status_is(409) + ->json_has('/error', 'Fails when trying to POST duplicate cardnumber or userid') + ->json_has('/conflict', { userid => $newpatron->{ userid }, cardnumber => $newpatron->{ cardnumber } }); + +### PUT /api/v1/patrons +$tx = $t->ua->build_tx(PUT => "/api/v1/patrons/0" => json => {}); +$tx->req->cookies({name => 'CGISESSID', value => $session->id}); +$t->request_ok($tx) + ->status_is(404) + ->json_has('/error', 'Fails when trying to PUT nonexistent patron'); + +$tx = $t->ua->build_tx(PUT => "/api/v1/patrons/" . $newpatron->{ borrowernumber } => json => {categorycode => "nonexistent"}); +$tx->req->cookies({name => 'CGISESSID', value => $session->id}); +$t->request_ok($tx) + ->status_is(404) + ->json_is('/error' => "Patron category \"nonexistent\" does not exist"); + +$tx = $t->ua->build_tx(PUT => "/api/v1/patrons/" . $newpatron->{ borrowernumber } => json => {branchcode => "nonexistent"}); +$tx->req->cookies({name => 'CGISESSID', value => $session->id}); +$t->request_ok($tx) + ->status_is(404) + ->json_is('/error' => "Library with branchcode \"nonexistent\" does not exist"); + +$newpatron->{ cardnumber } = $patron-> { cardnumber }; +$newpatron->{ userid } = $patron-> { userid }; + +$tx = $t->ua->build_tx(PUT => "/api/v1/patrons/" . $newpatron->{ borrowernumber } => json => $newpatron); +$tx->req->cookies({name => 'CGISESSID', value => $session->id}); +$t->request_ok($tx) + ->status_is(409) + ->json_has('/error' => "Fails when trying to update to an existing cardnumber or userid") + ->json_has('/conflict', { cardnumber => $patron->{ cardnumber }, userid => $patron->{ userid } }); + +$newpatron->{ cardnumber } = "123456"; +$newpatron->{ userid } = "testuser"; +$tx = $t->ua->build_tx(PUT => "/api/v1/patrons/" . $newpatron->{ borrowernumber } => json => $newpatron); +$tx->req->cookies({name => 'CGISESSID', value => $session->id}); +$t->request_ok($tx) + ->status_is(204, 'No changes - patron NOT updated'); + +$newpatron->{ cardnumber } = "234567"; +$newpatron->{ userid } = "updatedtestuser"; +$newpatron->{ surname } = "UpdatedTestUser"; + +$tx = $t->ua->build_tx(PUT => "/api/v1/patrons/" . $newpatron->{ borrowernumber } => json => $newpatron); +$tx->req->cookies({name => 'CGISESSID', value => $session->id}); +$t->request_ok($tx) + ->status_is(200, 'Patron updated successfully') + ->json_has($newpatron); + +### DELETE /api/v1/patrons + +$tx = $t->ua->build_tx(DELETE => "/api/v1/patrons/0"); +$tx->req->cookies({name => 'CGISESSID', value => $session->id}); +$t->request_ok($tx) + ->status_is(404, 'Patron not found'); + +$tx = $t->ua->build_tx(DELETE => "/api/v1/patrons/" . $newpatron->{ borrowernumber }); +$tx->req->cookies({name => 'CGISESSID', value => $session->id}); +$t->request_ok($tx) + ->status_is(200, 'Patron deleted successfully'); + +$schema->storage->txn_rollback; -$dbh->rollback; -- 2.1.4