Bug 40313 - Legacy/migrated categorycode can break patron save
Summary: Legacy/migrated categorycode can break patron save
Status: Failed QA
Alias: None
Product: Koha
Classification: Unclassified
Component: Patrons (show other bugs)
Version: Main
Hardware: All All
: P5 - low minor
Assignee: Martin Renvoize (ashimema)
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2025-07-07 01:58 UTC by David Cook
Modified: 2025-08-27 20:58 UTC (History)
5 users (show)

See Also:
GIT URL:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
Circulation function:


Attachments
Bug 40313: Prevent jQuery error when saving patron with bad categorycode (2.29 KB, patch)
2025-07-07 02:07 UTC, David Cook
Details | Diff | Splinter Review
Bug 40313: Prevent jQuery error when saving patron with bad categorycode (2.34 KB, patch)
2025-07-18 23:18 UTC, David Nind
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description David Cook 2025-07-07 01:58:57 UTC
This should be quite an edge case, but if you have done a data migration and have category codes like "(ABC123", you may notice that you cannot edit your patrons.

Due to some suboptimal Javascript, the "(" gets erroneously interpreted by jQuery and leads to an error which breaks other Javascript and prevents the page save from working.
Comment 2 David Cook 2025-07-07 02:07:36 UTC
Created attachment 183819 [details] [review]
Bug 40313: Prevent jQuery error when saving patron with bad categorycode

This patch prevents a problem string from being passed to $().

Out of the box, this shouldn't be a problem, but if you migrate categorycodes
from another system which may or may not include the "(" character in them,
you'll find you cannot edit and save these patrons anymore due to a Javascript
error.

To reproduce:
0. DO NOT APPLY THE PATCH
1. Create a new patron category with a code called "TEST"
2. In the mysql client, change this patron category code to "(TEST"
3. Create a new patron with this category
4. Try to edit and save the patron
5. Note you cannot (and if you open the browser console, you'll see JS errors)

Test plan:
0. Apply the patch
1. Try to edit and save the patron from above
2. Note that you can now save the patron

Bonus points:
0. Edit the new patron category to have a password length of 16 or higher
1. Create a patron attribute just for this new patron category
2. Test editing patrons by changing the category to this TEST category
and other established categories.
3. Note that the password length and patron attributes change as you
change the category
Comment 4 David Nind 2025-07-18 23:18:30 UTC
Created attachment 184418 [details] [review]
Bug 40313: Prevent jQuery error when saving patron with bad categorycode

This patch prevents a problem string from being passed to $().

Out of the box, this shouldn't be a problem, but if you migrate categorycodes
from another system which may or may not include the "(" character in them,
you'll find you cannot edit and save these patrons anymore due to a Javascript
error.

To reproduce:
0. DO NOT APPLY THE PATCH
1. Create a new patron category with a code called "TEST"
2. In the mysql client, change this patron category code to "(TEST"
3. Create a new patron with this category
4. Try to edit and save the patron
5. Note you cannot (and if you open the browser console, you'll see JS errors)

Test plan:
0. Apply the patch
1. Try to edit and save the patron from above
2. Note that you can now save the patron

Bonus points:
0. Edit the new patron category to have a password length of 16 or higher
1. Create a patron attribute just for this new patron category
2. Test editing patrons by changing the category to this TEST category
and other established categories.
3. Note that the password length and patron attributes change as you
change the category

Signed-off-by: David Nind <david@davidnind.com>
Comment 5 David Nind 2025-07-18 23:19:55 UTC
Testing notes (using KTD):
1. SQL to update patron category: update categories set categorycode="(TEST" where categorycode="TEST";
2. I didn't try for bonus points! 8-)
Comment 6 Marcel de Rooy 2025-07-25 08:13:04 UTC
git grep
koha-tmpl/intranet-tmpl/prog/en/modules/members/memberentrygen.tt:                update_category_code( "[% borrower_data.categorycode | html %]" );
koha-tmpl/intranet-tmpl/prog/en/modules/members/memberentrygen.tt:                    update_category_code( category_code );
koha-tmpl/intranet-tmpl/prog/en/modules/members/memberentrygen.tt:                update_category_code(this);
koha-tmpl/intranet-tmpl/prog/js/members.js:function update_category_code(category_code) {

Shouldnt we change this part in the template:

            $("#categorycode_entry").on("change", function(){
                update_category_code(this);
            });
And have a consistent param type in the function?
Comment 7 David Cook 2025-07-28 05:47:10 UTC
(In reply to Marcel de Rooy from comment #6)
> git grep
> koha-tmpl/intranet-tmpl/prog/en/modules/members/memberentrygen.tt:          
> update_category_code( "[% borrower_data.categorycode | html %]" );
> koha-tmpl/intranet-tmpl/prog/en/modules/members/memberentrygen.tt:          
> update_category_code( category_code );
> koha-tmpl/intranet-tmpl/prog/en/modules/members/memberentrygen.tt:          
> update_category_code(this);
> koha-tmpl/intranet-tmpl/prog/js/members.js:function
> update_category_code(category_code) {
> 
> Shouldnt we change this part in the template:
> 
>             $("#categorycode_entry").on("change", function(){
>                 update_category_code(this);
>             });
> And have a consistent param type in the function?

I think that would be the optimal outcome for sure.

I was short on time locally, so this was the shortest path to a working version, and I figured I'd upstream that. 

But yes I think a better overall solution would be to change the rest of the template/javascript to be more consistent.
Comment 8 Marcel de Rooy 2025-08-01 07:46:30 UTC
(In reply to David Cook from comment #7)
> But yes I think a better overall solution would be to change the rest of the
> template/javascript to be more consistent.

Agreed. Please do. Shouldnt take you much time?
Comment 9 David Cook 2025-08-03 23:25:00 UTC
(In reply to Marcel de Rooy from comment #8)
> (In reply to David Cook from comment #7)
> > But yes I think a better overall solution would be to change the rest of the
> > template/javascript to be more consistent.
> 
> Agreed. Please do. Shouldnt take you much time?

Unfortunately, it looks like my schedule has changed to include less Koha time, so I will have to consider my contributions more carefully. This one probably isn't important enough to come back to. 

Resetting assignee.