Attachment 61184 Details for Bug 18284 – Bug 18284: (bug 17196 follow-up) Move biblio metadata when a biblio is deleted

[patch] Bug 18284: (bug 17196 follow-up) Move biblio metadata when a biblio is deleted

Bug-18284-bug-17196-follow-up-Move-biblio-metadata.patch (text/plain), 4.86 KB, created by Jonathan Druart on 2017-03-16 18:27:12 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Jonathan Druart

Created: 2017-03-16 18:27:12 UTC

Size: 4.86 KB

patch

obsolete

>From e0ac574f006a71138f220c3b3aefa365bd2a2585 Mon Sep 17 00:00:00 2001
>From: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
>Date: Thu, 16 Mar 2017 15:20:10 -0300
>Subject: [PATCH] Bug 18284: (bug 17196 follow-up) Move biblio metadata when a
> biblio is deleted
>
>There is a deletedbiblio_metadata table but it is not populated when a
>biblio is deleted. Since we have a ON DELETE constraint on
>biblio_metadata.biblionumber, the row is deleted when the biblio entry
>is deleted => data lost!
>
>Test plan:
>- Create a biblio
>- Delete it
>
>=> Without this patch the deletedbiblio_metadata table is not populated
>with the biblio_metadata row related to the biblio
>
>=> With this patch applied you should see that the row has been moved.
>---
> C4/Biblio.pm            | 30 ++++++++++++++++++++++++++++++
> t/db_dependent/Biblio.t | 30 +++++++++++++++++++++++-------
> 2 files changed, 53 insertions(+), 7 deletions(-)
>
>diff --git a/C4/Biblio.pm b/C4/Biblio.pm
>index ef2e432..07f2b16 100644
>--- a/C4/Biblio.pm
>+++ b/C4/Biblio.pm
>@@ -431,6 +431,7 @@ sub DelBiblio {
>         return $error if $error;
>     }
> 
>+
>     # delete biblio from Koha tables and save in deletedbiblio
>     # must do this *after* _koha_delete_biblioitems, otherwise
>     # delete cascade will prevent deletedbiblioitems rows
>@@ -3360,6 +3361,8 @@ sub _koha_delete_biblio {
>     my $sth = $dbh->prepare("SELECT * FROM biblio WHERE biblionumber=?");
>     $sth->execute($biblionumber);
> 
>+    # FIXME There is a transaction in _koha_delete_biblio_metadata
>+    # But actually all the following should be done inside a single transaction
>     if ( my $data = $sth->fetchrow_hashref ) {
> 
>         # save the record in deletedbiblio
>@@ -3377,6 +3380,8 @@ sub _koha_delete_biblio {
>         $bkup_sth->execute(@bind);
>         $bkup_sth->finish;
> 
>+        _koha_delete_biblio_metadata( $biblionumber );
>+
>         # delete the biblio
>         my $sth2 = $dbh->prepare("DELETE FROM biblio WHERE biblionumber=?");
>         $sth2->execute($biblionumber);
>@@ -3438,6 +3443,31 @@ sub _koha_delete_biblioitems {
>     return;
> }
> 
>+=head2 _koha_delete_biblio_metadata
>+
>+  $error = _koha_delete_biblio_metadata($biblionumber);
>+
>+C<$biblionumber> - the biblionumber of the biblio metadata to be deleted
>+
>+=cut
>+
>+sub _koha_delete_biblio_metadata {
>+    my ($biblionumber) = @_;
>+
>+    my $dbh    = C4::Context->dbh;
>+    my $schema = Koha::Database->new->schema;
>+    $schema->txn_do(
>+        sub {
>+            $dbh->do( q|
>+                INSERT INTO deletedbiblio_metadata (biblionumber, format, marcflavour, metadata)
>+                SELECT biblionumber, format, marcflavour, metadata FROM biblio_metadata WHERE biblionumber=?
>+            |,  undef, $biblionumber );
>+            $dbh->do( q|DELETE FROM biblio_metadata WHERE biblionumber=?|,
>+                undef, $biblionumber );
>+        }
>+    );
>+}
>+
> =head1 UNEXPORTED FUNCTIONS
> 
> =head2 ModBiblioMarc
>diff --git a/t/db_dependent/Biblio.t b/t/db_dependent/Biblio.t
>index 48ed72f..df5f93c 100755
>--- a/t/db_dependent/Biblio.t
>+++ b/t/db_dependent/Biblio.t
>@@ -17,21 +17,22 @@
> 
> use Modern::Perl;
> 
>-use Test::More tests => 6;
>+use Test::More tests => 7;
> use Test::MockModule;
> 
> use List::MoreUtils qw( uniq );
> use MARC::Record;
> use t::lib::Mocks qw( mock_preference );
> 
>+use Koha::Database;
>+
> BEGIN {
>     use_ok('C4::Biblio');
> }
> 
>+my $schema = Koha::Database->new->schema;
>+$schema->storage->txn_begin;
> my $dbh = C4::Context->dbh;
>-# Start transaction
>-$dbh->{AutoCommit} = 0;
>-$dbh->{RaiseError} = 1;
> 
> subtest 'GetMarcSubfieldStructureFromKohaField' => sub {
>     plan tests => 23;
>@@ -328,19 +329,22 @@ sub create_issn_field {
> subtest 'MARC21' => sub {
>     plan tests => 31;
>     run_tests('MARC21');
>-    $dbh->rollback;
>+    $schema->storage->txn_rollback;
>+    $schema->storage->txn_begin;
> };
> 
> subtest 'UNIMARC' => sub {
>     plan tests => 31;
>     run_tests('UNIMARC');
>-    $dbh->rollback;
>+    $schema->storage->txn_rollback;
>+    $schema->storage->txn_begin;
> };
> 
> subtest 'NORMARC' => sub {
>     plan tests => 31;
>     run_tests('NORMARC');
>-    $dbh->rollback;
>+    $schema->storage->txn_rollback;
>+    $schema->storage->txn_begin;
> };
> 
> subtest 'IsMarcStructureInternal' => sub {
>@@ -362,4 +366,16 @@ subtest 'IsMarcStructureInternal' => sub {
>     is( grep( /^a$/, @internals ), 0, 'no subfield a' );
> };
> 
>+subtest 'deletedbiblio_metadata' => sub {
>+    plan tests => 2;
>+
>+    my ($biblionumber, $biblioitemnumber) = AddBiblio(MARC::Record->new, '');
>+    my $biblio_metadata = C4::Biblio::GetXmlBiblio( $biblionumber );
>+    C4::Biblio::DelBiblio( $biblionumber );
>+    my ( $moved ) = $dbh->selectrow_array(q|SELECT biblionumber FROM deletedbiblio WHERE biblionumber=?|, undef, $biblionumber);
>+    is( $moved, $biblionumber );
>+    ( $moved ) = $dbh->selectrow_array(q|SELECT biblionumber FROM deletedbiblio_metadata WHERE biblionumber=?|, undef, $biblionumber);
>+    is( $moved, $biblionumber );
>+};
>+
> 1;
>-- 
>2.9.3

Actions: View | Diff | Splinter Review

Attachments on bug 18284: 61184 | 61193 | 61208 | 61209