This patch adds a new hook "after_recall_action" with the action "add". This is consistent with when UpdateStats is called so that a plugin can be used to log the same kind of data as the normal statistics.
Created attachment 142267 [details] [review] Bug 31896: New after_recall_action hook Hooks added: after_recall_action with new action add How to test: Run tests in t/db_dependent/Koha/Plugins/Recall_hooks.t Sponsored by: Gothenburg University Library
Created attachment 142279 [details] [review] Bug 31896: New after_recall_action hook Hooks added: after_recall_action with new action add How to test: Run tests in t/db_dependent/Koha/Plugins/Recall_hooks.t Sponsored by: Gothenburg University Library Signed-off-by: David Nind <david@davidnind.com>
Note that I got a whitespace error warning - it may pay to fix this so that there is one less thing for the QA Team to deal with: Applying: Bug 31896: New after_recall_action hook .git/rebase-apply/patch:102: trailing whitespace. warning: 1 line adds whitespace errors. Using index info to reconstruct a base tree... M Koha/Recalls.pm .git/rebase-apply/patch:102: trailing whitespace. warning: 1 line applied after fixing whitespace errors. Falling back to patching base and 3-way merge... Auto-merging Koha/Recalls.pm
FAIL t/db_dependent/Koha/Plugins/Recall_hooks.t OK critic FAIL file permissions File must have the exec flag Tip: Please run qa tools on your patches before submitting?
Created attachment 142307 [details] [review] Bug 31896: New after_recall_action hook Hooks added: after_recall_action with new action add How to test: Run tests in t/db_dependent/Koha/Plugins/Recall_hooks.t Sponsored by: Gothenburg University Library
Now passes QA in my system. Fixed file permissions and trailing whitespace.
Created attachment 142331 [details] [review] Bug 31896: New after_recall_action hook Hooks added: after_recall_action with new action add How to test: Run tests in t/db_dependent/Koha/Plugins/Recall_hooks.t Sponsored by: Gothenburg University Library Signed-off-by: David Nind <david@davidnind.com>
+ $schema->storage->txn_rollback; + Koha::Plugins::Methods->delete;
+sub after_recall_action { + my ( $self, $params ) = @_; + + my $action = $params->{action}; + my $recall = $params->{payload}->{recall}; + + Koha::Exception->throw( + "after_recall_action called with action: $action, ref: " . ref($recall) ); +} Yeah, I understand you doing that. It does not really test more than the thing was called. No blocker, just noting.
Please come back after 31894
Working here now
Created attachment 142757 [details] [review] Bug 31896: New after_recall_action hook Hooks added: after_recall_action with new action add How to test: Run tests in t/db_dependent/Koha/Plugins/Recall_hooks.t Sponsored by: Gothenburg University Library Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 142758 [details] [review] Bug 31896: (QA follow-up) Fix modules Triggered by seeing Koha::Account. Copy and paste ;) But a bunch of other modules should (at least formally) be there. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 142759 [details] [review] Bug 31896: (QA follow-up) Fix rollback Yes, it should be the last statement. Actually the preceding delete is unneeded. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
I will restore the get_from_storage here in the first patch to be consistent. With a FIXME
Created attachment 143169 [details] [review] Bug 31896: New after_recall_action hook Hooks added: after_recall_action with new action add How to test: Run tests in t/db_dependent/Koha/Plugins/Recall_hooks.t Sponsored by: Gothenburg University Library Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> [EDIT} Adding get_from_storage ;) For current consistency. See further bug 32107.
Created attachment 143170 [details] [review] Bug 31896: (QA follow-up) Fix modules Triggered by seeing Koha::Account. Copy and paste ;) But a bunch of other modules should (at least formally) be there. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 143171 [details] [review] Bug 31896: (QA follow-up) Fix rollback Yes, it should be the last statement. Actually the preceding delete is unneeded. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Pushed to master for 22.11. Nice work everyone, thanks!
Please remember to update https://wiki.koha-community.org/wiki/Koha_Plugin_Hooks! :)