Attachment 128902 Details for Bug 29102 – Bug 29102: Do not count patron's own hold against limits

[patch] Bug 29102: Do not count patron's own hold against limits

Bug-29102-Do-not-count-patrons-own-hold-against-li.patch (text/plain), 5.37 KB, created by Nick Clemens (kidclamp) on 2021-12-23 17:18:36 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Nick Clemens (kidclamp)

Created: 2021-12-23 17:18:36 UTC

Size: 5.37 KB

patch

obsolete

>From c94dd86dee04fe09f8168a112869ee95d3ae71d3 Mon Sep 17 00:00:00 2001
>From: Nick Clemens <nick@bywatersolutions.com>
>Date: Thu, 23 Dec 2021 17:05:59 +0000
>Subject: [PATCH] Bug 29102: Do not count patron's own hold against limits
>
>This patch makes two changes:
>1 - The borrower's own holds are not counted towards HighHolds limit
>2 - We exclude all hold counts from CanItemBeReserved
>
>Previously a patron's hold could put the count over the threshhold, and
>if the patron is only allowed 1 hold per record, and the hold wasn't found before
>the checkout, it would make all items unholdable, thus lowering the theshhold for
>dynamic HighHolds
>
>To test:
>1 - Set sysaprefs:
>     decreaseLoanHighHolds  - enable
>     decreaseLoanHighHoldsDuration - 1
>     decreaseLoanHighHoldsValue - 1
>     decreaseLoanHighHoldsControl - dynamic
>     decreaseLoanHighHoldsIgnoreStatuses - blank
>2 - Set circ rules to allow 1 hold per record and loan period of 5
>3 - Find/create a record with 3 items
>4 - Place a title level hold for a patron
>5 - Attempt to checkout item - note warning about high holds
>6 - Cancel checkout
>7 - Set decreaseLoanHighHoldsControl - static
>8 - Attempt checkout - note warning about high holds
>9 - Apply patrch
>10 - Checkout item - no warning
>11 - checkin item, replace hold
>12 - Set decreaseLoanHighHoldsControl - dynamic
>13 - Checkout item - no warning
>---
> C4/Circulation.pm                      | 11 +++++++++--
> t/db_dependent/DecreaseLoanHighHolds.t | 16 ++++++----------
> 2 files changed, 15 insertions(+), 12 deletions(-)
>
>diff --git a/C4/Circulation.pm b/C4/Circulation.pm
>index c2edf81f94..dac5bb4f0e 100644
>--- a/C4/Circulation.pm
>+++ b/C4/Circulation.pm
>@@ -1289,7 +1289,11 @@ sub checkHighHolds {
>         due_date    => undef,
>     };
> 
>-    my $holds = Koha::Holds->search( { biblionumber => $item->{'biblionumber'} } );
>+    # Count holds on this record, ignoring the borrowers own holds as they would be filled by the checkout
>+    my $holds = Koha::Holds->search({
>+        biblionumber => $item->{'biblionumber'},
>+        borrowernumber => { '!=' => $borrower->{borrowernumber} }
>+    });
> 
>     if ( $holds->count() ) {
>         $return_data->{outstanding} = $holds->count();
>@@ -1322,7 +1326,9 @@ sub checkHighHolds {
>             }
> 
>             # Remove any items that are not holdable for this patron
>-            @items = grep { CanItemBeReserved( $borrower->{borrowernumber}, $_->itemnumber, undef, { ignore_found_holds => 1 } )->{status} eq 'OK' } @items;
>+            # We need to ignore hold counts as the borrower's own hold that will be filled by the checkout
>+            # could prevent them from placing further holds
>+            @items = grep { CanItemBeReserved( $borrower->{borrowernumber}, $_->itemnumber, undef, { ignore_hold_counts => 1 } )->{status} eq 'OK' } @items;
> 
>             my $items_count = scalar @items;
> 
>@@ -1468,6 +1474,7 @@ sub AddIssue {
>             );
>         }
>         else {
>+
>             unless ($datedue) {
>                 my $itype = $item_object->effective_itemtype;
>                 $datedue = CalcDateDue( $issuedate, $itype, $branchcode, $borrower );
>diff --git a/t/db_dependent/DecreaseLoanHighHolds.t b/t/db_dependent/DecreaseLoanHighHolds.t
>index e981e32aee..9998910d4e 100755
>--- a/t/db_dependent/DecreaseLoanHighHolds.t
>+++ b/t/db_dependent/DecreaseLoanHighHolds.t
>@@ -30,7 +30,7 @@ use Koha::CirculationRules;
> use t::lib::TestBuilder;
> use t::lib::Mocks;
> 
>-use Test::More tests => 22;
>+use Test::More tests => 23;
> 
> my $dbh    = C4::Context->dbh;
> my $schema = Koha::Database->new()->schema();
>@@ -132,7 +132,7 @@ my $patron_hr = { borrowernumber => $patron->id, branchcode => $library->{branch
> 
> my $data = C4::Circulation::checkHighHolds( $item_hr, $patron_hr );
> is( $data->{exceeded},        1,          "Static mode should exceed threshold" );
>-is( $data->{outstanding},     6,          "Should have 6 outstanding holds" );
>+is( $data->{outstanding},     5,          "Should have 5 outstanding holds" );
> is( $data->{duration},        0,          "Should have duration of 0 because of specific circulation rules" );
> is( ref( $data->{due_date} ), 'DateTime', "due_date should be a DateTime object" );
> 
>@@ -164,8 +164,8 @@ $data = C4::Circulation::checkHighHolds( $item_hr, $patron_hr );
> is( $data->{exceeded}, 0, "Should not exceed threshold" );
> 
> 
>-# Place 6 more holds - patrons 5,6,7,8,9,10
>-for my $i ( 5 .. 10 ) {
>+# Place 7 more holds - patrons 5,6,7,8,9,10,11
>+for my $i ( 5 .. 11 ) {
>     my $patron = $patrons[$i];
>     my $hold   = Koha::Hold->new(
>         {
>@@ -176,6 +176,8 @@ for my $i ( 5 .. 10 ) {
>     )->store();
> }
> 
>+# Note in counts below, patron's own hold is not counted
>+
> # 12 holds, threshold is 1 over 10 holdable items = 11
> $data = C4::Circulation::checkHighHolds( $item_hr, $patron_hr );
> is( $data->{exceeded}, 1, "Should exceed threshold of 1" );
>@@ -218,12 +220,6 @@ $unholdable->store();
> $data = C4::Circulation::checkHighHolds( $item_hr, $patron_hr );
> is( $data->{exceeded}, 1, "Should exceed threshold with one withdrawn item" );
> 
>-$patron_hold->found('F')->store;
>-# 11 holds, threshold is 2 over 9 holdable items = 11
>-$data = C4::Circulation::checkHighHolds( $item_hr, $patron_hr );
>-is( $data->{exceeded}, 1, "Should exceed threshold with one withdrawn item" );
>-$patron_hold->found(undef)->store;
>-
> t::lib::Mocks::mock_preference('CircControl', 'PatronLibrary');
> 
> my $patron_object = Koha::Patrons->find( $patron_hr->{borrowernumber} );
>-- 
>2.30.2

Actions: View | Diff | Splinter Review

Attachments on bug 29102: 128901 | 128902 | 128903 | 137722 | 137723 | 137745 | 137746 | 140970 | 140971 | 142990 | 142991