From c94dd86dee04fe09f8168a112869ee95d3ae71d3 Mon Sep 17 00:00:00 2001 From: Nick Clemens 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