|
Description
Stefan Berndtsson
2022-10-20 16:43:23 UTC
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! :) |