Bugzilla – Attachment 103250 Details for
Bug 22005
System preference DecreaseLoanHighHolds incorrectly counts the number of holds
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 22005 - DecreaseLoanHighHolds counts holds wrong
Bug-22005---DecreaseLoanHighHolds-counts-holds-wro.patch (text/plain), 9.26 KB, created by
Magnus Enger
on 2020-04-20 09:36:54 UTC
(
hide
)
Description:
Bug 22005 - DecreaseLoanHighHolds counts holds wrong
Filename:
MIME Type:
Creator:
Magnus Enger
Created:
2020-04-20 09:36:54 UTC
Size:
9.26 KB
patch
obsolete
>From a3870d9f41f13be815c2db458ab0c2386dfa0104 Mon Sep 17 00:00:00 2001 >From: Magnus Enger <magnus@libriotech.no> >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
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 22005
: 103250 |
103251