@@ -, +, @@ - Apply this patches and the dependencies - Run: $ kshell k$ prove t/db_dependent/api/v1/patrons.t - Sign off :-D --- Koha/REST/V1/Patron.pm | 270 +++++++++++++++++--------- api/v1/swagger/paths/patrons.json | 389 ++++++++++++++++---------------------- t/db_dependent/api/v1/patrons.t | 105 +++------- 3 files changed, 372 insertions(+), 392 deletions(-) --- a/Koha/REST/V1/Patron.pm +++ a/Koha/REST/V1/Patron.pm @@ -20,38 +20,53 @@ 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; +=head1 NAME + +Koha::REST::V1::Patron + +=head1 API + +=head2 Methods + +=head3 list + +Controller function that handles listing Koha::Patron objects + +=cut + sub list { my $c = shift->openapi->valid_input or return; - 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($filter); - return $c->render(status => 200, openapi => $patrons); + my $patrons_set = Koha::Patrons->new; + my @patrons = $c->objects->search( $patrons_set )->as_list; + return $c->render( status => 200, openapi => \@patrons ); } catch { if ( $_->isa('DBIx::Class::Exception') ) { - return $c->render( status => 500, openapi => { error => $_->{msg} } ); + return $c->render( status => 500, + openapi => { error => $_->{msg} } ); } else { - return $c->render( status => 500, openapi => { error => "Something went wrong, check the logs." } ); + return $c->render( + status => 500, + openapi => { error => "Something went wrong, check the logs." } + ); } }; } +=head3 get + +Controller function that handles retrieving a single Koha::Patron object + +=cut + sub get { my $c = shift->openapi->valid_input or return; @@ -65,140 +80,213 @@ sub get { return $c->render(status => 200, openapi => $patron); } +=head3 add + +Controller function that handles adding a new Koha::Patron object + +=cut + sub add { - my ($c, $args, $cb) = @_; + my $c = shift->openapi->valid_input or return; return try { - my $body = $c->req->json; + my $body = $c->validation->param('body'); 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); + my $patron = Koha::Patrons->find($borrowernumber); - return $c->$cb($patron, 201); + return $c->render( status => 201, openapi => $patron ); } catch { - unless (blessed $_ && $_->can('rethrow')) { - return $c->$cb({ error => - "Something went wrong, check Koha logs for details."}, 500); + unless ( blessed $_ && $_->can('rethrow') ) { + return $c->render( + status => 500, + openapi => { + error => "Something went wrong, check Koha logs for details." + } + ); } - if ($_->isa('Koha::Exceptions::Patron::DuplicateObject')) { - return $c->$cb({ error => $_->error, conflict => $_->conflict }, 409); + if ( $_->isa('Koha::Exceptions::Patron::DuplicateObject') ) { + return $c->render( + status => 409, + openapi => { error => $_->error, conflict => $_->conflict } + ); } - elsif ($_->isa('Koha::Exceptions::Library::BranchcodeNotFound')) { - return $c->$cb({ error => "Given branchcode does not exist" }, 400); + elsif ( $_->isa('Koha::Exceptions::Library::BranchcodeNotFound') ) { + return $c->render( + status => 400, + openapi => { error => "Given branchcode does not exist" } + ); } - elsif ($_->isa('Koha::Exceptions::Category::CategorycodeNotFound')) { - return $c->$cb({ error => "Given categorycode does not exist"}, 400); + elsif ( $_->isa('Koha::Exceptions::Category::CategorycodeNotFound') ) { + return $c->render( + status => 400, + openapi => { error => "Given categorycode does not exist" } + ); } else { - return $c->$cb({ error => - "Something went wrong, check Koha logs for details."}, 500); + return $c->render( + status => 500, + openapi => { + error => "Something went wrong, check Koha logs for details." + } + ); } }; } -sub edit { - my ($c, $args, $cb) = @_; +=head3 update + +Controller function that handles updating a Koha::Patron object + +=cut + +sub update { + my $c = shift->openapi->valid_input or return; + + my $patron = Koha::Patrons->find( $c->validation->param('borrowernumber') ); - 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); - } + my $body = $c->validation->param('body'); + + $patron->set( _to_model($body) )->_validate; + + # TODO: Use ModMember until it has been moved to Koha-namespace + if ( ModMember(%$body) ) { + return $c->render( status => 200, openapi => $patron ); } 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); + return $c->render( + status => 500, + openapi => { + error => 'Something went wrong, check Koha logs for details.' + } + ); } } catch { unless ($patron) { - return $c->$cb({error => "Patron not found"}, 404); + return $c->render( + status => 404, + openapi => { error => "Patron not found" } + ); } - unless (blessed $_ && $_->can('rethrow')) { - return $c->$cb({ error => - "Something went wrong, check Koha logs for details."}, 500); + unless ( blessed $_ && $_->can('rethrow') ) { + return $c->render( + status => 500, + openapi => { + error => "Something went wrong, check Koha logs for details." + } + ); } - if ($_->isa('Koha::Exceptions::Patron::DuplicateObject')) { - return $c->$cb({ error => $_->error, conflict => $_->conflict }, 409); + if ( $_->isa('Koha::Exceptions::Patron::DuplicateObject') ) { + return $c->render( + status => 409, + openapi => { error => $_->error, conflict => $_->conflict } + ); } - elsif ($_->isa('Koha::Exceptions::Library::BranchcodeNotFound')) { - return $c->$cb({ error => "Given branchcode does not exist" }, 400); + elsif ( $_->isa('Koha::Exceptions::Library::BranchcodeNotFound') ) { + return $c->render( + status => 400, + openapi => { error => "Given branchcode does not exist" } + ); } - elsif ($_->isa('Koha::Exceptions::Category::CategorycodeNotFound')) { - return $c->$cb({ error => "Given categorycode does not exist"}, 400); + elsif ( $_->isa('Koha::Exceptions::Category::CategorycodeNotFound') ) { + return $c->render( + status => 400, + openapi => { error => "Given categorycode does not exist" } + ); } - elsif ($_->isa('Koha::Exceptions::MissingParameter')) { - return $c->$cb({error => "Missing mandatory parameter(s)", - parameters => $_->parameter }, 400); + elsif ( $_->isa('Koha::Exceptions::MissingParameter') ) { + return $c->render( + status => 400, + openapi => { + error => "Missing mandatory parameter(s)", + parameters => $_->parameter + } + ); } - elsif ($_->isa('Koha::Exceptions::BadParameter')) { - return $c->$cb({error => "Invalid parameter(s)", - parameters => $_->parameter }, 400); + elsif ( $_->isa('Koha::Exceptions::BadParameter') ) { + return $c->render( + status => 400, + openapi => { + error => "Invalid parameter(s)", + parameters => $_->parameter + } + ); } - elsif ($_->isa('Koha::Exceptions::NoChanges')) { - return $c->$cb({error => "No changes have been made"}, 204); + elsif ( $_->isa('Koha::Exceptions::NoChanges') ) { + return $c->render( + status => 204, + openapi => { error => "No changes have been made" } + ); } else { - return $c->$cb({ error => - "Something went wrong, check Koha logs for details."}, 500); + return $c->render( + status => 500, + openapi => { + error => + "Something went wrong, check Koha logs for details." + } + ); } }; } +=head3 delete + +Controller function that handles deleting a Koha::Patron object + +=cut + sub delete { - my ($c, $args, $cb) = @_; + my $c = shift->openapi->valid_input or return; my $patron; return try { - $patron = Koha::Patrons->find($args->{borrowernumber}); + $patron = Koha::Patrons->find( $c->validation->param('borrowernumber') ); + # check if loans, reservations, debarrment, etc. before deletion! my $res = $patron->delete; - - return $c->$cb({}, 200); + return $c->render( status => 200, openapi => {} ); } catch { unless ($patron) { - return $c->$cb({error => "Patron not found"}, 404); + return $c->render( + status => 404, + openapi => { error => "Patron not found" } + ); } else { - return $c->$cb({ error => - "Something went wrong, check Koha logs for details."}, 500); + return $c->render( + status => 500, + openapi => { + error => + "Something went wrong, check Koha logs for details." + } + ); } }; } -sub _delete_unmodifiable_parameters { - my ($body) = @_; +=head3 _to_model - my %columns = map { $_ => 1 } Koha::Patron::Modifications->columns; - foreach my $param (keys %$body) { - unless (exists $columns{$param}) { - delete $body->{$param}; - } - } - return $body; +Helper function that maps REST api objects into Koha::Patron +attribute names. + +=cut + +sub _to_model { + my $params = shift; + + $params->{lost} = ($params->{lost}) ? 1 : 0; + $params->{gonenoaddress} = ($params->{gonenoaddress}) ? 1 : 0; + + return $params; } 1; --- a/api/v1/swagger/paths/patrons.json +++ a/api/v1/swagger/paths/patrons.json @@ -10,513 +10,449 @@ "parameters": [{ "name": "borrowernumber", "in": "query", - "description": "Case insensetive 'starts_with' search on borrowernumber", + "description": "Case insensitive search on borrowernumber", "required": false, "type": "string" - }, - { + }, { "name": "cardnumber", "in": "query", - "description": "Case insensetive 'starts_with' search on cardnumber", + "description": "Case insensitive search on cardnumber", "required": false, "type": "string" - }, - { + }, { "name": "surname", "in": "query", - "description": "Case insensetive 'starts_with' search on surname", + "description": "Case insensitive search on surname", "required": false, "type": "string" - }, - { + }, { "name": "firstname", "in": "query", - "description": "Case insensetive 'starts_with' search on firstname", + "description": "Case insensitive search on firstname", "required": false, "type": "string" - }, - { + }, { "name": "title", "in": "query", - "description": "Case insensetive 'starts_with' search on title", + "description": "Case insensitive search on title", "required": false, "type": "string" - }, - { + }, { "name": "othernames", "in": "query", - "description": "Case insensetive 'starts_with' search on othernames", + "description": "Case insensitive search on othernames", "required": false, "type": "string" - }, - { + }, { "name": "initials", "in": "query", - "description": "Case insensetive 'starts_with' search on initials", + "description": "Case insensitive search on initials", "required": false, "type": "string" - }, - { + }, { "name": "streetnumber", "in": "query", - "description": "Case insensetive 'starts_with' search on streetnumber", + "description": "Case insensitive search on streetnumber", "required": false, "type": "string" - }, - { + }, { "name": "streettype", "in": "query", - "description": "Case insensetive 'starts_with' search on streettype", + "description": "Case insensitive search on streettype", "required": false, "type": "string" - }, - { + }, { "name": "address", "in": "query", - "description": "Case insensetive 'starts_with' search on address", + "description": "Case insensitive search on address", "required": false, "type": "string" - }, - { + }, { "name": "address2", "in": "query", - "description": "Case insensetive 'starts_with' search on address2", + "description": "Case insensitive search on address2", "required": false, "type": "string" - }, - { + }, { "name": "city", "in": "query", - "description": "Case insensetive 'starts_with' search on city", + "description": "Case insensitive search on city", "required": false, "type": "string" - }, - { + }, { "name": "state", "in": "query", - "description": "Case insensetive 'starts_with' search on state", + "description": "Case insensitive search on state", "required": false, "type": "string" - }, - { + }, { "name": "zipcode", "in": "query", - "description": "Case insensetive 'starts_with' search on zipcode", + "description": "Case insensitive search on zipcode", "required": false, "type": "string" - }, - { + }, { "name": "country", "in": "query", - "description": "Case insensetive 'starts_with' search on country", + "description": "Case insensitive search on country", "required": false, "type": "string" - }, - { + }, { "name": "email", "in": "query", - "description": "Case insensetive 'starts_with' search on email", + "description": "Case insensitive search on email", "required": false, "type": "string" - }, - { + }, { "name": "phone", "in": "query", - "description": "Case insensetive 'starts_with' search on phone", + "description": "Case insensitive search on phone", "required": false, "type": "string" - }, - { + }, { "name": "mobile", "in": "query", - "description": "Case insensetive 'starts_with' search on mobile", + "description": "Case insensitive search on mobile", "required": false, "type": "string" - }, - { + }, { "name": "fax", "in": "query", - "description": "Case insensetive 'starts_with' search on fax", + "description": "Case insensitive search on fax", "required": false, "type": "string" - }, - { + }, { "name": "emailpro", "in": "query", - "description": "Case insensetive 'starts_with' search on emailpro", + "description": "Case insensitive search on emailpro", "required": false, "type": "string" - }, - { + }, { "name": "phonepro", "in": "query", - "description": "Case insensetive 'starts_with' search on phonepro", + "description": "Case insensitive search on phonepro", "required": false, "type": "string" - }, - { + }, { "name": "B_streetnumber", "in": "query", - "description": "Case insensetive 'starts_with' search on B_streetnumber", + "description": "Case insensitive search on B_streetnumber", "required": false, "type": "string" - }, - { + }, { "name": "B_streettype", "in": "query", - "description": "Case insensetive 'starts_with' search on B_streettype", + "description": "Case insensitive search on B_streettype", "required": false, "type": "string" - }, - { + }, { "name": "B_address", "in": "query", - "description": "Case insensetive 'starts_with' search on B_address", + "description": "Case insensitive search on B_address", "required": false, "type": "string" - }, - { + }, { "name": "B_address2", "in": "query", - "description": "Case insensetive 'starts_with' search on B_address2", + "description": "Case insensitive search on B_address2", "required": false, "type": "string" - }, - { + }, { "name": "B_city", "in": "query", - "description": "Case insensetive 'starts_with' search on B_city", + "description": "Case insensitive search on B_city", "required": false, "type": "string" - }, - { + }, { "name": "B_state", "in": "query", - "description": "Case insensetive 'starts_with' search on B_state", + "description": "Case insensitive search on B_state", "required": false, "type": "string" - }, - { + }, { "name": "B_zipcode", "in": "query", - "description": "Case insensetive 'starts_with' search on B_zipcode", + "description": "Case insensitive search on B_zipcode", "required": false, "type": "string" - }, - { + }, { "name": "B_country", "in": "query", - "description": "Case insensetive 'starts_with' search on B_country", + "description": "Case insensitive search on B_country", "required": false, "type": "string" - }, - { + }, { "name": "B_email", "in": "query", - "description": "Case insensetive 'starts_with' search on B_email", + "description": "Case insensitive search on B_email", "required": false, "type": "string" - }, - { + }, { "name": "B_phone", "in": "query", - "description": "Case insensetive 'starts_with' search on B_phone", + "description": "Case insensitive search on B_phone", "required": false, "type": "string" - }, - { + }, { "name": "dateofbirth", "in": "query", - "description": "Case insensetive 'starts_with' search on dateofbirth", + "description": "Case insensitive search on dateofbirth", "required": false, "type": "string" - }, - { + }, { "name": "branchcode", "in": "query", - "description": "Case insensetive 'starts_with' search on branchcode", + "description": "Case insensitive search on branchcode", "required": false, "type": "string" - }, - { + }, { "name": "categorycode", "in": "query", - "description": "Case insensetive 'starts_with' search on categorycode", + "description": "Case insensitive search on categorycode", "required": false, "type": "string" - }, - { + }, { "name": "dateenrolled", "in": "query", - "description": "Case insensetive 'starts_with' search on dateenrolled", + "description": "Case insensitive search on dateenrolled", "required": false, "type": "string" - }, - { + }, { "name": "dateexpiry", "in": "query", - "description": "Case insensetive 'starts_with' search on dateexpiry", + "description": "Case insensitive search on dateexpiry", "required": false, "type": "string" - }, - { + }, { "name": "gonenoaddress", "in": "query", - "description": "Case insensetive 'starts_with' search on gonenoaddress", + "description": "Search on gonenoaddress", "required": false, - "type": "string" - }, - { + "type": "boolean" + }, { "name": "lost", "in": "query", - "description": "Case insensetive 'starts_with' search on lost", + "description": "Search on lost", "required": false, - "type": "string" - }, - { + "type": "boolean" + }, { "name": "debarred", "in": "query", - "description": "Case insensetive 'starts_with' search on debarred", + "description": "Case insensitive search on debarred", "required": false, "type": "string" - }, - { + }, { "name": "debarredcomment", "in": "query", - "description": "Case insensetive 'starts_with' search on debarredcomment", + "description": "Case insensitive search on debarredcomment", "required": false, "type": "string" - }, - { + }, { "name": "contactname", "in": "query", - "description": "Case insensetive 'starts_with' search on contactname", + "description": "Case insensitive search on contactname", "required": false, "type": "string" - }, - { + }, { "name": "contactfirstname", "in": "query", - "description": "Case insensetive 'starts_with' search on contactfirstname", + "description": "Case insensitive search on contactfirstname", "required": false, "type": "string" - }, - { + }, { "name": "contacttitle", "in": "query", - "description": "Case insensetive 'starts_with' search on contacttitle", + "description": "Case insensitive search on contacttitle", "required": false, "type": "string" - }, - { + }, { "name": "guarantorid", "in": "query", - "description": "Case insensetive 'starts_with' search on guarantorid", + "description": "Case insensitive search on guarantorid", "required": false, "type": "string" - }, - { + }, { "name": "borrowernotes", "in": "query", - "description": "Case insensetive 'starts_with' search on borrowernotes", + "description": "Case insensitive search on borrowernotes", "required": false, "type": "string" - }, - { + }, { "name": "relationship", "in": "query", - "description": "Case insensetive 'starts_with' search on relationship", + "description": "Case insensitive search on relationship", "required": false, "type": "string" - }, - { + }, { "name": "sex", "in": "query", - "description": "Case insensetive 'starts_with' search on sex", + "description": "Case insensitive search on sex", "required": false, "type": "string" - }, - { + }, { "name": "password", "in": "query", - "description": "Case insensetive 'starts_with' search on password", + "description": "Case insensitive search on password", "required": false, "type": "string" - }, - { + }, { "name": "flags", "in": "query", - "description": "Case insensetive 'starts_with' search on flags", + "description": "Case insensitive search on flags", "required": false, "type": "string" - }, - { + }, { "name": "userid", "in": "query", - "description": "Case insensetive 'starts_with' search on userid", + "description": "Case insensitive search on userid", "required": false, "type": "string" - }, - { + }, { "name": "opacnote", "in": "query", - "description": "Case insensetive 'starts_with' search on opacnote", + "description": "Case insensitive search on opacnote", "required": false, "type": "string" - }, - { + }, { "name": "contactnote", "in": "query", - "description": "Case insensetive 'starts_with' search on contactnote", + "description": "Case insensitive search on contactnote", "required": false, "type": "string" - }, - { + }, { "name": "sort1", "in": "query", - "description": "Case insensetive 'starts_with' search on sort1", + "description": "Case insensitive search on sort1", "required": false, "type": "string" - }, - { + }, { "name": "sort2", "in": "query", - "description": "Case insensetive 'starts_with' search on sort2", + "description": "Case insensitive search on sort2", "required": false, "type": "string" - }, - { + }, { "name": "altcontactfirstname", "in": "query", - "description": "Case insensetive 'starts_with' search on altcontactfirstname", + "description": "Case insensitive search on altcontactfirstname", "required": false, "type": "string" - }, - { + }, { "name": "altcontactsurname", "in": "query", - "description": "Case insensetive 'starts_with' search on altcontactsurname", + "description": "Case insensitive search on altcontactsurname", "required": false, "type": "string" - }, - { + }, { "name": "altcontactaddress1", "in": "query", - "description": "Case insensetive 'starts_with' search on altcontactaddress1", + "description": "Case insensitive search on altcontactaddress1", "required": false, "type": "string" - }, - { + }, { "name": "altcontactaddress2", "in": "query", - "description": "Case insensetive 'starts_with' search on altcontactaddress2", + "description": "Case insensitive search on altcontactaddress2", "required": false, "type": "string" - }, - { + }, { "name": "altcontactaddress3", "in": "query", - "description": "Case insensetive 'starts_with' search on altcontactaddress3", + "description": "Case insensitive search on altcontactaddress3", "required": false, "type": "string" - }, - { + }, { "name": "altcontactstate", "in": "query", - "description": "Case insensetive 'starts_with' search on altcontactstate", + "description": "Case insensitive search on altcontactstate", "required": false, "type": "string" - }, - { + }, { "name": "altcontactzipcode", "in": "query", - "description": "Case insensetive 'starts_with' search on altcontactzipcode", + "description": "Case insensitive search on altcontactzipcode", "required": false, "type": "string" - }, - { + }, { "name": "altcontactcountry", "in": "query", - "description": "Case insensetive 'starts_with' search on altcontactcountry", + "description": "Case insensitive search on altcontactcountry", "required": false, "type": "string" - }, - { + }, { "name": "altcontactphone", "in": "query", - "description": "Case insensetive 'starts_with' search on altcontactphone", + "description": "Case insensitive search on altcontactphone", "required": false, "type": "string" - }, - { + }, { "name": "smsalertnumber", "in": "query", - "description": "Case insensetive 'starts_with' search on smsalertnumber", + "description": "Case insensitive search on smsalertnumber", "required": false, "type": "string" - }, - { + }, { "name": "sms_provider_id", "in": "query", - "description": "Case insensetive 'starts_with' search on sms_provider_id", + "description": "Case insensitive search on sms_provider_id", "required": false, "type": "string" - }, - { + }, { "name": "privacy", "in": "query", - "description": "Case insensetive 'starts_with' search on privacy", + "description": "Case insensitive search on privacy", "required": false, "type": "string" - }, - { + }, { "name": "privacy_guarantor_checkouts", "in": "query", - "description": "Case insensetive 'starts_with' search on privacy_guarantor_checkouts", + "description": "Case insensitive search on privacy_guarantor_checkouts", "required": false, "type": "string" - }, - { + }, { "name": "checkprevcheckout", "in": "query", - "description": "Case insensetive 'starts_with' search on checkprevcheckout", + "description": "Case insensitive search on checkprevcheckout", "required": false, "type": "string" - }, - { + }, { "name": "updated_on", "in": "query", - "description": "Case insensetive 'starts_with' search on updated_on", + "description": "Case insensitive search on updated_on", "required": false, "type": "string" - }, - { + }, { "name": "lastseen", "in": "query", - "description": "Case insensetive 'starts_with' search on lastseen", + "description": "Case insensitive search on lastseen", "required": false, "type": "string" - }, - { + }, { "name": "lang", "in": "query", - "description": "Case insensetive 'starts_with' search on lang", + "description": "Case insensitive search on lang", "required": false, "type": "string" - }, - { + }, { "name": "login_attempts", "in": "query", - "description": "Case insensetive 'starts_with' search on login_attempts", + "description": "Case insensitive search on login_attempts", "required": false, "type": "string" - }, - { + }, { "name": "overdrive_auth_token", "in": "query", - "description": "Case insensetive 'starts_with' search on overdrive_auth_token", + "description": "Case insensitive search on overdrive_auth_token", "required": false, "type": "string" + }, { + "$ref": "../parameters.json#/match" + }, { + "$ref": "../parameters.json#/order_by" + }, { + "$ref": "../parameters.json#/page" + }, { + "$ref": "../parameters.json#/per_page" }], "responses": { "200": { @@ -554,6 +490,7 @@ } }, "post": { + "x-mojo-to": "Patron#add", "operationId": "addPatron", "tags": ["patrons"], "parameters": [{ @@ -606,14 +543,14 @@ "$ref": "../definitions.json#/error" } }, - "503": { - "description": "Under maintenance", + "500": { + "description": "Internal server error", "schema": { "$ref": "../definitions.json#/error" } }, - "500": { - "description": "Internal server error", + "503": { + "description": "Under maintenance", "schema": { "$ref": "../definitions.json#/error" } @@ -684,7 +621,8 @@ } }, "put": { - "operationId": "editPatron", + "x-mojo-to": "Patron#update", + "operationId": "updatePatron", "tags": ["patrons"], "parameters": [ { @@ -755,14 +693,13 @@ } }, "x-koha-authorization": { - "allow-owner": true, - "allow-guarantor": true, "permissions": { "borrowers": "1" } } }, "delete": { + "x-mojo-to": "Patron#delete", "operationId": "deletePatron", "tags": ["patrons"], "parameters": [{ --- a/t/db_dependent/api/v1/patrons.t +++ a/t/db_dependent/api/v1/patrons.t @@ -19,13 +19,11 @@ use Modern::Perl; use Test::More tests => 5; use Test::Mojo; -use Test::Warn; use t::lib::TestBuilder; use t::lib::Mocks; use C4::Auth; -use Koha::Cities; use Koha::Database; my $schema = Koha::Database->new->schema; @@ -168,7 +166,7 @@ subtest 'add() tests' => sub { authorized => 1 }); $newpatron->{branchcode} = "nonexistent"; # Test invalid branchcode - my $tx = $t->ua->build_tx(POST => "/api/v1/patrons" =>json => $newpatron); + my $tx = $t->ua->build_tx(POST => "/api/v1/patrons" => json => $newpatron ); $tx->req->cookies({name => 'CGISESSID', value => $sessionid}); $t->request_ok($tx) ->status_is(400) @@ -216,92 +214,48 @@ subtest 'add() tests' => sub { $schema->storage->txn_rollback; }; -subtest 'edit() tests' => sub { - plan tests => 3; +subtest 'update() tests' => sub { + plan tests => 2; $schema->storage->txn_begin; unauthorized_access_tests('PUT', 123, {email => 'nobody@example.com'}); - subtest 'patron modifying own data' => sub { - plan tests => 7; - - my ($borrowernumber, $sessionid) = create_user_and_session({ - authorized => 0 }); - my $patron = Koha::Patrons->find($borrowernumber)->TO_JSON; - - t::lib::Mocks::mock_preference("OPACPatronDetails", 0); - my $tx = $t->ua->build_tx(PUT => "/api/v1/patrons/" . - $patron->{borrowernumber} => json => $patron); - $tx->req->cookies({name => 'CGISESSID', value => $sessionid}); - $t->request_ok($tx) - ->status_is(403, 'OPACPatronDetails off - modifications not allowed.'); - - t::lib::Mocks::mock_preference("OPACPatronDetails", 1); - $tx = $t->ua->build_tx(PUT => "/api/v1/patrons/" . - $patron->{borrowernumber} => json => $patron); - $tx->req->cookies({name => 'CGISESSID', value => $sessionid}); - $t->request_ok($tx) - ->status_is(204, 'Updating myself with my current data'); - - $patron->{'firstname'} = "noob"; - $tx = $t->ua->build_tx(PUT => "/api/v1/patrons/" . - $patron->{borrowernumber} => json => $patron); - $tx->req->cookies({name => 'CGISESSID', value => $sessionid}); - $t->request_ok($tx) - ->status_is(202, 'Updating myself with my current data'); - - # Approve changes - Koha::Patron::Modifications->find({ - borrowernumber => $patron->{borrowernumber}, - firstname => "noob" - })->approve; - is(Koha::Patrons->find({ - borrowernumber => $patron->{borrowernumber}})->firstname, - "noob", "Changes approved"); - }; - subtest 'librarian access tests' => sub { plan tests => 20; t::lib::Mocks::mock_preference('minPasswordLength', 1); - my ($borrowernumber, $sessionid) = create_user_and_session({ - authorized => 1 }); - my ($borrowernumber2, undef) = create_user_and_session({ - authorized => 0 }); - my $patron = Koha::Patrons->find($borrowernumber2); - my $newpatron = Koha::Patrons->find($borrowernumber2)->TO_JSON; + my ($borrowernumber, $sessionid) = create_user_and_session({ authorized => 1 }); + my ($borrowernumber2, undef) = create_user_and_session({ authorized => 0 }); + + my $patron_1 = Koha::Patrons->find($borrowernumber); + my $patron_2 = Koha::Patrons->find($borrowernumber2); + my $newpatron = $patron_2->TO_JSON; - my $tx = $t->ua->build_tx(PUT => "/api/v1/patrons/-1" => - json => $newpatron); + my $tx = $t->ua->build_tx(PUT => "/api/v1/patrons/-1" => json => $newpatron ); $tx->req->cookies({name => 'CGISESSID', value => $sessionid}); $t->request_ok($tx) ->status_is(404) ->json_has('/error', 'Fails when trying to PUT nonexistent patron'); $newpatron->{categorycode} = 'nonexistent'; - $tx = $t->ua->build_tx(PUT => "/api/v1/patrons/" . - $newpatron->{borrowernumber} => json => $newpatron - ); + $tx = $t->ua->build_tx(PUT => "/api/v1/patrons/$borrowernumber2" => json => $newpatron ); $tx->req->cookies({name => 'CGISESSID', value => $sessionid}); $t->request_ok($tx) ->status_is(400) ->json_is('/error' => "Given categorycode does not exist"); - $newpatron->{categorycode} = $patron->categorycode; + $newpatron->{categorycode} = $patron_2->categorycode; $newpatron->{branchcode} = 'nonexistent'; - $tx = $t->ua->build_tx(PUT => "/api/v1/patrons/" . - $newpatron->{borrowernumber} => json => $newpatron - ); + $tx = $t->ua->build_tx(PUT => "/api/v1/patrons/$borrowernumber2" => json => $newpatron ); $tx->req->cookies({name => 'CGISESSID', value => $sessionid}); $t->request_ok($tx) ->status_is(400) ->json_is('/error' => "Given branchcode does not exist"); - $newpatron->{branchcode} = $patron->branchcode; + $newpatron->{branchcode} = $patron_2->branchcode; $newpatron->{falseproperty} = "Non existent property"; - $tx = $t->ua->build_tx(PUT => "/api/v1/patrons/" . - $newpatron->{borrowernumber} => json => $newpatron); + $tx = $t->ua->build_tx(PUT => "/api/v1/patrons/$borrowernumber2" => json => $newpatron ); $tx->req->cookies({name => 'CGISESSID', value => $sessionid}); $t->request_ok($tx) ->status_is(400) @@ -309,18 +263,20 @@ subtest 'edit() tests' => sub { 'Properties not allowed: falseproperty.'); delete $newpatron->{falseproperty}; - $tx = $t->ua->build_tx(PUT => "/api/v1/patrons/" . - $borrowernumber => json => $newpatron); - $tx->req->cookies({name => 'CGISESSID', value => $sessionid}); - $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 => $newpatron->{ cardnumber }, - userid => $newpatron->{ userid } - } - ); + # Set both cardnumber and userid to already existing values + $newpatron->{cardnumber} = $patron_1->cardnumber; + $newpatron->{userid} = $patron_1->userid; + + $tx = $t->ua->build_tx( PUT => "/api/v1/patrons/$borrowernumber2" => json => $newpatron ); + $tx->req->cookies({ name => 'CGISESSID', value => $sessionid }); + $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 => $newpatron->{cardnumber}, + userid => $newpatron->{userid} + } + ); $newpatron->{ cardnumber } = $borrowernumber.$borrowernumber2; $newpatron->{ userid } = "user".$borrowernumber.$borrowernumber2; @@ -398,7 +354,6 @@ sub create_user_and_session { my $args = shift; my $flags = ( $args->{authorized} ) ? 16 : 0; - my $dbh = C4::Context->dbh; my $user = $builder->build( { @@ -409,7 +364,7 @@ sub create_user_and_session { lost => 0, email => 'nobody@example.com', emailpro => 'nobody@example.com', - B_email => 'nobody@example.com', + B_email => 'nobody@example.com' } } ); --