Bugzilla – Attachment 105199 Details for
Bug 25566
Change in DecreaseLoanHighHolds behaviour
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 25566: Add option to ignore found holds and use it when checking high holds
Bug-25566-Add-option-to-ignore-found-holds-and-use.patch (text/plain), 10.12 KB, created by
Nick Clemens (kidclamp)
on 2020-05-21 13:47:38 UTC
(
hide
)
Description:
Bug 25566: Add option to ignore found holds and use it when checking high holds
Filename:
MIME Type:
Creator:
Nick Clemens (kidclamp)
Created:
2020-05-21 13:47:38 UTC
Size:
10.12 KB
patch
obsolete
>From 2875c455fff740ca12e2aa76c5d7551d4f7d4876 Mon Sep 17 00:00:00 2001 >From: Nick Clemens <nick@bywatersolutions.com> >Date: Thu, 21 May 2020 13:41:23 +0000 >Subject: [PATCH] Bug 25566: Add option to ignore found holds and use it when > checking high holds > >To test: > 1 - Find or create a record with 10 items > 2 - Set sysprefs: > decreaseLoanHighHolds - enable > decreaseLoanHighHoldsDuration - 2 > decreaseLoanHighHoldsValue - 2 > decreaseLoanHighHoldsControl - 'over the number of holdable items'/dynamic > 3 - Set circ rules to allow 1 hold per record on the relevant record > 4 - Place 3 holds on the record > 5 - Check one item in and confirm hold to set to waiting > 6 - Issue to the patron with the waiting hold > 7 - Get a notice that loan period is decreased > 8 - Don't confirm the checkout > 9 - Apply patch >10 - Restart all the things >11 - Repeat checkout, no decrease this time! >--- > C4/Circulation.pm | 2 +- > C4/Reserves.pm | 29 ++++++++++++++++++----------- > t/db_dependent/DecreaseLoanHighHolds.t | 26 ++++++++++++++++++++++---- > t/db_dependent/Reserves/MultiplePerRecord.t | 7 +++++-- > 4 files changed, 46 insertions(+), 18 deletions(-) > >diff --git a/C4/Circulation.pm b/C4/Circulation.pm >index e439250933..52ff4925fb 100644 >--- a/C4/Circulation.pm >+++ b/C4/Circulation.pm >@@ -1223,7 +1223,7 @@ sub checkHighHolds { > } > > # Remove any items that are not holdable for this patron >- @items = grep { CanItemBeReserved( $borrower->{borrowernumber}, $_->itemnumber )->{status} eq 'OK' } @items; >+ @items = grep { CanItemBeReserved( $borrower->{borrowernumber}, $_->itemnumber, undef, { ignore_found_holds => 1 } )->{status} eq 'OK' } @items; > > my $items_count = scalar @items; > >diff --git a/C4/Reserves.pm b/C4/Reserves.pm >index 9f8d31637a..097399cbaf 100644 >--- a/C4/Reserves.pm >+++ b/C4/Reserves.pm >@@ -301,15 +301,17 @@ sub AddReserve { > > =head2 CanBookBeReserved > >- $canReserve = &CanBookBeReserved($borrowernumber, $biblionumber, $branchcode) >+ $canReserve = &CanBookBeReserved($borrowernumber, $biblionumber, $branchcode, $params) > if ($canReserve eq 'OK') { #We can reserve this Item! } > >+ $params are passed directly through to CanItemBeReserved >+ > See CanItemBeReserved() for possible return values. > > =cut > > sub CanBookBeReserved{ >- my ($borrowernumber, $biblionumber, $pickup_branchcode) = @_; >+ my ($borrowernumber, $biblionumber, $pickup_branchcode, $params) = @_; > > my @itemnumbers = Koha::Items->search({ biblionumber => $biblionumber})->get_column("itemnumber"); > #get items linked via host records >@@ -320,7 +322,7 @@ sub CanBookBeReserved{ > > my $canReserve = { status => '' }; > foreach my $itemnumber (@itemnumbers) { >- $canReserve = CanItemBeReserved( $borrowernumber, $itemnumber, $pickup_branchcode ); >+ $canReserve = CanItemBeReserved( $borrowernumber, $itemnumber, $pickup_branchcode, $params ); > return { status => 'OK' } if $canReserve->{status} eq 'OK'; > } > return $canReserve; >@@ -328,9 +330,13 @@ sub CanBookBeReserved{ > > =head2 CanItemBeReserved > >- $canReserve = &CanItemBeReserved($borrowernumber, $itemnumber, $branchcode) >+ $canReserve = &CanItemBeReserved($borrowernumber, $itemnumber, $branchcode, $params) > if ($canReserve->{status} eq 'OK') { #We can reserve this Item! } > >+ current params are 'ignore_found_holds' - if true holds that have been trapped are not counted >+ toward the patron limit, used by checkHighHolds to avoid counting the hold we will fill with the >+ current checkout against the high holds threshold >+ > @RETURNS { status => OK }, if the Item can be reserved. > { status => ageRestricted }, if the Item is age restricted for this borrower. > { status => damaged }, if the Item is damaged. >@@ -346,7 +352,7 @@ sub CanBookBeReserved{ > =cut > > sub CanItemBeReserved { >- my ( $borrowernumber, $itemnumber, $pickup_branchcode ) = @_; >+ my ( $borrowernumber, $itemnumber, $pickup_branchcode, $params ) = @_; > > my $dbh = C4::Context->dbh; > my $ruleitemtype; # itemtype of the matching issuing rule >@@ -409,12 +415,13 @@ sub CanItemBeReserved { > $ruleitemtype = undef; > } > >- my $holds = Koha::Holds->search( >- { >- borrowernumber => $borrowernumber, >- biblionumber => $item->biblionumber, >- } >- ); >+ my $search_params = { >+ borrowernumber => $borrowernumber, >+ biblionumber => $item->biblionumber, >+ }; >+ $search_params->{found} = undef if $params->{ignore_found_holds}; >+ >+ my $holds = Koha::Holds->search($search_params); > if ( defined $holds_per_record && $holds_per_record ne '' > && $holds->count() >= $holds_per_record ) { > return { status => "tooManyHoldsForThisRecord", limit => $holds_per_record }; >diff --git a/t/db_dependent/DecreaseLoanHighHolds.t b/t/db_dependent/DecreaseLoanHighHolds.t >index f0bf9b61d0..9228f32c93 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 => 19; > > my $dbh = C4::Context->dbh; > my $schema = Koha::Database->new()->schema(); >@@ -71,6 +71,7 @@ for my $i ( 1 .. 20 ) { > > my $biblio = $builder->build_sample_biblio(); > >+# The biblio gets 10 items > my @items; > for my $i ( 1 .. 10 ) { > my $item = $builder->build_sample_item( >@@ -82,6 +83,7 @@ for my $i ( 1 .. 10 ) { > push( @items, $item ); > } > >+# Place 6 holds, patrons 0,1,2,3,4,5 > for my $i ( 0 .. 5 ) { > my $patron = $patrons[$i]; > my $hold = Koha::Hold->new( >@@ -93,8 +95,9 @@ for my $i ( 0 .. 5 ) { > )->store(); > } > >-my $item = pop(@items); >-my $patron = pop(@patrons); >+my $item = shift(@items); >+my $patron = shift(@patrons); >+my $patron_hold = Koha::Holds->find({ borrowernumber => $patron->borrowernumber, biblionumber => $item->biblionumber }); > > Koha::CirculationRules->set_rules( > { >@@ -129,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 5 outstanding holds" ); >+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" ); > >@@ -142,6 +145,8 @@ t::lib::Mocks::mock_preference( 'decreaseLoanHighHoldsControl', 'dynamic' ); > $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 ) { > my $patron = $patrons[$i]; > my $hold = Koha::Hold->new( >@@ -153,9 +158,12 @@ for my $i ( 5 .. 10 ) { > )->store(); > } > >+# 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" ); >+is( $data->{outstanding}, 12, "Should exceed threshold of 1" ); > >+# 12 holds, threshold is 2 over 10 holdable items = 12 (equal is okay) > 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" ); >@@ -164,6 +172,7 @@ my $unholdable = pop(@items); > $unholdable->damaged(-1); > $unholdable->store(); > >+# 12 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 damaged item" ); > >@@ -171,6 +180,7 @@ $unholdable->damaged(0); > $unholdable->itemlost(-1); > $unholdable->store(); > >+# 12 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 lost item" ); > >@@ -178,6 +188,7 @@ $unholdable->itemlost(0); > $unholdable->notforloan(-1); > $unholdable->store(); > >+# 12 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 notforloan item" ); > >@@ -185,8 +196,15 @@ $unholdable->notforloan(0); > $unholdable->withdrawn(-1); > $unholdable->store(); > >+# 12 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('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'); > >diff --git a/t/db_dependent/Reserves/MultiplePerRecord.t b/t/db_dependent/Reserves/MultiplePerRecord.t >index 6fb431da09..1a46a2f353 100755 >--- a/t/db_dependent/Reserves/MultiplePerRecord.t >+++ b/t/db_dependent/Reserves/MultiplePerRecord.t >@@ -19,7 +19,7 @@ > > use Modern::Perl; > >-use Test::More tests => 39; >+use Test::More tests => 40; > use t::lib::TestBuilder; > use t::lib::Mocks; > >@@ -277,7 +277,7 @@ Koha::CirculationRules->set_rules( > itemtype => undef, > branchcode => undef, > rules => { >- reservesallowed => 2, >+ reservesallowed => 3, > holds_per_record => 2, > } > } >@@ -314,4 +314,7 @@ Koha::Holds->find($hold_id)->found("W")->store; > $can = CanBookBeReserved($patron->{borrowernumber}, $biblio->{biblionumber}); > is( $can->{status}, 'tooManyHoldsForThisRecord', 'Third hold exceeds limit of holds per record' ); > >+$can = CanBookBeReserved($patron->{borrowernumber}, $biblio->{biblionumber}, undef, { ignore_found_holds => 1 }); >+is( $can->{status}, 'OK', 'Third hold is allowed when ignoring waiting holds' ); >+ > $schema->storage->txn_rollback; >-- >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 25566
:
105199
|
105223
|
106930