From 8012cc5ebdd2439791d87c11bce22134d1e76533 Mon Sep 17 00:00:00 2001 From: Jonathan Druart Date: Fri, 23 Feb 2018 13:45:09 -0300 Subject: [PATCH] Bug 19936: Reuse existing userid if none provided Found this regression when working on other patches: If you edit a patron and blank the userid field, it will be regenerated with an incremented value (firstname.surname will be firstname.surname1) This is because we use a non-existing patron and ->in_storage in has_valid_userid is always false. The trick here is to backup the value, generate the userid, then reset userid to the previous value. As the POD says, it will be fix later, when AddMember and ModMember will be replaced with Koha::Patron->store Signed-off-by: Josef Moravec Signed-off-by: Katrin Fischer Signed-off-by: Jonathan Druart --- Koha/Patron.pm | 11 ++++++++--- t/db_dependent/Koha/Patrons.t | 7 ++++++- 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/Koha/Patron.pm b/Koha/Patron.pm index 7458bc5671..c8e319e6ac 100644 --- a/Koha/Patron.pm +++ b/Koha/Patron.pm @@ -916,7 +916,7 @@ sub generate_userid { my ($self) = @_; my $userid; my $offset = 0; - my $patron = Koha::Patron->new; + 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 @@ -926,9 +926,14 @@ sub generate_userid { $userid = lc(($firstname)? "$firstname.$surname" : $surname); $userid = unac_string('utf-8',$userid); $userid .= $offset unless $offset == 0; - $patron->userid( $userid ); + $self->userid( $userid ); $offset++; - } while (! $patron->has_valid_userid ); + } 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; diff --git a/t/db_dependent/Koha/Patrons.t b/t/db_dependent/Koha/Patrons.t index c614f33ccb..0b6e122703 100644 --- a/t/db_dependent/Koha/Patrons.t +++ b/t/db_dependent/Koha/Patrons.t @@ -1265,7 +1265,7 @@ subtest 'userid_is_valid' => sub { }; subtest 'generate_userid' => sub { - plan tests => 6; + plan tests => 7; my $library = $builder->build_object( { class => 'Koha::Libraries' } ); my $patron_category = $builder->build_object( @@ -1303,6 +1303,11 @@ subtest 'generate_userid' => sub { $userid = $new_patron->generate_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; + is( $userid, $expected_userid_patron_1, 'generate_userid should generate the userid we expect' ); + # Cleanup $patron_1->delete; $patron_2->delete; -- 2.39.5