We should standardize the way plugin hooks are implemented and behave. Looking at: https://wiki.koha-community.org/wiki/Koha_Plugin_Hooks https://github.com/bywatersolutions/dev-koha-plugin-kitchen-sink Some hooks are called like: Koha::Plugins->new()->GetPlugins( { method => 'background_tasks', } ); and then the respective method invoked Others are just called like: Koha::Plugins->call('patron_barcode_transform', \$patron_id ); Some hooks return stuff (e.g. opac_detail_xslt_variables), others expect the param to be a ref and change in place without returning anything (e.g. patron_barcode_transform) --- Discussion: We already have hooks that behave like actions, and others that behave like filters. Wordpress put's it this way: - an action interrupts the code flow to do something, and then returns back to the normal flow without modifying anything; - a filter is used to modify something in a specific way so that the modification is then used by code later on. My proposal is: - Make it clear that a hook is either an action or a filter, either by name or by the way it's called - All actions should implement the same (example) Plugins->call('action_name', $args) signature - All filters should implement the same (example) Plugins->call('filter_name', $args) signature and return $args
(In reply to Pedro Amorim from comment #0) > We should standardize the way plugin hooks are implemented and behave. > Others are just called like: > Some hooks return stuff (e.g. opac_detail_xslt_variables), others expect the > param to be a ref and change in place without returning anything (e.g. > patron_barcode_transform) The ->call pattern is more recent. I think we should go in that direction. > We already have hooks that behave like actions, and others that behave like > filters. > - Make it clear that a hook is either an action or a filter, either by name > or by the way it's called Not sure what you exactly mean with a filter here. But a plugin could modify or just return results that allow further customization. If you pass a ref to a plugin, you wont know what will happen. So you cant say that it is just a filter or so? Passing that ref means that it may be changed. Calling the plugin may lead to db changes too. The name is no protection..
I don't think the distinction between filters and actions is that important. A plugin method can do both. For instance an item_barcode_transform hook can modify the barcode, or log it somewhere, or both. Sure, the caller sometimes expects the plugin to act as a filter (like item_barcode_transform) but most of the times it does not expect anything. It's more like "This event just happened (or will happen right now), do what you want with that info" I think standardizing name cannot be a bad thing, but the thing that matters the most (to me at least) is a proper documentation of the hook arguments: what they contain and if (and how) they are used after the hook returns. In some implementations of an event manager, the event listener (the hook) gets an object acting as a container for the event and its parameter. Maybe we can take inspirations from that to standardize the hook signatures.
(In reply to Marcel de Rooy from comment #1) > The ->call pattern is more recent. I think we should go in that direction. Great! I agree. I think any eventual future documentation should have this as a pattern to be adopted (for new hooks being added in the future). (In reply to Julian Maurice from comment #2) > I think standardizing name cannot be a bad thing, but the thing that matters > the most (to me at least) is a proper documentation of the hook arguments: > what they contain and if (and how) they are used after the hook returns. Yes, I agree and this is something I left out of my initial post but something I strongly believe we need: Proper documentation (and usage examples) of all available plugin hooks, as well as their arguments and return value - if any. The best closest thing we have atm is the kitchen sink plugin's actual code imo. @Marcel the idea behind "filter" versus "action" would be to make it clear, right off the bat, to anyone implementing any hook whether it just triggers an action (and has no return value) e.g. after_biblio_action or if the hook filters some args and returns that after some modifications have been made e.g. patron_barcode_transform I only used the naming argument as a starting point of discussion regarding standardization and I don't feel strongly one way or another. I started that as a suggestion but I believe the main 2 issues are: - Hooks are not standardized - each hook has its own way of handling arguments, and in different ways, or no arguments at all. - Docs are poor, the koha plugin hooks page is better than nothing but a bit lacking in terms of dev documentation or usage examples. On a wider topic: https://perldoc.koha-community.org/ - Doesn't look like it's been maintained (20.05 is the latest listed there). And looks ugly =D I wonder if we could use something like https://about.readthedocs.com/ for Koha, to document patterns we use, not just plugin related. Or a (better) way of getting documentation generated using POD from the code.