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 <josef.moravec@gmail.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This commit is contained in:
Jonathan Druart 2018-02-23 13:45:09 -03:00
parent c252ab6a87
commit 8012cc5ebd
2 changed files with 14 additions and 4 deletions

View file

@ -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;

View file

@ -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;