Attachment 149410 Details for Bug 33470 – Bug 33470: Don't calculate overridden hold limits

[patch] Bug 33470: Don't calculate overridden hold limits

Bug-33470-Dont-calculate-overridden-hold-limits.patch (text/plain), 4.22 KB, created by Emily Lamancusa (emlam) on 2023-04-10 19:49:10 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Emily Lamancusa (emlam)

Created: 2023-04-10 19:49:10 UTC

Size: 4.22 KB

patch

obsolete

>From 742914bc7d128c58c42bd2c5bb65a494b7771493 Mon Sep 17 00:00:00 2001
>From: Nick Clemens <nick@bywatersolutions.com>
>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 <emily.lamancusa@montgomerycountymd.gov>
>---
> 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.34.1

Actions: View | Diff | Splinter Review

Attachments on bug 33470: 149378 | 149410 | 150581