From 36ee00ed952ed9081eeeca5e98fe006fbaa12db6 Mon Sep 17 00:00:00 2001 From: Nick Clemens Date: Fri, 28 Jun 2019 12:37:25 +0000 Subject: [PATCH] Bug 14549: Check patrons reserves for any holds on a bib when checking out an item MoveReserve uses CheckReserves to see if the current patron has any holds on the title they are checking out, however, CheckReserves doesn't return all holds on a biblio, it returns holds on the item from the holdsqueue if they exist This can create a condition where we check holds on an item, find we have it planned for another borrower, confirm checkout to the current borrower, but don't fill their hold To test: 1) Find record 2) place record level holds for 2 different patrons (record level) 3) Run holds queue builder, check the queue to confirm an item was selected for patron 1 Circulation->Holds queue->Library="All" 4) Check out the item queued for patron with priority 1 to the second patron 5) You should be asked to confirm, do so 6) Note the item checks out, but both holds remain 7) Apply patch 8) Check in the item 9) Don't confirm the hold 10) Check the holds on the record and the holds queue 11) Patron 1 should be priority 1 with an item selected from the holds queue 12) Checkout to patron 2 as before 13) Note the hold for patron 2 is filled this time 14) Prove -v t/db_dependent/Reserves.t Signed-off-by: Chris Cormack Signed-off-by: Tomas Cohen Arazi Signed-off-by: Martin Renvoize --- C4/Reserves.pm | 19 +++++++------------ 1 file changed, 7 insertions(+), 12 deletions(-) diff --git a/C4/Reserves.pm b/C4/Reserves.pm index be51d56be2..b54b41d39a 100644 --- a/C4/Reserves.pm +++ b/C4/Reserves.pm @@ -737,11 +737,9 @@ sub CheckReserves { } # note: we get the itemnumber because we might have started w/ just the barcode. Now we know for sure we have it. my ( $biblio, $bibitem, $notforloan_per_itemtype, $notforloan_per_item, $itemnumber, $damaged, $item_homebranch, $item_holdingbranch ) = $sth->fetchrow_array; - return if ( $damaged && !C4::Context->preference('AllowHoldsOnDamagedItems') ); return unless $itemnumber; # bail if we got nothing. - # if item is not for loan it cannot be reserved either..... # except where items.notforloan < 0 : This indicates the item is holdable. return if ( $notforloan_per_item > 0 ) or $notforloan_per_itemtype; @@ -979,7 +977,6 @@ sub ModReserveFill { my $reserve_id = $res->{'reserve_id'}; my $hold = Koha::Holds->find($reserve_id); - # get the priority on this record.... my $priority = $hold->priority; @@ -1795,18 +1792,16 @@ sub MoveReserve { # The item is reserved by someone else. # Find this item in the reserves - my $borr_res; - foreach (@$all_reserves) { - $_->{'borrowernumber'} == $borrowernumber or next; - $_->{'biblionumber'} == $biblionumber or next; - - $borr_res = $_; - last; - } + my $borr_res = Koha::Holds->search({ + borrowernumber => $borrowernumber, + biblionumber => $biblionumber, + },{ + order_by => 'priority' + })->next(); if ( $borr_res ) { # The item is reserved by the current patron - ModReserveFill($borr_res); + ModReserveFill($borr_res->unblessed); } if ( $cancelreserve eq 'revert' ) { ## Revert waiting reserve to priority 1 -- 2.39.5