From 309e1b0cb8df29265919080e8c772fe3997a69e3 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>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
(cherry picked from commit e3c31212df4d1917004a20998f47777ef0aea35d)
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
---
 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