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.
Created attachment 125369 [details] [review] Bug 29114: Trim whitespace before the barcode The solution was to add regexp which trimming 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;
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
Created attachment 125752 [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;
Created attachment 125753 [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>
(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.
PS : In the future, we may make this trim inside Koha::Items->find({barcode => $barcode})
Created attachment 125815 [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: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 125816 [details] [review] Bug 29114: (QA follow-up) Carriage return is whitespace too Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
(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.
Shouldn't we actually call C4::Circulation::barcodedecode here?
(In reply to Jonathan Druart from comment #10) > Shouldn't we actually call C4::Circulation::barcodedecode here? Kyle, Marcel?
(In reply to Jonathan Druart from comment #10) > Shouldn't we actually call C4::Circulation::barcodedecode here? Fine with me.
Created attachment 126867 [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;
Created attachment 126868 [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;
Created attachment 126896 [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;
(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.
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>
(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.
The barcodedecode does the trimming, please remove the regexes.
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;
(In reply to Marcel de Rooy from comment #19) > The barcodedecode does the trimming, please remove the regexes. That's a great catch! I looked through the code and saw lots of places where barcodedecode using with regexp. So, I created a new ticket for it. https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29375 Thank you!
(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 ?
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>
Testing notes (using koha-testing-docker): - Patch still applies. - Things work as per the test plan.
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>
Great work, thanks Andrii.. Passing QA
Pushed to master for 22.11. Nice work everyone, thanks!
Backported to 22.05.x for 22.05.05
thx pushed to 21.11.x for 21.11.12
Not backported to oldoldstable (21.05.x). Feel free to ask if it's needed. Nothing to document, marking resolved.
(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