From bf1086512f459b23de9c4be8a8e50c96e6414af1 Mon Sep 17 00:00:00 2001 From: Martin Renvoize Date: Mon, 9 Mar 2026 16:26:42 +0000 Subject: [PATCH] Bug 40135: Include MARC field changes in CATALOGUING diff logs The previous commit populated the diff column for biblio ADD/MODIFY/DELETE logs with changes to biblio table columns (title, author, frameworkcode, etc.), but lost the MARC-level change tracking that previously stored a pre-change MARC dump in the info column. This follow-up extends the diff to also capture MARC field changes by including a synthetic _marc key in the before/after hashrefs passed to logaction. MARC fields are represented as tag => array-of-formatted-strings (e.g. "10 $a Title" for data fields, scalar for control fields), allowing Struct::Diff to produce readable field-level diffs. The _marc entries appear alongside biblio column changes in the existing diff table in the log viewer, using dotted keys like _marc.245.0. A helper _marc_record_to_diffable() converts a MARC::Record to this diffable hashref. All three functions (AddBiblio, ModBiblio, DelBiblio) use record_strip_nonxml (wrapped in eval) to safely capture the MARC even for records with invalid XML characters, without producing warnings or affecting the success of the main operation. C4/Log.pm is also updated to Dumper() a plain hashref passed as $infos, so the info column contains useful data rather than a stringified ref. Tests are extended to verify that the diff column is populated and that _marc changes are captured when MARC content actually changes. --- C4/Biblio.pm | 54 ++++++++++++++++++++++++++++++++++++----- C4/Log.pm | 4 +++ t/db_dependent/Biblio.t | 8 +++++- 3 files changed, 59 insertions(+), 7 deletions(-) mode change 100644 => 100755 t/db_dependent/Biblio.t diff --git a/C4/Biblio.pm b/C4/Biblio.pm index 1608e41e10e..d52ac44ab3c 100644 --- a/C4/Biblio.pm +++ b/C4/Biblio.pm @@ -311,7 +311,12 @@ sub AddBiblio { _after_biblio_action_hooks( { action => 'create', biblio_id => $biblionumber } ); if ( C4::Context->preference("CataloguingLog") ) { - logaction( "CATALOGUING", "ADD", $biblionumber, "biblio", undef, Koha::Biblios->find($biblionumber) ); + my $new_biblio = Koha::Biblios->find($biblionumber); + my $marc_for_log = eval { $new_biblio->metadata->record_strip_nonxml }; + logaction( + "CATALOGUING", "ADD", $biblionumber, "biblio", undef, + { %{ $new_biblio->unblessed }, _marc => _marc_record_to_diffable($marc_for_log) } + ); } # We index now, after the transaction is committed @@ -397,9 +402,11 @@ sub ModBiblio { return 0; } - my $original; + my ( $original, $original_marc ); if ( C4::Context->preference("CataloguingLog") ) { - $original = Koha::Biblios->find($biblionumber)->unblessed; + my $pre_biblio = Koha::Biblios->find($biblionumber); + $original = $pre_biblio->unblessed; + $original_marc = eval { $pre_biblio->metadata->record_strip_nonxml }; } if ( !$options->{disable_autolink} && C4::Context->preference('AutoLinkBiblios') ) { @@ -463,7 +470,14 @@ sub ModBiblio { _after_biblio_action_hooks( { action => 'modify', biblio_id => $biblionumber } ); if ( C4::Context->preference("CataloguingLog") ) { - logaction( "CATALOGUING", "MODIFY", $biblionumber, Koha::Biblios->find($biblionumber), undef, $original ); + my $updated_biblio = Koha::Biblios->find($biblionumber); + my $updated_marc_log = eval { $updated_biblio->metadata->record_strip_nonxml }; + logaction( + "CATALOGUING", "MODIFY", $biblionumber, + { %{ $updated_biblio->unblessed }, _marc => _marc_record_to_diffable($updated_marc_log) }, + undef, + { %{$original}, _marc => _marc_record_to_diffable($original_marc) } + ); } # update OAI-PMH sets @@ -487,6 +501,27 @@ MARC bib. =cut +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 _strip_item_fields { my $record = shift; my $frameworkcode = shift; @@ -533,6 +568,11 @@ sub DelBiblio { my $dbh = C4::Context->dbh; my $error; # for error handling + my $biblio_marc = + C4::Context->preference("CataloguingLog") + ? eval { $biblio->metadata->record_strip_nonxml } + : undef; + # First make sure this biblio has no items attached my $sth = $dbh->prepare("SELECT itemnumber FROM items WHERE biblionumber=?"); $sth->execute($biblionumber); @@ -585,8 +625,10 @@ sub DelBiblio { _after_biblio_action_hooks( { action => 'delete', biblio_id => $biblionumber } ); - logaction( "CATALOGUING", "DELETE", $biblionumber, "biblio", undef, $biblio ) - if C4::Context->preference("CataloguingLog"); + logaction( + "CATALOGUING", "DELETE", $biblionumber, "biblio", undef, + { %{ $biblio->unblessed }, _marc => _marc_record_to_diffable($biblio_marc) } + ) if C4::Context->preference("CataloguingLog"); Koha::BackgroundJob::BatchUpdateBiblioHoldsQueue->new->enqueue( { biblio_ids => [$biblionumber] } ) unless $params->{skip_holds_queue} diff --git a/C4/Log.pm b/C4/Log.pm index 603bc8695a1..24c9b949311 100644 --- a/C4/Log.pm +++ b/C4/Log.pm @@ -94,6 +94,10 @@ sub logaction { } } else { $updated = $infos; + if ( ref $infos eq 'HASH' ) { + local $Data::Dumper::Sortkeys = 1; + $infos = Dumper($updated); + } } my $script = diff --git a/t/db_dependent/Biblio.t b/t/db_dependent/Biblio.t old mode 100644 new mode 100755 index 22965c2344f..72f51e5663a --- a/t/db_dependent/Biblio.t +++ b/t/db_dependent/Biblio.t @@ -1205,7 +1205,7 @@ subtest 'GetFrameworkCode' => sub { }; subtest 'ModBiblio on invalid record' => sub { - plan tests => 2; + plan tests => 4; t::lib::Mocks::mock_preference( "CataloguingLog", 1 ); @@ -1216,12 +1216,18 @@ subtest 'ModBiblio on invalid record' => sub { my ($biblionumber) = C4::Biblio::AddBiblio( $record, '' ); + # Modify the record to add a title field so MARC changes are captured in the diff + my $title_field = MARC::Field->new( '245', '1', '0', 'a' => 'New Title' ); + $record->append_fields($title_field); + C4::Biblio::ModBiblio( $record, $biblionumber, '' ); my $action_logs = Koha::ActionLogs->search( { object => $biblionumber, module => 'Cataloguing', action => 'MODIFY' } ); is( $action_logs->count, 1, "Modification of biblio was successful and recorded" ); my $action_log = $action_logs->next; like( $action_log->info, qr/biblionumber/, "Biblio data logged in action log info" ); + ok( defined $action_log->diff, "Diff column is populated for MODIFY" ); + like( $action_log->diff, qr/_marc/, "MARC field changes are captured in the diff column" ); }; subtest 'UpdateTotalIssues on Invalid record' => sub { -- 2.53.0