From 5e838385e18532bdd91414676fa789dc1bc65a1b Mon Sep 17 00:00:00 2001 From: Ere Maijala Date: Wed, 4 Jul 2018 14:56:34 +0300 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. --- C4/Holdings.pm | 47 +++++++++++++ Koha/Exporter/Record.pm | 78 +++++++++++++++++----- .../intranet-tmpl/prog/en/modules/tools/export.tt | 9 ++- misc/export_records.pl | 15 ++++- misc/migration_tools/bulkmarcimport.pl | 61 ++++++++++++++--- t/db_dependent/Exporter/Record.t | 63 ++++++++++++++++- tools/export.pl | 3 + 7 files changed, 246 insertions(+), 30 deletions(-) diff --git a/C4/Holdings.pm b/C4/Holdings.pm index 5e27fae..6704a4b 100644 --- a/C4/Holdings.pm +++ b/C4/Holdings.pm @@ -378,6 +378,53 @@ sub GetXmlHolding { return $marcxml; } +=head2 GetMarcHoldingsByBiblionumber + + my $records = GetMarcHoldingsByBiblionumber(biblionumber); + +Returns MARC::Record array representing holding records + +=over 4 + +=item C<$biblionumber> + +biblionumber + +=back + +=cut + +sub GetMarcHoldingsByBiblionumber { + my $biblionumber = shift; + + my $marcflavour = C4::Context->preference('marcflavour'); + my $sth = C4::Context->dbh->prepare( + q| + SELECT metadata + FROM holdings_metadata + WHERE holding_id IN (SELECT holding_id FROM holdings WHERE biblionumber=?) + AND format='marcxml' + AND marcflavour=? + | + ); + + $sth->execute( $biblionumber, $marcflavour ); + + my @records; + while (my ($marcxml) = $sth->fetchrow()) { + $marcxml = StripNonXmlChars( $marcxml ); + my $record = eval { + MARC::Record::new_from_xml( $marcxml, "utf8", $marcflavour ); + }; + if ($@) { + warn " problem with holding for biblio $biblionumber : $@ \n$marcxml"; + } + push @records, $record if $record; + } + $sth->finish(); + return \@records; +} + =head2 GetHoldingFrameworkCode $frameworkcode = GetFrameworkCode( $holding_id ) diff --git a/Koha/Exporter/Record.pm b/Koha/Exporter/Record.pm index 42ac002..caad528 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 { @@ -144,6 +148,26 @@ 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 = C4::Holdings::GetMarcHoldingsByBiblionumber($biblionumber); + foreach my $record (@$records) { + _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); + } + } + return $records; +} + sub export { my ($params) = @_; @@ -155,6 +179,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 +209,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 +235,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 5df6fb1..e069a3f 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/tools/export.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/tools/export.tt @@ -117,7 +117,14 @@
Options -
  1. +
      + [% IF ( show_summary_holdings ) %] +
    1. + + +
      2. + [% END %] +
      3. diff --git a/misc/export_records.pl b/misc/export_records.pl index a1eecc8..e331cb8 100755 --- a/misc/export_records.pl +++ b/misc/export_records.pl @@ -53,6 +53,7 @@ my ( $start_accession, $end_accession, $marc_conditions, + $export_holdings, $help ); @@ -77,6 +78,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); @@ -111,6 +113,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; @@ -243,6 +250,7 @@ else { format => $output_format, csv_profile_id => $csv_profile_id, export_items => (not $dont_export_items), + export_holdings => $export_holdings, clean => $clean || 0, } ); @@ -256,7 +264,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 @@ -281,6 +289,11 @@ Print a brief help message. --record-type=TYPE TYPE is 'bibs' or 'auths'. +=item B<--holdings> + + --holdings Export MARC 21 holding 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 d820d3e..8144455 100755 --- a/misc/migration_tools/bulkmarcimport.pl +++ b/misc/migration_tools/bulkmarcimport.pl @@ -24,6 +24,7 @@ use C4::Debug; use C4::Charset; use C4::Items; use C4::MarcModificationTemplates; +use C4::Holdings; use YAML; use Unicode::Normalize; @@ -40,7 +41,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 = ''; @@ -84,13 +85,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; @@ -174,11 +179,12 @@ if ($fk_off) { if ($delete) { - if ($biblios){ - print "deleting biblios\n"; - $dbh->do("truncate biblio"); - $dbh->do("truncate biblioitems"); - $dbh->do("truncate items"); + if ($biblios) { + print "deleting biblios\n"; + $dbh->do("truncate holdings"); + $dbh->do("truncate biblio"); + $dbh->do("truncate biblioitems"); + $dbh->do("truncate items"); } else { print "deleting authorities\n"; @@ -193,8 +199,6 @@ if ($test_parameter) { print "TESTING MODE ONLY\n DOING NOTHING\n===============\n"; } -my $marcFlavour = C4::Context->preference('marcflavour') || 'MARC21'; - print "Characteristic MARC flavour: $marcFlavour\n" if $verbose; my $starttime = gettimeofday; my $batch; @@ -260,6 +264,7 @@ my $searcher = Koha::SearchEngine::Search->new( } ); +my $biblionumber; RECORD: while ( ) { my $record; # get records @@ -308,11 +313,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' ); @@ -428,8 +441,29 @@ RECORD: while ( ) { $yamlhash->{$originalid}->{'subfields'} = \@subfields; } } + elsif ($holdings_record) { + if ($import_holdings) { + if (!defined $biblionumber) { + warn "ERROR: Encountered holdings record without preceding biblio record\n"; + } else { + if ($insert) { + my $holdings_id = 0; + eval { ( $holdings_id ) = AddHolding( $record, '', $biblionumber ) }; + if ($@) { + warn "ERROR: Adding holdings record $holdings_id for biblio $biblionumber failed: $@\n"; + printlog( { $holdings_id, op => "insert", status => "ERROR" } ) if ($logfile); + next RECORD; + } else { + printlog( { $holdings_id, op => "insert", status => "ok" } ) if ($logfile); + } + } else { + printlog( { 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) { @@ -701,6 +735,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" @@ -746,7 +785,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 68eef06..faacecf 100644 --- 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; @@ -35,6 +35,7 @@ use Koha::Biblio; use Koha::Biblioitem; use Koha::Exporter::Record; use Koha::Biblio::Metadata; +use Koha::Holdings; my $schema = Koha::Database->new->schema; $schema->storage->txn_begin; @@ -62,6 +63,13 @@ my $bad_biblio = Koha::Biblio->new()->store(); Koha::Biblio::Metadata->new( { biblionumber => $bad_biblio->id, format => 'marcxml', metadata => 'something wrong', marcflavour => C4::Context->preference('marcflavour') } )->store(); my $bad_biblionumber = $bad_biblio->id; +my $holding_1 = MARC::Record->new(); +$holding_1->leader('00202cx a22000973 4500'); +$holding_1->append_fields( + MARC::Field->new('852', '8', ' ', b => 'Location', k => '1973', h => 'Tb', t => 'Copy 1') +); +C4::Holdings::AddHolding($holding_1, '', $biblionumber_1); + my $builder = t::lib::TestBuilder->new; my $item_1_1 = $builder->build({ source => 'Item', @@ -199,6 +207,59 @@ subtest 'export iso2709' => sub { is( $title, $biblio_2_title, 'Export ISO2709: The title is correctly encoded' ); }; +subtest 'export xml with holdings' => sub { + plan tests => 2; + 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 $location = $holding_record->subfield('852', 'b'); + is( $location, 'Location', 'Export XML with holdings: The holding record is correctly exported' ); +}; + +subtest 'export iso2709 with holdings' => sub { + plan tests => 2; + 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 $location = $holding_record->subfield('852', 'b'); + is( $location, 'Location', 'Export ISO2709 with holdings: The holding record is correctly exported' ); +}; + subtest 'export without record_type' => sub { plan tests => 1; diff --git a/tools/export.pl b/tools/export.pl index 0704b57..33c0d8c 100755 --- a/tools/export.pl +++ b/tools/export.pl @@ -35,6 +35,7 @@ use Koha::Libraries; my $query = new CGI; 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'; @@ -208,6 +209,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, } ); @@ -302,6 +304,7 @@ else { export_remove_fields => C4::Context->preference("ExportRemoveFields"), csv_profiles => [ Koha::CsvProfiles->search({ type => 'marc', used_for => 'export_records' }) ], messages => \@messages, + show_summary_holdings => C4::Context->preference('SummaryHoldings') ? 1 : 0, ); output_html_with_http_headers $query, $cookie, $template->output; -- 2.7.4