When creating we still call generate_userid and verify the result.
When modifying we do not accept an invalid userid. When needed,
we recreate the userid; this should be very exceptional.
Test plan:
Run t/db_dependent/Koha/Patrons.t
Go to staff interface. Try changing userid of a patron to an
existing one.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
isa => 'Koha::Exceptions::Patron',
description => "Deleting patron failed, AnonymousPatron is not deleteable"
},
+ 'Koha::Exceptions::Patron::InvalidUserid' => {
+ isa => 'Koha::Exceptions::Patron',
+ description => 'Field userid is not valid (probably not unique)',
+ fields => [ 'userid' ],
+ },
'Koha::Exceptions::Patron::MissingMandatoryExtendedAttribute' => {
isa => 'Koha::Exceptions::Patron',
description => "Mandatory extended attribute missing",
unless ( $self->in_storage ) { #AddMember
# Generate a valid userid/login if needed
- $self->generate_userid
- if not $self->userid or not $self->has_valid_userid;
+ $self->generate_userid unless $self->userid;
+ Koha::Exceptions::Patron::InvalidUserid->throw( userid => $self->userid )
+ unless $self->has_valid_userid;
# Add expiration date if it isn't already there
unless ( $self->dateexpiry ) {
else { #ModMember
my $self_from_storage = $self->get_from_storage;
- # FIXME We should not deal with that here, callers have to do this job
- # Moved from ModMember to prevent regressions
- unless ( $self->userid ) {
- my $stored_userid = $self_from_storage->userid;
- $self->userid($stored_userid);
- }
+
+ # Do not accept invalid userid here
+ $self->generate_userid unless $self->userid;
+ Koha::Exceptions::Patron::InvalidUserid->throw( userid => $self->userid )
+ unless $self->has_valid_userid;
# Password must be updated using $self->set_password
$self->password($self_from_storage->password);
};
subtest '->store' => sub {
- plan tests => 8;
+ plan tests => 9;
my $schema = Koha::Database->new->schema;
$schema->storage->txn_begin;
- my $print_error = $schema->storage->dbh->{PrintError};
- $schema->storage->dbh->{PrintError} = 0; ; # FIXME This does not longer work - because of the transaction in Koha::Patron->store?
-
my $patron_1 = $builder->build_object({class=> 'Koha::Patrons'});
my $patron_2 = $builder->build_object({class=> 'Koha::Patrons'});
- {
- local *STDERR;
- open STDERR, '>', '/dev/null';
- throws_ok { $patron_2->userid( $patron_1->userid )->store; }
- 'Koha::Exceptions::Object::DuplicateID',
- 'Koha::Patron->store raises an exception on duplicate ID';
- close STDERR;
- }
+ throws_ok { $patron_2->userid( $patron_1->userid )->store; }
+ 'Koha::Exceptions::Patron::InvalidUserid',
+ 'Koha::Patron->store raises an exception on duplicate ID';
+
+ # Clear userid and check regeneration
+ $patron_2->userid(undef)->store;
+ like( $patron_2->userid, qr/\w+\.\w+/, 'Userid regenerated' ); # old school userid
# Test password
t::lib::Mocks::mock_preference( 'RequireStrongPassword', 0 );
$patron_1->relationship("")->store;
is( $patron_1->relationship, undef, );
- $schema->storage->dbh->{PrintError} = $print_error;
$schema->storage->txn_rollback;
subtest 'skip updated_on for BorrowersLog' => sub {