Some of our clients use external systems to populate Koha's patron information. They can use Koha's API to update patron info, however, we don't have any way (that I know of) to notify an external system when a patron info is changed in Koha. In order to allow bidirectional communications, we are planning to add hooks. This will allow creating plugins to send messages to external systems in order to fulfil the following requirements : - Update patron information stored in the borrowers table (name, address, email, etc.) - Notify an external system when a restriction is placed on a user (fines, manual restrictions, overdues, etc.)
Since we have hooks after_item_action and after_biblio_action, I'd say add after_patron_action hook. See https://wiki.koha-community.org/wiki/Koha_Plugin_Hooks
Created attachment 116007 [details] [review] Bug 27066: Add plugin hook after_patron_action The hook is called after patron manipulations. The value of the 'action' parameter can be 'create', 'modify' or 'delete'. To test: 1. Write a simple plugin that implements at least the new after_patron_action hook. 2. Install the plugin and enable it. 3. Create, modify or delete a patron and verify that the plugin method for the hook has been called with the right parameters.
Created attachment 116008 [details] [review] Bug 27066: Unit tests for after_patron_action hook Adds unit tests for the new hook. To test: prove -v t/db_dependent/Koha/Plugins/Patron.t
Created attachment 117867 [details] [review] Bug 27066: Add plugin hook after_patron_action The hook is called after patron manipulations. The value of the 'action' parameter can be 'create', 'modify' or 'delete'. To test: 1. Write a simple plugin that implements at least the new after_patron_action hook. 2. Install the plugin and enable it. 3. Create, modify or delete a patron and verify that the plugin method for the hook has been called with the right parameters. Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Created attachment 117868 [details] [review] Bug 27066: Unit tests for after_patron_action hook Adds unit tests for the new hook. To test: prove -v t/db_dependent/Koha/Plugins/Patron.t Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Works has expected, looks a lot like "after_item_action"
Please document on https://wiki.koha-community.org/wiki/Koha_Plugin_Hooks Looking at the code and comparing with the similar after_item_action hooks I wonder about the placement in the code. The implementation on bug 22709 adds the hooks before the logs are written, this adds them after. I could imagine that plugins could actually change the data, so would result in a different logging result. So I think "before" would be better. What do you think?
(In reply to Katrin Fischer from comment #7) > Please document on https://wiki.koha-community.org/wiki/Koha_Plugin_Hooks > > Looking at the code and comparing with the similar after_item_action hooks I > wonder about the placement in the code. > > The implementation on bug 22709 adds the hooks before the logs are written, > this adds them after. > > I could imagine that plugins could actually change the data, so would result > in a different logging result. So I think "before" would be better. What do > you think? I agree we must have same behavior. But I see _after_item_action_hooks is at the very end of store() : https://git.koha-community.org/Koha-community/Koha/src/commit/4f96b6767a1be022a4e38a856491b2b1eae782fc/Koha/Item.pm#L202 After action logs and search engine indexing. Note that a main difference is that patron store() is in a DB transaction.
(In reply to Eric Bégin from comment #0) > Some of our clients use external systems to populate Koha's patron > information. > > They can use Koha's API to update patron info, however, we don't have any > way (that I know of) to notify an external system when a patron info is > changed in Koha. > > In order to allow bidirectional communications, we are planning to add hooks. > > This will allow creating plugins to send messages to external systems in > order to fulfil the following requirements : > - Update patron information stored in the borrowers table (name, address, > email, etc.) > - Notify an external system when a restriction is placed on a user (fines, > manual restrictions, overdues, etc.) Hi, maybe it is an interesting opportunity to look at adding hooks for more fine grained things. Because in that hook, it seems you will end up checking all those things (DB access). And if the hooks were added to other places like when a debarment is generated, then you would have the debarment object in context, and no DB access would be required.
(In reply to Katrin Fischer from comment #7) > Please document on https://wiki.koha-community.org/wiki/Koha_Plugin_Hooks > > Looking at the code and comparing with the similar after_item_action hooks I > wonder about the placement in the code. > > The implementation on bug 22709 adds the hooks before the logs are written, > this adds them after. > > I could imagine that plugins could actually change the data, so would result > in a different logging result. So I think "before" would be better. What do > you think? Hmmm, tl;dr I don't think 'before' or 'after' hooks should directly affect the data of the action. We probably want 'before' and 'after' hooks.. and maybe even an 'around'. Generally, a 'before' hook is for triggering actions prior to store (but not affecting the data for the store itself).. 'after' is for triggering actions after the store (like triggering zebraqueue.. again, it shouldn't affect the data of the store itself) and an around wraps the store and returns the updated data for the store to then act upon. At least, that's my vague understanding of 'before', 'after', 'around' in other frameworks?
(In reply to Eric Bégin from comment #0) > - Update patron information stored in the borrowers table (name, address, > email, etc.) I would say that this plugin works perfectly for the case where you want to trigger the notification of other systems.. but I do not think, as it stands, it works for the above statement (and I'm not sure it should.. we should implement an 'around' for that in my opinion.)
(In reply to Tomás Cohen Arazi from comment #9) > (In reply to Eric Bégin from comment #0) > > Hi, maybe it is an interesting opportunity to look at adding hooks for more > fine grained things. Because in that hook, it seems you will end up checking > all those things (DB access). And if the hooks were added to other places > like when a debarment is generated, then you would have the debarment object > in context, and no DB access would be required. Also agree with this.. it would be nice to add more specific hooks.. but I don't see that as a blocker for this one.
(In reply to Katrin Fischer from comment #7) > Please document on https://wiki.koha-community.org/wiki/Koha_Plugin_Hooks > > Looking at the code and comparing with the similar after_item_action hooks I > wonder about the placement in the code. > > The implementation on bug 22709 adds the hooks before the logs are written, > this adds them after. > > I could imagine that plugins could actually change the data, so would result > in a different logging result. So I think "before" would be better. What do > you think? To come back to the original question - do we care if the plugin calls are before of after logaction and should we be consistent? :)
(In reply to Katrin Fischer from comment #13) > (In reply to Katrin Fischer from comment #7) > > Please document on https://wiki.koha-community.org/wiki/Koha_Plugin_Hooks > > > > Looking at the code and comparing with the similar after_item_action hooks I > > wonder about the placement in the code. > > > > The implementation on bug 22709 adds the hooks before the logs are written, > > this adds them after. > > > > I could imagine that plugins could actually change the data, so would result > > in a different logging result. So I think "before" would be better. What do > > you think? > > To come back to the original question - do we care if the plugin calls are > before of after logaction and should we be consistent? :) The questions are related ;) If 'before' and 'after' actions don't affect the data of the object.. then they wont affect the actionlogs call.. so it shouldn't matter if they're before or after. Personally.. I prefer to have them as late as possible, so after, but that's just a style thing.
(In reply to Martin Renvoize from comment #14) > If 'before' and 'after' actions don't affect the data of the object.. then > they wont affect the actionlogs call.. so it shouldn't matter if they're > before or after. Personally.. I prefer to have them as late as possible, so > after, but that's just a style thing. I agree - the log should come before the after action hook - as the log is that state when Koha is done with its work The after action hook shouldn't change the data, but it seems more stable to put our log before passing the object, in case something outside does change it - we can at least record what Koha thinks the object should be
How do we continue here ?
(In reply to Martin Renvoize from comment #10) > tl;dr I don't think 'before' or 'after' hooks should directly affect the > data of the action. > > We probably want 'before' and 'after' hooks.. and maybe even an 'around'. > > Generally, a 'before' hook is for triggering actions prior to store (but not > affecting the data for the store itself).. 'after' is for triggering actions > after the store (like triggering zebraqueue.. again, it shouldn't affect the > data of the store itself) and an around wraps the store and returns the > updated data for the store to then act upon. > > At least, that's my vague understanding of 'before', 'after', 'around' in > other frameworks? That's an interesting point. I hadn't thought of that...