From 861c0f91e56f81ed7a5d124aedf630ad5b28cdb6 Mon Sep 17 00:00:00 2001 From: Alex Arnaud Date: Wed, 10 Feb 2016 17:46:07 +0100 Subject: [PATCH] Bug 15567 - Allow to place multiple at once via RESTFUL Test plan: Same tests than Bug 15565 but via RESTFUL Signed-off-by: Benjamin Rokseth --- Koha/REST/V1/Reserve.pm | 155 +++++++++++++++++++++++++-------------- api/v1/swagger.json | 54 ++++++++------ t/db_dependent/api/v1/reserves.t | 68 ++++++++++++++--- 3 files changed, 190 insertions(+), 87 deletions(-) diff --git a/Koha/REST/V1/Reserve.pm b/Koha/REST/V1/Reserve.pm index 69e6e98..d3a7cec 100644 --- a/Koha/REST/V1/Reserve.pm +++ b/Koha/REST/V1/Reserve.pm @@ -44,71 +44,118 @@ 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::Patrons->find($borrowernumber); - unless ($borrower) { - return $c->$cb({error => "Borrower not found"}, 404); + my $placed; + my $sorted_requests; + foreach my $requested (@$body) { + my $key = $requested->{borrowernumber} . '_' . $requested->{biblionumber}; + push @{ $sorted_requests->{$key} }, $requested; } - 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); - } + foreach my $key (keys %$sorted_requests) { + my $requests = $sorted_requests->{$key}; + my ($borrowernumber, $biblionumber) = split('_', $key); + + my $holds = Koha::Holds->search({ + borrowernumber => $borrowernumber, + biblionumber => $biblionumber, + found => undef + }); - if ($itemnumber) { - my $item_biblionumber = C4::Biblio::GetBiblionumberFromItemnumber($itemnumber); - if ($biblionumber and $biblionumber != $item_biblionumber) { + my $max_for_record = GetMaxPatronHoldsForRecord($borrowernumber, $biblionumber); + if ($holds->count() + scalar(@$requests) > $max_for_record) { return $c->$cb({ - error => "Item $itemnumber doesn't belong to biblio $biblionumber" + error => "Too many holds for this record" }, 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); - } - - my $priority = C4::Reserves::CalculatePriority($biblionumber); - $itemnumber ||= undef; + my @toreserve; + my $hold_level = $holds->forced_hold_level(); + foreach my $request (@$requests) { + my $itemnumber = $request->{itemnumber}; + my $branchcode = $request->{branchcode}; + my $expirationdate = $request->{expirationdate}; + + my $level = $itemnumber ? 'item' : 'record'; + if ($hold_level && $hold_level ne $level) { + return $c->$cb({ + error => "Differents holds level for the same record." + }, 400); + } + $hold_level = $level; + + my $borrower = Koha::Patrons->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); + } + + if ($itemnumber) { + my $item_biblionumber = C4::Biblio::GetBiblionumberFromItemnumber($itemnumber); + if ($itemnumber and $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); + } + + 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')); + } + + push @toreserve, { + branchcode => $branchcode, + borrowernumber => $borrowernumber, + biblionumber => $biblionumber, + priority => $priority, + expirationdate => $expirationdate, + title => $biblio->{title}, + itemnumber => $itemnumber, + }; + } - # AddReserve expects date to be in syspref format - if ($expirationdate) { - $expirationdate = output_pref(dt_from_string($expirationdate, 'iso')); - } + foreach my $r (@toreserve) { + my $reserve_id = C4::Reserves::AddReserve($r->{branchcode}, + $r->{borrowernumber}, $r->{biblionumber}, undef, $r->{priority}, undef, + $r->{expirationdate}, undef, $r->{title}, $r->{itemnumber}); - 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); + } - unless ($reserve_id) { - return $c->$cb({ - error => "Error while placing reserve. See Koha logs for details." - }, 500); + push @{ $placed }, C4::Reserves::GetReserve($reserve_id); + } } - - my $reserve = C4::Reserves::GetReserve($reserve_id); - - return $c->$cb($reserve, 201); + return $c->$cb($placed, 201); } sub edit { diff --git a/api/v1/swagger.json b/api/v1/swagger.json index b5c6d0a..27101b0 100644 --- a/api/v1/swagger.json +++ b/api/v1/swagger.json @@ -109,28 +109,31 @@ "description": "A JSON object containing informations about the new reserve", "required": true, "schema": { - "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" + "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" + } } } } @@ -140,8 +143,11 @@ "produces": ["application/json"], "responses": { "201": { - "description": "Created reserve", - "schema": { "$ref": "#/definitions/reserve" } + "description": "Created reserves", + "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 eeccbfd..af2d791 100644 --- a/t/db_dependent/api/v1/reserves.t +++ b/t/db_dependent/api/v1/reserves.t @@ -17,8 +17,9 @@ use Modern::Perl; -use Test::More tests => 30; +use Test::More tests => 43; use Test::Mojo; +use t::lib::TestBuilder; use DateTime; @@ -34,10 +35,12 @@ my $dbh = C4::Context->dbh; $dbh->{AutoCommit} = 0; $dbh->{RaiseError} = 1; +my $builder = t::lib::TestBuilder->new(); my $t = Test::Mojo->new('Koha::REST::V1'); -my $categorycode = Koha::Database->new()->schema()->resultset('Category')->first()->categorycode(); -my $branchcode = Koha::Database->new()->schema()->resultset('Branch')->first()->branchcode(); +my $categorycode = $builder->build({ source => 'Category' })->{ categorycode }; +my $categorycode2 = $builder->build({ source => 'Category' })->{ categorycode }; +my $branchcode = $builder->build({ source => 'Branch' })->{ branchcode }; my $borrower = Koha::Patron->new; $borrower->categorycode( $categorycode ); @@ -47,7 +50,7 @@ $borrower->store; my $borrowernumber = $borrower->borrowernumber; my $borrower2 = Koha::Patron->new; -$borrower2->categorycode( $categorycode ); +$borrower2->categorycode( $categorycode2 ); $borrower2->branchcode( $branchcode ); $borrower2->surname("Test Surname 2"); $borrower2->store; @@ -55,6 +58,12 @@ my $borrowernumber2 = $borrower2->borrowernumber; my $biblionumber = create_biblio('RESTful Web APIs'); my $itemnumber = create_item($biblionumber, 'TEST000001'); +my $itemnumber2 = create_item($biblionumber, 'TEST000002'); + +my $biblionumber2 = create_biblio('Calamity'); +my $itemnumber3 = create_item($biblionumber2, 'TEST000003'); +my $itemnumber4 = create_item($biblionumber2, 'TEST000004'); +my $itemnumber5 = create_item($biblionumber2, 'TEST000005'); my $reserve_id = C4::Reserves::AddReserve($branchcode, $borrowernumber, $biblionumber, undef, 1, undef, undef, undef, '', $itemnumber); @@ -99,21 +108,26 @@ $dbh->do('DELETE FROM issuingrules'); $dbh->do(q{ INSERT INTO issuingrules (categorycode, branchcode, itemtype, reservesallowed) VALUES (?, ?, ?, ?) -}, {}, '*', '*', '*', 1); +}, {}, $categorycode, '*', '*', 1); + +$dbh->do(q{ + INSERT INTO issuingrules (categorycode, branchcode, itemtype, holds_per_record, reservesallowed) + VALUES (?, ?, ?, ?, ?) +}, {}, $categorycode2, '*', '*', '3', 5); my $expirationdate = DateTime->now->add(days => 10)->ymd; -my $post_data = { +my $post_data = [{ 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) @@ -121,10 +135,46 @@ $t->get_ok("/api/v1/reserves?borrowernumber=$borrowernumber") ->json_is('/0/expirationdate', $expirationdate) ->json_is('/0/branchcode', $branchcode); +$post_data->[0]->{biblionumber} = int($biblionumber2); +$post_data->[0]->{itemnumber} = int($itemnumber3); $t->post_ok("/api/v1/reserves" => json => $post_data) ->status_is(403) ->json_like('/error', qr/tooManyReserves/); +$post_data = [ + { + borrowernumber => int($borrowernumber2), + biblionumber => int($biblionumber2), + itemnumber => int($itemnumber3), + branchcode => $branchcode, + }, + { + borrowernumber => int($borrowernumber2), + biblionumber => int($biblionumber2), + branchcode => $branchcode, + }, +]; + +$t->post_ok("/api/v1/reserves" => json => $post_data) + ->status_is(400) + ->json_like('/error', qr/Differents holds level/); + +delete $post_data->[0]->{itemnumber}; +$t->post_ok("/api/v1/reserves" => json => $post_data) + ->status_is(201) + ->json_has('/0/reserve_id') + ->json_has('/1/reserve_id'); + +$t->post_ok("/api/v1/reserves" => json => $post_data) + ->status_is(400) + ->json_like('/error', qr/Too many holds/); + +delete $post_data->[1]; +$post_data->[0]->{itemnumber} = int($itemnumber3); +$t->post_ok("/api/v1/reserves" => json => $post_data) + ->status_is(400) + ->json_like('/error', qr/Differents holds level/); + $dbh->rollback; -- 2.1.4