From 3c73ce2ad8dc1f04fdf56d2556d1395f0901a1dd Mon Sep 17 00:00:00 2001 From: Martin Renvoize Date: Fri, 6 Mar 2026 16:35:08 +0000 Subject: [PATCH] Bug 40135: Populate diff column for biblio ADD/MODIFY/DELETE logs Previously the CATALOGUING action log entries for biblio operations stored minimal data: ADD and DELETE logged just the string "biblio", while MODIFY logged a raw MARC-formatted dump of the pre-change record as a plain text string. None of these populated the diff column. This follow-up updates AddBiblio, ModBiblio and DelBiblio in C4/Biblio.pm to follow the same pattern as Koha::Item::store, passing the Koha::Biblio object and an original unblessed hashref to logaction so that the diff column is automatically populated with Struct::Diff JSON reflecting the changes to the biblio table columns (title, author, frameworkcode, etc.). For ADD, logaction diffs an empty hashref against the new biblio state, producing added-field entries for all columns. For DELETE, it diffs the pre-deletion state against an empty hashref, producing removed-field entries. For MODIFY, the before-state is captured prior to the update and the after-state is read back once ModBiblioMarc and _koha_modify_biblio have completed, so the diff reflects the actual committed changes. The test for ModBiblio on an invalid MARC record is updated to reflect that the MARC decoding error is no longer logged in the info column; the biblio table data is now logged instead. --- C4/Biblio.pm | 24 +++++++++++------------- t/db_dependent/Biblio.t | 15 +++++---------- 2 files changed, 16 insertions(+), 23 deletions(-) mode change 100755 => 100644 t/db_dependent/Biblio.t diff --git a/C4/Biblio.pm b/C4/Biblio.pm index 863f4d34b4a..1608e41e10e 100644 --- a/C4/Biblio.pm +++ b/C4/Biblio.pm @@ -310,7 +310,9 @@ sub AddBiblio { ); _after_biblio_action_hooks( { action => 'create', biblio_id => $biblionumber } ); - logaction( "CATALOGUING", "ADD", $biblionumber, "biblio" ) if C4::Context->preference("CataloguingLog"); + if ( C4::Context->preference("CataloguingLog") ) { + logaction( "CATALOGUING", "ADD", $biblionumber, "biblio", undef, Koha::Biblios->find($biblionumber) ); + } # We index now, after the transaction is committed unless ($skip_record_index) { @@ -395,18 +397,9 @@ sub ModBiblio { return 0; } + my $original; if ( C4::Context->preference("CataloguingLog") ) { - my $biblio = Koha::Biblios->find($biblionumber); - my $record; - my $decoding_error = ""; - eval { $record = $biblio->metadata->record }; - if ($@) { - my $exception = $@; - $exception->rethrow unless ( $exception->isa('Koha::Exceptions::Metadata::Invalid') ); - $decoding_error = "There was an error with this bibliographic record: " . $exception; - $record = $biblio->metadata->record_strip_nonxml; - } - logaction( "CATALOGUING", "MODIFY", $biblionumber, "biblio $decoding_error BEFORE=>" . $record->as_formatted ); + $original = Koha::Biblios->find($biblionumber)->unblessed; } if ( !$options->{disable_autolink} && C4::Context->preference('AutoLinkBiblios') ) { @@ -469,6 +462,10 @@ 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 ); + } + # update OAI-PMH sets if ( C4::Context->preference("OAI-PMH:AutoUpdateSets") ) { C4::OAI::Sets::UpdateOAISetsBiblio( $biblionumber, $record ); @@ -588,7 +585,8 @@ sub DelBiblio { _after_biblio_action_hooks( { action => 'delete', biblio_id => $biblionumber } ); - logaction( "CATALOGUING", "DELETE", $biblionumber, "biblio" ) if C4::Context->preference("CataloguingLog"); + logaction( "CATALOGUING", "DELETE", $biblionumber, "biblio", undef, $biblio ) + if C4::Context->preference("CataloguingLog"); Koha::BackgroundJob::BatchUpdateBiblioHoldsQueue->new->enqueue( { biblio_ids => [$biblionumber] } ) unless $params->{skip_holds_queue} diff --git a/t/db_dependent/Biblio.t b/t/db_dependent/Biblio.t old mode 100755 new mode 100644 index bca89014a23..22965c2344f --- a/t/db_dependent/Biblio.t +++ b/t/db_dependent/Biblio.t @@ -1205,28 +1205,23 @@ subtest 'GetFrameworkCode' => sub { }; subtest 'ModBiblio on invalid record' => sub { - plan tests => 3; + plan tests => 2; t::lib::Mocks::mock_preference( "CataloguingLog", 1 ); - # We create a record with an onvalid control character in the MARC + # We create a record with an invalid control character in the MARC my $record = MARC::Record->new(); - my $field = MARC::Field->new( '650', '', '', 'a' => '00aD000015937' ); + my $field = MARC::Field->new( '650', '', '', 'a' => "00aD000015937" ); $record->append_fields($field); my ($biblionumber) = C4::Biblio::AddBiblio( $record, '' ); - warning_like { C4::Biblio::ModBiblio( $record, $biblionumber, '' ); } - qr/parser error : PCDATA invalid Char value 31/, - 'Modding the biblio warns about the encoding issues'; + 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/parser error : PCDATA invalid Char value 31/, - "Metadata issue successfully logged in action logs" - ); + like( $action_log->info, qr/biblionumber/, "Biblio data logged in action log info" ); }; subtest 'UpdateTotalIssues on Invalid record' => sub { -- 2.53.0