Bug 33690 - Add ability to send welcome notice when creating patrons using the REST API
Summary: Add ability to send welcome notice when creating patrons using the REST API
Status: Needs documenting
Alias: None
Product: Koha
Classification: Unclassified
Component: REST API (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Kyle M Hall
QA Contact: Tomás Cohen Arazi
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2023-05-05 16:24 UTC by Kyle M Hall
Modified: 2023-11-08 15:57 UTC (History)
11 users (show)

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


Attachments
Bug 33690: Add ability to send welcome notice when creating patrons using the REST API (3.07 KB, patch)
2023-05-05 16:27 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 33690: Add ability to send welcome notice when creating patrons using the REST API (8.48 KB, patch)
2023-05-05 18:51 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 33690: Add ability to send welcome notice when creating patrons using the REST API (8.52 KB, patch)
2023-05-07 19:41 UTC, David Nind
Details | Diff | Splinter Review
Bug 33690: (QA follow-up) Mock C4::Letters subroutines for unit tests (2.08 KB, patch)
2023-06-23 14:55 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 33690: Add ability to send welcome notice when creating patrons using the REST API (8.59 KB, patch)
2023-09-08 11:10 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 33690: (QA follow-up) Mock C4::Letters subroutines for unit tests (2.14 KB, patch)
2023-09-08 11:10 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 33690: Convert to x-koha-override (2.97 KB, patch)
2023-09-08 11:10 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 33690: Convert to x-koha-override (3.03 KB, patch)
2023-09-08 11:28 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 33690: Tidy test (1.75 KB, patch)
2023-09-08 11:28 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 33690: Add ability to send welcome notice when creating patrons using the REST API (8.54 KB, patch)
2023-10-04 13:53 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 33690: (QA follow-up) Mock C4::Letters subroutines for unit tests (2.32 KB, patch)
2023-10-04 13:54 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 33690: Convert to x-koha-override (3.10 KB, patch)
2023-10-04 13:54 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Kyle M Hall 2023-05-05 16:24:23 UTC
As an addition/alternative to Bug 30299, it would be nice to have the ability to send a welcome notice when creating patrons via the API.
Comment 1 Kyle M Hall 2023-05-05 16:27:14 UTC
Created attachment 150777 [details] [review]
Bug 33690: Add ability to send welcome notice when creating patrons using the REST API

It would be nice to have the ability to send a welcome notice when creating patrons via the API.

Test Plan:
1) Apply this patch
2) Ensure you have a WELCOME notice
3) Create a new patron using the REST API ( api/v1/patrons )
4) Note no welcome notice is sent to the patron ( you can check the
   notices tab for the patron )
5) Repeat step 3, but send the header X-Koha-SendWelcomeEmail with a
   value of 1 as part of the POST
6) Note the welcome message for the patron is in their notices!
Comment 2 Kyle M Hall 2023-05-05 18:51:14 UTC
Created attachment 150787 [details] [review]
Bug 33690: Add ability to send welcome notice when creating patrons using the REST API

It would be nice to have the ability to send a welcome notice when creating patrons via the API.

Test Plan:
1) Apply this patch
2) Ensure you have a WELCOME notice
3) Create a new patron using the REST API ( api/v1/patrons )
4) Note no welcome notice is sent to the patron ( you can check the
   notices tab for the patron )
5) Repeat step 3, but send the header X-Koha-SendWelcomeEmail with a
   value of 1 as part of the POST
6) Note the welcome message for the patron is in their notices!
Comment 3 David Nind 2023-05-06 22:49:15 UTC
I'd be happy to test, but don't know enough for step 3.

Could you provide an example of the JSON for adding a patron?

I can use Postman for basic things....

Otehrwsie, I'll leave for someone else who knows what to do, to test.
Comment 4 Tomás Cohen Arazi 2023-05-06 23:31:39 UTC
(In reply to David Nind from comment #3)
> I'd be happy to test, but don't know enough for step 3.
> 
> Could you provide an example of the JSON for adding a patron?
> 
> I can use Postman for basic things....
> 
> Otehrwsie, I'll leave for someone else who knows what to do, to test.

When you do a GET /API/V1/patrons you can get an idea of how a patron object for POST should look like. I'd suggest you pick a couple attributes like firstname, surname and user id. And see if it tells you you need more.
Comment 5 David Nind 2023-05-07 19:41:29 UTC
Created attachment 150791 [details] [review]
Bug 33690: Add ability to send welcome notice when creating patrons using the REST API

It would be nice to have the ability to send a welcome notice when creating patrons via the API.

Test Plan:
1) Apply this patch
2) Ensure you have a WELCOME notice
3) Create a new patron using the REST API ( api/v1/patrons )
4) Note no welcome notice is sent to the patron ( you can check the
   notices tab for the patron )
5) Repeat step 3, but send the header X-Koha-SendWelcomeEmail with a
   value of 1 as part of the POST
6) Note the welcome message for the patron is in their notices!

Signed-off-by: David Nind <david@davidnind.com>
Comment 6 David Nind 2023-05-07 19:42:17 UTC
(In reply to Tomás Cohen Arazi from comment #4)
> (In reply to David Nind from comment #3)
> > I'd be happy to test, but don't know enough for step 3.
> > 
> > Could you provide an example of the JSON for adding a patron?
> > 
> > I can use Postman for basic things....
> > 
> > Otehrwsie, I'll leave for someone else who knows what to do, to test.
> 
> When you do a GET /API/V1/patrons you can get an idea of how a patron object
> for POST should look like. I'd suggest you pick a couple attributes like
> firstname, surname and user id. And see if it tells you you need more.

Thanks Tomás!

That helped me figure it out.

David
Comment 7 David Nind 2023-05-07 19:44:41 UTC
Testing notes (using KTD)

1. Enabled RESTBasicAuth system preference.

2. Used Postman to create a new HTTP Request with these values/settings:
   - POST to http://127.0.0.1:8081/api/v1/patrons
   - Authorization: Basic Auth with koha as both username password
   - Headers: added x-koha-welcome with a value of email
   - Body (raw and JSON selected):
{
    "cardnumber": "987654321",
    "category_id": "PT",
    "library_id": "CPL",
    "email": "email@example.com",
    "surname": "bz33690",
    "userid": "bz33690"
}

3. Tests pass: prove -v t/db_dependent/api/v1/patrons.t
Comment 8 Ed Veal 2023-06-22 16:18:41 UTC
This is becoming a major service issue for us.
Comment 9 Kyle M Hall 2023-06-23 14:55:21 UTC
Created attachment 152642 [details] [review]
Bug 33690: (QA follow-up) Mock C4::Letters subroutines for unit tests
Comment 10 Tomás Cohen Arazi 2023-07-03 17:07:38 UTC
I like where this is going towards.

There's one thing that bugs me: the header.

While I agree with using a header for controlling this behavior (to send or not send), I find it weird that the spec says it should contain the word `email`, but there's no check on this.

Could you consider for a bit using the 'email' value? And what about other transports? Shouldn't we make the `x-koha-welcome` header be defined as:

      - name: x-koha-welcome
        in: header
        required: false
        description: The transport to use for sending a WELCOME message. Empty means no message is sent.
        type: array
        items:
          type: string
          enum:
            - email
            - sms
            - pidgeon
        collectionFormat: csv
Comment 11 Martin Renvoize 2023-09-08 10:21:41 UTC
Do we even need a header here?

All other cases where the WELCOME notice is sent use the value of the system preference to determine whether to send or not.

I'd argue perhaps we could support a header optionally to allow locally enabling/disabling send at the API level.. but I don't think we should require the header to trigger the action by default.. default should just adhere to the preference.
Comment 12 Martin Renvoize 2023-09-08 10:25:00 UTC
OK.. I stand corrected.. the csv import tool requires a checkbox be ticked.
Comment 13 Martin Renvoize 2023-09-08 10:26:46 UTC
I still think defaulting to 'AutoEmailNewUser' should be used however.
Comment 14 Martin Renvoize 2023-09-08 11:10:44 UTC
Created attachment 155396 [details] [review]
Bug 33690: Add ability to send welcome notice when creating patrons using the REST API

It would be nice to have the ability to send a welcome notice when creating patrons via the API.

Test Plan:
1) Apply this patch
2) Ensure you have a WELCOME notice
3) Create a new patron using the REST API ( api/v1/patrons )
4) Note no welcome notice is sent to the patron ( you can check the
   notices tab for the patron )
5) Repeat step 3, but send the header X-Koha-SendWelcomeEmail with a
   value of 1 as part of the POST
6) Note the welcome message for the patron is in their notices!

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 15 Martin Renvoize 2023-09-08 11:10:47 UTC
Created attachment 155397 [details] [review]
Bug 33690: (QA follow-up) Mock C4::Letters subroutines for unit tests

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 16 Martin Renvoize 2023-09-08 11:10:50 UTC
Created attachment 155398 [details] [review]
Bug 33690: Convert to x-koha-override

This patch converts the existing work to use x-koha-override instead of
introducing an entirely new header and we default to the system
preference value if the override isn't passed.

I believe this is more consistent with our approach in other areas of
koha to date.
Comment 17 Martin Renvoize 2023-09-08 11:12:21 UTC
The one niggle outstanding for me here is that we're not wrapping the email sending block in an eval like we do with the other WELCOME notice locations.. I'm wondering it we should be doing that and returning some sort of error to the api should it fail?
Comment 18 Martin Renvoize 2023-09-08 11:28:12 UTC
Created attachment 155406 [details] [review]
Bug 33690: Convert to x-koha-override

This patch converts the existing work to use x-koha-override instead of
introducing an entirely new header and we default to the system
preference value if the override isn't passed.

I believe this is more consistent with our approach in other areas of
koha to date.
Comment 19 Martin Renvoize 2023-09-08 11:28:15 UTC
Created attachment 155407 [details] [review]
Bug 33690: Tidy test
Comment 20 David Nind 2023-09-08 12:51:20 UTC
Testing notes (using KTD)

1. Enabled RESTBasicAuth system preference.

2. For step 3 of the test plan, header should now be x-koha-override.
   - If AutoEmailNewUser enabled, header not required
   - Use x-koha-override to override = welcome_yes or welcome_no
   - Should work with the various combinations:
     . AutoEmailNewUser enabled, no header required: welcome message sent
     . AutoEmailNewUser not enabled, x-koha-override = welcome_yes : welcome message sent
     . AutoEmailNewUser enabled, x-koha-override = welcome_no : welcome message not sent

3. Used Postman to create a new HTTP Request with these values/settings:
   - POST to http://127.0.0.1:8081/api/v1/patrons
   - Authorization: Basic Auth with koha as both username password
   - Headers: used without x-koha-override and AutoEmailNewUser enabled + various combinations as in testing notes 2 above
   - Body (raw and JSON selected):
{
    "cardnumber": "987654321",
    "category_id": "PT",
    "library_id": "CPL",
    "email": "email@example.com",
    "surname": "bz33690",
    "userid": "bz33690"
}

4. Tests pass: prove -v t/db_dependent/api/v1/patrons.t
Comment 21 Jonathan Druart 2023-10-04 13:52:58 UTC
Not considering blocker, but worth raising it: if you pass an invalid header's value ("foobar"), it is ignored. I would have expected swagger's spec validation to raise an error.
Comment 22 Jonathan Druart 2023-10-04 13:53:59 UTC
Created attachment 156544 [details] [review]
Bug 33690: Add ability to send welcome notice when creating patrons using the REST API

It would be nice to have the ability to send a welcome notice when creating patrons via the API.

Test Plan:
1) Apply this patch
2) Ensure you have a WELCOME notice
3) Create a new patron using the REST API ( api/v1/patrons )
4) Note no welcome notice is sent to the patron ( you can check the
   notices tab for the patron )
5) Repeat step 3, but send the header X-Koha-SendWelcomeEmail with a
   value of 1 as part of the POST
6) Note the welcome message for the patron is in their notices!

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Bug 33690: Tidy test

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 23 Jonathan Druart 2023-10-04 13:54:02 UTC
Created attachment 156545 [details] [review]
Bug 33690: (QA follow-up) Mock C4::Letters subroutines for unit tests

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

JD Amended patch: tidy

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 24 Jonathan Druart 2023-10-04 13:54:05 UTC
Created attachment 156546 [details] [review]
Bug 33690: Convert to x-koha-override

This patch converts the existing work to use x-koha-override instead of
introducing an entirely new header and we default to the system
preference value if the override isn't passed.

I believe this is more consistent with our approach in other areas of
koha to date.

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 25 Tomás Cohen Arazi 2023-10-04 15:09:33 UTC
Pushed to master for 23.11.

Nice work everyone, thanks!
Comment 26 Fridolin Somers 2023-10-06 00:33:23 UTC
Enhancement not pushed to 23.05.x