Bug 39408 - Cannot add patron via API if AutoEmailNewUser and WELCOME content blank
Summary: Cannot add patron via API if AutoEmailNewUser and WELCOME content blank
Status: Failed QA
Alias: None
Product: Koha
Classification: Unclassified
Component: Patrons (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal
Assignee: Nick Clemens (kidclamp)
QA Contact: Martin Renvoize (ashimema)
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2025-03-21 13:52 UTC by Nick Clemens (kidclamp)
Modified: 2025-05-12 16:16 UTC (History)
4 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 39408: Handle BadParameter exception better (2.09 KB, patch)
2025-03-21 14:00 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 39408: Don't attempt to send a message if one is not generated (1.21 KB, patch)
2025-03-21 14:00 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 39408: Handle BadParameter exception better (2.15 KB, patch)
2025-04-02 08:37 UTC, claire.hernandez@biblibre.com
Details | Diff | Splinter Review
Bug 39408: Don't attempt to send a message if one is not generated (1.28 KB, patch)
2025-04-02 08:37 UTC, claire.hernandez@biblibre.com
Details | Diff | Splinter Review
Bug 39408: Handle BadParameter exception better (2.22 KB, patch)
2025-05-09 12:27 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 39408: Don't attempt to send a message if one is not generated (1.34 KB, patch)
2025-05-09 12:27 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 39408: (QA follow-up) Add unit tests (12.44 KB, patch)
2025-05-09 12:27 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Nick Clemens (kidclamp) 2025-03-21 13:52:53 UTC
Some libraries use Template Toolkit logic to avoid sending emails. In the case of the welcome notice they may only want to send for patrons fully added, not self registered.

In the case of Aspen, we currently use the patrons API for self registration as there is not a self registration API.

If you add logic like:
[% USE Koha %]
[% IF borrower.categorycode != 'SELFREG' %]
Hello [% borrower.title %] [% borrower.firstname %] [% borrower.surname %].<br>
<br>
Thank you for joining [% IF Koha.Preference('LibraryName') %][% Koha.Preference('LibraryName') %][% ELSE %]the library[% END %]<br>
<br>
You can search for all our materials in our <a href='[% Koha.Preference('OPACBaseURL') %]'>catalog</a>.<br>
<br>
Your library card number is [% borrower.cardnumber %]<br>
<br>
If you have any problems or questions regarding your account, please contact the library.
[% END %]

Then add a patron via the API in the self reg class:
{"surname":"PATRON","firstname":"PROBLEM","library_id":"CPL","category_id":"SELFREG","email":"eample@example.org"}

You get an error response:
"Given does not exist"
Comment 1 Nick Clemens (kidclamp) 2025-03-21 14:00:18 UTC
Created attachment 179613 [details] [review]
Bug 39408: Handle BadParameter exception better

Currently we assume BadParameter has been thrown with a parameter that is a valid patron field.
It may only include a message, or it may have come from outside the patron object, we should
handle all cases.

To test:
1 - Wrap "WELCOME" notice content in conditional:
    [% IF borrower.categorycode != 'SELFREG' %]
    ...
    [% END %]
2 - Enable preference AutoEmailNewUser
3 - Attempt to self register a patron via API with body:
    {"surname":"PATRON","firstname":"PROBLEM","library_id":"CPL","category_id":"SELFREG","email":"eample@example.org"}
4 - Response error: "Given  does not exist"
5 - Apply this patch, restart all
6 - Repeat 3
7 - Response error: "Exception 'Koha::Exceptions::BadParameter' thrown 'Parameter message_id cannot be empty if passed.'\n"
8 - It's uglier, but actually helpful
Comment 2 Nick Clemens (kidclamp) 2025-03-21 14:00:20 UTC
Created attachment 179614 [details] [review]
Bug 39408: Don't attempt to send a message if one is not generated

The routine to create the message can return nothing if there is no content,
we should handle this case and not attempt to send.

To test:
1 - Test plan from last patch
2 - Apply this patch
3 - POST to patron API again
4 - Success!
Comment 3 claire.hernandez@biblibre.com 2025-04-02 08:37:09 UTC
Created attachment 180255 [details] [review]
Bug 39408: Handle BadParameter exception better

Currently we assume BadParameter has been thrown with a parameter that is a valid patron field.
It may only include a message, or it may have come from outside the patron object, we should
handle all cases.

To test:
1 - Wrap "WELCOME" notice content in conditional:
    [% IF borrower.categorycode != 'SELFREG' %]
    ...
    [% END %]
2 - Enable preference AutoEmailNewUser
3 - Attempt to self register a patron via API with body:
    {"surname":"PATRON","firstname":"PROBLEM","library_id":"CPL","category_id":"SELFREG","email":"eample@example.org"}
4 - Response error: "Given  does not exist"
5 - Apply this patch, restart all
6 - Repeat 3
7 - Response error: "Exception 'Koha::Exceptions::BadParameter' thrown 'Parameter message_id cannot be empty if passed.'\n"
8 - It's uglier, but actually helpful

Signed-off-by: Claire Hernandez <claire.hernandez@biblibre.com>
Comment 4 claire.hernandez@biblibre.com 2025-04-02 08:37:12 UTC
Created attachment 180256 [details] [review]
Bug 39408: Don't attempt to send a message if one is not generated

The routine to create the message can return nothing if there is no content,
we should handle this case and not attempt to send.

To test:
1 - Test plan from last patch
2 - Apply this patch
3 - POST to patron API again
4 - Success!

Signed-off-by: Claire Hernandez <claire.hernandez@biblibre.com>
Comment 5 Martin Renvoize (ashimema) 2025-05-09 12:27:22 UTC
Created attachment 182165 [details] [review]
Bug 39408: Handle BadParameter exception better

Currently we assume BadParameter has been thrown with a parameter that is a valid patron field.
It may only include a message, or it may have come from outside the patron object, we should
handle all cases.

To test:
1 - Wrap "WELCOME" notice content in conditional:
    [% IF borrower.categorycode != 'SELFREG' %]
    ...
    [% END %]
2 - Enable preference AutoEmailNewUser
3 - Attempt to self register a patron via API with body:
    {"surname":"PATRON","firstname":"PROBLEM","library_id":"CPL","category_id":"SELFREG","email":"eample@example.org"}
4 - Response error: "Given  does not exist"
5 - Apply this patch, restart all
6 - Repeat 3
7 - Response error: "Exception 'Koha::Exceptions::BadParameter' thrown 'Parameter message_id cannot be empty if passed.'\n"
8 - It's uglier, but actually helpful

Signed-off-by: Claire Hernandez <claire.hernandez@biblibre.com>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Comment 6 Martin Renvoize (ashimema) 2025-05-09 12:27:25 UTC
Created attachment 182166 [details] [review]
Bug 39408: Don't attempt to send a message if one is not generated

The routine to create the message can return nothing if there is no content,
we should handle this case and not attempt to send.

To test:
1 - Test plan from last patch
2 - Apply this patch
3 - POST to patron API again
4 - Success!

Signed-off-by: Claire Hernandez <claire.hernandez@biblibre.com>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Comment 7 Martin Renvoize (ashimema) 2025-05-09 12:27:27 UTC
Created attachment 182167 [details] [review]
Bug 39408: (QA follow-up) Add unit tests

Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Comment 8 Katrin Fischer 2025-05-12 16:16:01 UTC
I applied the patches and ran yarn build. But:


 FAIL	Koha/REST/V1/Patrons.pm
   FAIL	  tidiness
		File is not tidy, please run `perl misc/devel/tidy.pl Koha/REST/V1/Patrons.pm`

 FAIL	t/db_dependent/api/v1/patrons.t
   FAIL	  critic
		# Subroutines::ProhibitExplicitReturnUndef: Got 1 violation(s).  


Please also double check the tests, I had some trouble there that only went away after a reset_all.