From b53075b58df01e65371e13dee0b6848d12a181f2 Mon Sep 17 00:00:00 2001 From: Jonathan Druart Date: Tue, 3 May 2016 08:58:33 +0100 Subject: [PATCH] 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 Signed-off-by: Katrin Fischer Signed-off-by: Brendan Gallagher (cherry picked from commit 7b76b24fad305b0253eb1d779f074d265087ca73) Signed-off-by: Julian Maurice --- C4/Members.pm | 3 +++ .../prog/en/modules/tools/import_borrowers.tt | 2 +- tools/import_borrowers.pl | 18 ++++++++++-------- 3 files changed, 14 insertions(+), 9 deletions(-) diff --git a/C4/Members.pm b/C4/Members.pm index 3114b0f7a4..ca1b02fb0e 100644 --- a/C4/Members.pm +++ b/C4/Members.pm @@ -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 diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/tools/import_borrowers.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/tools/import_borrowers.tt index f31d7dab84..d9f25db0c1 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/tools/import_borrowers.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/tools/import_borrowers.tt @@ -92,7 +92,7 @@ [% END %] [% IF ERROR.duplicate_userid %]
  • - Userid [% ERROR.userid %] is already used by another patron or is empty. + Userid [% ERROR.userid %] is already used by another patron.
  • [% END %] [% END %] diff --git a/tools/import_borrowers.pl b/tools/import_borrowers.pl index d5cf3b272b..190a5f2bca 100755 --- a/tools/import_borrowers.pl +++ b/tools/import_borrowers.pl @@ -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... -- 2.39.5