From 6c24da119d3ccb77e505e97f2063f22c2666f712 Mon Sep 17 00:00:00 2001 From: Jonathan Druart Date: Tue, 14 Dec 2010 22:58:19 +0100 Subject: [PATCH] [PASSED QA] Bug 5771 : Adds a download link for big error reports to import_borrowers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If the import patrons tools produced a lot of errors(e.g. 1000), the browser will crash or not be glad. This patch allow to download an error file for report producing at least 25 errors. Test plan: Import patrons with at least 25 errors and check that a "Download report" button appears and produce a text file to download. Signed-off-by: Holger Meißner Signed-off-by: Martin Renvoize --- .../prog/en/modules/tools/import_borrowers.tt | 89 +++++++------ tools/import_borrowers.pl | 136 +++++++++++++++----- 2 files changed, 152 insertions(+), 73 deletions(-) diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/tools/import_borrowers.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/tools/import_borrowers.tt index 1ed23d0..6c4d53f 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/tools/import_borrowers.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/tools/import_borrowers.tt @@ -23,12 +23,12 @@ [% IF ( uploadborrowers ) %]
Import results :
    -
  • [% imported %] imported records [% IF ( lastimported ) %](last was [% lastimported %])[% END %]
  • -
  • [% overwritten %] overwritten [% IF ( lastoverwritten ) %](last was [% lastoverwritten %])[% END %]
  • -
  • [% alreadyindb %] not imported because already in borrowers table and overwrite disabled [% IF ( lastalreadyindb ) %](last was [% lastalreadyindb %])[% END %]
  • -
  • [% invalid %] not imported because they are not in the expected format [% IF ( lastinvalid ) %](last was [% lastinvalid %])[% END %]
  • -
  • [% total %] records parsed
  • -
  • Back to Tools
  • +
  • [% imported %] imported records [% IF ( lastimported ) %](last was [% lastimported %])[% END %]
  • +
  • [% overwritten %] overwritten [% IF ( lastoverwritten ) %](last was [% lastoverwritten %])[% END %]
  • +
  • [% alreadyindb %] not imported because already in borrowers table and overwrite disabled [% IF ( lastalreadyindb ) %](last was [% lastalreadyindb %])[% END %]
  • +
  • [% invalid %] not imported because they are not in the expected format [% IF ( lastinvalid ) %](last was [% lastinvalid %])[% END %]
  • +
  • [% total %] records parsed
  • +
  • Back to Tools
[% IF ( FEEDBACK ) %]

@@ -48,29 +48,35 @@ [% END %] [% IF ( ERRORS ) %] -

Error analysis:
-
    - [% FOREACH ERROR IN ERRORS %] + [% IF download_errors %] +
    + + + +
    + [% ELSE %] +
      + [% FOREACH ERROR IN ERRORS %] [% IF ( ERROR.badheader ) %]
    • Header row could not be parsed
    • [% END %] - [% FOREACH missing_critical IN ERROR.missing_criticals %] -
    • - Line [% missing_critical.line %] - [% IF ( missing_critical.badparse ) %] - could not be parsed! - [% ELSIF ( missing_critical.bad_date ) %] - has "[% missing_critical.key %]" in unrecognized format: "[% missing_critical.value %]" - [% ELSE %] - Critical field "[% missing_critical.key %]" - [% IF ( missing_critical.branch_map ) %]has unrecognized value "[% missing_critical.value %]" - [% ELSIF ( missing_critical.category_map ) %]has unrecognized value "[% missing_critical.value %]" - [% ELSE %]missing - [% END %] - (borrowernumber: [% missing_critical.borrowernumber %]; surname: [% missing_critical.surname %]). - [% END %] -
      [% missing_critical.lineraw %] -
    • + [% FOREACH missing_critical IN ERROR.missing_criticals %] +
    • + Line [% missing_critical.line %] + [% IF ( missing_critical.badparse ) %] + could not be parsed! + [% ELSIF ( missing_critical.bad_date ) %] + has "[% missing_critical.key %]" in unrecognized format: "[% missing_critical.value %]" + [% ELSE %] + Critical field "[% missing_critical.key %]" + [% IF ( missing_critical.branch_map ) %]has unrecognized value "[% missing_critical.value %]" + [% ELSIF ( missing_critical.category_map ) %]has unrecognized value "[% missing_critical.value %]" + [% ELSE %]missing + [% END %] + (borrowernumber: [% missing_critical.borrowernumber %]; surname: [% missing_critical.surname %]). + [% END %] +
      [% missing_critical.lineraw %] +
    • [% END %] [% IF ERROR.invalid_cardnumber %]
    • @@ -80,21 +86,22 @@ [% END %] [% END %]
    + [% END %]
[% END %] [% ELSE %]
    -
  • Select a file to import into the borrowers table
  • -
  • If a cardnumber exists in the table, you can choose whether to ignore the new one or overwrite the old one.
  • +
  • Select a file to import into the borrowers table
  • +
  • If a cardnumber exists in the table, you can choose whether to ignore the new one or overwrite the old one.
Import into the borrowers table
    -
  1. - - -
  2. +
  3. + + +
Field to use for record matching @@ -113,25 +120,25 @@ Default values
    [% FOREACH columnkey IN columnkeys %] -
  1. +
  2. - -
  3. + + [% END %]
-
- If matching record is already in the borrowers table: +
+ If matching record is already in the borrowers table:
  1. - +
[% IF ( ExtendedPatronAttributes ) %] -
- Patron Attributes +
+ Patron Attributes
  1. @@ -141,7 +148,7 @@
[% END %] -
+
[% END %] diff --git a/tools/import_borrowers.pl b/tools/import_borrowers.pl index 4445db5..5e8772d 100755 --- a/tools/import_borrowers.pl +++ b/tools/import_borrowers.pl @@ -1,7 +1,7 @@ #!/usr/bin/perl # Copyright 2007 Liblime -# Parts copyright 2010 BibLibre +# Parts copyright 2013 BibLibre # # This file is part of Koha. # @@ -34,8 +34,7 @@ # dates should be in the format you have set up Koha to expect # branchcode and categorycode need to be valid -use strict; -use warnings; +use Modern::Perl; use C4::Auth; use C4::Output; @@ -46,8 +45,12 @@ use C4::Members; use C4::Members::Attributes qw(:all); use C4::Members::AttributeTypes; use C4::Members::Messaging; - +use Koha::DateUtils; +use Date::Calc qw(Today_and_Now); +use Getopt::Long; +use File::Temp; use Text::CSV; + # Text::CSV::Unicode, even in binary mode, fails to parse lines with these diacriticals: # ė # č @@ -75,10 +78,9 @@ my ( $template, $loggedinuser, $cookie ) = get_template_and_user({ authnotrequired => 0, flagsrequired => { tools => 'import_patrons' }, debug => 1, -}); + }); $template->param(columnkeys => $columnkeystpl); - if ($input->param('sample')) { print $input->header( -type => 'application/vnd.sun.xml.calc', # 'application/vnd.ms-excel' ? @@ -88,6 +90,23 @@ if ($input->param('sample')) { print $csv->string, "\n"; exit 1; } + +if ($input->param('report')) { + print $input->header( + -type => 'text/plain', + -attachment => 'import_borrowers_report.txt' + ); + my $filename = $input->param('errors_filename'); + if ( -f $filename ) { + open my $fh, '<', $filename; + print <$fh>; + close $fh; + + unlink $filename; + exit 1; + } +} + my $uploadborrowers = $input->param('uploadborrowers'); my $matchpoint = $input->param('matchpoint'); if ($matchpoint) { @@ -110,7 +129,7 @@ if ( $uploadborrowers && length($uploadborrowers) > 0 ) { my $alreadyindb = 0; my $overwritten = 0; my $invalid = 0; - my $matchpoint_attr_type; + my $matchpoint_attr_type; my %defaults = $input->Vars; # use header line to construct key to column map @@ -121,11 +140,11 @@ if ( $uploadborrowers && length($uploadborrowers) > 0 ) { my %csvkeycol; my $col = 0; foreach my $keycol (@csvcolumns) { - # columnkeys don't contain whitespace, but some stupid tools add it - $keycol =~ s/ +//g; + # columnkeys don't contain whitespace, but some stupid tools add it + $keycol =~ s/ +//g; $csvkeycol{$keycol} = $col++; } - #warn($borrowerline); + my $ext_preserve = $input->param('ext_preserve') || 0; if ($extended) { $matchpoint_attr_type = C4::Members::AttributeTypes->fetch($matchpoint); @@ -137,6 +156,8 @@ if ( $uploadborrowers && length($uploadborrowers) > 0 ) { my @bad_dates; # I've had a few. my $date_re = C4::Dates->new->regexp('syspref'); my $iso_re = C4::Dates->new->regexp('iso'); + my $lastalreadyindb; + my $lastinvalid; LINE: while ( my $borrowerline = <$handle> ) { my %borrower; my @missing_criticals; @@ -152,23 +173,23 @@ if ( $uploadborrowers && length($uploadborrowers) > 0 ) { if ($borrower{$key} !~ /\S/) { $borrower{$key} = $defaults{$key}; } - } + } } else { # MJR: try to recover gracefully by using default values foreach my $key (@columnkeys) { - if (defined($csvkeycol{$key}) and $columns[$csvkeycol{$key}] =~ /\S/) { - $borrower{$key} = $columns[$csvkeycol{$key}]; - } elsif ( $defaults{$key} ) { - $borrower{$key} = $defaults{$key}; - } elsif ( scalar grep {$key eq $_} @criticals ) { - # a critical field is undefined - push @missing_criticals, {key=>$key, line=>$., lineraw=>$borrowerline}; - } else { - $borrower{$key} = ''; - } + if (defined($csvkeycol{$key}) and $columns[$csvkeycol{$key}] =~ /\S/) { + $borrower{$key} = $columns[$csvkeycol{$key}]; + } elsif ( $defaults{$key} ) { + $borrower{$key} = $defaults{$key}; + } elsif ( scalar grep {$key eq $_} @criticals ) { + # a critical field is undefined + push @missing_criticals, {key=>$key, line=>$., lineraw=>$borrowerline}; + } else { + $borrower{$key} = ''; + } } } - #warn join(':',%borrower); + if ($borrower{categorycode}) { push @missing_criticals, {key=>'categorycode', line=>$. , lineraw=>$borrowerline, value=>$borrower{categorycode}, category_map=>1} unless GetBorrowercategory($borrower{categorycode}); @@ -187,8 +208,7 @@ if ( $uploadborrowers && length($uploadborrowers) > 0 ) { $_->{surname} = $borrower{surname} || 'UNDEF'; } $invalid++; - (25 > scalar @errors) and push @errors, {missing_criticals=>\@missing_criticals}; - # The first 25 errors are enough. Keeping track of 30,000+ would destroy performance. + push @errors, {missing_criticals=>\@missing_criticals}; next LINE; } if ($extended) { @@ -197,9 +217,9 @@ if ( $uploadborrowers && length($uploadborrowers) > 0 ) { $attr_str =~ s/\xe2\x80\x9d/"/g; push @feedback, {feedback=>1, name=>'attribute string', value=>$attr_str, filename=>$uploadborrowers}; delete $borrower{patron_attributes}; # not really a field in borrowers, so we don't want to pass it to ModMember. - $patron_attributes = extended_attributes_code_value_arrayref($attr_str); + $patron_attributes = extended_attributes_code_value_arrayref($attr_str); } - # Popular spreadsheet applications make it difficult to force date outputs to be zero-padded, but we require it. + # Popular spreadsheet applications make it difficult to force date outputs to be zero-padded, but we require it. foreach (qw(dateofbirth dateenrolled dateexpiry)) { my $tempdate = $borrower{$_} or next; if ($tempdate =~ /$date_re/) { @@ -211,8 +231,8 @@ if ( $uploadborrowers && length($uploadborrowers) > 0 ) { push @missing_criticals, {key=>$_, line=>$. , lineraw=>$borrowerline, bad_date=>1}; } } - $borrower{dateenrolled} = $today_iso unless $borrower{dateenrolled}; - $borrower{dateexpiry} = GetExpiryDate($borrower{categorycode},$borrower{dateenrolled}) unless $borrower{dateexpiry}; + $borrower{dateenrolled} = $today_iso unless $borrower{dateenrolled}; + $borrower{dateexpiry} = GetExpiryDate($borrower{categorycode},$borrower{dateenrolled}) unless $borrower{dateexpiry}; my $borrowernumber; my $member; if ( ($matchpoint eq 'cardnumber') && ($borrower{'cardnumber'}) ) { @@ -242,12 +262,12 @@ if ( $uploadborrowers && length($uploadborrowers) > 0 ) { next; } - if ($borrowernumber) { # borrower exists unless ($overwrite_cardnumber) { $alreadyindb++; - $template->param('lastalreadyindb'=>$borrower{'surname'}.' / '.$borrowernumber); + $lastalreadyindb = $borrower{'surname'}.' / '.$borrowernumber; + $template->param('lastalreadyindb'=>$lastalreadyindb); next LINE; } $borrower{'borrowernumber'} = $borrowernumber; @@ -266,7 +286,8 @@ if ( $uploadborrowers && length($uploadborrowers) > 0 ) { $invalid++; # untill we have better error trapping, we have no way of knowing why ModMember errored out... push @errors, {unknown_error => 1}; - $template->param('lastinvalid'=>$borrower{'surname'}.' / '.$borrowernumber); + $lastinvalid = $borrower{'surname'}.' / '.$borrowernumber; + $template->param('lastinvalid'=>$lastinvalid); next LINE; } if ($extended) { @@ -312,6 +333,58 @@ if ( $uploadborrowers && length($uploadborrowers) > 0 ) { 'total' => $imported + $alreadyindb + $invalid + $overwritten, ); + my $max_errors = 25; + if (scalar(@errors) > $max_errors ) { + + my $total = $imported + $alreadyindb + $invalid + $overwritten; + my $output; + + my $timestamp = output_pref( dt_from_string ); + $output .= "Timestamp : $timestamp\n"; + $output .= "Import results\n"; + $output .= "$imported imported records\n"; + $output .= "$overwritten overwritten records\n"; + $output .= "$alreadyindb not imported because already in borrowers table and overwrite disabled\n"; + $output .= "(last was $lastalreadyindb)\n" if ($lastalreadyindb); + $output .= "$invalid not imported because they are not in the expected format\n"; + $output .= "(last was $lastinvalid)\n" if ($lastinvalid); + $output .= "$total records parsed\n"; + + + $output .= "\nError analysis\n"; + foreach my $error (@errors) { + $output .= "Header row could not be parsed" if ($error->{'badheader'}); + foreach my $array ($error->{'missing_criticals'}) { + foreach (@$array) { + $output .= "Line $_->{'line'}: "; + if ($error->{'badparse'}) { + $output .= "could not be parsed!"; + } elsif ($error->{'bad_date'}) { + $output .= "has $_->{'key'} in unrecognized format: $_->{'value'} "; + } else { + $output .= "Critical field $_->{'key'}: "; + if ($_->{'branch_map'} || $_->{'category_map'}) { + $output .= "has unrecognized value: $_->{'value'}"; + } else { + $output .= " missing"; + } + $output .= " (borrowernumber: $_->{'borrowernumber'}; surname: $_->{'surname'})"; + } + $output .= "\n"; + } + } + } + + my $tmpf = File::Temp->new(UNLINK => 0); + print $tmpf $output; + $template->param( + download_errors => 1, + errors_filename => $tmpf->filename + ); + close $tmpf; + + } + } else { if ($extended) { my @matchpoints = (); @@ -327,4 +400,3 @@ if ( $uploadborrowers && length($uploadborrowers) > 0 ) { } output_html_with_http_headers $input, $cookie, $template->output; - -- 1.7.10.4