@@ -, +, @@ preventing circulation from spamming the log -- there will be data. This is the floodiness that will be removed. -- there should be no data. -- there should be data reflecting the changes made. --- C4/Circulation.pm | 18 ++++++++++-------- C4/Items.pm | 17 +++++++++++------ t/db_dependent/Items.t | 46 +++++++++++++++++++++++++++++++++++++++++++--- 3 files changed, 64 insertions(+), 17 deletions(-) --- a/C4/Circulation.pm +++ a/C4/Circulation.pm @@ -1399,7 +1399,8 @@ sub AddIssue { datelastborrowed => DateTime->now( time_zone => C4::Context->tz() )->ymd(), }, $item->{'biblionumber'}, - $item->{'itemnumber'} + $item->{'itemnumber'}, + 0 ); ModDateLastSeen( $item->{'itemnumber'} ); @@ -1832,7 +1833,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) @@ -1856,7 +1857,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; } } @@ -1922,7 +1923,7 @@ sub AddReturn { } - ModItem({ onloan => undef }, $item->{biblionumber}, $item->{'itemnumber'}); + ModItem( { onloan => undef }, $item->{biblionumber}, $item->{itemnumber}, 0 ); } # the holdingbranch is updated if the document is returned to another location. @@ -2158,7 +2159,7 @@ sub MarkIssueReturned { # And finally delete the issue $issue->delete; - ModItem( { 'onloan' => undef }, undef, $itemnumber ); + ModItem( { 'onloan' => undef }, undef, $itemnumber, 0 ); if ( C4::Context->preference('StoreLastBorrower') ) { my $item = Koha::Items->find( $itemnumber ); @@ -2397,7 +2398,7 @@ sub _FixAccountForLostAndReturned { } ); - ModItem( { paidfor => '' }, undef, $itemnumber ); + ModItem( { paidfor => '' }, undef, $itemnumber, 0 ); return $credit_id; } @@ -2822,7 +2823,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); + ModItem( { renewals => $renews, onloan => $datedue->strftime('%Y-%m-%d %H:%M')}, $item->{biblionumber}, $itemnumber, 0 ); # Charge a new rental fee, if applicable? my ( $charge, $type ) = GetIssuingCharges( $itemnumber, $borrowernumber ); @@ -3701,7 +3702,8 @@ sub ProcessOfflineReturn { ModItem( { renewals => 0, onloan => undef }, $issue->{'biblionumber'}, - $itemnumber + $itemnumber, + 0 ); return "Success."; } else { --- a/C4/Items.pm +++ a/C4/Items.pm @@ -517,7 +517,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. @@ -538,12 +538,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) { @@ -552,8 +556,8 @@ sub ModItem { my $dbh = @_ ? shift : C4::Context->dbh; my $frameworkcode = @_ ? shift : C4::Biblio::GetFrameworkCode( $biblionumber ); - - my $unlinked_item_subfields; + + my $unlinked_item_subfields; if (@_) { $unlinked_item_subfields = shift; $item->{'more_subfields_xml'} = _get_unlinked_subfields_xml($unlinked_item_subfields); @@ -602,7 +606,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 @@ -646,9 +651,9 @@ C<$itemnum> is the item number sub ModDateLastSeen { my ($itemnumber) = @_; - + my $today = output_pref({ dt => dt_from_string, dateformat => 'iso', dateonly => 1 }); - ModItem({ itemlost => 0, datelastseen => $today }, undef, $itemnumber); + ModItem( { itemlost => 0, datelastseen => $today }, undef, $itemnumber, 0 ); } =head2 DelItem --- a/t/db_dependent/Items.t +++ a/t/db_dependent/Items.t @@ -25,13 +25,12 @@ use Koha::Database; use Koha::DateUtils qw( dt_from_string ); use Koha::Library; use Koha::DateUtils; +use Koha::MarcSubfieldStructures; +use Koha::Caches; use t::lib::Mocks; use t::lib::TestBuilder; -use Koha::MarcSubfieldStructures; -use Koha::Caches; - use Test::More tests => 13; use Test::Warn; @@ -797,6 +796,47 @@ subtest 'get_hostitemnumbers_of' => sub { is( @itemnumbers, 0, ); }; +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 ) = @_; --