From 111d948a2c5861254d51704180850441a698c8e7 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 --- C4/Reserves.pm | 19 +++++++------------ 1 file changed, 7 insertions(+), 12 deletions(-) diff --git a/C4/Reserves.pm b/C4/Reserves.pm index 097a5c09d6..cfdfb5ec1d 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.11.0