From 86d9ff72d5ecab9ef9be9a9febf6489b00da7ad5 Mon Sep 17 00:00:00 2001 From: Marcel de Rooy Date: Thu, 15 Aug 2013 13:03:43 +0200 Subject: [PATCH] Bug 10663: Correcting GetReserveStatus so that holds block renewal Content-Type: text/plain; charset=utf-8 This patch is a counterproposal for the patch already present on 10663. Instead of replacing the call to GetReserveStatus, it repairs GetReserveStatus. Note that the biblionumber parameter of GetReserveStatus is actually not used. It can be removed. The sql statement is adjusted to include title level holds. And that solves the problem reported. Test plan: Issue a book. Enter a title level hold. Check if you can renew on circulation page. Remove the title level hold. Enter an item level hold. Check again. Signed-off-by: Marcel de Rooy --- C4/Reserves.pm | 39 +++++++++++++++------------------------ C4/Search.pm | 2 +- 2 files changed, 16 insertions(+), 25 deletions(-) diff --git a/C4/Reserves.pm b/C4/Reserves.pm index 0291def..38dbce4 100644 --- a/C4/Reserves.pm +++ b/C4/Reserves.pm @@ -767,40 +767,31 @@ sub GetReservesForBranch { =head2 GetReserveStatus - $reservestatus = GetReserveStatus($itemnumber, $biblionumber); + $reservestatus = GetReserveStatus($itemnumber); -Take an itemnumber or a biblionumber and return the status of the reserve places on it. -If several reserves exist, the reserve with the lower priority is given. +Takes an itemnumber and returns the status of the reserve placed on it. +Note that this could be an item level hold OR a title level hold. +If several reserves exist, the reserve with the lowest priority is given. =cut -## FIXME: I don't think this does what it thinks it does. -## It only ever checks the first reserve result, even though -## multiple reserves for that bib can have the itemnumber set -## the sub is only used once in the codebase. sub GetReserveStatus { - my ($itemnumber, $biblionumber) = @_; + my ($itemnumber) = @_; my $dbh = C4::Context->dbh; my ($sth, $found, $priority); - if ( $itemnumber ) { - $sth = $dbh->prepare("SELECT found, priority FROM reserves WHERE itemnumber = ? order by priority LIMIT 1"); - $sth->execute($itemnumber); - ($found, $priority) = $sth->fetchrow_array; - } - - if ( $biblionumber and not defined $found and not defined $priority ) { - $sth = $dbh->prepare("SELECT found, priority FROM reserves WHERE biblionumber = ? order by priority LIMIT 1"); - $sth->execute($biblionumber); - ($found, $priority) = $sth->fetchrow_array; - } + $sth = $dbh->prepare("SELECT found, priority FROM reserves re LEFT JOIN items it ON re.biblionumber=it.biblionumber WHERE (re.itemnumber IS NULL OR re.itemnumber=it.itemnumber) AND it.itemnumber=? ORDER BY priority LIMIT 1"); + #Note that we catch title level holds by joining the items table + #via the biblionumber. + #The reserves table does not yet contain the item number in that case ! + $sth->execute($itemnumber); + ($found, $priority) = $sth->fetchrow_array; + $found//=''; - if(defined $found) { - return 'Waiting' if $found eq 'W' and $priority == 0; - return 'Finished' if $found eq 'F'; - return 'Reserved' if $priority > 0; - } + return 'Waiting' if $found eq 'W' and $priority == 0; + return 'Finished' if $found eq 'F'; + return 'Reserved' if $priority > 0; return ''; #empty string here will remove need for checking undef, or less log lines } diff --git a/C4/Search.pm b/C4/Search.pm index aec024b..eefaa4c 100644 --- a/C4/Search.pm +++ b/C4/Search.pm @@ -1921,7 +1921,7 @@ sub searchResults { # should map transit status to record indexed in Zebra. # ($transfertwhen, $transfertfrom, $transfertto) = C4::Circulation::GetTransfers($item->{itemnumber}); - $reservestatus = C4::Reserves::GetReserveStatus( $item->{itemnumber}, $oldbiblio->{biblionumber} ); + $reservestatus = C4::Reserves::GetReserveStatus( $item->{itemnumber}); } # item is withdrawn, lost, damaged, not for loan, reserved or in transit -- 1.7.7.6