From a881167915eae5f43888039532f99520a4acd259 Mon Sep 17 00:00:00 2001 From: Olli-Antti Kivilahti Date: Tue, 14 Mar 2017 16:28:58 +0200 Subject: [PATCH] Bug 18265 - Followup to Buuug 17196. deletedbiblio_metadata is not used. biblioitemnumber was forgot. perl misc/devel/update_dbix_class_files.pl perl t/db_dependent/Biblio_crud.t perl t/db_dependent/Holds/HoldFulfillmentPolicy.t + other tests from Buug 17196 --- C4/Biblio.pm | 37 +++++++- Koha/Biblio/DeletedMetadata.pm | 32 +++++++ Koha/Biblio/DeletedMetadatas.pm | 38 +++++++++ Koha/Biblio/Metadata.pm | 2 +- .../Bug-18265-Followup_to_Buuug_17196.sql | 15 ++++ t/db_dependent/Biblio_crud.t | 99 ++++++++++++++++++++++ 6 files changed, 218 insertions(+), 5 deletions(-) create mode 100644 Koha/Biblio/DeletedMetadata.pm create mode 100644 Koha/Biblio/DeletedMetadatas.pm create mode 100644 installer/data/mysql/atomicupdate/Bug-18265-Followup_to_Buuug_17196.sql create mode 100644 t/db_dependent/Biblio_crud.t diff --git a/C4/Biblio.pm b/C4/Biblio.pm index ef2e432..b523413 100644 --- a/C4/Biblio.pm +++ b/C4/Biblio.pm @@ -257,7 +257,7 @@ sub AddBiblio { _koha_marc_update_biblioitem_cn_sort( $record, $olddata, $frameworkcode ); # now add the record - ModBiblioMarc( $record, $biblionumber, $frameworkcode ) unless $defer_marc_save; + ModBiblioMarc( $record, $biblionumber, $biblioitemnumber, $frameworkcode ) unless $defer_marc_save; # update OAI-PMH sets if(C4::Context->preference("OAI-PMH:AutoUpdateSets")) { @@ -335,7 +335,7 @@ sub ModBiblio { _koha_marc_update_biblioitem_cn_sort( $record, $oldbiblio, $frameworkcode ); # update the MARC record (that now contains biblio and items) with the new record data - &ModBiblioMarc( $record, $biblionumber, $frameworkcode ); + &ModBiblioMarc( $record, $biblionumber, $biblioitemnumber, $frameworkcode ); # modify the other koha tables _koha_modify_biblio( $dbh, $oldbiblio, $frameworkcode ); @@ -429,6 +429,9 @@ sub DelBiblio { # delete this biblioitem $error = _koha_delete_biblioitems( $dbh, $biblioitemnumber ); return $error if $error; + + $error = _koha_delete_biblio_metadata( $dbh, $biblioitemnumber, $biblionumber ); + return $error if $error; } # delete biblio from Koha tables and save in deletedbiblio @@ -436,6 +439,7 @@ sub DelBiblio { # delete cascade will prevent deletedbiblioitems rows # from being generated by _koha_delete_biblioitems $error = _koha_delete_biblio( $dbh, $biblionumber ); + return $error if $error; logaction( "CATALOGUING", "DELETE", $biblionumber, "biblio" ) if C4::Context->preference("CataloguingLog"); @@ -3438,11 +3442,35 @@ sub _koha_delete_biblioitems { return; } +=head2 _koha_delete_biblio_metadata + + $error = _koha_delete_biblio_metadata($dbh, $biblioitemnumber, $biblionumber); + +Internal sub for deleting from biblio_metadata table -- also saves to deletedbiblio_metadata + +C<$dbh> - the database handle +C<$biblioitemnumber> - the biblioitemnumber of the biblio_metadata to be deleted + +=cut + +# FIXME: add error handling + +sub _koha_delete_biblio_metadata { + my ( $dbh, $biblioitemnumber ) = @_; + + my $m = Koha::Biblio::Metadatas->find({biblioitemnumber => $biblioitemnumber}); + die "_koha_delete_biblio_metadata($biblioitemnumber):> No biblio_metadata-row!" unless $m; + + my $dm = Koha::Biblio::DeletedMetadatas->new( $m->_result->{_column_data} ); + $dm->store; + $m->delete(); +} + =head1 UNEXPORTED FUNCTIONS =head2 ModBiblioMarc - &ModBiblioMarc($newrec,$biblionumber,$frameworkcode); + &ModBiblioMarc($newrec,$biblionumber,$biblioitemnumber,$frameworkcode); Add MARC XML data for a biblio to koha @@ -3453,7 +3481,7 @@ Function exported, but should NOT be used, unless you really know what you're do sub ModBiblioMarc { # pass the MARC::Record to this function, and it will create the records in # the marcxml field - my ( $record, $biblionumber, $frameworkcode ) = @_; + my ( $record, $biblionumber, $biblioitemnumber, $frameworkcode ) = @_; if ( !$record ) { carp 'ModBiblioMarc passed an undefined record'; return; @@ -3500,6 +3528,7 @@ sub ModBiblioMarc { } my $metadata = { + biblioitemnumber => $biblioitemnumber, biblionumber => $biblionumber, format => 'marcxml', marcflavour => C4::Context->preference('marcflavour'), diff --git a/Koha/Biblio/DeletedMetadata.pm b/Koha/Biblio/DeletedMetadata.pm new file mode 100644 index 0000000..5a96ff2 --- /dev/null +++ b/Koha/Biblio/DeletedMetadata.pm @@ -0,0 +1,32 @@ +package Koha::Biblio::DeletedMetadata; + +# This file is part of Koha. +# + +use Modern::Perl; + +use Carp; + +use Koha::Database; + +use base qw(Koha::Object); + +=head1 NAME + +Koha::Biblio::DeletedMetadata - Koha Metadata Object class + +=head1 API + +=head2 Class Methods + +=cut + +=head3 type + +=cut + +sub _type { + return 'DeletedbiblioMetadata'; +} + +1; diff --git a/Koha/Biblio/DeletedMetadatas.pm b/Koha/Biblio/DeletedMetadatas.pm new file mode 100644 index 0000000..23e4e08 --- /dev/null +++ b/Koha/Biblio/DeletedMetadatas.pm @@ -0,0 +1,38 @@ +package Koha::Biblio::DeletedMetadatas; + +# This file is part of Koha. +# + +use Modern::Perl; + +use Carp; + +use Koha::Database; + +use Koha::Biblio::Metadata; + +use base qw(Koha::Objects); + +=head1 NAME + +Koha::Biblio::DeletedMetadatas - Koha Metadata Object set class + +=head1 API + +=head2 Class Methods + +=cut + +=head3 type + +=cut + +sub _type { + return 'DeletedbiblioMetadata'; +} + +sub object_class { + return 'Koha::Biblio::DeletedMetadata'; +} + +1; diff --git a/Koha/Biblio/Metadata.pm b/Koha/Biblio/Metadata.pm index f271f6f..9c5dbe3 100644 --- a/Koha/Biblio/Metadata.pm +++ b/Koha/Biblio/Metadata.pm @@ -25,7 +25,7 @@ use base qw(Koha::Object); =head1 NAME -Koha::Metadata - Koha Metadata Object class +Koha::Biblio::Metadata - Koha Metadata Object class =head1 API diff --git a/installer/data/mysql/atomicupdate/Bug-18265-Followup_to_Buuug_17196.sql b/installer/data/mysql/atomicupdate/Bug-18265-Followup_to_Buuug_17196.sql new file mode 100644 index 0000000..e4a401c --- /dev/null +++ b/installer/data/mysql/atomicupdate/Bug-18265-Followup_to_Buuug_17196.sql @@ -0,0 +1,15 @@ +ALTER TABLE biblio_metadata DROP FOREIGN KEY `record_metadata_fk_1`; +ALTER TABLE biblio_metadata DROP KEY `biblio_metadata_uniq_key`; +ALTER TABLE biblio_metadata ADD COLUMN `biblioitemnumber` int(11) NOT NULL; +ALTER TABLE biblio_metadata ADD FOREIGN KEY `biblioitems_metadata_fk_1` (biblioitemnumber) REFERENCES biblioitems (biblioitemnumber) ON DELETE CASCADE ON UPDATE CASCADE; +ALTER TABLE biblio_metadata ADD UNIQUE KEY `biblio_metadata_uniq_key` (`biblioitemnumber`, `biblionumber`,`format`,`marcflavour`); + + + +ALTER TABLE deletedbiblio_metadata DROP FOREIGN KEY `deletedrecord_metadata_fk_1`; +ALTER TABLE deletedbiblio_metadata DROP KEY `deletedbiblio_metadata_uniq_key`; +ALTER TABLE deletedbiblio_metadata ADD COLUMN `biblioitemnumber` int(11) NOT NULL; +ALTER TABLE deletedbiblio_metadata ADD FOREIGN KEY `deletedbiblioitems_metadata_fk_1` (biblioitemnumber) REFERENCES deletedbiblioitems (biblioitemnumber) ON DELETE CASCADE ON UPDATE CASCADE; +ALTER TABLE deletedbiblio_metadata ADD UNIQUE KEY `deletedbiblio_metadata_uniq_key` (`biblioitemnumber`, `biblionumber`,`format`,`marcflavour`); + + diff --git a/t/db_dependent/Biblio_crud.t b/t/db_dependent/Biblio_crud.t new file mode 100644 index 0000000..8be3546 --- /dev/null +++ b/t/db_dependent/Biblio_crud.t @@ -0,0 +1,99 @@ +#!/usr/bin/perl + +# This file is part of Koha. + +use Modern::Perl; +use Test::More; + +use C4::Biblio; +use Koha::Biblios; +use Koha::Biblio::Metadatas; +use Koha::Biblio::DeletedMetadatas; + + + +##This test is not intended to fully cover all aspects of Biblio creation and it's linked business objects. +##It is simply intended for now to test biblio_metadata-table population. +subtest "Scenario: CRUD a Biblio, happy path", \&crudBiblio; +sub crudBiblio { plan tests => 9; + my ($record, $kohaBiblio, $deletedKohaBiblio, $biblionumber, $biblioitemnumber, $metadata, $deletedMetadata); + my $title = 'Incurable disease of test apparatuses'; + eval { + + $record = C4::Biblio::TransformKohaToMarc({ + title => $title, + isbn => '0123456789', + }); + ok($record, + "Given a fresh Biblio from the top of my head"); + + subtest "Create", sub { + ($biblionumber, $biblioitemnumber) = C4::Biblio::AddBiblio($record,''); + ok($biblionumber && $biblioitemnumber, + "When the Biblio is added to DB"); + }; + + subtest "Read", sub { + $kohaBiblio = Koha::Biblios->find($biblionumber); + ok($kohaBiblio, + "Then it can be found from the DB"); + + is($kohaBiblio->biblionumber, + "And it looks like a real biblio"); + + $metadata = Koha::Biblio::Metadatas->find($biblioitemnumber); + ok($metadata, + "When I fetch the Metadata record from DB"); + + is($metadata->format, 'marcxml', + "Then the Metadata record format is 'marcxml'"); + + is($metadata->biblionumber, $biblionumber, + "And biblionumber matches the original Biblio"); + + is($metadata->biblioitemnumber, $biblioitemnumber, + "And the biblioitemnumber matches the original Biblio"); + + #Shouldn't marcflavour be just flavour since we don't always store MARC to the biblio_metadata? + is($metadata->marcflavour, C4::Context->marcformat(), + "And the marcflavour is the default"); + + like($metadata->metadata, qr/$title/, + "And the record itself contains the correct title"); + }; + + subtest "Delete", sub { + ok(C4::Biblio::DelBiblio($biblionumber), + "When I delete the Biblio"); + + ok(! Koha::Biblios->find($biblionumber), + "Then I get nothing when I fetch it from DB"); + + $deletedKohaBiblio = Koha::DeletedBiblios->find($biblionumber); + ok($deletedKohaBiblio, + "But I find it in the deletedbiblios"); + + is($deletedKohaBiblio->title, $kohaBiblio->title, + "And the Biblio looks just like before!"); + + ok(! Koha::Biblio::Metadatas->find($biblioitemnumber), + "When I fetch the Metadata record, I get nothing"); + + $deletedMetadata = Koha::Biblio::DeletedMetadatas->find($biblioitemnumber); + ok($deletedMetadata, + "But I found it in the deletedbiblio_metadata"); + + is_deeply($deletedMetadata->_result->{_column_data}, $metadata->_result->{_column_data}, + "And the Metadata looks just like before!"); + }; + + }; + ok(0, $@) if $@; +} + + + + + +done_testing; + -- 2.7.4