From 28f9e9fde049fcb280367e3da9e505fbf3ea9f51 Mon Sep 17 00:00:00 2001 From: Tomas Cohen Arazi Date: Mon, 9 Jun 2025 19:42:06 -0300 Subject: [PATCH] Bug 39657: Add more checks and overrides to hold creation endpoint This patch adds new behaviors to the `POST /holds` endpoint. It uses the framework we created in the past for overriding policy rules, and add new options: * expired * debt_limit * bad_address * card_lost * restricted * hold_limit Some status codes are changed from 403 to 409. This should be revisited accross the codebase, as I think we made a wrong choice. Happy to review in this bug. The feature makes use of the newly introduced `$patron->can_place_holds()` method, which accepts to be passed through the overrides. To test: 1. Apply this patches 2. Run: $ ktd --shell k$ yarn api:bundle k$ koha-plack --restart kohadev k$ prove t/db_dependent/api/v1/holds.t => SUCCESS: Tests pass! 3. Test the endpoint with the various scenarios using your favourite REST tool (Postman!) 4. Sign off :-D Signed-off-by: Kristi Krueger Signed-off-by: Martin Renvoize --- Koha/REST/V1/Holds.pm | 31 ++++++++++++++++++++----------- 1 file changed, 20 insertions(+), 11 deletions(-) diff --git a/Koha/REST/V1/Holds.pm b/Koha/REST/V1/Holds.pm index 7f8764e6cb1..b6c80906e82 100644 --- a/Koha/REST/V1/Holds.pm +++ b/Koha/REST/V1/Holds.pm @@ -88,7 +88,9 @@ sub add { my $non_priority = $body->{non_priority}; my $overrides = $c->stash('koha.overrides'); - my $can_override = $overrides->{any} && C4::Context->preference('AllowHoldPolicyOverride'); + my $can_override = C4::Context->preference('AllowHoldPolicyOverride') // 0; + + my $override_all = $overrides->{any} && C4::Context->preference('AllowHoldPolicyOverride') ? 1 : 0; if ( !C4::Context->preference('AllowHoldDateInFuture') && $hold_date ) { return $c->render( @@ -142,7 +144,7 @@ sub add { } # If the hold is being forced, no need to validate - unless ($can_override) { + unless ($override_all) { # Validate pickup location my $valid_pickup_location; @@ -161,21 +163,28 @@ sub add { openapi => { error => 'The supplied pickup location is not valid' } ) unless $valid_pickup_location; - my $can_place_hold = + my $can_place_holds = $patron->can_place_holds(); + + if ( !$can_place_holds ) { + my $error_code = $can_place_holds->messages->[0]->message; + return $c->render( + status => 409, + openapi => { + error => 'Hold cannot be placed. Reason: ' . $error_code, + error_code => $error_code, + } + ) unless $overrides->{$error_code}; + } + + my $can_hold_be_placed = $item ? C4::Reserves::CanItemBeReserved( $patron, $item ) : C4::Reserves::CanBookBeReserved( $patron_id, $biblio_id ); - 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' ) { + unless ( $can_hold_be_placed->{status} eq 'OK' ) { return $c->render( status => 403, - openapi => { error => "Hold cannot be placed. Reason: " . $can_place_hold->{status} } + openapi => { error => "Hold cannot be placed. Reason: " . $can_hold_be_placed->{status} } ); } } -- 2.49.0