From 92be11bbcf20628dd7db8b83fc0b902ba578a28a Mon Sep 17 00:00:00 2001 From: Marcel de Rooy Date: Thu, 26 Sep 2013 10:33:46 +0200 Subject: [PATCH] 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 Signed-off-by: Jonathan Druart Signed-off-by: Galen Charlton --- 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 afef863fa7..ea46f2ab7c 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 a8d15e9a3c..25893be82a 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 676a2c5921..365f20075d 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 87d3128982..3b1ca2b5f7 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 0afc53d7d5..8701586ae1 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 ) { -- 2.39.5