From bd147a38323bb810f85c41134abfc894b0ac8e60 Mon Sep 17 00:00:00 2001 From: Jonathan Druart 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 Signed-off-by: Katrin Fischer Signed-off-by: Jonathan Druart --- 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 da5ed59e50..ee3904ec6f 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 db77a3be7b..a8d17c15ba 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 631759e6ce..2d002c3b7e 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.39.5