Bug 29114

Summary: Can not add barcodes with whitespaces at the beginning to the list
Product: Koha Reporter: Andrii Veremeienko <andriiverem>
Component: ListsAssignee: Andrii Veremeienko <andriiverem>
Status: CLOSED FIXED QA Contact: Martin Renvoize <martin.renvoize>
Severity: enhancement    
Priority: P5 - low CC: arthur.suzuki, david, fridolin.somers, jonathan.druart, kyle, lucas, m.de.rooy, martin.renvoize, nick, nugged, victor
Version: Main   
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26351
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22204
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30409
Change sponsored?: --- Patch complexity: Trivial patch
Documentation contact: Documentation submission:
Text to go in the release notes:
This fixes an issue where barcodes with white spaces at the beginning could not be added to a list.
Version(s) released in:
22.11.00, 22.05.05, 21.11.12
Bug Depends on: 30409    
Bug Blocks:    
Attachments: Bug 29114: Trim whitespace before the barcode
Bug 29114: Trim whitespace before the barcode
Bug 29114: Trim whitespace before the barcode
Bug 29114: Trim whitespace before the barcode
Bug 29114: (QA follow-up) Carriage return is whitespace too
Bug 29114: Trim whitespace before the barcode
Bug 29114: Trim whitespace before the barcode
Bug 29114: Trim whitespace before the barcode
Bug 29114: Trim whitespace before the barcode
Bug 29114: Trim whitespace before the barcode
Bug 29114: Trim whitespace before the barcode
Bug 29114: Trim whitespace before the barcode

Description Andrii Veremeienko 2021-09-26 15:00:50 UTC
When you try to add a barcode that has any whitespaces before it, it is not recognized as a valid item.

Steps to reproduce:
1. Go to the koha/virtualshelves/shelves.pl;
2. Add new list or edit existing one;
3. Start adding new items;
4. In the "Barcode" field add barcode(s) with whitespaces before them;
5. Observe that it wasn't added as the barcode isn't recognized because of whitespaces.
Comment 1 Andrii Veremeienko 2021-09-27 17:14:46 UTC Comment hidden (obsolete)
Comment 2 Fridolin Somers 2021-09-28 06:46:56 UTC
A agree 100%, but I think we should trim also trailing whitespaces, like in check-out :
https://git.koha-community.org/Koha-community/Koha/src/commit/870e3e8e161f6da998c1deae216d7fdf0d0490e7/circ/circulation.pl#L159
Comment 3 Andrii Veremeienko 2021-10-05 21:16:01 UTC Comment hidden (obsolete)
Comment 4 David Nind 2021-10-05 23:47:39 UTC Comment hidden (obsolete)
Comment 5 Fridolin Somers 2021-10-06 02:52:16 UTC
(In reply to Fridolin Somers from comment #2)
> A agree 100%, but I think we should trim also trailing whitespaces, like in
> check-out :
> https://git.koha-community.org/Koha-community/Koha/src/commit/
> 870e3e8e161f6da998c1deae216d7fdf0d0490e7/circ/circulation.pl#L159

Super, it is in patch :
+                    $barcode =~ s/^\s*|\s*$//g; # remove leading/trailing whitespace

We should add "trailing" to bug description, release notes and commit message.
Comment 6 Fridolin Somers 2021-10-06 02:53:09 UTC
PS : 
In the future, we may make this trim inside 
  Koha::Items->find({barcode => $barcode})
Comment 7 Marcel de Rooy 2021-10-06 13:44:19 UTC Comment hidden (obsolete)
Comment 8 Marcel de Rooy 2021-10-06 13:44:23 UTC Comment hidden (obsolete)
Comment 9 Marcel de Rooy 2021-10-06 13:45:03 UTC
(In reply to Fridolin Somers from comment #6)
> PS : 
> In the future, we may make this trim inside 
>   Koha::Items->find({barcode => $barcode})

Not sure if we should btw. Probably not.
Comment 10 Jonathan Druart 2021-10-07 09:00:52 UTC
Shouldn't we actually call C4::Circulation::barcodedecode here?
Comment 11 Jonathan Druart 2021-10-08 10:18:05 UTC
(In reply to Jonathan Druart from comment #10)
> Shouldn't we actually call C4::Circulation::barcodedecode here?

Kyle, Marcel?
Comment 12 Marcel de Rooy 2021-10-11 12:45:49 UTC
(In reply to Jonathan Druart from comment #10)
> Shouldn't we actually call C4::Circulation::barcodedecode here?

Fine with me.
Comment 13 Andrii Veremeienko 2021-10-26 06:29:35 UTC Comment hidden (obsolete)
Comment 14 Andrii Veremeienko 2021-10-26 06:56:23 UTC Comment hidden (obsolete)
Comment 15 Andrii Veremeienko 2021-10-26 08:13:58 UTC Comment hidden (obsolete)
Comment 16 Owen Leonard 2021-10-28 18:34:16 UTC
(In reply to Marcel de Rooy from comment #8)
> Created attachment 125816 [details] [review] [review]
> Bug 29114: (QA follow-up) Carriage return is whitespace too
> 
> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

I've marked this as obsolete. Please correct me if I'm wrong.
Comment 17 Owen Leonard 2021-10-28 18:37:08 UTC
Created attachment 127075 [details] [review]
Bug 29114: Trim whitespace before the barcode

The solution was to add regexp that trims all whitespaces.

How to test:
1. Go to the koha/virtualshelves/shelves.pl;
2. Add new list or edit existing one;
3. Start adding new items;
4. In the "Barcode" field add barcode(s) with whitespaces before them;
5. Observe that it wasn't added as the barcode isn't recognized because
   of whitespaces;
6. Apply the patch;
7. Repeat step 4;
8. Observe that the item was successfully added;

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 18 Marcel de Rooy 2021-10-29 06:21:14 UTC
(In reply to Owen Leonard from comment #16)
> (In reply to Marcel de Rooy from comment #8)
> > Created attachment 125816 [details] [review] [review] [review]
> > Bug 29114: (QA follow-up) Carriage return is whitespace too
> > 
> > Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
> 
> I've marked this as obsolete. Please correct me if I'm wrong.

Just a general remark: Marking patches of marcelr as obsolete is wrong on itself.
Comment 19 Marcel de Rooy 2021-10-29 06:22:37 UTC
The barcodedecode does the trimming, please remove the regexes.
Comment 20 Andrii Veremeienko 2021-10-29 13:46:54 UTC
Created attachment 127098 [details] [review]
Bug 29114: Trim whitespace before the barcode

The solution was to add regexp that trims all whitespaces.

How to test:
1. Go to the koha/virtualshelves/shelves.pl;
2. Add new list or edit existing one;
3. Start adding new items;
4. In the "Barcode" field add barcode(s) with whitespaces before them;
5. Observe that it wasn't added as the barcode isn't recognized because of whitespaces;
6. Apply the patch;
7. Repeat step 4;
8. Observe that the item was successfully added;
Comment 21 Andrii Veremeienko 2021-10-29 14:45:26 UTC Comment hidden (obsolete)
Comment 22 Fridolin Somers 2021-11-12 07:45:41 UTC
(In reply to Marcel de Rooy from comment #19)
> The barcodedecode does the trimming, please remove the regexes.

I'm testing but this does not work.
I don't see a trim in barcodedecode().
Unless there is itemBarcodeInputFilter = whitespace, but we should always trim no ?
Comment 23 David Nind 2022-07-13 21:48:45 UTC
Created attachment 137687 [details] [review]
Bug 29114: Trim whitespace before the barcode

The solution was to add regexp that trims all whitespaces.

How to test:
1. Go to the koha/virtualshelves/shelves.pl;
2. Add new list or edit existing one;
3. Start adding new items;
4. In the "Barcode" field add barcode(s) with whitespaces before them;
5. Observe that it wasn't added as the barcode isn't recognized because of whitespaces;
6. Apply the patch;
7. Repeat step 4;
8. Observe that the item was successfully added;

Signed-off-by: David Nind <david@davidnind.com>
Comment 24 David Nind 2022-07-13 21:51:26 UTC
Testing notes (using koha-testing-docker):

- Patch still applies.

- Things work as per the test plan.
Comment 25 Martin Renvoize 2022-07-15 15:13:13 UTC
Created attachment 137773 [details] [review]
Bug 29114: Trim whitespace before the barcode

The solution was to add regexp that trims all whitespaces.

How to test:
1. Go to the koha/virtualshelves/shelves.pl;
2. Add new list or edit existing one;
3. Start adding new items;
4. In the "Barcode" field add barcode(s) with whitespaces before them;
5. Observe that it wasn't added as the barcode isn't recognized because of whitespaces;
6. Apply the patch;
7. Repeat step 4;
8. Observe that the item was successfully added;

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 26 Martin Renvoize 2022-07-15 15:13:42 UTC
Great work, thanks Andrii..

Passing QA
Comment 27 Tomás Cohen Arazi 2022-07-18 18:16:26 UTC
Pushed to master for 22.11.

Nice work everyone, thanks!
Comment 28 Lucas Gass 2022-08-23 19:30:06 UTC
Backported to 22.05.x for 22.05.05
Comment 29 Arthur Suzuki 2022-08-30 09:24:34 UTC
thx pushed to 21.11.x for 21.11.12
Comment 30 Victor Grousset/tuxayo 2022-09-14 19:17:47 UTC
Not backported to oldoldstable (21.05.x). Feel free to ask if it's needed.

Nothing to document, marking resolved.
Comment 31 Nick Clemens 2022-11-28 19:43:57 UTC
(In reply to Arthur Suzuki from comment #29)
> thx pushed to 21.11.x for 21.11.12

This is causing a regression in 21.11.x - missing dependency bug 30409