[Koha-bugs] [Bug 34972] Canceling a waiting hold from the holds over tab can make the next hold unfillable

[bugzilla-daemon at bugs.koha-community.org]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34972

--- Comment #14 from Marcel de Rooy <m.de.rooy at rijksmuseum.nl> ---
Hi Emily,
Brave effort to try improve here :) This code is a mine field.

Some small observations:
my $patron = Koha::Patrons->find( $nextreservinfo );
Does not look good. We should just pass the borrowernumber to find. Cleaner
code.

L615 my ( $messages, $nextreservinfo ) =
GetOtherReserves($reserve->{itemnumber});
Confusing that we start here with another $messages while the former one came
from AddReturn.

General
I am just wondering how much sense it makes to still have GetOtherReserves.
Couldnt we just obsolete it here now? Note that the name is quite misleading.
IIUC it checks if the next reserve is waiting or transit.
Around the first call I am wondering what happens now if you came from the
$cancel_reserve branch of the if statement. After that you go to
GetOtherReserves. Which changed behavior and does nothing now. Did it formerly
handle the next reserve after the cancelled one?
If I am coming from the else branch, I know if the hold is waiting or transit
(look at diffBranchSend). So why still call GetOtherReserves to find out what
you already know?

Around L600 the second call. There is just one route to the call now. You
already know again diffBranchSend. Why call GetOtherReserves?


Appreciate some feedback. Thanks.

-- 
You are receiving this mail because:
You are watching all bug changes.