Bug 39971

Summary: Patron attribute types form logic should be reusable
Product: Koha Reporter: Pedro Amorim <pedro.amorim>
Component: Architecture, internals, and plumbingAssignee: Pedro Amorim <pedro.amorim>
Status: Passed QA --- QA Contact: Nick Clemens (kidclamp) <nick>
Severity: enhancement    
Priority: P5 - low CC: david, jonathan.druart, lucas, martin.renvoize, nick, pedro.amorim, tomascohen
Version: Main   
Hardware: All   
OS: All   
GIT URL: Initiative type: ---
Sponsorship status: --- Comma delimited list of Sponsors:
Crowdfunding goal: 0 Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Circulation function:
Attachments: Bug 39971: Move form rendering to reusable tt block
Bug 39971: Move patron_attributes_form method to a class
Bug 39971: Add POD
Bug 39971: Add query param
Bug 39971: Move form rendering to reusable tt block
Bug 39971: Move patron_attributes_form method to a class
Bug 39971: Add POD
Bug 39971: Add query param
Bug 39971: Move form rendering to reusable tt block
Bug 39971: Move patron_attributes_form method to a class
Bug 39971: Add POD
Bug 39971: Add query param
Bug 39971: Move form rendering to reusable tt block
Bug 39971: Move patron_attributes_form method to a class
Bug 39971: Add POD
Bug 39971: Add query param
Bug 39971: (QA follow-up) Unit tests
Bug 39971: Move form rendering to reusable tt block
Bug 39971: Move patron_attributes_form method to a class
Bug 39971: Add POD
Bug 39971: Add query param
Bug 39971: (QA follow-up) Unit tests

Description Pedro Amorim 2025-05-22 15:22:59 UTC
Other parts of Koha that need to render this should not be repeating code but instead make use of established DRY logic.
Comment 1 Pedro Amorim 2025-05-22 15:31:17 UTC
Created attachment 182739 [details] [review]
Bug 39971: Move form rendering to reusable tt block
Comment 2 Pedro Amorim 2025-05-22 15:31:20 UTC
Created attachment 182740 [details] [review]
Bug 39971: Move patron_attributes_form method to a class

This makes it reusable, documentable, and testable.

This is changing code without adding a new feature or fixing a bug.
The first test plan should be to just make sure that the existing functionality is kept intact, i.e. rendering and saving the patron attribute types in the form.

As to why we need this, we're maintaining the ILL Actions plugin where we have a feature to quickly add a patron. This is designed to be as minimal as possible, but if there are any mandatory patron attribute types, those need to be considered.
To test this:
1) Apply patches and enable ILLModule syspref.
2) Install the plugin:
https://github.com/openfifth/koha-plugin-ill-actions/releases/tag/v2.4.0
3) Add a mandatory patron attribute type, you can edit the existing one in k-t-d, visit:
   <staff_url>/cgi-bin/koha/admin/patron-attr-types.pl?op=edit_attribute_type&code=SHOW_BCODE
5) Tick the 'Staff interface mandatory' checkbox
6) Create a new ILL request, visit:
   <staff_url>/cgi-bin/koha/ill/ill-requests.pl?method=create&backend=Standard
7) Notice there's a '+Add new user' link next to the cardnumber input at the bottom of the form. Click that.
8) Notice there's a 'Additional attributes and identifiers' section. This is only possible (without writing it again) because of the patches here.
Comment 3 Pedro Amorim 2025-05-22 15:31:23 UTC
Created attachment 182741 [details] [review]
Bug 39971: Add POD
Comment 4 Pedro Amorim 2025-05-22 15:31:26 UTC
Created attachment 182742 [details] [review]
Bug 39971: Add query param
Comment 5 David Nind 2025-06-07 01:20:26 UTC
Created attachment 183087 [details] [review]
Bug 39971: Move form rendering to reusable tt block

Signed-off-by: David Nind <david@davidnind.com>
Comment 6 David Nind 2025-06-07 01:20:29 UTC
Created attachment 183088 [details] [review]
Bug 39971: Move patron_attributes_form method to a class

This makes it reusable, documentable, and testable.

This is changing code without adding a new feature or fixing a bug.
The first test plan should be to just make sure that the existing functionality is kept intact, i.e. rendering and saving the patron attribute types in the form.

As to why we need this, we're maintaining the ILL Actions plugin where we have a feature to quickly add a patron. This is designed to be as minimal as possible, but if there are any mandatory patron attribute types, those need to be considered.
To test this:
1) Apply patches and enable ILLModule syspref.
2) Install the plugin:
https://github.com/openfifth/koha-plugin-ill-actions/releases/tag/v2.4.0
3) Add a mandatory patron attribute type, you can edit the existing one in k-t-d, visit:
   <staff_url>/cgi-bin/koha/admin/patron-attr-types.pl?op=edit_attribute_type&code=SHOW_BCODE
5) Tick the 'Staff interface mandatory' checkbox
6) Create a new ILL request, visit:
   <staff_url>/cgi-bin/koha/ill/ill-requests.pl?method=create&backend=Standard
7) Notice there's a '+Add new user' link next to the cardnumber input at the bottom of the form. Click that.
8) Notice there's a 'Additional attributes and identifiers' section. This is only possible (without writing it again) because of the patches here.

Signed-off-by: David Nind <david@davidnind.com>
Comment 7 David Nind 2025-06-07 01:20:32 UTC
Created attachment 183089 [details] [review]
Bug 39971: Add POD

Signed-off-by: David Nind <david@davidnind.com>
Comment 8 David Nind 2025-06-07 01:20:34 UTC
Created attachment 183090 [details] [review]
Bug 39971: Add query param

Signed-off-by: David Nind <david@davidnind.com>
Comment 9 David Nind 2025-06-07 01:22:17 UTC
Things worked as per the test plan.

In this case, a new patron wasn't created. However, I assume that is because the plugin has to implement that part? (It also let you press submit even if you didn't add a mandatory field.)
Comment 10 Pedro Amorim 2025-11-05 15:29:19 UTC
Created attachment 189077 [details] [review]
Bug 39971: Move form rendering to reusable tt block

Signed-off-by: David Nind <david@davidnind.com>
Comment 11 Pedro Amorim 2025-11-05 15:29:21 UTC
Created attachment 189078 [details] [review]
Bug 39971: Move patron_attributes_form method to a class

This makes it reusable, documentable, and testable.

This is changing code without adding a new feature or fixing a bug.
The first test plan should be to just make sure that the existing functionality is kept intact, i.e. rendering and saving the patron attribute types in the form.

As to why we need this, we're maintaining the ILL Actions plugin where we have a feature to quickly add a patron. This is designed to be as minimal as possible, but if there are any mandatory patron attribute types, those need to be considered.
To test this:
1) Apply patches and enable ILLModule syspref.
2) Install the plugin:
https://github.com/openfifth/koha-plugin-ill-actions/releases/tag/v2.4.0
3) Add a mandatory patron attribute type, you can edit the existing one in k-t-d, visit:
   <staff_url>/cgi-bin/koha/admin/patron-attr-types.pl?op=edit_attribute_type&code=SHOW_BCODE
5) Tick the 'Staff interface mandatory' checkbox
6) Create a new ILL request, visit:
   <staff_url>/cgi-bin/koha/ill/ill-requests.pl?method=create&backend=Standard
7) Notice there's a '+Add new user' link next to the cardnumber input at the bottom of the form. Click that.
8) Notice there's a 'Additional attributes and identifiers' section. This is only possible (without writing it again) because of the patches here.

Signed-off-by: David Nind <david@davidnind.com>
Comment 12 Pedro Amorim 2025-11-05 15:29:23 UTC
Created attachment 189079 [details] [review]
Bug 39971: Add POD

Signed-off-by: David Nind <david@davidnind.com>
Comment 13 Pedro Amorim 2025-11-05 15:29:26 UTC
Created attachment 189080 [details] [review]
Bug 39971: Add query param

Signed-off-by: David Nind <david@davidnind.com>
Comment 14 Pedro Amorim 2025-11-05 15:29:39 UTC
Rebased on bug 40917
Comment 15 Nick Clemens (kidclamp) 2025-11-05 16:58:14 UTC
Created attachment 189125 [details] [review]
Bug 39971: Move form rendering to reusable tt block

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 16 Nick Clemens (kidclamp) 2025-11-05 16:58:16 UTC
Created attachment 189126 [details] [review]
Bug 39971: Move patron_attributes_form method to a class

This makes it reusable, documentable, and testable.

This is changing code without adding a new feature or fixing a bug.
The first test plan should be to just make sure that the existing functionality is kept intact, i.e. rendering and saving the patron attribute types in the form.

As to why we need this, we're maintaining the ILL Actions plugin where we have a feature to quickly add a patron. This is designed to be as minimal as possible, but if there are any mandatory patron attribute types, those need to be considered.
To test this:
1) Apply patches and enable ILLModule syspref.
2) Install the plugin:
https://github.com/openfifth/koha-plugin-ill-actions/releases/tag/v2.4.0
3) Add a mandatory patron attribute type, you can edit the existing one in k-t-d, visit:
   <staff_url>/cgi-bin/koha/admin/patron-attr-types.pl?op=edit_attribute_type&code=SHOW_BCODE
5) Tick the 'Staff interface mandatory' checkbox
6) Create a new ILL request, visit:
   <staff_url>/cgi-bin/koha/ill/ill-requests.pl?method=create&backend=Standard
7) Notice there's a '+Add new user' link next to the cardnumber input at the bottom of the form. Click that.
8) Notice there's a 'Additional attributes and identifiers' section. This is only possible (without writing it again) because of the patches here.

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 17 Nick Clemens (kidclamp) 2025-11-05 16:58:19 UTC
Created attachment 189127 [details] [review]
Bug 39971: Add POD

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 18 Nick Clemens (kidclamp) 2025-11-05 16:58:22 UTC
Created attachment 189128 [details] [review]
Bug 39971: Add query param

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 19 Nick Clemens (kidclamp) 2025-11-05 16:58:24 UTC
Created attachment 189129 [details] [review]
Bug 39971: (QA follow-up) Unit tests

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 20 Pedro Amorim 2025-11-05 17:01:08 UTC
Thank you, Nick. I'm sorry, I realize now I wrongfully set this to PQA after rebase when I should've set it to 'Signed-off' as it was originally.
Comment 21 Nick Clemens (kidclamp) 2025-11-05 17:05:50 UTC
Created attachment 189130 [details] [review]
Bug 39971: Move form rendering to reusable tt block

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 22 Nick Clemens (kidclamp) 2025-11-05 17:05:53 UTC
Created attachment 189131 [details] [review]
Bug 39971: Move patron_attributes_form method to a class

This makes it reusable, documentable, and testable.

This is changing code without adding a new feature or fixing a bug.
The first test plan should be to just make sure that the existing functionality is kept intact, i.e. rendering and saving the patron attribute types in the form.

As to why we need this, we're maintaining the ILL Actions plugin where we have a feature to quickly add a patron. This is designed to be as minimal as possible, but if there are any mandatory patron attribute types, those need to be considered.
To test this:
1) Apply patches and enable ILLModule syspref.
2) Install the plugin:
https://github.com/openfifth/koha-plugin-ill-actions/releases/tag/v2.4.0
3) Add a mandatory patron attribute type, you can edit the existing one in k-t-d, visit:
   <staff_url>/cgi-bin/koha/admin/patron-attr-types.pl?op=edit_attribute_type&code=SHOW_BCODE
5) Tick the 'Staff interface mandatory' checkbox
6) Create a new ILL request, visit:
   <staff_url>/cgi-bin/koha/ill/ill-requests.pl?method=create&backend=Standard
7) Notice there's a '+Add new user' link next to the cardnumber input at the bottom of the form. Click that.
8) Notice there's a 'Additional attributes and identifiers' section. This is only possible (without writing it again) because of the patches here.

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 23 Nick Clemens (kidclamp) 2025-11-05 17:05:56 UTC
Created attachment 189132 [details] [review]
Bug 39971: Add POD

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 24 Nick Clemens (kidclamp) 2025-11-05 17:05:58 UTC
Created attachment 189133 [details] [review]
Bug 39971: Add query param

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 25 Nick Clemens (kidclamp) 2025-11-05 17:06:01 UTC
Created attachment 189134 [details] [review]
Bug 39971: (QA follow-up) Unit tests

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>