Description
Tomás Cohen Arazi (tcohen)
2022-02-10 14:02:56 UTC
Created attachment 130517 [details] [review] Bug 30072: Unit tests Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 130518 [details] [review] Bug 30072: Add missing hold actions plugin hooks This patch introduces the after_hold_action plugin hook, with 4 different 'action' parameters: - fill - cancel - suspend - resume To test: 1. Apply the unit tests 2. Run: $ kshell k$ t/db_dependent/Koha/Plugins/Holds_hooks.t -v => FAIL: The hooks are not in the code, so the expected output from the Koha::Plugin::Test plugin is not there, and the tests fail 3. Apply this patch 4. Repeat 2 => SUCCESS: Tests pass! 5. Sign off :-D Note: I think we could deprecate 'after_hold_create' and migrate it to the one introduced here, using the 'place' action. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 130519 [details] [review] Bug 30072: (follow-up) Silence useless warning Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> I have doubts about relying on ->get_from_storage, as the current Koha::Plugins->call design will refetch the object even if there are no installed plugins. I submit this as-is because it is what is being done in places like AddReturn anyway. But my gut feeling was telling me to pass the id only, and let the plugin writers decide what to do with it. But, for consistency, we should fix the existing calls... Hard call. Ideas welcome. I like this as-is and will provide a patchset for branches that don't have bug 29869 in them. Created attachment 130522 [details] [review] [STABLE] Bug 30072: Unit tests Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 130523 [details] [review] [STABLE] Bug 30072: Add missing hold actions plugin hooks This patch introduces the after_hold_action plugin hook, with 4 different 'action' parameters: - fill - cancel - suspend - resume To test: 1. Apply the unit tests 2. Run: $ kshell k$ t/db_dependent/Koha/Plugins/Holds_hooks.t -v => FAIL: The hooks are not in the code, so the expected output from the Koha::Plugin::Test plugin is not there, and the tests fail 3. Apply this patch 4. Repeat 2 => SUCCESS: Tests pass! 5. Sign off :-D Note: I think we could deprecate 'after_hold_create' and migrate it to the one introduced here, using the 'place' action. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 130524 [details] [review] [STABLE] Bug 30072: (follow-up) Silence useless warning Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 130621 [details] [review] Bug 30072: Unit tests Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> bsoletes: 130517 - Bug 30072: Unit tests Created attachment 130622 [details] [review] Bug 30072: Unit tests Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Created attachment 130623 [details] [review] Bug 30072: Add missing hold actions plugin hooks This patch introduces the after_hold_action plugin hook, with 4 different 'action' parameters: - fill - cancel - suspend - resume To test: 1. Apply the unit tests 2. Run: $ kshell k$ t/db_dependent/Koha/Plugins/Holds_hooks.t -v => FAIL: The hooks are not in the code, so the expected output from the Koha::Plugin::Test plugin is not there, and the tests fail 3. Apply this patch 4. Repeat 2 => SUCCESS: Tests pass! 5. Sign off :-D Note: I think we could deprecate 'after_hold_create' and migrate it to the one introduced here, using the 'place' action. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Created attachment 130624 [details] [review] Bug 30072: (follow-up) Silence useless warning Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Stable patches tested as well. Created attachment 130718 [details] [review] Bug 30072: Unit tests Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 130719 [details] [review] Bug 30072: Add missing hold actions plugin hooks This patch introduces the after_hold_action plugin hook, with 4 different 'action' parameters: - fill - cancel - suspend - resume To test: 1. Apply the unit tests 2. Run: $ kshell k$ t/db_dependent/Koha/Plugins/Holds_hooks.t -v => FAIL: The hooks are not in the code, so the expected output from the Koha::Plugin::Test plugin is not there, and the tests fail 3. Apply this patch 4. Repeat 2 => SUCCESS: Tests pass! 5. Sign off :-D Note: I think we could deprecate 'after_hold_create' and migrate it to the one introduced here, using the 'place' action. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 130720 [details] [review] Bug 30072: (follow-up) Silence useless warning Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> This works well and uses the reference passing style we now require (Is there a coding guideline for this yet?) QA scripts are happy, tests are passing. Passing QA Created attachment 130721 [details] [review] [STABLE] Bug 30072: Unit tests Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 130722 [details] [review] [STABLE] Bug 30072: Add missing hold actions plugin hooks This patch introduces the after_hold_action plugin hook, with 4 different 'action' parameters: - fill - cancel - suspend - resume To test: 1. Apply the unit tests 2. Run: $ kshell k$ t/db_dependent/Koha/Plugins/Holds_hooks.t -v => FAIL: The hooks are not in the code, so the expected output from the Koha::Plugin::Test plugin is not there, and the tests fail 3. Apply this patch 4. Repeat 2 => SUCCESS: Tests pass! 5. Sign off :-D Note: I think we could deprecate 'after_hold_create' and migrate it to the one introduced here, using the 'place' action. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 130723 [details] [review] [STABLE] Bug 30072: (follow-up) Silence useless warning Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Please remember to update the wiki page as well :) https://wiki.koha-community.org/wiki/Koha_Plugin_Hooks Ah, rebase conflict due to Bug 29857 I create a followup Created attachment 131366 [details] [review] Bug 30072: (follow-up) Fix UT after Bug 29857 Pushed to master for 22.05, thanks to everybody involved [U+1F984] Pushed to 21.11.x for 21.11.04 (In reply to Katrin Fischer from comment #20) > Please remember to update the wiki page as well :) > https://wiki.koha-community.org/wiki/Koha_Plugin_Hooks *cough* (In reply to Katrin Fischer from comment #25) > (In reply to Katrin Fischer from comment #20) > > Please remember to update the wiki page as well :) > > https://wiki.koha-community.org/wiki/Koha_Plugin_Hooks > > *cough* Done. (In reply to Tomás Cohen Arazi from comment #26) > (In reply to Katrin Fischer from comment #25) > > (In reply to Katrin Fischer from comment #20) > > > Please remember to update the wiki page as well :) > > > https://wiki.koha-community.org/wiki/Koha_Plugin_Hooks > > > > *cough* > > Done. Thx :) Pushed to 21.05.x for 21.05.12 Not backported to oldoldstable (20.11.x). Feel free to ask if it's needed. Created attachment 134005 [details] [review] Bug 30072: (followup) Fix random failure of Holds_hooks.t When building Koha::Holds object, do not let 'found' random otherwise it may take a valid value indicating hold is found. See other holds unit tests. > Bug 30072: (followup) Fix random failure of Holds_hooks.t
Pushed to master
|