From 309e1b0cb8df29265919080e8c772fe3997a69e3 Mon Sep 17 00:00:00 2001 From: Nick Clemens Date: Sun, 9 Apr 2023 18:25:41 +0000 Subject: [PATCH] Bug 33470: Don't calculate overridden hold limits This patch can largely be checked in the code to ensure it makes sense, it moves a conditional up one level for the two different checks that could prevent holds To test: 1 - Attempt to place a hold via the REST API with an invalid pickup location Set pickup location as no in branches to make this easy 2 - Confirm you get an error 3 - Attempt again with header (bug 27760): x-koha-override: any 4 - Hold is placed 5 - Repeat 1-4 above but with an item that cannot be held 6 - Apply patch 7 - Repeat 1-4 above 8 - Results should be the same Signed-off-by: Emily Lamancusa Signed-off-by: Jonathan Druart Signed-off-by: Tomas Cohen Arazi (cherry picked from commit e3c31212df4d1917004a20998f47777ef0aea35d) Signed-off-by: Martin Renvoize --- Koha/REST/V1/Holds.pm | 69 ++++++++++++++++++++++--------------------- 1 file changed, 36 insertions(+), 33 deletions(-) diff --git a/Koha/REST/V1/Holds.pm b/Koha/REST/V1/Holds.pm index afd62badca..a8a998ef12 100644 --- a/Koha/REST/V1/Holds.pm +++ b/Koha/REST/V1/Holds.pm @@ -143,43 +143,46 @@ sub add { ); } - # Validate pickup location - my $valid_pickup_location; - if ($item) { # item-level hold - $valid_pickup_location = - any { $_->branchcode eq $pickup_library_id } - $item->pickup_locations( - { patron => $patron } )->as_list; - } - else { - $valid_pickup_location = - any { $_->branchcode eq $pickup_library_id } - $biblio->pickup_locations( - { patron => $patron } )->as_list; - } - - return $c->render( - status => 400, - openapi => { - error => 'The supplied pickup location is not valid' + # If the hold is being forced, no need to validate + unless( $can_override ){ + # Validate pickup location + my $valid_pickup_location; + if ($item) { # item-level hold + $valid_pickup_location = + any { $_->branchcode eq $pickup_library_id } + $item->pickup_locations( + { patron => $patron } )->as_list; + } + else { + $valid_pickup_location = + any { $_->branchcode eq $pickup_library_id } + $biblio->pickup_locations( + { patron => $patron } )->as_list; } - ) unless $valid_pickup_location || $can_override; - my $can_place_hold - = $item - ? C4::Reserves::CanItemBeReserved( $patron, $item ) - : C4::Reserves::CanBookBeReserved( $patron_id, $biblio_id ); + return $c->render( + status => 400, + openapi => { + error => 'The supplied pickup location is not valid' + } + ) unless $valid_pickup_location; - if ( C4::Context->preference('maxreserves') && $patron->holds->count + 1 > C4::Context->preference('maxreserves') ) { - $can_place_hold->{status} = 'tooManyReserves'; - } + my $can_place_hold + = $item + ? C4::Reserves::CanItemBeReserved( $patron, $item ) + : C4::Reserves::CanBookBeReserved( $patron_id, $biblio_id ); - unless ( $can_override || $can_place_hold->{status} eq 'OK' ) { - return $c->render( - status => 403, - openapi => - { error => "Hold cannot be placed. Reason: " . $can_place_hold->{status} } - ); + if ( C4::Context->preference('maxreserves') && $patron->holds->count + 1 > C4::Context->preference('maxreserves') ) { + $can_place_hold->{status} = 'tooManyReserves'; + } + + unless ( $can_place_hold->{status} eq 'OK' ) { + return $c->render( + status => 403, + openapi => + { error => "Hold cannot be placed. Reason: " . $can_place_hold->{status} } + ); + } } my $priority = C4::Reserves::CalculatePriority($biblio_id); -- 2.39.5