Bug 21468: (QA follow-up) Simplify payload

This patch simplifies the payload as suggested on bug 25855. It also
keeps some specific params that cannot be deduced from the passed
checkout object, (e.g. if it is an onsite checkout).

Tests are cleared and added for this special exceptions.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

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

diff --git a/C4/Circulation.pm b/C4/Circulation.pm
index 3c17e0d..a0994e1 100644 (file)
--- a/C4/Circulation.pm
+++ b/C4/Circulation.pm
@@ -1509,23 +1509,6 @@ sub AddIssue {
                 }
             }
 
-            _after_circ_actions(
-                {
-                    action  => 'checkout',
-                    payload => {
-                        type              => ( $onsite_checkout ? 'onsite_checkout' : 'issue' ),
-                        library_id        => C4::Context->userenv->{'branch'},
-                        charge            => $charge,
-                        item_id           => $item_object->itemnumber,
-                        item_type         => $item_object->effective_itemtype,
-                        shelving_location => $item_object->location // q{},
-                        patron_id         => $borrower->{'borrowernumber'},
-                        collection_code   => $item_object->ccode // q{},
-                        date_due          => $datedue
-                    }
-                }
-            ) if C4::Context->config("enable_plugins");
-
             # Record the fact that this book was issued.
             &UpdateStats(
                 {
@@ -1564,6 +1547,16 @@ sub AddIssue {
                 $borrower->{'borrowernumber'},
                 $item_object->itemnumber,
             ) if C4::Context->preference("IssueLog");
+
+            _after_circ_actions(
+                {
+                    action  => 'checkout',
+                    payload => {
+                        type     => ( $onsite_checkout ? 'onsite_checkout' : 'issue' ),
+                        checkout => $issue->get_from_storage
+                    }
+                }
+            ) if C4::Context->config("enable_plugins");
         }
     }
     return $issue;
@@ -2143,22 +2136,6 @@ sub AddReturn {
         $messages->{'ResFound'} = $resrec;
     }
 
-    _after_circ_actions(
-        {
-            action  => 'checkin',
-            payload => {
-                library_id        => C4::Context->userenv->{'branch'},
-                item_id           => $item->itemnumber,
-                item_type         => $item->effective_itemtype,
-                shelving_location => $item->location // q{},
-                patron_id         => $borrowernumber,
-                collection_code   => $item->ccode // q{},
-                date_returned     => $return_date,
-                date_due          => $issue ? $issue->date_due : q{}
-            }
-        }
-    ) if C4::Context->config("enable_plugins");
-
     # Record the fact that this book was returned.
     UpdateStats({
         branch         => $branch,
@@ -2231,6 +2208,17 @@ sub AddReturn {
         }
     }
 
+    my $checkin = Koha::Old::Checkouts->find($issue->id);
+
+    _after_circ_actions(
+        {
+            action  => 'checkin',
+            payload => {
+                checkout=> $checkin
+            }
+        }
+    ) if C4::Context->config("enable_plugins");
+
     return ( $doreturn, $messages, $issue, ( $patron ? $patron->unblessed : {} ));
 }
 

diff --git a/t/db_dependent/Koha/Plugins/Circulation_hooks.t b/t/db_dependent/Koha/Plugins/Circulation_hooks.t
index 6dcf0ef..be0f707 100755 (executable)
--- a/t/db_dependent/Koha/Plugins/Circulation_hooks.t
+++ b/t/db_dependent/Koha/Plugins/Circulation_hooks.t
@@ -68,22 +68,25 @@ subtest 'after_circ_action() hook tests' => sub {
     $test_plugin->mock( 'after_biblio_action', undef );
 
     my $biblio = $builder->build_sample_biblio();
-    my $item =
-      $builder->build_sample_item( { biblionumber => $biblio->biblionumber } );
+    my $item_1 = $builder->build_sample_item( { biblionumber => $biblio->biblionumber } );
+    my $item_2 = $builder->build_sample_item( { biblionumber => $biblio->biblionumber } );
 
     subtest 'AddIssue' => sub {
-        plan tests => 1;
+        plan tests => 2;
 
-        warning_like { AddIssue( $patron->unblessed, $item->barcode ); }
-        qr/after_circ_action called with action: checkout, ref: DateTime/,
+        warning_like { AddIssue( $patron->unblessed, $item_1->barcode ); }
+        qr/after_circ_action called with action: checkout, ref: Koha::Checkout type: issue/,
           'AddIssue calls the after_circ_action hook';
 
+        warning_like { AddIssue( $patron->unblessed, $item_2->barcode, undef, undef, undef, undef, { onsite_checkout => 1 } ); }
+        qr/after_circ_action called with action: checkout, ref: Koha::Checkout type: onsite_checkout/,
+          'AddIssue calls the after_circ_action hook (onsite_checkout case)';
     };
 
     subtest 'AddRenewal' => sub {
         plan tests => 1;
 
-        warning_like { AddRenewal( $patron->borrowernumber, $item->id, $patron->branchcode ); }
+        warning_like { AddRenewal( $patron->borrowernumber, $item_1->id, $patron->branchcode ); }
                 qr/after_circ_action called with action: renewal, ref: Koha::Checkout/,
                 'AddRenewal calls the after_circ_action hook';
     };
@@ -92,9 +95,9 @@ subtest 'after_circ_action() hook tests' => sub {
         plan tests => 1;
 
         warning_like {
-            AddReturn( $item->barcode, $patron->branchcode );
+            AddReturn( $item_1->barcode, $patron->branchcode );
         }
-        qr/after_circ_action called with action: checkin, ref: DateTime/,
+        qr/after_circ_action called with action: checkin, ref: Koha::Old::Checkout/,
           'AddReturn calls the after_circ_action hook';
     };
 

diff --git a/t/lib/Koha/Plugin/Test.pm b/t/lib/Koha/Plugin/Test.pm
index 3638fe2..06b24d6 100644 (file)
--- a/t/lib/Koha/Plugin/Test.pm
+++ b/t/lib/Koha/Plugin/Test.pm
@@ -167,24 +167,16 @@ sub after_circ_action {
     my $checkout = $params->{payload}->{checkout};
     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};
-    my $date_returned      = $payload->{date_returned};
+    my $type = $payload->{type};
 
     if ( $action eq 'renewal' ) {
         Koha::Exceptions::Exception->throw("after_circ_action called with action: $action, ref: " . ref($checkout));
     }
     elsif ( $action eq 'checkout') {
-        Koha::Exceptions::Exception->throw("after_circ_action called with action: $action, ref: " . ref($date_due));
+        Koha::Exceptions::Exception->throw("after_circ_action called with action: $action, ref: " . ref($checkout) . " type: $type");
     }
     elsif ( $action eq 'checkin' ) {
-        Koha::Exceptions::Exception->throw("after_circ_action called with action: $action, ref: " . ref($date_returned));
+        Koha::Exceptions::Exception->throw("after_circ_action called with action: $action, ref: " . ref($checkout));
     }
 }