From 7345c965670fe37269bdb17941c2eb6115031c02 Mon Sep 17 00:00:00 2001 From: Magnus Enger Date: Tue, 19 Mar 2024 21:31:12 +0100 Subject: [PATCH] Bug 34902: decreaseLoanHighHolds does not reduce loan period See the bug for a description of the problem. Settings: - decreaseLoanHighHolds = Enable - decreaseLoanHighHoldsDuration = 7 - decreaseLoanHighHoldsValue = 1 - decreaseLoanHighHoldsControl = over the number of holdable items on the record - decreaseLoanHighHoldsIgnoreStatuses = [none selcted] Set a default loan period of 28 days under Administration > Circulation and fine rules To reproduce: - Find a record with one item - Add three holds on the record, for three different patrons - Check out the item to the first patron on the waiting list - Verify that the item is issued without any shortened loan time To test: - Return the loan you made above, and ignore the holds on the record - Add another hold to the record - Apply the patch - Restart all the things - Check out the item to the first patron on the waiting list - Verify that there is now a warning about a shortened loan time --- C4/Circulation.pm | 2 +- t/db_dependent/DecreaseLoanHighHolds.t | 50 +++++++++++++++++++------- 2 files changed, 38 insertions(+), 14 deletions(-) diff --git a/C4/Circulation.pm b/C4/Circulation.pm index 6df7ff8341..de81dd75a3 100644 --- a/C4/Circulation.pm +++ b/C4/Circulation.pm @@ -1445,7 +1445,7 @@ sub checkHighHolds { # If the number of holds is less than the count of items we have # plus the number of holds allowed above that count, we can stop here - if ( $holds->count() <= $threshold ) { + if ( $holds->count() < $threshold ) { return $return_data; } } diff --git a/t/db_dependent/DecreaseLoanHighHolds.t b/t/db_dependent/DecreaseLoanHighHolds.t index 2d42d28b9b..70a47ecb9f 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 => 26; +use Test::More tests => 31; my $dbh = C4::Context->dbh; my $schema = Koha::Database->new()->schema(); @@ -99,6 +99,9 @@ my $item = shift(@items); my $patron = shift(@patrons); my $patron_hold = Koha::Holds->find({ borrowernumber => $patron->borrowernumber, biblionumber => $item->biblionumber }); +my $holds = Koha::Holds->search({ biblionumber => $item->biblionumber }); +is( $holds->count, 6, "Should have 6 holds in total" ); + Koha::CirculationRules->set_rules( { branchcode => undef, @@ -165,9 +168,18 @@ is($duedate->min, $orig_due->min, 'New due minute is equal to original due minut is($duedate->sec, 0, 'New due date second is zero.'); t::lib::Mocks::mock_preference( 'decreaseLoanHighHoldsControl', 'dynamic' ); + +# Note in counts below, patron's own hold is not counted ("outstanding holds" is +# the total number of holds minus the hold for the patron we are looking at) +# In the tests below, we have: +# threshold = holdable items count + decreaseLoanHighHoldsValue; +# If the number of outstanding holds is greater than or equal to the threshold, +# decreaseLoanHighHolds will be activated + +# 10 items, 5 outstanding holds $data = C4::Circulation::checkHighHolds( $item, $patron ); is( $data->{exceeded}, 0, "Should not exceed threshold" ); - +is( $data->{outstanding}, 5, "Should have 5 outstanding holds" ); # Place 7 more holds - patrons 5,6,7,8,9,10,11 for my $i ( 5 .. 11 ) { @@ -180,24 +192,36 @@ for my $i ( 5 .. 11 ) { } )->store(); } +$holds = Koha::Holds->search({ biblionumber => $item->biblionumber }); +is( $holds->count, 13, "Should have 13 holds in total" ); -# Note in counts below, patron's own hold is not counted +# 12 holds, threshold is 0 over 10 holdable items = 10 +t::lib::Mocks::mock_preference( 'decreaseLoanHighHoldsValue', 0 ); +$data = C4::Circulation::checkHighHolds( $item, $patron ); +is( $data->{exceeded}, 1, "Should exceed decreaseLoanHighHoldsValue of 0" ); +is( $data->{outstanding}, 12, "Should have 12 outstanding holds" ); # 12 holds, threshold is 1 over 10 holdable items = 11 +t::lib::Mocks::mock_preference( 'decreaseLoanHighHoldsValue', 1 ); $data = C4::Circulation::checkHighHolds( $item, $patron ); -is( $data->{exceeded}, 1, "Should exceed threshold of 1" ); -is( $data->{outstanding}, 12, "Should exceed threshold of 1" ); +is( $data->{exceeded}, 1, "Should exceed decreaseLoanHighHoldsValue of 1" ); -# 12 holds, threshold is 2 over 10 holdable items = 12 (equal is okay) +# 12 holds, threshold is 2 over 10 holdable items = 12 +# The numbers are equal, so we should still exceed the threshold and trigger decreaseLoanHighHolds t::lib::Mocks::mock_preference( 'decreaseLoanHighHoldsValue', 2 ); $data = C4::Circulation::checkHighHolds( $item, $patron ); -is( $data->{exceeded}, 0, "Should not exceed threshold of 2" ); +is( $data->{exceeded}, 1, "Should exceed decreaseLoanHighHoldsValue of 2" ); + +# 12 holds, threshold is 3 over 10 holdable items = 13 +t::lib::Mocks::mock_preference( 'decreaseLoanHighHoldsValue', 3 ); +$data = C4::Circulation::checkHighHolds( $item, $patron ); +is( $data->{exceeded}, 0, "Should not exceed decreaseLoanHighHoldsValue of 3" ); my $unholdable = pop(@items); $unholdable->damaged(-1); $unholdable->store(); -# 12 holds, threshold is 2 over 9 holdable items = 11 +# 12 holds, threshold is 3 over 9 holdable items = 12 $data = C4::Circulation::checkHighHolds( $item, $patron ); is( $data->{exceeded}, 1, "Should exceed threshold with one damaged item" ); @@ -205,7 +229,7 @@ $unholdable->damaged(0); $unholdable->itemlost(-1); $unholdable->store(); -# 12 holds, threshold is 2 over 9 holdable items = 11 +# 12 holds, threshold is 3 over 9 holdable items = 12 $data = C4::Circulation::checkHighHolds( $item, $patron ); is( $data->{exceeded}, 1, "Should exceed threshold with one lost item" ); @@ -213,7 +237,7 @@ $unholdable->itemlost(0); $unholdable->notforloan(-1); $unholdable->store(); -# 12 holds, threshold is 2 over 9 holdable items = 11 +# 12 holds, threshold is 3 over 9 holdable items = 12 $data = C4::Circulation::checkHighHolds( $item, $patron ); is( $data->{exceeded}, 1, "Should exceed threshold with one notforloan item" ); @@ -221,14 +245,14 @@ $unholdable->notforloan(0); $unholdable->withdrawn(-1); $unholdable->store(); -# 12 holds, threshold is 2 over 9 holdable items = 11 +# 12 holds, threshold is 3 over 9 holdable items = 12 $data = C4::Circulation::checkHighHolds( $item, $patron ); 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 +# 11 holds, threshold is 3 over 9 holdable items = 12 $data = C4::Circulation::checkHighHolds( $item, $patron ); -is( $data->{exceeded}, 1, "Should exceed threshold with one withdrawn item" ); +is( $data->{exceeded}, 1, "Should exceed threshold with one found item" ); $patron_hold->found(undef)->store; t::lib::Mocks::mock_preference('CircControl', 'PatronLibrary'); -- 2.34.1