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.

Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>

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}

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 =

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 {