Bug 30180 - Deprecate after_hold_create hook
Summary: Deprecate after_hold_create hook
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Plugin architecture (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Tomás Cohen Arazi
QA Contact: Testopia
URL:
Keywords:
Depends on: 30072
Blocks:
  Show dependency treegraph
 
Reported: 2022-02-25 08:24 UTC by Tomás Cohen Arazi
Modified: 2022-05-17 12:38 UTC (History)
3 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


Attachments
Bug 30180: Unit tests (2.71 KB, patch)
2022-02-25 08:57 UTC, Tomás Cohen Arazi
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
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
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

Note You need to log in before you can comment on or make changes to this bug.
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 🦄