From fdc5735f2e5f75f459c79627be2d613059aba077 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 --- 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 76d91a5b09e..c67b3752e2d 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 738ee8cf40d..6b7aecbe812 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 0cc22343c51..98da8af73aa 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.27.0