Bug 30072 - Add more holds hooks
Summary: Add more holds hooks
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Plugin architecture (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low enhancement
Assignee: Tomás Cohen Arazi (tcohen)
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks: 30180
  Show dependency tree / graph
 
Reported: 2022-02-10 14:02 UTC by Tomás Cohen Arazi (tcohen)
Modified: 2022-12-12 21:25 UTC (History)
7 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
This development adds plugin hooks for several holds actions. The hook is called *after_hold_action* and has two parameters * **action**: containing a string that represents the _action_, possible values: _fill_, _cancel_, _suspend_ and _resume_. * **payload**: A hashref containing a _hold_ key, which points to the Koha::Hold object.
Version(s) released in:
22.05.00,21.11.04,21.05.13
Circulation function:

Attachments
Bug 30072: Unit tests (3.10 KB, patch)
2022-02-11 18:36 UTC, Tomás Cohen Arazi (tcohen) 		Details | Diff | Splinter Review
Bug 30072: Add missing hold actions plugin hooks (2.79 KB, patch)
2022-02-11 18:37 UTC, Tomás Cohen Arazi (tcohen) 		Details | Diff | Splinter Review
Bug 30072: (follow-up) Silence useless warning (1.03 KB, patch)
2022-02-11 18:37 UTC, Tomás Cohen Arazi (tcohen) 		Details | Diff | Splinter Review
[STABLE] Bug 30072: Unit tests (3.30 KB, patch)
2022-02-11 19:20 UTC, Tomás Cohen Arazi (tcohen) 		Details | Diff | Splinter Review
[STABLE] Bug 30072: Add missing hold actions plugin hooks (3.11 KB, patch)
2022-02-11 19:20 UTC, Tomás Cohen Arazi (tcohen) 		Details | Diff | Splinter Review
[STABLE] Bug 30072: (follow-up) Silence useless warning (1.04 KB, patch)
2022-02-11 19:20 UTC, Tomás Cohen Arazi (tcohen) 		Details | Diff | Splinter Review
Bug 30072: Unit tests (3.16 KB, patch)
2022-02-15 15:18 UTC, Kyle M Hall (khall) 		Details | Diff | Splinter Review
Bug 30072: Unit tests (3.16 KB, patch)
2022-02-15 15:22 UTC, Kyle M Hall (khall) 		Details | Diff | Splinter Review
Bug 30072: Add missing hold actions plugin hooks (2.85 KB, patch)
2022-02-15 15:22 UTC, Kyle M Hall (khall) 		Details | Diff | Splinter Review
Bug 30072: (follow-up) Silence useless warning (1.09 KB, patch)
2022-02-15 15:22 UTC, Kyle M Hall (khall) 		Details | Diff | Splinter Review
Bug 30072: Unit tests (3.22 KB, patch)
2022-02-17 11:44 UTC, Martin Renvoize (ashimema) 		Details | Diff | Splinter Review
Bug 30072: Add missing hold actions plugin hooks (2.91 KB, patch)
2022-02-17 11:44 UTC, Martin Renvoize (ashimema) 		Details | Diff | Splinter Review
Bug 30072: (follow-up) Silence useless warning (1.15 KB, patch)
2022-02-17 11:44 UTC, Martin Renvoize (ashimema) 		Details | Diff | Splinter Review
[STABLE] Bug 30072: Unit tests (3.35 KB, patch)
2022-02-17 11:48 UTC, Martin Renvoize (ashimema) 		Details | Diff | Splinter Review
[STABLE] Bug 30072: Add missing hold actions plugin hooks (3.16 KB, patch)
2022-02-17 11:48 UTC, Martin Renvoize (ashimema) 		Details | Diff | Splinter Review
[STABLE] Bug 30072: (follow-up) Silence useless warning (1.10 KB, patch)
2022-02-17 11:48 UTC, Martin Renvoize (ashimema) 		Details | Diff | Splinter Review
Bug 30072: (follow-up) Fix UT after Bug 29857 (812 bytes, patch)
2022-03-04 00:43 UTC, Fridolin Somers 		Details | Diff | Splinter Review
Bug 30072: (followup) Fix random failure of Holds_hooks.t (1.25 KB, patch)
2022-04-27 00:19 UTC, Fridolin Somers 		Details | Diff | Splinter Review
Show Obsolete (10) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Tomás Cohen Arazi (tcohen) 2022-02-10 14:02:56 UTC 
We should add more plugin hooks. At least:

- after_hold_fill
- after_hold_cancel
Comment 1 Tomás Cohen Arazi (tcohen) 2022-02-11 18:36:51 UTC 
Created attachment 130517 [details] [review]
Bug 30072: Unit tests

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 2 Tomás Cohen Arazi (tcohen) 2022-02-11 18:37:03 UTC 
Created attachment 130518 [details] [review]
Bug 30072: Add missing hold actions plugin hooks

This patch introduces the after_hold_action plugin hook, with 4
different 'action' parameters:

- fill
- cancel
- suspend
- resume

To test:
1. Apply the unit tests
2. Run:
   $ kshell
  k$ t/db_dependent/Koha/Plugins/Holds_hooks.t -v
=> FAIL: The hooks are not in the code, so the expected output from the
Koha::Plugin::Test plugin is not there, and the tests fail
3. Apply this patch
4. Repeat 2
=> SUCCESS: Tests pass!
5. Sign off :-D

Note: I think we could deprecate 'after_hold_create' and migrate it to
the one introduced here, using the 'place' action.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 3 Tomás Cohen Arazi (tcohen) 2022-02-11 18:37:10 UTC 
Created attachment 130519 [details] [review]
Bug 30072: (follow-up) Silence useless warning

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 4 Tomás Cohen Arazi (tcohen) 2022-02-11 18:41:08 UTC 
I have doubts about relying on ->get_from_storage, as the current Koha::Plugins->call design will refetch the object even if there are no installed plugins.

I submit this as-is because it is what is being done in places like AddReturn anyway. But my gut feeling was telling me to pass the id only, and let the plugin writers decide what to do with it. But, for consistency, we should fix the existing calls... Hard call. Ideas welcome. I like this as-is and will provide a patchset for branches that don't have bug 29869 in them.
Comment 5 Tomás Cohen Arazi (tcohen) 2022-02-11 19:20:44 UTC 
Created attachment 130522 [details] [review]
[STABLE] Bug 30072: Unit tests

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 6 Tomás Cohen Arazi (tcohen) 2022-02-11 19:20:48 UTC 
Created attachment 130523 [details] [review]
[STABLE] Bug 30072: Add missing hold actions plugin hooks

This patch introduces the after_hold_action plugin hook, with 4
different 'action' parameters:

- fill
- cancel
- suspend
- resume

To test:
1. Apply the unit tests
2. Run:
   $ kshell
  k$ t/db_dependent/Koha/Plugins/Holds_hooks.t -v
=> FAIL: The hooks are not in the code, so the expected output from the
Koha::Plugin::Test plugin is not there, and the tests fail
3. Apply this patch
4. Repeat 2
=> SUCCESS: Tests pass!
5. Sign off :-D

Note: I think we could deprecate 'after_hold_create' and migrate it to
the one introduced here, using the 'place' action.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 7 Tomás Cohen Arazi (tcohen) 2022-02-11 19:20:52 UTC 
Created attachment 130524 [details] [review]
[STABLE] Bug 30072: (follow-up) Silence useless warning

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 8 Kyle M Hall (khall) 2022-02-15 15:18:51 UTC 
Created attachment 130621 [details] [review]
Bug 30072: Unit tests

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

bsoletes: 130517 - Bug 30072: Unit tests
Comment 9 Kyle M Hall (khall) 2022-02-15 15:22:02 UTC 
Created attachment 130622 [details] [review]
Bug 30072: Unit tests

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 10 Kyle M Hall (khall) 2022-02-15 15:22:13 UTC 
Created attachment 130623 [details] [review]
Bug 30072: Add missing hold actions plugin hooks

This patch introduces the after_hold_action plugin hook, with 4
different 'action' parameters:

- fill
- cancel
- suspend
- resume

To test:
1. Apply the unit tests
2. Run:
   $ kshell
  k$ t/db_dependent/Koha/Plugins/Holds_hooks.t -v
=> FAIL: The hooks are not in the code, so the expected output from the
Koha::Plugin::Test plugin is not there, and the tests fail
3. Apply this patch
4. Repeat 2
=> SUCCESS: Tests pass!
5. Sign off :-D

Note: I think we could deprecate 'after_hold_create' and migrate it to
the one introduced here, using the 'place' action.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 11 Kyle M Hall (khall) 2022-02-15 15:22:16 UTC 
Created attachment 130624 [details] [review]
Bug 30072: (follow-up) Silence useless warning

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 12 Kyle M Hall (khall) 2022-02-15 15:26:48 UTC 
Stable patches tested as well.
Comment 13 Martin Renvoize (ashimema) 2022-02-17 11:44:00 UTC 
Created attachment 130718 [details] [review]
Bug 30072: Unit tests

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 14 Martin Renvoize (ashimema) 2022-02-17 11:44:05 UTC 
Created attachment 130719 [details] [review]
Bug 30072: Add missing hold actions plugin hooks

This patch introduces the after_hold_action plugin hook, with 4
different 'action' parameters:

- fill
- cancel
- suspend
- resume

To test:
1. Apply the unit tests
2. Run:
   $ kshell
  k$ t/db_dependent/Koha/Plugins/Holds_hooks.t -v
=> FAIL: The hooks are not in the code, so the expected output from the
Koha::Plugin::Test plugin is not there, and the tests fail
3. Apply this patch
4. Repeat 2
=> SUCCESS: Tests pass!
5. Sign off :-D

Note: I think we could deprecate 'after_hold_create' and migrate it to
the one introduced here, using the 'place' action.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 15 Martin Renvoize (ashimema) 2022-02-17 11:44:08 UTC 
Created attachment 130720 [details] [review]
Bug 30072: (follow-up) Silence useless warning

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 16 Martin Renvoize (ashimema) 2022-02-17 11:45:14 UTC 
This works well and uses the reference passing style we now require (Is there a coding guideline for this yet?)

QA scripts are happy, tests are passing. Passing QA
Comment 17 Martin Renvoize (ashimema) 2022-02-17 11:48:16 UTC 
Created attachment 130721 [details] [review]
[STABLE] Bug 30072: Unit tests

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 18 Martin Renvoize (ashimema) 2022-02-17 11:48:20 UTC 
Created attachment 130722 [details] [review]
[STABLE] Bug 30072: Add missing hold actions plugin hooks

This patch introduces the after_hold_action plugin hook, with 4
different 'action' parameters:

- fill
- cancel
- suspend
- resume

To test:
1. Apply the unit tests
2. Run:
   $ kshell
  k$ t/db_dependent/Koha/Plugins/Holds_hooks.t -v
=> FAIL: The hooks are not in the code, so the expected output from the
Koha::Plugin::Test plugin is not there, and the tests fail
3. Apply this patch
4. Repeat 2
=> SUCCESS: Tests pass!
5. Sign off :-D

Note: I think we could deprecate 'after_hold_create' and migrate it to
the one introduced here, using the 'place' action.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 19 Martin Renvoize (ashimema) 2022-02-17 11:48:24 UTC 
Created attachment 130723 [details] [review]
[STABLE] Bug 30072: (follow-up) Silence useless warning

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 20 Katrin Fischer 2022-02-19 13:22:07 UTC 
Please remember to update the wiki page as well :) 
https://wiki.koha-community.org/wiki/Koha_Plugin_Hooks
Comment 21 Fridolin Somers 2022-03-04 00:43:18 UTC 
Ah, rebase conflict due to Bug 29857
I create a followup
Comment 22 Fridolin Somers 2022-03-04 00:43:37 UTC 
Created attachment 131366 [details] [review]
Bug 30072: (follow-up) Fix UT after Bug 29857
Comment 23 Fridolin Somers 2022-03-04 02:12:20 UTC 
Pushed to master for 22.05, thanks to everybody involved ﷐[U+1F984]﷑
Comment 24 Kyle M Hall (khall) 2022-03-04 14:29:48 UTC 
Pushed to 21.11.x for 21.11.04
Comment 25 Katrin Fischer 2022-03-05 23:02:16 UTC 
(In reply to Katrin Fischer from comment #20)
> Please remember to update the wiki page as well :) 
> https://wiki.koha-community.org/wiki/Koha_Plugin_Hooks

*cough*
Comment 26 Tomás Cohen Arazi (tcohen) 2022-03-07 11:23:30 UTC 
(In reply to Katrin Fischer from comment #25)
> (In reply to Katrin Fischer from comment #20)
> > Please remember to update the wiki page as well :) 
> > https://wiki.koha-community.org/wiki/Koha_Plugin_Hooks
> 
> *cough*

Done.
Comment 27 Katrin Fischer 2022-03-07 11:44:49 UTC 
(In reply to Tomás Cohen Arazi from comment #26)
> (In reply to Katrin Fischer from comment #25)
> > (In reply to Katrin Fischer from comment #20)
> > > Please remember to update the wiki page as well :) 
> > > https://wiki.koha-community.org/wiki/Koha_Plugin_Hooks
> > 
> > *cough*
> 
> Done.

Thx :)
Comment 28 Andrew Fuerste-Henry 2022-03-07 20:00:36 UTC 
Pushed to 21.05.x for 21.05.12
Comment 29 Victor Grousset/tuxayo 2022-03-18 02:36:46 UTC 
Not backported to oldoldstable (20.11.x). Feel free to ask if it's needed.
Comment 30 Fridolin Somers 2022-04-27 00:19:10 UTC 
Created attachment 134005 [details] [review]
Bug 30072: (followup) Fix random failure of Holds_hooks.t

When building Koha::Holds object,
do not let 'found' random otherwise it may take a valid value indicating
hold is found.

See other holds unit tests.
Comment 31 Fridolin Somers 2022-04-27 00:20:00 UTC 
> Bug 30072: (followup) Fix random failure of Holds_hooks.t 
Pushed to master