Attachment 35850 Details for Bug 12598 – Bug 12598 - Move importing code to a subroutine

[patch] Bug 12598 - Move importing code to a subroutine

Bug-12598---Move-importing-code-to-a-subroutine.patch (text/plain), 51.17 KB, created by Kyle M Hall (khall) on 2015-02-11 16:09:43 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Kyle M Hall (khall)

Created: 2015-02-11 16:09:43 UTC

Size: 51.17 KB

patch

obsolete

>From 97552ce177bcd18d1e11386738e1df1d3dfabbfa Mon Sep 17 00:00:00 2001
>From: Kyle M Hall <kyle@bywatersolutions.com>
>Date: Tue, 27 Jan 2015 10:14:02 -0500
>Subject: [PATCH] Bug 12598 - Move importing code to a subroutine
>
>---
> Koha/Patrons/Import.pm                             |  370 ++++++++++++++++++
> .../prog/en/modules/tools/import_borrowers.tt      |  405 +++++++++++---------
> tools/import_borrowers.pl                          |  346 ++---------------
> 3 files changed, 643 insertions(+), 478 deletions(-)
> create mode 100644 Koha/Patrons/Import.pm
>
>diff --git a/Koha/Patrons/Import.pm b/Koha/Patrons/Import.pm
>new file mode 100644
>index 0000000..1041262
>--- /dev/null
>+++ b/Koha/Patrons/Import.pm
>@@ -0,0 +1,370 @@
>+package Koha::Patrons::Import;
>+
>+# This file is part of Koha.
>+#
>+# Koha is free software; you can redistribute it and/or modify it under the
>+# terms of the GNU General Public License as published by the Free Software
>+# Foundation; either version 3 of the License, or (at your option) any later
>+# version.
>+#
>+# Koha is distributed in the hope that it will be useful, but WITHOUT ANY
>+# WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR
>+# A PARTICULAR PURPOSE.  See the GNU General Public License for more details.
>+#
>+# You should have received a copy of the GNU General Public License along
>+# with Koha; if not, write to the Free Software Foundation, Inc.,
>+# 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
>+
>+use Modern::Perl;
>+
>+use Carp;
>+use Text::CSV;
>+
>+use C4::Members;
>+use C4::Branch;
>+
>+sub import_patrons {
>+    my ($params) = @_;
>+
>+    my $handle               = $params->{file};
>+    my $matchpoint           = $params->{matchpoint};
>+    my $defaults             = $params->{defaults};
>+    my $ext_preserve         = $params->{preserve_extended_attributes};
>+    my $overwrite_cardnumber = $params->{overwrite_cardnumber};
>+
>+    carp("No file handle passed in!") && return unless $handle;
>+
>+    my $extended            = C4::Context->preference('ExtendedPatronAttributes');
>+    my $set_messaging_prefs = C4::Context->preference('EnhancedMessagingPreferences');
>+
>+    my @columnkeys = map { $_ ne 'borrowernumber' ? $_ : () } C4::Members::columns();
>+    push( @columnkeys, 'patron_attributes' ) if $extended;
>+
>+    our $csv = Text::CSV->new( { binary => 1 } );    # binary needed for non-ASCII Unicode
>+
>+    my @feedback;
>+    my @errors;
>+
>+    my $imported    = 0;
>+    my $alreadyindb = 0;
>+    my $overwritten = 0;
>+    my $invalid     = 0;
>+    my $matchpoint_attr_type;
>+
>+    # use header line to construct key to column map
>+    my $borrowerline = <$handle>;
>+    my $status       = $csv->parse($borrowerline);
>+    ($status) or push @errors, { badheader => 1, line => $., lineraw => $borrowerline };
>+    my @csvcolumns = $csv->fields();
>+    my %csvkeycol;
>+    my $col = 0;
>+    foreach my $keycol (@csvcolumns) {
>+
>+        # columnkeys don't contain whitespace, but some stupid tools add it
>+        $keycol =~ s/ +//g;
>+        $csvkeycol{$keycol} = $col++;
>+    }
>+
>+    #warn($borrowerline);
>+    if ($extended) {
>+        $matchpoint_attr_type = C4::Members::AttributeTypes->fetch($matchpoint);
>+    }
>+
>+    push @feedback, { feedback => 1, name => 'headerrow', value => join( ', ', @csvcolumns ) };
>+    my $today_iso = C4::Dates->new()->output('iso');
>+    my @criticals = qw(surname branchcode categorycode);    # there probably should be others
>+    my @bad_dates;                                          # I've had a few.
>+    my $date_re = C4::Dates->new->regexp('syspref');
>+    my $iso_re  = C4::Dates->new->regexp('iso');
>+  LINE: while ( my $borrowerline = <$handle> ) {
>+        my %borrower;
>+        my @missing_criticals;
>+        my $patron_attributes;
>+        my $status  = $csv->parse($borrowerline);
>+        my @columns = $csv->fields();
>+        if ( !$status ) {
>+            push @missing_criticals, { badparse => 1, line => $., lineraw => $borrowerline };
>+        }
>+        elsif ( @columns == @columnkeys ) {
>+            @borrower{@columnkeys} = @columns;
>+
>+            # MJR: try to fill blanks gracefully by using default values
>+            foreach my $key (@columnkeys) {
>+                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} = '';
>+                }
>+            }
>+        }
>+
>+        #warn join(':',%borrower);
>+        if ( $borrower{categorycode} ) {
>+            push @missing_criticals,
>+              {
>+                key          => 'categorycode',
>+                line         => $.,
>+                lineraw      => $borrowerline,
>+                value        => $borrower{categorycode},
>+                category_map => 1
>+              }
>+              unless GetBorrowercategory( $borrower{categorycode} );
>+        }
>+        else {
>+            push @missing_criticals, { key => 'categorycode', line => $., lineraw => $borrowerline };
>+        }
>+        if ( $borrower{branchcode} ) {
>+            push @missing_criticals,
>+              {
>+                key        => 'branchcode',
>+                line       => $.,
>+                lineraw    => $borrowerline,
>+                value      => $borrower{branchcode},
>+                branch_map => 1
>+              }
>+              unless GetBranchName( $borrower{branchcode} );
>+        }
>+        else {
>+            push @missing_criticals, { key => 'branchcode', line => $., lineraw => $borrowerline };
>+        }
>+        if (@missing_criticals) {
>+            foreach (@missing_criticals) {
>+                $_->{borrowernumber} = $borrower{borrowernumber} || 'UNDEF';
>+                $_->{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.
>+            next LINE;
>+        }
>+        if ($extended) {
>+            my $attr_str = $borrower{patron_attributes};
>+            $attr_str =~ s/\xe2\x80\x9c/"/g;    # fixup double quotes in case we are passed smart quotes
>+            $attr_str =~ s/\xe2\x80\x9d/"/g;
>+            push @feedback, { feedback => 1, name => 'attribute string', value => $attr_str };
>+            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);
>+        }
>+
>+        # 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/ ) {
>+                $borrower{$_} = format_date_in_iso($tempdate);
>+            }
>+            elsif ( $tempdate =~ /$iso_re/ ) {
>+                $borrower{$_} = $tempdate;
>+            }
>+            else {
>+                $borrower{$_} = '';
>+                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};
>+        my $borrowernumber;
>+        my $member;
>+        if ( ( $matchpoint eq 'cardnumber' ) && ( $borrower{'cardnumber'} ) ) {
>+            $member = GetMember( 'cardnumber' => $borrower{'cardnumber'} );
>+            if ($member) {
>+                $borrowernumber = $member->{'borrowernumber'};
>+            }
>+        }
>+        elsif ($extended) {
>+            if ( defined($matchpoint_attr_type) ) {
>+                foreach my $attr (@$patron_attributes) {
>+                    if ( $attr->{code} eq $matchpoint and $attr->{value} ne '' ) {
>+                        my @borrowernumbers = $matchpoint_attr_type->get_patrons( $attr->{value} );
>+                        $borrowernumber = $borrowernumbers[0] if scalar(@borrowernumbers) == 1;
>+                        last;
>+                    }
>+                }
>+            }
>+        }
>+
>+        if ( C4::Members::checkcardnumber( $borrower{cardnumber}, $borrowernumber ) ) {
>+            push @errors,
>+              {
>+                invalid_cardnumber => 1,
>+                borrowernumber     => $borrowernumber,
>+                cardnumber         => $borrower{cardnumber}
>+              };
>+            $invalid++;
>+            next;
>+        }
>+
>+        if ($borrowernumber) {
>+
>+            # borrower exists
>+            unless ($overwrite_cardnumber) {
>+                $alreadyindb++;
>+                push(
>+                    @feedback,
>+                    {
>+                        already_in_db => 1,
>+                        value         => $borrower{'surname'} . ' / ' . $borrowernumber
>+                    }
>+                );
>+                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.
>+
>+                # The password is always encrypted, skip it!
>+                next if $col eq 'password';
>+
>+                unless ( exists( $csvkeycol{$col} ) || $defaults->{$col} ) {
>+                    $borrower{$col} = $member->{$col} if ( $member->{$col} );
>+                }
>+            }
>+            unless ( ModMember(%borrower) ) {
>+                $invalid++;
>+
>+                push(
>+                    @errors,
>+                    {
>+                        name  => 'lastinvalid',
>+                        value => $borrower{'surname'} . ' / ' . $borrowernumber
>+                    }
>+                );
>+                next LINE;
>+            }
>+            if ( $borrower{debarred} ) {
>+
>+                # Check to see if this debarment already exists
>+                my $debarrments = GetDebarments(
>+                    {
>+                        borrowernumber => $borrowernumber,
>+                        expiration     => $borrower{debarred},
>+                        comment        => $borrower{debarredcomment}
>+                    }
>+                );
>+
>+                # If it doesn't, then add it!
>+                unless (@$debarrments) {
>+                    AddDebarment(
>+                        {
>+                            borrowernumber => $borrowernumber,
>+                            expiration     => $borrower{debarred},
>+                            comment        => $borrower{debarredcomment}
>+                        }
>+                    );
>+                }
>+            }
>+            if ($extended) {
>+                if ($ext_preserve) {
>+                    my $old_attributes = GetBorrowerAttributes($borrowernumber);
>+                    $patron_attributes = extended_attributes_merge( $old_attributes, $patron_attributes );
>+                }
>+                push @errors, { unknown_error => 1 }
>+                  unless SetBorrowerAttributes( $borrower{'borrowernumber'}, $patron_attributes );
>+            }
>+            $overwritten++;
>+            push(
>+                @feedback,
>+                {
>+                    feedback => 1,
>+                    name     => 'lastoverwritten',
>+                    value    => $borrower{'surname'} . ' / ' . $borrowernumber
>+                }
>+            );
>+        }
>+        else {
>+            # FIXME: fixup_cardnumber says to lock table, but the web interface doesn't so this doesn't either.
>+            # At least this is closer to AddMember than in members/memberentry.pl
>+            if ( !$borrower{'cardnumber'} ) {
>+                $borrower{'cardnumber'} = fixup_cardnumber(undef);
>+            }
>+            if ( $borrowernumber = AddMember(%borrower) ) {
>+
>+                if ( $borrower{debarred} ) {
>+                    AddDebarment(
>+                        {
>+                            borrowernumber => $borrowernumber,
>+                            expiration     => $borrower{debarred},
>+                            comment        => $borrower{debarredcomment}
>+                        }
>+                    );
>+                }
>+
>+                if ($extended) {
>+                    SetBorrowerAttributes( $borrowernumber, $patron_attributes );
>+                }
>+
>+                if ($set_messaging_prefs) {
>+                    C4::Members::Messaging::SetMessagingPreferencesFromDefaults(
>+                        {
>+                            borrowernumber => $borrowernumber,
>+                            categorycode   => $borrower{categorycode}
>+                        }
>+                    );
>+                }
>+
>+                $imported++;
>+                push(
>+                    @feedback,
>+                    {
>+                        feedback => 1,
>+                        name     => 'lastimported',
>+                        value    => $borrower{'surname'} . ' / ' . $borrowernumber
>+                    }
>+                );
>+            }
>+            else {
>+                $invalid++;
>+                push @errors, { unknown_error => 1 };
>+                push(
>+                    @errors,
>+                    {
>+                        name  => 'lastinvalid',
>+                        value => $borrower{'surname'} . ' / AddMember',
>+                    }
>+                );
>+            }
>+        }
>+    }
>+
>+    return {
>+        feedback      => \@feedback,
>+        errors        => \@errors,
>+        imported      => $imported,
>+        overwritten   => $overwritten,
>+        already_in_db => $alreadyindb,
>+        invalid       => $invalid,
>+    };
>+}
>+
>+END { }    # module clean-up code here (global destructor)
>+
>+1;
>+
>+__END__
>+
>+=head1 AUTHOR
>+
>+Koha Team
>+
>+=cut
>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 f8d9303..7a4c8af 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
>@@ -33,188 +33,249 @@
>      <div class="yui-u first">
> <h1>Import patrons</h1>
> [% 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>
>-</ul>
>-  [% IF ( FEEDBACK ) %]
>-  <br /><br />
>-    <div>
>-    <h5>Feedback:</h5>
>-    <ul class="feedback">
>-    [% FOREACH FEEDBAC IN FEEDBACK %]
>-    <li>
>-        [% IF ( FEEDBAC.filename ) %]Parsing upload file <span class="filename">[% FEEDBAC.filename %]</span>
>-        [% ELSIF ( FEEDBAC.backend ) %]Upload parsed using [% FEEDBAC.backend %]
>-        [% ELSIF ( FEEDBAC.headerrow ) %]These fields found: [% FEEDBAC.value %]
>-        [% ELSE %][% FEEDBAC.name %] : [% FEEDBAC.value %]
>-        [% END %]
>-    </li>
>-    [% END %]
>-    </ul>
>-    </div>
>-  [% END %]
>-  [% IF ( ERRORS ) %]
>-  <br /><br />
>-    <div>
>-    <h5>Error analysis:</h5>
>+    <h5>Import results :</h5>
>     <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
>+        <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 />
>+
>+        <div>
>+            <h5>Feedback:</h5>
>+                <ul class="feedback">
>+                    [% FOREACH f IN feedback %]
>+                        <li>
>+                            [% IF ( f.filename ) %]
>+                                Parsing upload file <span class="filename">[% f.filename %]</span>
>+                            [% ELSIF ( f.backend ) %]
>+                                Upload parsed using [% f.backend %]
>+                            [% ELSIF ( f.headerrow ) %]
>+                                These fields found: [% f.value %]
>+                            [% ELSIF ( f.already_in_db ) %]
>+                                Patron already in database: [% f.value %]
>+                            [% ELSE %]
>+                                [% f.name %] : [% f.value %]
>+                            [% END %]
>+                        </li>
>+                    [% END %]
>+                </ul>
>+        </div>
>+    [% END %]
>+
>+    [% IF ( errors ) %]
>+        <br /><br />
>+
>+        <div>
>+            <h5>Error analysis:</h5>
>+            <ul>
>+                [% FOREACH e IN errors %]
>+                    [% IF ( e.badheader ) %]<li>Header row could not be parsed</li>[% END %]
>+
>+                    [% FOREACH missing_critical IN e.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 %]
>+
>+                    [% IF e.invalid_cardnumber %]
>+                        <li class="line_error">
>+                            Cardnumber [% e.cardnumber %] is not a valid cardnumber
>+                            [% IF e.borrowernumber %] (for patron with borrowernumber [% e.borrowernumber %])[% END %]
>+                        </li>
>+                    [% END %]
>                 [% END %]
>-                (borrowernumber: [% missing_critical.borrowernumber %]; surname: [% missing_critical.surname %]).
>-            [% END %]
>-            <br /><code>[% missing_critical.lineraw %]</code>
>-        </li>
>-        [% END %]
>-        [% IF ERROR.invalid_cardnumber %]
>-            <li class="line_error">
>-                Cardnumber [% ERROR.cardnumber %] is not a valid cardnumber
>-                [% IF ERROR.borrowernumber %] (for patron with borrowernumber [% ERROR.borrowernumber %])[% END %]
>-            </li>
>-        [% END %]
>+            </ul>
>+        </div>
>     [% END %]
>-    </ul>
>-    </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>
>-</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>
>-</ol></fieldset>
>-    <fieldset class="rows">
>-        <legend>Field to use for record matching</legend>
>-        <ol>
>-            <li class="radio">
>-                <select name="matchpoint" id="matchpoint">
>-                    <option value="cardnumber">Cardnumber</option>
>-                    <option value="userid">Username</option>
>-                    [% FOREACH matchpoint IN matchpoints %]
>-                        <option value="[% matchpoint.code %]">[% matchpoint.description %]</option>
>+    <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>
>+    </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>
>+            </ol>
>+        </fieldset>
>+
>+        <fieldset class="rows">
>+            <legend>Field to use for record matching</legend>
>+            <ol>
>+                <li class="radio">
>+                    <select name="matchpoint" id="matchpoint">
>+                        <option value="cardnumber">Cardnumber</option>
>+                        [% FOREACH matchpoint IN matchpoints %]
>+                            <option value="[% matchpoint.code %]">[% matchpoint.description %]</option>
>+                        [% END %]
>+                    </select>
>+                </li>
>+            </ol>
>+        </fieldset>
>+
>+        <fieldset class="rows">
>+            <legend>Default values</legend>
>+
>+            <ol>
>+                [% FOREACH borrower_field IN borrower_fields %]
>+
>+                    [% SWITCH borrower_field.field %]
>+                    [% CASE 'branchcode' %]
>+                        <li>
>+                            <label class="description" for="branchcode">[% borrower_field.description %]: </label>
>+                            <select id="branchcode" name="branchcode">
>+                                <option value="" selected="selected"></option>
>+                                [% FOREACH branch IN branches %]
>+                                    <option value="[% branch.branchcode %]">[% branch.branchname %]</option>
>+                                [% END %]
>+                            </select>
>+                            <span class="field_hint">[% borrower_field.field %]</span>
>+                        </li>
>+                    [% CASE 'categorycode' %]
>+                        <li>
>+                            <label class="description" for="categorycode">[% borrower_field.description %]: </label>
>+                            <select id="categorycode" name="categorycode">
>+                                <option value="" selected="selected"></option>
>+                                [% FOREACH category IN categories %]
>+                                    <option value="[% category.categorycode %]">[% category.description %]</option>
>+                                [% END %]
>+                            </select>
>+                            <span class="field_hint">[% borrower_field.field %]</span>
>+                        </li>
>+                    [% CASE %]
>+                        <li>
>+                            <label class="description" for="[% borrower_field.field %]">[% borrower_field.description %]: </label>
>+                            <input id="[% borrower_field.field %]" name="[% borrower_field.field %]" />
>+                            <span class="field_hint">[% borrower_field.field %]</span>
>+                        </li>
>                     [% END %]
>-                </select>
>-            </li>
>-        </ol>
>-    </fieldset>
>-<fieldset class="rows">
>-<legend>Default values</legend>
>-<ol>
>-[% FOREACH borrower_field IN borrower_fields %]
>-  [% SWITCH borrower_field.field %]
>-  [% CASE 'branchcode' %]
>-    <li>
>-        <label class="description" for="branchcode">[% borrower_field.description %]: </label>
>-        <select id="branchcode" name="branchcode">
>-            <option value="" selected="selected"></option>
>-        [% FOREACH branch IN branches %]
>-            <option value="[% branch.branchcode %]">
>-                [% branch.branchname %]</option>
>-        [% END %]
>-        </select><span class="field_hint">[% borrower_field.field %]</span>
>-    </li>
>-  [% CASE 'categorycode' %]
>-    <li>
>-        <label class="description" for="categorycode">[% borrower_field.description %]: </label>
>-        <select id="categorycode" name="categorycode">
>-            <option value="" selected="selected"></option>
>-        [% FOREACH category IN categories %]
>-            <option value="[% category.categorycode %]">
>-                [% category.description %]</option>
>+
>+                [% END %]
>+
>+                [% IF ( Koha.Preference('ExtendedPatronAttributes') == 1 ) %]
>+                    <li>
>+                        <label class="description" for="patron_attributes">Patron attributes: </label>
>+                        <input id="patron_attributes" name="patron_attributes" />
>+                        <span class="field_hint">patron_attributes</span>
>+                    </li>
>+                [% END %]
>+
>+            </ol>
>+        </fieldset>
>+
>+        <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>
>+                </li>
>+            </ol>
>+        </fieldset>
>+
>+        [% IF ( Koha.Preference('ExtendedPatronAttributes') == 1 ) %]
>+            <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>
>+
>+                    <li class="radio">
>+                        <input type="radio" id="ext_preserve_1" name="ext_preserve" value="1" /><label for="ext_preserve_1">Replace only included patron attributes</label>
>+                    </li>
>+                </ol>
>+            </fieldset>
>         [% END %]
>-        </select><span class="field_hint">[% borrower_field.field %]</span>
>-    </li>
>-  [% CASE %]
>-    <li>
>-        <label class="description" for="[% borrower_field.field %]">[% borrower_field.description %]: </label>
>-        <input id="[% borrower_field.field %]" name="[% borrower_field.field %]" /><span class="field_hint">[% borrower_field.field %]</span>
>-    </li>
>-  [% END %]
>-[% END %]
>-[% IF ( Koha.Preference('ExtendedPatronAttributes') == 1 ) %]
>-    <li>
>-        <label class="description" for="patron_attributes">Patron attributes: </label>
>-        <input id="patron_attributes" name="patron_attributes" />
>-        <span class="field_hint">patron_attributes</span>
>-    </li>
>+
>+        <fieldset class="action"><input type="submit" value="Import" /></fieldset>
>+    </form>
> [% END %]
>-</ol></fieldset>
>-	<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>
>+
>+</div>
>+
>+<div class="yui-u">
>+    <h2>Notes:</h2>
>+    <ul>
>+        <li>The first line in the file must be a header row defining which columns you are supplying in the import file.</li>
>+
>+        <li><b>Download a starter CSV file with all the columns <a href="?sample=1">here</a>.</b>  Values are comma-separated.</li>
>+
>+        <li>
>+            OR choose which fields you want to supply from the following list:
>+            <ul>
>+                <li>
>+                    [% FOREACH columnkey IN borrower_fields %]'[% columnkey.field %]', [% END %]
>+                </li>
>+            </ul>
>         </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>
>+
>+        [% IF ( ExtendedPatronAttributes ) %]
>+            <li>
>+                If loading patron attributes, the 'patron_attributes' field should contain a comma-separated list of attribute types and values. The attribute type code and a colon should precede each value. For example: <b>INSTID:12345,LANG:fr</b> or <b>STARTDATE:January 1 2010,TRACK:Day</b>. If an input record has more than one attribute, the fields should either be entered as an unquoted string (previous examples), or with each field wrapped in separate double quotes and delimited by a comma: <b>&quot;STARTDATE:January 1, 2010&quot;,&quot;TRACK:Day&quot;</b>.  The second syntax would be required if the data might have a comma in it, like a date string.
>+            </li>
>+        [% END %]
>+
>+        <li>
>+            The fields 'branchcode' and 'categorycode' are <b>required</b> and <b>must match</b> valid entries in your database.
>         </li>
>-    </ol>
>-    </fieldset>
>-    [% IF ( Koha.Preference('ExtendedPatronAttributes') == 1 ) %]
>-	<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>
>+            'password' should be stored in plaintext, and will be converted to a Bcrypt hash (if your passwords are already encrypted, talk to your system administrator about options).
>         </li>
>-        <li class="radio">
>-        <input type="radio" id="ext_preserve_1" name="ext_preserve" value="1" /><label for="ext_preserve_1">Replace only included patron attributes</label>
>+
>+        <li>
>+            Date formats should match your system preference, and <b>must</b> be zero-padded, e.g. '01/02/2008'.  Alternatively,
>+you can supply dates in ISO format (e.g., '2010-10-28').
>         </li>
>-    </ol>
>-    </fieldset>
>-    [% END %]
>-	<fieldset class="action"><input type="submit" value="Import" /></fieldset>
>-</form>
>-[% END %]
>+    </ul>
>+
> </div>
>-<div class="yui-u">
>-<h2>Notes:</h2>
>-<ul>
>-<li>The first line in the file must be a header row defining which columns you are supplying in the import file.</li>
>-<li><b>Download a starter CSV file with all the columns <a href="?sample=1">here</a>.</b>  Values are comma-separated.</li>
>-<li>OR choose which fields you want to supply from the following list:<ul><li>
>-    [% FOREACH columnkey IN borrower_fields %]'[% columnkey.field %]', [% END %]
>-</li></ul></li>
>-[% IF ( ExtendedPatronAttributes ) %]
>-<li>If loading patron attributes, the 'patron_attributes' field should contain a comma-separated list of attribute types and values. The attribute type code and a colon should precede each value. For example: <b>INSTID:12345,LANG:fr</b> or <b>STARTDATE:January 1 2010,TRACK:Day</b>. If an input record has more than one attribute, the fields should either be entered as an unquoted string (previous examples), or with each field wrapped in separate double quotes and delimited by a comma: <b>&quot;STARTDATE:January 1, 2010&quot;,&quot;TRACK:Day&quot;</b>.  The second syntax would be required if the data might have a comma in it, like a date string.
>-</li>
>-[% END %]
>-<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 Bcrypt hash (if your passwords are already encrypted, talk to your system administrator about options).</li>
>-<li>Date formats should match your system preference, and <b>must</b> be zero-padded, e.g. '01/02/2008'.  Alternatively,
>-you can supply dates in ISO format (e.g., '2010-10-28').
>-</li>
>-</ul>
>-
>-     </div>
>-    </div>
>-   </div>
>-  </div>
>-  <div class="yui-b noprint">
>-[% INCLUDE 'tools-menu.inc' %]
>-  </div>
>- </div>
>+</div>
>+</div>
>+</div>
>+
>+<div class="yui-b noprint">
>+    [% INCLUDE 'tools-menu.inc' %]
>+</div>
>+</div>
>+
> [% INCLUDE 'intranet-bottom.inc' %]
>diff --git a/tools/import_borrowers.pl b/tools/import_borrowers.pl
>index 2c7c020..183ced1 100755
>--- a/tools/import_borrowers.pl
>+++ b/tools/import_borrowers.pl
>@@ -34,21 +34,16 @@
> # 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;
>-use C4::Dates qw(format_date_in_iso);
>-use C4::Context;
>-use C4::Branch qw/GetBranchesLoop GetBranchName/;
>-use C4::Members;
>-use C4::Members::Attributes qw(:all);
>-use C4::Members::AttributeTypes;
>-use C4::Members::Messaging;
>-use C4::Reports::Guided;
> use C4::Templates;
>-use Koha::Borrower::Debarments;
>+
>+use C4::Branch;
>+use C4::Members;
>+
>+use Koha::Patrons::Import qw(import_patrons);
> 
> use Text::CSV;
> 
>@@ -61,16 +56,12 @@ use CGI qw ( -utf8 );
> # use encoding 'utf8';    # don't do this
> 
> my ( @errors, @feedback );
>-my $extended            = C4::Context->preference('ExtendedPatronAttributes');
>-my $set_messaging_prefs = C4::Context->preference('EnhancedMessagingPreferences');
>-my @columnkeys          = C4::Members::columns();
>-@columnkeys = map { $_ ne 'borrowernumber' ? $_ : () } @columnkeys;
>-if ($extended) {
>-    push @columnkeys, 'patron_attributes';
>-}
>+my $extended = C4::Context->preference('ExtendedPatronAttributes');
>+
>+my @columnkeys = map { $_ ne 'borrowernumber' ? $_ : () } C4::Members::columns();
>+push( @columnkeys, 'patron_attributes' ) if $extended;
> 
> my $input = CGI->new();
>-our $csv = Text::CSV->new( { binary => 1 } );    # binary needed for non-ASCII Unicode
> 
> #push @feedback, {feedback=>1, name=>'backend', value=>$csv->backend, backend=>$csv->backend}; #XXX
> 
>@@ -97,6 +88,7 @@ $columns = [ grep { $_->{field} ne 'borrowernumber' ? $_ : () } @$columns ];
> $template->param( borrower_fields => $columns );
> 
> if ( $input->param('sample') ) {
>+    our $csv = Text::CSV->new( { binary => 1 } );    # binary needed for non-ASCII Unicode
>     print $input->header(
>         -type       => 'application/vnd.sun.xml.calc',    # 'application/vnd.ms-excel' ?
>         -attachment => 'patron_import.csv',
>@@ -105,310 +97,52 @@ if ( $input->param('sample') ) {
>     print $csv->string, "\n";
>     exit 1;
> }
>+
> my $uploadborrowers = $input->param('uploadborrowers');
> my $matchpoint      = $input->param('matchpoint');
> if ($matchpoint) {
>     $matchpoint =~ s/^patron_attribute_//;
> }
>-my $overwrite_cardnumber = $input->param('overwrite_cardnumber');
> 
> $template->param( SCRIPT_NAME => $ENV{'SCRIPT_NAME'} );
> 
> if ( $uploadborrowers && length($uploadborrowers) > 0 ) {
>-    push @feedback, { feedback => 1, name => 'filename', value => $uploadborrowers, filename => $uploadborrowers };
>-    my $handle     = $input->upload('uploadborrowers');
>-    my $uploadinfo = $input->uploadInfo($uploadborrowers);
>-    foreach ( keys %$uploadinfo ) {
>-        push @feedback, { feedback => 1, name => $_, value => $uploadinfo->{$_}, $_ => $uploadinfo->{$_} };
>-    }
>-    my $imported    = 0;
>-    my $alreadyindb = 0;
>-    my $overwritten = 0;
>-    my $invalid     = 0;
>-    my $matchpoint_attr_type;
>+    my $handle   = $input->upload('uploadborrowers');
>     my %defaults = $input->Vars;
> 
>-    # use header line to construct key to column map
>-    my $borrowerline = <$handle>;
>-    my $status       = $csv->parse($borrowerline);
>-    ($status) or push @errors, { badheader => 1, line => $., lineraw => $borrowerline };
>-    my @csvcolumns = $csv->fields();
>-    my %csvkeycol;
>-    my $col = 0;
>-    foreach my $keycol (@csvcolumns) {
>-
>-        # 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);
>-    }
>-
>-    push @feedback, { feedback => 1, name => 'headerrow', value => join( ', ', @csvcolumns ) };
>-    my $today_iso = C4::Dates->new()->output('iso');
>-    my @criticals = qw(surname branchcode categorycode);    # there probably should be others
>-    my @bad_dates;                                          # I've had a few.
>-    my $date_re = C4::Dates->new->regexp('syspref');
>-    my $iso_re  = C4::Dates->new->regexp('iso');
>-  LINE: while ( my $borrowerline = <$handle> ) {
>-        my %borrower;
>-        my @missing_criticals;
>-        my $patron_attributes;
>-        my $status  = $csv->parse($borrowerline);
>-        my @columns = $csv->fields();
>-        if ( !$status ) {
>-            push @missing_criticals, { badparse => 1, line => $., lineraw => $borrowerline };
>+    my $return = Koha::Patrons::Import::import_patrons(
>+        {
>+            file                         => $handle,
>+            defaults                     => \%defaults,
>+            matchpoint                   => $matchpoint,
>+            overwrite_cardnumber         => $input->param('overwrite_cardnumber'),
>+            preserve_extended_attributes => $input->param('ext_preserve') || 0,
>         }
>-        elsif ( @columns == @columnkeys ) {
>-            @borrower{@columnkeys} = @columns;
>-
>-            # MJR: try to fill blanks gracefully by using default values
>-            foreach my $key (@columnkeys) {
>-                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} = '';
>-                }
>-            }
>-        }
>-
>-        #warn join(':',%borrower);
>-        if ( $borrower{categorycode} ) {
>-            push @missing_criticals,
>-              {
>-                key          => 'categorycode',
>-                line         => $.,
>-                lineraw      => $borrowerline,
>-                value        => $borrower{categorycode},
>-                category_map => 1
>-              }
>-              unless GetBorrowercategory( $borrower{categorycode} );
>-        }
>-        else {
>-            push @missing_criticals, { key => 'categorycode', line => $., lineraw => $borrowerline };
>-        }
>-        if ( $borrower{branchcode} ) {
>-            push @missing_criticals,
>-              {
>-                key        => 'branchcode',
>-                line       => $.,
>-                lineraw    => $borrowerline,
>-                value      => $borrower{branchcode},
>-                branch_map => 1
>-              }
>-              unless GetBranchName( $borrower{branchcode} );
>-        }
>-        else {
>-            push @missing_criticals, { key => 'branchcode', line => $., lineraw => $borrowerline };
>-        }
>-        if (@missing_criticals) {
>-            foreach (@missing_criticals) {
>-                $_->{borrowernumber} = $borrower{borrowernumber} || 'UNDEF';
>-                $_->{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.
>-            next LINE;
>-        }
>-        if ($extended) {
>-            my $attr_str = $borrower{patron_attributes};
>-            $attr_str =~ s/\xe2\x80\x9c/"/g;    # fixup double quotes in case we are passed smart quotes
>-            $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);
>-        }
>-
>-        # 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/ ) {
>-                $borrower{$_} = format_date_in_iso($tempdate);
>-            }
>-            elsif ( $tempdate =~ /$iso_re/ ) {
>-                $borrower{$_} = $tempdate;
>-            }
>-            else {
>-                $borrower{$_} = '';
>-                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};
>-        my $borrowernumber;
>-        my $member;
>-        if ( ( $matchpoint eq 'cardnumber' ) && ( $borrower{'cardnumber'} ) ) {
>-            $member = GetMember( 'cardnumber' => $borrower{'cardnumber'} );
>-            if ($member) {
>-                $borrowernumber = $member->{'borrowernumber'};
>-            }
>-        } elsif ( ($matchpoint eq 'userid') && ($borrower{'userid'}) ) {
>-            $member = GetMember( 'userid' => $borrower{'userid'} );
>-            if ($member) {
>-                $borrowernumber = $member->{'borrowernumber'};
>-            }
>-        } elsif ($extended) {
>-            if (defined($matchpoint_attr_type)) {
>-                foreach my $attr (@$patron_attributes) {
>-                    if ( $attr->{code} eq $matchpoint and $attr->{value} ne '' ) {
>-                        my @borrowernumbers = $matchpoint_attr_type->get_patrons( $attr->{value} );
>-                        $borrowernumber = $borrowernumbers[0] if scalar(@borrowernumbers) == 1;
>-                        last;
>-                    }
>-                }
>-            }
>-        }
>-
>-        if ( C4::Members::checkcardnumber( $borrower{cardnumber}, $borrowernumber ) ) {
>-            push @errors,
>-              {
>-                invalid_cardnumber => 1,
>-                borrowernumber     => $borrowernumber,
>-                cardnumber         => $borrower{cardnumber}
>-              };
>-            $invalid++;
>-            next;
>-        }
>-
>-        if ($borrowernumber) {
>-
>-            # borrower exists
>-            unless ($overwrite_cardnumber) {
>-                $alreadyindb++;
>-                $template->param( 'lastalreadyindb' => $borrower{'surname'} . ' / ' . $borrowernumber );
>-                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.
>-
>-                # The password is always encrypted, skip it!
>-                next if $col eq 'password';
>-
>-                unless ( exists( $csvkeycol{$col} ) || $defaults{$col} ) {
>-                    $borrower{$col} = $member->{$col} if ( $member->{$col} );
>-                }
>-            }
>-            unless ( ModMember(%borrower) ) {
>-                $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 );
>-                next LINE;
>-            }
>-            if ( $borrower{debarred} ) {
>-
>-                # Check to see if this debarment already exists
>-                my $debarrments = GetDebarments(
>-                    {
>-                        borrowernumber => $borrowernumber,
>-                        expiration     => $borrower{debarred},
>-                        comment        => $borrower{debarredcomment}
>-                    }
>-                );
>-
>-                # If it doesn't, then add it!
>-                unless (@$debarrments) {
>-                    AddDebarment(
>-                        {
>-                            borrowernumber => $borrowernumber,
>-                            expiration     => $borrower{debarred},
>-                            comment        => $borrower{debarredcomment}
>-                        }
>-                    );
>-                }
>-            }
>-            if ($extended) {
>-                if ($ext_preserve) {
>-                    my $old_attributes = GetBorrowerAttributes($borrowernumber);
>-                    $patron_attributes = extended_attributes_merge( $old_attributes, $patron_attributes )
>-                      ;    #TODO: expose repeatable options in template
>-                }
>-                push @errors, { unknown_error => 1 }
>-                  unless SetBorrowerAttributes( $borrower{'borrowernumber'}, $patron_attributes );
>-            }
>-            $overwritten++;
>-            $template->param( 'lastoverwritten' => $borrower{'surname'} . ' / ' . $borrowernumber );
>-        }
>-        else {
>-            # FIXME: fixup_cardnumber says to lock table, but the web interface doesn't so this doesn't either.
>-            # At least this is closer to AddMember than in members/memberentry.pl
>-            if ( !$borrower{'cardnumber'} ) {
>-                $borrower{'cardnumber'} = fixup_cardnumber(undef);
>-            }
>-            if ( $borrowernumber = AddMember(%borrower) ) {
>+    );
> 
>-                if ( $borrower{debarred} ) {
>-                    AddDebarment(
>-                        {
>-                            borrowernumber => $borrowernumber,
>-                            expiration     => $borrower{debarred},
>-                            comment        => $borrower{debarredcomment}
>-                        }
>-                    );
>-                }
>+    my $feedback    = $return->{feedback};
>+    my $errors      = $return->{errors};
>+    my $imported    = $return->{imported};
>+    my $overwritten = $return->{overwritten};
>+    my $alreadyindb = $return->{already_in_db};
>+    my $invalid     = $return->{invalid};
> 
>-                if ($extended) {
>-                    SetBorrowerAttributes( $borrowernumber, $patron_attributes );
>-                }
>+    my $uploadinfo = $input->uploadInfo($uploadborrowers);
>+    foreach ( keys %$uploadinfo ) {
>+        push @$feedback, { feedback => 1, name => $_, value => $uploadinfo->{$_}, $_ => $uploadinfo->{$_} };
>+    }
> 
>-                if ($set_messaging_prefs) {
>-                    C4::Members::Messaging::SetMessagingPreferencesFromDefaults(
>-                        {
>-                            borrowernumber => $borrowernumber,
>-                            categorycode   => $borrower{categorycode}
>-                        }
>-                    );
>-                }
>+    push @$feedback, { feedback => 1, name => 'filename', value => $uploadborrowers, filename => $uploadborrowers };
> 
>-                $imported++;
>-                $template->param( 'lastimported' => $borrower{'surname'} . ' / ' . $borrowernumber );
>-            }
>-            else {
>-                $invalid++;
>-                push @errors, { unknown_error => 1 };
>-                $template->param( 'lastinvalid' => $borrower{'surname'} . ' / AddMember' );
>-            }
>-        }
>-    }
>-    (@errors)   and $template->param( ERRORS   => \@errors );
>-    (@feedback) and $template->param( FEEDBACK => \@feedback );
>     $template->param(
>-        'uploadborrowers' => 1,
>-        'imported'        => $imported,
>-        'overwritten'     => $overwritten,
>-        'alreadyindb'     => $alreadyindb,
>-        'invalid'         => $invalid,
>-        'total'           => $imported + $alreadyindb + $invalid + $overwritten,
>+        uploadborrowers => 1,
>+        errors          => $errors,
>+        feedback        => $feedback,
>+        imported        => $imported,
>+        overwritten     => $overwritten,
>+        alreadyindb     => $alreadyindb,
>+        invalid         => $invalid,
>+        total           => $imported + $alreadyindb + $invalid + $overwritten,
>     );
> 
> }
>-- 
>1.7.2.5

Actions: View | Diff | Splinter Review

Attachments on bug 12598: 29807 | 29808 | 29809 | 29939 | 30755 | 30780 | 35557 | 35558 | 35559 | 35560 | 35848 | 35849 | 35850 | 35851 | 35964 | 36684 | 36685 | 36687 | 36688 | 36690 | 36692 | 36693 | 36694 | 36695 | 36696 | 39124 | 39125 | 39126 | 39127 | 39128 | 41790 | 41791 | 41792 | 41793 | 41794 | 44065 | 44066 | 44067 | 44068 | 44069 | 48753 | 48754 | 48755 | 48756 | 48757 | 50693 | 50694 | 50695 | 50696 | 50697 | 50698 | 50699 | 50700 | 50704 | 51122 | 51357 | 51358 | 51359 | 51360 | 51361 | 51362 | 51363 | 51364 | 51365 | 51366 | 51367 | 51368 | 51369 | 51370 | 51371 | 51372 | 52323 | 52324 | 52325 | 52326 | 52327 | 52328 | 52329 | 52330 | 52331 | 52332 | 52333 | 52540 | 64437 | 64438 | 64855 | 65214 | 65215 | 65216 | 65217 | 65218 | 65819 | 65820 | 65821 | 65822 | 65823 | 65824 | 66353 | 66354 | 66355 | 66356 | 66357 | 66358 | 66359 | 66575 | 66576 | 66577 | 66578 | 66579 | 66580 | 66581 | 66582 | 67276 | 67277 | 70265 | 70266 | 71755 | 71756 | 71757 | 71758 | 71759 | 71760 | 71761 | 71762 | 71763 | 71764 | 71765 | 71766