From 6bb94322ac36d050bd92413db703d999630872e3 Mon Sep 17 00:00:00 2001 From: Kyle M Hall Date: Wed, 29 Jan 2014 10:52:08 -0500 Subject: [PATCH] Bug 11634 - Allow renewal of item with unfilled holds if other available items can fill those holds MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The current holds behavior in Koha allows a situation like this: - Patron A has an item currently checked out. - Patron B places a hold on the next available copy of that title. - Then Patron A will not be able to renew his item, even if there are other available copies of that title that could potentially fill Patron B's hold. Since this seems unfair to Patron A, we should allow renewal of items even if there are unfilled holds, but those holds could all be filled with currently available items. Test Plan: 1) Apply this patch 2) Create a record with two items 3) Check out the item to a patron 4) Place a hold on the record 5) Note you cannot renew the item for the patron 6) Enable the new system preference AllowRenewalIfOtherItemsAvailable 7) Note you can now renew the item, as all the holds can be satisfied by available items. 8) Place a second hold on the record 9) Note you can no longer renew the item, as all the holds *cannot* be filled by currently available items Signed-off-by: Holger Meißner --- C4/Circulation.pm | 54 ++++++++++++++++++++ C4/Reserves.pm | 19 ++++--- installer/data/mysql/sysprefs.sql | 1 + installer/data/mysql/updatedatabase.pl | 10 ++++ .../en/modules/admin/preferences/circulation.pref | 6 +++ 5 files changed, 82 insertions(+), 8 deletions(-) diff --git a/C4/Circulation.pm b/C4/Circulation.pm index f8b204a..ba5b18b 100644 --- a/C4/Circulation.pm +++ b/C4/Circulation.pm @@ -48,7 +48,9 @@ use Data::Dumper; use Koha::DateUtils; use Koha::Calendar; use Koha::Borrower::Debarments; +use Koha::Database; use Carp; +use List::MoreUtils qw( uniq ); use Date::Calc qw( Today Today_and_Now @@ -2494,6 +2496,58 @@ sub CanBookBeRenewed { my ( $resfound, $resrec, undef ) = C4::Reserves::CheckReserves( $itemnumber ); + # This item can fill one or more unfilled reserve, can those unfilled reserves + # all be filled by other available items? + if ( $resfound + && C4::Context->preference('AllowRenewalIfOtherItemsAvailable') ) + { + my $schema = Koha::Database->new()->schema(); + + # Get all other items that could possibly fill reserves + my @itemnumbers = $schema->resultset('Item')->search( + { + biblionumber => $resrec->{biblionumber}, + onloan => undef, + -not => { itemnumber => $itemnumber } + }, + { columns => 'itemnumber' } + )->get_column('itemnumber')->all(); + + # Get all other reserves that could have been filled by this item + my @borrowernumbers; + while (1) { + my ( $reserve_found, $reserve, undef ) = + C4::Reserves::CheckReserves( $itemnumber, undef, undef, + \@borrowernumbers ); + + if ($reserve_found) { + push( @borrowernumbers, $reserve->{borrowernumber} ); + } + else { + last; + } + } + + # If the count of the union of the lists of reservable items for each borrower + # is equal or greater than the number of borrowers, we know that all reserves + # can be filled with available items. We can get the union of the sets simply + # by pushing all the elements onto an array and removing the duplicates. + my @reservable; + foreach my $b (@borrowernumbers) { + foreach my $i (@itemnumbers) { + if ( CanItemBeReserved( $b, $i ) ) { + push( @reservable, $i ); + } + } + } + + @reservable = uniq(@reservable); + + if ( @reservable >= @borrowernumbers ) { + $resfound = 0; + } + } + if ( $resfound ) { # '' when no hold was found $renewokay = 0; $error = "on_reserve"; diff --git a/C4/Reserves.pm b/C4/Reserves.pm index 8fe0fd0..8bb212f 100644 --- a/C4/Reserves.pm +++ b/C4/Reserves.pm @@ -39,7 +39,7 @@ use C4::Dates qw( format_date_in_iso ); use Koha::DateUtils; -use List::MoreUtils qw( firstidx ); +use List::MoreUtils qw( firstidx any ); use vars qw($VERSION @ISA @EXPORT @EXPORT_OK %EXPORT_TAGS); @@ -859,7 +859,7 @@ table in the Koha database. =cut sub CheckReserves { - my ( $item, $barcode, $lookahead_days) = @_; + my ( $item, $barcode, $lookahead_days, $ignore_borrowers) = @_; my $dbh = C4::Context->dbh; my $sth; my $select; @@ -906,7 +906,7 @@ sub CheckReserves { return ( '' ) if ( $notforloan_per_item > 0 ) or $notforloan_per_itemtype; # Find this item in the reserves - my @reserves = _Findgroupreserve( $bibitem, $biblio, $itemnumber, $lookahead_days); + my @reserves = _Findgroupreserve( $bibitem, $biblio, $itemnumber, $lookahead_days, $ignore_borrowers); # $priority and $highest are used to find the most important item # in the list returned by &_Findgroupreserve. (The lower $priority, @@ -1740,7 +1740,7 @@ sub _FixPriority { =head2 _Findgroupreserve - @results = &_Findgroupreserve($biblioitemnumber, $biblionumber, $itemnumber, $lookahead); + @results = &_Findgroupreserve($biblioitemnumber, $biblionumber, $itemnumber, $lookahead, $ignore_borrowers); Looks for an item-specific match first, then for a title-level match, returning the first match found. If neither, then we look for a 3rd kind of match based on @@ -1757,7 +1757,7 @@ C. =cut sub _Findgroupreserve { - my ( $bibitem, $biblio, $itemnumber, $lookahead) = @_; + my ( $bibitem, $biblio, $itemnumber, $lookahead, $ignore_borrowers) = @_; my $dbh = C4::Context->dbh; # TODO: consolidate at least the SELECT portion of the first 2 queries to a common $select var. @@ -1789,7 +1789,8 @@ sub _Findgroupreserve { $sth->execute($itemnumber, $lookahead||0); my @results; if ( my $data = $sth->fetchrow_hashref ) { - push( @results, $data ); + push( @results, $data ) + unless any{ $data->{borrowernumber} eq $_ } @$ignore_borrowers ; } return @results if @results; @@ -1820,7 +1821,8 @@ sub _Findgroupreserve { $sth->execute($itemnumber, $lookahead||0); @results = (); if ( my $data = $sth->fetchrow_hashref ) { - push( @results, $data ); + push( @results, $data ) + unless any{ $data->{borrowernumber} eq $_ } @$ignore_borrowers ; } return @results if @results; @@ -1852,7 +1854,8 @@ sub _Findgroupreserve { $sth->execute( $biblio, $bibitem, $itemnumber, $lookahead||0); @results = (); while ( my $data = $sth->fetchrow_hashref ) { - push( @results, $data ); + push( @results, $data ) + unless any{ $data->{borrowernumber} eq $_ } @$ignore_borrowers ; } return @results; } diff --git a/installer/data/mysql/sysprefs.sql b/installer/data/mysql/sysprefs.sql index 1668d37..ac5c51f 100644 --- a/installer/data/mysql/sysprefs.sql +++ b/installer/data/mysql/sysprefs.sql @@ -23,6 +23,7 @@ INSERT INTO systempreferences ( `variable`, `value`, `options`, `explanation`, ` ('AllowOnShelfHolds','0','','Allow hold requests to be placed on items that are not on loan','YesNo'), ('AllowPKIAuth','None','None|Common Name|emailAddress','Use the field from a client-side SSL certificate to look a user in the Koha database','Choice'), ('AllowPurchaseSuggestionBranchChoice','0','1','Allow user to choose branch when making a purchase suggestion','YesNo'), +('AllowRenewalIfOtherItemsAvailable','0',NULL,'If enabled, allow a patron to renew an item with unfilled holds if other available items can fill that hold.','YesNo'), ('AllowRenewalLimitOverride','0',NULL,'if ON, allows renewal limits to be overridden on the circulation screen','YesNo'), ('AllowReturnToBranch','anywhere','anywhere|homebranch|holdingbranch|homeorholdingbranch','Where an item may be returned','Choice'), ('AllowSelfCheckReturns','0','','If enabled, patrons may return items through the Web-based Self Checkout','YesNo'), diff --git a/installer/data/mysql/updatedatabase.pl b/installer/data/mysql/updatedatabase.pl index f043d0b..35a06e6 100755 --- a/installer/data/mysql/updatedatabase.pl +++ b/installer/data/mysql/updatedatabase.pl @@ -7953,6 +7953,16 @@ if ( C4::Context->preference("Version") < TransformToNum($DBversion) ) { SetVersion($DBversion); } +$DBversion = "3.15.00.XXX"; +if (CheckVersion($DBversion)) { + $dbh->do(q{ + INSERT INTO systempreferences ( variable, value, options, explanation, type ) VALUES + ('AllowRenewalIfOtherItemsAvailable','0',NULL,'If enabled, allow a patron to renew an item with unfilled holds if other available items can fill that hold.','YesNo') + }); + print "Upgrade to $DBversion done (Bug 11634 - Allow renewal of item with unfilled holds if other available items can fill those holds)\n"; + SetVersion($DBversion); +} + =head1 FUNCTIONS =head2 TableExists($table) 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 33ae4c9..7dbffc9 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 @@ -353,6 +353,12 @@ Circulation: -
NOTE If you are doing hourly loans then you should have this on. Holds Policy: - + - pref: AllowRenewalIfOtherItemsAvailable + choices: + yes: Allow + no: "Don't allow" + - a patron to renew an item with unfilled holds if other available items can fill that hold. + - - pref: AllowHoldPolicyOverride choices: yes: Allow -- 1.7.10.4