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

See Also:
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
Circulation function:

Attachments
Bug 30180: Unit tests (2.71 KB, patch)
2022-02-25 08:57 UTC, Tomás Cohen Arazi (tcohen) 		Details | Diff | Splinter Review
Bug 30180: Add 'placed' after_hold_hook (1.07 KB, patch)
2022-02-25 08:57 UTC, Tomás Cohen Arazi (tcohen) 		Details | Diff | Splinter Review
Bug 30180: (follow-up) Add temporary deprecation warning (3.49 KB, patch)
2022-02-25 08:57 UTC, Tomás Cohen Arazi (tcohen) 		Details | Diff | Splinter Review
Bug 30180: Unit tests (2.75 KB, patch)
2022-04-19 19:42 UTC, David Nind 		Details | Diff | Splinter Review
Bug 30180: Add 'placed' after_hold_hook (1.12 KB, patch)
2022-04-19 19:42 UTC, David Nind 		Details | Diff | Splinter Review
Bug 30180: (follow-up) Add temporary deprecation warning (3.54 KB, patch)
2022-04-19 19:42 UTC, David Nind 		Details | Diff | Splinter Review
Bug 30180: Unit tests (2.81 KB, patch)
2022-04-25 16:17 UTC, Katrin Fischer 		Details | Diff | Splinter Review
Bug 30180: Add 'placed' after_hold_hook (1.18 KB, patch)
2022-04-25 16:17 UTC, Katrin Fischer 		Details | Diff | Splinter Review
Bug 30180: (follow-up) Add temporary deprecation warning (3.60 KB, patch)
2022-04-25 16:17 UTC, Katrin Fischer 		Details | Diff | Splinter Review
Show Obsolete (6) 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-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 (tcohen) 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 (tcohen) 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 (tcohen) 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 ﷐[U+1F984]﷑