From a3870d9f41f13be815c2db458ab0c2386dfa0104 Mon Sep 17 00:00:00 2001 From: Magnus Enger Date: Mon, 20 Apr 2020 11:19:39 +0200 Subject: [PATCH] Bug 22005 - DecreaseLoanHighHolds counts holds wrong This patch adds some more comments and tests to the ones already in t/db_dependent/DecreaseLoanHighHolds.t. After applying just this patch on current master, running prove on the tests should. A followup patch will fix the underlying problem. --- t/db_dependent/DecreaseLoanHighHolds.t | 88 +++++++++++++++++++++++++++++----- 1 file changed, 76 insertions(+), 12 deletions(-) diff --git a/t/db_dependent/DecreaseLoanHighHolds.t b/t/db_dependent/DecreaseLoanHighHolds.t index f0bf9b61d0..5bf7ecfc27 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 => 17; +use Test::More tests => 42; my $dbh = C4::Context->dbh; my $schema = Koha::Database->new()->schema(); @@ -57,6 +57,7 @@ my $patron_category = $builder->build({ } }); +# Create 20 patrons my @patrons; for my $i ( 1 .. 20 ) { my $patron = Koha::Patron->new({ @@ -69,19 +70,24 @@ for my $i ( 1 .. 20 ) { push( @patrons, $patron ); } +# Create biblio my $biblio = $builder->build_sample_biblio(); +# Attach 10 items to the biblio my @items; for my $i ( 1 .. 10 ) { my $item = $builder->build_sample_item( { biblionumber => $biblio->id(), - itype => $itemtype + itype => $itemtype, + homebranch => $library->{branchcode}, + holdingbranch => $library->{branchcode}, } ); push( @items, $item ); } +# Add 6 holds to the biblio for my $i ( 0 .. 5 ) { my $patron = $patrons[$i]; my $hold = Koha::Hold->new( @@ -93,19 +99,20 @@ for my $i ( 0 .. 5 ) { )->store(); } +# Get the first item and first patron from the lists (this removes them from the lists) my $item = pop(@items); my $patron = pop(@patrons); Koha::CirculationRules->set_rules( { - branchcode => undef, - categorycode => undef, + branchcode => $library->{branchcode}, + categorycode => $category->{categorycode}, itemtype => $item->itype, rules => { - issuelength => '14', + issuelength => 14, lengthunit => 'days', - reservesallowed => '99', - holds_per_record => '99', + reservesallowed => 99, + holds_per_record => 99, } } ); @@ -118,6 +125,8 @@ my $orig_due = C4::Circulation::CalcDateDue( $patron->unblessed ); +## decreaseLoanHighHoldsControl = static + t::lib::Mocks::mock_preference( 'decreaseLoanHighHolds', 1 ); t::lib::Mocks::mock_preference( 'decreaseLoanHighHoldsDuration', 1 ); t::lib::Mocks::mock_preference( 'decreaseLoanHighHoldsValue', 1 ); @@ -128,8 +137,8 @@ my $item_hr = { itemnumber => $item->id, biblionumber => $biblio->id, homebranch my $patron_hr = { borrowernumber => $patron->id, branchcode => $library->{branchcode} }; my $data = C4::Circulation::checkHighHolds( $item_hr, $patron_hr ); -is( $data->{exceeded}, 1, "Static mode should exceed threshold" ); -is( $data->{outstanding}, 6, "Should have 5 outstanding holds" ); +is( $data->{exceeded}, 1, "Static mode should exceed threshold when decreaseLoanHighHoldsValue = 1" ); +is( $data->{outstanding}, 6, "Should have 6 outstanding holds" ); is( $data->{duration}, 1, "Should have duration of 1" ); is( ref( $data->{due_date} ), 'DateTime', "due_date should be a DateTime object" ); @@ -138,10 +147,45 @@ is($duedate->hour, $orig_due->hour, 'New due hour is equal to original due hour. is($duedate->min, $orig_due->min, 'New due minute is equal to original due minute.'); is($duedate->sec, 0, 'New due date second is zero.'); +# We have 6 holds so this SHOULD exceed the limit and trigger shortened loan time +t::lib::Mocks::mock_preference( 'decreaseLoanHighHoldsValue', 5 ); +$data = C4::Circulation::checkHighHolds( $item_hr, $patron_hr ); +is( $data->{exceeded}, 1, "Static mode should exceed threshold when decreaseLoanHighHoldsValue = 5" ); +is( $data->{outstanding}, 6, "Should have 6 outstanding holds" ); +is( $data->{duration}, 1, "Should have duration of 1" ); +is( ref( $data->{due_date} ), 'DateTime', "due_date should be a DateTime object" ); + +# We have 6 holds so this should NOT exceed the limit and trigger shortened loan time +# The sysprefs say: "[Enable] the reduction of loan period to [x] days for high demand items with more than [y] holds [on the record]." +# This must mean that if the number of holds and the value of decreaseLoanHighHoldsValue are equal, checkHighHolds should NOT be triggered +t::lib::Mocks::mock_preference( 'decreaseLoanHighHoldsValue', 6 ); +$data = C4::Circulation::checkHighHolds( $item_hr, $patron_hr ); +is( $data->{exceeded}, 0, "Static mode should exceed threshold when decreaseLoanHighHoldsValue = 6" ); +is( $data->{outstanding}, 6, "Should have 6 outstanding holds" ); +is( $data->{duration}, 0, "Should have duration of 1" ); +is( ref( $data->{due_date} ), '', "due_date should NOT be a DateTime object" ); + +# We have 6 holds so this should NOT exceed the limit and trigger shortened loan time +t::lib::Mocks::mock_preference( 'decreaseLoanHighHoldsValue', 7 ); +$data = C4::Circulation::checkHighHolds( $item_hr, $patron_hr ); +is( $data->{exceeded}, 0, "Static mode should not exceed threshold when decreaseLoanHighHoldsValue = 7" ); +is( $data->{outstanding}, 6, "Should have 6 outstanding holds" ); +is( $data->{duration}, 0, "Should have duration of 0" ); +is( ref( $data->{due_date} ), '', "due_date should not be a DateTime object" ); + +## decreaseLoanHighHoldsControl = dynamic + t::lib::Mocks::mock_preference( 'decreaseLoanHighHoldsControl', 'dynamic' ); +t::lib::Mocks::mock_preference( 'decreaseLoanHighHoldsValue', 1 ); + +# We have 10 items and 6 holds, so this should not exceed a threshold of 1 $data = C4::Circulation::checkHighHolds( $item_hr, $patron_hr ); -is( $data->{exceeded}, 0, "Should not exceed threshold" ); +is( $data->{exceeded}, 0, "Dynamic mode should not exceed threshold of 1" ); +is( $data->{outstanding}, 6, "Should have 6 outstanding holds" ); +is( $data->{duration}, 0, "Should have duration of 0" ); +is( ref( $data->{due_date} ), '', "due_date should not be a DateTime object" ); +# Add 6 more holds, for a total of 12 for my $i ( 5 .. 10 ) { my $patron = $patrons[$i]; my $hold = Koha::Hold->new( @@ -153,13 +197,33 @@ for my $i ( 5 .. 10 ) { )->store(); } +# We have 10 holdable items + decreaseLoanHighHoldsValue=1 = threshold of 11, versus 12 holds, so this should trigger decreased loan time $data = C4::Circulation::checkHighHolds( $item_hr, $patron_hr ); -is( $data->{exceeded}, 1, "Should exceed threshold of 1" ); +is( $data->{exceeded}, 1, "Dynamic mode should not exceed threshold when decreaseLoanHighHoldsValue = 1" ); +is( $data->{outstanding}, 12, "Should have 12 outstanding holds" ); +is( $data->{duration}, 1, "Should have duration of 1" ); +is( ref( $data->{due_date} ), 'DateTime', "due_date should be a DateTime object" ); +# We have 10 holdable items + decreaseLoanHighHoldsValue=2 = threshold of 12, versus 12 holds, so this should NOT trigger decreased loan time +# The sysprefs say: +# [Enable] the reduction of loan period to [x] days for high demand items with more than [y] holds [over the number of holdable items on the record]. +# This must mean that if the number of holdable items + decreaseLoanHighHoldsValue equals the number of holds, loan time should NOT be decreased t::lib::Mocks::mock_preference( 'decreaseLoanHighHoldsValue', 2 ); $data = C4::Circulation::checkHighHolds( $item_hr, $patron_hr ); -is( $data->{exceeded}, 0, "Should not exceed threshold of 2" ); +is( $data->{exceeded}, 0, "Dynamic mode should not exceed threshold when decreaseLoanHighHoldsValue = 2" ); +is( $data->{outstanding}, 12, "Should have 12 outstanding holds" ); +is( $data->{duration}, 0, "Should have duration of 0" ); +is( ref( $data->{due_date} ), '', "due_date should not be a DateTime object" ); + +# We have 10 holdable items + decreaseLoanHighHoldsValue=3 = threshold of 13, versus 12 holds, so this should trigger decreased loan time +t::lib::Mocks::mock_preference( 'decreaseLoanHighHoldsValue', 3 ); +$data = C4::Circulation::checkHighHolds( $item_hr, $patron_hr ); +is( $data->{exceeded}, 0, "Dynamic mode should not exceed threshold when decreaseLoanHighHoldsValue = 3" ); +is( $data->{outstanding}, 12, "Should have 12 outstanding holds" ); +is( $data->{duration}, 0, "Should have duration of 0" ); +is( ref( $data->{due_date} ), '', "due_date should not be a DateTime object" ); +t::lib::Mocks::mock_preference( 'decreaseLoanHighHoldsValue', 2 ); my $unholdable = pop(@items); $unholdable->damaged(-1); $unholdable->store(); -- 2.11.0