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 &quot;[% missing_critical.key %]&quot; in unrecognized format: &quot;[% missing_critical.value %]&quot;
-            [% ELSE %]
-                Critical field &quot;[% missing_critical.key %]&quot;
-                [% IF ( missing_critical.branch_map ) %]has unrecognized value &quot;[% missing_critical.value %]&quot;
-                [% ELSIF ( missing_critical.category_map ) %]has unrecognized value &quot;[% missing_critical.value %]&quot;
-                [% 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 &quot;[% missing_critical.key %]&quot; in unrecognized format: &quot;[% missing_critical.value %]&quot;
+              [% ELSE %]
+                  Critical field &quot;[% missing_critical.key %]&quot;
+                  [% IF ( missing_critical.branch_map ) %]has unrecognized value &quot;[% missing_critical.value %]&quot;
+                  [% ELSIF ( missing_critical.category_map ) %]has unrecognized value &quot;[% missing_critical.value %]&quot;
+                  [% 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