From 8d8bb843514abe857322e2055c15caa6c2bc79ee 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

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
---
 C4/Members.pm          | 12 +++++++-----
 Koha/Patrons/Import.pm | 12 ++++++------
 members/memberentry.pl |  4 +++-
 3 files changed, 16 insertions(+), 12 deletions(-)

diff --git a/C4/Members.pm b/C4/Members.pm
index da5ed59..ee3904e 100644
--- a/C4/Members.pm
+++ b/C4/Members.pm
@@ -415,9 +415,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;
@@ -523,7 +524,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
 
@@ -531,16 +532,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/Koha/Patrons/Import.pm b/Koha/Patrons/Import.pm
index db77a3b..a8d17c1 100644
--- a/Koha/Patrons/Import.pm
+++ b/Koha/Patrons/Import.pm
@@ -163,12 +163,12 @@ sub import_patrons {
         $borrower{dateexpiry} = Koha::Patron::Categories->find( $borrower{categorycode} )->get_expiry_date( $borrower{dateenrolled} ) unless $borrower{dateexpiry};
 
         my $borrowernumber;
-        my $member;
+        my ( $member, $patron );
         if ( defined($matchpoint) && ( $matchpoint eq 'cardnumber' ) && ( $borrower{'cardnumber'} ) ) {
-            $member = Koha::Patrons->find( { cardnumber => $borrower{'cardnumber'} } );
+            $patron = Koha::Patrons->find( { cardnumber => $borrower{'cardnumber'} } );
         }
         elsif ( defined($matchpoint) && ($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) ) {
@@ -182,8 +182,8 @@ sub import_patrons {
             }
         }
 
-        if ($member) {
-            $member = $member->unblessed;
+        if ($patron) {
+            $member = $patron->unblessed;
             $borrowernumber = $member->{'borrowernumber'};
         } else {
             $member = {};
@@ -203,7 +203,7 @@ sub import_patrons {
         # Check if the userid provided does not exist yet
         if ( defined($matchpoint) and $matchpoint ne 'userid' and exists $borrower{userid}
                  and $borrower{userid}
-             and not Check_Userid( $borrower{userid}, $borrower{borrowernumber} ) ) {
+             and ( $patron and not $patron->userid($borrower{userid})->has_valid_userid ) ) {
              push @errors, { duplicate_userid => 1, userid => $borrower{userid} };
              $invalid++;
              next LINE;
diff --git a/members/memberentry.pl b/members/memberentry.pl
index 631759e..2d002c3 100755
--- a/members/memberentry.pl
+++ b/members/memberentry.pl
@@ -348,7 +348,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";
   }
 
-- 
2.1.4