REST API: Add route for messaging preferences
Just to be clear about the implementation details .. For messaging preferences to work, there must be set EnhancedMessagingPreferences to Allow in order to let anyone change anything. This will ensure only staff can edit messaging preferences. For regular users to allow them editing those, EnhancedMessagingPreferencesOPAC must be also enabled. If is EnhancedMessagingPreferencesOPAC enabled, but EnhancedMessagingPreferences is not, no editing is allowed to anyone. API URL will be in this format: /api/v1/messaging_preferences/{borrowernumber} Next, I suppose, there will be GET & POST methods. No else. GET method only returns user's messaging prefs. User can view only it's own prefs, unless he has 'borrowers' rights. POST method updates user's messaging pref. User can update only it's own prefs, unless he has 'borrowers' rights. Both, GET & POST methods will send / receive JSON in this format: { item_due: { email: 0/1, digest_only: 0/1, do_not_notify: 0/1 } advance_notice: { days_in_advance: 0-30, email: 0/1, digest_only: 0/1, do_not_notify: 0/1 } hold_filled: { email: 0/1, do_not_notify: 0/1 } item_checkin: { email: 0/1, do_not_notify: 0/1 } item_checkout: { email: 0/1, do_not_notify: 0/1 } } Please someone take a look at this concept, if it is okay to implement this feature in this way.
Hey. Any update on this? I will start working on it if you haven't already. I attached Koha-objects to Bug 17499 which could be useful for this one.
(In reply to Lari Taskula from comment #2) > Hey. Any update on this? I will start working on it if you haven't already. > I attached Koha-objects to Bug 17499 which could be useful for this one. Hey there Lari. Yes, I've already done some work, but now I'm going to be away from work on Koha until I finish final school exams for bachelor's degree. If you want, you can take a look at the work & finish it ;) All the work is in this single commit: https://github.com/open-source-knihovna/KohaCZ/commit/7bddf4f709fce467296ddce814b4612aeaa00088 Basically I made it work for GET request. Feel free to change my coding style as you wish :D
(In reply to Jiri Kozlovsky from comment #3) > (In reply to Lari Taskula from comment #2) > > Hey. Any update on this? I will start working on it if you haven't already. > > I attached Koha-objects to Bug 17499 which could be useful for this one. > > Hey there Lari. Yes, I've already done some work, but now I'm going to be > away from work on Koha until I finish final school exams for bachelor's > degree. > > If you want, you can take a look at the work & finish it ;) All the work is > in this single commit: > https://github.com/open-source-knihovna/KohaCZ/commit/ > 7bddf4f709fce467296ddce814b4612aeaa00088 > > Basically I made it work for GET request. Feel free to change my coding > style as you wish :D Cheers! It looks awesome! I will integrate your work with mine, and attach a patch here. Good luck with your exams!
Hi Lari, any progress on it? Jiri propably will ready to join us again and do some code on REST. So, we need to know actual state of Your work. Thank You.
(In reply to Mike from comment #5) > Hi Lari, > any progress on it? Jiri propably will ready to join us again and do some > code on REST. So, we need to know actual state of Your work. > > Thank You. Hi! Sorry for late answer, I was on holidays. Yes, I have some code ready, attaching it by tomorrow so you can review it. Nice to hear Jiri is joining you again!
Created attachment 64796 [details] [review] Bug 17505: REST API route for messaging preferences Get patron's / category's messaging preferences: GET /messaging_preferences?borrowernumber=123 - allowed if getting own preferences - otherwise borrowers flag required GET /messaging_preferences?categorycode=K - borrowers flag required Modify patron's / category's messaging preferences: PUT /messaging_preferences?borrowernumber=123 - allowed if modifying own preferences - otherwise borrowers flag required PUT /messaging_preferences?categorycode=K - borrowers flag required Returns the following JSON object (same for PUT except for readOnly values): { "Advanced_Notice": { "days_in_advance": { "configurable": true, "value": 20 }, "digest": { "configurable": true, "value": true }, "transport_types": { "email": true, "sms": false } }, "Another_Message_Name": { "days_in_advance": { "configurable": false, "value": null }, "digest": { ... }, ... } To test: 1. prove t/db_dependent/api/v1/messagingpreferences.t 2. Send GET and PUT requests to /api/v1/messaging_preferences
Depends on Bug 18137 and 17499. These Bugs will conflict in Koha/Exceptions.pm but should be easily fixable.
(In reply to Lari Taskula from comment #8) > Depends on Bug 18137 and 17499. These Bugs will conflict in > Koha/Exceptions.pm but should be easily fixable. I somehow cannot apply the patch, it shows following error: Apply? [(y)es, (n)o, (i)nteractive] y Applying: Bug 17505: REST API route for messaging preferences fatal: sha1 information is lacking or useless (api/v1/swagger/definitions.json). Repository lacks necessary blobs to fall back on 3-way merge. Cannot fall back to three-way merge. Patch failed at 0001 Bug 17505: REST API route for messaging preferences The copy of the patch that failed is found in: /home/vagrant/kohaclone/.git/rebase-apply/patch When you have resolved this problem run "git bz apply --continue". If you would prefer to skip this patch, instead run "git bz apply --skip". To restore the original branch and stop patching run "git bz apply --abort". Patch left in /tmp/Bug-17505-REST-API-route-for-messaging-preferences-QwXcYD.patch When trying to apply it manually using 'git apply /home/vagrant/kohaclone/.git/rebase-apply/patch -3 -v', it also shows an error: error: while searching for: "holds": { "$ref": "definitions/holds.json" }, "patron": { "$ref": "definitions/patron.json" } error: patch failed: api/v1/swagger/definitions.json:11 error: repository lacks the necessary blob to fall back on 3-way merge. error: api/v1/swagger/definitions.json: patch does not apply ... Didn't you forgot to add another dependency? I've applied both 17499 & 18137 on top of current master.
The only dependencies are Bug 18137 and Bug 17499... Are you sure all patches from Bug 18137 were applied? (Especially the "sort definitions.json"?) Because otherwise I don't see why it has an issue in definitions.json... However Bug 18137 needs a rebase anyway since it conflicts in the Holds controller after Bug 18276 and Bug 18260.
(In reply to Lari Taskula from comment #10) > The only dependencies are Bug 18137 and Bug 17499... Are you sure all > patches from Bug 18137 were applied? (Especially the "sort > definitions.json"?) Because otherwise I don't see why it has an issue in > definitions.json... However Bug 18137 needs a rebase anyway since it > conflicts in the Holds controller after Bug 18276 and Bug 18260. Well yes, I'm sure I've applied all the patches. It shows out that patches 18137 & 17499 must be applied in correct order. If I run "git bz apply 17505", it first wants to apply 17499 & then 18137, which results in a fail. But if I apply those patches manually in reversed order (18137 first & 17499 second), then it works.
To overcome testing problems with missing OpenApi (from bug 18137) one should run: $ sudo cpanm Mojolicious::Plugin::OpenAPI@1.10 .. but that doesn't work for me, here is my output: --> Working on Mojolicious::Plugin::OpenAPI Fetching http://backpan.perl.org/authors/id/J/JH/JHTHORSEN/Mojolicious-Plugin-OpenAPI-1.10.tar.gz ... OK Configuring Mojolicious-Plugin-OpenAPI-1.10 ... OK ==> Found dependencies: JSON::Validator, Mojolicious --> Working on JSON::Validator Fetching http://www.cpan.org/authors/id/J/JH/JHTHORSEN/JSON-Validator-1.00.tar.gz ... OK Configuring JSON-Validator-1.00 ... OK ==> Found dependencies: Mojolicious --> Working on Mojolicious Fetching http://www.cpan.org/authors/id/S/SR/SRI/Mojolicious-7.37.tar.gz ... OK Configuring Mojolicious-7.37 ... OK ==> Found dependencies: IO::Socket::IP --> Working on IO::Socket::IP Fetching http://www.cpan.org/authors/id/P/PE/PEVANS/IO-Socket-IP-0.39.tar.gz ... OK Configuring IO-Socket-IP-0.39 ... OK Building and testing IO-Socket-IP-0.39 ... OK Successfully installed IO-Socket-IP-0.39 (upgraded from 0.29) Building and testing Mojolicious-7.37 ... OK Successfully installed Mojolicious-7.37 (upgraded from 6.15) Building and testing JSON-Validator-1.00 ... OK Successfully installed JSON-Validator-1.00 (upgraded from 0.67) Building and testing Mojolicious-Plugin-OpenAPI-1.10 ... FAIL ! Installing Mojolicious::Plugin::OpenAPI failed. See /root/.cpanm/work/1501423561.19833/build.log for details. Retry with --force to force install it. 3 distributions installed What worked is this process as proposed by Tomás (https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18137#c54): $ sudo cpanm Mojolicious@7.31 $ sudo cpanm JSON::Validator@0.96 $ sudo cpanm Mojolicious::Plugin::OpenAPI@1.13
Created attachment 65326 [details] [review] Bug 17505: REST API route for messaging preferences Get patron's / category's messaging preferences: GET /messaging_preferences?borrowernumber=123 - allowed if getting own preferences - otherwise borrowers flag required GET /messaging_preferences?categorycode=K - borrowers flag required Modify patron's / category's messaging preferences: PUT /messaging_preferences?borrowernumber=123 - allowed if modifying own preferences - otherwise borrowers flag required PUT /messaging_preferences?categorycode=K - borrowers flag required Returns the following JSON object (same for PUT except for readOnly values): { "Advanced_Notice": { "days_in_advance": { "configurable": true, "value": 20 }, "digest": { "configurable": true, "value": true }, "transport_types": { "email": true, "sms": false } }, "Another_Message_Name": { "days_in_advance": { "configurable": false, "value": null }, "digest": { ... }, ... } To test: 1. prove t/db_dependent/api/v1/messagingpreferences.t 2. Send GET and PUT requests to /api/v1/messaging_preferences Signed-off-by: Jiri Kozlovsky <mail@jkozlovsky.cz>
I think the endpoint should be /patrons/<patron_id>/messaging_preferences and somehow like /categories/<category_id>/messaging_preferences It seems more logical to me... I am CCing to Tomas, to get his opinion
Tomas, could we get your opinion here, plz? :) Lari, did you see https://wiki.koha-community.org/wiki/REST_api_RFCs? I think it might be good to make some changes to make this endpoint match. For example: borrowernumber > patron_id categorycode > category_id
Hi Jiri, all looks good. We agreed new endpoints need to be proposed in the form of an RFC and voted on a dev meeting. This is mostly because (usually) the attribute names might require some tweaks. Could you please add an RFC [1], check the agreed coding guidelines for the REST api [2] and add the RFC for consideration on a dev meeting [3]? It should be (probably straight forward, but a needed step). I will help with the process if needed! Thanks for your contributions and looking forward to communicate with you about your work on the REST api! [1] https://wiki.koha-community.org/wiki/REST_api_RFCs [2] https://wiki.koha-community.org/wiki/Coding_Guidelines_-_API [3] https://wiki.koha-community.org/wiki/Next_IRC_meetings
(In reply to Tomás Cohen Arazi from comment #16) > Hi Jiri, all looks good. We agreed new endpoints need to be proposed in the > form of an RFC and voted on a dev meeting. This is mostly because (usually) > the attribute names might require some tweaks. Could you please add an RFC > [1], check the agreed coding guidelines for the REST api [2] and add the RFC > for consideration on a dev meeting [3]? It should be (probably straight > forward, but a needed step). I will help with the process if needed! > > Thanks for your contributions and looking forward to communicate with you > about your work on the REST api! > > [1] https://wiki.koha-community.org/wiki/REST_api_RFCs > [2] https://wiki.koha-community.org/wiki/Coding_Guidelines_-_API > [3] https://wiki.koha-community.org/wiki/Next_IRC_meetings As I know, Jiri is not working on Koha anymore, I tried to somehow make a first draft of RF for this: https://wiki.koha-community.org/wiki/Patrons_messaging_preferences_endpoint_RFC I wanted to simplify it, but I have not an idea if it will work ;) Maybe I over-simplify it ;) If you can have a quick look on it, I would really aprecciate it.
I'm open to simplifying, but some thoughts behind the current patch that I think should not be forgotten (most handled in the current patch): For building a selection list client-wise, user should know which messaging transport types they are able to select. * Which transport types are enabled in Koha, and which transport types are available for a particular message type? * Does patron have defined a valid contact information to select the transport type (it does not make sense to provide sms selection as transport type if patron has no smsalertnumber)? * Is digest available for this transport type? What if a library wants to force digest ON for some messages? * Is days in advance available for this message? Combine above with patron's current settings. Or do you want another endpoint for checking what options are available and another for displaying patron's current selections? Also consider checking the same restrictions when PUTting data. Some of these thoughts are probably outside of scope of this Bug but my main point is that we should announce the available configuration options to API consumer, not just the current selections for this patron.
(In reply to Lari Taskula from comment #18) > I'm open to simplifying, but some thoughts behind the current patch that I > think should not be forgotten (most handled in the current patch): > > For building a selection list client-wise, user should know which messaging > transport types they are able to select. > * Which transport types are enabled in Koha, and which transport types are > available for a particular message type? > * Does patron have defined a valid contact information to select the > transport type (it does not make sense to provide sms selection as transport > type if patron has no smsalertnumber)? > * Is digest available for this transport type? What if a library wants to > force digest ON for some messages? > * Is days in advance available for this message? > > Combine above with patron's current settings. Or do you want another > endpoint for checking what options are available and another for displaying > patron's current selections? > > Also consider checking the same restrictions when PUTting data. > > Some of these thoughts are probably outside of scope of this Bug but my main > point is that we should announce the available configuration options to API > consumer, not just the current selections for this patron. Maybe there's room for a separate endpoint, I'd say.
Left some comments on the wiki, hope they make sense!
Does someone working on this BZ recently ?
*** Bug 23718 has been marked as a duplicate of this bug. ***
Created attachment 151912 [details] [review] Bug 17505: REST API route for messaging preferences Get patron's / category's messaging preferences: GET /messaging_preferences?borrowernumber=123 - allowed if getting own preferences - otherwise borrowers flag required GET /messaging_preferences?categorycode=K - borrowers flag required Modify patron's / category's messaging preferences: PUT /messaging_preferences?borrowernumber=123 - allowed if modifying own preferences - otherwise borrowers flag required PUT /messaging_preferences?categorycode=K - borrowers flag required Returns the following JSON object (same for PUT except for readOnly values): { "Advanced_Notice": { "days_in_advance": { "configurable": true, "value": 20 }, "digest": { "configurable": true, "value": true }, "transport_types": { "email": true, "sms": false } }, "Another_Message_Name": { "days_in_advance": { "configurable": false, "value": null }, "digest": { ... }, ... } To test: 1. prove t/db_dependent/api/v1/messagingpreferences.t 2. Send GET and PUT requests to /api/v1/messaging_preferences Signed-off-by: Jiri Kozlovsky <mail@jkozlovsky.cz>
Patch rebased on master. What is needed to make this patch move forward ? A vote on the RFC ?
I would also like to know what would be needed to make this move again.
Hi, in my opinion this needs to be redone. Besides naming conventions (e.g. borrowernumber) we have moved forward in some things a while back. My thoughts: We need: GET /patron_categories/:patron_category_id/messaging_preferences GET /patrons/:patron_id/messaging_preferences As for CRUD operations, I don't feel comfortable with just doing PUT /patron_categories/:patron_category_id/messaging_preferences with all the message types and transports. We need more fine-grained control. So probably PUT /patrons/:patron_id/messaging_preferences/:message_type And as I said some time ago, I think we also need some endpoint to fetch the valid message types and also transport types.