From e8c4bd8ab4e8986bfe2cfae58e9b14ac666bfe3f Mon Sep 17 00:00:00 2001 From: Kyle M Hall Date: Wed, 28 Jan 2015 08:31:30 -0500 Subject: [PATCH] Bug 13636 - Staff search results item status incorrect for holds Imagine this scenario: we have one record with four items. Two of those items are checked out, one of those items is a waiting hold, and one of those items is available. We would expect to see this on the search results page. Instead, we will see both non-checked out items as unavailable due to waiting holds. This is due to a semantic issue GetReserveStatus. C4::Search::searchResults uses GetReserveStatus to get the reserve status of each item, but unlike all other calls to the sub, this one passes in not only itemnumber, but biblionumber. When no reserve is found for the available item, the subroutine uses the biblionumber to grab what is essentially an arbitrary reserve to use for the status. This makes no sense and this functionality should be entirely removed from the subroutine so regressions like this will be prevented in the future. Test Plan: 1) Create one record with 4 items a) check two of the items out to patrons b) set one of the items as a waiting hold c) leave the fourth item as available 2) Run a search where this record will be in the results list 3) Note that the results list 2 items on loan, two unavailable 4) Apply this patch, reload the search results 5) Note that the results list 1 available, 2 on loan, 1 unavailable Signed-off-by: John Andrews Signed-off-by: Sheila Kearns Signed-off-by: Katrin Fischer Note: This is for the staff search result list! Works as expected. Signed-off-by: Tomas Cohen Arazi (cherry picked from commit fcaa6f35c080c079ba422a70dec49ca327346c82) Signed-off-by: Chris Cormack --- C4/Reserves.pm | 12 +++--------- C4/Search.pm | 2 +- 2 files changed, 4 insertions(+), 10 deletions(-) diff --git a/C4/Reserves.pm b/C4/Reserves.pm index 22a9ecb438..ae7062e83f 100644 --- a/C4/Reserves.pm +++ b/C4/Reserves.pm @@ -852,9 +852,9 @@ 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. +Takes an itemnumber and returns the status of the reserve placed on it. If several reserves exist, the reserve with the lower priority is given. =cut @@ -864,7 +864,7 @@ If several reserves exist, the reserve with the lower priority is given. ## 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; @@ -875,12 +875,6 @@ sub GetReserveStatus { ($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; - } - if(defined $found) { return 'Waiting' if $found eq 'W' and $priority == 0; return 'Finished' if $found eq 'F'; diff --git a/C4/Search.pm b/C4/Search.pm index 1326f692bb..e32218e498 100644 --- a/C4/Search.pm +++ b/C4/Search.pm @@ -2151,7 +2151,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 -- 2.39.5