From 208aab6a8ac206915407964b4f9e7feba5f00b4d Mon Sep 17 00:00:00 2001 From: Marcel de Rooy Date: Fri, 17 Jan 2025 08:24:23 +0000 Subject: [PATCH] Bug 37650: (follow-up) Remove FIXME around CheckReserves call Content-Type: text/plain; charset=utf-8 The FIXME correctly stated that the CheckReserves call and follow-up only should be executed within the cancel hold scope. NOTE: Actually, I do not see how to trigger it in current code. Since checking in a waiting hold triggers the NotIssued error. And the cancel hold button is not available for holds in queue. Test plan: Place a hold on an item. Checkin at pickup location. Confirm. Hold waiting now? Try again but checkin at another branch. Transfer triggered? The hard part (see my note). Try to get at the form where you can cancel the hold (waiting or first in queue). Signed-off-by: Marcel de Rooy --- circ/returns.pl | 34 +++++++++++++++------------------- 1 file changed, 15 insertions(+), 19 deletions(-) diff --git a/circ/returns.pl b/circ/returns.pl index 67c7b008b8..83121cff75 100755 --- a/circ/returns.pl +++ b/circ/returns.pl @@ -157,12 +157,23 @@ if ( $query->param('reserve_id') && $op eq 'cud-affect_reserve') { # fix up item type for display my $item = Koha::Items->find( $itemnumber ); my $biblio = $item->biblio; + my $hold = Koha::Holds->find($reserve_id) or die "Hold for item $itemnumber not found\n"; #TODO Not very elegant if ( $cancel_reserve ) { - my $hold = Koha::Holds->find( $reserve_id ); - if ( $hold ) { - $hold->cancel( { charge_cancel_fee => !$forgivemanualholdsexpire, cancellation_reason => $cancel_reason} ); - } # FIXME else? + $hold->cancel( { charge_cancel_fee => !$forgivemanualholdsexpire, cancellation_reason => $cancel_reason} ); + + # check if we have other reserves for this document, if we have a result send the message of transfer + my ( undef, $nextreservinfo, undef ) = CheckReserves( $item, C4::Context->preference('ConfirmFutureHolds') ); + if ( $userenv_branch ne $nextreservinfo->{'branchcode'} ) { + my $patron = Koha::Patrons->find( $nextreservinfo->{'borrowernumber'} ); + $template->param( + itemtitle => $biblio->title, + itembiblionumber => $biblio->biblionumber, + iteminfo => $biblio->author, + patron => $patron, + diffbranch => 1, + ); + } } else { my $diffBranchSend = ($userenv_branch ne $diffBranchReturned) ? $diffBranchReturned : undef; @@ -170,7 +181,6 @@ if ( $query->param('reserve_id') && $op eq 'cud-affect_reserve') { # i.e., whether to apply waiting status ModReserveAffect( $itemnumber, $borrowernumber, $diffBranchSend, $reserve_id, $desk_id ); - my $hold = Koha::Holds->find($reserve_id); if ($diffBranchSend) { my $tobranch = $hold->pickup_library(); @@ -179,20 +189,6 @@ if ( $query->param('reserve_id') && $op eq 'cud-affect_reserve') { $transfer->transit; } } - # check if we have other reserves for this document, if we have a result send the message of transfer - # FIXME do we need to do this if we didn't take the cancel_reserve branch above? - my ( undef, $nextreservinfo, undef ) = CheckReserves( $item, C4::Context->preference('ConfirmFutureHolds') ); - - if ( $userenv_branch ne $nextreservinfo->{'branchcode'} ) { - my $patron = Koha::Patrons->find( $nextreservinfo->{'borrowernumber'} ); - $template->param( - itemtitle => $biblio->title, - itembiblionumber => $biblio->biblionumber, - iteminfo => $biblio->author, - patron => $patron, - diffbranch => 1, - ); - } } if ( $query->param('recall_id') && $op eq 'cud-affect_recall' ) { -- 2.39.5