From b53075b58df01e65371e13dee0b6848d12a181f2 Mon Sep 17 00:00:00 2001
From: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Date: Tue, 3 May 2016 08:58:33 +0100
Subject: [PATCH] Bug 16426: follow-up of bug 15840 - correctly manage userid
 while inserting patrons

Bug 15840 tried to fix a bug but makes things more complicated than it
was before.
If an userid is not provided for 1 or more rows of the csv file, it
should not be updated. However, if a userid is provided and it already
used by an other patron, the import should fail for this row (but not
crash!).

Test plan:
0/ Create a patron with a userid=your_userid
1/ Use the import patron tool to update this userid
=> userid should have been updated
2/ Update another data and do not provide the userid
=> data should have been updated and not the userid
3/ Update another data and provide the userid, but set it to an empty
string, or '0'
=> data should have been updated and not the userid
4/ Update another patron, and set userid=your_userid
=> Update should fail and an error whouls be displayed ("already used by
another patron")

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com>
(cherry picked from commit 7b76b24fad305b0253eb1d779f074d265087ca73)
Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
---
 C4/Members.pm                                  |  3 +++
 .../prog/en/modules/tools/import_borrowers.tt  |  2 +-
 tools/import_borrowers.pl                      | 18 ++++++++++--------
 3 files changed, 14 insertions(+), 9 deletions(-)

diff --git a/C4/Members.pm b/C4/Members.pm
index 3114b0f7a4..ca1b02fb0e 100644
--- a/C4/Members.pm
+++ b/C4/Members.pm
@@ -662,6 +662,9 @@ sub ModMember {
     my $rs = $schema->resultset('Borrower')->search({
         borrowernumber => $new_borrower->{borrowernumber},
      });
+
+    delete $new_borrower->{userid} if exists $new_borrower->{userid} and not $new_borrower->{userid};
+
     my $execute_success = $rs->update($new_borrower);
     if ($execute_success ne '0E0') { # only proceed if the update was a success
         # ok if its an adult (type) it may have borrowers that depend on it as a guarantor
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 f31d7dab84..d9f25db0c1 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
@@ -92,7 +92,7 @@
         [% END %]
         [% IF ERROR.duplicate_userid %]
             <li class="line_error">
-                Userid [% ERROR.userid %] is already used by another patron or is empty.
+                Userid [% ERROR.userid %] is already used by another patron.
             </li>
         [% END %]
     [% END %]
diff --git a/tools/import_borrowers.pl b/tools/import_borrowers.pl
index d5cf3b272b..190a5f2bca 100755
--- a/tools/import_borrowers.pl
+++ b/tools/import_borrowers.pl
@@ -253,14 +253,6 @@ if ( $uploadborrowers && length($uploadborrowers) > 0 ) {
             next;
         }
 
-        # generate a proper login if none provided
-        if ( $borrower{userid} eq '' || !Check_Userid( $borrower{userid} ) ) {
-            push @errors, { duplicate_userid => 1, userid => $borrower{userid} };
-            $invalid++;
-            next LINE;
-        }
-
-
         if ($borrowernumber) {
             # borrower exists
             unless ($overwrite_cardnumber) {
@@ -280,6 +272,16 @@ if ( $uploadborrowers && length($uploadborrowers) > 0 ) {
                     $borrower{$col} = $member->{$col} if($member->{$col}) ;
                 }
             }
+
+            # Check if the userid provided does not exist yet
+            if (  exists $borrower{userid}
+                     and $borrower{userid}
+                 and not Check_Userid( $borrower{userid}, $borrower{borrowernumber} ) ) {
+                push @errors, { duplicate_userid => 1, userid => $borrower{userid} };
+                $invalid++;
+                next LINE;
+            }
+
             unless (ModMember(%borrower)) {
                 $invalid++;
                 # until we have better error trapping, we have no way of knowing why ModMember errored out...
-- 
2.39.5