Items barcode has a UNIQ constraint in database, but when there are trailling spaces its is not considered has a duplicate. Since we trim spaces the barcode in input of checkout/checkin whe should also trim in barcodedecode() so in any barcode creation/edition.
Created attachment 132693 [details] [review] Bug 30409: barcodedecode() should always trim barcode Barcode is trimmed of leading/trailing whitespaces in many instances before the barcodedecode sub was called. This patch instead makes that barcodedecode sub is going to trim it itself and removes unnecessary, and repetitive code that was used before barcodedecode was called.
Created attachment 132781 [details] [review] Bug 30409: barcodedecode() should always trim barcode Barcode is trimmed of leading/trailing whitespaces in many instances before the barcodedecode sub was called. This patch instead makes that barcodedecode sub is going to trim it itself and removes unnecessary, and repetitive code that was used before barcodedecode was called. With this addition, filet "whitespace" was removed from barcodedecode sub, as it's going to be always trimmed from now on anyway. Also changed tests to fit the way barcodedecode works currently.
Created attachment 132881 [details] [review] Bug 30409: barcodedecode() should always trim barcode Barcode is trimmed of leading/trailing whitespaces in many instances before the barcodedecode sub was called. This patch instead makes that barcodedecode sub is going to trim it itself and removes unnecessary, and repetitive code that was used before barcodedecode was called.
Created attachment 132884 [details] [review] Bug 30409: barcodedecode() should always trim barcode Barcode is trimmed of leading/trailing whitespaces in many instances before the barcodedecode sub was called. This patch instead makes that barcodedecode sub is going to trim it itself and removes unnecessary, and repetitive code that was used before barcodedecode was called.
Created attachment 132885 [details] [review] Bug 30409: barcodedecode() should always trim barcode Barcode is trimmed of leading/trailing whitespaces in many instances before the barcodedecode sub was called. This patch instead makes that barcodedecode sub is going to trim it itself and removes unnecessary, and repetitive code that was used before barcodedecode was called.
Could you please add a test plan?
*** Bug 22204 has been marked as a duplicate of this bug. ***
Petro, please prioritize.
Created attachment 135258 [details] [review] Bug 30409: barcodedecode() should always trim barcode Barcode is trimmed of leading/trailing whitespaces in many instances before the barcodedecode sub was called. This patch instead makes that barcodedecode sub is going to trim it itself and removes unnecessary, and repetitive code that was used before barcodedecode was called. Steps to test: 1. Edit item with any barcode, add a bunch of whitespaces at the start and at the bottom of it. Save the item. Ensure that this action ruins the barcode and ensure that the spaces are still there by editing the same item again. 2. Apply the patch. 3. Edit the same item again in the same fashion. Ensure that now all whitespaces are getting trimmed and it doesn't affect the barcode in any negative way.
1) Add a test case to t/Circulation_barcodedecode.t 2) In addition the cases you handled already, please remove the duplicate whitespace trimming also from - course_reserves/add_items.pl - circ/branchtransfers.pl (grep for # FIXME: barcodeInputFilter and add barcodedecode call) - circ/returns.pl (there's two occurences) - circ/renew.pl (there's two occurences)
Created attachment 135448 [details] [review] Bug 30409: barcodedecode() should always trim barcode Barcode is trimmed of leading/trailing whitespaces in many instances before the barcodedecode sub was called. This patch instead makes that barcodedecode sub is going to trim it itself and removes unnecessary, and repetitive code that was used before barcodedecode was called. Steps to test: 1. Edit item with any barcode, add a bunch of whitespaces at the start and at the bottom of it. Save the item. Ensure that this action ruins the barcode and ensure that the spaces are still there by editing the same item again. 2. Apply the patch. 3. Edit the same item again in the same fashion. Ensure that now all whitespaces are getting trimmed and it doesn't affect the barcode in any negative way.
Created attachment 135453 [details] [review] Bug 30409: barcodedecode() should always trim barcode Barcode is trimmed of leading/trailing whitespaces in many instances before the barcodedecode sub was called. This patch instead makes that barcodedecode sub is going to trim it itself and removes unnecessary, and repetitive code that was used before barcodedecode was called. Steps to test: 1. Edit item with any barcode, add a bunch of whitespaces at the start and at the bottom of it. Save the item. Ensure that this action ruins the barcode and ensure that the spaces are still there by editing the same item again. 2. Apply the patch. 3. Edit the same item again in the same fashion. Ensure that now all whitespaces are getting trimmed and it doesn't affect the barcode in any negative way. Signed-off-by: David Nind <david@davidnind.com>
After signing off, I checked again and have changed to Failed QA - as trimming spaces on the check out, check in, renew, course reserves forms doesn't seem to work. It certainly works when editing the item and saving it - the spaces before and after are removed. However, after applying the patch, but not editing the item where spaces before and after the barcode were added in step 1, I tried: . Circulation > Check out: doesn't work - barcode not found . Circulation > Check in: doesn't work - barcode not found . Circulation > Renew: doesn't work - barcode not found . Course reserves > Add reserves for [course name]: doesn't work - barcode not found . Search the catalog: works (finds record with item barcode, as it did before the patch was applied) I hope I have understood what the patch is supposed to do correctly! Notes: The OPAC continue to work as expected (search with spaces before and after the barcode, and advanced search).
If this is just supposed to fix the item edit form, then please change the status, and I'll sign off again.
Hey David, could you please check this again? How I reproduced your problems with the patch applied: - Circulation > Check in: I used " 39999000008098 " and it did work, it trimmed all trailing whitespaces and found the correct item with the barcode I was looking for, - Circulation > Check out: I used " 39999000008098 " again and again it did work, it trimmed all trailing whitespaces and found the correct item. - Circulation > Renew: I used " 39999000008098 " again, worked as well. - Course reserves > Add reserves for [course name]: used " 39999000008098 " input and it worked here as well. I also tried different barcodes and they all work while trailing whitespaces were properly removed.
Hi Peter. I was too slow retesting, as the patch no longer applies 8-(... git bz apply 30409 Bug 30409 - barcodedecode() should always trim barcode 135453 - Bug 30409: barcodedecode() should always trim barcode Apply? [(y)es, (n)o, (i)nteractive] y Applying: Bug 30409: barcodedecode() should always trim barcode .git/rebase-apply/patch:18: trailing whitespace. For proper functioning of this filter, calling the function on the .git/rebase-apply/patch:22: trailing whitespace. The optional $filter argument is to allow for testing or explicit .git/rebase-apply/patch:23: trailing whitespace. behavior that ignores the System Pref. Valid values are the same as the warning: 3 lines add whitespace errors. Using index info to reconstruct a base tree... M course_reserves/add_items.pl .git/rebase-apply/patch:18: trailing whitespace. For proper functioning of this filter, calling the function on the .git/rebase-apply/patch:22: trailing whitespace. The optional $filter argument is to allow for testing or explicit .git/rebase-apply/patch:23: trailing whitespace. behavior that ignores the System Pref. Valid values are the same as the warning: 3 lines add whitespace errors. Falling back to patching base and 3-way merge... Auto-merging course_reserves/add_items.pl CONFLICT (content): Merge conflict in course_reserves/add_items.pl error: Failed to merge in the changes. Patch failed at 0001 Bug 30409: barcodedecode() should always trim barcode
Created attachment 135705 [details] [review] Bug 30409: barcodedecode() should always trim barcode Barcode is trimmed of leading/trailing whitespaces in many instances before the barcodedecode sub was called. This patch instead makes that barcodedecode sub is going to trim it itself and removes unnecessary, and repetitive code that was used before barcodedecode was called. Steps to test: 1. Edit item with any barcode, add a bunch of whitespaces at the start and at the bottom of it. Save the item. Ensure that this action ruins the barcode and ensure that the spaces are still there by editing the same item again. 2. Apply the patch. 3. Edit the same item again in the same fashion. Ensure that now all whitespaces are getting trimmed and it doesn't affect the barcode in any negative way. Signed-off-by: David Nind <david@davidnind.com>
Rebased on current master
Created attachment 135731 [details] [review] Bug 30409: barcodedecode() should always trim barcode Barcode is trimmed of leading/trailing whitespaces in many instances before the barcodedecode sub was called. This patch instead makes that barcodedecode sub is going to trim it itself and removes unnecessary, and repetitive code that was used before barcodedecode was called. Steps to test: 1. Edit item with any barcode, add a bunch of whitespaces at the start and at the bottom of it. Save the item. Ensure that this action ruins the barcode and ensure that the spaces are still there by editing the same item again. 2. Apply the patch. 3. Edit the same item again in the same fashion. Ensure that now all whitespaces are getting trimmed and it doesn't affect the barcode in any negative way. Signed-off-by: David Nind <david@davidnind.com>
(In reply to Peter Vashchuk from comment #18) > Rebased on current master Thanks Peter. Have retested and changed to signed off. Things now seem to work as expected. I think what I was getting confused with, was that if a barcode in the actual item had spaces before and after, then this was not found on input forms. This was the case both before and after the patch was applied: 1. Edit a barcode for an item so that it has spaces before and after. 2. Check input forms such as Check out - item not found. 3. After patch applied, item still not found, for example Check out input form. 4. Edit item, save - spaces before and after removed and now found on input forms even if extra spaces added on input forms. Other testing notes: 1. Card barcodes: worked before and after patch for Check out, Search patrons, and Patron lists. 2. Item barcodes: worked before and after patch applied: . From header block: Check out, Check in, Renew . Add reserves . Search (keyword) . From Circulation screen: Check out, Check in, Renew, Transfers, Overdue report . Batch checkout . Tools: Batch item deletion, Batch item modification 3. Item barcodes: didn't work before the patch, but now works after patch applied: . Tools: Barcode image generator, Rotating collections, Inventory 4. Item barcodes: didn't work before the patch, and don't work after patch - possible future enhancements: . Lists . Quick spine label creator . Item search - barcode is
Created attachment 135961 [details] [review] Bug 30409: barcodedecode() should always trim barcode Barcode is trimmed of leading/trailing whitespaces in many instances before the barcodedecode sub was called. This patch instead makes that barcodedecode sub is going to trim it itself and removes unnecessary, and repetitive code that was used before barcodedecode was called. Steps to test: 1. Edit item with any barcode, add a bunch of whitespaces at the start and at the bottom of it. Save the item. Ensure that this action ruins the barcode and ensure that the spaces are still there by editing the same item again. 2. Apply the patch. 3. Edit the same item again in the same fashion. Ensure that now all whitespaces are getting trimmed and it doesn't affect the barcode in any negative way. Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Joonas Kylmälä <joonas.kylmala@iki.fi>
Created attachment 135962 [details] [review] Bug 30409: (QA follow-up) Fix grammar in syspref help text Signed-off-by: Joonas Kylmälä <joonas.kylmala@iki.fi>
Passing QA. I noticed a small grammar error in the syspref text so added a follow-up for that.
Pushed to master for 22.11. Nice work everyone, thanks!
Created attachment 136067 [details] [review] Bug 30409: (QA follow-up) Avoid uninitialized variable warnings Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Backported to 22.05.x for 22.05.03
This is missing from 21.11.x - its dependent bug was pushed, however, which is causing failures in libraries adding items to lists - please backport ASAP
*** Bug 14335 has been marked as a duplicate of this bug. ***