From 40a2b8c12150d6fe193ba10c808b47069494157a Mon Sep 17 00:00:00 2001 From: Marcel de Rooy Date: Fri, 16 Sep 2016 11:27:05 +0200 Subject: [PATCH] Bug 17088: [Follow-up] Use Logger for failed exports Fixes a TODO for logging unsupported record_type in _get_record_for_export. Also logs a warning when the record_type parameter is not supplied at all in sub export. Replaces a warn by a log message about an invalid record for format iso2709. Also adds a log message about an invalid record for format xml. Adds a trivial unit test for passing no record_type to export. Test plan: See also first patch. Run t/db_dependent/Exporter/Record.t. Signed-off-by: Marcel de Rooy Also tested the log messages for iso2709 and xml by manipulating the record_type parameter with: $params->{record_type}='xx'; before calling _get_record_for_export in Record.pm. Signed-off-by: Jonathan Druart Signed-off-by: Jonathan Druart Signed-off-by: Brendan Gallagher (cherry picked from commit 0eec191ed3ab1ae1bd24972a559d12627b70f681) --- Koha/Exporter/Record.pm | 22 ++++++++++++++-------- t/db_dependent/Exporter/Record.t | 14 +++++++++++++- 2 files changed, 27 insertions(+), 9 deletions(-) diff --git a/Koha/Exporter/Record.pm b/Koha/Exporter/Record.pm index c441550bfa..0508660f86 100644 --- a/Koha/Exporter/Record.pm +++ b/Koha/Exporter/Record.pm @@ -7,6 +7,7 @@ use MARC::File::USMARC; use C4::AuthoritiesMarc; use C4::Biblio; use C4::Record; +use Koha::Logger; sub _get_record_for_export { my ($params) = @_; @@ -21,11 +22,8 @@ sub _get_record_for_export { } elsif ( $record_type eq 'bibs' ) { $record = _get_biblio_for_export( { %$params, biblionumber => $record_id } ); } else { - - # TODO log "record_type not supported" - return; + Koha::Logger->get->warn( "Record_type $record_type not supported." ); } - return unless $record; if ($dont_export_fields) { @@ -98,7 +96,10 @@ sub export { my $csv_profile_id = $params->{csv_profile_id}; my $output_filepath = $params->{output_filepath}; - return unless $record_type; + if( !$record_type ) { + Koha::Logger->get->warn( "No record_type given." ); + return; + } return unless @$record_ids; my $fh; @@ -115,8 +116,10 @@ sub export { my $record = _get_record_for_export( { %$params, record_id => $record_id } ); my $errorcount_on_decode = eval { scalar( MARC::File::USMARC->decode( $record->as_usmarc )->warnings() ) }; if ( $errorcount_on_decode or $@ ) { - warn $@ if $@; - warn "record (number $record_id) is invalid and therefore not exported because its reopening generates warnings above"; + my $msg = "Record $record_id could not be exported. " . + ( $@ // '' ); + chomp $msg; + Koha::Logger->get->info( $msg ); next; } print $record->as_usmarc(); @@ -129,7 +132,10 @@ sub export { print "\n"; for my $record_id (@$record_ids) { my $record = _get_record_for_export( { %$params, record_id => $record_id } ); - next unless $record; + if( !$record ) { + Koha::Logger->get->info( "Record $record_id could not be exported." ); + next; + } print MARC::File::XML::record($record); print "\n"; } diff --git a/t/db_dependent/Exporter/Record.t b/t/db_dependent/Exporter/Record.t index 72b8c9f42c..8566e4f0b9 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 => 3; +use Test::More tests => 4; use t::lib::TestBuilder; use MARC::Record; @@ -176,6 +176,18 @@ subtest 'export iso2709' => sub { is( $title, $biblio_2_title, 'Export ISO2709: The title is correctly encoded' ); }; +subtest 'export without record_type' => sub { + plan tests => 1; + + my $rv = Koha::Exporter::Record::export({ + record_ids => [ $biblionumber_1, $biblionumber_2 ], + format => 'iso2709', + output_filepath => 'does_not_matter_here', + }); + is( $rv, undef, 'export returns undef' ); + #Depending on your logger config, you might have a warn in your logs +}; + $schema->storage->txn_rollback; 1; -- 2.39.5