gitea
/
Koha-clms
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Bug 35104: Catch invalid metadata and surface repair warnings in AddBiblio/ModBiblio 

AddBiblio/ModBiblio/ModBiblioMarc gain an optional warnings arrayref
option: any repair messages recorded by Koha::Biblio::Metadata->store()
(see previous commit) are appended to it via object_messages, so a
caller can show the cataloguer what was automatically fixed instead of
the fix happening invisibly.

AddBiblio also now distinguishes Koha::Exceptions::Metadata::Invalid
from other errors in its catch block and rethrows it, so a caller can
tell "this record is unrecoverable" apart from an unexpected internal
error and react accordingly (see following bugs for callers doing so).

The inline StripNonXmlChars calls in TransformHtmlToMarc are removed,
since stripping is now handled centrally and consistently by
Koha::Biblio::Metadata->store() rather than ad-hoc at every call site.

Signed-off-by: Baptiste Bayche <baptiste.bayche@inlibro.com>
Signed-off-by: Baptiste Wojtkowski <baptiste.wojtkowski@biblibre.com>
Sponsored-by: OpenFifth
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
This commit is contained in:
Martin Renvoize 2026-07-08 12:40:23 +01:00 committed by Pedro Amorim
parent 112712e31c
commit 6726b1a254
No known key found for this signature in database
GPG Key ID: 06B75BA7F3894D55
2 changed files with 56 additions and 7 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

27
C4/Biblio.pm
View File

@ -89,7 +89,6 @@ use C4::Charset qw(
nsb_clean
SetMarcUnicodeFlag
SetUTF8Flag
StripNonXmlChars
);
use C4::Languages;
use C4::Linker;
@ -301,6 +300,7 @@ sub AddBiblio {
{
skip_record_index => 1,
record_source_id => $options->{record_source_id},
( ref $options->{warnings} eq 'ARRAY' ? ( warnings => $options->{warnings} ) : () ),
}
);
@ -330,8 +330,14 @@ sub AddBiblio {
$indexer->index_records( $biblionumber, "specialUpdate", "biblioserver" );
}
} catch {
warn $_;
( $biblionumber, $biblioitemnumber ) = ( undef, undef );
my $error = $_;
if ( ref($error) eq 'Koha::Exceptions::Metadata::Invalid' ) {
( $biblionumber, $biblioitemnumber ) = ( undef, undef );
$error->rethrow;
} else {
warn $error;
( $biblionumber, $biblioitemnumber ) = ( undef, undef );
}
};
return ( $biblionumber, $biblioitemnumber );
}
@ -399,7 +405,12 @@ sub ModBiblio {
( exists $options->{record_source_id} )
? ( record_source_id => $options->{record_source_id} )
: ()
)
),
(
( ref $options->{warnings} eq 'ARRAY' )
? ( warnings => $options->{warnings} )
: ()
),
};
if ( !$record ) {
@ -2352,7 +2363,6 @@ sub TransformHtmlToMarc {
# between 001 and 009 (included)
} elsif ( $fval ne '' ) {
$fval = StripNonXmlChars($fval); #Strip out any non-XML characters like control characters
$newfield = MARC::Field->new( $tag, $fval, );
}
@ -2377,7 +2387,6 @@ sub TransformHtmlToMarc {
if ( $fval ne '' && $newfield ) {
$newfield->add_subfields( $fkey => $fval );
} elsif ( $fval ne '' ) {
$fval = StripNonXmlChars($fval); #Strip out any non-XML characters like control characters
$newfield = MARC::Field->new( $tag, $ind1, $ind2, $fkey => $fval );
}
$j += 2;
@ -3030,6 +3039,12 @@ sub ModBiblioMarc {
}
)->store;
if ( ref $options->{warnings} eq 'ARRAY' ) {
for my $msg ( grep { $_->type eq 'warning' } @{ $m_rs->object_messages } ) {
push @{ $options->{warnings} }, { type => $msg->message, message => $msg->payload };
}
}
unless ($skip_record_index) {
my $indexer = Koha::SearchEngine::Indexer->new( { index => $Koha::SearchEngine::BIBLIOS_INDEX } );
$indexer->index_records( $biblionumber, "specialUpdate", "biblioserver" );

36
t/db_dependent/Biblio.t
View File

@ -17,7 +17,7 @@
use Modern::Perl;
use Test::More tests => 26;
use Test::More tests => 27;
use Test::NoWarnings;
use Test::MockModule;
use Test::Warn;
@ -1230,6 +1230,40 @@ subtest 'ModBiblio on invalid record' => sub {
like( $action_log->diff, qr/_marc/, "MARC field changes are captured in the diff column" );
};
subtest 'ModBiblio on record with strippable non-XML characters' => sub {
plan tests => 4;
t::lib::Mocks::mock_preference( "CataloguingLog", 1 );
# We create a record with an invalid control character (chr(31)) in the MARC
my $record = MARC::Record->new();
my $field = MARC::Field->new( '650', '', '', 'a' => '00' . chr(31) . 'aD000015937' );
$record->append_fields($field);
my ($biblionumber) = C4::Biblio::AddBiblio( $record, '' );
# New behaviour: strippable non-XML characters are silently fixed; the
# stripping event is reported via the warnings arrayref, not Perl warn()
my @warnings;
C4::Biblio::ModBiblio( $record, $biblionumber, '', { warnings => \@warnings } );
like(
$warnings[0]{message}, qr/650\$a: invalid char value 31 \(U\+001F\) at position \d+/,
'Non-XML character stripping reported via warnings arrayref with field and char context'
);
my $metadata = Koha::Biblios->find($biblionumber)->metadata;
my @nonxml_errors = $metadata->metadata_errors->search( { error_type => 'nonxml_stripped' } )->as_list;
ok( scalar @nonxml_errors, 'nonxml_stripped errors persisted in biblio_metadata_errors after ModBiblio' );
like(
$nonxml_errors[0]->message, qr/650\$a: invalid char value 31 \(U\+001F\) at position \d+/,
'error message identifies field, char value, and position'
);
my $action_logs =
Koha::ActionLogs->search( { object => $biblionumber, module => 'Cataloguing', action => 'MODIFY' } );
is( $action_logs->count, 1, "Modification of biblio was successfully recorded in action logs" );
};
subtest 'UpdateTotalIssues on Invalid record' => sub {
plan tests => 2;