[Bug 21848] New: Handle encoding when calling Text::Unaccent::unac_string
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21848 Bug ID: 21848 Summary: Handle encoding when calling Text::Unaccent::unac_string Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: enhancement Priority: P5 - low Component: Architecture, internals, and plumbing Assignee: koha-bugs@lists.koha-community.org Reporter: m.de.rooy@rijksmuseum.nl QA Contact: testopia@bugs.koha-community.org Triggered by running Circulation.t on bug 18677: t/db_dependent/Circulation.t .. 1/121 unac_string: Invalid or incomplete multibyte or wide character unac_string: Invalid or incomplete multibyte or wide character Note that Text::Unaccent says: Return the unaccented equivalent of the string $string. The character set of $string is specified by the $charset argument. The returned string is coded using the same character set. The only place we call unac_string is: Koha::Patron->generate_userid: $userid = unac_string('utf-8',$userid); Since we say to unac_string that $userid is utf-8, we should first encode it and the result we should decode again for further handling. -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21848 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Patch complexity|--- |Trivial patch Status|NEW |Needs Signoff -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21848 --- Comment #1 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 82384 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=82384&action=edit Bug 21848: Handle encoding when calling Text::Unaccent::unac_string Resolve warnings like: unac_string: Invalid or incomplete multibyte or wide character Note that Text::Unaccent says: Return the unaccented equivalent of the string $string. The character set of $string is specified by the $charset argument. The returned string is coded using the same character set. The only place we call unac_string is: Koha::Patron->generate_userid: $userid = unac_string('utf-8',$userid); Since we say to unac_string that $userid is utf-8, we should first encode it and the result we should decode again for further handling. Test plan: [1] Run t/db_dependent/Koha/Patrons.t [2] Run t/db_dependent/Circulation.t (before this patch it gave a few warns) Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21848 --- Comment #2 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 82385 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=82385&action=edit Bug 21848: Remove Text::Unaccent from C4::Members At refactoring time the unac_string call was moved to Koha::Patron. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21848 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off --- Comment #3 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Signing off in stream of 18677 -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21848 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Severity|enhancement |minor Assignee|koha-bugs@lists.koha-commun |m.de.rooy@rijksmuseum.nl |ity.org | See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=18677 --- Comment #4 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Tomas: Please QA. It is all about tomasito ;) -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21848 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |tomascohen@gmail.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21848 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jonathan.druart@bugs.koha-c | |ommunity.org --- Comment #5 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Marcel, do you understand why we do not get the warning from Patrons.t? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21848 --- Comment #6 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- I think that we should add a test to show that we removed the warning. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21848 --- Comment #7 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Jonathan Druart from comment #5)
Marcel, do you understand why we do not get the warning from Patrons.t?
Since we do not use special chars in the names there. Which we did in Circulation.t. At least I see: my %expired_borrower_data = ( firstname => 'Ça', surname => 'Glisse', categorycode => $patron_category->{categorycode}, branchcode => $branch, dateexpiry => dt_from_string->subtract( months => 1 ), ); […] Koha::Patron->new(\%expired_borrower_data)->store->borrowernumber; Yeah, renaming Ça is enough to get rid of the warn. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21848 --- Comment #8 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Hmm. Something strange going on here. Note that adding use utf8; to Circulation.t also resolves the warning. Having trouble to catch the warn with warning_like and its friends. Should we just add "use utf8;" and leave Patron.pm alone ? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21848 --- Comment #9 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 83146 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=83146&action=edit Bug 21848: Counterpatch to resolve unac_string warning Just add "use utf8". Test plan: Prove Circulation.t without warnings. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21848 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |dcook@prosentient.com.au See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=14759 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21848 --- Comment #10 from David Cook <dcook@prosentient.com.au> --- Comment on attachment 82384 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=82384 Bug 21848: Handle encoding when calling Text::Unaccent::unac_string Review of attachment 82384: --> (https://bugs.koha-community.org/bugzilla3/page.cgi?id=splinter.html&bug=21848&attachment=82384) ----------------------------------------------------------------- ::: Koha/Patron.pm @@ +25,3 @@
use List::MoreUtils qw( uniq ); use JSON qw( to_json ); use Text::Unaccent qw( unac_string );
Do we still need to load Text::Unaccent here? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21848 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #82384|0 |1 is obsolete| | Attachment #82385|0 |1 is obsolete| | Attachment #83146|0 |1 is obsolete| | --- Comment #11 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 83210 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=83210&action=edit Bug 21848: Resolve unac_string warning from Circulation.t Just add "use utf8". Test plan: Run Circulation.t; verify there are no warnings. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21848 --- Comment #12 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 83211 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=83211&action=edit Bug 21848: Remove Text::Unaccent from C4::Members At refactoring time the unac_string call was moved to Koha::Patron. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21848 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Summary|Handle encoding when |Resolve unac_string warning |calling |from Circulation.t |Text::Unaccent::unac_string | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21848 --- Comment #13 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Jonathan: I decided to simplify here. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21848 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21848 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #83210|0 |1 is obsolete| | Attachment #83211|0 |1 is obsolete| | --- Comment #14 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 83217 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=83217&action=edit Bug 21848: Resolve unac_string warning from Circulation.t Just add "use utf8". Test plan: Run Circulation.t; verify there are no warnings. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21848 --- Comment #15 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 83218 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=83218&action=edit Bug 21848: Remove Text::Unaccent from C4::Members At refactoring time the unac_string call was moved to Koha::Patron. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21848 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to Master CC| |nick@bywatersolutions.com --- Comment #16 from Nick Clemens <nick@bywatersolutions.com> --- Awesome work all! Pushed to master for 19.05 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21848 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to Master |Pushed to Stable CC| |martin.renvoize@ptfs-europe | |.com --- Comment #17 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Pushed to 18.11.x for 18.11.01 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21848 Lucas Gass <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |lucas@bywatersolutions.com --- Comment #18 from Lucas Gass <lucas@bywatersolutions.com> --- backported to 18.05 for 18.05.08 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21848 Fridolin SOMERS <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |fridolin.somers@biblibre.co | |m --- Comment #19 from Fridolin SOMERS <fridolin.somers@biblibre.com> --- Pushed to 17.11.x for 17.11.14 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21848 Fridolin SOMERS <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |19936 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19936 [Bug 19936] Move Check_userid and Generate_Userid to Koha::Patron -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21848 Fridolin SOMERS <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to Stable |RESOLVED Resolution|--- |FIXED --- Comment #20 from Fridolin SOMERS <fridolin.somers@biblibre.com> --- Ah brakes Jenkins. In the end the warn does not exist in 17.11.x. Caused by Bug 19936 who moved generate_userid to Koha::Patron. -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon@bugs.koha-community.org