From 692215baaabf6850e59974cac77509f583b45a26 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. GET /api/v1/patrons Get patron list from params GET /api/v1/patrons/ Get single patron 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. Signed-off-by: Bernardo Gonzalez Kriegel On top of 15126 Tested using postman, first try with the api GET, POST, PUT and DELETE works, i.e. list, add, update, search and delete. Small errors fixed in followup. --- Koha/REST/V1/Patron.pm | 157 ++++++++++++++++++++++++++++++++++++- api/v1/swagger.json | 137 ++++++++++++++++++++++++++++++++ t/db_dependent/api/v1/patrons.t | 167 ++++++++++++++++++++++++++++++++++++---- 3 files changed, 446 insertions(+), 15 deletions(-) diff --git a/Koha/REST/V1/Patron.pm b/Koha/REST/V1/Patron.pm index 2851308..0411719 100644 --- a/Koha/REST/V1/Patron.pm +++ b/Koha/REST/V1/Patron.pm @@ -20,7 +20,10 @@ use Modern::Perl; 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) = @_; @@ -30,7 +33,17 @@ sub list { return $c->$cb({error => "You don't have the required permission"}, 403); } - my $patrons = Koha::Patrons->search; + my $params = $c->req->query_params->to_hash; + my $patrons; + if (keys %$params) { + my @valid_params = Koha::Patrons->_resultset->result_source->columns; + foreach my $key (keys %$params) { + delete $params->{$key} unless grep { $key eq $_ } @valid_params; + } + $patrons = Koha::Patrons->search($params); + } else { + $patrons = Koha::Patrons->search; + } $c->$cb($patrons->unblessed, 200); } @@ -55,4 +68,146 @@ 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 = eval { + Koha::Patron->new($body)->store; + }; + + unless ($patron) { + return $c->$cb({error => "Something went wrong, check Koha logs for details"}, 500); + } + + 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 = eval { + $patron->set($body); + }; + + if ($updatedpatron) { + if ($updatedpatron->is_changed) { + + my $res = eval { + $updatedpatron->store; + }; + + unless ($res) { + return $c->$cb({error => "Something went wrong, check Koha logs for details"}, 500); + } + return $c->$cb($res->unblessed, 200); + + } else { + return $c->$cb({}, 204); # No Content = No changes made + } + } else { + return $c->$cb({error => "Something went wrong, check Koha logs for details"}, 500); + } +} + +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; + + if ($res eq '1') { + return $c->$cb({}, 200); + } elsif ($res eq '-1') { + return $c->$cb({}, 404); + } else { + return $c->$cb({}, 400); + } +} + 1; diff --git a/api/v1/swagger.json b/api/v1/swagger.json index 1488aba..7e122d5 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" } + } + } } }, "/holds": { diff --git a/t/db_dependent/api/v1/patrons.t b/t/db_dependent/api/v1/patrons.t index 6bd4e1a..bcd8359 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 => 56; +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, @@ -46,10 +50,12 @@ my $borrower = $builder->build({ } }); +### GET /api/v1/patrons + $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({ @@ -73,12 +79,145 @@ $tx->req->cookies({name => 'CGISESSID', value => $session->id}); $tx->req->env({REMOTE_ADDR => '127.0.0.1'}); $t->request_ok($tx) ->status_is(200); +ok(@{$tx->res->json} >= 1, 'Json response lists all when no params given'); + +$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' => $patron->{ borrowernumber }) + ->json_is('/surname' => $patron->{ surname }); + +$tx = $t->ua->build_tx(GET => '/api/v1/patrons' => form => {surname => 'nonexistent'}); +$tx->req->cookies({name => 'CGISESSID', value => $session->id}); +$tx->req->env({REMOTE_ADDR => '127.0.0.1'}); +$t->request_ok($tx) + ->status_is(200); +ok(@{$tx->res->json} == 0, 'Json response yields no results when params doesnt match'); -$tx = $t->ua->build_tx(GET => "/api/v1/patrons/" . $borrower->{ borrowernumber }); +$tx = $t->ua->build_tx(GET => '/api/v1/patrons' => form => {surname => $patron->{ surname }}); $tx->req->cookies({name => 'CGISESSID', value => $session->id}); +$tx->req->env({REMOTE_ADDR => '127.0.0.1'}); $t->request_ok($tx) ->status_is(200) - ->json_is('/borrowernumber' => $borrower->{ borrowernumber }) - ->json_is('/surname' => $borrower->{ surname }); + ->json_has($patron); +ok(@{$tx->res->json} == 1, 'Json response yields expected results when params match'); + +### 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; + +$newpatron->{ falseproperty } = "Non existent property"; +$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(500) + ->json_is('/error' => "Something went wrong, check Koha logs for details"); + +delete $newpatron->{ falseproperty }; +$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->{ falseproperty } = "Non existent property"; +$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(500) + ->json_is('/error' => "Something went wrong, check Koha logs for details"); +delete $newpatron->{ falseproperty }; + +$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