diff --git a/C4/AuthoritiesMarc.pm b/C4/AuthoritiesMarc.pm index 8866539268..f91e948f2f 100644 --- a/C4/AuthoritiesMarc.pm +++ b/C4/AuthoritiesMarc.pm @@ -692,7 +692,6 @@ sub AddAuthority { )->store(); $authority->discard_changes(); $authid = $authority->authid; - logaction( "AUTHORITIES", "ADD", $authid, "authority" ) if C4::Context->preference("AuthoritiesLog"); } else { $action = 'modify'; $authority = Koha::Authorities->find($authid); @@ -722,6 +721,15 @@ sub AddAuthority { } _after_authority_action_hooks( { action => $action, authority_id => $authid } ); + + if ( $action eq 'create' && C4::Context->preference("AuthoritiesLog") ) { + $authority->discard_changes; + logaction( + "AUTHORITIES", "ADD", $authid, "authority", undef, + _authority_log_data( $authority, $record ) + ); + } + return ($authid); } @@ -751,7 +759,13 @@ sub DelAuthority { Koha::Authority::MergeRequests->search($condition)->delete; merge( { mergefrom => $authid } ) if !$skip_merge; - my $authority = Koha::Authorities->find($authid); + my $authority = Koha::Authorities->find($authid); + my $deleted_marc = eval { $authority->record } if C4::Context->preference("AuthoritiesLog"); + my $deleted_data = + C4::Context->preference("AuthoritiesLog") + ? _authority_log_data( $authority, $deleted_marc ) + : undef; + $schema->txn_do( sub { $authority->move_to_deleted; #FIXME We should define 'move' .. @@ -759,7 +773,8 @@ sub DelAuthority { } ); - logaction( "AUTHORITIES", "DELETE", $authid, "authority" ) if C4::Context->preference("AuthoritiesLog"); + logaction( "AUTHORITIES", "DELETE", $authid, "authority", undef, $deleted_data ) + if C4::Context->preference("AuthoritiesLog"); unless ($skip_record_index) { my $indexer = Koha::SearchEngine::Indexer->new( { index => $Koha::SearchEngine::AUTHORITIES_INDEX } ); $indexer->index_records( $authid, "recordDelete", "authorityserver", undef ); @@ -791,13 +806,26 @@ sub ModAuthority { my $skip_record_index = $params->{skip_record_index} || 0; my $oldrecord = GetAuthority($authid); + my ( $original_data, $original_marc ); + if ( C4::Context->preference("AuthoritiesLog") ) { + my $old_authority = Koha::Authorities->find($authid); + $original_data = _authority_log_data( $old_authority, $oldrecord ); + } #Now rewrite the $record to table with an add $authid = AddAuthority( $record, $authid, $authtypecode, { skip_record_index => $skip_record_index } ); merge( { mergefrom => $authid, MARCfrom => $oldrecord, mergeto => $authid, MARCto => $record } ) if !$params->{skip_merge}; - logaction( "AUTHORITIES", "MODIFY", $authid, "authority BEFORE=>" . $oldrecord->as_formatted ) - if C4::Context->preference("AuthoritiesLog"); + + if ( C4::Context->preference("AuthoritiesLog") ) { + my $new_authority = Koha::Authorities->find($authid); + logaction( + "AUTHORITIES", "MODIFY", $authid, + _authority_log_data( $new_authority, $record ), + undef, $original_data + ); + } + return $authid; } @@ -1502,6 +1530,35 @@ sub _get_authid_subfield { return $field->subfield('9') || $field->subfield('3'); } +sub _marc_record_to_diffable { + my ($record) = @_; + return {} unless defined $record; + my %marc; + for my $field ( $record->fields ) { + my $tag = $field->tag; + if ( $field->is_control_field ) { + $marc{$tag} = $field->data; + } else { + my $ind1 = $field->indicator(1); + my $ind2 = $field->indicator(2); + my $formatted = "$ind1$ind2"; + for my $subfield ( $field->subfields ) { + $formatted .= " \$$subfield->[0] $subfield->[1]"; + } + push @{ $marc{$tag} }, $formatted; + } + } + return \%marc; +} + +sub _authority_log_data { + my ( $authority, $marc_record ) = @_; + my %data = %{ $authority->unblessed }; + delete $data{marcxml}; + $data{_marc} = _marc_record_to_diffable($marc_record); + return \%data; +} + =head2 GetHeaderAuthority $ref= &GetHeaderAuthority( $authid) diff --git a/t/db_dependent/AuthoritiesMarc.t b/t/db_dependent/AuthoritiesMarc.t index e223ddba34..ac33398bf7 100755 --- a/t/db_dependent/AuthoritiesMarc.t +++ b/t/db_dependent/AuthoritiesMarc.t @@ -6,7 +6,7 @@ use Modern::Perl; use Test::NoWarnings; -use Test::More tests => 14; +use Test::More tests => 15; use Test::MockModule; use Test::Warn; use MARC::Field; @@ -14,6 +14,7 @@ use MARC::Record; use t::lib::Mocks; use t::lib::TestBuilder; +use Koha::ActionLogs; use Koha::Database; use Koha::Authority::Types; @@ -398,3 +399,49 @@ subtest 'DelAuthority() tests' => sub { $schema->storage->txn_rollback; }; + +subtest 'Authority action logs include MARC diff' => sub { + plan tests => 12; + + $schema->storage->txn_begin; + + t::lib::Mocks::mock_preference( 'AuthoritiesLog', 1 ); + + my $auth_type = 'GEOGR_NAME'; + + # ADD + my $add_record = MARC::Record->new; + $add_record->add_fields( [ '151', ' ', ' ', a => 'France' ] ); + my $auth_id = AddAuthority( $add_record, undef, $auth_type ); + + my $add_logs = Koha::ActionLogs->search( { object => $auth_id, module => 'AUTHORITIES', action => 'ADD' } ); + is( $add_logs->count, 1, 'ADD action logged' ); + my $add_log = $add_logs->next; + ok( defined $add_log->diff, 'ADD: diff column populated' ); + like( $add_log->diff, qr/_marc/, 'ADD: diff contains MARC field data' ); + like( $add_log->diff, qr/France/, 'ADD: diff contains heading content' ); + + # MODIFY - change the heading + my $mod_record = MARC::Record->new; + $mod_record->add_fields( [ '151', ' ', ' ', a => 'France (updated)' ] ); + ModAuthority( $auth_id, $mod_record, $auth_type, { skip_merge => 1 } ); + + my $mod_logs = Koha::ActionLogs->search( { object => $auth_id, module => 'AUTHORITIES', action => 'MODIFY' } ); + is( $mod_logs->count, 1, 'MODIFY action logged' ); + my $mod_log = $mod_logs->next; + ok( defined $mod_log->diff, 'MODIFY: diff column populated' ); + like( $mod_log->diff, qr/_marc/, 'MODIFY: diff contains MARC field data' ); + like( $mod_log->diff, qr/France/, 'MODIFY: diff captures before value' ); + like( $mod_log->diff, qr/France.*updated/s, 'MODIFY: diff captures after value' ); + + # DELETE + DelAuthority( { authid => $auth_id, skip_merge => 1 } ); + + my $del_logs = Koha::ActionLogs->search( { object => $auth_id, module => 'AUTHORITIES', action => 'DELETE' } ); + is( $del_logs->count, 1, 'DELETE action logged' ); + my $del_log = $del_logs->next; + ok( defined $del_log->diff, 'DELETE: diff column populated' ); + like( $del_log->diff, qr/_marc/, 'DELETE: diff contains MARC field data' ); + + $schema->storage->txn_rollback; +};