From 873fcbb51f677718db8fcd2b0f0f3fd476ae224c Mon Sep 17 00:00:00 2001 From: Martin Renvoize Date: Thu, 26 Mar 2026 11:38:45 +0100 Subject: [PATCH] Bug 42188: Use MARC-in-JSON for biblio action log diffs Replace the custom _marc_record_to_diffable helper with MARC::Record::MiJ's to_mij_structure method, which produces standard MARC-in-JSON. This fixes three problems with the old approach: - Dollar signs used as subfield delimiters in the formatted string were ambiguous when subfield content itself contained " $x " sequences. - Grouping fields by tag into a hash lost inter-field ordering, making lossless reconstruction of the MARC record impossible. - Non-ASCII characters were subject to Perl string context encoding issues; JSON serialisation of the MiJ structure handles UTF-8 cleanly. _unblessed_for_log in Koha::Biblio is updated to return only the three biblio columns that are genuinely independent of the MARC record (frameworkcode, opac_suppressed, serial). The remaining columns (author, title, subtitle, notes, etc.) are denormalised copies of MARC data already captured in the _marc key, so including them produced redundant and noisy diffs. The fmt() helper in viewlog.js is updated to render JavaScript arrays as ordered lists rather than objects with numeric keys, improving the display of MARC subfield arrays in the diff table. Signed-off-by: David Nind Signed-off-by: Lisette Scheer Signed-off-by: Lucas Gass --- C4/Biblio.pm | 46 ++++++---------------- Koha/Biblio.pm | 32 +++++++++------ koha-tmpl/intranet-tmpl/prog/js/viewlog.js | 8 ++++ 3 files changed, 38 insertions(+), 48 deletions(-) diff --git a/C4/Biblio.pm b/C4/Biblio.pm index 0fefd336dd..4f84b45bec 100644 --- a/C4/Biblio.pm +++ b/C4/Biblio.pm @@ -75,6 +75,7 @@ use Try::Tiny qw( catch try ); use Encode; use List::MoreUtils qw( uniq ); use MARC::Record; +use MARC::Record::MiJ; use MARC::File::USMARC; use MARC::File::XML; use POSIX qw( strftime ); @@ -316,7 +317,10 @@ sub AddBiblio { my $marc_for_log = eval { $new_biblio->metadata->record_strip_nonxml }; logaction( "CATALOGUING", "ADD", $biblionumber, "biblio", undef, - { %{ $new_biblio->_unblessed_for_log }, _marc => _marc_record_to_diffable($marc_for_log) } + { + %{ $new_biblio->_unblessed_for_log }, + _marc => ( $marc_for_log ? $marc_for_log->to_mij_structure : {} ) + } ); } @@ -475,9 +479,12 @@ sub ModBiblio { my $updated_marc_log = eval { $updated_biblio->metadata->record_strip_nonxml }; logaction( "CATALOGUING", "MODIFY", $biblionumber, - { %{ $updated_biblio->_unblessed_for_log }, _marc => _marc_record_to_diffable($updated_marc_log) }, + { + %{ $updated_biblio->_unblessed_for_log }, + _marc => ( $updated_marc_log ? $updated_marc_log->to_mij_structure : {} ) + }, undef, - { %{$original}, _marc => _marc_record_to_diffable($original_marc) } + { %{$original}, _marc => ( $original_marc ? $original_marc->to_mij_structure : {} ) } ); } @@ -502,37 +509,6 @@ MARC bib. =cut -=head2 _marc_record_to_diffable - - my $hashref = _marc_record_to_diffable($record); - -Converts a MARC::Record object into a plain hashref suitable for structural -diffing. Control fields are stored as scalar values; data fields are stored -as arrays of formatted strings containing indicators and subfields. - -=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; @@ -638,7 +614,7 @@ sub DelBiblio { logaction( "CATALOGUING", "DELETE", $biblionumber, "biblio", undef, - { %{ $biblio->_unblessed_for_log }, _marc => _marc_record_to_diffable($biblio_marc) } + { %{ $biblio->_unblessed_for_log }, _marc => ( $biblio_marc ? $biblio_marc->to_mij_structure : {} ) } ) if C4::Context->preference("CataloguingLog"); Koha::BackgroundJob::BatchUpdateBiblioHoldsQueue->new->enqueue( { biblio_ids => [$biblionumber] } ) diff --git a/Koha/Biblio.pm b/Koha/Biblio.pm index eff3a5f2c9..377251c345 100644 --- a/Koha/Biblio.pm +++ b/Koha/Biblio.pm @@ -2405,25 +2405,31 @@ sub merge_with { =head3 _unblessed_for_log -Returns a plain hashref of biblio column values safe for JSON serialisation -and action log diffing. Timestamp is excluded as it changes on every write, -and undef/empty-string fields are removed so that genuinely unset columns -do not produce noise in diffs. +Returns a plain hashref of biblio column values that are not derived from +the attached MARC record, for use in action log diffing. + +Fields such as C, C, C<subtitle> etc. are omitted because +they are denormalised copies of MARC data and would duplicate what is +already captured in the C<_marc> key of the log entry. Only fields that +carry independent Koha metadata are retained: C<frameworkcode>, +C<opac_suppressed>, and C<serial>. The C<timestamp> and C<datecreated> +columns are also excluded as they are system-managed and add no signal. =cut +# Biblio table columns that exist independently of the MARC record. +my @_LOG_FIELDS = qw( frameworkcode opac_suppressed serial ); + sub _unblessed_for_log { my ($self) = @_; - my $data = $self->unblessed; - delete $data->{timestamp}; - for my $key ( keys %$data ) { - if ( defined $data->{$key} && $data->{$key} ne '' ) { - $data->{$key} = "$data->{$key}" if blessed( $data->{$key} ); - } else { - delete $data->{$key}; - } + my $full = $self->unblessed; + my %data; + for my $key (@_LOG_FIELDS) { + next unless exists $full->{$key}; + next unless defined $full->{$key} && $full->{$key} ne ''; + $data{$key} = blessed( $full->{$key} ) ? "$full->{$key}" : $full->{$key}; } - return $data; + return \%data; } =head3 _type diff --git a/koha-tmpl/intranet-tmpl/prog/js/viewlog.js b/koha-tmpl/intranet-tmpl/prog/js/viewlog.js index f0e1d1f332..399732f0e2 100644 --- a/koha-tmpl/intranet-tmpl/prog/js/viewlog.js +++ b/koha-tmpl/intranet-tmpl/prog/js/viewlog.js @@ -43,6 +43,14 @@ function renderStructDiff(raw) { function fmt(v) { if (v === undefined || v === null) return ""; if (typeof v !== "object") return $("<span>").text(String(v)).html(); + if (Array.isArray(v)) { + var items = v + .map(function (item) { + return "<li>" + fmt(item) + "</li>"; + }) + .join(""); + return '<ul class="diff-obj">' + items + "</ul>"; + } var items = Object.keys(v) .map(function (k) { return (