From 8bbd73cb795433ba8497cb187cfb4eb98ac93699 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 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. --- 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 613f6b8..40dd99a 100644 --- a/C4/Reserves.pm +++ b/C4/Reserves.pm @@ -350,26 +350,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 1c818c0..55fc8f9 100755 --- a/catalogue/detail.pl +++ b/catalogue/detail.pl @@ -220,8 +220,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 8a87e66..cf851e0 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.7.6