Attachment 31706 Details for Bug 11634 – Bug 11634 - Allow renewal of item with unfilled holds if other available items can fill those holds

[patch] Bug 11634 - Allow renewal of item with unfilled holds if other available items can fill those holds

Bug-11634---Allow-renewal-of-item-with-unfilled-ho.patch (text/plain), 9.98 KB, created by Kyle M Hall (khall) on 2014-09-18 12:23:33 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Kyle M Hall (khall)

Created: 2014-09-18 12:23:33 UTC

Size: 9.98 KB

patch

obsolete

>From dae159883ec4ef2cd464d9c44501198470b36010 Mon Sep 17 00:00:00 2001
>From: Kyle M Hall <kyle@bywatersolutions.com>
>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 <h.meissner.82@web.de>
>Signed-off-by: Chris Rohde <crohde@roseville.ca.us>
>---
> 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 8c0b208..bd0697d 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
>@@ -2632,6 +2634,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 8ecd5ca..a6704ba 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);
> 
>@@ -879,7 +879,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;
>@@ -930,7 +930,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,
>@@ -1785,7 +1785,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
>@@ -1802,7 +1802,7 @@ C<biblioitemnumber>.
> =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.
>@@ -1834,7 +1834,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;
>     
>@@ -1866,7 +1867,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;
> 
>@@ -1899,7 +1901,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 b06b163..55a9cdf 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 aae80f1..59ed411 100755
>--- a/installer/data/mysql/updatedatabase.pl
>+++ b/installer/data/mysql/updatedatabase.pl
>@@ -8776,6 +8776,16 @@ if ( CheckVersion($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 2190004..f1dec0e 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
>@@ -385,6 +385,12 @@ Circulation:
>             - 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:
>                   yes: Allow
>-- 
>1.7.2.5

Actions: View | Diff | Splinter Review

Attachments on bug 11634: 24866 | 25032 | 25148 | 25822 | 25825 | 25826 | 25827 | 25837 | 28212 | 28213 | 28214 | 31706 | 31707 | 31708 | 31709 | 32034 | 32035 | 32036 | 32037 | 32875 | 32876 | 32877 | 32878 | 33492