Bug 42188: Add/update unit tests for MARC-in-JSON action log diffs

Update the existing _unblessed_for_log() subtest to reflect the new
behaviour: MARC-derived columns (title, author, copyrightdate, etc.)
are now excluded, while frameworkcode is always included even when it
is an empty string (the Default framework).

Add a new subtest 'CataloguingLog MARC-in-JSON diff tests' covering:
- ADD:    diff _marc key is MARC-in-JSON (has leader + fields array),
          indicators and subfield values are correct, and MARC-derived
          biblio columns are absent from the diff.
- MODIFY: diff _marc key present, MARC-derived columns absent.
- DELETE: diff _marc key present with correct MARC-in-JSON structure.

Use from_json() instead of decode_json() when reading diffs back from
the database, as the DBD layer returns decoded Perl Unicode strings
which decode_json (which expects UTF-8 bytes) would reject for any
non-ASCII content.

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
This commit is contained in:
Martin Renvoize 2026-03-26 12:12:05 +01:00 committed by Lucas Gass
parent 873fcbb51f
commit 3d78340552
No known key found for this signature in database
GPG Key ID: 60B447398B0D36CA
2 changed files with 82 additions and 32 deletions

View File

@ -2425,8 +2425,7 @@ sub _unblessed_for_log {
my $full = $self->unblessed;
my %data;
for my $key (@_LOG_FIELDS) {
next unless exists $full->{$key};
next unless defined $full->{$key} && $full->{$key} ne '';
next unless exists $full->{$key} && defined $full->{$key};
$data{$key} = blessed( $full->{$key} ) ? "$full->{$key}" : $full->{$key};
}
return \%data;

View File

@ -18,7 +18,7 @@
use Modern::Perl;
use Test::NoWarnings;
use Test::More tests => 42;
use Test::More tests => 43;
use Test::Exception;
use Test::Warn;
@ -37,7 +37,7 @@ use Koha::MarcSubfieldStructures;
use Koha::ActionLogs;
use Koha::Exception;
use JSON qw( decode_json );
use JSON qw( decode_json from_json );
use MARC::Field;
use MARC::Record;
@ -2447,42 +2447,93 @@ subtest 'check_booking tests' => sub {
subtest '_unblessed_for_log() tests' => sub {
plan tests => 10;
plan tests => 8;
$schema->storage->txn_begin;
# Build a biblio where several nullable columns are undef
my $biblio = $builder->build_sample_biblio( { author => 'Test Author', title => 'Test Title' } );
# Force some nullable columns to undef/empty
$biblio->set( { abstract => undef, notes => '', seriestitle => undef } )->store;
$biblio->discard_changes;
my $data = $biblio->_unblessed_for_log;
ok( ref $data eq 'HASH', '_unblessed_for_log returns a hashref' );
ok( exists $data->{biblionumber}, 'biblionumber is present' );
ok( exists $data->{title}, 'title (set value) is present' );
ok( exists $data->{author}, 'author (set value) is present' );
ok( !exists $data->{timestamp}, 'timestamp is excluded' );
ok( !exists $data->{abstract}, 'undef field (abstract) is excluded' );
ok( !exists $data->{notes}, 'empty-string field (notes) is excluded' );
ok( !exists $data->{seriestitle}, 'undef field (seriestitle) is excluded' );
ok( ref $data eq 'HASH', '_unblessed_for_log returns a hashref' );
t::lib::Mocks::mock_preference( 'CataloguingLog', 1 );
# ADD log: diff should not contain timestamp
my $record = MARC::Record->new();
$record->append_fields( MARC::Field->new( '245', '1', '0', 'a' => 'Log Test Title' ) );
my ($new_biblionumber) = AddBiblio( $record, '' );
my $add_log =
Koha::ActionLogs->search( { object => $new_biblionumber, module => 'Cataloguing', action => 'ADD' } )->next;
ok( defined $add_log->diff, 'ADD log populates the diff column' );
my $add_diff = decode_json( $add_log->diff );
my $diff_keys = exists $add_diff->{D} ? $add_diff->{D} : {};
ok( !exists $diff_keys->{timestamp}, 'timestamp not present in ADD diff' );
# Only non-MARC-derived Koha metadata fields should be present
ok( exists $data->{frameworkcode}, 'frameworkcode (Koha metadata) is present' );
ok( !exists $data->{biblionumber}, 'biblionumber is excluded (internal ID)' );
ok( !exists $data->{title}, 'title is excluded (MARC-derived)' );
ok( !exists $data->{author}, 'author is excluded (MARC-derived)' );
ok( !exists $data->{timestamp}, 'timestamp is excluded' );
ok( !exists $data->{copyrightdate}, 'copyrightdate is excluded (MARC-derived)' );
ok( !exists $data->{abstract}, 'abstract is excluded (MARC-derived)' );
$schema->storage->txn_rollback;
};
subtest 'CataloguingLog MARC-in-JSON diff tests' => sub {
plan tests => 18;
$schema->storage->txn_begin;
t::lib::Mocks::mock_preference( 'CataloguingLog', 1 );
# --- ADD ---
my $record = MARC::Record->new();
$record->leader('00000cam a2200000 a 4500');
$record->append_fields(
MARC::Field->new( '245', '1', '0', a => 'Test Title', b => 'A Subtitle' ),
MARC::Field->new( '100', '1', ' ', a => 'Test Author' ),
);
my ($biblionumber) = AddBiblio( $record, '' );
my $add_log =
Koha::ActionLogs->search( { object => $biblionumber, module => 'Cataloguing', action => 'ADD' } )->next;
ok( defined $add_log->diff, 'ADD log populates the diff column' );
my $add_diff = from_json( $add_log->diff );
my $add_added = $add_diff->{D}{_marc}{A};
ok( defined $add_added, 'ADD diff contains _marc key' );
ok( exists $add_added->{leader}, '_marc has leader' );
ok( ref $add_added->{fields} eq 'ARRAY', '_marc.fields is an array' );
my ($f245) = grep { exists $_->{'245'} } @{ $add_added->{fields} };
ok( defined $f245, '245 field present in _marc.fields' );
is( $f245->{'245'}{ind1}, '1', '245 ind1 correct' );
is( $f245->{'245'}{subfields}[0]{a}, 'Test Title', '245 $a correct' );
my ($f100) = grep { exists $_->{'100'} } @{ $add_added->{fields} };
ok( defined $f100, '100 field present in _marc.fields' );
ok( !exists $add_diff->{D}{title}, 'MARC-derived title column absent from diff' );
ok( !exists $add_diff->{D}{author}, 'MARC-derived author column absent from diff' );
# --- MODIFY ---
my $mod_record = MARC::Record->new();
$mod_record->leader('00000cam a2200000 a 4500');
$mod_record->append_fields( MARC::Field->new( '245', '1', '0', a => 'Changed Title' ) );
ModBiblio( $mod_record, $biblionumber, '' );
my $mod_log =
Koha::ActionLogs->search( { object => $biblionumber, module => 'Cataloguing', action => 'MODIFY' } )->next;
ok( defined $mod_log->diff, 'MODIFY log populates the diff column' );
my $mod_diff = from_json( $mod_log->diff );
ok( exists $mod_diff->{D}{_marc}, 'MODIFY diff contains _marc key' );
ok( !exists $mod_diff->{D}{title}, 'MARC-derived title column absent from MODIFY diff' );
ok( !exists $mod_diff->{D}{author}, 'MARC-derived author column absent from MODIFY diff' );
# --- DELETE ---
DelBiblio( $biblionumber, { skip_record_index => 1 } );
my $del_log =
Koha::ActionLogs->search( { object => $biblionumber, module => 'Cataloguing', action => 'DELETE' } )->next;
ok( defined $del_log->diff, 'DELETE log populates the diff column' );
my $del_diff = from_json( $del_log->diff );
my $del_removed = $del_diff->{D}{_marc}{R};
ok( defined $del_removed, 'DELETE diff contains _marc key' );
ok( exists $del_removed->{leader}, '_marc has leader in DELETE diff' );
ok( ref $del_removed->{fields} eq 'ARRAY', '_marc.fields is an array in DELETE diff' );
$schema->storage->txn_rollback;
};