We should run any inputted barcode through barcodedecode before passing it to any subroutines. This was missed during the initial development of bug 26351.
Created attachment 138202 [details] [review] Bug 31252: Advanced search in staff interface should call barcodedecode if the search index is a barcode We should run any inputted barcode through barcodedecode before passing it to any subroutines. This was missed during the initial development of bug 26351. Test Plan: 1) Set itemBarcodeInputFilter to "Remove spaces from" 2) Browse to the staff side advanced search 3) Run a barcode search for "MY TEST" 4) Note the message returned is: No results match your search for 'bc,wrdl: "MY TEST"'. 5) Apply this patch 6) Restart all the things! 7) Repeat your search 8) Note the message returned is: No results match your search for 'bc,wrdl: "MYTEST"'. 9) Note the lack of the space between MY and TEST!
I like this, but I was a little worried it could create problems with the other barcode filters. So what happens if the barcode is not scanned but for example copied from a report or from the items table (already being converted?) Are we sure the existing filters will not "double-convert" those? So I have tested (looking at the test for the barcode filters): libsuite8 Unconverted: b000126 searches for: Converted: IMS-b-126 searches for: CPL-IMS-b--126 --> unwanted conversion This is odd, because the test covers this and they should actually be the same? The additional space should not upset Koha, so we can probably ignore this. Also tested CueCat, T, EAN etc. and it looks good.
Looking at this again... the libsuite8 conversion is an odd one: IMS-b-126 searches for: CPL-IMS-b--126 => Where is the branchcode suddenly coming from? This should not happen, IMS-B-125 should stay identical on conversion? t/Circulation_barcodedecode. 'libsuite8' => ['b000126', 'b12', 'B0126', 'IMS-B-126', 'ims-b-126','CD0000024','00123','11998'], 'libsuite8' => ['IMS-b-126', 'IMS-b-12', 'IMS-B-126', 'IMS-B-126', 'ims-b-126','IMS-CD-24','IMS-b-123','IMS-b-11998'], Can you have a look please?
I did some more research and it looks like we already use the filter like that in a lot of places. If there is an issue with libsuite8 we should probably move it to a separate bug.
I just ran into a case where the library has barcodes with trailing spaces from a batch of faulty barcode labels. Could it be an option to add the ItemBarcodeInputFilter into the item edit form too? (just wondering if you have an opinion about this, totally a separate bug)
Created attachment 140604 [details] [review] Bug 31252: Advanced search in staff interface should call barcodedecode if the search index is a barcode We should run any inputted barcode through barcodedecode before passing it to any subroutines. This was missed during the initial development of bug 26351. Test Plan: 1) Set itemBarcodeInputFilter to "Remove spaces from" 2) Browse to the staff side advanced search 3) Run a barcode search for "MY TEST" 4) Note the message returned is: No results match your search for 'bc,wrdl: "MY TEST"'. 5) Apply this patch 6) Restart all the things! 7) Repeat your search 8) Note the message returned is: No results match your search for 'bc,wrdl: "MYTEST"'. 9) Note the lack of the space between MY and TEST! Signed-off-by: David Nind <david@davidnind.com>
Patch still applies and have signed off, as everything works as per the test plan. Have changed status to 'In Discussion' as I think Katrin's comments need addressing.
Katrin, I think both the issue with the libsuite8 filter, and adding an option to add the ItemBarcodeInputFilter into the item edit form deserve their own bug reports! (In reply to Katrin Fischer from comment #3) > Looking at this again... the libsuite8 conversion is an odd one: > > IMS-b-126 searches for: CPL-IMS-b--126 => Where is the branchcode suddenly > coming from? > > This should not happen, IMS-B-125 should stay identical on conversion? > > t/Circulation_barcodedecode. > 'libsuite8' => ['b000126', 'b12', 'B0126', 'IMS-B-126', > 'ims-b-126','CD0000024','00123','11998'], > 'libsuite8' => ['IMS-b-126', 'IMS-b-12', 'IMS-B-126', 'IMS-B-126', > 'ims-b-126','IMS-CD-24','IMS-b-123','IMS-b-11998'], > > Can you have a look please? That's out of scope
Sorry, maybe too many comments from me here :) You'll have to read them all: (In reply to Katrin Fischer from comment #5) > I just ran into a case where the library has barcodes with trailing spaces > from a batch of faulty barcode labels. Could it be an option to add the > ItemBarcodeInputFilter into the item edit form too? (just wondering if you > have an opinion about this, totally a separate bug) Yep, totally a separate bug ;) (In reply to Katrin Fischer from comment #4) > I did some more research and it looks like we already use the filter like > that in a lot of places. If there is an issue with libsuite8 we should > probably move it to a separate bug. Again agreed ;) I am ok to move on with this patch set.
Thanks Kyle and Katrin!
Created attachment 141498 [details] [review] Bug 31252: Advanced search in staff interface should call barcodedecode if the search index is a barcode We should run any inputted barcode through barcodedecode before passing it to any subroutines. This was missed during the initial development of bug 26351. Test Plan: 1) Set itemBarcodeInputFilter to "Remove spaces from" 2) Browse to the staff side advanced search 3) Run a barcode search for "MY TEST" 4) Note the message returned is: No results match your search for 'bc,wrdl: "MY TEST"'. 5) Apply this patch 6) Restart all the things! 7) Repeat your search 8) Note the message returned is: No results match your search for 'bc,wrdl: "MYTEST"'. 9) Note the lack of the space between MY and TEST! Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Pushed to master for 22.11. Nice work everyone, thanks!
Missing 22.05.x dependencies, no backport