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 <m.de.rooy@rijksmuseum.nl>
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 <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com>

diff --git a/Koha/Exporter/Record.pm b/Koha/Exporter/Record.pm
index 6fad1f3..aa805e2 100644 (file)
--- a/Koha/Exporter/Record.pm
+++ b/Koha/Exporter/Record.pm
@@ -8,6 +8,7 @@ use C4::AuthoritiesMarc;
 use C4::Biblio;
 use C4::Record;
 use Koha::CsvProfiles;
+use Koha::Logger;
 
 sub _get_record_for_export {
     my ($params)           = @_;
@@ -22,11 +23,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) {
@@ -99,7 +97,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;
@@ -116,8 +117,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();
@@ -130,7 +133,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 72b8c9f..8566e4f 100644 (file)
--- 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;