@@ -, +, @@ preventing circulation from spamming the log but editing the item will continue to cause log entries. --- C4/Circulation.pm | 18 ++++++++++-------- C4/Items.pm | 9 +++++++-- t/db_dependent/Items.t | 43 ++++++++++++++++++++++++++++++++++++++++++- 3 files changed, 59 insertions(+), 11 deletions(-) --- a/C4/Circulation.pm +++ a/C4/Circulation.pm @@ -1421,7 +1421,8 @@ sub AddIssue { datelastborrowed => DateTime->now( time_zone => C4::Context->tz() )->ymd(), }, $item->{'biblionumber'}, - $item->{'itemnumber'} + $item->{'itemnumber'}, + 0 ); ModDateLastSeen( $item->{'itemnumber'} ); @@ -1863,7 +1864,7 @@ sub AddReturn { $item->{location} = $item->{permanent_location}; } - ModItem( $item, $item->{'biblionumber'}, $item->{'itemnumber'} ); + ModItem( $item, $item->{'biblionumber'}, $item->{'itemnumber'}, 0 ); } # full item data, but no borrowernumber or checkout info (no issue) @@ -1887,7 +1888,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 ); + ModItem( { notforloan => $rules->{$key} }, undef, $itemnumber, 0 ); last; } } @@ -1959,7 +1960,7 @@ sub AddReturn { } - ModItem({ onloan => undef }, $issue->{'biblionumber'}, $item->{'itemnumber'}); + ModItem({ onloan => undef }, $issue->{'biblionumber'}, $item->{'itemnumber'}, 0 ); } # the holdingbranch is updated if the document is returned to another location. @@ -2200,7 +2201,7 @@ sub MarkIssueReturned { $dbh->do(q|DELETE FROM issues WHERE issue_id = ?|, undef, $issue_id); - ModItem( { 'onloan' => undef }, undef, $itemnumber ); + ModItem( { 'onloan' => undef }, undef, $itemnumber, 0 ); if ( C4::Context->preference('StoreLastBorrower') ) { my $item = Koha::Items->find( $itemnumber ); @@ -2437,7 +2438,7 @@ sub _FixAccountForLostAndReturned { } ); - ModItem( { paidfor => '' }, undef, $itemnumber ); + ModItem( { paidfor => '' }, undef, $itemnumber, 0 ); return $credit_id; } @@ -2894,7 +2895,7 @@ sub AddRenewal { # Update the renewal count on the item, and tell zebra to reindex $renews = $biblio->{'renewals'} + 1; - ModItem({ renewals => $renews, onloan => $datedue->strftime('%Y-%m-%d %H:%M')}, $biblio->{'biblionumber'}, $itemnumber); + ModItem({ renewals => $renews, onloan => $datedue->strftime('%Y-%m-%d %H:%M')}, $biblio->{'biblionumber'}, $itemnumber, 0); # Charge a new rental fee, if applicable? my ( $charge, $type ) = GetIssuingCharges( $itemnumber, $borrowernumber ); @@ -3760,7 +3761,8 @@ sub ProcessOfflineReturn { ModItem( { renewals => 0, onloan => undef }, $issue->{'biblionumber'}, - $itemnumber + $itemnumber, + 0 ); return "Success."; } else { --- a/C4/Items.pm +++ a/C4/Items.pm @@ -518,7 +518,7 @@ sub ModItemFromMarc { =head2 ModItem - ModItem({ column => $newvalue }, $biblionumber, $itemnumber); + ModItem({ column => $newvalue }, $biblionumber, $itemnumber, $log_action ); Change one or more columns in an item record and update the MARC representation of the item. @@ -539,12 +539,16 @@ the derived value of a column such as C, this routine will perform the necessary calculation and set the value. +If log_action is set to false, the action will not be logged. +If log_action is true or undefined, the action will be logged. + =cut sub ModItem { my $item = shift; my $biblionumber = shift; my $itemnumber = shift; + my $log_action = shift // 1; # if $biblionumber is undefined, get it from the current item unless (defined $biblionumber) { @@ -603,7 +607,8 @@ sub ModItem { # item status is possible ModZebra( $biblionumber, "specialUpdate", "biblioserver" ); - logaction("CATALOGUING", "MODIFY", $itemnumber, "item ".Dumper($item)) if C4::Context->preference("CataloguingLog"); + logaction( "CATALOGUING", "MODIFY", $itemnumber, "item " . Dumper($item) ) + if $log_action && C4::Context->preference("CataloguingLog"); } =head2 ModItemTransfer --- a/t/db_dependent/Items.t +++ a/t/db_dependent/Items.t @@ -26,7 +26,7 @@ use Koha::Library; use t::lib::Mocks; use t::lib::TestBuilder; -use Test::More tests => 11; +use Test::More tests => 12; use Test::Warn; @@ -782,6 +782,47 @@ subtest '_mod_item_dates' => sub { 'yetanotherdatetime is ok' ); }; +subtest 'Test logging for AddItem' => sub { + + plan tests => 3; + + t::lib::Mocks::mock_preference('CataloguingLog', 1); + + $schema->storage->txn_begin; + + my $builder = t::lib::TestBuilder->new; + my $library = $builder->build({ + source => 'Branch', + }); + my $itemtype = $builder->build({ + source => 'Itemtype', + }); + + # Create a biblio instance for testing + t::lib::Mocks::mock_preference('marcflavour', 'MARC21'); + my ($bibnum, $bibitemnum) = get_biblio(); + + # Add an item. + my ($item_bibnum, $item_bibitemnum, $itemnumber) = AddItem({ homebranch => $library->{branchcode}, holdingbranch => $library->{branchcode}, location => $location, itype => $itemtype->{itemtype} } , $bibnum); + + # False means no logging + $schema->resultset('ActionLog')->search()->delete(); + ModItem({ location => $location }, $bibnum, $itemnumber, 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 ); + + # Undefined defaults to true + $schema->resultset('ActionLog')->search()->delete(); + ModItem({ location => $location }, $bibnum, $itemnumber); + is( $schema->resultset('ActionLog')->count(), 1, 'Undefined value defaults to true, triggers logging' ); + + $schema->storage->txn_rollback; +}; + # Helper method to set up a Biblio. sub get_biblio { my ( $frameworkcode ) = @_; --