From e3e89c1b768605a810cae5d0e910adace4de150c Mon Sep 17 00:00:00 2001 From: Jonathan Druart Date: Tue, 16 Oct 2018 14:52:25 -0300 Subject: [PATCH] Bug 21556: Do not crash if a biblio is deleted twice To recreate: - Go to a bibliographic detail page - Delete it - Go back - Delete it again => Without this patch you will get a 500 Can't call method "holds" on an undefined value at /home/vagrant/kohaclone/C4/Biblio.pm line 406. => With this patch applied it will silently redirect you to the search form. Note: We could/should improve the behavior and display a message, but DelBiblio will need to be moved to Koha::Biblio->delete and other callers adjusted Signed-off-by: Mark Tompsett Signed-off-by: Jonathan Druart Signed-off-by: Nick Clemens (cherry picked from commit 515ed462cff54afd0725471c977d437fbeb54aa4) Signed-off-by: Martin Renvoize (cherry picked from commit b5148f412b741d1342c564360c690f14c9be6b94) Signed-off-by: Fridolin Somers --- C4/Biblio.pm | 5 ++++- t/db_dependent/Biblio.t | 12 +++++++++++- 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/C4/Biblio.pm b/C4/Biblio.pm index 262cc66834..c7ac3e6c57 100644 --- a/C4/Biblio.pm +++ b/C4/Biblio.pm @@ -382,6 +382,10 @@ C<$error> : undef unless an error occurs sub DelBiblio { my ($biblionumber) = @_; + + my $biblio = Koha::Biblios->find( $biblionumber ); + return unless $biblio; # Should we throw an exception instead? + my $dbh = C4::Context->dbh; my $error; # for error handling @@ -404,7 +408,6 @@ sub DelBiblio { } # We delete any existing holds - my $biblio = Koha::Biblios->find( $biblionumber ); my $holds = $biblio->holds; while ( my $hold = $holds->next ) { $hold->cancel; diff --git a/t/db_dependent/Biblio.t b/t/db_dependent/Biblio.t index 0890350263..44ddcf6e82 100755 --- a/t/db_dependent/Biblio.t +++ b/t/db_dependent/Biblio.t @@ -17,7 +17,7 @@ use Modern::Perl; -use Test::More tests => 7; +use Test::More tests => 8; use Test::MockModule; use List::MoreUtils qw( uniq ); @@ -391,3 +391,13 @@ subtest 'deletedbiblio_metadata' => sub { is( $moved, $biblionumber, 'Found in deletedbiblio_metadata' ); }; +subtest 'DelBiblio' => sub { + plan tests => 2; + + my ($biblionumber, $biblioitemnumber) = C4::Biblio::AddBiblio(MARC::Record->new, ''); + my $deleted = C4::Biblio::DelBiblio( $biblionumber ); + is( $deleted, undef, 'DelBiblio returns undef is the biblio has been deleted correctly - Must be 1 instead'); # FIXME We should return 1 instead! + + $deleted = C4::Biblio::DelBiblio( $biblionumber ); + is( $deleted, undef, 'DelBiblo should return undef is the record did not exist'); +}; -- 2.39.5