From 4a25b95e14e1e0116c3f3015a814037148719ef0 Mon Sep 17 00:00:00 2001 From: Jonathan Druart Date: Fri, 23 Feb 2018 15:03:57 -0300 Subject: [PATCH] Bug 20287: generate_userid now set the userid Signed-off-by: Josef Moravec Signed-off-by: Tomas Cohen Arazi Signed-off-by: Martin Renvoize Signed-off-by: Nick Clemens --- Koha/Patron.pm | 19 +++++-------------- members/memberentry.pl | 6 ++++-- t/db_dependent/Koha/Patrons.t | 12 ++++++++---- 3 files changed, 17 insertions(+), 20 deletions(-) diff --git a/Koha/Patron.pm b/Koha/Patron.pm index 3b0d804add..420b576058 100644 --- a/Koha/Patron.pm +++ b/Koha/Patron.pm @@ -177,7 +177,7 @@ sub store { unless ( $self->in_storage ) { #AddMember # Generate a valid userid/login if needed - $self->userid($self->generate_userid) + $self->generate_userid if not $self->userid or not $self->has_valid_userid; # Add expiration date if it isn't already there @@ -1234,40 +1234,31 @@ sub has_valid_userid { =head3 generate_userid my $patron = Koha::Patron->new( $params ); -my $userid = $patron->generate_userid +$patron->generate_userid Generate a userid using the $surname and the $firstname (if there is a value in $firstname). -Return the generate userid ($firstname.$surname if there is a $firstname, or $surname if there is no value in $firstname) plus offset (0 if the $userid is unique, or a higher numeric value if not unique). - -# TODO: Set $self->userid with the generated value +Set a generated userid ($firstname.$surname if there is a $firstname, or $surname if there is no value in $firstname) plus offset (0 if the $userid is unique, or a higher numeric value if not unique). =cut sub generate_userid { my ($self) = @_; - my $userid; my $offset = 0; - my $existing_userid = $self->userid; my $firstname = $self->firstname // q{}; my $surname = $self->surname // q{}; #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; - $userid = lc(($firstname)? "$firstname.$surname" : $surname); + my $userid = lc(($firstname)? "$firstname.$surname" : $surname); $userid = unac_string('utf-8',$userid); $userid .= $offset unless $offset == 0; $self->userid( $userid ); $offset++; } while (! $self->has_valid_userid ); - # Resetting to the previous value as the callers do not expect - # this method to modify the userid attribute - # This will be done later (move of AddMember and ModMember) - $self->userid( $existing_userid ); - - return $userid; + return $self; } diff --git a/members/memberentry.pl b/members/memberentry.pl index 041dda23f6..7f48bfdcd8 100755 --- a/members/memberentry.pl +++ b/members/memberentry.pl @@ -283,7 +283,8 @@ if ( ( defined $newdata{'userid'} && $newdata{'userid'} eq '' ) || $check_Borrow # Full page edit, firstname and surname input zones are present $patron->firstname($newdata{firstname}); $patron->surname($newdata{surname}); - $newdata{'userid'} = $patron->generate_userid; + $patron->generate_userid; + $newdata{'userid'} = $patron->userid; } elsif ( ( defined $data{'firstname'} || $category_type eq 'I' ) && ( defined $data{'surname'} ) ) { # Partial page edit (access through "Details"/"Library details" tab), firstname and surname input zones are not used @@ -291,7 +292,8 @@ if ( ( defined $newdata{'userid'} && $newdata{'userid'} eq '' ) || $check_Borrow # FIXME clean thiscode newdata vs data is very confusing $patron->firstname($data{firstname}); $patron->surname($data{surname}); - $newdata{'userid'} = $patron->generate_userid; + $patron->generate_userid; + $newdata{'userid'} = $patron->userid; } else { $newdata{'userid'} = $data{'userid'}; diff --git a/t/db_dependent/Koha/Patrons.t b/t/db_dependent/Koha/Patrons.t index 0a88ea8d9b..fa3c8dd43a 100644 --- a/t/db_dependent/Koha/Patrons.t +++ b/t/db_dependent/Koha/Patrons.t @@ -1324,13 +1324,15 @@ subtest 'generate_userid' => sub { my $expected_userid_patron_1 = 'tomasito.none'; my $new_patron = Koha::Patron->new({ firstname => $data{firstname}, surname => $data{surname} } ); - my $userid = $new_patron->generate_userid; + $new_patron->generate_userid; + my $userid = $new_patron->userid; is( $userid, $expected_userid_patron_1, 'generate_userid should generate the userid we expect' ); my $borrowernumber = Koha::Patron->new(\%data)->store->borrowernumber; my $patron_1 = Koha::Patrons->find($borrowernumber); is ( $patron_1->userid, $expected_userid_patron_1, 'The userid generated should be the one we expect' ); - $userid = $new_patron->generate_userid; + $new_patron->generate_userid; + $userid = $new_patron->userid; is( $userid, $expected_userid_patron_1 . '1', 'generate_userid should generate the userid we expect' ); $data{cardnumber} = '987654321'; my $new_borrowernumber = Koha::Patron->new(\%data)->store->borrowernumber; @@ -1340,12 +1342,14 @@ subtest 'generate_userid' => sub { is( $patron_2->userid, $expected_userid_patron_1 . '1', # TODO we could make that configurable "Patron with duplicate userid has new userid generated (1 is appened" ); - $userid = $new_patron->generate_userid; + $new_patron->generate_userid; + $userid = $new_patron->userid; is( $userid, $expected_userid_patron_1 . '2', 'generate_userid should generate the userid we expect' ); $patron_1 = Koha::Patrons->find($borrowernumber); $patron_1->userid(undef); - $userid = $patron_1->generate_userid; + $patron_1->generate_userid; + $userid = $patron_1->userid; is( $userid, $expected_userid_patron_1, 'generate_userid should generate the userid we expect' ); # Cleanup -- 2.39.5