Bug 31896 - New recall hook when adding recall
Summary: New recall hook when adding recall
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Plugin architecture (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Stefan Berndtsson
QA Contact: Marcel de Rooy
URL:
Keywords:
Depends on: 31895
Blocks: 31734
  Show dependency treegraph
 
Reported: 2022-10-20 16:43 UTC by Stefan Berndtsson
Modified: 2023-06-08 22:32 UTC (History)
3 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
This adds a new hook "after_recall_action" with the action "add".
Version(s) released in:
22.11.00


Attachments
Bug 31896: New after_recall_action hook (5.44 KB, patch)
2022-10-20 16:43 UTC, Stefan Berndtsson
Details | Diff | Splinter Review
Bug 31896: New after_recall_action hook (5.46 KB, patch)
2022-10-20 18:55 UTC, David Nind
Details | Diff | Splinter Review
Bug 31896: New after_recall_action hook (5.43 KB, patch)
2022-10-21 08:53 UTC, Stefan Berndtsson
Details | Diff | Splinter Review
Bug 31896: New after_recall_action hook (5.46 KB, patch)
2022-10-21 11:22 UTC, David Nind
Details | Diff | Splinter Review
Bug 31896: New after_recall_action hook (5.64 KB, patch)
2022-10-28 08:46 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 31896: (QA follow-up) Fix modules (1.31 KB, patch)
2022-10-28 08:47 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 31896: (QA follow-up) Fix rollback (1.04 KB, patch)
2022-10-28 08:47 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 31896: New after_recall_action hook (5.76 KB, patch)
2022-11-04 09:26 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 31896: (QA follow-up) Fix modules (1.31 KB, patch)
2022-11-04 09:26 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 31896: (QA follow-up) Fix rollback (1.04 KB, patch)
2022-11-04 09:26 UTC, Marcel de Rooy
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Stefan Berndtsson 2022-10-20 16:43:23 UTC
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.
Comment 1 Stefan Berndtsson 2022-10-20 16:43:53 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
Comment 2 David Nind 2022-10-20 18:55:16 UTC
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>
Comment 3 David Nind 2022-10-20 18:57:38 UTC
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
Comment 4 Marcel de Rooy 2022-10-21 06:56:55 UTC
 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?
Comment 5 Stefan Berndtsson 2022-10-21 08:53:22 UTC
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
Comment 6 Stefan Berndtsson 2022-10-21 08:54:11 UTC
Now passes QA in my system. Fixed file permissions and trailing whitespace.
Comment 7 David Nind 2022-10-21 11:22:59 UTC
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>
Comment 8 Marcel de Rooy 2022-10-25 14:57:30 UTC
+    $schema->storage->txn_rollback;
+    Koha::Plugins::Methods->delete;
Comment 9 Marcel de Rooy 2022-10-25 14:59:07 UTC
+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.
Comment 10 Marcel de Rooy 2022-10-25 14:59:37 UTC
Please come back after 31894
Comment 11 Marcel de Rooy 2022-10-28 07:56:35 UTC
Working here now
Comment 12 Marcel de Rooy 2022-10-28 08:46:59 UTC
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>
Comment 13 Marcel de Rooy 2022-10-28 08:47:04 UTC
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>
Comment 14 Marcel de Rooy 2022-10-28 08:47:09 UTC
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>
Comment 15 Marcel de Rooy 2022-11-04 09:22:34 UTC
I will restore the get_from_storage here in the first patch to be consistent. With a FIXME
Comment 16 Marcel de Rooy 2022-11-04 09:26:10 UTC
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.
Comment 17 Marcel de Rooy 2022-11-04 09:26:14 UTC
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>
Comment 18 Marcel de Rooy 2022-11-04 09:26:18 UTC
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>
Comment 19 Tomás Cohen Arazi 2022-11-09 17:39:54 UTC
Pushed to master for 22.11.

Nice work everyone, thanks!
Comment 20 Katrin Fischer 2022-11-09 22:34:15 UTC
Please remember to update https://wiki.koha-community.org/wiki/Koha_Plugin_Hooks! :)