Bug 11811 - tools/import_borrowers.pl doesn't support utf-8 encoded CSV
Summary: tools/import_borrowers.pl doesn't support utf-8 encoded CSV
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Tools (show other bugs)
Version: 3.14
Hardware: All All
: P1 - high major
Assignee: Dobrica Pavlinusic
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-02-21 12:50 UTC by Dobrica Pavlinusic
Modified: 2015-12-03 22:00 UTC (History)
4 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
Circulation function:


Attachments
Bug 11811 - tools/import_borrowers.pl doesn't support utf-8 encoded CSV (1.78 KB, patch)
2014-02-25 11:45 UTC, Dobrica Pavlinusic
Details | Diff | Splinter Review
My test record. (899 bytes, application/vnd.ms-excel)
2014-02-26 06:23 UTC, Mark Tompsett
Details
[SIGNED OFF] Bug 11811 - tools/import_borrowers.pl doesn't support utf-8 encoded CSV (1.96 KB, patch)
2014-02-26 06:26 UTC, Mark Tompsett
Details | Diff | Splinter Review
Bug 11811 - tools/import_borrowers.pl utf-8 support for CSV (2.76 KB, patch)
2014-03-11 09:44 UTC, Dobrica Pavlinusic
Details | Diff | Splinter Review
CSV file referenced in test scenario (198 bytes, application/octet-stream)
2014-03-11 09:48 UTC, Dobrica Pavlinusic
Details
Bug 11811 - tools/import_borrowers.pl utf-8 support for CSV (2.83 KB, patch)
2014-03-11 11:48 UTC, Alex Sassmannshausen
Details | Diff | Splinter Review
[PASSED QA] Bug 11811 - tools/import_borrowers.pl utf-8 support for CSV (3.01 KB, patch)
2014-03-16 12:57 UTC, Katrin Fischer
Details | Diff | Splinter Review
script to change collation of Koha's tables to utf8_unicode_ci (13.35 KB, text/plain)
2014-04-14 16:22 UTC, Galen Charlton
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Dobrica Pavlinusic 2014-02-21 12:50:38 UTC

    
Comment 1 Katrin Fischer 2014-02-23 20:51:17 UTC
Hi Dobrica, can you explain the problem a bit? Is this a new problem? We are using the import and had no problems with UTF-8 encoded files so far, but using older versions of Koha.
Comment 2 Dobrica Pavlinusic 2014-02-25 11:45:50 UTC Comment hidden (obsolete)
Comment 3 Dobrica Pavlinusic 2014-02-25 12:16:18 UTC
It does work as-is but for wrong reasons.

We are really not decoding utf-8 characters and our only hope is that mysql will insert bytes as it gets them (which it does) so it works without this modification. This might change in future if mysql decided to check validity of data we are inserting into it.
Comment 4 Mark Tompsett 2014-02-26 05:58:45 UTC
I did the following:
Home -> Tools -> Import Patrons
I downloaded the sample CSV
I set the borrower number and card numbers to a high number like 777.
I made the first name and surname equal to BogusÁàö
I set an email address.
I set a branch code and category code to something valid.
Set date enrolled to 2014-01-01
Set userid and password to test1 and test1.
I saved, and my MS Excel complained about the format. I saved it anyways.
I then went back to the staff client, browsed for it,
and scrolled down to click Import.
It complained the name was Bogus���
And searching for the patron had nothing on the end of Bogus.
I have the problem under master and patched.
Comment 5 Katrin Fischer 2014-02-26 06:03:06 UTC
My guess is, that Excel changed the encoding. I does that for me every time and it's why I switched to LibreOffice for preparing the files for patron import. The CSV export options work nicer there. Check that your CSV is really UTF-8.
Comment 6 Mark Tompsett 2014-02-26 06:23:53 UTC Comment hidden (obsolete)
Comment 7 Mark Tompsett 2014-02-26 06:26:52 UTC Comment hidden (obsolete)
Comment 8 Dobrica Pavlinusic 2014-03-11 09:44:42 UTC Comment hidden (obsolete)
Comment 9 Dobrica Pavlinusic 2014-03-11 09:48:55 UTC
Created attachment 26060 [details]
CSV file referenced in test scenario

I removed sign off from this patch because just decoding utf-8 didn't solve our initial problem completely.

Since it might be challenge to create both variants of unicode encoding, I'm attaching this file which includes my surname "Pavlinušić" in both formats.
Comment 10 Alex Sassmannshausen 2014-03-11 11:48:42 UTC Comment hidden (obsolete)
Comment 11 Katrin Fischer 2014-03-16 12:57:04 UTC
Created attachment 26375 [details] [review]
[PASSED QA] Bug 11811 - tools/import_borrowers.pl utf-8 support for CSV

Decode utf-8 characters in CSV file and perform canonical decomposition
followed by canonical composition.

Use case for this is XLS file converted into CSV which includes utf-8
characters which display correctly in interface but are not searchable
in Koha's patrons search since browser does same decomposition as this
code does.

Test scenario:
1. go to Tools > Import patrons
2. create patrons using attached example files which include both
   variants of my surname. You will have to fill in branchcode and
   categorycode according to configuration of Koha instance which
   is used in web form since those values are not included in CSV
3. import patrons and verify then utf-8 characters are correct and
   that you get just one results when searching for my surname
   (cardnumber ITI-00000151832). You can just copy/paste my surname
   from example file (any of them) since browser does similar
   decomposition as this code.
4. apply this patch and repeat import of patrons, selecting option
   to overwrite existing records
5. verify that after searching for my surname you get two borrowers
   (cardnumbers ITI-00000151831 and ITI-00000151832)

Signed-off-by: A. Sassmannshausen <alex.sassmannshausen@ptfs-europe.com>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Passes all tests and QA script.
Tested with my own file, some German umlauts and non-Latin scripts.
All works as expected.
Comment 12 Galen Charlton 2014-04-14 16:19:17 UTC
(In reply to Katrin Fischer from comment #11)
> Use case for this is XLS file converted into CSV which includes utf-8
> characters which display correctly in interface but are not searchable
> in Koha's patrons search since browser does same decomposition as this
> code does.

Not so.  There is no guarantee that strings entered via the web browser, or patron searches entered via the web browser, will end up in NFC.  Consequently, one can end up with a mixture of NFD and NFC data in the database, meaning that patron searches will not necessarily bring up the same results.

One of the root issues is the default collation that is typically used.  In my development database, it happens to be utf8_general_ci -- which in general is problematic [1], and in particular doesn't treat the NFD and NFC forms of Dobrica's surname as equivalent.  Spot-checking suggests that utf8_general_ci ends up as the default for databases installed from Debian packages, with the exception of a couple columns that kohastructure.sql sets to utf8_unicode_ci.

Rather than piecemeal sticking in NFC(), I think a better solution would be to change the collation to utf8_unicode_ci across the board.

[1] http://stackoverflow.com/questions/1036454/what-are-the-diffrences-between-utf8-general-ci-and-utf8-unicode-ci
Comment 13 Galen Charlton 2014-04-14 16:22:33 UTC
Created attachment 27099 [details]
script to change collation of Koha's tables to utf8_unicode_ci

This is a script to change the collation of Koha's tables to utf8_unicode_ci.  Dobrica, please try it and advise if this fixes this particular bug, and whether it works in general for you.

Of course, changing the collation like this could have unintended consequences.
Comment 14 Jonathan Druart 2014-06-12 10:35:51 UTC
(In reply to Galen Charlton from comment #13)
> Created attachment 27099 [details]
> script to change collation of Koha's tables to utf8_unicode_ci
> 
> This is a script to change the collation of Koha's tables to
> utf8_unicode_ci.  Dobrica, please try it and advise if this fixes this
> particular bug, and whether it works in general for you.
> 
> Of course, changing the collation like this could have unintended
> consequences.

All looks good to me.
It seems that it is the way to do if the collation was utf8_general_ci but not for others values.
Maybe should we add an entry in the updatedb and if the collation is utf8_general_ci, we convert to utf8_unicode_ci, otherwise ask for help on the IRC channel?
Or just a note in the release notes.
Comment 15 Mark Tompsett 2014-06-12 14:49:28 UTC
(In reply to Jonathan Druart from comment #14)
> Or just a note in the release notes.

-- you are assuming people will read. I'm not sure that is a safe assumption.
Comment 16 Jonathan Druart 2015-02-20 16:01:39 UTC
I have just tested with the CSV file against master and it works \o/
Please reopen if I am wrong.