From e3fa9f2d5ffdc0542604f1a16a7b16a318879f0a Mon Sep 17 00:00:00 2001
From: Ryan Higgins <rch@liblime.com>
Date: Sat, 9 Aug 2008 15:27:27 -0500
Subject: [PATCH] Prevent borrower import from overwriting data that it
 shouldn't, and polish off a couple error conditions.

When an imported patron matches an existing one, keep data in the original record unless our import file
includes that column.  Also, check for branch & categorycodes, and produce error if invalid.  Finally,
auto-calculate expiry date if not given.

Signed-off-by: Joshua Ferraro <jmf@liblime.com>
---
 .../en/modules/tools/import_borrowers.tmpl    |  2 ++
 tools/import_borrowers.pl                     | 21 +++++++++++++++----
 2 files changed, 19 insertions(+), 4 deletions(-)

diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/tools/import_borrowers.tmpl b/koha-tmpl/intranet-tmpl/prog/en/modules/tools/import_borrowers.tmpl
index e6d6c82f0e..0c5f5c5319 100644
--- a/koha-tmpl/intranet-tmpl/prog/en/modules/tools/import_borrowers.tmpl
+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/tools/import_borrowers.tmpl
@@ -116,6 +116,8 @@ means that if an input record has more than one attribute, the 'patron_attribute
 <!-- /TMPL_IF -->
 <li>The fields 'branchcode' and 'categorycode' are <b>required</b> and <b>must match</b> valid entries in your database.</li>
 <li>'password' should be stored in plaintext, and will be converted to a md5 hash (if your passwords are already encrypted, talk to your systems administrator about options).</li>
+<li>Date formats should match your system preference, and <b>must</b> be zero-padded, e.g. '01/02/2008'.</li>
+<li>You may optionally include a header row, defining which columns you are supplying in the import file.</li>
 </ul>
 </div>
 </div>
diff --git a/tools/import_borrowers.pl b/tools/import_borrowers.pl
index d8c4068bd6..5cdf3c15f5 100755
--- a/tools/import_borrowers.pl
+++ b/tools/import_borrowers.pl
@@ -38,6 +38,7 @@ use C4::Auth;
 use C4::Output;
 use C4::Dates qw(format_date_in_iso);
 use C4::Context;
+use C4::Branch qw(GetBranchName);
 use C4::Members;
 use C4::Members::Attributes;
 use C4::Members::AttributeTypes;
@@ -51,7 +52,7 @@ my @columnkeys = C4::Members->columns;
 if ($extended) {
     push @columnkeys, 'patron_attributes';
 }
-my $columnkeystpl = [ map { {'key' => $_} } @columnkeys ];  # ref. to array of hashrefs.
+my $columnkeystpl = [ map { {'key' => $_} }  grep {$_ ne 'borrowernumber' && $_ ne 'cardnumber'} @columnkeys ];  # ref. to array of hashrefs.
 
 my $input = CGI->new();
 my $csv   = Text::CSV->new();
@@ -108,12 +109,11 @@ if ( $uploadborrowers && length($uploadborrowers) > 0 ) {
         $csvkeycol{$keycol} = $col++;
     }
     #warn($borrowerline);
-
     if ($extended) {
         $matchpoint_attr_type = C4::Members::AttributeTypes->fetch($matchpoint);
     }
 
-    my @criticals = qw(surname);    # there probably should be others
+    my @criticals = qw(cardnumber surname categorycode);    # there probably should be others
     my @errors;
     LINE: while ( my $borrowerline = <$uploadborrowers> ) {
         my %borrower;
@@ -141,6 +141,8 @@ if ( $uploadborrowers && length($uploadborrowers) > 0 ) {
             }
         }
         #warn join(':',%borrower);
+	push @missing_criticals, {key=>'categorycode' , line=>$. , lineraw=>$borrowerline } unless(  GetBorrowercategory($borrower{categorycode}) );
+	push @missing_criticals, {key=>'branchcode' , line=>$. , lineraw=>$borrowerline } unless(  GetBranchName($borrower{branchcode}) );
         if (@missing_criticals) {
             foreach (@missing_criticals) {
                 $_->{borrowernumber} = $borrower{borrowernumber} || 'UNDEF';
@@ -160,13 +162,17 @@ if ( $uploadborrowers && length($uploadborrowers) > 0 ) {
             # FIXME error handling
             $patron_attributes = [ map { map { my @arr = split /:/, $_, 2; { code => $arr[0], value => $arr[1] } } $_ } @list ];
         }
+	# FIXME date handling.  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;
             $borrower{$_} = format_date_in_iso($tempdate) || '';
         }
+	$borrower{dateenrolled} = C4::Dates->new()->output('iso') unless $borrower{dateenrolled};
+	$borrower{dateexpiry} = GetExpiryDate($borrower{categorycode},$borrower{dateenrolled}) unless $borrower{dateexpiry}; 
         my $borrowernumber;
+        my $member;
         if ($matchpoint eq 'cardnumber') {
-            my $member = GetMember( $borrower{'cardnumber'}, 'cardnumber' );
+            $member = GetMember( $borrower{'cardnumber'}, 'cardnumber' );
             if ($member) {
                 $borrowernumber = $member->{'borrowernumber'};
             }
@@ -190,6 +196,13 @@ if ( $uploadborrowers && length($uploadborrowers) > 0 ) {
                 next LINE;
             }
             $borrower{'borrowernumber'} = $borrowernumber;
+	    for my $col ( keys %borrower) {
+            # use values from extant patron unless our csv file includes this column or we provided a default.
+            # FIXME : You cannot update a field with a  perl-evaluated false value using the defaults.
+            unless(exists($csvkeycol{$col}) || $defaults{$col}) {
+                $borrower{$col} = $member->{$col} if($member->{$col}) ;
+            }
+        }
             unless (ModMember(%borrower)) {
                 $invalid++;
                 $template->param('lastinvalid'=>$borrower{'surname'}.' / '.$borrowernumber);
-- 
2.39.5