Bug 22709 - Add hooks to notify plugins of biblio and item changes
Summary: Add hooks to notify plugins of biblio and item changes
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Plugin architecture (show other bugs)
Version: Main
Hardware: All All
: P5 - low new feature
Assignee: Tomás Cohen Arazi (tcohen)
QA Contact: Josef Moravec
URL:
Keywords:
Depends on:
Blocks: 23395 34786 26470 29274
  Show dependency tree / graph
 
Reported: 2019-04-15 16:17 UTC by Tomás Cohen Arazi (tcohen)
Modified: 2024-04-08 09:12 UTC (History)
7 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
This new feature allows plugin authors to implement `after_biblio_action` and `after_item_action` methods which may be used to take various actions upon biblio and item creations, modifications and deletions. **Warning**: Care should be taken when installing any plugins and only plugins you trust should be used.
Version(s) released in:
19.11.00
Circulation function:

Attachments
Bug 22709: Unit tests (5.57 KB, patch)
2019-06-24 14:57 UTC, Tomás Cohen Arazi (tcohen) 		Details | Diff | Splinter Review
Bug 22709: Add after biblio/item action hooks (5.70 KB, patch)
2019-06-24 14:57 UTC, Tomás Cohen Arazi (tcohen) 		Details | Diff | Splinter Review
Bug 22709: (follow-up) Mock enable_plugins config entry (929 bytes, patch)
2019-07-02 13:07 UTC, Tomás Cohen Arazi (tcohen) 		Details | Diff | Splinter Review
Bug 22709: Unit tests (5.64 KB, patch)
2019-07-03 12:19 UTC, Kyle M Hall (khall) 		Details | Diff | Splinter Review
Bug 22709: Add after biblio/item action hooks (5.77 KB, patch)
2019-07-03 12:20 UTC, Kyle M Hall (khall) 		Details | Diff | Splinter Review
Bug 22709: (follow-up) Mock enable_plugins config entry (1001 bytes, patch)
2019-07-03 12:20 UTC, Kyle M Hall (khall) 		Details | Diff | Splinter Review
Bug 22709: (follow-up) Move new test file into a Plugins subdirectory (1.20 KB, patch)
2019-07-03 12:20 UTC, Kyle M Hall (khall) 		Details | Diff | Splinter Review
Bug 22709: Unit tests (5.69 KB, patch)
2019-07-03 21:19 UTC, Josef Moravec 		Details | Diff | Splinter Review
Bug 22709: Add after biblio/item action hooks (5.81 KB, patch)
2019-07-03 21:19 UTC, Josef Moravec 		Details | Diff | Splinter Review
Bug 22709: (follow-up) Mock enable_plugins config entry (1.02 KB, patch)
2019-07-03 21:19 UTC, Josef Moravec 		Details | Diff | Splinter Review
Bug 22709: (follow-up) Move new test file into a Plugins subdirectory (1.23 KB, patch)
2019-07-03 21:19 UTC, Josef Moravec 		Details | Diff | Splinter Review
Show Obsolete (7) 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) 2019-04-15 16:17:25 UTC 
We should have a way to make

*Biblio
*Item

methods trigger actions on plugins. Maybe if we have a message queue, then enqueue messages for registered plugins (i.e. to avoid synchronous calls).
Comment 1 Tomás Cohen Arazi (tcohen) 2019-04-15 16:18:55 UTC Comment hidden (obsolete)
Comment 2 Tomás Cohen Arazi (tcohen) 2019-06-24 14:57:11 UTC 
Created attachment 90950 [details] [review]
Bug 22709: Unit tests

This patch introduces tests for new plugin hooks added to
C4::Biblio::{Add|Mod|Del}Biblio and C4::Items::{Add|Mod|Del}Item.

They are tested together as for creating an item you need to first create
a biblio and tests looks basically the same.

The testing strategy is
- Make sure the plugin is passed the right params
- Throw an exception at plugin-level, to be trapped by the C4::Biblio
and C4::Items lib, and converted into a warning. So we test for the
warning.
- Also, the fact that C4::Biblio and C4::Items warns, means the
exception was cought, and then Koha won't break on faulty plugins or
fatal errors from plugins.
Comment 3 Tomás Cohen Arazi (tcohen) 2019-06-24 14:57:14 UTC 
Created attachment 90951 [details] [review]
Bug 22709: Add after biblio/item action hooks

This patch introduces after-action hooks for
- C4::Biblio::{Add|Mod|Del}Biblio
- C4::Items::{Add|Mod|Del}Biblio

After the action has taken place, a call to a helper method is done,
which loops through all plugins implementing the hooks, and calls the
plugin method. The related object is passed, along with an 'action'
string specifying the action that took place, and the object id (which
is specially important for the 'Del' case).

To test:
- Apply this patchset
- Run:
  $ kshell
 k$ prove t/db_dependent/Biblio_and_Items_plugin_hooks.t
=> SUCCESS: Tests pass!
- Sign off :-D
Comment 4 Kyle M Hall (khall) 2019-07-01 18:40:58 UTC 
Unit tests are failing.
Comment 5 Tomás Cohen Arazi (tcohen) 2019-07-02 13:07:45 UTC 
Created attachment 91186 [details] [review]
Bug 22709: (follow-up) Mock enable_plugins config entry

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 6 Tomás Cohen Arazi (tcohen) 2019-07-02 13:08:37 UTC 
(In reply to Kyle M Hall from comment #4)
> Unit tests are failing.

Thanks for testing! I've added a follow-up to fix the issue. I didn't notice I forgot to mock the enable_plugins config entry.
Comment 7 Kyle M Hall (khall) 2019-07-03 12:19:53 UTC 
Created attachment 91223 [details] [review]
Bug 22709: Unit tests

This patch introduces tests for new plugin hooks added to
C4::Biblio::{Add|Mod|Del}Biblio and C4::Items::{Add|Mod|Del}Item.

They are tested together as for creating an item you need to first create
a biblio and tests looks basically the same.

The testing strategy is
- Make sure the plugin is passed the right params
- Throw an exception at plugin-level, to be trapped by the C4::Biblio
and C4::Items lib, and converted into a warning. So we test for the
warning.
- Also, the fact that C4::Biblio and C4::Items warns, means the
exception was cought, and then Koha won't break on faulty plugins or
fatal errors from plugins.

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 8 Kyle M Hall (khall) 2019-07-03 12:20:04 UTC 
Created attachment 91224 [details] [review]
Bug 22709: Add after biblio/item action hooks

This patch introduces after-action hooks for
- C4::Biblio::{Add|Mod|Del}Biblio
- C4::Items::{Add|Mod|Del}Biblio

After the action has taken place, a call to a helper method is done,
which loops through all plugins implementing the hooks, and calls the
plugin method. The related object is passed, along with an 'action'
string specifying the action that took place, and the object id (which
is specially important for the 'Del' case).

To test:
- Apply this patchset
- Run:
  $ kshell
 k$ prove t/db_dependent/Biblio_and_Items_plugin_hooks.t
=> SUCCESS: Tests pass!
- Sign off :-D

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 9 Kyle M Hall (khall) 2019-07-03 12:20:06 UTC 
Created attachment 91225 [details] [review]
Bug 22709: (follow-up) Mock enable_plugins config entry

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

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 10 Kyle M Hall (khall) 2019-07-03 12:20:10 UTC 
Created attachment 91226 [details] [review]
Bug 22709: (follow-up) Move new test file into a Plugins subdirectory

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 11 Tomás Cohen Arazi (tcohen) 2019-07-03 13:06:17 UTC 
(In reply to Kyle M Hall from comment #10)
> Created attachment 91226 [details] [review] [review]
> Bug 22709: (follow-up) Move new test file into a Plugins subdirectory
> 
> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Good idea!
Comment 12 Josef Moravec 2019-07-03 21:19:24 UTC 
Created attachment 91244 [details] [review]
Bug 22709: Unit tests

This patch introduces tests for new plugin hooks added to
C4::Biblio::{Add|Mod|Del}Biblio and C4::Items::{Add|Mod|Del}Item.

They are tested together as for creating an item you need to first create
a biblio and tests looks basically the same.

The testing strategy is
- Make sure the plugin is passed the right params
- Throw an exception at plugin-level, to be trapped by the C4::Biblio
and C4::Items lib, and converted into a warning. So we test for the
warning.
- Also, the fact that C4::Biblio and C4::Items warns, means the
exception was cought, and then Koha won't break on faulty plugins or
fatal errors from plugins.

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

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 13 Josef Moravec 2019-07-03 21:19:29 UTC 
Created attachment 91245 [details] [review]
Bug 22709: Add after biblio/item action hooks

This patch introduces after-action hooks for
- C4::Biblio::{Add|Mod|Del}Biblio
- C4::Items::{Add|Mod|Del}Biblio

After the action has taken place, a call to a helper method is done,
which loops through all plugins implementing the hooks, and calls the
plugin method. The related object is passed, along with an 'action'
string specifying the action that took place, and the object id (which
is specially important for the 'Del' case).

To test:
- Apply this patchset
- Run:
  $ kshell
 k$ prove t/db_dependent/Biblio_and_Items_plugin_hooks.t
=> SUCCESS: Tests pass!
- Sign off :-D

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

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 14 Josef Moravec 2019-07-03 21:19:33 UTC 
Created attachment 91246 [details] [review]
Bug 22709: (follow-up) Mock enable_plugins config entry

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

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

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 15 Josef Moravec 2019-07-03 21:19:37 UTC 
Created attachment 91247 [details] [review]
Bug 22709: (follow-up) Move new test file into a Plugins subdirectory

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

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 16 Martin Renvoize (ashimema) 2019-07-04 12:57:58 UTC 
Nice work!

Pushed to master for 19.11.00
Comment 17 Katrin Fischer 2019-07-05 17:31:46 UTC 
Do we have documentation for this and other new hooks? Should this be added to the kitchen sink plugin?
Comment 18 Tomás Cohen Arazi (tcohen) 2019-07-05 17:32:42 UTC 
(In reply to Katrin Fischer from comment #17)
> Do we have documentation for this and other new hooks? Should this be added
> to the kitchen sink plugin?

Good point. Will add them to KitchenSink ASAP.
Comment 19 Katrin Fischer 2019-07-05 18:28:25 UTC 
Started a wiki page too... content to be added ;)
https://wiki.koha-community.org/wiki/Koha_Plugin_Hooks
Comment 20 Martin Renvoize (ashimema) 2019-07-09 10:30:56 UTC 
Nice work!

Pushed to master for 19.11.00
Comment 21 Martin Renvoize (ashimema) 2019-07-15 11:43:42 UTC 
Nice work!

Pushed to master for 19.11.00
Comment 22 Fridolin Somers 2019-07-23 16:01:29 UTC 
Awesome \o/
Comment 23 David Cook 2019-07-24 05:12:36 UTC 
Nice! A few years ago, I was thinking it would be great to add hooks like this, but never got around to doing it.
Comment 24 Fridolin Somers 2019-07-25 13:31:16 UTC 
I could backport to stable but maybe this needs the plugins performance patch.
Comment 25 Tomás Cohen Arazi (tcohen) 2019-07-25 13:43:02 UTC 
(In reply to Fridolin SOMERS from comment #24)
> I could backport to stable but maybe this needs the plugins performance
> patch.

Indeed. And we noticed a slight performance hit we need to address.
Comment 26 Kyle M Hall (khall) 2019-07-25 15:02:35 UTC 
(In reply to Tomás Cohen Arazi from comment #25)
> (In reply to Fridolin SOMERS from comment #24)
> > I could backport to stable but maybe this needs the plugins performance
> > patch.
> 
> Indeed. And we noticed a slight performance hit we need to address.

Agreed, with the performance patch, the addition time is essentially negligible.