I say this because it doesn't appear like we honor case sensitivity for barcodes elsewhere. So, to recreate: 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. If Koha treats 'ABC123' and 'abc123' the same when creating an item, it should probably treat them the same when using the inventory tool.
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!
Nice work everyone! Pushed to 24.11.x for 24.11.01
Nice work everyone! Pushed to 24.05.x for 24.05.08
I prefer not to backport to "very-old-stable" 23.11.x to avoid behavior change.
This feels like a bug fix and should not affect the manual. However, should we mention that barcodes are case insensitive somewhere?
(In reply to Caroline Cyr La Rose from comment #19) > This feels like a bug fix and should not affect the manual. However, should > we mention that barcodes are case insensitive somewhere? I think it would be a good idea. Maybe in the cataloguing guide for 952$p/995$f? https://koha-community.org/manual/latest/en/html/cataloging.html#p-995-f-barcode
(In reply to David Nind from comment #20) > (In reply to Caroline Cyr La Rose from comment #19) > > This feels like a bug fix and should not affect the manual. However, should > > we mention that barcodes are case insensitive somewhere? > > I think it would be a good idea. > > Maybe in the cataloguing guide for 952$p/995$f? > https://koha-community.org/manual/latest/en/html/cataloging.html#p-995-f- > barcode Thanks David! Good idea for the section. I added the note there. https://gitlab.com/koha-community/koha-manual/-/merge_requests/1032