From b60975c33c0e0a2e79b3a3af52753ea6c37444e4 Mon Sep 17 00:00:00 2001 From: Marcel de Rooy Date: Thu, 16 Oct 2025 08:47:24 +0200 Subject: [PATCH] Bug 40272: (QA follow-up) Fix new biblio/empty metadata case Content-Type: text/plain; charset=utf-8 Test plan: Add new biblio record in basic editor. Run t/db_dependent/Koha/Biblio/Metadata/Extractor/MARC/MARC21.t Signed-off-by: Marcel de Rooy --- Koha/Biblio/Metadata/Extractor/MARC/MARC21.pm | 4 ++++ cataloguing/addbiblio.pl | 9 ++++----- .../Koha/Biblio/Metadata/Extractor/MARC/MARC21.t | 16 +++++++++++++--- 3 files changed, 21 insertions(+), 8 deletions(-) diff --git a/Koha/Biblio/Metadata/Extractor/MARC/MARC21.pm b/Koha/Biblio/Metadata/Extractor/MARC/MARC21.pm index 42cd8ec523..f7c226a3fd 100644 --- a/Koha/Biblio/Metadata/Extractor/MARC/MARC21.pm +++ b/Koha/Biblio/Metadata/Extractor/MARC/MARC21.pm @@ -129,6 +129,10 @@ sub check_fixed_length { '008' => 40, }; my $record = $self->metadata; + if ( ref($record) ne 'MARC::Record' ) { # TODO Move to Extractor::MARC->metadata? + warn "MARC extractor: metadata is not a MARC::Record object\n"; + return; + } my $result = { passed => [], failed => [] }; foreach my $field ( $record->field( '005', '006', '007', '008' ) ) { my ( $length, $pass ); diff --git a/cataloguing/addbiblio.pl b/cataloguing/addbiblio.pl index d7ec674e6e..cad8309e0a 100755 --- a/cataloguing/addbiblio.pl +++ b/cataloguing/addbiblio.pl @@ -831,16 +831,15 @@ if ( $op eq "cud-addbiblio" ) { } else { - #---------------------------------------------------------------------------- - # If we're in a duplication case, we have to set to "" the biblionumber - # as we'll save the biblio as a new one. - - if ( C4::Context->preference('marcflavour') eq 'MARC21' ) { + # Check fixed length fields for existing biblio + if ( $biblio && C4::Context->preference('marcflavour') eq 'MARC21' ) { my $fixed_length_info = Koha::Biblio::Metadata::Extractor::MARC::MARC21->new( { biblio => $biblio } )->check_fixed_length; $template->param( marc21_fixlen => $fixed_length_info ) if @{ $fixed_length_info->{failed} }; } + # If we're in a duplication case, we have to set to "" the biblionumber + # as we'll save the biblio as a new one. $template->param( biblionumberdata => $biblionumber, op => $op, diff --git a/t/db_dependent/Koha/Biblio/Metadata/Extractor/MARC/MARC21.t b/t/db_dependent/Koha/Biblio/Metadata/Extractor/MARC/MARC21.t index 6797cb5ed5..e25decd9b5 100755 --- a/t/db_dependent/Koha/Biblio/Metadata/Extractor/MARC/MARC21.t +++ b/t/db_dependent/Koha/Biblio/Metadata/Extractor/MARC/MARC21.t @@ -22,6 +22,7 @@ use Modern::Perl; use Test::NoWarnings; use Test::More tests => 5; use Test::Exception; +use Test::Warn; use t::lib::TestBuilder; use t::lib::Mocks; @@ -92,19 +93,28 @@ subtest 'get_normalized_oclc() tests' => sub { subtest 'check_fixed_length' => sub { - plan tests => 6; + plan tests => 9; $schema->storage->txn_begin; + # Check empty string (no valid metadata) + my $extractor = Koha::Biblio::Metadata::Extractor::MARC::MARC21->new( { metadata => q{} } ); + warning_like { $extractor->check_fixed_length } qr/not a MARC::Record object/, 'Extractor should warn'; + + # Check empty object my $record = MARC::Record->new; + $extractor = Koha::Biblio::Metadata::Extractor::MARC::MARC21->new( { metadata => $record } ); + my $result = $extractor->check_fixed_length; + is( scalar @{ $result->{passed} }, 0, 'No passed fields' ); + is( scalar @{ $result->{failed} }, 0, 'No failed fields' ); + $record->append_fields( MARC::Field->new( '005', '0123456789012345' ), ); my $biblio = $builder->build_sample_biblio; ModBiblio( $record, $biblio->biblionumber ); - my $extractor; $extractor = Koha::Biblio::Metadata::Extractor::MARC::MARC21->new( { biblio => $biblio } ); - my $result = $extractor->check_fixed_length; + $result = $extractor->check_fixed_length; is( $result->{passed}->[0], '005', 'Check first passed field' ); is( scalar @{ $result->{failed} }, 0, 'Check failed count' ); -- 2.39.5