From f56087f76791bf25ddc3c17e4ea8b5ef10d7e38a Mon Sep 17 00:00:00 2001 From: Ere Maijala Date: Wed, 20 Feb 2019 16:32:02 +0200 Subject: [PATCH] Bug 20447: Add import/export support for holdings https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20447 Adds support for importing and exporting biblios with interleaved holdings. Test plan: 1.) Import the MARCXML file attached to the bug with the -holdings parameter: misc/migration_tools/bulkmarcimport.pl -biblios -holdings -file holdings.xml -m MARCXML 2.) Verify that the holdings records were imported. 3.) Export the same records with holdings: misc/export_records.pl --record-type=bibs --starting_biblionumber 103 --ending_biblionumber 104 --filename=holdings_export.xml --format=xml --holdings 4.) Verify that both bibliographic and holding records are exported. 5.) Verify that tests in t/db_dependent/Exporter/Record.t pass. --- Koha/Exporter/Record.pm | 84 ++++++++++--- .../prog/en/modules/tools/export.tt | 10 +- misc/export_records.pl | 15 ++- misc/migration_tools/bulkmarcimport.pl | 64 ++++++++-- t/db_dependent/Exporter/Record.t | 116 +++++++++++++++++- tools/export.pl | 2 + 6 files changed, 262 insertions(+), 29 deletions(-) diff --git a/Koha/Exporter/Record.pm b/Koha/Exporter/Record.pm index eaebe57639..448755fb09 100644 --- a/Koha/Exporter/Record.pm +++ b/Koha/Exporter/Record.pm @@ -82,27 +82,31 @@ sub _get_record_for_export { } } - if ($dont_export_fields) { - for my $f ( split / /, $dont_export_fields ) { - if ( $f =~ m/^(\d{3})(.)?$/ ) { - my ( $field, $subfield ) = ( $1, $2 ); - - # skip if this record doesn't have this field - if ( defined $record->field($field) ) { - if ( defined $subfield ) { - my @tags = $record->field($field); - foreach my $t (@tags) { - $t->delete_subfields($subfield); - } - } else { - $record->delete_fields( $record->field($field) ); + _strip_unwanted_fields($record, $dont_export_fields) if $dont_export_fields; + C4::Biblio::RemoveAllNsb($record) if $clean; + return $record; +} + +sub _strip_unwanted_fields { + my ($record, $dont_export_fields) = @_; + + for my $f ( split / /, $dont_export_fields ) { + if ( $f =~ m/^(\d{3})(.)?$/ ) { + my ( $field, $subfield ) = ( $1, $2 ); + + # skip if this record doesn't have this field + if ( defined $record->field($field) ) { + if ( defined $subfield ) { + my @tags = $record->field($field); + foreach my $t (@tags) { + $t->delete_subfields($subfield); } + } else { + $record->delete_fields( $record->field($field) ); } } } } - C4::Biblio::RemoveAllNsb($record) if $clean; - return $record; } sub _get_authority_for_export { @@ -128,7 +132,8 @@ sub _get_biblio_for_export { C4::Biblio::EmbedItemsInMarcBiblio({ marc_record => $record, biblionumber => $biblionumber, - item_numbers => $itemnumbers }); + item_numbers => $itemnumbers, + skip_holdings => 1 }); if ($only_export_items_for_branches && @$only_export_items_for_branches) { my %export_items_for_branches = map { $_ => 1 } @$only_export_items_for_branches; my ( $homebranchfield, $homebranchsubfield ) = GetMarcFromKohaField( 'items.homebranch' ); @@ -144,6 +149,29 @@ sub _get_biblio_for_export { return $record; } +sub _get_holdings_for_export { + my ($params) = @_; + my $dont_export_fields = $params->{dont_export_fields}; + my $clean = $params->{clean}; + my $biblionumber = $params->{biblionumber}; + + my @records; + my $holdings = Koha::Holdings->search({ biblionumber => $biblionumber, deleted_on => undef }); + while (my $holding = $holdings->next()) { + my $record = $holding->metadata()->record(); + _strip_unwanted_fields($record, $dont_export_fields) if $dont_export_fields; + C4::Biblio::RemoveAllNsb($record) if $clean; + C4::Biblio::UpsertMarcControlField($record, '004', $biblionumber); + my $leader = $record->leader(); + if ($leader !~ /^.{6}[uvxy]/) { + $leader =~ s/^(.{6})./$1u/; + $record->leader($leader); + } + push @records, $record; + } + return \@records; +} + sub export { my ($params) = @_; @@ -155,6 +183,7 @@ sub export { my $dont_export_fields = $params->{dont_export_fields}; my $csv_profile_id = $params->{csv_profile_id}; my $output_filepath = $params->{output_filepath}; + my $export_holdings = $params->{export_holdings} // 0; if( !$record_type ) { Koha::Logger->get->warn( "No record_type given." ); @@ -184,6 +213,20 @@ sub export { next; } print $record->as_usmarc(); + if ($export_holdings && $record_type eq 'bibs') { + my $holdings = _get_holdings_for_export( { %$params, biblionumber => $record_id } ); + foreach my $holding (@$holdings) { + my $errorcount_on_decode = eval { scalar( MARC::File::USMARC->decode( $holding->as_usmarc )->warnings() ) }; + if ( $errorcount_on_decode or $@ ) { + my $msg = "Holdings record for biblio $record_id could not be exported. " . + ( $@ // '' ); + chomp $msg; + Koha::Logger->get->info( $msg ); + next; + } + print $holding->as_usmarc(); + } + } } } elsif ( $format eq 'xml' ) { my $marcflavour = C4::Context->preference("marcflavour"); @@ -196,6 +239,13 @@ sub export { next unless $record; print MARC::File::XML::record($record); print "\n"; + if ($export_holdings && $record_type eq 'bibs') { + my $holdings = _get_holdings_for_export( { %$params, biblionumber => $record_id } ); + foreach my $holding (@$holdings) { + print MARC::File::XML::record($holding); + print "\n"; + } + } } print MARC::File::XML::footer(); print "\n"; diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/tools/export.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/tools/export.tt index 4e9a992c04..3c5abdcf1b 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/tools/export.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/tools/export.tt @@ -1,6 +1,7 @@ [% USE raw %] [% USE Asset %] [% USE Branches %] +[% USE Koha %] [% SET footerjs = 1 %] [% INCLUDE 'doc-head-open.inc' %] Koha › Tools › Export data @@ -117,7 +118,14 @@
Options -
  1. +
      + [% IF ( Koha.Preference('SummaryHoldings') ) %] +
    1. + + +
      2. + [% END %] +
      3. diff --git a/misc/export_records.pl b/misc/export_records.pl index 2a05e794df..2063c22a29 100755 --- a/misc/export_records.pl +++ b/misc/export_records.pl @@ -54,6 +54,7 @@ my ( $start_accession, $end_accession, $marc_conditions, + $export_holdings, $help ); @@ -78,6 +79,7 @@ GetOptions( 'start_accession=s' => \$start_accession, 'end_accession=s' => \$end_accession, 'marc_conditions=s' => \$marc_conditions, + 'holdings' => \$export_holdings, 'h|help|?' => \$help ) || pod2usage(1); @@ -112,6 +114,11 @@ if ( $deleted_barcodes and $record_type ne 'bibs' ) { pod2usage(q|--deleted_barcodes can only be used with biblios|); } +my $marcFlavour = C4::Context->preference('marcflavour') || 'MARC21'; +if ( $export_holdings and ( $record_type ne 'bibs' or $marcFlavour ne 'MARC21' ) ) { + pod2usage(q|--holdings can only be used with MARC 21 biblios|); +} + $start_accession = dt_from_string( $start_accession ) if $start_accession; $end_accession = dt_from_string( $end_accession ) if $end_accession; @@ -255,6 +262,7 @@ else { format => $output_format, csv_profile_id => $csv_profile_id, export_items => (not $dont_export_items), + export_holdings => $export_holdings, clean => $clean || 0, } ); @@ -268,7 +276,7 @@ export records - This script exports record (biblios or authorities) =head1 SYNOPSIS -export_records.pl [-h|--help] [--format=format] [--date=datetime] [--record-type=TYPE] [--dont_export_items] [--deleted_barcodes] [--clean] [--id_list_file=PATH] --filename=outputfile +export_records.pl [-h|--help] [--format=format] [--date=datetime] [--record-type=TYPE] [--holdings] [--dont_export_items] [--deleted_barcodes] [--clean] [--id_list_file=PATH] --filename=outputfile =head1 OPTIONS @@ -293,6 +301,11 @@ Print a brief help message. --record-type=TYPE TYPE is 'bibs' or 'auths'. +=item B<--holdings> + + --holdings Export MARC 21 holdings records interleaved with bibs. Used only if TYPE + is 'bibs' and FORMAT is 'xml' or 'marc'. + =item B<--dont_export_items> --dont_export_items If enabled, the item infos won't be exported. diff --git a/misc/migration_tools/bulkmarcimport.pl b/misc/migration_tools/bulkmarcimport.pl index 80d3396cad..6d81b70067 100755 --- a/misc/migration_tools/bulkmarcimport.pl +++ b/misc/migration_tools/bulkmarcimport.pl @@ -34,6 +34,7 @@ use IO::File; use Pod::Usage; use Koha::Biblios; +use Koha::Holdings; use Koha::SearchEngine; use Koha::SearchEngine::Search; @@ -41,7 +42,7 @@ use open qw( :std :encoding(UTF-8) ); binmode( STDOUT, ":encoding(UTF-8)" ); my ( $input_marc_file, $number, $offset) = ('',0,0); my ($version, $delete, $test_parameter, $skip_marc8_conversion, $char_encoding, $verbose, $commit, $fk_off,$format,$biblios,$authorities,$keepids,$match, $isbn_check, $logfile); -my ( $insert, $filters, $update, $all, $yamlfile, $authtypes, $append ); +my ( $insert, $filters, $update, $all, $yamlfile, $authtypes, $append, $import_holdings ); my $cleanisbn = 1; my ($sourcetag,$sourcesubfield,$idmapfl, $dedup_barcode); my $framework = ''; @@ -85,13 +86,17 @@ GetOptions( 'framework=s' => \$framework, 'custom:s' => \$localcust, 'marcmodtemplate:s' => \$marc_mod_template, + 'holdings' => \$import_holdings, ); $biblios ||= !$authorities; $insert ||= !$update; my $writemode = ($append) ? "a" : "w"; +my $marcFlavour = C4::Context->preference('marcflavour') || 'MARC21'; pod2usage( -msg => "\nYou must specify either --biblios or --authorities, not both.\n", -exitval ) if $biblios && $authorities; +pod2usage( -msg => "\nHoldings only supported for MARC 21 biblios.\n", -exitval ) if $import_holdings && (!$biblios || $marcFlavour ne 'MARC21'); + if ($all) { $insert = 1; $update = 1; @@ -169,12 +174,14 @@ if ($fk_off) { if ($delete) { - if ($biblios){ - print "deleting biblios\n"; + if ($biblios) { + print "deleting biblios\n"; $dbh->do("DELETE FROM biblio"); $dbh->do("ALTER TABLE biblio AUTO_INCREMENT = 1"); $dbh->do("DELETE FROM biblioitems"); $dbh->do("ALTER TABLE biblioitems AUTO_INCREMENT = 1"); + $dbh->do("DELETE FROM holdings"); + $dbh->do("ALTER TABLE holdings AUTO_INCREMENT = 1"); $dbh->do("DELETE FROM items"); $dbh->do("ALTER TABLE items AUTO_INCREMENT = 1"); } @@ -191,8 +198,6 @@ if ($test_parameter) { print "TESTING MODE ONLY\n DOING NOTHING\n===============\n"; } -my $marcFlavour = C4::Context->preference('marcflavour') || 'MARC21'; - # The definition of $searcher must be before MARC::Batch->new my $searcher = Koha::SearchEngine::Search->new( { @@ -230,6 +235,7 @@ if (defined $format && $format =~ /XML/i) { $batch->warnings_off(); $batch->strict_off(); my $i=0; +my $holdings_count = 0; my $commitnum = $commit ? $commit : 50; my $yamlhash; @@ -259,6 +265,7 @@ if ($logfile){ print $loghandle "id;operation;status\n"; } +my $biblionumber; RECORD: while ( ) { my $record; # get records @@ -307,11 +314,19 @@ RECORD: while ( ) { $field->update('a' => $isbn); } } + + # check for holdings records + my $holdings_record = 0; + if ($biblios && $marcFlavour eq 'MARC21') { + my $leader = $record->leader(); + $holdings_record = $leader =~ /^.{6}[uvxy]/; + } + my $id; # search for duplicates (based on Local-number) my $originalid; $originalid = GetRecordId( $record, $tagid, $subfieldid ); - if ($match) { + if ($match && !$holdings_record) { require C4::Search; my $query = build_query( $match, $record ); my $server = ( $authorities ? 'authorityserver' : 'biblioserver' ); @@ -418,8 +433,30 @@ RECORD: while ( ) { $yamlhash->{$originalid}->{'updated'} = 1; } } + elsif ($holdings_record) { + if ($import_holdings) { + if (!defined $biblionumber) { + warn "ERROR: Encountered holdings record without preceding biblio record\n"; + } else { + if ($insert) { + my $holding = Koha::Holding->new({ biblionumber => $biblionumber }); + $holding->set_marc({ record => $record }); + my $branchcode = $holding->holdingbranch(); + if (defined $branchcode && !Koha::Libraries->find({ branchcode => $branchcode})) { + printlog( { id => 0, op => 'insert', status => "warning : library $branchcode not defined, holdings record skipped" } ) if ($logfile); + } else { + $holding->store(); + ++$holdings_count; + printlog( { id => $holding->holding_id(), op => 'insert', status => 'ok' } ) if ($logfile); + } + } else { + printlog( { id => 0, op => 'update', status => 'warning : not in database' } ) if ($logfile); + } + } + } + } else { - my ( $biblionumber, $biblioitemnumber, $itemnumbers_ref, $errors_ref ); + my ( $biblioitemnumber, $itemnumbers_ref, $errors_ref ); $biblionumber = $id; # check for duplicate, based on ISBN (skip it if we already have found a duplicate with match parameter if (!$biblionumber && $isbn_check && $isbn) { @@ -555,7 +592,11 @@ delete $ENV{OVERRIDE_SYSPREF_CataloguingLog}; delete $ENV{OVERRIDE_SYSPREF_AuthoritiesLog}; my $timeneeded = gettimeofday - $starttime; -print "\n$i MARC records done in $timeneeded seconds\n"; +if ($import_holdings) { + printf("\n%i MARC bibliographic records and %i holdings records done in %0.3f seconds\n", $i, $holdings_count, $timeneeded); +} else { + printf("\n%i MARC records done in %0.3f seconds\n", $i, $timeneeded); +} if ($logfile){ print $loghandle "file : $input_marc_file\n"; print $loghandle "$i MARC records done in $timeneeded seconds\n"; @@ -676,6 +717,11 @@ Type of import: authority records The I to import +=item B<-holdings> + +Import MARC 21 holdings records when interleaved with bibliographic records +(insert only, update not supported). Used only with -biblios. + =item B<-v> Verbose mode. 1 means "some infos", 2 means "MARC dumping" @@ -721,7 +767,7 @@ I are supported. MARC21 by default. =item B<-d> Delete EVERYTHING related to biblio in koha-DB before import. Tables: biblio, -biblioitems, items +biblioitems, items, holdings =item B<-m>=I diff --git a/t/db_dependent/Exporter/Record.t b/t/db_dependent/Exporter/Record.t index 9993d7874b..1268279d59 100755 --- a/t/db_dependent/Exporter/Record.t +++ b/t/db_dependent/Exporter/Record.t @@ -17,7 +17,7 @@ use Modern::Perl; -use Test::More tests => 6; +use Test::More tests => 8; use Test::Warn; use t::lib::TestBuilder; @@ -34,7 +34,11 @@ use Koha::Database; use Koha::Biblio; use Koha::Biblioitem; use Koha::Exporter::Record; +use Koha::Biblio::Metadata; use Koha::Biblio::Metadatas; +use Koha::BiblioFramework; +use Koha::BiblioFrameworks; +use Koha::Holdings; my $schema = Koha::Database->new->schema; $schema->storage->txn_begin; @@ -62,7 +66,60 @@ my $bad_biblio = Koha::Biblio->new()->store(); Koha::Biblio::Metadata->new( { biblionumber => $bad_biblio->id, format => 'marcxml', metadata => 'something wrong', schema => C4::Context->preference('marcflavour') } )->store(); my $bad_biblionumber = $bad_biblio->id; +# Add a framework for holdings +my $frameworkcode = 'HLD'; +my $existing_mss = Koha::MarcSubfieldStructures->search({frameworkcode => $frameworkcode}); +$existing_mss->delete() if $existing_mss; +my $existing_fw = Koha::BiblioFrameworks->find({frameworkcode => $frameworkcode}); +$existing_fw->delete() if $existing_fw; +Koha::BiblioFramework->new({ + frameworkcode => $frameworkcode, + frameworktext => 'Holdings' +})->store(); +Koha::MarcSubfieldStructure->new({ + frameworkcode => $frameworkcode, + tagfield => 852, + tagsubfield => 'b', + kohafield => 'holdings.holdingbranch' +})->store(); +Koha::MarcSubfieldStructure->new({ + frameworkcode => $frameworkcode, + tagfield => 852, + tagsubfield => 'c', + kohafield => 'holdings.location' +})->store(); +Koha::MarcSubfieldStructure->new({ + frameworkcode => $frameworkcode, + tagfield => 942, + tagsubfield => 'n', + kohafield => 'holdings.suppress' +})->store(); +Koha::MarcSubfieldStructure->new({ + frameworkcode => $frameworkcode, + tagfield => 999, + tagsubfield => 'c', + kohafield => 'biblio.biblionumber' +})->store(); +Koha::MarcSubfieldStructure->new({ + frameworkcode => $frameworkcode, + tagfield => 999, + tagsubfield => 'e', + kohafield => 'holdings.holding_id' +})->store(); + my $builder = t::lib::TestBuilder->new; +my $holdingbranch = $builder->build({ source => 'Branch' }); + +my $holding_marc_1 = MARC::Record->new(); +$holding_marc_1->leader('00202cx a22000973 4500'); +$holding_marc_1->append_fields( + MARC::Field->new('852', '8', ' ', b => $holdingbranch->{branchcode}, c => 'Location', k => '1973', h => 'Tb', t => 'Copy 1') +); +my $holding = Koha::Holding->new(); +$holding->biblionumber($biblionumber_1); +$holding->set_marc({ record => $holding_marc_1 }); +$holding->store(); + my $item_1_1 = $builder->build_sample_item( { biblionumber => $biblionumber_1, @@ -192,6 +249,63 @@ subtest 'export iso2709' => sub { is( $title, $biblio_2_title, 'Export ISO2709: The title is correctly encoded' ); }; +subtest 'export xml with holdings' => sub { + plan tests => 3; + my $generated_xml_file = '/tmp/test_export.xml'; + Koha::Exporter::Record::export( + { record_type => 'bibs', + record_ids => [ $biblionumber_1, $biblionumber_2 ], + format => 'xml', + output_filepath => $generated_xml_file, + export_holdings => 1, + } + ); + + my $generated_xml_content = read_file( $generated_xml_file ); + $MARC::File::XML::_load_args{BinaryEncoding} = 'utf-8'; + open my $fh, '<', $generated_xml_file; + my $records = MARC::Batch->new( 'XML', $fh ); + my @records; + # The following statement produces + # Use of uninitialized value in concatenation (.) or string at /usr/share/perl5/MARC/File/XML.pm line 398, <$fh> chunk 5. + # Why? + while ( my $record = $records->next ) { + push @records, $record; + } + is( scalar( @records ), 3, 'Export XML with holdings: 3 records should have been exported' ); + my $holding_record = $records[1]; + my $branch = $holding_record->subfield('852', 'b'); + is( $branch, $holdingbranch->{branchcode}, 'Export XML with holdings: The holdings record contains correct branch code' ); + my $location = $holding_record->subfield('852', 'c'); + is( $location, 'Location', 'Export XML with holdings: The holdings record contains correct location' ); +}; + +subtest 'export iso2709 with holdings' => sub { + plan tests => 3; + my $generated_mrc_file = '/tmp/test_export.mrc'; + # Get all item infos + Koha::Exporter::Record::export( + { record_type => 'bibs', + record_ids => [ $biblionumber_1, $biblionumber_2 ], + format => 'iso2709', + output_filepath => $generated_mrc_file, + export_holdings => 1, + } + ); + + my $records = MARC::File::USMARC->in( $generated_mrc_file ); + my @records; + while ( my $record = $records->next ) { + push @records, $record; + } + is( scalar( @records ), 3, 'Export ISO2709 with holdings: 3 records should have been exported' ); + my $holding_record = $records[1]; + my $branch = $holding_record->subfield('852', 'b'); + is( $branch, $holdingbranch->{branchcode}, 'Export ISO2709 with holdings: The holdings record contains correct branch code' ); + my $location = $holding_record->subfield('852', 'c'); + is( $location, 'Location', 'Export ISO2709 with holdings: The holdings record contains correct location' ); +}; + subtest 'export without record_type' => sub { plan tests => 1; diff --git a/tools/export.pl b/tools/export.pl index 6d677abf5b..cec1701f7a 100755 --- a/tools/export.pl +++ b/tools/export.pl @@ -35,6 +35,7 @@ use Koha::Libraries; my $query = CGI->new; my $dont_export_items = $query->param("dont_export_item") || 0; +my $export_holdings = $query->param("export_holdings") || 0; my $record_type = $query->param("record_type"); my $op = $query->param("op") || ''; my $output_format = $query->param("format") || $query->param("output_format") || 'iso2709'; @@ -210,6 +211,7 @@ if ( $op eq "export" ) { dont_export_fields => $export_remove_fields, csv_profile_id => $csv_profile_id, export_items => (not $dont_export_items), + export_holdings => $export_holdings, only_export_items_for_branches => $only_export_items_for_branches, } ); -- 2.25.1