http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10625 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|gmcharlt@gmail.com |kyle@bywatersolutions.com --- Comment #1 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 19857 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=19857&action=edit Sample file of barcodes for testing -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10625 --- Comment #2 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 19858 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=19858&action=edit Bug 10625 - Inventory/Stocktaking tool cannot handle windows file uploads The inventory/stocktaking tool interprets windows text editor file as a single large barcode because the text editor only uses carriage returns for line breaks, rather than a carriage return and line break combination. This tools should be able to handle \r, \n, and/or \r\n. Test Plan: 1) Download the attached sample file of barcodes 2) Upload this file to the inventory/stocktaking tool. For this test, it doesn't matter that these barcodes aren't cataloged in your system. 3) Note that the "barcode not found" is a single extremely long barcode 4) Apply this patch 5) Repeat step 2 6) Note that you now get a "barcode not found" error for each barcode individually. -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10625 Mirko Tietgen <mirko@abunchofthings.net> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off CC| |mirko@abunchofthings.net -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10625 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #19970|0 |1 is obsolete| | --- Comment #4 from Jonathan Druart <jonathan.druart@biblibre.com> --- Created attachment 20466 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=20466&action=edit Bug 10625 - Inventory/Stocktaking tool cannot handle windows file uploads The inventory/stocktaking tool interprets windows text editor file as a single large barcode because the text editor only uses carriage returns for line breaks, rather than a carriage return and line break combination. This tools should be able to handle \r, \n, and/or \r\n. Test Plan: 1) Download the attached sample file of barcodes 2) Upload this file to the inventory/stocktaking tool. For this test, it doesn't matter that these barcodes aren't cataloged in your system. 3) Note that the "barcode not found" is a single extremely long barcode 4) Apply this patch 5) Repeat step 2 6) Note that you now get a "barcode not found" error for each barcode individually. Signed-off-by: Mirko Tietgen <mirko@abunchofthings.net> Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com> -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10625 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |7684 -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10625 --- Comment #6 from Jonathan Druart <jonathan.druart@biblibre.com> --- (In reply to Galen Charlton from comment #5)

The sample file does not actually use \r\n as its delimiter -- it's using \r.

Can you confirm the source of the sample file you attached to the bug, as does not appear to actually be produced by a Windows text editor -- or in any event, not one that uses the DOS newline.

Galen, yes you are right, \r\n is under windows and it worked before this patch. But \r (Mac OS) did not worked before. The sample file contains \r. I tested the \r\n too : I added a newline character after \r in order to have a \r\n and it still works with the patch. Maybe should we change the commit message. -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10625 --- Comment #8 from Galen Charlton <gmcharlt@gmail.com> --- (In reply to Jonathan Druart from comment #6)

Galen, yes you are right, \r\n is under windows and it worked before this patch. But \r (Mac OS) did not worked before.

Ancient versions of Mac OS, not recent ones. Are there folks actually using MacOS 9 or earlier with Koha?

The sample file contains \r. I tested the \r\n too : I added a newline character after \r in order to have a \r\n and it still works with the patch.

DOS/Windows line endings also worked without the patch -- see commit 890f673d64e2. Since the use of File::Slurp means loading the entire input barcode file into memory, this patch also changes the performance characters of the inventory tool when processing very large files. Admittedly, this may not be a big deal in practice, but I would appreciate confirmation as to whether there are barcode scanners or other utilities that are *actually* producing files that use \r as a delimiter. -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10625 mathieu saby <mathieu.saby@univ-rennes2.fr> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |mathieu.saby@univ-rennes2.f | |r -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10625 --- Comment #10 from M. Tompsett <mtompset@hotmail.com> --- Created attachment 37846 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=37846&action=edit Bug 10625: Inventory/Stocktaking tool cannot handle windows file uploads The current code uses $barcode = <fh>; logic. This reads until \n, as far as I can tell. EOL is indicated by \n, \r, and \r\n depending on OS and software. So, to this end, rather than File::Slurp (which is a potential memory hog, which is already an issue with no filters), a loop to pre-read the barcodes was written. This loop includes: $barcode =~ s/\r/\n/g; $barcode =~ s/\n\n/\n/g; $barcode =~ s/\n$//; $barcode =~ s/^\n//; my @data = split(/\n/, $barcode); push @uploadedbarcodes,@data; So, that means that lines ending in \n would have it stripped and pushed into the uploaded barcodes array. Lines ending in \r would likely be read as one giant block, have everything converted to single \n's and then using a split, the set of barcodes are pushed into the uploaded barcodes array. Lines ending in \r\n would get that stripped and pushed into the uploaded barcodes array. It is then the uploaded barcodes array that is looped over for validating the barcodes. TEST PLAN --------- 1) Create a file with at least three different barcodes. --- BEGIN EXAMPLE --- BARCODE1\n BARCODE2\r BARCODE3\r\n --- END EXAMPLE --- $ echo "BARCODE1\nBARCODE2\rBARCODE3\r\n" | sed -e "s#\\\\n#\\n#g" | sed -e "s#\\\\r#\\r#g" > test.txt 2) Log into staff client 3) Tools -> Inventory/stocktaking 4) Click 'Browse' and select the file to upload (e.g. test.txt) 5) Click 'Submit' 6) A Javascript warning comes up, click 'Yes' -- This will take a LONG time, at least 1 minute. -- eventual results will be wrong. 7) Apply patch 8) repeat steps 3-6 -- This time correct number of barcodes will be given. 9) run koha qa test tools -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10625 --- Comment #12 from Marc Véron <veron@veron.ch> --- Created attachment 37848 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=37848&action=edit [Signed-off] Bug 10625: Inventory/Stocktaking tool cannot handle windows file uploads The current code uses $barcode = <fh>; logic. This reads until \n, as far as I can tell. EOL is indicated by \n, \r, and \r\n depending on OS and software. So, to this end, rather than File::Slurp (which is a potential memory hog, which is already an issue with no filters), a loop to pre-read the barcodes was written. This loop includes: $barcode =~ s/\r/\n/g; $barcode =~ s/\n\n/\n/g; $barcode =~ s/\n$//; $barcode =~ s/^\n//; my @data = split(/\n/, $barcode); push @uploadedbarcodes,@data; So, that means that lines ending in \n would have it stripped and pushed into the uploaded barcodes array. Lines ending in \r would likely be read as one giant block, have everything converted to single \n's and then using a split, the set of barcodes are pushed into the uploaded barcodes array. Lines ending in \r\n would get that stripped and pushed into the uploaded barcodes array. It is then the uploaded barcodes array that is looped over for validating the barcodes. TEST PLAN --------- 1) Create a file with at least three different barcodes. --- I tested with the test file provided and with a file where some lines have CR and other lines have CR NL (inserted with Win text editor). Worked as expected. Signed-off-by: Marc Véron <veron@veron.ch> -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10625 Marc Véron <veron@veron.ch> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #37846|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10625 --- Comment #17 from M. Tompsett <mtompset@hotmail.com> --- Created attachment 38572 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=38572&action=edit Three Barcodes and blank lines (\n delimiter) -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10625 --- Comment #19 from M. Tompsett <mtompset@hotmail.com> --- Created attachment 38574 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=38574&action=edit Three Barcodes and blank lines (\r\n delimiter) -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10625 M. Tompsett <mtompset@hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #38575|0 |1 is obsolete| | --- Comment #21 from M. Tompsett <mtompset@hotmail.com> --- Created attachment 38576 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=38576&action=edit Bug 10625: Inventory/Stocktaking tool cannot handle windows file uploads The current code uses $barcode = <fh>; logic. This reads until \n, as far as I can tell. EOL is indicated by \n, \r, and \r\n depending on OS and software. So, to this end, rather than File::Slurp (which is a potential memory hog, which is already an issue with no filters), a loop to pre-read the barcodes was written. This loop includes: $barcode =~ s/\r/\n/g; $barcode =~ s/\n\n/\n/g; my @data = split(/\n/, $barcode); push @uploadedbarcodes,@data; So, that means that lines ending in \n would have it stripped and pushed into the uploaded barcodes array. Lines ending in \r would likely be read as one giant block, have everything converted to single \n's and then using a split, the set of barcodes are pushed into the uploaded barcodes array. Lines ending in \r\n would get that stripped and pushed into the uploaded barcodes array. It is then the uploaded barcodes array that is looped over for validating the barcodes. TEST PLAN --------- 1) Back up your database 2) Download the three sample files (or create your own) 3) Log in to staff client 4) Create a branch with no inventory. 5) Home -> Tools -> Inventory/Stocktaking 6) Browse for your '\r' test file. 7) Limit to just that branch 8) Click 'Submit' -- Confirm expected errors 9) Repeat steps 5-8 with the '\n' test file. 10) Repeat steps 5-8 with the '\r\n' test file. -- one of these repetitions should have problems. 11) Apply patch 12) Repeat steps 5-8 for each of the 3 test files. -- there should be no issues. 13) run koha qa test tools. Note: This is a tweak based on Jonathan Druart's comment #16 I have reset it to needs sign off again. -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10625 M. Tompsett <mtompset@hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #20466|0 |1 is obsolete| | --- Comment #22 from M. Tompsett <mtompset@hotmail.com> --- Comment on attachment 20466 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=20466 Bug 10625 - Inventory/Stocktaking tool cannot handle windows file uploads I think my patch is the way we are going. Obsoleting this. Feel free to unobsolete, as required. -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10625 Marc Véron <veron@veron.ch> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #38576|0 |1 is obsolete| | --- Comment #24 from Marc Véron <veron@veron.ch> --- Created attachment 38730 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=38730&action=edit Bug 10625: Inventory/Stocktaking tool cannot handle windows file uploads The current code uses $barcode = <fh>; logic. This reads until \n, as far as I can tell. EOL is indicated by \n, \r, and \r\n depending on OS and software. So, to this end, rather than File::Slurp (which is a potential memory hog, which is already an issue with no filters), a loop to pre-read the barcodes was written. This loop includes: $barcode =~ s/\r/\n/g; $barcode =~ s/\n\n/\n/g; my @data = split(/\n/, $barcode); push @uploadedbarcodes,@data; So, that means that lines ending in \n would have it stripped and pushed into the uploaded barcodes array. Lines ending in \r would likely be read as one giant block, have everything converted to single \n's and then using a split, the set of barcodes are pushed into the uploaded barcodes array. Lines ending in \r\n would get that stripped and pushed into the uploaded barcodes array. It is then the uploaded barcodes array that is looped over for validating the barcodes. TEST PLAN --------- 1) Back up your database 2) Download the three sample files (or create your own) 3) Log in to staff client 4) Create a branch with no inventory. 5) Home -> Tools -> Inventory/Stocktaking 6) Browse for your '\r' test file. 7) Limit to just that branch 8) Click 'Submit' -- Confirm expected errors 9) Repeat steps 5-8 with the '\n' test file. 10) Repeat steps 5-8 with the '\r\n' test file. -- one of these repetitions should have problems. 11) Apply patch 12) Repeat steps 5-8 for each of the 3 test files. -- there should be no issues. 13) run koha qa test tools. Note: This is a tweak based on Jonathan Druart's comment #16 I have reset it to needs sign off again. Followed test plan. Works as expected. qa OK. Signed-off-by: Marc Véron <veron@veron.ch> -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10625 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10625 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |tomascohen@gmail.com Status|Passed QA |Pushed to Master --- Comment #26 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Patch pushed to master. Thanks Mark! -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10625 Liz Rea <wizzyrea@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |wizzyrea@gmail.com --- Comment #28 from Liz Rea <wizzyrea@gmail.com> --- Patch pushed to 3.18.x will be in 3.18.08 -- You are receiving this mail because: You are watching all bug changes.