Attachment 71789 Details for Bug 19936 – Bug 19936: Replace Check_userid - Update the occurrences

[patch] Bug 19936: Replace Check_userid - Update the occurrences

Bug-19936-Replace-Checkuserid---Update-the-occurre.patch (text/plain), 5.55 KB, created by Jonathan Druart on 2018-02-16 14:34:19 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Jonathan Druart

Created: 2018-02-16 14:34:19 UTC

Size: 5.55 KB

patch

obsolete

>From 5b51b2efc7e43e141ad1dc2bccd20e1b2891b3c5 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 90ebb54afe..ef82ba29b8 100644
>--- a/C4/Members.pm
>+++ b/C4/Members.pm
>@@ -398,9 +398,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;
>@@ -506,7 +507,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
> 
>@@ -514,16 +515,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 228bc5ed0f..644f9e4d6f 100755
>--- a/members/memberentry.pl
>+++ b/members/memberentry.pl
>@@ -349,7 +349,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 d17bc408bc..b0664de704 100755
>--- a/tools/import_borrowers.pl
>+++ b/tools/import_borrowers.pl
>@@ -233,11 +233,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) {
>@@ -250,8 +250,8 @@ if ( $uploadborrowers && length($uploadborrowers) > 0 ) {
>             }
>         }
> 
>-        if ($member) {
>-            $member = $member->unblessed;
>+        if ($patron) {
>+            $member = $patron->unblessed;
>             $borrowernumber = $member->{'borrowernumber'};
>         } else {
>             $member = {};
>@@ -290,7 +290,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

Actions: View | Diff | Splinter Review

Attachments on bug 19936: 70365 | 70366 | 70367 | 70368 | 70369 | 70370 | 70371 | 70372 | 70373 | 70374 | 71787 | 71788 | 71789 | 71790 | 71791 | 71792 | 71793 | 71794 | 71795 | 71796 | 72129 | 72130 | 72131 | 72132 | 72133 | 72134 | 72135 | 72136 | 72137 | 72138 | 72139 | 72140 | 72141 | 72142 | 72143 | 72144 | 72145 | 72146 | 72147 | 72148 | 72149 | 72150 | 72151 | 72152 | 73630 | 73631 | 73632 | 73633 | 73634 | 73635 | 73636 | 73637 | 73638 | 73639 | 73640 | 73641 | 73963 | 73964 | 73965 | 73966 | 73967 | 73968 | 73969 | 73970 | 73971 | 73972 | 73973 | 73974 | 74071 | 74072 | 74073 | 74074 | 74075 | 74076 | 74077 | 74078 | 74079 | 74080 | 74081 | 74082