In the Koha version of 18.05 using the patron import tool, if a patron has the same userid and cardnumber, and a library attempts of overlaying an updated file on patron information into Koha, an error will occur. Test Plan 1. Create patron csv according to the download starter CSV in Koha. 2. Patrons were assigned a card number by the library and this card number was also used for their User ID. 3. Import the Patron CSV into Koha. 4. Adjust this same Patron CSV file adding/updating an address, adding/updating a field of data. 5. Use this new file to import into Koha 6 Choose to match on the cardnumber 7. Choose to overwrite the existing one with this file. 8. Koha will produce an error saying that the userid already in use by another patron. Different Scenario Do Steps 1-5 the same 6. Choose to match on the User Id 7. Choose to overwrite the existing one with this file. 8. Koha will do the overwrite correctly. Libraries do tend to give patrons a cardnumber and also use that as the patron's userid. It would appear that matching on the userid works, however, this should not work this way, it should be able to match on either and produce same results- a successful overlay.
This appears to have been intentional according to tests, but I think the tests and assumption were wrong. Essentially if not matching by userid and a userid is inthe file we try to create a new user and see if the user id is valid. It isn't, of course, because the patron exists. has_valid_userid is supposed to ignore a duplicate for the existing borrower, but we avoid that check by creating a new borrower!?!? Patch coming
Created attachment 79850 [details] [review] Bug 21465: Don't throw duplicate userid error if userid belongs to the matched patron To test: 1 - Export your patrons a - Create a report 'SELECT * FROM borrowers' b - Run and save the report as csv (check your delimiter) c - Delete the borrowernumebr column 2 - Use the Patron Import tool to import the csv from above 3 - Set matching to 'cardnumber' 4 - Set 'If matching record is already in the borrowers table:' to Overwrite 5 - Import 6 - None are import because of matchign userid (their own) 7 - Apply patch 8 - Repeat 9 - Patrons are successfully overwritten 10 - prove -v t/db_dependent/Koha/Patrons/Import.t 11 - prove -v t/db_dependent/Koha/Patrons.t
Created attachment 79908 [details] [review] Bug 21465: Don't throw duplicate userid error if userid belongs to the matched patron To test: 1 - Export your patrons a - Create a report 'SELECT * FROM borrowers' b - Run and save the report as csv (check your delimiter) c - Delete the borrowernumebr column 2 - Use the Patron Import tool to import the csv from above 3 - Set matching to 'cardnumber' 4 - Set 'If matching record is already in the borrowers table:' to Overwrite 5 - Import 6 - None are import because of matchign userid (their own) 7 - Apply patch 8 - Repeat 9 - Patrons are successfully overwritten 10 - prove -v t/db_dependent/Koha/Patrons/Import.t 11 - prove -v t/db_dependent/Koha/Patrons.t
Created attachment 82877 [details] [review] Bug 21465: Don't throw duplicate userid error if userid belongs to the matched patron To test: 1 - Export your patrons a - Create a report 'SELECT * FROM borrowers' b - Run and save the report as csv (check your delimiter) c - Delete the borrowernumebr column 2 - Use the Patron Import tool to import the csv from above 3 - Set matching to 'cardnumber' 4 - Set 'If matching record is already in the borrowers table:' to Overwrite 5 - Import 6 - None are import because of matchign userid (their own) 7 - Apply patch 8 - Repeat 9 - Patrons are successfully overwritten 10 - prove -v t/db_dependent/Koha/Patrons/Import.t 11 - prove -v t/db_dependent/Koha/Patrons.t Signed-off-by: Owen Leonard <oleonard@myacpl.org>
The tests aren't successful but as far as I can tell the errors are the same as without the patch, so not relevant.
Is this is a duplicate of bug 21951?
*** Bug 21951 has been marked as a duplicate of this bug. ***
(In reply to Katrin Fischer from comment #6) > Is this is a duplicate of bug 21951? Other way round, only because there are tests here All tests pass for me
Nick, did you investigate the new warnings your tests add? Argument "" isn't numeric in numeric eq (==) at /usr/share/perl5/DBIx/Class/Row.pm line 1018, <$handle_3a> line 2.
Comes from eval { $patron->set(\%borrower)->store }; in import_patrons, it complains on invalid patron attribute's values (login_attempts, lost, etc.) Related to bug 21761.
Created attachment 83033 [details] [review] Bug 21465: Don't throw duplicate userid error if userid belongs to the matched patron To test: 1 - Export your patrons a - Create a report 'SELECT * FROM borrowers' b - Run and save the report as csv (check your delimiter) c - Delete the borrowernumebr column 2 - Use the Patron Import tool to import the csv from above 3 - Set matching to 'cardnumber' 4 - Set 'If matching record is already in the borrowers table:' to Overwrite 5 - Import 6 - None are import because of matchign userid (their own) 7 - Apply patch 8 - Repeat 9 - Patrons are successfully overwritten 10 - prove -v t/db_dependent/Koha/Patrons/Import.t 11 - prove -v t/db_dependent/Koha/Patrons.t Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Awesome work all! Pushed to master for 19.05
Pushed to 18.11.x for 18.11.01
Pushed to 17.11.x for 17.11.14
Depends on Bug 12598 not in 17.11.x
This was backported to 18.05.x for 18.05.07
(In reply to Jesse Maseto from comment #16) > This was backported to 18.05.x for 18.05.07 I think it is in 18.05.08?
(In reply to Nick Clemens from comment #17) > (In reply to Jesse Maseto from comment #16) > > This was backported to 18.05.x for 18.05.07 > > I think it is in 18.05.08? Yes, 18.05.08 NOT 18.05.07
(In reply to Jonathan Druart from comment #9) > Nick, did you investigate the new warnings your tests add? > > Argument "" isn't numeric in numeric eq (==) at > /usr/share/perl5/DBIx/Class/Row.pm line 1018, <$handle_3a> line 2. (In reply to Jonathan Druart from comment #10) > Comes from eval { $patron->set(\%borrower)->store }; in import_patrons, it > complains on invalid patron attribute's values (login_attempts, lost, etc.) > > Related to bug 21761. See bug 23734