Description
Kelly McElligott
2018-10-01 18:59:10 UTC
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. *** 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 |