|
Description
Aleisha Amohia
2019-04-29 01:09:10 UTC
Created attachment 88991 [details] [review] Bug 22799: Avoiding batch item modification case sensitivity when entering barcodes Note: this fixes the bug that is present right now where batch item modification barcode matching is case sensitive, but if Koha barcodes ever do become case sensitive, this patch will need to be removed. To test: 1) Make/use an item with a barcode with letters in it. For testing purposes, I'll be using an item with the barcode 'abcde'. 2) Go to Tools -> Batch item modification and enter this barcode in the barcode list text input field, but change the case of one or some letters. (i.e. enter the barcode in the field as 'abCDE') 3) Click Continue 4) Notice that the next page shows an error 'the following barcodes were not found', but the item still shows underneath (was correctly fetched from database). 5) Create a text file and put the changed-case barcode in it (i.e. put the barcode 'abCDE' in the text file) 6) Go back to batch item modification and upload the text file as a barcode file 7) Notice same error as in Step 4 8) Apply patch and refresh page 9) Run through steps 1 to 6. There should no longer be an error and the barcodes should be found as expected. Sponsored-by: TBC Sponsored-by line is not final, but this can be tested Created attachment 88994 [details] [review] Bug 22799: Avoiding batch item modification case sensitivity when entering barcodes Note: this fixes the bug that is present right now where batch item modification barcode matching is case sensitive, but if Koha barcodes ever do become case sensitive, this patch will need to be removed. To test: 1) Make/use an item with a barcode with letters in it. For testing purposes, I'll be using an item with the barcode 'abcde'. 2) Go to Tools -> Batch item modification and enter this barcode in the barcode list text input field, but change the case of one or some letters. (i.e. enter the barcode in the field as 'abCDE') 3) Click Continue 4) Notice that the next page shows an error 'the following barcodes were not found', but the item still shows underneath (was correctly fetched from database). 5) Create a text file and put the changed-case barcode in it (i.e. put the barcode 'abCDE' in the text file) 6) Go back to batch item modification and upload the text file as a barcode file 7) Notice same error as in Step 4 8) Apply patch and refresh page 9) Run through steps 1 to 6. There should no longer be an error and the barcodes should be found as expected. Sponsored-by: South Taranaki District Council I don't think so the barcode should be case insensitive - if you refer to barcode as main identifier for items, you need to make exact matches - taking your examples barcodes "abcde" and "abcDE" will not have same graphics represantation - so they are different. (In reply to Josef Moravec from comment #4) > I don't think so the barcode should be case insensitive - if you refer to > barcode as main identifier for items, you need to make exact matches - > taking your examples barcodes "abcde" and "abcDE" will not have same > graphics represantation - so they are different. +1 (In reply to Josef Moravec from comment #4) > I don't think so the barcode should be case insensitive - if you refer to > barcode as main identifier for items, you need to make exact matches - > taking your examples barcodes "abcde" and "abcDE" will not have same > graphics represantation - so they are different. That's fine but that isn't currently the behaviour anywhere in Koha. Everything (except passwords) is case insensitive because the database does case insensitive searching. If you wanted to make the batch item modification tool case sensitive then this behaviour needs to be made consistent everywhere in Koha. For example, circulation - issuing with the barcode 'abcde' would check out the same item as issuing with the barcode 'abcDE'. I think while the behaviour in Koha is not case sensitive, this patch is still valid and fixes the current problem.
> If you wanted to make the batch item modification tool case sensitive then
> this behaviour needs to be made consistent everywhere in Koha. For example,
> circulation - issuing with the barcode 'abcde' would check out the same item
> as issuing with the barcode 'abcDE'.
In case this wasn't clear, I am describing the current behaviour for circulation. You can check out an item with the barcode 'abcde' or 'abcDE' currently.
Hi Aleisha, I get your point. But can't help to wonder if we might have created a possible problem with using CI on the database tables. If you compare ABC and abc with Perl, they will be different, if you do a WHERE barcode = "abc" it will return both. But... you can't add A and a to the database for the same reason (just tested). Not sure if this is ideal, but probably very hard to fix. And we might have lots of places where we don't do lc when comparing? Still the behaviour is definitely wrong here showing the error and the item. Yeah but if we change the behaviour here to be case sensitive, this will be the only place in the whole of koha that barcodes are case sensitive. In fact everything in the db is case insensitive, the only thing that isn't in the whole of Koha is passwords, because we store a hash. While I think a wider move to move everything to being case sensitive is worth exploring that is massively out of scope of making this script consistent with everything else in Koha. Lets just do that first, and then open another bug to switch things to case sensitive. Because that is a massive job, touching every part of Koha. Yes, I am sorry. I think the patch should go in, it was just something I hadn't thought about a lot before. Created attachment 89324 [details] [review] Bug 22799: Avoiding batch item modification case sensitivity when entering barcodes Note: this fixes the bug that is present right now where batch item modification barcode matching is case sensitive, but if Koha barcodes ever do become case sensitive, this patch will need to be removed. To test: 1) Make/use an item with a barcode with letters in it. For testing purposes, I'll be using an item with the barcode 'abcde'. 2) Go to Tools -> Batch item modification and enter this barcode in the barcode list text input field, but change the case of one or some letters. (i.e. enter the barcode in the field as 'abCDE') 3) Click Continue 4) Notice that the next page shows an error 'the following barcodes were not found', but the item still shows underneath (was correctly fetched from database). 5) Create a text file and put the changed-case barcode in it (i.e. put the barcode 'abCDE' in the text file) 6) Go back to batch item modification and upload the text file as a barcode file 7) Notice same error as in Step 4 8) Apply patch and refresh page 9) Run through steps 1 to 6. There should no longer be an error and the barcodes should be found as expected. Sponsored-by: South Taranaki District Council Signed-off-by: Maryse Simard <maryse.simard@inlibro.com> The display is definitely wrong indeed: https://snag.gy/UFqlg8.jpg Aleisha, the patch is wrong, try: 1. Use abCDE for barcode 2. Use the batch tool with abcde => Same wrong display You are using twice lc on the same element (from @barcodelist), so you are still comparing lc with another string which has not been lc'ed Better would be to use a map with a grep, let me know on IRC if you need help ;) Created attachment 91763 [details] [review] Bug 22799: Avoiding batch item modification case sensitivity when entering barcodes Note: this fixes the bug that is present right now where batch item modification barcode matching is case sensitive, but if Koha barcodes ever do become case sensitive, this patch will need to be removed. To test: 1) Make/use an item with a barcode with letters in it. For testing purposes, I'll be using an item with the barcode 'abcde'. 2) Go to Tools -> Batch item modification and enter this barcode in the barcode list text input field, but change the case of one or some letters. (i.e. enter the barcode in the field as 'abCDE') 3) Click Continue 4) Notice that the next page shows an error 'the following barcodes were not found', but the item still shows underneath (was correctly fetched from database). 5) Create a text file and put the changed-case barcode in it (i.e. put the barcode 'abCDE' in the text file) 6) Go back to batch item modification and upload the text file as a barcode file 7) Notice same error as in Step 4 8) Apply patch and refresh page 9) Run through steps 1 to 6. There should no longer be an error and the barcodes should be found as expected. Sponsored-by: South Taranaki District Council Signed-off-by: Maryse Simard <maryse.simard@inlibro.com> Created attachment 91816 [details] [review] Bug 22799: Avoiding batch item modification case sensitivity when entering barcodes Note: this fixes the bug that is present right now where batch item modification barcode matching is case sensitive, but if Koha barcodes ever do become case sensitive, this patch will need to be removed. To test: 1) Make/use an item with a barcode with letters in it. For testing purposes, I'll be using an item with the barcode 'abcde'. 2) Go to Tools -> Batch item modification and enter this barcode in the barcode list text input field, but change the case of one or some letters. (i.e. enter the barcode in the field as 'abCDE') 3) Click Continue 4) Notice that the next page shows an error 'the following barcodes were not found', but the item still shows underneath (was correctly fetched from database). 5) Create a text file and put the changed-case barcode in it (i.e. put the barcode 'abCDE' in the text file) 6) Go back to batch item modification and upload the text file as a barcode file 7) Notice same error as in Step 4 8) Apply patch and refresh page 9) Run through steps 1 to 6. There should no longer be an error and the barcodes should be found as expected. Sponsored-by: South Taranaki District Council Signed-off-by: Maryse Simard <maryse.simard@inlibro.com> Signed-off-by: Kyle Hall <kyle.m.hall@gmail.com> Created attachment 91817 [details] [review] Bug 22799: Avoiding batch item modification case sensitivity when entering barcodes Note: this fixes the bug that is present right now where batch item modification barcode matching is case sensitive, but if Koha barcodes ever do become case sensitive, this patch will need to be removed. To test: 1) Make/use an item with a barcode with letters in it. For testing purposes, I'll be using an item with the barcode 'abcde'. 2) Go to Tools -> Batch item modification and enter this barcode in the barcode list text input field, but change the case of one or some letters. (i.e. enter the barcode in the field as 'abCDE') 3) Click Continue 4) Notice that the next page shows an error 'the following barcodes were not found', but the item still shows underneath (was correctly fetched from database). 5) Create a text file and put the changed-case barcode in it (i.e. put the barcode 'abCDE' in the text file) 6) Go back to batch item modification and upload the text file as a barcode file 7) Notice same error as in Step 4 8) Apply patch and refresh page 9) Run through steps 1 to 6. There should no longer be an error and the barcodes should be found as expected. Sponsored-by: South Taranaki District Council Signed-off-by: Maryse Simard <maryse.simard@inlibro.com> Signed-off-by: Kyle Hall <kyle@bywatersolutions.com> Signed-off-by: Donna Bachowski <donna@bywatersolutions.com> Created attachment 92260 [details] [review] Bug 22799: Avoiding batch item modification case sensitivity when entering barcodes Note: this fixes the bug that is present right now where batch item modification barcode matching is case sensitive, but if Koha barcodes ever do become case sensitive, this patch will need to be removed. To test: 1) Make/use an item with a barcode with letters in it. For testing purposes, I'll be using an item with the barcode 'abcde'. 2) Go to Tools -> Batch item modification and enter this barcode in the barcode list text input field, but change the case of one or some letters. (i.e. enter the barcode in the field as 'abCDE') 3) Click Continue 4) Notice that the next page shows an error 'the following barcodes were not found', but the item still shows underneath (was correctly fetched from database). 5) Create a text file and put the changed-case barcode in it (i.e. put the barcode 'abCDE' in the text file) 6) Go back to batch item modification and upload the text file as a barcode file 7) Notice same error as in Step 4 8) Apply patch and refresh page 9) Run through steps 1 to 6. There should no longer be an error and the barcodes should be found as expected. Sponsored-by: South Taranaki District Council Signed-off-by: Maryse Simard <maryse.simard@inlibro.com> Signed-off-by: Kyle Hall <kyle@bywatersolutions.com> Signed-off-by: Donna Bachowski <donna@bywatersolutions.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Appreciate the theoretical objection. But since we search case insensitive almost everywhere, I agree that we should be pragmatic here. Just wondering if a few other scripts like inventory or so would suffer from the same thing? Nice work! Pushed to master for 19.11.00 Congratulations on your first Signoff Donna Bachowski, nice to have you on board the testing wagon :) Pushed to 19.05.x for 19.05.04 backported to 18.11.x for 18.11.10 |
