From 7fc5e472512234007a465141cba8b50c9c92f494 Mon Sep 17 00:00:00 2001
From: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Date: Mon, 30 Oct 2023 14:11:17 +0000
Subject: [PATCH] Bug 19097: Remove wantarray from GetMarcFromKohaField

The routine should be called in list context now.
Warns also about use of obsoleted framework parameter. This
is the case for several years already btw. But may help us
catch a forgotten occurrence? Can be removed later.

In a follow-up we will check for second parameters and
we will update calls in scalar context.

Test plan:
Prove t/db_dependent/Biblio.t

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
---
 C4/Biblio.pm            | 21 +++++++++++----------
 t/db_dependent/Biblio.t | 25 ++++++++++---------------
 2 files changed, 21 insertions(+), 25 deletions(-)

diff --git a/C4/Biblio.pm b/C4/Biblio.pm
index f4d8261384..833701c476 100644
--- a/C4/Biblio.pm
+++ b/C4/Biblio.pm
@@ -1192,24 +1192,25 @@ sub GetMarcSubfieldStructure {
 
 =head2 GetMarcFromKohaField
 
-    ( $field,$subfield ) = GetMarcFromKohaField( $kohafield );
-    @fields = GetMarcFromKohaField( $kohafield );
-    $field = GetMarcFromKohaField( $kohafield );
+    my ( $field, $subfield )    = GetMarcFromKohaField($kohafield);
+    my ( $f1, $sf1, $f2, $sf2 ) = GetMarcFromKohaField($kohafield);
 
-    Returns the MARC fields & subfields mapped to $kohafield.
+    Returns list of MARC fields and subfields mapped to $kohafield.
     Since the Default framework is considered as authoritative for such
     mappings, the former frameworkcode parameter is obsoleted.
 
-    In list context all mappings are returned; there can be multiple
-    mappings. Note that in the above example you could miss a second
-    mappings in the first call.
-    In scalar context only the field tag of the first mapping is returned.
+    NOTE: There may be multiple mappings! In the first example above
+    you could miss the second mapping (altough only a few of these
+    will normally exist).
+    Calling in scalar context has been deprecated as of 10/2023.
 
 =cut
 
 sub GetMarcFromKohaField {
-    my ( $kohafield ) = @_;
+    my ($kohafield) = @_;
+    warn "GetMarcFromKohaField: framework parameter has been obsoleted for long" if @_ > 1;    # TODO Remove later
     return unless $kohafield;
+
     # The next call uses the Default framework since it is AUTHORITATIVE
     # for all Koha to MARC mappings.
     my $mss = GetMarcSubfieldStructure( '', { unsafe => 1 } ); # Do not change framework
@@ -1217,7 +1218,7 @@ sub GetMarcFromKohaField {
     foreach( @{ $mss->{$kohafield} } ) {
         push @retval, $_->{tagfield}, $_->{tagsubfield};
     }
-    return wantarray ? @retval : ( @retval ? $retval[0] : undef );
+    return @retval;
 }
 
 =head2 GetMarcSubfieldStructureFromKohaField
diff --git a/t/db_dependent/Biblio.t b/t/db_dependent/Biblio.t
index 64326d4b99..7b6e09408e 100755
--- a/t/db_dependent/Biblio.t
+++ b/t/db_dependent/Biblio.t
@@ -176,9 +176,9 @@ subtest "GetMarcSubfieldStructure" => sub {
 };
 
 subtest "GetMarcFromKohaField" => sub {
-    plan tests => 8;
+    plan tests => 7;
 
-    #NOTE: We are building on data from the previous subtest
+    # NOTE: We are building on data from the previous subtest
     # With: field 399 / mytable.nicepages
 
     # Check call in list context for multiple mappings
@@ -190,19 +190,14 @@ subtest "GetMarcFromKohaField" => sub {
     is( $retval[3], 'b', 'Check second subfield' );
 
     # Check same call in scalar context
-    is( C4::Biblio::GetMarcFromKohaField('mytable.nicepages'), '399',
-        'GetMarcFromKohaField returns first tag in scalar context' );
-
-    # Bug 19096 Default is authoritative
-    # If we add a new empty framework, we should still get the mappings
-    # from Default. CAUTION: This test passes intentionally the obsoleted
-    # framework parameter.
-    my $new_fw = t::lib::TestBuilder->new->build({source => 'BiblioFramework'});
-    @retval = C4::Biblio::GetMarcFromKohaField(
-        'mytable.nicepages', $new_fw->{frameworkcode},
-    );
-    is( @retval, 4, 'Still got two pairs of tags/subfields' );
-    is( $retval[0].$retval[1], '399a', 'Including 399a' );
+    is( C4::Biblio::GetMarcFromKohaField('mytable.nicepages'), 4,
+        'GetMarcFromKohaField returns list count in scalar context' );
+
+    # Check for warning about obsoleted framework parameter
+    warning_like
+        { @retval = C4::Biblio::GetMarcFromKohaField( 'mytable.nicepages', 1 ) }
+        qr/obsoleted for long/,
+        'Found warning about obsoleted parameter';
 };
 
 subtest "Authority creation with default linker" => sub {
-- 
2.39.5