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 <david@davidnind.com>
Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>

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] } )

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<author>, C<title>, 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

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 (