From a4ccd2b775eaad4ff5f0cd850ef9dbf8c517b9a4 Mon Sep 17 00:00:00 2001 From: Kyle M Hall Date: Tue, 29 Sep 2015 13:24:29 -0400 Subject: [PATCH] Bug 14694 - Make decreaseloanHighHolds more flexible This patch allows for more flexibility for determining when the number of holds a record has should trigger the reduction of the loan length for items on that record. This patch adds a new system preference decreaseLoanHighHoldsControl, which defaults to 'static', the original behavior of the feature. It also has a new behavior 'dynamic' which makes the feature only decrease the loan length if the number of holds on the record exceeds the number of holdable items + decreaseLoanHighHoldsValue. It also allows items to be filtered from the list of items based on the damaged, lost, not for loan, and withdrawn values even if those values would have allowed holds ( i.e. values < 0 ) Test Plan: 1) Apply this patch 2) Run updatedatabase.pl 3) Set decreaseLoanHighHolds to Enable 4) Set decreaseLoanHighHoldsControl to "over the number of items on the record" 5) Set decreaseLoanHighHoldsDuration to 1 6) Set decreaseLoanHighHoldsValue to 3 7) Create a record with 5 items 8) Please 8 or more holds on the record 9) Check out one of the items to a patron 10) Note the loan length is reduced to 1 day 11) Set decreaseLoanHighHoldsValue to 3 to 2 12) Check out one of the items to a patron 13) Note the loan length is *not* reduced 14) Enbale all the filters possible in decreaseLoanHighHoldsIgnoreStatuses 15) Set one item to be damaged 16) Note the loan length is reduced 17) Unset the damaged status 18) Repeat steps 15 - 17 for lost, not for loan, and withdrawn --- C4/Circulation.pm | 94 +++++++++---- Koha/Biblio.pm | 15 ++ installer/data/mysql/atomicupdate/bug_14694.sql | 3 + installer/data/mysql/sysprefs.sql | 2 + .../en/modules/admin/preferences/circulation.pref | 13 ++- t/db_dependent/DecreaseLoanHighHolds.t | 151 ++++++++++++++++++++ 6 files changed, 252 insertions(+), 26 deletions(-) create mode 100644 installer/data/mysql/atomicupdate/bug_14694.sql create mode 100755 t/db_dependent/DecreaseLoanHighHolds.t diff --git a/C4/Circulation.pm b/C4/Circulation.pm index e5f95f3..eadaba6 100644 --- a/C4/Circulation.pm +++ b/C4/Circulation.pm @@ -50,6 +50,7 @@ use Koha::DateUtils; use Koha::Calendar; use Koha::Borrower::Debarments; use Koha::Database; +use Koha::Holds; use Carp; use List::MoreUtils qw( uniq ); use Date::Calc qw( @@ -1009,16 +1010,14 @@ sub CanBookBeIssued { } ## check for high holds decreasing loan period - my $decrease_loan = C4::Context->preference('decreaseLoanHighHolds'); - if ( $decrease_loan && $decrease_loan == 1 ) { - my ( $reserved, $num, $duration, $returndate ) = - checkHighHolds( $item, $borrower ); + if ( C4::Context->preference('decreaseLoanHighHolds') ) { + my $check = checkHighHolds( $item, $borrower ); - if ( $num >= C4::Context->preference('decreaseLoanHighHoldsValue') ) { + if ( $check->{exceeded} ) { $needsconfirmation{HIGHHOLDS} = { - num_holds => $num, - duration => $duration, - returndate => output_pref($returndate), + num_holds => $check->{outstanding}, + duration => $check->{duration}, + returndate => output_pref( $check->{due_date} ), }; } } @@ -1113,13 +1112,58 @@ sub checkHighHolds { my ( $item, $borrower ) = @_; my $biblio = GetBiblioFromItemNumber( $item->{itemnumber} ); my $branch = _GetCircControlBranch( $item, $borrower ); - my $dbh = C4::Context->dbh; - my $sth = $dbh->prepare( -'select count(borrowernumber) as num_holds from reserves where biblionumber=?' - ); - $sth->execute( $item->{'biblionumber'} ); - my ($holds) = $sth->fetchrow_array; - if ($holds) { + + my $return_data = { + exceeded => 0, + outstanding => 0, + duration => 0, + due_date => undef, + }; + + my $holds = Koha::Holds->search( { biblionumber => $item->{'biblionumber'} } ); + + if ( $holds->count() ) { + $return_data->{outstanding} = $holds->count(); + + my $decreaseLoanHighHoldsControl = C4::Context->preference('decreaseLoanHighHoldsControl'); + my $decreaseLoanHighHoldsValue = C4::Context->preference('decreaseLoanHighHoldsValue'); + my $decreaseLoanHighHoldsIgnoreStatuses = C4::Context->preference('decreaseLoanHighHoldsIgnoreStatuses'); + + my @decreaseLoanHighHoldsIgnoreStatuses = split( /,/, $decreaseLoanHighHoldsIgnoreStatuses ); + + if ( $decreaseLoanHighHoldsControl eq 'static' ) { + + # static means just more than a given number of holds on the record + + # If the number of holds is less than the threshold, we can stop here + if ( $holds->count() < $decreaseLoanHighHoldsValue ) { + return $return_data; + } + } + elsif ( $decreaseLoanHighHoldsControl eq 'dynamic' ) { + + # dynamic means X more than the number of holdable items on the record + + # let's get the items + my @items = $holds->next()->biblio()->items(); + + # Remove any items with status defined to be ignored even if the would not make item unholdable + foreach my $status (@decreaseLoanHighHoldsIgnoreStatuses) { + @items = grep { !$_->$status } @items; + } + + # Remove any items that are not holdable for this patron + @items = grep { CanItemBeReserved( $borrower->{borrowernumber}, $_->itemnumber ) ne 'OK' } @items; + + my $items_count = scalar @items; + + # 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() < $items_count + $decreaseLoanHighHoldsValue ) { + return $return_data; + } + } + my $issuedate = DateTime->now( time_zone => C4::Context->tz() ); my $calendar = Koha::Calendar->new( branchcode => $branch ); @@ -1128,21 +1172,21 @@ sub checkHighHolds { ( C4::Context->preference('item-level_itypes') ) ? $biblio->{'itype'} : $biblio->{'itemtype'}; - my $orig_due = - C4::Circulation::CalcDateDue( $issuedate, $itype, $branch, - $borrower ); - my $reduced_datedue = - $calendar->addDate( $issuedate, - C4::Context->preference('decreaseLoanHighHoldsDuration') ); + my $orig_due = C4::Circulation::CalcDateDue( $issuedate, $itype, $branch, $borrower ); + + my $decreaseLoanHighHoldsDuration = C4::Context->preference('decreaseLoanHighHoldsDuration'); + + my $reduced_datedue = $calendar->addDate( $issuedate, $decreaseLoanHighHoldsDuration ); if ( DateTime->compare( $reduced_datedue, $orig_due ) == -1 ) { - return ( 1, $holds, - C4::Context->preference('decreaseLoanHighHoldsDuration'), - $reduced_datedue ); + $return_data->{exceeded} = 1; + $return_data->{duration} = $decreaseLoanHighHoldsDuration; + $return_data->{due_date} = $reduced_datedue; } } - return ( 0, 0, 0, undef ); + + return $return_data; } =head2 AddIssue diff --git a/Koha/Biblio.pm b/Koha/Biblio.pm index 9c467d7..3e21eb9 100644 --- a/Koha/Biblio.pm +++ b/Koha/Biblio.pm @@ -35,6 +35,21 @@ Koha::Biblio - Koha Biblio Object class =cut +=head3 items + +Returns the related Koha::Items object for this biblio in scalar context, +or list of Koha::Item objects in list context. + +=cut + +sub items { + my ($self) = @_; + + $self->{_items} ||= Koha::Items->search( { biblionumber => $self->biblionumber() } ); + + return wantarray ? $self->{_items}->as_list : $self->{_items}; +} + =head3 type =cut diff --git a/installer/data/mysql/atomicupdate/bug_14694.sql b/installer/data/mysql/atomicupdate/bug_14694.sql new file mode 100644 index 0000000..e468b53 --- /dev/null +++ b/installer/data/mysql/atomicupdate/bug_14694.sql @@ -0,0 +1,3 @@ +INSERT INTO systempreferences ( `variable`, `value`, `options`, `explanation`, `type` ) VALUES +('decreaseLoanHighHoldsControl', 'static', 'static|dynamic', "Chooses between static and dynamic high holds checking", 'Choice'), +('decreaseLoanHighHoldsIgnoreStatuses', '', 'damaged|itemlost|notforloan|withdrawn', "Ignore items with these statuses for dynamic high holds checking", 'Choice'); diff --git a/installer/data/mysql/sysprefs.sql b/installer/data/mysql/sysprefs.sql index 88a1f5f..80fc53f 100644 --- a/installer/data/mysql/sysprefs.sql +++ b/installer/data/mysql/sysprefs.sql @@ -98,8 +98,10 @@ INSERT INTO systempreferences ( `variable`, `value`, `options`, `explanation`, ` ('dateformat','us','metric|us|iso','Define global date format (us mm/dd/yyyy, metric dd/mm/yyy, ISO yyyy-mm-dd)','Choice'), ('DebugLevel','2','0|1|2','Define the level of debugging information sent to the browser when errors are encountered (set to 0 in production). 0=none, 1=some, 2=most','Choice'), ('decreaseLoanHighHolds',NULL,'','Decreases the loan period for items with number of holds above the threshold specified in decreaseLoanHighHoldsValue','YesNo'), +('decreaseLoanHighHoldsControl', 'static', 'static|dynamic', "Chooses between static and dynamic high holds checking", 'Choice'), ('decreaseLoanHighHoldsDuration',NULL,'','Specifies a number of days that a loan is reduced to when used in conjunction with decreaseLoanHighHolds','Integer'), ('decreaseLoanHighHoldsValue',NULL,'','Specifies a threshold for the minimum number of holds needed to trigger a reduction in loan duration (used with decreaseLoanHighHolds)','Integer'), +('decreaseLoanHighHoldsIgnoreStatuses', '', 'damaged|itemlost|notforloan|withdrawn', "Ignore items with these statuses for dynamic high holds checking", 'Choice'), ('DefaultClassificationSource','ddc',NULL,'Default classification scheme used by the collection. E.g., Dewey, LCC, etc.','ClassSources'), ('DefaultLanguageField008','','','Fill in the default language for field 008 Range 35-37 of MARC21 records (e.g. eng, nor, ger, see MARC Code List for Languages)','Free'), ('DefaultLongOverdueChargeValue', NULL, NULL, "Charge a lost item to the borrower's account when the LOST value of the item changes to n.", 'integer'), diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref index df72493..8332e14 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref @@ -556,7 +556,18 @@ Circulation: - days for items with more than - pref: decreaseLoanHighHoldsValue class: integer - - holds. + - holds + - pref: decreaseLoanHighHoldsControl + choices: + static: "on the record" + dynamic: "over the number of holdable items on the record" + - . Ignore items with the following statuses when counting items + - pref: decreaseLoanHighHoldsIgnoreStatuses + multiple: + damaged: Damaged + itemlost: Lost + withdrawn: Withdrawn + notforloan: Not for loan - - pref: AllowHoldsOnPatronsPossessions choices: diff --git a/t/db_dependent/DecreaseLoanHighHolds.t b/t/db_dependent/DecreaseLoanHighHolds.t new file mode 100755 index 0000000..5b1e592 --- /dev/null +++ b/t/db_dependent/DecreaseLoanHighHolds.t @@ -0,0 +1,151 @@ +#!/usr/bin/perl + +# This file is part of Koha. +# +# Koha is free software; you can redistribute it and/or modify it +# under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 3 of the License, or +# (at your option) any later version. +# +# Koha is distributed in the hope that it will be useful, but +# WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with Koha; if not, see . + +use Modern::Perl; + +use DateTime; + +use C4::Circulation; +use Koha::Database; +use Koha::Borrower; +use Koha::Biblio; +use Koha::Item; +use Koha::Holds; +use Koha::Hold; + +use Test::More tests => 12; + +my $dbh = C4::Context->dbh; +my $schema = Koha::Database->new()->schema(); + +# Start transaction +$dbh->{RaiseError} = 1; +$schema->storage->txn_begin(); + +$dbh->do('DELETE FROM issues'); +$dbh->do('DELETE FROM issuingrules'); +$dbh->do('DELETE FROM borrowers'); +$dbh->do('DELETE FROM items'); + +# Set userenv +C4::Context->_new_userenv('xxx'); +C4::Context->set_userenv( 0, 0, 0, 'firstname', 'surname', 'MPL', 'Midway Public Library', '', '', '' ); +is( C4::Context->userenv->{branch}, 'MPL', 'userenv set' ); + +my @patrons; +for my $i ( 1 .. 20 ) { + my $patron = Koha::Borrower->new( + { cardnumber => $i, firstname => 'Kyle', surname => 'Hall', categorycode => 'S', branchcode => 'MPL' } ) + ->store(); + push( @patrons, $patron ); +} + +my $biblio = Koha::Biblio->new()->store(); +my $biblioitem = + $schema->resultset('Biblioitem')->new( { biblionumber => $biblio->biblionumber } )->insert(); + +my @items; +for my $i ( 1 .. 10 ) { + my $item = Koha::Item->new( { biblionumber => $biblio->id(), biblioitemnumber => $biblioitem->id(), } )->store(); + push( @items, $item ); +} + +for my $i ( 0 .. 4 ) { + my $patron = $patrons[$i]; + my $hold = Koha::Hold->new( + { + borrowernumber => $patron->id, + biblionumber => $biblio->id, + branchcode => 'MPL', + } + )->store(); +} + +$schema->resultset('Issuingrule') + ->new( { branchcode => '*', categorycode => '*', itemtype => '*', issuelength => '14', lengthunit => 'days' } ) + ->insert(); + +my $item = pop(@items); +my $patron = pop(@patrons); + +C4::Context->set_preference( 'decreaseLoanHighHolds', 1 ); +C4::Context->set_preference( 'decreaseLoanHighHoldsDuration', 1 ); +C4::Context->set_preference( 'decreaseLoanHighHoldsValue', 1 ); +C4::Context->set_preference( 'decreaseLoanHighHoldsControl', 'static' ); +C4::Context->set_preference( 'decreaseLoanHighHoldsIgnoreStatuses', 'damaged,itemlost,notforloan,withdrawn' ); + +my $item_hr = { itemnumber => $item->id, biblionumber => $biblio->id, homebranch => 'MPL', holdingbranch => 'MPL' }; +my $patron_hr = { borrower => $patron->id, branchcode => 'MPL' }; + +my $data = C4::Circulation::checkHighHolds( $item_hr, $patron_hr ); +is( $data->{exceeded}, 1, "Should exceed threshold" ); +is( $data->{outstanding}, 5, "Should have 5 outstanding holds" ); +is( $data->{duration}, 1, "Should have duration of 1" ); +is( ref( $data->{due_date} ), 'DateTime', "due_date should be a DateTime object" ); + +C4::Context->set_preference( 'decreaseLoanHighHoldsControl', 'dynamic' ); +$data = C4::Circulation::checkHighHolds( $item_hr, $patron_hr ); +is( $data->{exceeded}, 0, "Should not exceed threshold" ); + +for my $i ( 5 .. 10 ) { + my $patron = $patrons[$i]; + my $hold = Koha::Hold->new( + { + borrowernumber => $patron->id, + biblionumber => $biblio->id, + branchcode => 'MPL', + } + )->store(); +} + +$data = C4::Circulation::checkHighHolds( $item_hr, $patron_hr ); +is( $data->{exceeded}, 1, "Should exceed threshold" ); + +C4::Context->set_preference( 'decreaseLoanHighHoldsValue', 2 ); +$data = C4::Circulation::checkHighHolds( $item_hr, $patron_hr ); +is( $data->{exceeded}, 0, "Should not exceed threshold" ); + +my $unholdable = pop(@items); +$unholdable->damaged(-1); +$unholdable->store(); + +$data = C4::Circulation::checkHighHolds( $item_hr, $patron_hr ); +is( $data->{exceeded}, 1, "Should exceed threshold" ); + +$unholdable->damaged(0); +$unholdable->itemlost(-1); +$unholdable->store(); + +$data = C4::Circulation::checkHighHolds( $item_hr, $patron_hr ); +is( $data->{exceeded}, 1, "Should exceed threshold" ); + +$unholdable->itemlost(0); +$unholdable->notforloan(-1); +$unholdable->store(); + +$data = C4::Circulation::checkHighHolds( $item_hr, $patron_hr ); +is( $data->{exceeded}, 1, "Should exceed threshold" ); + +$unholdable->notforloan(0); +$unholdable->withdrawn(-1); +$unholdable->store(); + +$data = C4::Circulation::checkHighHolds( $item_hr, $patron_hr ); +is( $data->{exceeded}, 1, "Should exceed threshold" ); + +$schema->storage->txn_rollback(); +1; -- 1.7.2.5