From 0a71e1df9858d55dddbeff6b31f60426e086570c Mon Sep 17 00:00:00 2001
From: Kyle M Hall <kyle@bywatetsolutions.com>
Date: Thu, 22 Mar 2018 06:43:44 -0400
Subject: [PATCH] Bug 18816: (QA follow-up) Convert param to hashref, fix typo

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
---
 C4/Circulation.pm      | 16 ++++++++--------
 C4/Items.pm            |  6 ++++--
 t/db_dependent/Items.t | 10 +++++-----
 3 files changed, 17 insertions(+), 15 deletions(-)

diff --git a/C4/Circulation.pm b/C4/Circulation.pm
index 15c05fc899..b9bd533ca0 100644
--- a/C4/Circulation.pm
+++ b/C4/Circulation.pm
@@ -1398,7 +1398,7 @@ sub AddIssue {
                 },
                 $item->{'biblionumber'},
                 $item->{'itemnumber'},
-                0
+                { log_action => 0 }
             );
             ModDateLastSeen( $item->{'itemnumber'} );
 
@@ -1831,7 +1831,7 @@ sub AddReturn {
             $item->{location} = $item->{permanent_location};
         }
 
-        ModItem( $item, $item->{'biblionumber'}, $item->{'itemnumber'}, 0 );
+        ModItem( $item, $item->{'biblionumber'}, $item->{'itemnumber'}, { log_action => 0 } );
     }
 
         # full item data, but no borrowernumber or checkout info (no issue)
@@ -1855,7 +1855,7 @@ sub AddReturn {
             foreach my $key ( keys %$rules ) {
                 if ( $item->{notforloan} eq $key ) {
                     $messages->{'NotForLoanStatusUpdated'} = { from => $item->{notforloan}, to => $rules->{$key} };
-                    ModItem( { notforloan => $rules->{$key} }, undef, $itemnumber, 0 );
+                    ModItem( { notforloan => $rules->{$key} }, undef, $itemnumber, { log_action => 0 } );
                     last;
                 }
             }
@@ -1921,7 +1921,7 @@ sub AddReturn {
 
         }
 
-        ModItem( { onloan => undef }, $item->{biblionumber}, $item->{itemnumber}, 0 );
+        ModItem( { onloan => undef }, $item->{biblionumber}, $item->{itemnumber}, { log_action => 0 } );
     }
 
     # the holdingbranch is updated if the document is returned to another location.
@@ -2157,7 +2157,7 @@ sub MarkIssueReturned {
         # And finally delete the issue
         $issue->delete;
 
-        ModItem( { 'onloan' => undef }, undef, $itemnumber, 0 );
+        ModItem( { 'onloan' => undef }, undef, $itemnumber, { log_action => 0 } );
 
         if ( C4::Context->preference('StoreLastBorrower') ) {
             my $item = Koha::Items->find( $itemnumber );
@@ -2396,7 +2396,7 @@ sub _FixAccountForLostAndReturned {
         }
     );
 
-    ModItem( { paidfor => '' }, undef, $itemnumber, 0 );
+    ModItem( { paidfor => '' }, undef, $itemnumber, { log_action => 0 } );
 
     return $credit_id;
 }
@@ -2821,7 +2821,7 @@ sub AddRenewal {
 
     # Update the renewal count on the item, and tell zebra to reindex
     $renews = $item->{renewals} + 1;
-    ModItem( { renewals => $renews, onloan => $datedue->strftime('%Y-%m-%d %H:%M')}, $item->{biblionumber}, $itemnumber, 0 );
+    ModItem( { renewals => $renews, onloan => $datedue->strftime('%Y-%m-%d %H:%M')}, $item->{biblionumber}, $itemnumber, { log_action => 0 } );
 
     # Charge a new rental fee, if applicable?
     my ( $charge, $type ) = GetIssuingCharges( $itemnumber, $borrowernumber );
@@ -3701,7 +3701,7 @@ sub ProcessOfflineReturn {
                 { renewals => 0, onloan => undef },
                 $issue->{'biblionumber'},
                 $itemnumber,
-                0
+                { log_action => 0 }
             );
             return "Success.";
         } else {
diff --git a/C4/Items.pm b/C4/Items.pm
index f2c7dc238d..5de34a535c 100644
--- a/C4/Items.pm
+++ b/C4/Items.pm
@@ -547,7 +547,9 @@ sub ModItem {
     my $item = shift;
     my $biblionumber = shift;
     my $itemnumber = shift;
-    my $log_action = shift // 1;
+    my $additional_params = shift;
+
+    my $log_action = $additional_params->{log_action} // 1;
 
     # if $biblionumber is undefined, get it from the current item
     unless (defined $biblionumber) {
@@ -653,7 +655,7 @@ sub ModDateLastSeen {
     my ($itemnumber) = @_;
 
     my $today = output_pref({ dt => dt_from_string, dateformat => 'iso', dateonly => 1 });
-    ModItem( { itemlost => 0, datelastseen => $today }, undef, $itemnumber, 0 );
+    ModItem( { itemlost => 0, datelastseen => $today }, undef, $itemnumber, { log_action => 0 } );
 }
 
 =head2 DelItem
diff --git a/t/db_dependent/Items.t b/t/db_dependent/Items.t
index da74ca3eef..ef098734c9 100755
--- a/t/db_dependent/Items.t
+++ b/t/db_dependent/Items.t
@@ -31,7 +31,7 @@ use Koha::Caches;
 use t::lib::Mocks;
 use t::lib::TestBuilder;
 
-use Test::More tests => 13;
+use Test::More tests => 14;
 
 use Test::Warn;
 
@@ -796,7 +796,7 @@ subtest 'get_hostitemnumbers_of' => sub {
     is( @itemnumbers, 0, );
 };
 
-subtest 'Test logging for AddItem' => sub {
+subtest 'Test logging for ModItem' => sub {
 
     plan tests => 3;
 
@@ -821,13 +821,13 @@ subtest 'Test logging for AddItem' => sub {
 
     # False means no logging
     $schema->resultset('ActionLog')->search()->delete();
-    ModItem({ location => $location }, $bibnum, $itemnumber, 0);
+    ModItem({ location => $location }, $bibnum, $itemnumber, { log_action => 0 });
     is( $schema->resultset('ActionLog')->count(), 0, 'False value does not trigger logging' );
 
     # True means logging
     $schema->resultset('ActionLog')->search()->delete();
-    ModItem({ location => $location }, $bibnum, $itemnumber, 1, 'True value does trigger logging');
-    is( $schema->resultset('ActionLog')->count(), 1 );
+    ModItem({ location => $location }, $bibnum, $itemnumber, { log_action => 1 });
+    is( $schema->resultset('ActionLog')->count(), 1, 'True value does trigger logging' );
 
     # Undefined defaults to true
     $schema->resultset('ActionLog')->search()->delete();
-- 
2.39.5