@@ -, +, @@ --- Koha/Patron.pm | 10 +++--- Koha/REST/V1/Patrons.pm | 9 +++-- t/db_dependent/api/v1/acquisitions_baskets.t | 12 +++++-- t/db_dependent/api/v1/acquisitions_funds.t | 26 ++++++++++++--- t/db_dependent/api/v1/patrons.t | 35 ++++++++++++-------- 5 files changed, 64 insertions(+), 28 deletions(-) --- a/Koha/Patron.pm +++ a/Koha/Patron.pm @@ -37,6 +37,7 @@ use Koha::Club::Enrollments; use Koha::Database; use Koha::DateUtils qw( dt_from_string ); use Koha::Encryption; +use Koha::Exceptions; use Koha::Exceptions::Password; use Koha::Holds; use Koha::Old::Checkouts; @@ -1881,11 +1882,10 @@ Returns 0 if the I parameter is missing. sub is_accessible { my ( $self, $params ) = @_; - # FIXME? It felt tempting to return 0 instead - # but it would mean needing to explicitly add the 'user' - # param in all tests... - return 1 - unless $params->{user}; + unless ( defined( $params->{user} ) ) { + Koha::Exceptions::MissingParameter->throw( + error => "The `user` parameter is mandatory" ); + } my $consumer = $params->{user}; return $consumer->can_see_patron_infos($self); --- a/Koha/REST/V1/Patrons.pm +++ a/Koha/REST/V1/Patrons.pm @@ -102,6 +102,7 @@ Controller function that handles adding a new Koha::Patron object sub add { my $c = shift->openapi->valid_input or return; + my $current_user = $c->stash( 'koha.user' ); return try { @@ -131,7 +132,7 @@ sub add { $c->res->headers->location($c->req->url->to_string . '/' . $patron->borrowernumber); return $c->render( status => 201, - openapi => $patron->to_api + openapi => $patron->to_api({ user => $current_user }) ); } ); @@ -218,6 +219,7 @@ Controller function that handles updating a Koha::Patron object sub update { my $c = shift->openapi->valid_input or return; + my $current_user = $c->stash( 'koha.user' ); my $patron_id = $c->validation->param('patron_id'); my $patron = Koha::Patrons->find( $patron_id ); @@ -265,7 +267,10 @@ sub update { $patron->set_from_api($c->validation->param('body'))->store; $patron->discard_changes; - return $c->render( status => 200, openapi => $patron->to_api ); + return $c->render( + status => 200, + openapi => $patron->to_api( { user => $current_user } ) + ); } catch { unless ( blessed $_ && $_->can('rethrow') ) { --- a/t/db_dependent/api/v1/acquisitions_baskets.t +++ a/t/db_dependent/api/v1/acquisitions_baskets.t @@ -57,8 +57,16 @@ subtest 'list_managers() tests' => sub { } ); - $t->get_ok("//$userid:$password@/api/v1/acquisitions/baskets/managers?q=$api_filter") - ->status_is(200)->json_is( [ $patron_with_permission->to_api, $superlibrarian->to_api ] ); + $t->get_ok( + "//$userid:$password@/api/v1/acquisitions/baskets/managers?q=$api_filter" + )->status_is(200)->json_is( + [ + $patron_with_permission->to_api( + { user => $patron_with_permission } + ), + $superlibrarian->to_api( { user => $superlibrarian } ) + ] + ); $schema->storage->txn_rollback; }; --- a/t/db_dependent/api/v1/acquisitions_funds.t +++ a/t/db_dependent/api/v1/acquisitions_funds.t @@ -113,11 +113,27 @@ subtest 'list_owners() and list_users() tests' => sub { } ); - $t->get_ok("//$userid:$password@/api/v1/acquisitions/funds/owners?q=$api_filter") - ->status_is(200)->json_is( [ $patron_with_permission->to_api, $superlibrarian->to_api ] ); - - $t->get_ok("//$userid:$password@/api/v1/acquisitions/funds/users?q=$api_filter") - ->status_is(200)->json_is( [ $patron_with_permission->to_api, $superlibrarian->to_api ] ); + $t->get_ok( + "//$userid:$password@/api/v1/acquisitions/funds/owners?q=$api_filter") + ->status_is(200)->json_is( + [ + $patron_with_permission->to_api( + { user => $patron_with_permission } + ), + $superlibrarian->to_api( { user => $superlibrarian } ) + ] + ); + + $t->get_ok( + "//$userid:$password@/api/v1/acquisitions/funds/users?q=$api_filter") + ->status_is(200)->json_is( + [ + $patron_with_permission->to_api( + { user => $patron_with_permission } + ), + $superlibrarian->to_api( { user => $superlibrarian } ) + ] + ); $schema->storage->txn_rollback; }; --- a/t/db_dependent/api/v1/patrons.t +++ a/t/db_dependent/api/v1/patrons.t @@ -295,7 +295,13 @@ subtest 'add() tests' => sub { $schema->storage->txn_begin; - my $patron = $builder->build_object( { class => 'Koha::Patrons' } )->to_api; + my $librarian = $builder->build_object( + { + class => 'Koha::Patrons', + value => { flags => 2**4 } # borrowers flag = 4 + } + ); + my $patron = $builder->build_object( { class => 'Koha::Patrons' } )->to_api({ user => $librarian }); unauthorized_access_tests('POST', undef, $patron); @@ -336,8 +342,15 @@ subtest 'add() tests' => sub { } ); - my $patron = $builder->build_object({ class => 'Koha::Patrons' }); - my $newpatron = $patron->to_api; + my $librarian = $builder->build_object( + { + class => 'Koha::Patrons', + value => { flags => 2**4 } # borrowers flag = 4 + } + ); + + my $patron = $builder->build_object( { class => 'Koha::Patrons' } ); + my $newpatron = $patron->to_api({ user => $librarian }); # delete RO attributes delete $newpatron->{patron_id}; delete $newpatron->{restricted}; @@ -349,12 +362,6 @@ subtest 'add() tests' => sub { # Delete library $library_to_delete->delete; - my $librarian = $builder->build_object( - { - class => 'Koha::Patrons', - value => { flags => 2**4 } # borrowers flag = 4 - } - ); my $password = 'thePassword123'; $librarian->set_password( { password => $password, skip_validation => 1 } ); my $userid = $librarian->userid; @@ -390,7 +397,7 @@ subtest 'add() tests' => sub { delete $newpatron->{falseproperty}; my $patron_to_delete = $builder->build_object({ class => 'Koha::Patrons' }); - $newpatron = $patron_to_delete->to_api; + $newpatron = $patron_to_delete->to_api({ user => $librarian }); # delete RO attributes delete $newpatron->{patron_id}; delete $newpatron->{restricted}; @@ -615,7 +622,7 @@ subtest 'update() tests' => sub { my $patron_1 = $authorized_patron; my $patron_2 = $unauthorized_patron; - my $newpatron = $unauthorized_patron->to_api; + my $newpatron = $unauthorized_patron->to_api({ user => $authorized_patron }); # delete RO attributes delete $newpatron->{patron_id}; delete $newpatron->{restricted}; @@ -695,9 +702,9 @@ subtest 'update() tests' => sub { ->status_is(200, 'Patron updated successfully'); # Put back the RO attributes - $newpatron->{patron_id} = $unauthorized_patron->to_api->{patron_id}; - $newpatron->{restricted} = $unauthorized_patron->to_api->{restricted}; - $newpatron->{anonymized} = $unauthorized_patron->to_api->{anonymized}; + $newpatron->{patron_id} = $unauthorized_patron->to_api({ user => $authorized_patron })->{patron_id}; + $newpatron->{restricted} = $unauthorized_patron->to_api({ user => $authorized_patron })->{restricted}; + $newpatron->{anonymized} = $unauthorized_patron->to_api({ user => $authorized_patron })->{anonymized}; my $got = $result->tx->res->json; my $updated_on_got = delete $got->{updated_on}; --