From 4330dd35236be6153ce8cb4e0144e2bc4fe51330 Mon Sep 17 00:00:00 2001 From: Nick Clemens Date: Mon, 12 Aug 2019 15:53:06 +0000 Subject: [PATCH] Bug 18958: Make check reserves find in transit holds To test: Imagine you have a record with 3 items, each item is held by LibraryA, LibraryB and LibraryC respectively. Now, a patron places two record level holds for pickup at LibraryD. When any of the items are checked in, that item is trapped to fill the first hold, but a side affect is that the next open hold because item level for the item that was just checked in! This is clearly incorrect and prevents the patron from placing more record level holds on the record. The problem seems to be that we use GetOtherReserves to initiate transfers for holds This routine calls CheckReserves to see what holds are on the record. We have an implicit assumption here that anything matching the holds queue is the correct hold. This worked when we couldn't have multiple holds on a record, but now it can end up with false positive. The solution seems to be returning the in-transit hold before returning a second hold on the record to make sure we affect the correct reserve. Test Plan: 1 - Create and use a patron that can place multiple record level holds per record 2 - Create a record with X items, each at a different library 3 - Place X 'Next available' holds on the record for the patron using the 'Holds to place' box 4 - perl misc/cronjobs/holds/build_holdsqueue.pl 5 - Check in LibraryA's copy as LibraryA and confirm the hold 6 - Revisit request.pl for the record, notice the next hold in line is now item-specific 7 - Checkout the item to the patron, notice the remaining hold is marked waiting 8 - Attempt to place another hold for your patron, notice that it requires an item-specific hold 8 - Apply this patch 9 - Repeat steps 1-5 10 - Revisit request.pl for the record, notice the next hold in line has *not* become item-specific 11 - Checkout the item to the patron, ensure the first hold is filled and the second remains record level 12 - Repeat whole test plan without building holds queue to confirm holds are still treated correctly Signed-off-by: Bonnie Gardner Signed-off-by: Bouzid Fergani --- C4/Reserves.pm | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/C4/Reserves.pm b/C4/Reserves.pm index 2a77c3d..a680f0f 100644 --- a/C4/Reserves.pm +++ b/C4/Reserves.pm @@ -1577,8 +1577,7 @@ sub _Findgroupreserve { FROM reserves JOIN biblioitems USING (biblionumber) JOIN hold_fill_targets USING (biblionumber, borrowernumber) - WHERE found IS NULL - AND priority > 0 + WHERE ( ( found IS NULL AND priority > 0 ) OR found='T' ) AND item_level_request = 0 AND hold_fill_targets.itemnumber = ? AND reservedate <= DATE_ADD(NOW(),INTERVAL ? DAY) -- 2.7.4