From 592de330b5ba3a3e5aa9fc85db95f9a01d3f8a64 Mon Sep 17 00:00:00 2001
From: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Date: Mon, 8 Jan 2018 18:04:56 -0300
Subject: [PATCH] Bug 19936: Replace Check_userid - Update the occurrences

We previously prove that the method and the subroutine were equivalent,
we know update the controller calls.

Test plan:
- Add and update a patron with different variations of userid
(automatically generated or not)
- Import patrons with and without userid, as well as with existing
userid
---
 C4/Members.pm             | 12 +++++++-----
 members/memberentry.pl    |  4 +++-
 tools/import_borrowers.pl | 12 ++++++------
 3 files changed, 16 insertions(+), 12 deletions(-)

diff --git a/C4/Members.pm b/C4/Members.pm
index faa497f98c..cd4868994e 100644
--- a/C4/Members.pm
+++ b/C4/Members.pm
@@ -399,9 +399,10 @@ sub AddMember {
         }
     }
 
+    my $p = Koha::Patron->new( { userid => $data{userid} } );
     # generate a proper login if none provided
     $data{'userid'} = Generate_Userid( $data{'borrowernumber'}, $data{'firstname'}, $data{'surname'} )
-      if ( $data{'userid'} eq '' || !Check_Userid( $data{'userid'} ) );
+      if ( $data{'userid'} eq '' || ! $p->has_valid_userid );
 
     # add expiration date if it isn't already there
     $data{dateexpiry} ||= $category->get_expiry_date;
@@ -507,7 +508,7 @@ sub Check_Userid {
     $borrowernumber is optional (i.e. it can contain a blank value). A value is passed when generating a new userid for an existing borrower. When a new userid is created for a new borrower, a blank value is passed to this sub.
 
     return :
-        new userid ($firstname.$surname if there is a $firstname, or $surname if there is no value in $firstname) plus offset (0 if the $newuid is unique, or a higher numeric value if Check_Userid finds an existing match for the $newuid in the database).
+        new userid ($firstname.$surname if there is a $firstname, or $surname if there is no value in $firstname) plus offset (0 if the $newuid is unique, or a higher numeric value if not unique).
 
 =cut
 
@@ -515,16 +516,17 @@ sub Generate_Userid {
   my ($borrowernumber, $firstname, $surname) = @_;
   my $newuid;
   my $offset = 0;
-  #The script will "do" the following code and increment the $offset until Check_Userid = 1 (i.e. until $newuid comes back as unique)
+  my $patron = Koha::Patron->new;
+  #The script will "do" the following code and increment the $offset until the generated userid is unique
   do {
     $firstname =~ s/[[:digit:][:space:][:blank:][:punct:][:cntrl:]]//g;
     $surname =~ s/[[:digit:][:space:][:blank:][:punct:][:cntrl:]]//g;
     $newuid = lc(($firstname)? "$firstname.$surname" : $surname);
     $newuid = unac_string('utf-8',$newuid);
     $newuid .= $offset unless $offset == 0;
+    $patron->userid( $newuid );
     $offset++;
-
-   } while (!Check_Userid($newuid,$borrowernumber));
+   } while (! $patron->has_valid_userid );
 
    return $newuid;
 }
diff --git a/members/memberentry.pl b/members/memberentry.pl
index 13793e71cf..cc4a14cd9d 100755
--- a/members/memberentry.pl
+++ b/members/memberentry.pl
@@ -351,7 +351,9 @@ if ($op eq 'save' || $op eq 'insert'){
   # the edited values list when editing certain sub-forms. Get it straight
   # from the DB if absent.
   my $userid = $newdata{ userid } // $borrower_data->{ userid };
-  unless (Check_Userid($userid,$borrowernumber)) {
+  my $p = $borrowernumber ? Koha::Patrons->find( $borrowernumber ) : Koha::Patron->new;
+  $p->userid( $userid );
+  unless ( $p->has_valid_userid ) {
     push @errors, "ERROR_login_exist";
   }
 
diff --git a/tools/import_borrowers.pl b/tools/import_borrowers.pl
index d0aa3ebe67..9fb1686280 100755
--- a/tools/import_borrowers.pl
+++ b/tools/import_borrowers.pl
@@ -234,11 +234,11 @@ if ( $uploadborrowers && length($uploadborrowers) > 0 ) {
         $borrower{dateenrolled} ||= $today;
         $borrower{dateexpiry}   ||= Koha::Patron::Categories->find( $borrower{categorycode} )->get_expiry_date( $borrower{dateenrolled} );
         my $borrowernumber;
-        my $member;
+        my ( $member, $patron );
         if ( ($matchpoint eq 'cardnumber') && ($borrower{'cardnumber'}) ) {
-            $member = Koha::Patrons->find( { cardnumber => $borrower{'cardnumber'} } );
+            $patron = Koha::Patrons->find( { cardnumber => $borrower{'cardnumber'} } );
         } elsif ( ($matchpoint eq 'userid') && ($borrower{'userid'}) ) {
-            $member = Koha::Patrons->find( { userid => $borrower{'userid'} } );
+            $patron = Koha::Patrons->find( { userid => $borrower{'userid'} } );
         } elsif ($extended) {
             if (defined($matchpoint_attr_type)) {
                 foreach my $attr (@$patron_attributes) {
@@ -251,8 +251,8 @@ if ( $uploadborrowers && length($uploadborrowers) > 0 ) {
             }
         }
 
-        if ($member) {
-            $member = $member->unblessed;
+        if ($patron) {
+            $member = $patron->unblessed;
             $borrowernumber = $member->{'borrowernumber'};
         } else {
             $member = {};
@@ -291,7 +291,7 @@ if ( $uploadborrowers && length($uploadborrowers) > 0 ) {
             # Check if the userid provided does not exist yet
             if (  exists $borrower{userid}
                      and $borrower{userid}
-                 and not Check_Userid( $borrower{userid}, $borrower{borrowernumber} ) ) {
+                 and not $patron->userid($borrower{userid})->has_valid_userid ) {
                 push @errors, { duplicate_userid => 1, userid => $borrower{userid} };
                 $invalid++;
                 next LINE;
-- 
2.11.0