From 1a9d3209aefdadf1654e9d2327a20451b0a84008 Mon Sep 17 00:00:00 2001 From: Jonathan Druart Date: Fri, 28 Jan 2022 15:04:49 +0100 Subject: [PATCH] Bug 29969: Prevent crash if 'Update holds' clicked after bulk cancellation If you cancel holds in bulk, the list is not updated as we enqueued the task. But the "Update hold(s)" button will explode if clicked. Test plan: Place several holds on a bib record Use the "Cancel selected" link to cancel holds in bulk The job is enqueued and the hold list still show the holds you cancelled Click "Update holds" => Without this patch you get an ugly 500 Can't call method "found" on an undefined value at /kohadevbox/koha/C4/Reserves.pm line 1060 => With this patch applied the table is refresh, no crash (and there is a warning in the log, that may not be necessary) --- C4/Reserves.pm | 5 ++++- reserve/modrequest.pl | 12 +++++++++++- t/db_dependent/Holds.t | 11 ++++++++++- 3 files changed, 25 insertions(+), 3 deletions(-) diff --git a/C4/Reserves.pm b/C4/Reserves.pm index 0b79951cedb..0fe6aee7b96 100644 --- a/C4/Reserves.pm +++ b/C4/Reserves.pm @@ -1040,7 +1040,7 @@ sub ModReserve { my $cancellation_reason = $params->{'cancellation_reason'}; my $date = $params->{expirationdate}; - return if $rank eq "n"; + return if defined $rank && $rank eq "n"; return unless ( $reserve_id || ( $borrowernumber && ( $biblionumber || $itemnumber ) ) ); @@ -1054,6 +1054,9 @@ sub ModReserve { $hold ||= Koha::Holds->find($reserve_id); + # FIXME Other calls may fail + Koha::Exceptions::ObjectNotFound->throw( 'No hold with id ' . $reserve_id ) unless $hold; + if ( $rank eq "del" ) { $hold->cancel({ cancellation_reason => $cancellation_reason }); } diff --git a/reserve/modrequest.pl b/reserve/modrequest.pl index eb677995bf0..edeb443a0cc 100755 --- a/reserve/modrequest.pl +++ b/reserve/modrequest.pl @@ -26,6 +26,8 @@ use Modern::Perl; use CGI qw ( -utf8 ); use URI; use List::MoreUtils qw( uniq ); +use Try::Tiny; + use C4::Output; use C4::Reserves qw( ModReserve ModReserveCancelAll ); use C4::Auth qw( get_template_and_user ); @@ -84,7 +86,15 @@ else { $params->{reservedate} = $reservedates[$i] ? dt_from_string($reservedates[$i]) : undef; } - ModReserve($params); + try { + ModReserve($params); + } catch { + if ($_->isa('Koha::Exceptions::ObjectNotFound')){ + warn $_; + } else { + $_->rethrow; + } + } } } diff --git a/t/db_dependent/Holds.t b/t/db_dependent/Holds.t index 03bcc53f9f5..283ee0ae54e 100755 --- a/t/db_dependent/Holds.t +++ b/t/db_dependent/Holds.t @@ -7,7 +7,9 @@ use t::lib::TestBuilder; use C4::Context; -use Test::More tests => 72; +use Test::More tests => 73; +use Test::Exception; + use MARC::Record; use C4::Biblio; @@ -223,6 +225,13 @@ AlterPriority( 'bottom', $hold->reserve_id, undef, 2, 1, 6 ); $hold = Koha::Holds->find( $reserveid ); is( $hold->priority, '6', "Test AlterPriority(), move to bottom" ); + +$hold->delete; +throws_ok + { C4::Reserves::ModReserve({ reserve_id => $hold->reserve_id }) } + 'Koha::Exceptions::ObjectNotFound', + 'No hold with id ' . $hold->reserve_id; + # Regression test for bug 2394 # # If IndependentBranches is ON and canreservefromotherbranches is OFF, -- 2.25.1