Bug 32517 - Patron search dies on case mismatch of patron category
Summary: Patron search dies on case mismatch of patron category
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Staff interface (show other bugs)
Version: Main
Hardware: All All
: P5 - low major
Assignee: Nick Clemens (kidclamp)
QA Contact: Marcel de Rooy
URL:
Keywords:
Depends on:
Blocks: 35743
  Show dependency tree / graph
 
Reported: 2022-12-22 15:16 UTC by Nick Clemens (kidclamp)
Modified: 2024-10-09 09:38 UTC (History)
4 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
This fixes patron search so that searching by category will work regardless of the patron category code case (upper, lower, and sentence case). Before this, category codes in upper case were expected - where they weren't this caused the search to fail, resulting in no search results.
Version(s) released in:
23.05.00,22.11.02
Circulation function:

Attachments
Bug 32517: lowercase category codes to avoid mismatch (1.66 KB, patch)
2022-12-22 15:19 UTC, Nick Clemens (kidclamp) 		Details | Diff | Splinter Review
Bug 32517: lowercase category codes to avoid mismatch (1.71 KB, patch)
2022-12-22 21:12 UTC, David Nind 		Details | Diff | Splinter Review
Bug 32517: lowercase category codes to avoid mismatch (1.81 KB, patch)
2023-01-13 10:10 UTC, Marcel de Rooy 		Details | Diff | Splinter Review
Show Obsolete (2) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Nick Clemens (kidclamp) 2022-12-22 15:16:04 UTC 
We load the categories in JS for transforming category codes to names, however, this introduces case sensitivity

To recreate:
1 - sudo koha-mysql kohadev
2 - UPDATE borrowers SET categorycode = LCASE(categorycode)
3 - Try to search for patrons:
    Uncaught TypeError: categories_map[data] is undefined
Comment 1 Nick Clemens (kidclamp) 2022-12-22 15:19:37 UTC Comment hidden (obsolete)
Comment 2 David Nind 2022-12-22 21:12:34 UTC 
Created attachment 144805 [details] [review]
Bug 32517: lowercase category codes to avoid mismatch

To test:
1 - sudo koha-mysql kohadev
2 - UPDATE borrowers SET categorycode = LCASE(categorycode)
3 - Try to search for patrons:
    Uncaught TypeError: categories_map[data] is undefined
4 - Apply patch
5 - Search for patrons
6 - Success!

Signed-off-by: David Nind <david@davidnind.com>
Comment 3 Katrin Fischer 2022-12-27 22:04:51 UTC 
But... should category codes not always be uppercase?
Comment 4 Nick Clemens (kidclamp) 2022-12-28 05:26:14 UTC 
In my opinion, yes. But, we have never enforced that.

SQL is generally not case sensitive, unless you ask it to be - this is a consequence of Javascript. I think we should err on the side of codes needing to be unique, but not case sensitive
Comment 5 David Nind 2023-01-06 21:49:10 UTC 
Just noting that this issue (I think) was raised on the general mailing list:
https://lists.katipo.co.nz/pipermail/koha/2023-January/058867.html

Has a slightly different error message, but seems related to case sensitivity with patron category values.
Comment 6 Marcel de Rooy 2023-01-13 10:02:46 UTC 
(In reply to Nick Clemens from comment #4)
> In my opinion, yes. But, we have never enforced that.

Not sure when that changed. But currently you cannot save a lowercase one in the interface.
Comment 7 Marcel de Rooy 2023-01-13 10:10:49 UTC 
Created attachment 145256 [details] [review]
Bug 32517: lowercase category codes to avoid mismatch

To test:
1 - sudo koha-mysql kohadev
2 - UPDATE borrowers SET categorycode = LCASE(categorycode)
3 - Try to search for patrons:
    Uncaught TypeError: categories_map[data] is undefined
4 - Apply patch
5 - Search for patrons
6 - Success!

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 8 Marcel de Rooy 2023-01-13 10:12:55 UTC 
Confirm that this fixes the problem introduced by sql changing the category to lowercase. This is not possible now in the interface.

So it is a bit arguable patch. But I dont object. Note that you could go the other way and add a dbrev that makes all categorycodes uppercase in borrowers for example.

Marking it as major seems overkill btw.
Comment 9 Nick Clemens (kidclamp) 2023-01-13 13:52:25 UTC 
(In reply to Marcel de Rooy from comment #8)
> Marking it as major seems overkill btw.

A few bad patrons could break the patron search at large - this is a huge impact for libraries that may have a hard time diagnosing the issue.
Comment 10 Katrin Fischer 2023-01-13 14:00:03 UTC 
I am still curious on how they ended up there in the first place. As long as I can remember you can't add lower case ones... could this be related to API/patron import/SQL based migrations?

I am a little worried that if we fix this bug, it might just be a band aid in one spot while in other places we'll still see issues.
Comment 11 Nick Clemens (kidclamp) 2023-01-13 14:06:42 UTC 
(In reply to Katrin Fischer from comment #10)
> I am still curious on how they ended up there in the first place. As long as
> I can remember you can't add lower case ones... could this be related to
> API/patron import/SQL based migrations?
> 
> I am a little worried that if we fix this bug, it might just be a band aid
> in one spot while in other places we'll still see issues.

Yes, we believe related to API/patron imports, or just very old data
Comment 12 Martin Renvoize (ashimema) 2023-01-18 16:00:28 UTC 
Nice work everyone!

Pushed to 23.05.x for the next release
Comment 13 Jacob O'Mara 2023-01-18 17:21:32 UTC 
Nice work, thanks everyone!

Pushed to 22.11.x for the next release.
Comment 14 Jonathan Druart 2023-02-23 11:13:14 UTC 
We need to fix the root of the problem, during the import. We provide a script to catch this problem. We should not deal with that in the template, that's silly.
Comment 15 Jonathan Druart 2024-10-09 09:38:10 UTC 
(In reply to Jonathan Druart from comment #14)
> We need to fix the root of the problem, during the import. We provide a
> script to catch this problem. We should not deal with that in the template,
> that's silly.

Still valid, this needs to be fixed properly. I am now writing workarounds in code that is trying to be generic for this specific "category" search.