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 <mtompset@hotmail.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
(cherry picked from commit 515ed462cff54afd0725471c977d437fbeb54aa4)
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
(cherry picked from commit b5148f412b741d1342c564360c690f14c9be6b94)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>

diff --git a/C4/Biblio.pm b/C4/Biblio.pm
index 262cc66..c7ac3e6 100644 (file)
--- 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 0890350..44ddcf6 100755 (executable)
--- 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');
+};