Attachment 111522 Details for Bug 26659 – Bug 26659: Fix incorrect comparison in checkout renewability

[patch] Bug 26659: Fix incorrect comparison in checkout renewability

Bug-26659-Fix-incorrect-comparison-in-checkout-ren.patch (text/plain), 2.42 KB, created by Joonas Kylmälä on 2020-10-13 08:24:18 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Joonas Kylmälä

Created: 2020-10-13 08:24:18 UTC

Size: 2.42 KB

patch

obsolete

>From e15684cbcb91daa82840eb18298a04e4fd222ad0 Mon Sep 17 00:00:00 2001
>From: =?UTF-8?q?Joonas=20Kylm=C3=A4l=C3=A4?= <joonas.kylmala@helsinki.fi>
>Date: Mon, 12 Oct 2020 14:54:37 +0000
>Subject: [PATCH] Bug 26659: Fix incorrect comparison in checkout renewability
>
>The syntax used here was wrong and it allowed incorrectly to renew
>checkouts which should not have been allowed because there were
>holds waiting to be filled. This scenario can be reproduced at least
>with using local hold groups.
>
>To test:
>
>1. In the circ rules for all libraries under "Default holds policy by item type" section
>   select for itemtype Book the policies "From local hold group" and "patron's hold group"
>2. Enable AllowRenewalIfOtherItemsAvailable
>3. Create local hold group for branch A and branch B (/cgi-bin/koha/admin/library_groups.pl)
>4. Create biblio with 2 items of itemtype Book, one in branch A and other in B
>5. Checkout item from branch A to patron PA
>6. Create bib level hold for patron PB whose home library is branch A.
>7. Try go to renew now the loan for patron PA and notice it would work
>8. Apply patch
>9. Try to go renew the loan for patron PA and notice it doesn't work.
>---
> C4/Circulation.pm | 11 ++++++++++-
> 1 file changed, 10 insertions(+), 1 deletion(-)
>
>diff --git a/C4/Circulation.pm b/C4/Circulation.pm
>index 2f91511dbd..ee1068ba22 100644
>--- a/C4/Circulation.pm
>+++ b/C4/Circulation.pm
>@@ -2831,7 +2831,16 @@ sub CanBookBeRenewed {
>                 for my $borrowernumber (@borrowernumbers) {
>                     my $patron = $patrons{$borrowernumber} //= Koha::Patrons->find( $borrowernumber );
>                     next unless IsAvailableForItemLevelRequest($item, $patron);
>-                    next unless CanItemBeReserved($borrowernumber,$itemnumber);
>+                    my $can_reserve = CanItemBeReserved($borrowernumber,$itemnumber);
>+                    # Allowing tooMany* and itemAlreadyOnHold because we need to ignore the count for existing holds.
>+                    my @allowed_statuses = (
>+                        'OK',
>+                        'tooManyReserves',
>+                        'tooManyHoldsForThisRecord',
>+                        'tooManyReservesToday',
>+                        'itemAlreadyOnHold'
>+                    );
>+                    next unless grep { $_ eq $can_reserve->{status} } @allowed_statuses;
> 
>                     push @reservable, $itemnumber;
>                     if (@reservable >= @borrowernumbers) {
>-- 
>2.11.0

Actions: View | Diff | Splinter Review

Attachments on bug 26659: 111517 | 111522 | 112402