From 53f36fdde0f6fbb1bad0e21b0aea58694a13571c Mon Sep 17 00:00:00 2001
From: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Date: Thu, 26 Sep 2013 10:33:46 +0200
Subject: [PATCH] Bug 9788: Followup removing the alldates parameter
Content-Type: text/plain; charset=utf-8

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>
---
 C4/Reserves.pm             |   14 ++++++--------
 catalogue/detail.pl        |    3 +--
 circ/transferstoreceive.pl |    2 +-
 opac/opac-reserve.pl       |    2 +-
 reserve/request.pl         |    2 +-
 5 files changed, 10 insertions(+), 13 deletions(-)

diff --git a/C4/Reserves.pm b/C4/Reserves.pm
index da541aa..3e589b6 100644
--- 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
--- 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
--- 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
--- 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
--- 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 ) {
-- 
1.7.7.6