From d1350eeda7412901a489892cb5f025c2cf45494b Mon Sep 17 00:00:00 2001 From: Lari Taskula Date: Mon, 7 Nov 2016 15:02:34 +0200 Subject: [PATCH] Bug 17565: REST API: Let user cancel reserve according to CanReserveBeCanceledFromOpac This patch adds ability for patron to cancel their own holds. To test: 1. Make sure you have patches from dependant bugs. 2. Apply this patch. 3. Place a hold for a patron that has no permissions. 4. Set hold to Waiting or Transfer status (found => 'W' or 'T' in database). 5. Login with that patron (and get their CGISESSID for cURL). 6. Test DELETE operation for the reserve_id that you just created, e.g. with cURL: curl -X DELETE -H "Content-Type: application/json" \ --cookie 'CGISESSID=0230cb985c4fb5844f71ba41f76a0ced' \ http://yourlib/api/v1/holds/4621 7. Observe HTTP 403 code and following error message: "Hold is already in transfer or waiting and cannot be cancelled by patron." 8. Reset hold's "found"-status in database. 9. Run the same cURL command as in step 6. 10. Observe HTTP 200 code and empty object as return. 11. Test DELETE operation for any other reserve_id, e.g. with cURL curl -X DELETE -H "Content-Type: application/json" \ --cookie 'CGISESSID=0230cb985c4fb5844f71ba41f76a0ced' \ http://yourlib/api/v1/holds/123 12. Observe permission required error. 13. Run t/db_dependent/api/v1/holds.t --- Koha/REST/V1/Hold.pm | 9 +++++++++ api/v1/swagger/paths/holds.json | 8 ++++++++ t/db_dependent/api/v1/holds.t | 20 ++++++++++++++++++-- 3 files changed, 35 insertions(+), 2 deletions(-) diff --git a/Koha/REST/V1/Hold.pm b/Koha/REST/V1/Hold.pm index 7f0b1ff..8b202cd 100644 --- a/Koha/REST/V1/Hold.pm +++ b/Koha/REST/V1/Hold.pm @@ -148,11 +148,20 @@ sub delete { my $reserve_id = $args->{reserve_id}; my $reserve = C4::Reserves::GetReserve($reserve_id); + my $user = $c->stash('koha.user'); unless ($reserve) { return $c->$cb({error => "Reserve not found"}, 404); } + if ($user + && ($c->stash('is_owner_access') || $c->stash('is_guarantor_access')) + && !C4::Reserves::CanReserveBeCanceledFromOpac($reserve_id, + $user->borrowernumber)) { + return $c->$cb({error => "Hold is already in transfer or waiting and " + ."cannot be cancelled by patron."}, 403); + } + C4::Reserves::CancelReserve({ reserve_id => $reserve_id }); return $c->$cb({}, 200); diff --git a/api/v1/swagger/paths/holds.json b/api/v1/swagger/paths/holds.json index 9e7ec34..310d48d 100644 --- a/api/v1/swagger/paths/holds.json +++ b/api/v1/swagger/paths/holds.json @@ -288,6 +288,12 @@ "type": "object" } }, + "403": { + "description": "Cancellation forbidden", + "schema": { + "$ref": "../definitions.json#/error" + } + }, "404": { "description": "Hold not found", "schema": { @@ -296,6 +302,8 @@ } }, "x-koha-authorization": { + "allow-owner": true, + "allow-guarantor": true, "permissions": { "reserveforothers": "1" } diff --git a/t/db_dependent/api/v1/holds.t b/t/db_dependent/api/v1/holds.t index 7841d4a..136883b 100644 --- a/t/db_dependent/api/v1/holds.t +++ b/t/db_dependent/api/v1/holds.t @@ -180,7 +180,23 @@ subtest "Test endpoints without permission" => sub { ->status_is(403); }; subtest "Test endpoints without permission, but accessing own object" => sub { - plan tests => 15; + plan tests => 21; + + my $reserve_id3 = C4::Reserves::AddReserve($branchcode, $nopermission->{'borrowernumber'}, + $biblionumber, undef, 2, undef, undef, undef, '', $itemnumber, 'W'); + # try to delete my own hold while it's waiting; an error should occur + $tx = $t->ua->build_tx(DELETE => "/api/v1/holds/$reserve_id3" => json => $put_data); + $tx->req->cookies({name => 'CGISESSID', value => $session_nopermission->id}); + $t->request_ok($tx) + ->status_is(403) + ->json_is('/error', 'Hold is already in transfer or waiting and cannot be cancelled by patron.'); + # reset the hold's found status; after that, cancellation should work + Koha::Holds->find($reserve_id3)->found('')->store; + $tx = $t->ua->build_tx(DELETE => "/api/v1/holds/$reserve_id3" => json => $put_data); + $tx->req->cookies({name => 'CGISESSID', value => $session_nopermission->id}); + $t->request_ok($tx) + ->status_is(200); + is(Koha::Holds->find({ borrowernumber => $nopermission->{borrowernumber} }), undef, "Hold deleted."); my $borrno_tmp = $post_data->{'borrowernumber'}; $post_data->{'borrowernumber'} = int $nopermission->{'borrowernumber'}; @@ -201,7 +217,7 @@ subtest "Test endpoints without permission, but accessing own object" => sub { ->json_is('/0/expirationdate', $expirationdate) ->json_is('/0/branchcode', $branchcode); - my $reserve_id3 = Koha::Holds->find({ borrowernumber => $nopermission->{borrowernumber} })->reserve_id; + $reserve_id3 = Koha::Holds->find({ borrowernumber => $nopermission->{borrowernumber} })->reserve_id; $tx = $t->ua->build_tx(PUT => "/api/v1/holds/$reserve_id3" => json => $put_data); $tx->req->cookies({name => 'CGISESSID', value => $session_nopermission->id}); $t->request_ok($tx) # create hold to myself -- 1.9.1