Second installment on moving patronimages into the database

Signed-off-by: Joshua Ferraro <jmf@liblime.com>

koha-tmpl/intranet-tmpl/prog/en/modules/help/members/picture-upload.tmpl

@@ -0,0 +1,6 @@
+<!-- TMPL_INCLUDE NAME="help-top.inc" -->
+
+<h1>Patron Image Upload Screen</h1>
+<p>Select a file to upload to the server. Each .jpg file contained therein will be copied to the appropriate place on the server for patron pictures.</p>
+<p>You can include multiple pictures in a .zip file.</p>
+<p>There should be a DATALINK.TXT or IDLINK.TXT file for each group of pictures that has the cardnumber of the patron and the file containing that patrons picture.  One patron per line seperated by either commas or tabs.  Quotes around the fields are ignored.</p>

koha-tmpl/intranet-tmpl/prog/en/modules/tools/picture-upload.tmpl

@@ -15,7 +15,15 @@
     <div class="yui-b">
         <div class="yui-g">
             <div class="yui-u first">
+            <!-- TMPL_IF name="ERRORS" -->
+                <div class="dialog alert">
+                <h1>Patron Images Uploaded With Some Errors</h1>
+                </div>
+            <!-- TMPL_ELSE -->
+                <div class="dialog message">
                 <h1>Patron Images Successfully Uploaded</h1>
+                </div>
+            <!-- /TMPL_IF -->
 	        <ul class="data">
 	            <li>Unpacking completed</li>
                   <li><!-- TMPL_VAR NAME ="TOTAL" --> directories scanned.</li>
@@ -23,13 +31,20 @@
                 </ul>
                 <!-- TMPL_LOOP name="COUNTS" -->
 	            <ul class="data">
-                        <li><!-- TMPL_VAR name="TCOUNTS" --> image(s) moved from <!-- TMPL_VAR name="source" --> to <!-- TMPL_VAR name="dest" -->:</li>
+                        <li><!-- TMPL_VAR name="TCOUNTS" --> image(s) moved into the database:</li>
-                    </ul>
                         <!-- TMPL_LOOP name="filenames" -->
-                           <ul class="data">
+                            <li><!-- TMPL_VAR name="source" --> - Cardnumber: <!-- TMPL_VAR name="cardnumber" -->
-                            <li><!-- TMPL_VAR name="source" --> To <!-- TMPL_VAR name="dest" --></li>
+                                    <!-- TMPL_IF NAME="filerrors" --><br /> <b>WARNING:</b> This image <i>not</i> imported because 
-                            </ul>
+                                    <!-- TMPL_LOOP NAME="filerrors" -->
+                                        <!-- TMPL_IF NAME="DBERR" -->the database returned an error.</li><!-- /TMPL_IF -->
+                                        <!-- TMPL_IF NAME="MIMERR" -->the image format is unrecognized.</li><!-- /TMPL_IF -->
+                                        <!-- TMPL_IF NAME="OPNERR" -->Koha was unable to open the image for reading.</li>
+                                        <!-- TMPL_ELSE -->of an unknown error. Please refer to the error log for more details.</li><!-- /TMPL_IF -->
+                                    <!-- /TMPL_LOOP -->
+                                        <!--TMPL_ELSE --> imported successfully.</li>
+                                    <!-- /TMPL_IF -->
                         <!-- /TMPL_LOOP -->
+                        </ul>
                 <!-- /TMPL_LOOP -->
                 <a id="uploadmore" href="/cgi-bin/koha/tools/picture-upload.pl">Upload More Images</a>
                 <a id="doneupload" href="/cgi-bin/koha/tools/tools-home.pl">Return to Tools</a>
@@ -41,25 +56,32 @@
                 <h1>Upload Patron Images</h1>
                 <!-- TMPL_IF name="ERRORS" -->
                 <div class="dialog alert">
-                    <ul>
                     <!-- TMPL_LOOP name="ERRORS" -->
-		        <!-- TMPL_IF NAME="NODIR" -->
-			    <li>The <span class="ex">patronimages/</span> directory isn't present on your server, please ask your systems administrator to create it</li>
-		        <!-- /TMPL_IF -->
                         <!-- TMPL_IF name="NOTZIP" -->
-                            <li>The upload file does not appear to be a zip file.  The extention is not '.zip'.</li>
+                            <li><b>The upload file does not appear to be a zip file.  The extention is not '.zip'.</b></li>
                         <!-- /TMPL_IF -->
                         <!-- TMPL_IF name="NOWRITETEMP" -->
-                            <li>This script is not able to create/write to the necessary temporary directory.</li>
+                            <li><b>This script is not able to create/write to the necessary temporary directory.</b></li>
-                        <!-- /TMPL_IF -->
-                        <!-- TMPL_IF name="NOWRITEDEST" -->
-                            <li>This script is not able to write to the patron image storage directory.</li>
                         <!-- /TMPL_IF -->
                         <!-- TMPL_IF name="EMPTYUPLOAD" -->
-                            <li>The upload file appears to be empty.</li>
+                            <li><b>The upload file appears to be empty.</b></li>
+                        <!-- /TMPL_IF -->
+                        <!-- TMPL_IF name="OPNLINK" -->
+                            <li><b>Cannot open <!-- TMPL_VAR NAME="OPNLINK" --> to read.<br />Please verify that it exists.</b></li>
+                        <!-- /TMPL_IF -->
+                        <!-- TMPL_IF name="OPNIMG" -->
+                            <li><b>Cannot open <!-- TMPL_VAR NAME="OPNIMG" --> to read.<br />Please verify that it exists.</b></li>
+                        <!-- /TMPL_IF -->
+                        <!-- TMPL_IF name="DELERR" -->
+                            <li><b>Unrecognized or missing field delimeter.<br />Please verify that you are using either a single quote or a tab.</b></li>
+                        <!-- /TMPL_IF -->
+                        <!-- TMPL_IF name="UZIPFAIL" -->
+                            <li><b><!-- TMPL_VAR name="UZIPFAIL" --> failed to unpack.<br />Please verify the integrity of the zip file and retry.</b></li>
+                        <!-- /TMPL_IF -->
+                        <!-- TMPL_IF name="" -->
+                            <li><b>An unknown error has occurred.<br />Please review the error log for more details.</b></li>
                         <!-- /TMPL_IF -->
                     <!-- /TMPL_LOOP -->
-		    </ul>
                 </div>
                 <!-- /TMPL_IF -->
 	        <form method="post" action="/cgi-bin/koha/tools/picture-upload.pl" enctype="multipart/form-data">
@@ -75,14 +97,6 @@
                 </form>
 	
             </div>	
-            <div class="yui-u">
-            <h2>Notes:</h2>
-                <ul>
-                    <li>Select a file to upload to the server. Each .jpg file contained therein will be copied to the appropriate place on the server for patron pictures.</li>
-                    <li>You can include multiple pictures in a .zip file.</li>
-                    <li>There should be a DATALINK.TXT or IDLINK.TXT file for each group of pictures that has the cardnumber of the patron and the file containing that patrons picture.  One patron per line seperated by either commas or tabs.  Quotes around the fields are ignored.</li>
-                </ul>
-            </div>
 <!-- /TMPL_IF -->
         </div>
     </div>

tools/picture-upload.pl

@@ -7,15 +7,12 @@ use C4::Context;
 use C4::Auth;
 use C4::Output;
 use C4::Members;
+use Data::Dumper;
 
 my $DEBUG = ($ENV{DEBUG}) ? 1 : 0;
 
-#my $destdir = "/usr/local/koha/intranet/htdocs/intranet-tmpl/images/patronpictures";
-#my $uploadfile = shift @ARGV;
 my $input = new CGI;
-my $destdir = C4::Context->config('intrahtdocs') . "/patronimages";
 
-warn "DEST : $destdir";
 my ($template, $loggedinuser, $cookie)
 	= get_template_and_user({template_name => "tools/picture-upload.tmpl",
 					query => $input,
@@ -25,13 +22,6 @@ my ($template, $loggedinuser, $cookie)
 					debug => 0,
 					});
 
-unless (-d $destdir) {
-	$errors{'NODIR'} = 1;
-	warn "patronimages directory not present";
-}
-if ( %errors ) {
-    $template->param( ERRORS => [ \%errors ] );
-}
 my $uploadfilename = $input->param( 'uploadfile' );
 my $uploadfile = $input->upload( 'uploadfile' );
 my ( $total, $handled, @counts );
@@ -41,11 +31,10 @@ if ( $uploadfile ) {
     warn "dirname = $dirname" if $DEBUG;
     my ( $tfh, $tempfile ) = File::Temp::tempfile( SUFFIX => '.zip', UNLINK => 1 );
     warn "tempfile = $tempfile" if $DEBUG;
-    my ( @directories, %errors );
+    my ( @directories, $errors );
 
     $errors{'NOTZIP'} = 1 unless ( $uploadfilename =~ /\.zip$/i );
     $errors{'NOWRITETEMP'} = 1 unless ( -w $dirname );
-    $errors{'NOWRITEDEST'} = 1 unless ( -w $destdir );
     $errors{'EMPTYUPLOAD'} = 1 unless ( length( $uploadfile ) > 0 );
 
     if ( %errors ) {
@@ -53,34 +42,44 @@ if ( $uploadfile ) {
     } else {
 	while ( <$uploadfile> ) {
 	    print $tfh $_;
-	}
+        }
 
-	close $tfh;
+        close $tfh;
 
-	system("unzip $tempfile -d $dirname");
+        unless (system("unzip $tempfile -d $dirname")) {
+            $errors{'UZIPFAIL'} = $uploadfilename;
+	    $template->param( ERRORS => [ \%errors ] );
+            output_html_with_http_headers $input, $cookie, $template->output;   # This error is fatal to the import, so bail out here
+            exit;
+        }
+        push @directories, "$dirname";
+        foreach $recursive_dir ( @directories ) {
+            opendir $dir, $recursive_dir;
+            while ( my $entry = readdir $dir ) {
+	    push @directories, "$recursive_dir/$entry" if ( -d "$recursive_dir/$entry" and $entry !~ /^\./ );
+            warn "$recursive_dir/$entry" if $DEBUG;
+            }   
+            closedir $dir;
+        }       
+        my $results;
+        foreach my $dir ( @directories ) {
+            $results = handle_dir( $dir );
+            $handled++ if $results == 1;
+        }
 
-	push @directories, "$dirname";
+        if ( %$results || %errors ) {
-	foreach $recursive_dir ( @directories ) {
+            $template->param( ERRORS => [ \%$results ] );
-	    opendir $dir, $recursive_dir;
+        } else {
-	    while ( my $entry = readdir $dir ) {
+            $total = scalar @directories;
-			push @directories, "$recursive_dir/$entry" if ( -d "$recursive_dir/$entry" and $entry !~ /^\./ );
+            warn "Total files processed: $total" if $DEBUG;
-                        warn "$recursive_dir/$entry" if $DEBUG;
+            warn "Errors in \$errors." if $errors;
-	    }
+            $template->param(
-	    closedir $dir;
+    		 TOTAL => $total,
-	}
+		 HANDLED => $handled,
-
+		 COUNTS => \@counts,
-	foreach my $dir ( @directories ) {
+		 TCOUNTS => scalar(@counts),
-	    $handled += handle_dir( $dir );
+            );
-	}
+        }   
-
-	$total = scalar @directories;
-        warn "Total files processed: $total" if $DEBUG;
-	$template->param(
-			 TOTAL => $total,
-			 HANDLED => $handled,
-			 COUNTS => \@counts,
-			 TCOUNTS => scalar(@counts),
-			 );
     }
 }
 
@@ -105,43 +104,68 @@ sub handle_dir {
         $file = "$dir/$filename" if ($filename =~ m/datalink\.txt/i || $filename =~ m/idlink\.txt/i);
     }
     unless (open (FILE, $file)) { 
-		warn "Opening $dir/$file failed!" if $DEBUG;
+		warn "Opening $dir/$file failed!";
-		return 0;
+                $errors{'OPNLINK'} = $file;
-	};
+		return $errors; # This error is fatal to the import of this directory contents, so bail and return the error to the caller
+    };
 
     while (my $line = <FILE>) {
         warn "Reading contents of $file" if $DEBUG;
 	chomp $line;
         warn "Examining line: $line" if $DEBUG;
         my ( $filename, $cardnumber );
-	my $delim = ($line =~ /\t/) ? "\t" : ",";
+	my $delim = ($line =~ /\t/) ? "\t" : ($line =~ /,/) ? "," : "";
         warn "Delimeter is \'$delim\'" if $DEBUG;
+        unless ( $delim eq "," || $delim eq "\t" ) {
+            warn "Unrecognized or missing field delimeter. Please verify that you are using either a ',' or a 'tab'";
+            $errors{'DELERR'} = 1;      # This error is fatal to the import of this directory contents, so bail and return the error to the caller
+            return $errors;
+        }
 	($cardnumber, $filename) = split $delim, $line;
 	$cardnumber =~ s/[\"\r\n]//g;  # remove offensive characters
 	$filename   =~ s/[\"\r\n\s]//g;
         warn "Cardnumber: $cardnumber Filename: $filename" if $DEBUG;
 	if ($cardnumber && $filename) {
+            my %filerrors;
             warn "Source: $dir/$filename" if $DEBUG;
-            open (IMG, "$dir/$filename") or warn "Could not open $dir/$filename";
+            if (open (IMG, "$dir/$filename")) {
-            #binmode (IMG); # Not sure if we need this or not -fbcit
+                #binmode (IMG); # Not sure if we need this or not -fbcit
-            my $imgfile;
+                my $imgfile;
-            while (<IMG>) {
+                while (<IMG>) {
-                $imgfile .= $_;
+                    $imgfile .= $_;
+                }
+                my $mimetype = $mimemap->{lc ($1)} if $filename =~ m/\.([^.]+)$/i;
+                warn "$filename is mimetype \"$mimetype\"" if $DEBUG;
+                my $dberror = PutPatronImage($cardnumber,$mimetype, $imgfile) if $mimetype;
+                close (IMG);
+	        if ( !$dberror && $mimetype ) { # Errors from here on are fatal only to the import of a particular image, so don't bail, just note the error and keep going
+	            $count{count}++;
+	            push @{ $count{filenames} }, { source => $filename, cardnumber => $cardnumber };
+	        } elsif ( $dberror ) {
+                    warn "Database returned error. We're not logging it because it most likely contains binary data which does unpleasent things to terminal windows and logs.";
+                    $filerrors{'DBERR'} = 1;
+                    push my @filerrors, \%filerrors;
+	            push @{ $count{filenames} }, { filerrors => \@filerrors, source => $filename, cardnumber => $cardnumber };
+                    $template->param( ERRORS => 1 );
+                } elsif ( !$mimetype ) {
+                    warn "Unable to determine mime type of $filename. Please verify mimetype and add to \%mimemap if necessary.";
+                    $filerrors{'MIMERR'} = 1;
+                    push my @filerrors, \%filerrors;
+	            push @{ $count{filenames} }, { filerrors => \@filerrors, source => $filename, cardnumber => $cardnumber };
+                    $template->param( ERRORS => 1 );
+                }
+            } else {
+                warn "Opening $dir/$filename failed!";
+                $filerrors{'OPNERR'} = 1;
+                push my @filerrors, \%filerrors;
+	        push @{ $count{filenames} }, { filerrors => \@filerrors, source => $filename, cardnumber => $cardnumber };
+                $template->param( ERRORS => 1 );
             }
-            my $mimetype = $mimemap->{lc ($1)} if $filename =~ m/\.([^.]+)$/i;
-            warn "$filename is mimetype \"$mimetype\"" if $DEBUG;
-            my $dberror = PutPatronImage($cardnumber,$mimetype, $imgfile) if $mimetype;
-#            warn "Database says: $dberror" if $dberror;
-            close (IMG);
-	    unless ( $dberror || !$mimetype ) {
-	        $count{count}++;
-	        push @{ $count{filenames} }, { source => $filename, dest => $cardnumber };
-	    }
 	}
     }
     $count{source} = $dir;
-    $count{dest} = $destdir;
     push @counts, \%count;
     close FILE;
+    closedir ( $dirhandle );
     return 1;
 }