Bugzilla – Attachment 195024 Details for
Bug 42032
Add diff support to AUTHORITIES action logs
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 42032: Include MARC field changes in AUTHORITIES diff logs
01edef8.patch (text/plain), 9.35 KB, created by
David Nind
on 2026-03-09 19:23:54 UTC
(
hide
)
Description:
Bug 42032: Include MARC field changes in AUTHORITIES diff logs
Filename:
MIME Type:
Creator:
David Nind
Created:
2026-03-09 19:23:54 UTC
Size:
9.35 KB
patch
obsolete
>From 01edef829a2ed6af4f8addf3d75a00c7ff2fcf86 Mon Sep 17 00:00:00 2001 >From: Martin Renvoize <martin.renvoize@openfifth.co.uk> >Date: Mon, 9 Mar 2026 18:00:31 +0000 >Subject: [PATCH] Bug 42032: Include MARC field changes in AUTHORITIES diff > logs >MIME-Version: 1.0 >Content-Type: text/plain; charset=UTF-8 >Content-Transfer-Encoding: 8bit > >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> >--- > C4/AuthoritiesMarc.pm | 67 +++++++++++++++++++++++++++++--- > t/db_dependent/AuthoritiesMarc.t | 49 ++++++++++++++++++++++- > 2 files changed, 110 insertions(+), 6 deletions(-) > >diff --git a/C4/AuthoritiesMarc.pm b/C4/AuthoritiesMarc.pm >index c2070f4560..2e55a90bb3 100644 >--- a/C4/AuthoritiesMarc.pm >+++ b/C4/AuthoritiesMarc.pm >@@ -685,7 +685,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); >@@ -715,6 +714,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); > } > >@@ -744,7 +752,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' .. >@@ -752,7 +766,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 ); >@@ -784,13 +799,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; > } > >@@ -1495,6 +1523,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; >+}; >-- >2.39.5 >
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 42032
:
195021
| 195024