[Bug 28026] New: Add a 'call_recursive' method to Koha::Plugins
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28026 Bug ID: 28026 Summary: Add a 'call_recursive' method to Koha::Plugins Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: enhancement Priority: P5 - low Component: Plugin architecture Assignee: koha-bugs@lists.koha-community.org Reporter: martin.renvoize@ptfs-europe.com QA Contact: testopia@bugs.koha-community.org Blocks: 26351 We have a 'call' method already which can be used to call plugin hooks iteratively with the original arguments each time. It would be helpful to also have a method to allow continuation-passing by passing the returned arguments from one plugin call to the next. Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26351 [Bug 26351] Add plugin hooks to transform item barcodes -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28026 --- Comment #1 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 118679 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=118679&action=edit Bug 28026: Add call_recursive() as a supliment for call() The method call() is not sufficient for all plugin hook types. It's possible that more than one plugin will be installed that wants to return the arguaments passed in an updated form. These transformation plugins need to work recursively rather than independantly. This patch adss a `call_recursive()` method that takes the output of the first plugin and uses it as the input for the next plugin and so on. This allowes each plugin to see the current version of the arguament list and modify it as necessary. -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28026 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|testopia@bugs.koha-communit |martin.renvoize@ptfs-europe |y.org |.com Assignee|koha-bugs@lists.koha-commun |kyle@bywatersolutions.com |ity.org | -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28026 --- Comment #2 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Migrated this patch from bug 26351 so it gets it's own mention in the release notes. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28026 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28026 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #118679|0 |1 is obsolete| | --- Comment #3 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 118680 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=118680&action=edit Bug 28026: Add call_recursive() as a supliment for call() The method call() is not sufficient for all plugin hook types. It's possible that more than one plugin will be installed that wants to return the arguaments passed in an updated form. These transformation plugins need to work recursively rather than independantly. This patch adss a `call_recursive()` method that takes the output of the first plugin and uses it as the input for the next plugin and so on. This allowes each plugin to see the current version of the arguament list and modify it as necessary. Test plan 1/ Run the included tests - t/db_dependent/Koha/Plugins/Plugins.t Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28026 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |koha@trust-box.at -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28026 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28026 --- Comment #4 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- I'd love a comment/signoff from Mark Hofstetter to confirm this meets the needs he identifies in bug 28010.. and I think it's worth a QA eye other than my own as it skipped Signoff as a QA followup on it's "birth bug" -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28026 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=28010 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28026 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |katrin.fischer@bsz-bw.de, | |nick@bywatersolutions.com, | |tomascohen@gmail.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28026 Julian Maurice <julian.maurice@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |julian.maurice@biblibre.com --- Comment #5 from Julian Maurice <julian.maurice@biblibre.com> --- I suggested something that would not require adding a new method nor modifying the 'call' method (https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26351#c7) What do you think of it ? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28026 --- Comment #6 from Kyle M Hall <kyle@bywatersolutions.com> --- (In reply to Julian Maurice from comment #5)
I suggested something that would not require adding a new method nor modifying the 'call' method (https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26351#c7) What do you think of it ?
There's a decent amount of code utilizing call_recursive now, so I'm a bit hesitant to make changes to code that has passed qa. If you want to submit your idea is a dependency on 26352 I'd absolutely take a look! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28026 --- Comment #7 from Mark Hofstetter <koha@trust-box.at> --- I doubt that call() is sufficient, and as call_recursive() just adds functionality and does not interfere with the rest of teh plugin system. I'll try to QA it. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28026 --- Comment #8 from Julian Maurice <julian.maurice@biblibre.com> --- (In reply to Mark Hofstetter from comment #7)
I doubt that call() is sufficient, and as call_recursive() just adds functionality and does not interfere with the rest of teh plugin system. I'll try to QA it.
It is sufficient. I tested it. It's just a little less convenient for the caller when you want scalars to be modified (like in bug 26351 and bug 26252). Otherwise for objects it works very well: Koha::Plugins->call('object_transform', $object) will allow $object to be modified by all plugins. This is also true for hashrefs and arrayrefs. I believe call_recursive would be useful only for scalar values (and 'call' can already be used for that as shown in bug 26351 comment 7). And it duplicates the code of Koha::Plugins::call, which should be avoided IMO. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28026 --- Comment #9 from Julian Maurice <julian.maurice@biblibre.com> --- (In reply to Julian Maurice from comment #8)
[...] and bug 26252 bug 26352
-- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28026 --- Comment #10 from Mark Hofstetter <koha@trust-box.at> --- beeing new to (contributing to) Koha, I'd really to test/QA whatever comes out of this discussion (if this would be of help), but honestly I don't know what really to test - could somebody please give me a helping hand? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28026 --- Comment #11 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Usually for sign-off you start with the test plan in the commit message... but here it's a little sparse: Test plan 1/ Run the included tests - t/db_dependent/Koha/Plugins/Plugins.t Checking coding guidelines etc. is great, but not expected for sign off, but for QA. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28026 --- Comment #12 from Kyle M Hall <kyle@bywatersolutions.com> --- (In reply to Julian Maurice from comment #8) Bug 28211 is ready to sign off Julian! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28026 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |m.de.rooy@rijksmuseum.nl --- Comment #13 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Please merge with the other one -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28026 --- Comment #14 from Kyle M Hall <kyle@bywatersolutions.com> --- (In reply to Marcel de Rooy from comment #13)
Please merge with the other one
I'm not sure what bug you are referring too, but this is a series of 4 bugs now, so I don't think that would be practical. It would require modifying two bugs that are already passed qa. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28026 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Patch doesn't apply -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28026 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Patch doesn't apply |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28026 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #118680|0 |1 is obsolete| | --- Comment #15 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 122443 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=122443&action=edit Bug 28026: Add call_recursive() as a supliment for call() The method call() is not sufficient for all plugin hook types. It's possible that more than one plugin will be installed that wants to return the arguaments passed in an updated form. These transformation plugins need to work recursively rather than independantly. This patch adss a `call_recursive()` method that takes the output of the first plugin and uses it as the input for the next plugin and so on. This allowes each plugin to see the current version of the arguament list and modify it as necessary. Test plan 1/ Run the included tests - t/db_dependent/Koha/Plugins/Plugins.t Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28026 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |dcook@prosentient.com.au --- Comment #16 from David Cook <dcook@prosentient.com.au> --- I think "call_pipeline" would've been better than "call_recursive", since I would expect "call_recursive" to invoke itself, instead of just pipelining results through a series of plugin calls. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28026 --- Comment #17 from Mark Hofstetter <koha@trust-box.at> --- call_pipeline is a very fine name for me, I finally found the time to setup a proper devel system for koha. if somebody could find the time (30minutes) to chat with me how to do commit/package it/test it up to community standards I'd be happy to work on the patch -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28026 --- Comment #18 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Please see discussion and resulting bug 28211 where call_recursive is subsequently removed again in preference to passing references. If you don't believe this can fulfil your requirements Mark please let us know here to Julian and Kyle can chip in. Many thanks, -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28026 --- Comment #19 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Also, In bug 27066 comment 10 I talked about the concept of 'before', 'after' and 'around' as a hook naming convention.. I feel the hook names should be tied to their actions and effects.. So, in many ways I think 'before' should be passed the data prior to an action taking place and it's use should not affect said days, after should be passed the data after the action has taken place and again not affect that data.. around could be used for the case where we want to intercept and alter that data in the plugin... thoughts on such a naming convention would be great. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28026 --- Comment #20 from David Cook <dcook@prosentient.com.au> --- (In reply to Martin Renvoize from comment #18)
Please see discussion and resulting bug 28211 where call_recursive is subsequently removed again in preference to passing references.
I was wondering why call_recursive was copying data rather than passing references hehe. (In reply to Martin Renvoize from comment #19)
Also, In bug 27066 comment 10 I talked about the concept of 'before', 'after' and 'around' as a hook naming convention.. I feel the hook names should be tied to their actions and effects..
So, in many ways I think 'before' should be passed the data prior to an action taking place and it's use should not affect said days, after should be passed the data after the action has taken place and again not affect that data.. around could be used for the case where we want to intercept and alter that data in the plugin... thoughts on such a naming convention would be great.
+1 from me. I use before/after/around with frameworks on other system, and it's very easy. I like the predictability of knowing when before/after/around hooks are called. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28026 --- Comment #21 from Mark Hofstetter <koha@trust-box.at> --- As stated before I consider the plugin system to explicit ie you have to "tell" every function/method that there may be a plugin. For example something I am currently working on: Problem: Customer wants to change how patron cardnumbers are autogenerated, (a prefix and a fixed padded length) One could make a plugin for Koha::Patron::fixup_cardnumber OR, my suggestion (semi pseudo code, I hop my intentions are clear) extend the base class(es) in this case Koha::Object->_new_from_dbic(); if (<hooksenabled>) { require KohaObjectHooks; # whatever } KohaObjectHooks use Hook::LexWrap; wrap 'Koha::Patron::fixup_cardnumber', post => sub { my ( $self ) = @_; $self->cardnumber( sprintf("LB%09d", $self->cardnumber)); }; Hook::LexWrap hat pre/post a around hook may be implemented easily I know this a major change, but solves a least some problems -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28026 --- Comment #22 from David Cook <dcook@prosentient.com.au> --- (In reply to Mark Hofstetter from comment #21)
OR, my suggestion (semi pseudo code, I hop my intentions are clear)
I'm sorry but I didn't really follow that. But that might just be me. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28026 --- Comment #23 from Kyle M Hall <kyle@bywatersolutions.com> --- (In reply to Mark Hofstetter from comment #21)
As stated before I consider the plugin system to explicit ie you have to "tell" every function/method that there may be a plugin.
For example something I am currently working on:
Problem: Customer wants to change how patron cardnumbers are autogenerated, (a prefix and a fixed padded length)
One could make a plugin for Koha::Patron::fixup_cardnumber
OR, my suggestion (semi pseudo code, I hop my intentions are clear)
extend the base class(es) in this case
Koha::Object->_new_from_dbic();
if (<hooksenabled>) { require KohaObjectHooks; # whatever }
KohaObjectHooks
use Hook::LexWrap; wrap 'Koha::Patron::fixup_cardnumber', post => sub { my ( $self ) = @_; $self->cardnumber( sprintf("LB%09d", $self->cardnumber)); };
Hook::LexWrap hat pre/post a around hook may be implemented easily
I know this a major change, but solves a least some problems
That's interesting. I'd love to see a proof of concept! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28026 --- Comment #24 from David Cook <dcook@prosentient.com.au> --- I think I understand a little better now, although I still don't understand the logic flow. You'd have to load the plugin to override the original code, but your example doesn't say when that would happen. Unless you're preloading modules at startup time (which will only work with Plack and could mean large worker processes), then you're going to have to load at script load time, but then you'll probably want to be targeting particular plugins for particular situations, and then I'm not sure that you're actually gaining any advantage over the current way of doing things? Hook::LexWrap is also black magic. Perl lets a person do all kinds of wild things, but it reminds me of Jurassic Park: "Your scientists were so preoccupied with whether or not they could, they didn't stop to think if they should." -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28026 --- Comment #25 from Mark Hofstetter <koha@trust-box.at> --- Created attachment 122527 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=122527&action=edit THIS IS NOT A BUGFIX JUST A PROVE OF CONCEPT just run t/patron-object-hook.t and see what happens to make this work properly something like https://metacpan.org/pod/Module::Pluggable should be used to find & include the plugins Signed-off-by: Mark Hofstetter <mark@hofstetter.at> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28026 Mark Hofstetter <koha@trust-box.at> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |In Discussion --- Comment #26 from Mark Hofstetter <koha@trust-box.at> --- " Hook::LexWrap is also black magic. Perl lets a person do all kinds of wild things, but it reminds me of Jurassic Park: "Your scientists were so preoccupied with whether or not they could, they didn't stop to think if they should." " As I've got a background in biochemistry I am huge fan of tinkering with old (genetic) code Hook::LexWrap is widely used (and written by Damien), so it's not too bad please have a look at it, if the idea makes sense for you I'll make it pluggable (probably with something like Module::Pluggable) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28026 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|In Discussion |Signed Off --- Comment #27 from Kyle M Hall <kyle@bywatersolutions.com> --- (In reply to Mark Hofstetter from comment #26)
" Hook::LexWrap is also black magic. Perl lets a person do all kinds of wild things, but it reminds me of Jurassic Park: "Your scientists were so preoccupied with whether or not they could, they didn't stop to think if they should." "
As I've got a background in biochemistry I am huge fan of tinkering with old (genetic) code
Hook::LexWrap is widely used (and written by Damien), so it's not too bad
please have a look at it, if the idea makes sense for you I'll make it pluggable (probably with something like Module::Pluggable)
Mark, this is not the right place to put your patch. Can you please open a new b ug report and put it there? Thanks! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28026 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #122527|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28026 --- Comment #28 from Kyle M Hall <kyle@bywatersolutions.com> --- (In reply to Kyle M Hall from comment #27)
(In reply to Mark Hofstetter from comment #26)
" Hook::LexWrap is also black magic. Perl lets a person do all kinds of wild things, but it reminds me of Jurassic Park: "Your scientists were so preoccupied with whether or not they could, they didn't stop to think if they should." "
As I've got a background in biochemistry I am huge fan of tinkering with old (genetic) code
Hook::LexWrap is widely used (and written by Damien), so it's not too bad
please have a look at it, if the idea makes sense for you I'll make it pluggable (probably with something like Module::Pluggable)
Mark, this is not the right place to put your patch. Can you please open a new b ug report and put it there? Thanks!
And please add me on that bug! This idea is very interesting but definitely warrants discussion on its own bug. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28026 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA --- Comment #29 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- # Subtest: call_recursive() tests 1..6 ok 1 - Got back one element not ok 2 - Got expected response # Failed test 'Got expected response' # at t/db_dependent/Koha/Plugins/Plugins.t line 97. # got: '1' # expected: '4' ok 3 - Got back one element ok 4 - Got expected response ok 5 - Got back one element ok 6 - call_recursive should return the original arguments if plugins are disabled # Looks like you failed 1 test of 6. not ok 6 - call_recursive() tests -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28026 Mark Hofstetter <koha@trust-box.at> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |In Discussion --- Comment #30 from Mark Hofstetter <koha@trust-box.at> --- sorry for the noise, moved the "proposal" to https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28655 please don't hesitate to correct/nudge me to koha community standards as I am new at commiting to this project -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28026 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|In Discussion |Failed QA --- Comment #31 from Tomás Cohen Arazi <tomascohen@gmail.com> --- (In reply to Marcel de Rooy from comment #29)
# Subtest: call_recursive() tests 1..6 ok 1 - Got back one element not ok 2 - Got expected response # Failed test 'Got expected response' # at t/db_dependent/Koha/Plugins/Plugins.t line 97. # got: '1' # expected: '4' ok 3 - Got back one element ok 4 - Got expected response ok 5 - Got back one element ok 6 - call_recursive should return the original arguments if plugins are disabled # Looks like you failed 1 test of 6. not ok 6 - call_recursive() tests
Same here, @khall? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28026 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28026 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #122443|0 |1 is obsolete| | --- Comment #32 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 123788 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=123788&action=edit Bug 28026: Add call_recursive() as a supliment for call() The method call() is not sufficient for all plugin hook types. It's possible that more than one plugin will be installed that wants to return the arguaments passed in an updated form. These transformation plugins need to work recursively rather than independantly. This patch adss a `call_recursive()` method that takes the output of the first plugin and uses it as the input for the next plugin and so on. This allowes each plugin to see the current version of the arguament list and modify it as necessary. Test plan 1/ Run the included tests - t/db_dependent/Koha/Plugins/Plugins.t Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28026 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #123788|0 |1 is obsolete| | --- Comment #33 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 123789 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=123789&action=edit Bug 28026: Add call_recursive() as a supplement for call() The method call() is not sufficient for all plugin hook types. It's possible that more than one plugin will be installed that wants to return the arguaments passed in an updated form. These transformation plugins need to work recursively rather than independantly. This patch adss a `call_recursive()` method that takes the output of the first plugin and uses it as the input for the next plugin and so on. This allowes each plugin to see the current version of the arguament list and modify it as necessary. Test plan 1/ Run the included tests - t/db_dependent/Koha/Plugins/Plugins.t Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28026 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28026 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #123789|0 |1 is obsolete| | --- Comment #34 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 123829 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=123829&action=edit Bug 28026: Add call_recursive() as a supplement for call() The method call() is not sufficient for all plugin hook types. It's possible that more than one plugin will be installed that wants to return the arguaments passed in an updated form. These transformation plugins need to work recursively rather than independantly. This patch adss a `call_recursive()` method that takes the output of the first plugin and uses it as the input for the next plugin and so on. This allowes each plugin to see the current version of the arguament list and modify it as necessary. Test plan 1/ Run the included tests - t/db_dependent/Koha/Plugins/Plugins.t Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28026 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #123829|0 |1 is obsolete| | --- Comment #35 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 124311 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=124311&action=edit Bug 28026: Add call_recursive() as a supplement for call() The method call() is not sufficient for all plugin hook types. It's possible that more than one plugin will be installed that wants to return the arguaments passed in an updated form. These transformation plugins need to work recursively rather than independantly. This patch adss a `call_recursive()` method that takes the output of the first plugin and uses it as the input for the next plugin and so on. This allowes each plugin to see the current version of the arguament list and modify it as necessary. Test plan 1/ Run the included tests - t/db_dependent/Koha/Plugins/Plugins.t Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28026 --- Comment #36 from Mark Hofstetter <koha@trust-box.at> --- this is a rehash of my previous argument, I'll look at the code. How to you gurantee that the plugins are executed in a certain order because that's what you somehow imply by "method that takes the output of the first plugin and uses it as the input for the next plugin [..]" -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28026 --- Comment #37 from Kyle M Hall <kyle@bywatersolutions.com> --- (In reply to Mark Hofstetter from comment #36)
this is a rehash of my previous argument, I'll look at the code.
How to you gurantee that the plugins are executed in a certain order because that's what you somehow imply by "method that takes the output of the first plugin and uses it as the input for the next plugin [..]"
There is no guarantee, and plugins should be written with that understanding. It would be possible to some day give all plugins a "priority" but at this time every plugin author should understand their plugin could be run first, last and or anywhere in between. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28026 --- Comment #38 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- @RM: What holds you back from pushing this development? This makes me doubt if I should QA things that depend on it. If you have doubts on this one, please indicate so on the report? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28026 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to master Version(s)| |21.11.00 released in| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28026 --- Comment #39 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Pushed to master for 21.11, thanks to everybody involved! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28026 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Resolution|--- |FIXED Status|Pushed to master |RESOLVED -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org