From 679c6718cdf3df47ab28a49bd779e79fff2d1e59 Mon Sep 17 00:00:00 2001 From: Julian Maurice Date: Tue, 23 Aug 2022 13:47:56 +0200 Subject: [PATCH] Bug 31441: Fix Koha::Item::as_marc_field when kohafield = '' marc_subfield_structure.kohafield can be NULL, but it can also be an empty string. But in that case, Koha::Item::as_marc_field ignores them, which means the resulting MARC::Field object has missing data. This can produce a bug in OPAC ISBD view (and probably other places where this method is used) Test plan: 1. Edit the default biblio MARC framework for the item field: find or create a subfield that is not linked to a DB column. Save even if you made no changes to make sure that marc_subfield_structure.kohafield is set to an empty string. I'll use 995$Z as an example for the following steps. 2. Add the following to syspref OPACISBD: #995|
Item:|{995Z}| 3. Create a biblio with an item and put a value into 995$Z 4. Go to the ISBD detail page for this record at OPAC. Confirm that the value you entered in 995$Z is not visible 5. Apply patch and restart koha 6. Refresh the ISBD detail page. Confirm that the 995$Z is now visible. 7. Run `prove t/db_dependent/Koha/Item.t` --- Koha/Item.pm | 2 +- t/db_dependent/Koha/Item.t | 15 ++++++++++++--- 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/Koha/Item.pm b/Koha/Item.pm index f25ae42c57..185de60fff 100644 --- a/Koha/Item.pm +++ b/Koha/Item.pm @@ -958,7 +958,7 @@ sub as_marc_field { my $kohafield = $subfield->{kohafield}; my $tagsubfield = $subfield->{tagsubfield}; my $value; - if ( defined $kohafield ) { + if ( defined $kohafield && $kohafield ne '' ) { next if $kohafield !~ m{^items\.}; # That would be weird! ( my $attribute = $kohafield ) =~ s|^items\.||; $value = $self->$attribute # This call may fail if a kohafield is not a DB column but we don't want to add extra work for that there diff --git a/t/db_dependent/Koha/Item.t b/t/db_dependent/Koha/Item.t index b274f0d688..23757741ce 100755 --- a/t/db_dependent/Koha/Item.t +++ b/t/db_dependent/Koha/Item.t @@ -310,7 +310,7 @@ subtest "as_marc_field() tests" => sub { my @schema_columns = $schema->resultset('Item')->result_source->columns; my @mapped_columns = grep { exists $mss->{'items.'.$_} } @schema_columns; - plan tests => 2 * (scalar @mapped_columns + 1) + 3; + plan tests => 2 * (scalar @mapped_columns + 1) + 4; $schema->storage->txn_begin; @@ -355,9 +355,17 @@ subtest "as_marc_field() tests" => sub { tagsubfield => 'X', } )->store; + Koha::MarcSubfieldStructure->new( + { + frameworkcode => '', + tagfield => $itemtag, + tagsubfield => 'Y', + kohafield => '', + } + )->store; my @unlinked_subfields; - push @unlinked_subfields, X => 'Something weird'; + push @unlinked_subfields, X => 'Something weird', Y => 'Something else'; $item->more_subfields_xml( C4::Items::_get_unlinked_subfields_xml( \@unlinked_subfields ) )->store; Koha::Caches->get_instance->clear_from_cache( "MarcStructure-1-" ); @@ -377,7 +385,8 @@ subtest "as_marc_field() tests" => sub { } @subfields; is_deeply(\@subfields, \@ordered_subfields); - is( scalar $marc_field->subfield('X'), 'Something weird', 'more_subfield_xml is considered' ); + is( scalar $marc_field->subfield('X'), 'Something weird', 'more_subfield_xml is considered when kohafield is NULL' ); + is( scalar $marc_field->subfield('Y'), 'Something else', 'more_subfield_xml is considered when kohafield = ""' ); $schema->storage->txn_rollback; Koha::Caches->get_instance->clear_from_cache( "MarcStructure-1-" ); -- 2.30.2