From 4492b2acd7868e1236622dc394820260ed9d7d87 Mon Sep 17 00:00:00 2001
From: Julian Maurice <julian.maurice@biblibre.com>
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|<br>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`

Signed-off-by: David Nind <david@davidnind.com>
---
 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 d2f337772d..610315ac11 100644
--- a/Koha/Item.pm
+++ b/Koha/Item.pm
@@ -957,7 +957,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