Bugzilla – Attachment 195010 Details for
Bug 40135
Record diff in action logs when modifying a bibliographic record
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 40135: Include MARC field changes in CATALOGUING diff logs
466d89b.patch (text/plain), 7.43 KB, created by
Martin Renvoize (ashimema)
on 2026-03-09 16:28:42 UTC
(
hide
)
Description:
Bug 40135: Include MARC field changes in CATALOGUING diff logs
Filename:
MIME Type:
Creator:
Martin Renvoize (ashimema)
Created:
2026-03-09 16:28:42 UTC
Size:
7.43 KB
patch
obsolete
>From 466d89b4543597039d951e99912884bce3a39c1f Mon Sep 17 00:00:00 2001 >From: Martin Renvoize <martin.renvoize@openfifth.co.uk> >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(-) > >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 >index 22965c2344f..72f51e5663a 100644 >--- 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 >
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 40135
:
194785
|
194941
|
195009
|
195010
|
195033
|
195034
|
195035