Attachment 107576 Details for Bug 18958 – Bug 18958: Make check reserves find in transit holds

[patch] Bug 18958: Make check reserves find in transit holds

Bug-18958-Make-check-reserves-find-in-transit-hold.patch (text/plain), 2.90 KB, created by Nick Clemens (kidclamp) on 2020-07-30 11:41:35 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Nick Clemens (kidclamp)

Created: 2020-07-30 11:41:35 UTC

Size: 2.90 KB

patch

obsolete

>From 0faaad981e72a010463481643af77f9f482239ed Mon Sep 17 00:00:00 2001
>From: Nick Clemens <nick@bywatersolutions.com>
>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 <bgardner@cityoflewiston.org>
>Signed-off-by: Bouzid Fergani <bouzid.fergani@inlibro.com>
>---
> 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.11.0

Actions: View | Diff | Splinter Review

Attachments on bug 18958: 65104 | 65150 | 65184 | 71164 | 71232 | 72210 | 72211 | 72348 | 72349 | 92159 | 92160 | 93106 | 93107 | 93108 | 93111 | 93112 | 93113 | 93216 | 93217 | 93218 | 107576 | 107577 | 107582 | 107584 | 107585 | 107586 | 108256 | 108274 | 108473 | 108474 | 108497 | 108499 | 108500 | 108936 | 108937 | 109061 | 109222 | 109640 | 109641 | 109642 | 109643 | 109644 | 109645