Bug 38452

Summary: Inventory tool barcodes should not be case sensitive
Product: Koha Reporter: Lucas Gass (lukeg) <lucas>
Component: ToolsAssignee: 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
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.
Comment 1 Lucas Gass (lukeg) 2024-11-15 23:05:32 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.
Comment 2 ByWater Sandboxes 2024-11-29 16:59:03 UTC
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>
Comment 3 Kevin ONeil 2024-11-29 17:00:49 UTC
Laura O’Neil assisted with this sign off.
Comment 4 Marcel de Rooy 2024-12-13 10:05:19 UTC
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.
Comment 5 Marcel de Rooy 2024-12-13 10:08:13 UTC
(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}.
Comment 6 Marcel de Rooy 2024-12-13 10:09:28 UTC
(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 :)
Comment 7 Marcel de Rooy 2024-12-13 10:14:17 UTC
 WARN   tools/inventory.pl
   WARN   tidiness
                The file is less tidy than before (bad/messy lines before: 170, now: 171)

For changing one line :)
Comment 8 Lucas Gass (lukeg) 2024-12-13 15:49:25 UTC
(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.
Comment 9 Lucas Gass (lukeg) 2024-12-13 15:50:11 UTC
(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.
Comment 10 Lucas Gass (lukeg) 2024-12-13 15:53:10 UTC
Created attachment 175451 [details] [review]
Bug 38452: Use lc() for case insensitivity
Comment 11 Lucas Gass (lukeg) 2024-12-13 15:54:09 UTC
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.
Comment 12 Lucas Gass (lukeg) 2024-12-13 16:29:50 UTC
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.
Comment 13 David Nind 2024-12-14 21:36:34 UTC
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>
Comment 14 Marcel de Rooy 2025-01-10 08:42:03 UTC
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>
Comment 15 Katrin Fischer 2025-01-10 18:21:04 UTC
Pushed for 25.05!

Well done everyone, thank you!