From 64ee370e1f5bf700ad165dc672dcfd91b23229cd Mon Sep 17 00:00:00 2001 From: Jonathan Druart <jonathan.druart@biblibre.com> Date: Tue, 14 Dec 2010 22:58:19 +0100 Subject: [PATCH] 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 <h.meissner.82@web.de> --- .../prog/en/modules/tools/import_borrowers.tt | 91 +++++++------ tools/import_borrowers.pl | 137 +++++++++++++++----- 2 files changed, 154 insertions(+), 74 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 6744faf..6fb7308 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 ) %] <h5>Import results :</h5> <ul> - <li>[% imported %] imported records [% IF ( lastimported ) %](last was [% lastimported %])[% END %]</li> - <li>[% overwritten %] overwritten [% IF ( lastoverwritten ) %](last was [% lastoverwritten %])[% END %]</li> - <li>[% alreadyindb %] not imported because already in borrowers table and overwrite disabled [% IF ( lastalreadyindb ) %](last was [% lastalreadyindb %])[% END %]</li> - <li>[% invalid %] not imported because they are not in the expected format [% IF ( lastinvalid ) %](last was [% lastinvalid %])[% END %]</li> - <li>[% total %] records parsed</li> - <li><a href="/cgi-bin/koha/tools/tools-home.pl">Back to Tools</a></li> + <li>[% imported %] imported records [% IF ( lastimported ) %](last was [% lastimported %])[% END %]</li> + <li>[% overwritten %] overwritten [% IF ( lastoverwritten ) %](last was [% lastoverwritten %])[% END %]</li> + <li>[% alreadyindb %] not imported because already in borrowers table and overwrite disabled [% IF ( lastalreadyindb ) %](last was [% lastalreadyindb %])[% END %]</li> + <li>[% invalid %] not imported because they are not in the expected format [% IF ( lastinvalid ) %](last was [% lastinvalid %])[% END %]</li> + <li>[% total %] records parsed</li> + <li><a href="/cgi-bin/koha/tools/tools-home.pl">Back to Tools</a></li> </ul> [% IF ( FEEDBACK ) %] <br /><br /> @@ -48,47 +48,54 @@ </div> [% END %] [% IF ( ERRORS ) %] - <br /><br /> <div> <h5>Error analysis:</h5> - <ul> - [% FOREACH ERROR IN ERRORS %] + [% IF download_errors %] + <form method="post"> + <input type="hidden" name="errors_filename" value="[% errors_filename %]" /> + <input type="hidden" name="report" value="report" /> + <input type="submit" value="Download report" /> + </form> + [% ELSE %] + <ul> + [% FOREACH ERROR IN ERRORS %] [% IF ( ERROR.badheader ) %]<li>Header row could not be parsed</li>[% END %] - [% FOREACH missing_critical IN ERROR.missing_criticals %] - <li class="line_error"> - Line <span class="linenumber">[% missing_critical.line %]</span> - [% 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 %] - <br /><code>[% missing_critical.lineraw %]</code> - </li> + [% FOREACH missing_critical IN ERROR.missing_criticals %] + <li class="line_error"> + Line <span class="linenumber">[% missing_critical.line %]</span> + [% 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 %] + <br /><code>[% missing_critical.lineraw %]</code> + </li> [% END %] - [% END %] + [% END %] </ul> + [% END %] </div> [% END %] [% ELSE %] <ul> - <li>Select a file to import into the borrowers table</li> - <li>If a cardnumber exists in the table, you can choose whether to ignore the new one or overwrite the old one.</li> + <li>Select a file to import into the borrowers table</li> + <li>If a cardnumber exists in the table, you can choose whether to ignore the new one or overwrite the old one.</li> </ul> <form method="post" action="[% SCRIPT_NAME %]" enctype="multipart/form-data"> <fieldset class="rows"> <legend>Import into the borrowers table</legend> <ol> - <li> - <label for="uploadborrowers">Select the file to import: </label> - <input type="file" id="uploadborrowers" name="uploadborrowers" /> - </li> + <li> + <label for="uploadborrowers">Select the file to import: </label> + <input type="file" id="uploadborrowers" name="uploadborrowers" /> + </li> </ol></fieldset> <fieldset class="rows"> <legend>Field to use for record matching</legend> @@ -107,25 +114,25 @@ <legend>Default values</legend> <ol> [% FOREACH columnkey IN columnkeys %] - <li> + <li> <label class="widelabel" for="[% columnkey.key %]">[% columnkey.key %]: </label> - <input id="[% columnkey.key %]" name="[% columnkey.key %]" /> - </li> + <input id="[% columnkey.key %]" name="[% columnkey.key %]" /> + </li> [% END %] </ol></fieldset> - <fieldset class="rows"> - <legend>If matching record is already in the borrowers table:</legend> + <fieldset class="rows"> + <legend>If matching record is already in the borrowers table:</legend> <ol><li class="radio"> <input type="radio" id="overwrite_cardnumberno" name="overwrite_cardnumber" value="0" checked="checked" /><label for="overwrite_cardnumberno">Ignore this one, keep the existing one</label> </li> <li class="radio"> - <input type="radio" id="overwrite_cardnumberyes" name="overwrite_cardnumber" value="1" /><label for="overwrite_cardnumberyes">Overwrite the existing one with this</label> + <input type="radio" id="overwrite_cardnumberyes" name="overwrite_cardnumber" value="1" /><label for="overwrite_cardnumberyes">Overwrite the existing one with this</label> </li> </ol> </fieldset> [% IF ( ExtendedPatronAttributes ) %] - <fieldset class="rows"> - <legend>Patron Attributes</legend> + <fieldset class="rows"> + <legend>Patron Attributes</legend> <ol><li class="radio"> <input type="radio" id="ext_preserve_0" name="ext_preserve" value="0" checked="checked" /><label for="ext_preserve_0">Replace all Patron Attributes</label> </li> @@ -135,7 +142,7 @@ </ol> </fieldset> [% END %] - <fieldset class="action"><input type="submit" value="Import" /></fieldset> + <fieldset class="action"><input type="submit" value="Import" /></fieldset> </form> [% END %] </div> diff --git a/tools/import_borrowers.pl b/tools/import_borrowers.pl index 5f65c97..abf85ec 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'}) ) { @@ -231,12 +251,13 @@ if ( $uploadborrowers && length($uploadborrowers) > 0 ) { } } } - + 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; @@ -255,7 +276,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) { @@ -301,6 +323,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 = (); @@ -316,4 +390,3 @@ if ( $uploadborrowers && length($uploadborrowers) > 0 ) { } output_html_with_http_headers $input, $cookie, $template->output; - -- 1.7.10.4