@@ -, +, @@ --- 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 --- a/C4/Circulation.pm +++ a/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 --- a/Koha/Biblio.pm +++ a/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 --- a/installer/data/mysql/atomicupdate/bug_14694.sql +++ a/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'); --- a/installer/data/mysql/sysprefs.sql +++ a/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'), --- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref +++ a/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: --- a/t/db_dependent/DecreaseLoanHighHolds.t +++ a/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; --