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 <tomascohen@theke.io>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

diff --git a/C4/Circulation.pm b/C4/Circulation.pm
index efda14c..303dca0 100644 (file)
--- a/C4/Circulation.pm
+++ b/C4/Circulation.pm
@@ -3096,17 +3096,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");
 
@@ -4336,23 +4339,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 738ee8c..6b7aecb 100755 (executable)
--- 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 0cc2234..98da8af 100644 (file)
--- 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 {