Bug 21142 - Batch item/record modification/deletion tools does not open uploaded files in utf-8
Summary: Batch item/record modification/deletion tools does not open uploaded files in...
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Tools (show other bugs)
Version: master
Hardware: All All
: P5 - low normal (vote)
Assignee: Jonathan Druart
QA Contact: Testopia
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2018-08-01 19:52 UTC by Victor Grousset/tuxayo
Modified: 2018-09-21 13:16 UTC (History)
4 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:


Attachments
Numbers without BOM (47 bytes, text/csv)
2018-08-01 19:53 UTC, Victor Grousset/tuxayo
Details
Numbers with BOM (50 bytes, text/csv)
2018-08-01 19:55 UTC, Victor Grousset/tuxayo
Details
Bug 21142: Open uploaded files in utf-8 - batch mod/del (2.26 KB, patch)
2018-08-17 15:13 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 21142: Open uploaded files in utf-8 - batch mod/del (2.29 KB, patch)
2018-08-17 17:13 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 21142: Open uploaded files in utf-8 - batch mod/del (2.36 KB, patch)
2018-08-20 12:44 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Victor Grousset/tuxayo 2018-08-01 19:52:59 UTC
It also affects "Item number files" which triggers bug 21141
But it's easier to test with barcodes. And the fix should be the same.

== Test plan ==
1. Go to Staff › Tools › Batch item modification
2. Upload the file from the attachments (see next comments) named numbers-without-BOM.csv
3. See that it complains about non existing barcodes but the codes look ok
4. Upload the file from the attachments named numbers-with-BOM.csv
5. See that the first line is corrupted.
   Something like "3131545845784555"


If you don't know how to check that my test files are correct.
On a Unix-like system do:
file numbers-with-BOM.csv
file numbers-without-BOM.csv
less numbers-with-BOM.csv
less numbers-without-BOM.csv
Comment 1 Victor Grousset/tuxayo 2018-08-01 19:53:56 UTC
Created attachment 77406 [details]
Numbers without BOM
Comment 2 Victor Grousset/tuxayo 2018-08-01 19:55:49 UTC
Created attachment 77407 [details]
Numbers with BOM
Comment 3 Victor Grousset/tuxayo 2018-08-01 19:56:56 UTC
I'm not sure to work on this. Anyone can feel free to take it.
Comment 4 Jonathan Druart 2018-08-17 14:53:16 UTC
It is just a display problem, right?
Comment 5 Jonathan Druart 2018-08-17 15:13:34 UTC
Created attachment 77986 [details] [review]
Bug 21142: Open uploaded files in utf-8 - batch mod/del

Batch item/record modification/deletion tools does not open uploaded
files in utf-8

Test plan:
Edit and delete records and items using a file which will contain:
valid and invalid barcodes with and without utf-8 chars (💕)

You should see the warning message:
 "The *** 39999000💕001679 does not exist in the database."
Comment 6 Owen Leonard 2018-08-17 17:13:54 UTC
Created attachment 77992 [details] [review]
Bug 21142: Open uploaded files in utf-8 - batch mod/del

Batch item/record modification/deletion tools does not open uploaded
files in utf-8

Test plan:
Edit and delete records and items using a file which will contain:
valid and invalid barcodes with and without utf-8 chars (💕)

You should see the warning message:
 "The *** 39999000💕001679 does not exist in the database."

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 7 Tomás Cohen Arazi 2018-08-20 12:44:37 UTC
Created attachment 78015 [details] [review]
Bug 21142: Open uploaded files in utf-8 - batch mod/del

Batch item/record modification/deletion tools does not open uploaded
files in utf-8

Test plan:
Edit and delete records and items using a file which will contain:
valid and invalid barcodes with and without utf-8 chars (💕)

You should see the warning message:
 "The *** 39999000💕001679 does not exist in the database."

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 8 Nick Clemens 2018-08-21 13:56:58 UTC
Awesome work all!

Pushed to master for 18.11
Comment 9 Victor Grousset/tuxayo 2018-08-21 15:08:33 UTC
Thanks for the fix! 💕💕💕


(In reply to Jonathan Druart from comment #4)
> It is just a display problem, right?

Actually jes and no, because with the BOM test case, it's still there (invisible instead of being mojibake[1])
And when using itemnumbers it's not just a display problem, it will make the first itemnumber invalid.
For some reason, with barcodes it works; there must be some code that cleans the beginning and the end of the line.

[1] https://en.wikipedia.org/wiki/Mojibake
Comment 10 Martin Renvoize 2018-08-24 09:58:55 UTC
Pushed to 18.05.x for 18.05.03
Comment 11 Fridolin SOMERS 2018-09-21 13:16:51 UTC
Pushed to 17.11.x for 17.11.10