Browse Source

Bug 16330: 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/<borrowernumber>  Get single patron
POST /api/v1/patrons                   Create a new patron
PUT  /api/v1/patrons/<borrowernumber>  Update data about patron
DEL  /api/v1/patrons/<borrowernumber>  Delete a patron

Revised Test plan:
1) Apply this patch
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.
5) Use PUT /patrons/<borrowernumber> for a patron without borrowers
   flag. This should go into pending patron modification status and
   needs to be accepted by a librarian.

Please note there is no validation of body input in PUT/POST other
than branchcode,category,userid,cardnumber.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
18.05.x
Benjamin Rokseth 7 years ago
committed by Jonathan Druart
parent
commit
7b8909cb90
  1. 8
      Koha/Exceptions.pm
  2. 17
      Koha/Exceptions/Category.pm
  3. 17
      Koha/Exceptions/Library.pm
  4. 17
      Koha/Exceptions/Patron.pm
  5. 127
      Koha/Patron.pm
  6. 166
      Koha/REST/V1/Patron.pm
  7. 14
      api/v1/swagger/definitions/patron.json
  8. 708
      api/v1/swagger/paths/patrons.json
  9. 94
      t/db_dependent/Koha/Patrons.t
  10. 494
      t/db_dependent/api/v1/patrons.t

8
Koha/Exceptions.pm

@ -10,8 +10,8 @@ use Exception::Class (
},
'Koha::Exceptions::BadParameter' => {
isa => 'Koha::Exceptions::Exception',
description => 'Bad parameter was given',
fields => ["parameter"],
description => 'A bad parameter was given',
fields => ['parameter'],
},
'Koha::Exceptions::DuplicateObject' => {
isa => 'Koha::Exceptions::Exception',
@ -29,6 +29,10 @@ use Exception::Class (
isa => 'Koha::Exceptions::Exception',
description => 'A required parameter is missing'
},
'Koha::Exceptions::NoChanges' => {
isa => 'Koha::Exceptions::Exception',
description => 'No changes were made',
},
'Koha::Exceptions::WrongParameter' => {
isa => 'Koha::Exceptions::Exception',
description => 'One or more parameters are wrong',

17
Koha/Exceptions/Category.pm

@ -0,0 +1,17 @@
package Koha::Exceptions::Category;
use Modern::Perl;
use Exception::Class (
'Koha::Exceptions::Category' => {
description => 'Something went wrong!',
},
'Koha::Exceptions::Category::CategorycodeNotFound' => {
isa => 'Koha::Exceptions::Category',
description => "Category does not exist",
fields => ["categorycode"],
},
);
1;

17
Koha/Exceptions/Library.pm

@ -0,0 +1,17 @@
package Koha::Exceptions::Library;
use Modern::Perl;
use Exception::Class (
'Koha::Exceptions::Library' => {
description => 'Something went wrong!',
},
'Koha::Exceptions::Library::BranchcodeNotFound' => {
isa => 'Koha::Exceptions::Library',
description => "Library does not exist",
fields => ["branchcode"],
},
);
1;

17
Koha/Exceptions/Patron.pm

@ -0,0 +1,17 @@
package Koha::Exceptions::Patron;
use Modern::Perl;
use Exception::Class (
'Koha::Exceptions::Patron' => {
description => 'Something went wrong!',
},
'Koha::Exceptions::Patron::DuplicateObject' => {
isa => 'Koha::Exceptions::Patron',
description => "Patron cardnumber and userid must be unique",
fields => ["conflict"],
},
);
1;

127
Koha/Patron.pm

@ -2,6 +2,7 @@ package Koha::Patron;
# Copyright ByWater Solutions 2014
# Copyright PTFS Europe 2016
# Copyright Koha-Suomi Oy 2017
#
# This file is part of Koha.
#
@ -30,6 +31,11 @@ use Koha::Database;
use Koha::DateUtils;
use Koha::Holds;
use Koha::Old::Checkouts;
use Koha::Exceptions;
use Koha::Exceptions::Category;
use Koha::Exceptions::Library;
use Koha::Exceptions::Patron;
use Koha::Libraries;
use Koha::Patron::Categories;
use Koha::Patron::HouseboundProfile;
use Koha::Patron::HouseboundRole;
@ -831,6 +837,18 @@ sub is_child {
return $self->category->category_type eq 'C' ? 1 : 0;
}
=head3 store
=cut
sub store {
my ($self) = @_;
# $self->_validate();
return $self->SUPER::store();
}
=head3 type
=cut
@ -839,6 +857,115 @@ sub _type {
return 'Borrower';
}
=head2 Internal methods
=head3 _check_branchcode
Checks the existence of patron's branchcode and throws
Koha::Exceptions::Library::BranchcodeNotFound if branchcode is not found.
=cut
sub _check_branchcode {
my ($self) = @_;
return unless $self->branchcode;
unless (Koha::Libraries->find($self->branchcode)) {
Koha::Exceptions::Library::BranchcodeNotFound->throw(
error => "Library does not exist",
branchcode => $self->branchcode,
);
}
return 1;
}
=head3 _check_categorycode
Checks the existence of patron's categorycode and throws
Koha::Exceptions::Category::CategorycodeNotFound if categorycode is not found.
=cut
sub _check_categorycode {
my ($self) = @_;
return unless $self->categorycode;
unless (Koha::Patron::Categories->find($self->categorycode)) {
Koha::Exceptions::Category::CategorycodeNotFound->throw(
error => "Patron category does not exist",
categorycode => $self->categorycode,
);
}
return 1;
}
=head3 _check_uniqueness
Checks patron's cardnumber and userid for uniqueness and throws
Koha::Exceptions::Patron::DuplicateObject if conflicting with another patron.
=cut
sub _check_uniqueness {
my ($self) = @_;
my $select = {};
$select->{cardnumber} = $self->cardnumber if $self->cardnumber;
$select->{userid} = $self->userid if $self->userid;
return unless keys %$select;
# Find conflicting patrons
my $patrons = Koha::Patrons->search({
'-or' => $select
});
if ($patrons->count) {
my $conflict = {};
foreach my $patron ($patrons->as_list) {
# New patron $self: a conflicting patron $patron found.
# Updating patron $self: first make sure conflicting patron $patron is
# not this patron $self.
if (!$self->in_storage || $self->in_storage &&
$self->borrowernumber != $patron->borrowernumber) {
# Populate conflict information to exception
if ($patron->cardnumber && $self->cardnumber &&
$patron->cardnumber eq $self->cardnumber)
{
$conflict->{cardnumber} = $self->cardnumber;
}
if ($patron->userid && $self->userid &&
$patron->userid eq $self->userid)
{
$conflict->{userid} = $self->userid;
}
}
}
Koha::Exceptions::Patron::DuplicateObject->throw(
error => "Patron data conflicts with another patron",
conflict => $conflict
) if keys %$conflict;
}
return 1;
}
=head3 _validate
Performs a set of validations on this object and throws Koha::Exceptions if errors
are found.
=cut
sub _validate {
my ($self) = @_;
$self->_check_branchcode;
$self->_check_categorycode;
$self->_check_uniqueness;
return $self;
}
=head1 AUTHOR
Kyle M Hall <kyle@bywatersolutions.com>

166
Koha/REST/V1/Patron.pm

@ -19,15 +19,38 @@ use Modern::Perl;
use Mojo::Base 'Mojolicious::Controller';
use C4::Members qw( AddMember ModMember );
use Koha::AuthUtils qw(hash_password);
use Koha::Patrons;
use Koha::Patron::Categories;
use Koha::Patron::Modifications;
use Koha::Libraries;
use Scalar::Util qw(blessed);
use Try::Tiny;
sub list {
my $c = shift->openapi->valid_input or return;
# FIXME The limited does not work here, the userenv is not set
my $patrons = Koha::Patrons->search_limited;
return $c->render(status => 200, openapi => $patrons);
my $args = $c->req->params->to_hash;
my $filter = {};
for my $filter_param ( keys %$args ) {
$filter->{$filter_param} = { LIKE => $args->{$filter_param} . "%" };
}
return try {
my $patrons = Koha::Patrons->search_limited($filter);
return $c->render(status => 200, openapi => $patrons);
}
catch {
if ( $_->isa('DBIx::Class::Exception') ) {
return $c->render( status => 500, openapi => { error => $_->{msg} } );
}
else {
return $c->render( status => 500, openapi => { error => "Something went wrong, check the logs." } );
}
};
}
sub get {
@ -35,6 +58,7 @@ sub get {
my $borrowernumber = $c->validation->param('borrowernumber');
my $patron = Koha::Patrons->find($borrowernumber);
unless ($patron) {
return $c->render(status => 404, openapi => { error => "Patron not found." });
}
@ -42,4 +66,140 @@ sub get {
return $c->render(status => 200, openapi => $patron);
}
sub add {
my ($c, $args, $cb) = @_;
return try {
my $body = $c->req->json;
Koha::Patron->new($body)->_validate;
# TODO: Use AddMember until it has been moved to Koha-namespace
my $borrowernumber = AddMember(%$body);
my $patron = Koha::Patrons->find($borrowernumber);
return $c->$cb($patron, 201);
}
catch {
unless (blessed $_ && $_->can('rethrow')) {
return $c->$cb({ error =>
"Something went wrong, check Koha logs for details."}, 500);
}
if ($_->isa('Koha::Exceptions::Patron::DuplicateObject')) {
return $c->$cb({ error => $_->error, conflict => $_->conflict }, 409);
}
elsif ($_->isa('Koha::Exceptions::Library::BranchcodeNotFound')) {
return $c->$cb({ error => "Given branchcode does not exist" }, 400);
}
elsif ($_->isa('Koha::Exceptions::Category::CategorycodeNotFound')) {
return $c->$cb({ error => "Given categorycode does not exist"}, 400);
}
else {
return $c->$cb({ error =>
"Something went wrong, check Koha logs for details."}, 500);
}
};
}
sub edit {
my ($c, $args, $cb) = @_;
my $patron;
return try {
my $user = $c->stash('koha.user');
$patron = Koha::Patrons->find($args->{borrowernumber});
my $body = $c->req->json;
$body->{borrowernumber} = $args->{borrowernumber};
if (!C4::Auth::haspermission($user->userid, { borrowers => 1 }) &&
$user->borrowernumber == $patron->borrowernumber){
if (C4::Context->preference('OPACPatronDetails')) {
$body = _delete_unmodifiable_parameters($body);
die unless $patron->set($body)->_validate;
my $m = Koha::Patron::Modification->new($body)->store();
return $c->$cb({}, 202);
} else {
return $c->$cb({ error => "You need a permission to change"
." Your personal details"}, 403);
}
}
else {
delete $body->{borrowernumber};
die unless $patron->set($body)->_validate;
# TODO: Use ModMember until it has been moved to Koha-namespace
$body->{borrowernumber} = $args->{borrowernumber};
die unless ModMember(%$body);
return $c->$cb($patron, 200);
}
}
catch {
unless ($patron) {
return $c->$cb({error => "Patron not found"}, 404);
}
unless (blessed $_ && $_->can('rethrow')) {
return $c->$cb({ error =>
"Something went wrong, check Koha logs for details."}, 500);
}
if ($_->isa('Koha::Exceptions::Patron::DuplicateObject')) {
return $c->$cb({ error => $_->error, conflict => $_->conflict }, 409);
}
elsif ($_->isa('Koha::Exceptions::Library::BranchcodeNotFound')) {
return $c->$cb({ error => "Given branchcode does not exist" }, 400);
}
elsif ($_->isa('Koha::Exceptions::Category::CategorycodeNotFound')) {
return $c->$cb({ error => "Given categorycode does not exist"}, 400);
}
elsif ($_->isa('Koha::Exceptions::MissingParameter')) {
return $c->$cb({error => "Missing mandatory parameter(s)",
parameters => $_->parameter }, 400);
}
elsif ($_->isa('Koha::Exceptions::BadParameter')) {
return $c->$cb({error => "Invalid parameter(s)",
parameters => $_->parameter }, 400);
}
elsif ($_->isa('Koha::Exceptions::NoChanges')) {
return $c->$cb({error => "No changes have been made"}, 204);
}
else {
return $c->$cb({ error =>
"Something went wrong, check Koha logs for details."}, 500);
}
};
}
sub delete {
my ($c, $args, $cb) = @_;
my $patron;
return try {
$patron = Koha::Patrons->find($args->{borrowernumber});
# check if loans, reservations, debarrment, etc. before deletion!
my $res = $patron->delete;
return $c->$cb({}, 200);
}
catch {
unless ($patron) {
return $c->$cb({error => "Patron not found"}, 404);
}
else {
return $c->$cb({ error =>
"Something went wrong, check Koha logs for details."}, 500);
}
};
}
sub _delete_unmodifiable_parameters {
my ($body) = @_;
my %columns = map { $_ => 1 } Koha::Patron::Modifications->columns;
foreach my $param (keys %$body) {
unless (exists $columns{$param}) {
delete $body->{$param};
}
}
return $body;
}
1;

14
api/v1/swagger/definitions/patron.json

@ -121,6 +121,7 @@
},
"dateofbirth": {
"type": ["string", "null"],
"format": "date",
"description": "patron's date of birth"
},
"branchcode": {
@ -133,10 +134,12 @@
},
"dateenrolled": {
"type": ["string", "null"],
"format": "date",
"description": "date the patron was added to Koha"
},
"dateexpiry": {
"type": ["string", "null"],
"format": "date",
"description": "date the patron's card is set to expire"
},
"date_renewed": {
@ -153,6 +156,7 @@
},
"debarred": {
"type": ["string", "null"],
"format": "date",
"description": "until this date the patron can only check-in"
},
"debarredcomment": {
@ -272,15 +276,17 @@
"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", "null"],
"type": "string",
"format": "date-time",
"description": "time of last change could be useful for synchronization with external systems (among others)"
},
"lastseen": {
"type": ["string", "null"],
"format": "date-time",
"description": "last time a patron has been seen (connected at the OPAC or staff interface)"
},
"lang": {
"type": ["string", "null"],
"type": "string",
"description": "lang to use to send notices to this patron"
},
"login_attempts": {
@ -291,5 +297,7 @@
"type": ["string", "null"],
"description": "persist OverDrive auth token"
}
}
},
"additionalProperties": false,
"required": ["surname", "address", "city", "branchcode", "categorycode"]
}

708
api/v1/swagger/paths/patrons.json

@ -7,6 +7,517 @@
"produces": [
"application/json"
],
"parameters": [{
"name": "borrowernumber",
"in": "query",
"description": "Case insensetive 'starts_with' search on borrowernumber",
"required": false,
"type": "string"
},
{
"name": "cardnumber",
"in": "query",
"description": "Case insensetive 'starts_with' search on cardnumber",
"required": false,
"type": "string"
},
{
"name": "surname",
"in": "query",
"description": "Case insensetive 'starts_with' search on surname",
"required": false,
"type": "string"
},
{
"name": "firstname",
"in": "query",
"description": "Case insensetive 'starts_with' search on firstname",
"required": false,
"type": "string"
},
{
"name": "title",
"in": "query",
"description": "Case insensetive 'starts_with' search on title",
"required": false,
"type": "string"
},
{
"name": "othernames",
"in": "query",
"description": "Case insensetive 'starts_with' search on othernames",
"required": false,
"type": "string"
},
{
"name": "initials",
"in": "query",
"description": "Case insensetive 'starts_with' search on initials",
"required": false,
"type": "string"
},
{
"name": "streetnumber",
"in": "query",
"description": "Case insensetive 'starts_with' search on streetnumber",
"required": false,
"type": "string"
},
{
"name": "streettype",
"in": "query",
"description": "Case insensetive 'starts_with' search on streettype",
"required": false,
"type": "string"
},
{
"name": "address",
"in": "query",
"description": "Case insensetive 'starts_with' search on address",
"required": false,
"type": "string"
},
{
"name": "address2",
"in": "query",
"description": "Case insensetive 'starts_with' search on address2",
"required": false,
"type": "string"
},
{
"name": "city",
"in": "query",
"description": "Case insensetive 'starts_with' search on city",
"required": false,
"type": "string"
},
{
"name": "state",
"in": "query",
"description": "Case insensetive 'starts_with' search on state",
"required": false,
"type": "string"
},
{
"name": "zipcode",
"in": "query",
"description": "Case insensetive 'starts_with' search on zipcode",
"required": false,
"type": "string"
},
{
"name": "country",
"in": "query",
"description": "Case insensetive 'starts_with' search on country",
"required": false,
"type": "string"
},
{
"name": "email",
"in": "query",
"description": "Case insensetive 'starts_with' search on email",
"required": false,
"type": "string"
},
{
"name": "phone",
"in": "query",
"description": "Case insensetive 'starts_with' search on phone",
"required": false,
"type": "string"
},
{
"name": "mobile",
"in": "query",
"description": "Case insensetive 'starts_with' search on mobile",
"required": false,
"type": "string"
},
{
"name": "fax",
"in": "query",
"description": "Case insensetive 'starts_with' search on fax",
"required": false,
"type": "string"
},
{
"name": "emailpro",
"in": "query",
"description": "Case insensetive 'starts_with' search on emailpro",
"required": false,
"type": "string"
},
{
"name": "phonepro",
"in": "query",
"description": "Case insensetive 'starts_with' search on phonepro",
"required": false,
"type": "string"
},
{
"name": "B_streetnumber",
"in": "query",
"description": "Case insensetive 'starts_with' search on B_streetnumber",
"required": false,
"type": "string"
},
{
"name": "B_streettype",
"in": "query",
"description": "Case insensetive 'starts_with' search on B_streettype",
"required": false,
"type": "string"
},
{
"name": "B_address",
"in": "query",
"description": "Case insensetive 'starts_with' search on B_address",
"required": false,
"type": "string"
},
{
"name": "B_address2",
"in": "query",
"description": "Case insensetive 'starts_with' search on B_address2",
"required": false,
"type": "string"
},
{
"name": "B_city",
"in": "query",
"description": "Case insensetive 'starts_with' search on B_city",
"required": false,
"type": "string"
},
{
"name": "B_state",
"in": "query",
"description": "Case insensetive 'starts_with' search on B_state",
"required": false,
"type": "string"
},
{
"name": "B_zipcode",
"in": "query",
"description": "Case insensetive 'starts_with' search on B_zipcode",
"required": false,
"type": "string"
},
{
"name": "B_country",
"in": "query",
"description": "Case insensetive 'starts_with' search on B_country",
"required": false,
"type": "string"
},
{
"name": "B_email",
"in": "query",
"description": "Case insensetive 'starts_with' search on B_email",
"required": false,
"type": "string"
},
{
"name": "B_phone",
"in": "query",
"description": "Case insensetive 'starts_with' search on B_phone",
"required": false,
"type": "string"
},
{
"name": "dateofbirth",
"in": "query",
"description": "Case insensetive 'starts_with' search on dateofbirth",
"required": false,
"type": "string"
},
{
"name": "branchcode",
"in": "query",
"description": "Case insensetive 'starts_with' search on branchcode",
"required": false,
"type": "string"
},
{
"name": "categorycode",
"in": "query",
"description": "Case insensetive 'starts_with' search on categorycode",
"required": false,
"type": "string"
},
{
"name": "dateenrolled",
"in": "query",
"description": "Case insensetive 'starts_with' search on dateenrolled",
"required": false,
"type": "string"
},
{
"name": "dateexpiry",
"in": "query",
"description": "Case insensetive 'starts_with' search on dateexpiry",
"required": false,
"type": "string"
},
{
"name": "gonenoaddress",
"in": "query",
"description": "Case insensetive 'starts_with' search on gonenoaddress",
"required": false,
"type": "string"
},
{
"name": "lost",
"in": "query",
"description": "Case insensetive 'starts_with' search on lost",
"required": false,
"type": "string"
},
{
"name": "debarred",
"in": "query",
"description": "Case insensetive 'starts_with' search on debarred",
"required": false,
"type": "string"
},
{
"name": "debarredcomment",
"in": "query",
"description": "Case insensetive 'starts_with' search on debarredcomment",
"required": false,
"type": "string"
},
{
"name": "contactname",
"in": "query",
"description": "Case insensetive 'starts_with' search on contactname",
"required": false,
"type": "string"
},
{
"name": "contactfirstname",
"in": "query",
"description": "Case insensetive 'starts_with' search on contactfirstname",
"required": false,
"type": "string"
},
{
"name": "contacttitle",
"in": "query",
"description": "Case insensetive 'starts_with' search on contacttitle",
"required": false,
"type": "string"
},
{
"name": "guarantorid",
"in": "query",
"description": "Case insensetive 'starts_with' search on guarantorid",
"required": false,
"type": "string"
},
{
"name": "borrowernotes",
"in": "query",
"description": "Case insensetive 'starts_with' search on borrowernotes",
"required": false,
"type": "string"
},
{
"name": "relationship",
"in": "query",
"description": "Case insensetive 'starts_with' search on relationship",
"required": false,
"type": "string"
},
{
"name": "sex",
"in": "query",
"description": "Case insensetive 'starts_with' search on sex",
"required": false,
"type": "string"
},
{
"name": "password",
"in": "query",
"description": "Case insensetive 'starts_with' search on password",
"required": false,
"type": "string"
},
{
"name": "flags",
"in": "query",
"description": "Case insensetive 'starts_with' search on flags",
"required": false,
"type": "string"
},
{
"name": "userid",
"in": "query",
"description": "Case insensetive 'starts_with' search on userid",
"required": false,
"type": "string"
},
{
"name": "opacnote",
"in": "query",
"description": "Case insensetive 'starts_with' search on opacnote",
"required": false,
"type": "string"
},
{
"name": "contactnote",
"in": "query",
"description": "Case insensetive 'starts_with' search on contactnote",
"required": false,
"type": "string"
},
{
"name": "sort1",
"in": "query",
"description": "Case insensetive 'starts_with' search on sort1",
"required": false,
"type": "string"
},
{
"name": "sort2",
"in": "query",
"description": "Case insensetive 'starts_with' search on sort2",
"required": false,
"type": "string"
},
{
"name": "altcontactfirstname",
"in": "query",
"description": "Case insensetive 'starts_with' search on altcontactfirstname",
"required": false,
"type": "string"
},
{
"name": "altcontactsurname",
"in": "query",
"description": "Case insensetive 'starts_with' search on altcontactsurname",
"required": false,
"type": "string"
},
{
"name": "altcontactaddress1",
"in": "query",
"description": "Case insensetive 'starts_with' search on altcontactaddress1",
"required": false,
"type": "string"
},
{
"name": "altcontactaddress2",
"in": "query",
"description": "Case insensetive 'starts_with' search on altcontactaddress2",
"required": false,
"type": "string"
},
{
"name": "altcontactaddress3",
"in": "query",
"description": "Case insensetive 'starts_with' search on altcontactaddress3",
"required": false,
"type": "string"
},
{
"name": "altcontactstate",
"in": "query",
"description": "Case insensetive 'starts_with' search on altcontactstate",
"required": false,
"type": "string"
},
{
"name": "altcontactzipcode",
"in": "query",
"description": "Case insensetive 'starts_with' search on altcontactzipcode",
"required": false,
"type": "string"
},
{
"name": "altcontactcountry",
"in": "query",
"description": "Case insensetive 'starts_with' search on altcontactcountry",
"required": false,
"type": "string"
},
{
"name": "altcontactphone",
"in": "query",
"description": "Case insensetive 'starts_with' search on altcontactphone",
"required": false,
"type": "string"
},
{
"name": "smsalertnumber",
"in": "query",
"description": "Case insensetive 'starts_with' search on smsalertnumber",
"required": false,
"type": "string"
},
{
"name": "sms_provider_id",
"in": "query",
"description": "Case insensetive 'starts_with' search on sms_provider_id",
"required": false,
"type": "string"
},
{
"name": "privacy",
"in": "query",
"description": "Case insensetive 'starts_with' search on privacy",
"required": false,
"type": "string"
},
{
"name": "privacy_guarantor_checkouts",
"in": "query",
"description": "Case insensetive 'starts_with' search on privacy_guarantor_checkouts",
"required": false,
"type": "string"
},
{
"name": "checkprevcheckout",
"in": "query",
"description": "Case insensetive 'starts_with' search on checkprevcheckout",
"required": false,
"type": "string"
},
{
"name": "updated_on",
"in": "query",
"description": "Case insensetive 'starts_with' search on updated_on",
"required": false,
"type": "string"
},
{
"name": "lastseen",
"in": "query",
"description": "Case insensetive 'starts_with' search on lastseen",
"required": false,
"type": "string"
},
{
"name": "lang",
"in": "query",
"description": "Case insensetive 'starts_with' search on lang",
"required": false,
"type": "string"
},
{
"name": "login_attempts",
"in": "query",
"description": "Case insensetive 'starts_with' search on login_attempts",
"required": false,
"type": "string"
},
{
"name": "overdrive_auth_token",
"in": "query",
"description": "Case insensetive 'starts_with' search on overdrive_auth_token",
"required": false,
"type": "string"
}],
"responses": {
"200": {
"description": "A list of patrons",
@ -34,12 +545,78 @@
"schema": {
"$ref": "../definitions.json#/error"
}
}
},
"x-koha-authorization": {
"permissions": {
"borrowers": "1"
}
}
},
"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.json#/patron"
}
}],
"consumes": ["application/json"],
"produces": ["application/json"],
"responses": {
"201": {
"description": "A successfully created patron",
"schema": {
"items": {
"$ref": "../definitions.json#/patron"
}
}
},
"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": "Resource not found",
"schema": {
"$ref": "../definitions.json#/error"
}
},
"409": {
"description": "Conflict in creating resource",
"schema": {
"$ref": "../definitions.json#/error"
}
},
"503": {
"description": "Under maintenance",
"schema": {
"$ref": "../definitions.json#/error"
}
},
"500": {
"description": "Internal server error",
"schema": {
"$ref": "../definitions.json#/error"
}
}
},
"x-koha-authorization": {
@ -55,11 +632,10 @@
"operationId": "getPatron",
"tags": ["patrons"],
"parameters": [{
"$ref": "../parameters.json#/borrowernumberPathParam"
}
],
"$ref": "../parameters.json#/borrowernumberPathParam"
}],
"produces": [
"application/json"
"application/json"
],
"responses": {
"200": {
@ -106,6 +682,130 @@
"borrowers": "edit_borrowers"
}
}
},
"put": {
"operationId": "editPatron",
"tags": ["patrons"],
"parameters": [
{
"$ref": "../parameters.json#/borrowernumberPathParam"
},
{
"name": "body",
"in": "body",
"description": "A JSON object containing new information about existing patron",
"required": true,
"schema": {
"$ref": "../definitions.json#/patron"
}
}
],
"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": {
"allow-owner": true,
"allow-guarantor": true,
"permissions": {
"borrowers": "1"
}
}
},
"delete": {
"operationId": "deletePatron",
"tags": ["patrons"],
"parameters": [{
"$ref": "../parameters.json#/borrowernumberPathParam"
}],
"produces": ["application/json"],
"responses": {
"200": {
"description": "Patron deleted successfully",
"schema": {
"type": "object"
}
},
"400": {
"description": "Patron deletion failed",
"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": "Patron not found",
"schema": {
"$ref": "../definitions.json#/error"
}
}
},
"x-koha-authorization": {
"permissions": {
"borrowers": "1"
}
}
}
}
}

94
t/db_dependent/Koha/Patrons.t

@ -1153,3 +1153,97 @@ sub set_logged_in_user {
'', ''
);
}
subtest '_validate() tests' => sub {
plan tests => 4;
$schema->storage->txn_begin;
Koha::Patrons->delete;
my $categorycode = $builder->build({ source => 'Category' })->{categorycode};
my $branchcode = $builder->build({ source => 'Branch' })->{branchcode};
my $patron = $builder->build({
source => 'Borrower',
value => {
branchcode => $branchcode,
cardnumber => 'conflict',
categorycode => $categorycode,
}
});
ok(Koha::Patron->new({
surname => 'Store test',
branchcode => $branchcode,
categorycode => $categorycode
})->_validate->store, 'Stored a patron');
subtest '_check_categorycode' => sub {
plan tests => 2;
my $conflicting = $builder->build({
source => 'Borrower',
value => {
branchcode => $branchcode,
categorycode => 'nonexistent',
}
});
delete $conflicting->{borrowernumber};
eval { Koha::Patron->new($conflicting)->_validate };
isa_ok($@, "Koha::Exceptions::Category::CategorycodeNotFound");
is($@->{categorycode}, $conflicting->{categorycode},
'Exception describes non-existent categorycode');
};
subtest '_check_categorycode' => sub {
plan tests => 2;
my $conflicting = $builder->build({
source => 'Borrower',
value => {
branchcode => 'nonexistent',
categorycode => $categorycode,
}
});
delete $conflicting->{borrowernumber};
eval { Koha::Patron->new($conflicting)->_validate };
isa_ok($@, "Koha::Exceptions::Library::BranchcodeNotFound");
is($@->{branchcode}, $conflicting->{branchcode},
'Exception describes non-existent branchcode');
};
subtest '_check_uniqueness() tests' => sub {
plan tests => 4;
my $conflicting = $builder->build({
source => 'Borrower',
value => {
branchcode => $branchcode,
categorycode => $categorycode,
}
});
delete $conflicting->{borrowernumber};
$conflicting->{cardnumber} = 'conflict';
$conflicting->{userid} = $patron->{userid};
eval { Koha::Patron->new($conflicting)->_validate };
isa_ok($@, "Koha::Exceptions::Patron::DuplicateObject");
is($@->{conflict}->{cardnumber}, $conflicting->{cardnumber},
'Exception describes conflicting cardnumber');
is($@->{conflict}->{userid}, $conflicting->{userid},
'Exception describes conflicting userid');
$conflicting->{cardnumber} = 'notconflicting';
$conflicting->{userid} = 'notconflicting';
ok(Koha::Patron->new($conflicting)->_validate->store, 'After modifying'
.' cardnumber and userid to not conflict with others, no exception.');
};
$schema->storage->txn_rollback;
};

494
t/db_dependent/api/v1/patrons.t

@ -17,122 +17,410 @@
use Modern::Perl;
use Test::More tests => 21;
use Test::More tests => 5;
use Test::Mojo;
use Test::Warn;
use t::lib::TestBuilder;
use t::lib::Mocks;
use C4::Auth;
use C4::Context;
use Koha::Cities;
use Koha::Database;
use Koha::Patron;
my $schema = Koha::Database->new->schema;
my $builder = t::lib::TestBuilder->new();
$schema->storage->txn_begin;
my $builder = t::lib::TestBuilder->new;
# FIXME: sessionStorage defaults to mysql, but it seems to break transaction handling
# this affects the other REST api tests
t::lib::Mocks::mock_preference( 'SessionStorage', 'tmp' );
$ENV{REMOTE_ADDR} = '127.0.0.1';
my $t = Test::Mojo->new('Koha::REST::V1');
my $remote_address = '127.0.0.1';
my $t = Test::Mojo->new('Koha::REST::V1');
my $categorycode = $builder->build({ source => 'Category' })->{ categorycode };
my $branchcode = $builder->build({ source => 'Branch' })->{ branchcode };
my $guarantor = $builder->build({
source => 'Borrower',
value => {
branchcode => $branchcode,
categorycode => $categorycode,
flags => 0,
}
});
my $borrower = $builder->build({
source => 'Borrower',
value => {
branchcode => $branchcode,
categorycode => $categorycode,
flags => 0,
lost => 1,
guarantorid => $guarantor->{borrowernumber},
}
});
$t->get_ok('/api/v1/patrons')
->status_is(401);
$t->get_ok("/api/v1/patrons/" . $borrower->{ borrowernumber })
->status_is(401);
my $session = C4::Auth::get_session('');
$session->param('number', $borrower->{ borrowernumber });
$session->param('id', $borrower->{ userid });
$session->param('ip', '127.0.0.1');
$session->param('lasttime', time());
$session->flush;
my $session2 = C4::Auth::get_session('');
$session2->param('number', $guarantor->{ borrowernumber });
$session2->param('id', $guarantor->{ userid });
$session2->param('ip', '127.0.0.1');
$session2->param('lasttime', time());
$session2->flush;
my $tx = $t->ua->build_tx(GET => '/api/v1/patrons');
$tx->req->cookies({name => 'CGISESSID', value => $session->id});
$t->request_ok($tx)
->status_is(403);
$tx = $t->ua->build_tx(GET => "/api/v1/patrons/" . ($borrower->{ borrowernumber }-1));
$tx->req->cookies({name => 'CGISESSID', value => $session->id});
$t->request_ok($tx)
->status_is(403)
->json_is('/required_permissions', {"borrowers" => "edit_borrowers"});
# User without permissions, but is the owner of the object
$tx = $t->ua->build_tx(GET => "/api/v1/patrons/" . $borrower->{borrowernumber});
$tx->req->cookies({name => 'CGISESSID', value => $session->id});
$t->request_ok($tx)
->status_is(200);
# User without permissions, but is the guarantor of the owner of the object
$tx = $t->ua->build_tx(GET => "/api/v1/patrons/" . $borrower->{borrowernumber});
$tx->req->cookies({name => 'CGISESSID', value => $session2->id});
$t->request_ok($tx)
->status_is(200)
->json_is('/guarantorid', $guarantor->{borrowernumber});
my $loggedinuser = $builder->build({
source => 'Borrower',
value => {
subtest 'list() tests' => sub {
plan tests => 2;
$schema->storage->txn_begin;
unauthorized_access_tests('GET', undef, undef);
subtest 'librarian access tests' => sub {
plan tests => 8;
my ($borrowernumber, $sessionid) = create_user_and_session({
authorized => 1 });
my $patron = Koha::Patrons->find($borrowernumber);
Koha::Patrons->search({
borrowernumber => { '!=' => $borrowernumber},
cardnumber => { LIKE => $patron->cardnumber . "%" }
})->delete;
Koha::Patrons->search({
borrowernumber => { '!=' => $borrowernumber},
address2 => { LIKE => $patron->address2 . "%" }
})->delete;
my $tx = $t->ua->build_tx(GET => '/api/v1/patrons');
$tx->req->cookies({name => 'CGISESSID', value => $sessionid});
$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?cardnumber='.
$patron->cardnumber);
$tx->req->cookies({name => 'CGISESSID', value => $sessionid});
$tx->req->env({REMOTE_ADDR => '127.0.0.1'});
$t->request_ok($tx)
->status_is(200)
->json_is('/0/cardnumber' => $patron->cardnumber);
$tx = $t->ua->build_tx(GET => '/api/v1/patrons?address2='.
$patron->address2);
$tx->req->cookies({name => 'CGISESSID', value => $sessionid});
$tx->req->env({REMOTE_ADDR => '127.0.0.1'});
$t->request_ok($tx)
->status_is(200)
->json_is('/0/address2' => $patron->address2);
};
$schema->storage->txn_rollback;
};
subtest 'get() tests' => sub {
plan tests => 3;
$schema->storage->txn_begin;
unauthorized_access_tests('GET', -1, undef);
subtest 'access own object tests' => sub {
plan tests => 4;
my ($patronid, $patronsessionid) = create_user_and_session({
authorized => 0 });
# Access patron's own data even though they have no borrowers flag
my $tx = $t->ua->build_tx(GET => "/api/v1/patrons/$patronid");
$tx->req->cookies({name => 'CGISESSID', value => $patronsessionid});
$tx->req->env({REMOTE_ADDR => '127.0.0.1'});
$t->request_ok($tx)
->status_is(200);
my $guarantee = $builder->build({
source => 'Borrower',
value => {
guarantorid => $patronid,
}
});
# Access guarantee's data even though guarantor has no borrowers flag
my $guaranteenumber = $guarantee->{borrowernumber};
$tx = $t->ua->build_tx(GET => "/api/v1/patrons/$guaranteenumber");
$tx->req->cookies({name => 'CGISESSID', value => $patronsessionid});
$tx->req->env({REMOTE_ADDR => '127.0.0.1'});
$t->request_ok($tx)
->status_is(200);
};
subtest 'librarian access tests' => sub {
plan tests => 5;
my ($patron_id) = create_user_and_session({
authorized => 0 });
my $patron = Koha::Patrons->find($patron_id);
my ($borrowernumber, $sessionid) = create_user_and_session({
authorized => 1 });
my $tx = $t->ua->build_tx(GET => "/api/v1/patrons/$patron_id");
$tx->req->cookies({name => 'CGISESSID', value => $sessionid});
$t->request_ok($tx)
->status_is(200)
->json_is('/borrowernumber' => $patron_id)
->json_is('/surname' => $patron->surname)
->json_is('/lost' => Mojo::JSON->false );
};
$schema->storage->txn_rollback;
};
subtest 'add() tests' => sub {
plan tests => 2;
$schema->storage->txn_begin;
my $categorycode = $builder->build({ source => 'Category' })->{categorycode};
my $branchcode = $builder->build({ source => 'Branch' })->{branchcode};
my $newpatron = {
address => 'Street',
branchcode => $branchcode,
cardnumber =