From e75d234d12409967c5dff112e01aed878319614e Mon Sep 17 00:00:00 2001 From: Marcel de Rooy Date: Thu, 26 Sep 2013 10:33:46 +0200 Subject: [PATCH] Bug 9788: Followup removing the alldates parameter 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 --- 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 c0a334b..9033172 100644 --- a/C4/Reserves.pm +++ b/C4/Reserves.pm @@ -357,26 +357,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 3adbaaa..ed8b42d 100755 --- a/catalogue/detail.pl +++ b/catalogue/detail.pl @@ -221,8 +221,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 b684632..f29ac46 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.10.4