Bug 9788: (follow-up) removing the alldates parameter from GetReservesFromItemnumber

Before bug 9788 the alldates parameter of GetReservesFromItemnumber was
actually not used in the codebase.
The first patch of bug 9788 did change that and passed true by default.

But a closer look revealed that we do not really need it.

The parameter is removed by this patch; the SQL statement is slightly
adjusted: if reservedate<=now or a waitingdate is filled for the
requested itemnumber, GetReservesFromItemnumber will return the reserve.

This includes so-called future waits: a future hold that has been confirmed
ahead of time with pref ConfirmFutureHolds > 0 days.

Note that future item-level holds are not really interesting to return; this
just corresponds to original behavior. Future next-available holds are not
in view at all; they do not contain an item number.

Test plan:
Actually, the test plan of the first patch is valid. But for completeness I
repeat it here:

[1] Enable future holds and set ConfirmFutureHolds to 2 days.
[2] Place a future next-available hold for 2 days ahead.
[3] Check item status on catalogue detail. Available? That is fine.
[4] Confirm the future hold by checking it in. ('future wait')
[5] Look at item status again on catalogue detail. Must be Waiting now.
[6] Switch to OPAC and login as another opac user. Goto Place a hold.
[7] Check item status with item level hold info. Is it waiting?
[8] Try to place hold in staff, check item level status again. Waiting?
[9] Make a transfer for the item. Switch branch. Check hold status on
    Transfers to receive.

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Galen Charlton <gmc@esilibrary.com>

diff --git a/C4/Reserves.pm b/C4/Reserves.pm
index afef863..ea46f2a 100644 (file)
--- a/C4/Reserves.pm
+++ b/C4/Reserves.pm
@@ -358,26 +358,24 @@ sub GetReservesFromBiblionumber {
 
 =head2 GetReservesFromItemnumber
 
- ( $reservedate, $borrowernumber, $branchcode, $reserve_id, $waitingdate ) = GetReservesFromItemnumber($itemnumber, $all_dates);
+ ( $reservedate, $borrowernumber, $branchcode, $reserve_id, $waitingdate ) = GetReservesFromItemnumber($itemnumber);
 
 Get the first reserve for a specific item number (based on priority). Returns the abovementioned values for that reserve.
 
-all_dates is an optional parameter, telling Koha to include or exclude future holds
+The routine does not look at future reserves (read: item level holds), but DOES include future waits (a confirmed future hold).
 
 =cut
 
 sub GetReservesFromItemnumber {
-    my ( $itemnumber, $all_dates ) = @_;
+    my ( $itemnumber ) = @_;
     my $dbh   = C4::Context->dbh;
     my $query = "
     SELECT reservedate,borrowernumber,branchcode,reserve_id,waitingdate
     FROM   reserves
-    WHERE  itemnumber=?
+    WHERE  itemnumber=? AND ( reservedate <= CURRENT_DATE() OR
+           waitingdate IS NOT NULL )
+    ORDER BY priority
     ";
-    unless ( $all_dates ) {
-       $query .= " AND reservedate <= CURRENT_DATE()";
-    }
-    $query .= ' ORDER BY priority';
     my $sth_res = $dbh->prepare($query);
     $sth_res->execute($itemnumber);
     my ( $reservedate, $borrowernumber,$branchcode, $reserve_id, $wait ) = $sth_res->fetchrow_array;

diff --git a/catalogue/detail.pl b/catalogue/detail.pl
index a8d15e9..25893be 100755 (executable)
--- a/catalogue/detail.pl
+++ b/catalogue/detail.pl
@@ -233,8 +233,7 @@ foreach my $item (@items) {
     }
 
     # checking for holds
-    my ($reservedate,$reservedfor,$expectedAt,undef,$wait) = GetReservesFromItemnumber($item->{itemnumber}, 1); #second parameter: all dates
-        # all dates will include a future item level hold or a future wait
+    my ($reservedate,$reservedfor,$expectedAt,undef,$wait) = GetReservesFromItemnumber($item->{itemnumber});
     my $ItemBorrowerReserveInfo = GetMemberDetails( $reservedfor, 0);
     
     if (C4::Context->preference('HidePatronName')){

diff --git a/circ/transferstoreceive.pl b/circ/transferstoreceive.pl
index 676a2c5..365f200 100755 (executable)
--- a/circ/transferstoreceive.pl
+++ b/circ/transferstoreceive.pl
@@ -99,7 +99,7 @@ foreach my $br ( keys %$branches ) {
             $getransf{'subtitle'} = GetRecordValue('subtitle', $record, GetFrameworkCode($gettitle->{'biblionumber'}));
 
             # we check if we have a reserv for this transfer
-            my @checkreserv = GetReservesFromItemnumber($num->{'itemnumber'},1 ); #alldate parameter for future holds and waits
+            my @checkreserv = GetReservesFromItemnumber($num->{'itemnumber'});
             if ( $checkreserv[0] ) {
                 my $getborrower = GetMemberDetails( $checkreserv[1] );
                 $getransf{'borrowernum'}       = $getborrower->{'borrowernumber'};

diff --git a/opac/opac-reserve.pl b/opac/opac-reserve.pl
index 87d3128..3b1ca2b 100755 (executable)
--- a/opac/opac-reserve.pl
+++ b/opac/opac-reserve.pl
@@ -431,7 +431,7 @@ foreach my $biblioNum (@biblionumbers) {
         }
 
         # checking reserve
-        my ($reservedate,$reservedfor,$expectedAt,undef,$wait) = GetReservesFromItemnumber($itemNum,1); #with alldates parameter include future item level holds and waits
+        my ($reservedate,$reservedfor,$expectedAt,undef,$wait) = GetReservesFromItemnumber($itemNum);
         my $ItemBorrowerReserveInfo = GetMemberDetails( $reservedfor, 0);
 
         # the item could be reserved for this borrower vi a host record, flag this

diff --git a/reserve/request.pl b/reserve/request.pl
index 0afc53d..8701586 100755 (executable)
--- a/reserve/request.pl
+++ b/reserve/request.pl
@@ -328,7 +328,7 @@ foreach my $biblionumber (@biblionumbers) {
             }
 
             # checking reserve
-            my ($reservedate,$reservedfor,$expectedAt,$reserve_id,$wait) = GetReservesFromItemnumber($itemnumber,1); #alldates parameter to include future holds/waits
+            my ($reservedate,$reservedfor,$expectedAt,$reserve_id,$wait) = GetReservesFromItemnumber($itemnumber);
             my $ItemBorrowerReserveInfo = GetMember( borrowernumber => $reservedfor );
 
             if ( defined $reservedate ) {