[Bug 13502] New: A bug was introduced by 1861 - meaning borrowers sometimes cant be added
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13502 Bug ID: 13502 Summary: A bug was introduced by 1861 - meaning borrowers sometimes cant be added Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: normal Priority: P5 - low Component: Patrons Assignee: koha-bugs@lists.koha-community.org Reporter: chris@bigballofwax.co.nz QA Contact: testopia@bugs.koha-community.org CC: gmcharlt@gmail.com, kyle.m.hall@gmail.com Bug 1861 introduces some changes, Which is breaking some tests, and might break functional code also. DBD::mysql::st execute failed: Duplicate entry '' for key 'userid' at /srv/jenkins/workspace/Koha_3.18.x_U14/C4/SQLHelper.pm line 184. etc. This is caused by my $borrowernumber1 = AddMember(categorycode => $categorycode, branchcode => $branchcode); And then in the AddMember sub we do $data{'userid'} = Generate_Userid( $data{'borrowernumber'}, $data{'firstname'}, $data{'surname'} ) if ( $data{'userid'} eq '' || !Check_Userid( $data{'userid'} ) ); The problem here is if $data{'userid'} is undef the if fails and we don't create a new userid. ANd since the changes in bug 1861 make this column unique everything ASPLODES! -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13502 Chris Cormack <chris@bigballofwax.co.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |1861 -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13502 --- Comment #1 from Chris Cormack <chris@bigballofwax.co.nz> --- This is because we do this sub Check_Userid { my ( $uid, $borrowernumber ) = @_; return 1 unless ($uid); And 1 means unique .. so we do Check_Userid ... it tells us, yes its unique .. This is wrong we should return 0 here. If it is empty, assume it is non unique. This is a change in behaviour introduced by 1861. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13502 Chris Cormack <chris@bigballofwax.co.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13502 --- Comment #2 from Chris Cormack <chris@bigballofwax.co.nz> --- Created attachment 34832 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=34832&action=edit Bug 13502 : Code introcduced in 1861 wrongly assumes a null userid is unique To test 1/ Create a borrower with '' as their userid, you may have to edit a row in the db to do this 2/ Run perl t/db_dependent/Circulation/CheckIfIssuedToPatron.t 3/ Notice some tests fail and you see DBD::mysql::st execute failed: Duplicate entry '' for key 'userid' at /home/chrisc/git/catalyst-koha/C4/SQLHelper.pm line 184. 4/ Apply the patch 5/ Run the tests again, notice they now pass -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13502 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jonathan.druart@biblibre.co | |m --- Comment #3 from Jonathan Druart <jonathan.druart@biblibre.com> --- Chris, With all patches from bug 1861, I don't get the error: mysql> select count(*) from borrowers where userid=""; +----------+ | count(*) | +----------+ | 1 | +----------+ 1 row in set (0.01 sec) $ prove t/db_dependent/Circulation/CheckIfIssuedToPatron.t t/db_dependent/Circulation/CheckIfIssuedToPatron.t .. ok All tests successful. Files=1, Tests=22, 2 wallclock secs ( 0.04 usr 0.00 sys + 1.34 cusr 0.04 csys = 1.42 CPU) Result: PASS -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13502 --- Comment #4 from Chris Cormack <chris@bigballofwax.co.nz> --- Have you run the db updates? It won't manifest without it, if you check jenkins you can see it failing there too. Also the assumption that an undefined userid is unique is flawed. It's 12 mins until 2015 so happy new year. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13502 --- Comment #5 from Jonathan Druart <jonathan.druart@biblibre.com> --- (In reply to Chris Cormack from comment #4)
Have you run the db updates? It won't manifest without it, if you check jenkins you can see it failing there too. Also the assumption that an undefined userid is unique is flawed.
It's 12 mins until 2015 so happy new year.
Ha no, sorry about that! Happy new year Chris :) -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13502 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13502 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #34832|0 |1 is obsolete| | --- Comment #6 from Jonathan Druart <jonathan.druart@biblibre.com> --- Created attachment 34834 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=34834&action=edit Bug 13502 : Code introcduced in 1861 wrongly assumes a null userid is unique To test 1/ Create a borrower with '' as their userid, you may have to edit a row in the db to do this 2/ Run perl t/db_dependent/Circulation/CheckIfIssuedToPatron.t 3/ Notice some tests fail and you see DBD::mysql::st execute failed: Duplicate entry '' for key 'userid' at /home/chrisc/git/catalyst-koha/C4/SQLHelper.pm line 184. 4/ Apply the patch 5/ Run the tests again, notice they now pass Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com> -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13502 --- Comment #7 from Jonathan Druart <jonathan.druart@biblibre.com> --- Created attachment 34835 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=34835&action=edit Bug 13502: Add tests to highlight the problem Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com> -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13502 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #34835|0 |1 is obsolete| | --- Comment #8 from Jonathan Druart <jonathan.druart@biblibre.com> --- Created attachment 34836 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=34836&action=edit Bug 13502: Add tests to highlight the problem Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com> -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13502 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #34834|0 |1 is obsolete| | --- Comment #9 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 34847 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=34847&action=edit Bug 13502: Code introcduced in 1861 wrongly assumes a null userid is unique To test 1/ Create a borrower with '' as their userid, you may have to edit a row in the db to do this 2/ Run perl t/db_dependent/Circulation/CheckIfIssuedToPatron.t 3/ Notice some tests fail and you see DBD::mysql::st execute failed: Duplicate entry '' for key 'userid' at /home/chrisc/git/catalyst-koha/C4/SQLHelper.pm line 184. 4/ Apply the patch 5/ Run the tests again, notice they now pass Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com> -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13502 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #34836|0 |1 is obsolete| | --- Comment #10 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 34848 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=34848&action=edit Bug 13502: Add tests to highlight the problem Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com> -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13502 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |katrin.fischer@bsz-bw.de --- Comment #11 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- I am sorry, but something here is not quite right in my tests. I applied both patches, made sure the database is current: perl t/db_dependent/Members.t ok 67 - Borrower with duplicate userid has new userid generated not ok 68 - AddMember should insert the given userid # Failed test 'AddMember should insert the given userid' # at t/db_dependent/Members.t line 305. # got: 'tomasito.none2' # expected: 'a_user_id' I had to solve a conflict in Members.t. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13502 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA Patch complexity|--- |Small patch -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13502 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #34847|0 |1 is obsolete| | Attachment #34848|0 |1 is obsolete| | --- Comment #12 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Created attachment 34849 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=34849&action=edit [PASSED QA] Bug 13502: Code introcduced in 1861 wrongly assumes a null userid is unique To test 1/ Create a borrower with '' as their userid, you may have to edit a row in the db to do this 2/ Run perl t/db_dependent/Circulation/CheckIfIssuedToPatron.t 3/ Notice some tests fail and you see DBD::mysql::st execute failed: Duplicate entry '' for key 'userid' at /home/chrisc/git/catalyst-koha/C4/SQLHelper.pm line 184. 4/ Apply the patch 5/ Run the tests again, notice they now pass Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13502 --- Comment #13 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Passing Chris' original patch - I think the additional tests needs a little bit more work. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13502 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |tomascohen@gmail.com --- Comment #14 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Katrin, It seems you didn't run the new tests on top of the latest 1861 followups. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13502 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #34848|1 |0 is obsolete| | -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13502 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #34848|0 |1 is obsolete| | --- Comment #15 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Created attachment 34850 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=34850&action=edit [PASSED QA] Bug 13502: Add tests to highlight the problem Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13502 --- Comment #16 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- With the follow-up on 1861 applied, the tests pass! -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13502 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |martin.renvoize@ptfs-europe | |.com --- Comment #17 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Katrin, your plain too efficient.. you beat me to it ;) -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13502 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to Master --- Comment #18 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Patches pushed to master. Thanks Chris and Jonathan! -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13502 Mason James <mtj@kohaaloha.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |mtj@kohaaloha.com --- Comment #19 from Mason James <mtj@kohaaloha.com> --- Pushed to 3.16.x, will be in 3.16.7 -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13502 --- Comment #20 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Hm, wondering, looks like 1861 is not in 3.16 - so should this be? -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13502 --- Comment #21 from Mason James <mtj@kohaaloha.com> --- (In reply to Katrin Fischer from comment #20)
Hm, wondering, looks like 1861 is not in 3.16 - so should this be?
ah yes, i just noticed this myself - i'll revert it now -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13502 Fridolin SOMERS <fridolyn.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |13602 -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org