From dc6c8e68ed351583379d728e5d96b5bef1416cae 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 Signed-off-by: Kyle M Hall --- C4/Reserves.pm | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/C4/Reserves.pm b/C4/Reserves.pm index bf145845ee..3962a95d70 100644 --- a/C4/Reserves.pm +++ b/C4/Reserves.pm @@ -1685,8 +1685,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.24.1 (Apple Git-126)