From 27efe27809fdcdd9151c514bf9b943f1ab86db9c Mon Sep 17 00:00:00 2001 From: Kyle M Hall Date: Tue, 18 Jul 2023 09:06:36 -0400 Subject: [PATCH 3/5] Bug 25159: Add ability to specify a pre-modified version of action log data and store as diff Test Plan: 1) sudo cpanm Struct::Diff 2) prove t/db_dependent/Log.t Signed-off-by: Kyle Hall Signed-off-by: Andrew Fuerste-Henry Signed-off-by: Emmi Takkinen --- C4/Log.pm | 23 ++++++++++++++++------- cpanfile | 1 + t/db_dependent/Log.t | 17 ++++++++++++++++- 3 files changed, 33 insertions(+), 8 deletions(-) diff --git a/C4/Log.pm b/C4/Log.pm index d16d4236c5..cc3811a15b 100644 --- a/C4/Log.pm +++ b/C4/Log.pm @@ -25,9 +25,10 @@ use strict; use warnings; use Data::Dumper qw( Dumper ); -use JSON qw( to_json ); -use Scalar::Util qw( blessed ); use File::Basename qw( basename ); +use JSON qw( to_json encode_json ); +use Scalar::Util qw( blessed ); +use Struct::Diff qw( diff ); use C4::Context; use Koha::Logger; @@ -59,7 +60,7 @@ The functions in this module perform various functions in order to log all the o =item logaction - &logaction($modulename, $actionname, $objectnumber, $infos, $interface); + &logaction($modulename, $actionname, $objectnumber, $infos, $interface, $hashref_of_original); Adds a record into action_logs table to report the different changes upon the database. Each log entry includes the number of the user currently logged in. For batch @@ -70,7 +71,9 @@ number is set to 0, which is the same as the superlibrarian's number. #' sub logaction { - my ($modulename, $actionname, $objectnumber, $infos, $interface)=@_; + my ($modulename, $actionname, $objectnumber, $infos, $interface, $original )=@_; + + my $updated; # Get ID of logged in user. if called from a batch job, # no user session exists and C4::Context->userenv() returns @@ -82,13 +85,16 @@ sub logaction { if( blessed($infos) && $infos->isa('Koha::Object') ) { $infos = $infos->get_from_storage if $infos->in_storage; + $updated = $infos->unblessed; local $Data::Dumper::Sortkeys = 1; if ( $infos->isa('Koha::Item') && $modulename eq 'CATALOGUING' && $actionname eq 'MODIFY' ) { - $infos = "item " . Dumper( $infos->unblessed ); + $infos = "item " . Dumper( $updated ); } else { - $infos = Dumper( $infos->unblessed ); + $infos = Dumper( $updated ); } + } else { + $updated = $infos; } my $script = ( $interface eq 'cron' or $interface eq 'commandline' ) @@ -112,6 +118,8 @@ sub logaction { } my $trace = @trace ? to_json( \@trace, { utf8 => 1, pretty => 0 } ) : undef; + my $diff = ( $original && ref $updated eq 'HASH' ) ? encode_json( diff( $original, $updated, noU => 1 ) ) : undef; + Koha::ActionLog->new( { timestamp => \'NOW()', @@ -123,6 +131,7 @@ sub logaction { interface => $interface, script => $script, trace => $trace, + diff => $diff, } )->store(); @@ -134,7 +143,7 @@ sub logaction { ); $logger->debug( sub { - "ACTION LOG: " . to_json( + "ACTION LOG: " . encode_json( { user => $usernumber, module => $modulename, diff --git a/cpanfile b/cpanfile index 41a4eb7533..8f7faf6730 100644 --- a/cpanfile +++ b/cpanfile @@ -95,6 +95,7 @@ requires 'POSIX', '1.09'; requires 'Plack::Middleware::LogWarn', '0.001002'; requires 'Plack::Middleware::ReverseProxy', '0.14'; requires 'Readonly', '2.00'; +requires 'Struct::Diff', '0.98'; requires 'Schedule::At', '1.06'; requires 'Search::Elasticsearch', '6.80'; requires 'Sereal::Decoder', '3.0'; diff --git a/t/db_dependent/Log.t b/t/db_dependent/Log.t index ed2334f853..b870421856 100755 --- a/t/db_dependent/Log.t +++ b/t/db_dependent/Log.t @@ -16,7 +16,7 @@ use Modern::Perl; use Data::Dumper qw( Dumper ); -use Test::More tests => 5; +use Test::More tests => 6; use C4::Context; use C4::Log qw( logaction cronlogaction ); @@ -27,6 +27,8 @@ use Koha::ActionLogs; use t::lib::Mocks qw/mock_preference/; # to mock CronjobLog use t::lib::TestBuilder; +use JSON qw( decode_json ); + # Make sure we can rollback. our $schema = Koha::Database->new->schema; $schema->storage->txn_begin; @@ -194,4 +196,17 @@ subtest 'Reduce log size by unblessing Koha objects' => sub { like( $logs->next->info, qr/^t::lib::TestBuilder/, 'Dumped TestBuilder object' ); }; +subtest 'Test storing diff of objects' => sub { + plan tests => 1; + + my $builder = t::lib::TestBuilder->new; + my $item = $builder->build_sample_item; + my $original = $item->unblessed(); + $item->barcode('_MY_TEST_BARCODE_')->store(); + logaction( 'MY_MODULE', 'TEST01', $item->itemnumber, $item, 'opac', $original ); + my $logs = Koha::ActionLogs->search({ module => 'MY_MODULE', action => 'TEST01', object => $item->itemnumber })->next; + my $diff = decode_json( $logs->diff ); + is( $diff->{D}->{barcode}->{N}, '_MY_TEST_BARCODE_', 'Diff of changes logged successfully' ); +}; + $schema->storage->txn_rollback; -- 2.34.1