Bug 22799 - Batch item modification is case sensitive
Summary: Batch item modification is case sensitive
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Tools (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low minor
Assignee: Aleisha Amohia
QA Contact: Marcel de Rooy
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2019-04-29 01:09 UTC by Aleisha Amohia
Modified: 2020-11-30 21:44 UTC (History)
8 users (show)

See Also:
Change sponsored?: Sponsored
Patch complexity: Small patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
19.11.00,19.05.04
Circulation function:


Attachments
Bug 22799: Avoiding batch item modification case sensitivity when entering barcodes (2.82 KB, patch)
2019-04-29 01:35 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 22799: Avoiding batch item modification case sensitivity when entering barcodes (2.85 KB, patch)
2019-04-29 02:17 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 22799: Avoiding batch item modification case sensitivity when entering barcodes (2.91 KB, patch)
2019-05-03 19:04 UTC, Maryse Simard
Details | Diff | Splinter Review
Bug 22799: Avoiding batch item modification case sensitivity when entering barcodes (2.92 KB, patch)
2019-07-24 05:18 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 22799: Avoiding batch item modification case sensitivity when entering barcodes (2.97 KB, patch)
2019-07-26 15:01 UTC, ByWater Sandboxes
Details | Diff | Splinter Review
Bug 22799: Avoiding batch item modification case sensitivity when entering barcodes (3.05 KB, patch)
2019-07-26 15:05 UTC, Kyle M Hall (khall)
Details | Diff | Splinter Review
Bug 22799: Avoiding batch item modification case sensitivity when entering barcodes (3.13 KB, patch)
2019-08-16 07:43 UTC, Marcel de Rooy
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Aleisha Amohia 2019-04-29 01:09:10 UTC
The tool is able to search for barcodes in the database fine, but the code then tries to match barcodes to check if retrieved with barcodes requested. This part is case sensitive and it looks like the fetch failed if the case in the search isn't the same as the case in the DB.

To reproduce:
1) Make/use an item with a barcode with letters in it
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. if the barcode is 'abcde', 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).
Comment 1 Aleisha Amohia 2019-04-29 01:35:56 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
Comment 2 Aleisha Amohia 2019-04-29 01:36:38 UTC
Sponsored-by line is not final, but this can be tested
Comment 3 Aleisha Amohia 2019-04-29 02:17:35 UTC
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
Comment 4 Josef Moravec 2019-04-29 11:58:18 UTC
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.
Comment 5 Katrin Fischer 2019-04-29 12:24:29 UTC
(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
Comment 6 Aleisha Amohia 2019-04-30 23:50:48 UTC
(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.
Comment 7 Aleisha Amohia 2019-04-30 23:53:39 UTC
> 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.
Comment 8 Katrin Fischer 2019-05-01 08:50:34 UTC
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.
Comment 9 Chris Cormack 2019-05-02 23:17:12 UTC
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.
Comment 10 Katrin Fischer 2019-05-03 05:30:18 UTC
Yes, I am sorry. I think the patch should go in, it was just something I hadn't thought about a lot before.
Comment 11 Maryse Simard 2019-05-03 19:04:07 UTC
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>
Comment 12 Jonathan Druart 2019-05-06 16:31:15 UTC
The display is definitely wrong indeed: https://snag.gy/UFqlg8.jpg
Comment 13 Jonathan Druart 2019-05-06 16:40:12 UTC
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 ;)
Comment 14 Aleisha Amohia 2019-07-24 05:18:56 UTC
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>
Comment 15 ByWater Sandboxes 2019-07-26 15:01:16 UTC
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>
Comment 16 Kyle M Hall (khall) 2019-07-26 15:05:37 UTC
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>
Comment 17 Marcel de Rooy 2019-08-16 07:43:47 UTC
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>
Comment 18 Marcel de Rooy 2019-08-16 07:47:23 UTC
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?
Comment 19 Martin Renvoize (ashimema) 2019-08-16 11:21:13 UTC
Nice work!

Pushed to master for 19.11.00
Comment 20 Martin Renvoize (ashimema) 2019-08-16 11:24:42 UTC
Congratulations on your first Signoff Donna Bachowski, nice to have you on board the testing wagon :)
Comment 21 Fridolin Somers 2019-09-05 14:45:27 UTC
Pushed to 19.05.x for 19.05.04
Comment 22 Lucas Gass (lukeg) 2019-09-21 15:09:43 UTC
backported to 18.11.x for 18.11.10