From 6b29095a375e29142fcf63486a2524cf858764db Mon Sep 17 00:00:00 2001 From: Julian Maurice Date: Mon, 29 Feb 2016 10:07:06 +0100 Subject: [PATCH] Bug 15566: API - allow to reserve first available from a group of titles Test plan: 0. Find 1 borrower and 2 biblios they can place a hold on. Let's call them $borrowernumber, $biblionumber1 and $biblionumber2 1. Make a POST request to http://your.library.org/api/v1/reserves with the following body: { "onlyfirst": true, "reserves": [ { "borrowernumber": $borrowernumber, "biblionumber": $biblionumber1 }, { "borrowernumber": $borrowernumber, "biblionumber": $biblionumber2 }, ] } 2. Check that you get a 201 response with the reserves informations 3. Check in staff interface that the reserves were correctly made --- Koha/REST/V1/Reserve.pm | 137 ++++++++++++++++++++++----------------- api/v1/definitions/reserve.json | 4 ++ api/v1/swagger.json | 56 ++++++++++------ t/db_dependent/api/v1/reserves.t | 53 ++++++++++++--- 4 files changed, 164 insertions(+), 86 deletions(-) diff --git a/Koha/REST/V1/Reserve.pm b/Koha/REST/V1/Reserve.pm index beb0abb..5339798 100644 --- a/Koha/REST/V1/Reserve.pm +++ b/Koha/REST/V1/Reserve.pm @@ -44,70 +44,91 @@ sub add { my $body = $c->req->json; - my $borrowernumber = $body->{borrowernumber}; - my $biblionumber = $body->{biblionumber}; - my $itemnumber = $body->{itemnumber}; - my $branchcode = $body->{branchcode}; - my $expirationdate = $body->{expirationdate}; - - my $borrower = Koha::Borrowers->find($borrowernumber); - unless ($borrower) { - return $c->$cb({error => "Borrower not found"}, 404); - } - - unless ($biblionumber or $itemnumber) { - return $c->$cb({ - error => "At least one of biblionumber, itemnumber should be given" - }, 400); - } - unless ($branchcode) { - return $c->$cb({ - error => "Branchcode is required" - }, 400); - } - - my $item_biblionumber = C4::Biblio::GetBiblionumberFromItemnumber($itemnumber); - if ($biblionumber and $biblionumber != $item_biblionumber) { - return $c->$cb({ - error => "Item $itemnumber doesn't belong to biblio $biblionumber" - }, 400); + my $onlyfirst = $body->{onlyfirst}; + my $reserve_group_id; + if ($onlyfirst) { + $reserve_group_id = AddReserveGroup(); } - $biblionumber ||= $item_biblionumber; - my $biblio = C4::Biblio::GetBiblio($biblionumber); - - my $can_reserve = - $itemnumber - ? CanItemBeReserved( $borrowernumber, $itemnumber ) - : CanBookBeReserved( $borrowernumber, $biblionumber ); - - unless ($can_reserve eq 'OK') { - return $c->$cb({ - error => "Reserve cannot be placed. Reason: $can_reserve" - }, 403); + my @possible_reserves; + foreach my $reserve (@{ $body->{reserves} }) { + my $borrowernumber = $reserve->{borrowernumber}; + my $biblionumber = $reserve->{biblionumber}; + my $itemnumber = $reserve->{itemnumber}; + my $branchcode = $reserve->{branchcode}; + my $expirationdate = $reserve->{expirationdate}; + + my $borrower = Koha::Borrowers->find($borrowernumber); + unless ($borrower) { + return $c->$cb({error => "Borrower not found"}, 404); + } + + unless ($biblionumber or $itemnumber) { + return $c->$cb({ + error => "At least one of biblionumber, itemnumber should be given" + }, 400); + } + unless ($branchcode) { + return $c->$cb({ + error => "Branchcode is required" + }, 400); + } + + my $item_biblionumber = C4::Biblio::GetBiblionumberFromItemnumber($itemnumber); + if ($biblionumber and $biblionumber != $item_biblionumber) { + return $c->$cb({ + error => "Item $itemnumber doesn't belong to biblio $biblionumber" + }, 400); + } + + $biblionumber ||= $item_biblionumber; + my $biblio = C4::Biblio::GetBiblio($biblionumber); + + my $can_reserve = + $itemnumber + ? CanItemBeReserved( $borrowernumber, $itemnumber ) + : CanBookBeReserved( $borrowernumber, $biblionumber ); + + unless ($can_reserve eq 'OK') { + return $c->$cb({ + error => "Reserve cannot be placed. Reason: $can_reserve" + }, 403); + } + + $reserve->{title} = $biblio->{title}; + push @possible_reserves, $reserve; } - my $priority = C4::Reserves::CalculatePriority($biblionumber); - $itemnumber ||= undef; - - # AddReserve expects date to be in syspref format - if ($expirationdate) { - $expirationdate = output_pref(dt_from_string($expirationdate, 'iso')); + my @reserves; + foreach my $reserve (@possible_reserves) { + my $borrowernumber = $reserve->{borrowernumber}; + my $biblionumber = $reserve->{biblionumber}; + my $itemnumber = $reserve->{itemnumber}; + my $branchcode = $reserve->{branchcode}; + my $expirationdate = $reserve->{expirationdate}; + + my $priority = C4::Reserves::CalculatePriority($biblionumber); + $itemnumber ||= undef; + + # AddReserve expects date to be in syspref format + if ($expirationdate) { + $expirationdate = output_pref(dt_from_string($expirationdate, 'iso')); + } + + my $reserve_id = C4::Reserves::AddReserve($branchcode, $borrowernumber, + $biblionumber, undef, $priority, undef, $expirationdate, undef, + $reserve->{title}, $itemnumber, undef, $reserve_group_id); + + unless ($reserve_id) { + return $c->$cb({ + error => "Error while placing reserve. See Koha logs for details." + }, 500); + } + + push @reserves, C4::Reserves::GetReserve($reserve_id); } - my $reserve_id = C4::Reserves::AddReserve($branchcode, $borrowernumber, - $biblionumber, undef, $priority, undef, $expirationdate, undef, - $biblio->{title}, $itemnumber); - - unless ($reserve_id) { - return $c->$cb({ - error => "Error while placing reserve. See Koha logs for details." - }, 500); - } - - my $reserve = C4::Reserves::GetReserve($reserve_id); - - return $c->$cb($reserve, 201); + return $c->$cb(\@reserves, 201); } sub edit { diff --git a/api/v1/definitions/reserve.json b/api/v1/definitions/reserve.json index 74370fe..6db6f51 100644 --- a/api/v1/definitions/reserve.json +++ b/api/v1/definitions/reserve.json @@ -58,6 +58,10 @@ }, "suspend_until": { "description": "" + }, + "reserve_group_id": { + "description": "Identifier of the group the reserve belongs to", + "type": ["string", "null"] } } } diff --git a/api/v1/swagger.json b/api/v1/swagger.json index b5c6d0a..35d1b2a 100644 --- a/api/v1/swagger.json +++ b/api/v1/swagger.json @@ -111,26 +111,39 @@ "schema": { "type": "object", "properties": { - "borrowernumber": { - "description": "Borrower internal identifier", - "type": "integer" + "reserves": { + "description": "Reserves informations", + "type": "array", + "items": { + "type": "object", + "properties": { + "borrowernumber": { + "description": "Borrower internal identifier", + "type": "integer" + }, + "biblionumber": { + "description": "Biblio internal identifier", + "type": "integer" + }, + "itemnumber": { + "description": "Item internal identifier", + "type": "integer" + }, + "branchcode": { + "description": "Pickup location", + "type": "string" + }, + "expirationdate": { + "description": "Reserve end date", + "type": "string", + "format": "date" + } + } + } }, - "biblionumber": { - "description": "Biblio internal identifier", - "type": "integer" - }, - "itemnumber": { - "description": "Item internal identifier", - "type": "integer" - }, - "branchcode": { - "description": "Pickup location", - "type": "string" - }, - "expirationdate": { - "description": "Reserve end date", - "type": "string", - "format": "date" + "onlyfirst": { + "description": "Reserve only first available item", + "type": "boolean" } } } @@ -141,7 +154,10 @@ "responses": { "201": { "description": "Created reserve", - "schema": { "$ref": "#/definitions/reserve" } + "schema": { + "type": "array", + "items": { "$ref": "#/definitions/reserve" } + } }, "400": { "description": "Missing or wrong parameters", diff --git a/t/db_dependent/api/v1/reserves.t b/t/db_dependent/api/v1/reserves.t index 9741542..c8d6815 100644 --- a/t/db_dependent/api/v1/reserves.t +++ b/t/db_dependent/api/v1/reserves.t @@ -17,7 +17,7 @@ use Modern::Perl; -use Test::More tests => 30; +use Test::More tests => 37; use Test::Mojo; use DateTime; @@ -103,17 +103,21 @@ $dbh->do(q{ my $expirationdate = DateTime->now->add(days => 10)->ymd; my $post_data = { - borrowernumber => int($borrowernumber), - biblionumber => int($biblionumber), - itemnumber => int($itemnumber), - branchcode => $branchcode, - expirationdate => $expirationdate, + reserves => [ + { + borrowernumber => int($borrowernumber), + biblionumber => int($biblionumber), + itemnumber => int($itemnumber), + branchcode => $branchcode, + expirationdate => $expirationdate, + }, + ], }; $t->post_ok("/api/v1/reserves" => json => $post_data) ->status_is(201) - ->json_has('/reserve_id'); + ->json_has('/0/reserve_id'); -$reserve_id = $t->tx->res->json->{reserve_id}; +$reserve_id = $t->tx->res->json->[0]->{reserve_id}; $t->get_ok("/api/v1/reserves?borrowernumber=$borrowernumber") ->status_is(200) @@ -125,6 +129,39 @@ $t->post_ok("/api/v1/reserves" => json => $post_data) ->status_is(403) ->json_like('/error', qr/tooManyReserves/); +$t->delete_ok("/api/v1/reserves/$reserve_id") + ->status_is(200); + +my $biblionumber2 = create_biblio('RESTful Web APIs 2'); +my $itemnumber2 = create_item($biblionumber2, 'TEST000002'); + +$post_data = { + onlyfirst => \1, + reserves => [ + { + borrowernumber => int($borrowernumber), + biblionumber => int($biblionumber), + itemnumber => int($itemnumber), + branchcode => $branchcode, + expirationdate => $expirationdate, + }, + { + borrowernumber => int($borrowernumber), + biblionumber => int($biblionumber2), + itemnumber => int($itemnumber2), + branchcode => $branchcode, + expirationdate => $expirationdate, + }, + ], +}; +$t->post_ok("/api/v1/reserves" => json => $post_data) + ->status_is(201); + +$t->get_ok("/api/v1/reserves?borrowernumber=$borrowernumber") + ->status_is(200); + +my $json = $t->tx->res->json; +is($json->[0]->{reserve_group_id}, $json->[1]->{reserve_group_id}, "Both reserves belong to the same reserve group"); $dbh->rollback; -- 1.9.1