From fd152f1aa902d4f6677cae42148de404fee35224 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 ++ .../en/modules/admin/preferences/circulation.pref | 13 ++- t/db_dependent/DecreaseLoanHighHolds.t | 151 ++++++++++++++++++++ 4 files changed, 247 insertions(+), 26 deletions(-) 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/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