Bug 22709 - Add hooks to notify plugins of biblio and item changes
Summary: Add hooks to notify plugins of biblio and item changes
Status: Pushed to master
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Tomás Cohen Arazi
QA Contact: Josef Moravec
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2019-04-15 16:17 UTC by Tomás Cohen Arazi
Modified: 2019-07-15 11:43 UTC (History)
6 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:
19.11.00


Attachments
Bug 22709: Unit tests (5.57 KB, patch)
2019-06-24 14:57 UTC, Tomás Cohen Arazi
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
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
Details | Diff | Splinter Review
Bug 22709: Unit tests (5.64 KB, patch)
2019-07-03 12:19 UTC, Kyle M Hall
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
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
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
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

Note You need to log in before you can comment on or make changes to this bug.
Description Tomás Cohen Arazi 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 2019-04-15 16:18:55 UTC Comment hidden (obsolete)
Comment 2 Tomás Cohen Arazi 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 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 2019-07-01 18:40:58 UTC
Unit tests are failing.
Comment 5 Tomás Cohen Arazi 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 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 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 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 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 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 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 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 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 2019-07-09 10:30:56 UTC
Nice work!

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

Pushed to master for 19.11.00