gitea
/
Koha-clms
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Bug 42032: Include MARC field changes in AUTHORITIES diff logs 

The AUTHORITIES action log entries for authority operations previously
stored minimal data: ADD and DELETE logged just the string "authority",
while MODIFY logged a raw MARC-formatted dump of the pre-change record
as a plain text string ("authority BEFORE=>..."). None of these populated
the diff column.

This patch updates AddAuthority, ModAuthority and DelAuthority in
C4/AuthoritiesMarc.pm to follow the same pattern introduced for biblios
in bug 40135: passing combined hashrefs to logaction so that the diff
column is automatically populated with Struct::Diff JSON.

The combined hashref contains the auth_header table columns (authtypecode,
heading, origincode, etc. — excluding the raw marcxml blob) plus a _marc
key built by the new _marc_record_to_diffable helper, which converts a
MARC::Record into a tag => array-of-formatted-strings structure readable
by the existing diff viewer.

A new _authority_log_data helper packages the authority unblessed data
and MARC record into the combined hashref, keeping the three call sites
concise.

For ADD, the logaction is now emitted after the authority record has been
fully stored (after _after_authority_action_hooks), so the diff captures
the complete saved state. For MODIFY, the before-state is captured from
Koha::Authorities before calling AddAuthority, and the after-state is
read back once AddAuthority has committed the new data. For DELETE, the
MARC is captured before the authority is removed.

The diff column for MODIFY will now show both authority table column
changes (e.g. heading) and MARC field-level changes (e.g. _marc.151.0)
in the existing diff viewer table in the log viewer.

Tests are added to verify that ADD, MODIFY, and DELETE all populate the
diff column and that _marc content is captured correctly.

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
This commit is contained in:
Martin Renvoize 2026-03-09 18:00:31 +00:00 committed by Lucas Gass
parent 4bd9e7836b
commit 79b42fec90
No known key found for this signature in database
GPG Key ID: 60B447398B0D36CA
2 changed files with 110 additions and 6 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

67
C4/AuthoritiesMarc.pm
View File

@ -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)

49
t/db_dependent/AuthoritiesMarc.t
View File

@ -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;
};