From 3941c6eb52124278ba6e91037d81055ae85dbe64 Mon Sep 17 00:00:00 2001 From: Tomas Cohen Arazi Date: Fri, 3 Jul 2020 10:26:06 -0300 Subject: [PATCH] Bug 25855: (QA follow-up) Generalize hook and simplify tests This patch generalizes the hook so it can be used by other circulation actions. Tests are also simplified by mocking some of the (extensive) plugin hooks. To test: 1. Repeat the test plan on the original patch => SUCCESS: All good 2. Sign off :-D Signed-off-by: Tomas Cohen Arazi Signed-off-by: Martin Renvoize --- C4/Circulation.pm | 33 +++++++++-------- .../Koha/Plugins/Circulation_hooks.t | 27 ++++++-------- t/lib/Koha/Plugin/Test.pm | 35 ++++++++----------- 3 files changed, 43 insertions(+), 52 deletions(-) diff --git a/C4/Circulation.pm b/C4/Circulation.pm index 76d91a5b09..c67b3752e2 100644 --- a/C4/Circulation.pm +++ b/C4/Circulation.pm @@ -3067,17 +3067,20 @@ sub AddRenewal { DelUniqueDebarment({ borrowernumber => $borrowernumber, type => 'OVERDUES' }); } - _post_renewal_actions( + _after_circ_actions( { - renewal_library_id => - $item_object->renewal_branchcode( { branch => $branch } ), - charge => $charge, - item_id => $itemnumber, - item_type => $itemtype, - shelving_location => $item_object->location // q{}, - patron_id => $borrowernumber, - collection_code => $item_object->ccode // q{}, - date_due => $datedue + action => 'renewal', + payload => { + renewal_library_id => + $item_object->renewal_branchcode( { branch => $branch } ), + charge => $charge, + item_id => $itemnumber, + item_type => $itemtype, + shelving_location => $item_object->location // q{}, + patron_id => $borrowernumber, + collection_code => $item_object->ccode // q{}, + date_due => $datedue + } } ) if C4::Context->config("enable_plugins"); @@ -4307,23 +4310,23 @@ sub _item_denied_renewal { return 0; } -=head3 _post_renewal_actions +=head3 _after_circ_actions -Internal method that calls the post_renewal_action plugin hook on configured +Internal method that calls the after_circ_action plugin hook on configured plugins. =cut -sub _post_renewal_actions { +sub _after_circ_actions { my ($params) = @_; my @plugins = Koha::Plugins->new->GetPlugins({ - method => 'post_renewal_action', + method => 'after_circ_action', }); foreach my $plugin ( @plugins ) { try { - $plugin->post_renewal_action( $params ); + $plugin->after_circ_action( $params ); } catch { warn "$_"; diff --git a/t/db_dependent/Koha/Plugins/Circulation_hooks.t b/t/db_dependent/Koha/Plugins/Circulation_hooks.t index 738ee8cf40..6b7aecbe81 100755 --- a/t/db_dependent/Koha/Plugins/Circulation_hooks.t +++ b/t/db_dependent/Koha/Plugins/Circulation_hooks.t @@ -17,6 +17,7 @@ use Modern::Perl; use Test::More tests => 4; +use Test::MockModule; use Test::Warn; use File::Basename; @@ -43,7 +44,7 @@ t::lib::Mocks::mock_config( 'enable_plugins', 1 ); subtest 'post_renewal_action() hook tests' => sub { - plan tests => 4; + plan tests => 1; $schema->storage->txn_begin; @@ -61,23 +62,17 @@ subtest 'post_renewal_action() hook tests' => sub { } ); - my ($biblio, $item); + # Avoid testing useless warnings + my $test_plugin = Test::MockModule->new('Koha::Plugin::Test'); + $test_plugin->mock( 'after_item_action', undef ); + $test_plugin->mock( 'after_biblio_action', undef ); - warning_like { $biblio = $builder->build_sample_biblio(); } - qr/after_biblio_action called with action: create, ref: Koha::Biblio/, - 'AddBiblio calls the hook with action=create'; + my $biblio = $builder->build_sample_biblio(); + my $item = $builder->build_sample_item({ biblionumber => $biblio->biblionumber }); + AddIssue( $patron->unblessed, $item->barcode ); - warning_like { $item = $builder->build_sample_item({ biblionumber => $biblio->biblionumber }); } - qr/after_item_action called with action: create, ref: Koha::Item/, - 'AddItem calls the hook with action=create'; - - warning_like { AddIssue( $patron->unblessed, $item->barcode ); } - qr/after_item_action called with action: modify, ref: Koha::Item/, - 'AddItem calls the hook with action=modify'; - - warnings_like { AddRenewal( $patron->borrowernumber, $item->id, $patron->branchcode ); } - [ qr/after_item_action called with action: modify, ref: Koha::Item/, - qr/post_renewal_action .* DateTime/ ], + warning_like { AddRenewal( $patron->borrowernumber, $item->id, $patron->branchcode ); } + qr/after_circ_action called with action: renewal, ref: DateTime/, 'AddRenewal calls the post_renewal_action hook'; $schema->storage->txn_rollback; diff --git a/t/lib/Koha/Plugin/Test.pm b/t/lib/Koha/Plugin/Test.pm index 0cc22343c5..98da8af73a 100644 --- a/t/lib/Koha/Plugin/Test.pm +++ b/t/lib/Koha/Plugin/Test.pm @@ -155,29 +155,22 @@ sub after_item_action { } } -sub post_renewal_action { +sub after_circ_action { my ( $self, $params ) = @_; - my $renewal_library_id = $params->{renewal_library_id}; - my $charge = $params->{charge}; - my $item_id = $params->{item_id}; - my $item_type = $params->{item_type}; - my $shelving_location = $params->{shelving_location}; - my $patron_id = $params->{patron_id}; - my $collection_code = $params->{collection_code}; - my $date_due = $params->{date_due}; - - Koha::Exceptions::Exception->throw( - "post_renewal_action " . - "$renewal_library_id " . - "$charge " . - "$item_id " . - "$item_type " . - "$shelving_location " . - "$patron_id " . - "$collection_code " . - ref($date_due) - ); + my $action = $params->{action}; + my $payload = $params->{payload}; + + my $renewal_library_id = $payload->{renewal_library_id}; + my $charge = $payload->{charge}; + my $item_id = $payload->{item_id}; + my $item_type = $payload->{item_type}; + my $shelving_location = $payload->{shelving_location}; + my $patron_id = $payload->{patron_id}; + my $collection_code = $payload->{collection_code}; + my $date_due = $payload->{date_due}; + + Koha::Exceptions::Exception->throw("after_circ_action called with action: $action, ref: " . ref($date_due)); } sub api_routes { -- 2.20.1