Bug 30180

Summary: Deprecate after_hold_create hook
Product: Koha Reporter: Tomás Cohen Arazi <tomascohen>
Component: Plugin architectureAssignee: Tomás Cohen Arazi <tomascohen>
Status: CLOSED FIXED QA Contact: Testopia <testopia>
Severity: enhancement    
Priority: P5 - low CC: fridolin.somers, katrin.fischer, kyle
Version: Main   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: Small patch
Documentation contact: Documentation submission:
Text to go in the release notes:
The 'after_hold_create' hook is deprecated and scheduled for removal in the next major release. If you find deprecation warnings for some plugin in your logs, please ask the plugin authors to update it to use the new 'after_hold_action' hook instead.
Version(s) released in:
22.05.00
Bug Depends on: 30072    
Bug Blocks:    
Attachments: Bug 30180: Unit tests
Bug 30180: Add 'placed' after_hold_hook
Bug 30180: (follow-up) Add temporary deprecation warning
Bug 30180: Unit tests
Bug 30180: Add 'placed' after_hold_hook
Bug 30180: (follow-up) Add temporary deprecation warning
Bug 30180: Unit tests
Bug 30180: Add 'placed' after_hold_hook
Bug 30180: (follow-up) Add temporary deprecation warning

Description Tomás Cohen Arazi 2022-02-25 08:24:49 UTC
Bug 30072 introduces 'after_hold_action' hooks, that get passed an 'action' parameter. When the hold is placed, we should just use that hook, with the 'place' action parameter. This will make hooks more consistent.

Proposal: deprecate 'after_hold_create' on the 22.05 release. Adding a warning in the logs to make developers update their plugins using the hook. Remove it in the 22.11 release.

This should be voted on a dev meeting.
Comment 1 Tomás Cohen Arazi 2022-02-25 08:57:25 UTC
Created attachment 131101 [details] [review]
Bug 30180: Unit tests

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 2 Tomás Cohen Arazi 2022-02-25 08:57:29 UTC
Created attachment 131102 [details] [review]
Bug 30180: Add 'placed' after_hold_hook

This patch adds a new 'after_hold_action' hook, that is called with the
'placed' action parameter.

To test:
1. Apply the unit tests patch
2. Run:
   $ kshell
  k$ prove t/db_dependent/Koha/Plugins/Holds_hooks.t
=> FAIL: Boo, the hook is not called
3. Apply this patch
4. Repeat 2
=> SUCCESS: Tests pass!
5. Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 3 Tomás Cohen Arazi 2022-02-25 08:57:33 UTC
Created attachment 131103 [details] [review]
Bug 30180: (follow-up) Add temporary deprecation warning

This patch adds a temporary deprecation warning to let users know the
plugin needs to get updated before the next release.

To test:
1. Apply this patch
2. Run:
   $ kshell
  k$ prove t/db_dependent/Koha/Plugins/Holds_hooks.t
=> SUCCESS: Tests pass, warning is displayed
3. Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 4 David Nind 2022-04-19 19:42:38 UTC
Created attachment 133415 [details] [review]
Bug 30180: Unit tests

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

Signed-off-by: David Nind <david@davidnind.com>
Comment 5 David Nind 2022-04-19 19:42:43 UTC
Created attachment 133416 [details] [review]
Bug 30180: Add 'placed' after_hold_hook

This patch adds a new 'after_hold_action' hook, that is called with the
'placed' action parameter.

To test:
1. Apply the unit tests patch
2. Run:
   $ kshell
  k$ prove t/db_dependent/Koha/Plugins/Holds_hooks.t
=> FAIL: Boo, the hook is not called
3. Apply this patch
4. Repeat 2
=> SUCCESS: Tests pass!
5. Sign off :-D

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

Signed-off-by: David Nind <david@davidnind.com>
Comment 6 David Nind 2022-04-19 19:42:48 UTC
Created attachment 133417 [details] [review]
Bug 30180: (follow-up) Add temporary deprecation warning

This patch adds a temporary deprecation warning to let users know the
plugin needs to get updated before the next release.

To test:
1. Apply this patch
2. Run:
   $ kshell
  k$ prove t/db_dependent/Koha/Plugins/Holds_hooks.t
=> SUCCESS: Tests pass, warning is displayed
3. Sign off :-D

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

Signed-off-by: David Nind <david@davidnind.com>
Comment 7 Katrin Fischer 2022-04-25 16:16:16 UTC
How can we keep an eye on the need to remove the hook for 22.11? A new bug?
Comment 8 Katrin Fischer 2022-04-25 16:17:29 UTC
Created attachment 133818 [details] [review]
Bug 30180: Unit tests

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

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 9 Katrin Fischer 2022-04-25 16:17:33 UTC
Created attachment 133819 [details] [review]
Bug 30180: Add 'placed' after_hold_hook

This patch adds a new 'after_hold_action' hook, that is called with the
'placed' action parameter.

To test:
1. Apply the unit tests patch
2. Run:
   $ kshell
  k$ prove t/db_dependent/Koha/Plugins/Holds_hooks.t
=> FAIL: Boo, the hook is not called
3. Apply this patch
4. Repeat 2
=> SUCCESS: Tests pass!
5. Sign off :-D

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

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 10 Katrin Fischer 2022-04-25 16:17:38 UTC
Created attachment 133820 [details] [review]
Bug 30180: (follow-up) Add temporary deprecation warning

This patch adds a temporary deprecation warning to let users know the
plugin needs to get updated before the next release.

To test:
1. Apply this patch
2. Run:
   $ kshell
  k$ prove t/db_dependent/Koha/Plugins/Holds_hooks.t
=> SUCCESS: Tests pass, warning is displayed
3. Sign off :-D

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

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 11 Fridolin Somers 2022-05-05 00:32:40 UTC
Pushed to master for 22.05, thanks to everybody involved 🦄