From ebf4350735da7ea365f3b5b72410f6810a9182b8 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 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 Meissner Signed-off-by: Chris Rohde Signed-off-by: Katrin Fischer Signed-off-by: Tomas Cohen Arazi --- C4/Circulation.pm | 54 +++++++++++++++++++ C4/Reserves.pm | 19 ++++--- installer/data/mysql/sysprefs.sql | 1 + installer/data/mysql/updatedatabase.pl | 10 ++++ .../admin/preferences/circulation.pref | 6 +++ 5 files changed, 82 insertions(+), 8 deletions(-) diff --git a/C4/Circulation.pm b/C4/Circulation.pm index bdec2f6dfc..1c182930e9 100644 --- a/C4/Circulation.pm +++ b/C4/Circulation.pm @@ -49,7 +49,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 @@ -2641,6 +2643,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; + } + } + return ( 0, "on_reserve" ) if $resfound; # '' when no hold was found return ( 1, undef ) if $override_limit; diff --git a/C4/Reserves.pm b/C4/Reserves.pm index 5c4f981f47..abefa2daa5 100644 --- a/C4/Reserves.pm +++ b/C4/Reserves.pm @@ -41,7 +41,7 @@ use Koha::DateUtils; use Koha::Calendar; use Koha::Database; -use List::MoreUtils qw( firstidx ); +use List::MoreUtils qw( firstidx any ); use vars qw($VERSION @ISA @EXPORT @EXPORT_OK %EXPORT_TAGS); @@ -918,7 +918,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; @@ -973,7 +973,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, @@ -1857,7 +1857,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 @@ -1874,7 +1874,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. @@ -1907,7 +1907,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; @@ -1940,7 +1941,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; @@ -1974,7 +1976,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 4645c60802..6a0e0d61a2 100644 --- a/installer/data/mysql/sysprefs.sql +++ b/installer/data/mysql/sysprefs.sql @@ -28,6 +28,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 491c2b8bd6..c7290a7fab 100755 --- a/installer/data/mysql/updatedatabase.pl +++ b/installer/data/mysql/updatedatabase.pl @@ -9470,6 +9470,16 @@ if ( C4::Context->preference("Version") < TransformToNum($DBversion) ) { SetVersion($DBversion); } +$DBversion = "3.17.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 23cd3eae5b..c1166f8dc3 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 @@ -403,6 +403,12 @@ Circulation: - "it will be updated to the right-hand value. E.g. '-1: 0' will cause an item that was set to 'Ordered' to now be available for loan." - Each pair of values should be on a separate line. 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: -- 2.39.5