Bugzilla – Attachment 50083 Details for
Bug 15567
Update API to allow multiple holds for the same record
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 15567 - Allow to place multiple at once via RESTFUL
Bug-15567---Allow-to-place-multiple-at-once-via-RE.patch (text/plain), 14.03 KB, created by
Benjamin Rokseth
on 2016-04-08 13:43:19 UTC
(
hide
)
Description:
Bug 15567 - Allow to place multiple at once via RESTFUL
Filename:
MIME Type:
Creator:
Benjamin Rokseth
Created:
2016-04-08 13:43:19 UTC
Size:
14.03 KB
patch
obsolete
>From 861c0f91e56f81ed7a5d124aedf630ad5b28cdb6 Mon Sep 17 00:00:00 2001 >From: Alex Arnaud <alex.arnaud@biblibre.com> >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 <benjamin.rokseth@kul.oslo.kommune.no> >--- > 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
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 15567
:
47860
| 50083