Bug 19585

Summary: Inventory: Allow additional separators in a barcode file
Product: Koha Reporter: Marcel de Rooy <m.de.rooy>
Component: ToolsAssignee: Marcel de Rooy <m.de.rooy>
Status: CLOSED FIXED QA Contact: Josef Moravec <josef.moravec>
Severity: enhancement    
Priority: P5 - low CC: f.demians, josef.moravec, jwayway, katrin.fischer, nick
Version: MainKeywords: Academy
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22996
Change sponsored?: --- Patch complexity: Trivial patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Attachments: Bug 19585: Inventory: Allow additional separators in a barcode file
Bug 19585: Inventory: Allow additional separators in a barcode file
Bug 19585: Inventory: Allow additional separators in a barcode file
Bug 19585: Inventory: Allow additional separators in a barcode file
Bug 19585: Inventory: Allow additional separators in a barcode file
Bug 19585: Inventory: Allow additional separators in a barcode file

Description Marcel de Rooy 2017-11-07 15:02:02 UTC

    
Comment 1 Marcel de Rooy 2017-11-13 11:03:43 UTC Comment hidden (obsolete)
Comment 2 Marcel de Rooy 2017-11-13 11:11:04 UTC Comment hidden (obsolete)
Comment 3 Marcel de Rooy 2017-11-13 11:13:27 UTC Comment hidden (obsolete)
Comment 4 Marcel de Rooy 2017-11-15 13:47:05 UTC
Created attachment 69154 [details] [review]
Bug 19585: Inventory: Allow additional separators in a barcode file

This patch makes \n, \r, |, comma, semicolon and hyphen ('-') all valid
separators between barcodes passed to inventory.
An additional grep removes empty barcodes. (Note: The grep looks for at
least one non-whitespace character. We could be more specific, but this
allows for more flexibility like testing with alphanumeric barcodes etc.)

Test plan:
Upload a barcode file that contains various allowed separators.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 5 Marcel de Rooy 2017-11-15 13:49:21 UTC
Decided to add the hyphen ('-') too as a separator. We need him also.
Comment 6 jwayway 2018-01-16 02:11:35 UTC
Created attachment 70535 [details] [review]
Bug 19585: Inventory: Allow additional separators in a barcode file

This patch makes \n, \r, |, comma, semicolon and hyphen ('-') all valid
separators between barcodes passed to inventory.
An additional grep removes empty barcodes. (Note: The grep looks for at
least one non-whitespace character. We could be more specific, but this
allows for more flexibility like testing with alphanumeric barcodes etc.)

Test plan:
Upload a barcode file that contains various allowed separators.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Jenny Way <jwayway@hotmail.com>
Comment 7 Josef Moravec 2018-01-17 16:04:34 UTC
Created attachment 70625 [details] [review]
Bug 19585: Inventory: Allow additional separators in a barcode file

This patch makes \n, \r, |, comma, semicolon and hyphen ('-') all valid
separators between barcodes passed to inventory.
An additional grep removes empty barcodes. (Note: The grep looks for at
least one non-whitespace character. We could be more specific, but this
allows for more flexibility like testing with alphanumeric barcodes etc.)

Test plan:
Upload a barcode file that contains various allowed separators.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Jenny Way <jwayway@hotmail.com>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 8 Jonathan Druart 2018-01-19 18:56:01 UTC
Pushed to master for 18.05, thanks to everybody involved!
Comment 9 Nick Clemens 2018-01-23 04:17:27 UTC
Awesome work all!

Enhancement, not backported for 17.11.
Comment 10 Frédéric Demians 2019-01-30 11:02:04 UTC
And if a library has barcodes with hyphens (-)?
Comment 11 Marcel de Rooy 2019-01-30 12:05:27 UTC
(In reply to Frédéric Demians from comment #10)
> And if a library has barcodes with hyphens (-)?

Yeah, we'd better move these chars in a pref ?
Comment 12 Frédéric Demians 2019-01-30 13:19:08 UTC
When playing with this issue, I was comparing inventory tool with Batch item modification tool. Both tools accept as input a barcodes file.

Barcodes with hyphen are OK in Batch item modification. This regex is used to split file lines:

  /[\r\n]/

Barcodes with hyphen are not OK in Inventory tool. This regex is used:

  /[\n\r,;|-]/

Wouldn't it make sense to have the same splitting logic in both places?
Comment 13 Katrin Fischer 2019-01-30 17:28:17 UTC
I agree, especially since hyphen and other characters are not as unlikey as one might think.
Comment 14 Katrin Fischer 2019-01-30 17:28:53 UTC
But we should move this to a new bug as this has already been pushed.
Comment 15 Marcel de Rooy 2019-05-29 13:54:16 UTC
(In reply to Frédéric Demians from comment #12)
> When playing with this issue, I was comparing inventory tool with Batch item
> modification tool. Both tools accept as input a barcodes file.
> 
> Barcodes with hyphen are OK in Batch item modification. This regex is used
> to split file lines:
> 
>   /[\r\n]/
> 
> Barcodes with hyphen are not OK in Inventory tool. This regex is used:
> 
>   /[\n\r,;|-]/
> 
> Wouldn't it make sense to have the same splitting logic in both places?

See bug 22996.