Summary: | Inventory tool barcodes should not be case sensitive | ||
---|---|---|---|
Product: | Koha | Reporter: | Lucas Gass (lukeg) <lucas> |
Component: | Tools | Assignee: | Lucas Gass (lukeg) <lucas> |
Status: | Pushed to main --- | QA Contact: | Marcel de Rooy <m.de.rooy> |
Severity: | normal | ||
Priority: | P5 - low | CC: | david, kco.ipad, laura, m.de.rooy |
Version: | Main | ||
Hardware: | All | ||
OS: | All | ||
Change sponsored?: | --- | Patch complexity: | Small patch |
Documentation contact: | Documentation submission: | ||
Text to go in the release notes: |
This fixes the inventory tool so that it ignores case sensitivity for barcodes, similar to other areas of Koha such as checking in and checking out items (for example, ABC123 and abc123 are treated the same).
|
Version(s) released in: |
25.05.00
|
Circulation function: | |||
Attachments: |
Bug 38452: Add lc() for case insensitivity
Bug 38452: Add lc() for case insensitivity Bug 38452: Use lc() for case insensitivity Bug 38452: Use lc() for case insensitivity Bug 38452: Use lc() for case insensitivity Bug 38452: Use lc() for case insensitivity Bug 38452: Use lc() for case insensitivity |
Description
Lucas Gass (lukeg)
2024-11-14 22:37:06 UTC
Created attachment 174638 [details] [review] Bug 38452: Add lc() for case insensitivity To test: 1. Create an item give it a barcode like 'abc123'; 2. Try creating a 2nd item with a barcode: 'ABC123', you get an error 'Error saving item: Barcode must be unique.' 3. Checkin the item 'ABC123', the 'abc123' item successfully checks in. 4. The case doesn't seem to matter elsewhere in Koha ( check-in, checkout, searching, batch tools ) 5. Try the inventory tool and use 'ABC123'. The barcode is not found. 6. APPLY PATCH, restart_all 7. Try the inventory tool again with both 'abc123' and 'ABC123', they should now both work. Created attachment 175031 [details] [review] Bug 38452: Add lc() for case insensitivity To test: 1. Create an item give it a barcode like 'abc123'; 2. Try creating a 2nd item with a barcode: 'ABC123', you get an error 'Error saving item: Barcode must be unique.' 3. Checkin the item 'ABC123', the 'abc123' item successfully checks in. 4. The case doesn't seem to matter elsewhere in Koha ( check-in, checkout, searching, batch tools ) 5. Try the inventory tool and use 'ABC123'. The barcode is not found. 6. APPLY PATCH, restart_all 7. Try the inventory tool again with both 'abc123' and 'ABC123', they should now both work. Signed-off-by: Kevin ONeil <kco.ipad@ipad.com> Laura O’Neil assisted with this sign off. Not sure if this change is enough. Look at following lines: my @items = Koha::Items->search( { barcode => { -in => \@barcodes } } )->as_list; my %items = map { $_->barcode => $_ } @items; foreach my $barcode (@barcodes) { my $item = $items{ $barcode }; $barcode that is looked up in the hash comes from @barcodes which now contains only lowercase barcodes, right? But the %items hash comes from a Koha::Items search. The SQL search is case insensitive and might return uppercase barcodes. Therefore $items{ABC123} does not exist, resulting in ERR_BARCODE, right? This shows that your test plan should also cover the ABC123 barcode case. Did you check if we have similar issues further on in the script? Please clarify. (In reply to Marcel de Rooy from comment #4) > $barcode that is looked up in the hash comes from @barcodes which now > contains only lowercase barcodes, right? > But the %items hash comes from a Koha::Items search. The SQL search is case > insensitive and might return uppercase barcodes. Therefore $items{ABC123} > does not exist, resulting in ERR_BARCODE, right? Correction: ABC123 is in items.barcode. So items{ABC123} exists. But the loop tries items{abc132}. (In reply to Marcel de Rooy from comment #5) > Correction: ABC123 is in items.barcode. So items{ABC123} exists. But the > loop tries items{abc132}. Hmm. abc123 obviously :) WARN tools/inventory.pl WARN tidiness The file is less tidy than before (bad/messy lines before: 170, now: 171) For changing one line :) (In reply to Marcel de Rooy from comment #4) > Not sure if this change is enough. > Look at following lines: > > my @items = Koha::Items->search( { barcode => { -in => \@barcodes } } > )->as_list; > my %items = map { $_->barcode => $_ } @items; > foreach my $barcode (@barcodes) { > my $item = $items{ $barcode }; > > $barcode that is looked up in the hash comes from @barcodes which now > contains only lowercase barcodes, right? > But the %items hash comes from a Koha::Items search. The SQL search is case > insensitive and might return uppercase barcodes. Therefore $items{ABC123} > does not exist, resulting in ERR_BARCODE, right? > > This shows that your test plan should also cover the ABC123 barcode case. > > Did you check if we have similar issues further on in the script? > > Please clarify. Thanks Marcel, that's a valid point. The lc() conversion should be after the SQL lookup. I'll change my patch. (In reply to Marcel de Rooy from comment #7) > WARN tools/inventory.pl > WARN tidiness > The file is less tidy than before (bad/messy lines before: > 170, now: 171) > > For changing one line :) In the past I haven't been tidying these lines but I can start. Created attachment 175451 [details] [review] Bug 38452: Use lc() for case insensitivity Created attachment 175452 [details] [review] Bug 38452: Use lc() for case insensitivity To test: 1. Create an item give it a barcode like 'abc123'; 2. Try creating a 2nd item with a barcode: 'ABC123', you get an error 'Error saving item: Barcode must be unique.' 3. Checkin the item 'ABC123', the 'abc123' item successfully checks in. 4. The case doesn't seem to matter elsewhere in Koha ( check-in, checkout, searching, batch tools ) 5. Try the inventory tool and use 'ABC123'. The barcode is not found. 6. APPLY PATCH, restart_all 7. Try the inventory tool again with both 'abc123' and 'ABC123', they should now both work. Created attachment 175455 [details] [review] Bug 38452: Use lc() for case insensitivity To test: 1. Create an item give it a barcode like 'abc123'; 2. Try creating a 2nd item with a barcode: 'ABC123', you get an error 'Error saving item: Barcode must be unique.' 3. Checkin the item 'ABC123', the 'abc123' item successfully checks in. 4. The case doesn't seem to matter elsewhere in Koha ( check-in, checkout, searching, batch tools ) 5. Try the inventory tool and use 'ABC123'. The barcode is not found. 6. APPLY PATCH, restart_all 7. Try the inventory tool again with both 'abc123' and 'ABC123', they should now both work. Created attachment 175477 [details] [review] Bug 38452: Use lc() for case insensitivity To test: 1. Create an item give it a barcode like 'abc123'; 2. Try creating a 2nd item with a barcode: 'ABC123', you get an error 'Error saving item: Barcode must be unique.' 3. Checkin the item 'ABC123', the 'abc123' item successfully checks in. 4. The case doesn't seem to matter elsewhere in Koha ( check-in, checkout, searching, batch tools ) 5. Try the inventory tool and use 'ABC123'. The barcode is not found. 6. APPLY PATCH, restart_all 7. Try the inventory tool again with both 'abc123' and 'ABC123', they should now both work. Signed-off-by: David Nind <david@davidnind.com> Created attachment 176310 [details] [review] Bug 38452: Use lc() for case insensitivity To test: 1. Create an item give it a barcode like 'abc123'; 2. Try creating a 2nd item with a barcode: 'ABC123', you get an error 'Error saving item: Barcode must be unique.' 3. Checkin the item 'ABC123', the 'abc123' item successfully checks in. 4. The case doesn't seem to matter elsewhere in Koha ( check-in, checkout, searching, batch tools ) 5. Try the inventory tool and use 'ABC123'. The barcode is not found. 6. APPLY PATCH, restart_all 7. Try the inventory tool again with both 'abc123' and 'ABC123', they should now both work. Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Pushed for 25.05! Well done everyone, thank you! |