Attachment 130017 Details for Bug 29969 – Bug 29969: Prevent crash if 'Update holds' clicked after bulk cancellation

[patch] Bug 29969: Prevent crash if 'Update holds' clicked after bulk cancellation

Bug-29969-Prevent-crash-if-Update-holds-clicked-af.patch (text/plain), 3.41 KB, created by Tomás Cohen Arazi (tcohen) on 2022-01-31 16:17:19 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Tomás Cohen Arazi (tcohen)

Created: 2022-01-31 16:17:19 UTC

Size: 3.41 KB

patch

obsolete

>From 614e51eab846699ddce8cb90c1930cb5f550384e Mon Sep 17 00:00:00 2001
>From: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
>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)
>
>Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
>---
> 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 b95f0a61b8..91de97def4 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 3a7c3deb6b..2aa665deae 100755
>--- a/reserve/modrequest.pl
>+++ b/reserve/modrequest.pl
>@@ -25,6 +25,8 @@
> use Modern::Perl;
> use CGI qw ( -utf8 );
> 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 );
>@@ -83,7 +85,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 69e9b2bc10..1ff1dbc490 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;
>@@ -221,6 +223,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.32.0

Actions: View | Diff | Splinter Review

Attachments on bug 29969: 129948 | 129990 | 130017 | 130865