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.
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!
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!
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.
(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.
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>
(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
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
This is becoming a major service issue for us.
Created attachment 152642 [details] [review] Bug 33690: (QA follow-up) Mock C4::Letters subroutines for unit tests
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
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.
OK.. I stand corrected.. the csv import tool requires a checkbox be ticked.
I still think defaulting to 'AutoEmailNewUser' should be used however.
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>
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>
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.
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?
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.
Created attachment 155407 [details] [review] Bug 33690: Tidy test
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
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.
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>
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>
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>
Pushed to master for 23.11. Nice work everyone, thanks!
Enhancement not pushed to 23.05.x