Bug 16426: follow-up of bug 15840 - correctly manage userid while inserting patrons

Bug 15840 tried to fix a bug but makes things more complicated than it
was before.
If an userid is not provided for 1 or more rows of the csv file, it
should not be updated. However, if a userid is provided and it already
used by an other patron, the import should fail for this row (but not
crash!).

Test plan:
0/ Create a patron with a userid=your_userid
1/ Use the import patron tool to update this userid
=> userid should have been updated
2/ Update another data and do not provide the userid
=> data should have been updated and not the userid
3/ Update another data and provide the userid, but set it to an empty
string, or '0'
=> data should have been updated and not the userid
4/ Update another patron, and set userid=your_userid
=> Update should fail and an error whouls be displayed ("already used by
another patron")

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

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

Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com>
(cherry picked from commit 7b76b24fad)
Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
This commit is contained in:
Jonathan Druart 2016-05-03 08:58:33 +01:00 committed by Julian Maurice
parent ea45e3f39e
commit b53075b58d
3 changed files with 14 additions and 9 deletions

View File

@ -662,6 +662,9 @@ sub ModMember {
my $rs = $schema->resultset('Borrower')->search({
borrowernumber => $new_borrower->{borrowernumber},
});
delete $new_borrower->{userid} if exists $new_borrower->{userid} and not $new_borrower->{userid};
my $execute_success = $rs->update($new_borrower);
if ($execute_success ne '0E0') { # only proceed if the update was a success
# ok if its an adult (type) it may have borrowers that depend on it as a guarantor

View File

@ -92,7 +92,7 @@
[% END %]
[% IF ERROR.duplicate_userid %]
<li class="line_error">
Userid [% ERROR.userid %] is already used by another patron or is empty.
Userid [% ERROR.userid %] is already used by another patron.
</li>
[% END %]
[% END %]

View File

@ -253,14 +253,6 @@ if ( $uploadborrowers && length($uploadborrowers) > 0 ) {
next;
}
# generate a proper login if none provided
if ( $borrower{userid} eq '' || !Check_Userid( $borrower{userid} ) ) {
push @errors, { duplicate_userid => 1, userid => $borrower{userid} };
$invalid++;
next LINE;
}
if ($borrowernumber) {
# borrower exists
unless ($overwrite_cardnumber) {
@ -280,6 +272,16 @@ if ( $uploadborrowers && length($uploadborrowers) > 0 ) {
$borrower{$col} = $member->{$col} if($member->{$col}) ;
}
}
# Check if the userid provided does not exist yet
if ( exists $borrower{userid}
and $borrower{userid}
and not Check_Userid( $borrower{userid}, $borrower{borrowernumber} ) ) {
push @errors, { duplicate_userid => 1, userid => $borrower{userid} };
$invalid++;
next LINE;
}
unless (ModMember(%borrower)) {
$invalid++;
# until we have better error trapping, we have no way of knowing why ModMember errored out...