From 0a71e1df9858d55dddbeff6b31f60426e086570c Mon Sep 17 00:00:00 2001 From: Kyle M Hall 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 Signed-off-by: Jonathan Druart --- 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