From fa598383c4d0399a5ff5ec11d22641ffb2b9cf8d Mon Sep 17 00:00:00 2001 From: Marcel de Rooy Date: Mon, 30 Oct 2023 13:20:14 +0000 Subject: [PATCH] Bug 19097: Remove wantarray from GetMarcSubfieldStructureFromKohaField Content-Type: text/plain; charset=utf-8 Replacing wantarray by always returning all mappings. In a few cases only we expect multiple ones. Changing two calls to pick the first hit, and add comment about the implicit assumption being made (as before, no behavior change). Test plan: Look at results of git grep GetMarcSubfieldStructureFromKohaField Run t/db_dependent/Biblio.t Signed-off-by: Marcel de Rooy --- C4/Biblio.pm | 13 ++++++++----- reports/acquisitions_stats.pl | 2 +- reports/catalogue_stats.pl | 2 +- t/db_dependent/Biblio.t | 27 ++++++++++++++------------- 4 files changed, 24 insertions(+), 20 deletions(-) diff --git a/C4/Biblio.pm b/C4/Biblio.pm index eea56d7e4c..7328bf9e99 100644 --- a/C4/Biblio.pm +++ b/C4/Biblio.pm @@ -1191,13 +1191,17 @@ sub GetMarcFromKohaField { =head2 GetMarcSubfieldStructureFromKohaField - my $str = GetMarcSubfieldStructureFromKohaField( $kohafield ); + my $arrayref = GetMarcSubfieldStructureFromKohaField($kohafield); + my $hashref = GetMarcSubfieldStructureFromKohaField($kohafield)->[0]; Returns marc subfield structure information for $kohafield. The Default framework is used, since it is authoritative for kohafield mappings. - In list context returns a list of all hashrefs, since there may be - multiple mappings. In scalar context the first hashref is returned. + + Since there MAY be multiple mappings (not that often), you receive an + arrayref of all mappings found. In the second example above the first + one is picked only. If there are no mappings, you get an empty arrayref + (so in the call above $hashref will be undefined - without warnings). =cut @@ -1209,8 +1213,7 @@ sub GetMarcSubfieldStructureFromKohaField { # 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 - return unless $mss->{$kohafield}; - return wantarray ? @{$mss->{$kohafield}} : $mss->{$kohafield}->[0]; + return $mss->{$kohafield} // []; } =head2 GetXmlBiblio diff --git a/reports/acquisitions_stats.pl b/reports/acquisitions_stats.pl index 2c90fe32cc..ab77472bc2 100755 --- a/reports/acquisitions_stats.pl +++ b/reports/acquisitions_stats.pl @@ -173,7 +173,7 @@ else { my $libraries = Koha::Libraries->search({}, { order_by => 'branchname' }); - my $ccode_subfield_structure = GetMarcSubfieldStructureFromKohaField('items.ccode'); + my $ccode_subfield_structure = GetMarcSubfieldStructureFromKohaField('items.ccode')->[0]; # assuming one result.. my $ccode_label; my $ccode_avlist; if($ccode_subfield_structure) { diff --git a/reports/catalogue_stats.pl b/reports/catalogue_stats.pl index b4d72a384b..dd9a773d42 100755 --- a/reports/catalogue_stats.pl +++ b/reports/catalogue_stats.pl @@ -118,7 +118,7 @@ if ($do_it) { my @locations = map { { code => $_->{authorised_value}, description => $_->{lib} } } Koha::AuthorisedValues->get_descriptions_by_koha_field( { frameworkcode => '', kohafield => 'items.location' }, { order_by => ['description'] } ); foreach my $kohafield (qw(items.notforloan items.materials)) { - my $subfield_structure = GetMarcSubfieldStructureFromKohaField($kohafield); + my $subfield_structure = GetMarcSubfieldStructureFromKohaField($kohafield)->[0]; # assuming one result.. if($subfield_structure) { my $avlist; my $avcategory = $subfield_structure->{authorised_value}; diff --git a/t/db_dependent/Biblio.t b/t/db_dependent/Biblio.t index 4f096f7fe1..3af420a17c 100755 --- a/t/db_dependent/Biblio.t +++ b/t/db_dependent/Biblio.t @@ -102,7 +102,7 @@ subtest 'AddBiblio' => sub { }; subtest 'GetMarcSubfieldStructureFromKohaField' => sub { - plan tests => 25; + plan tests => 27; my @columns = qw( tagfield tagsubfield liblibrarian libopac repeatable mandatory kohafield tab @@ -113,23 +113,24 @@ subtest 'GetMarcSubfieldStructureFromKohaField' => sub { # biblio.biblionumber must be mapped so this should return something my $marc_subfield_structure = GetMarcSubfieldStructureFromKohaField('biblio.biblionumber'); - ok(defined $marc_subfield_structure, "There is a result"); - is(ref $marc_subfield_structure, "HASH", "Result is a hashref"); + is( ref $marc_subfield_structure, "ARRAY", "Result is an arrayref" ); + is( @$marc_subfield_structure, 1, 'Expecting one hit only' ); foreach my $col (@columns) { - ok(exists $marc_subfield_structure->{$col}, "Hashref contains key '$col'"); + ok( exists $marc_subfield_structure->[0]->{$col}, "Hashref contains key '$col'" ); } - is($marc_subfield_structure->{kohafield}, 'biblio.biblionumber', "Result is the good result"); - like($marc_subfield_structure->{tagfield}, qr/^\d{3}$/, "tagfield is a valid tagfield"); + is( $marc_subfield_structure->[0]->{kohafield}, 'biblio.biblionumber', "Result is the good result" ); + like( $marc_subfield_structure->[0]->{tagfield}, qr/^\d{3}$/, "tagfield is a valid tagfield" ); - # Add a test for list context (BZ 10306) - my @results = GetMarcSubfieldStructureFromKohaField('biblio.biblionumber'); - is( @results, 1, 'We expect only one mapping' ); - is_deeply( $results[0], $marc_subfield_structure, - 'The first entry should be the same hashref as we had before' ); + # Multiple mappings expected for kohafield biblio.copyrightdate (in 260, 264) + $marc_subfield_structure = GetMarcSubfieldStructureFromKohaField('biblio.copyrightdate'); + is( ref $marc_subfield_structure, "ARRAY", "Result is again an arrayref" ); + is( @$marc_subfield_structure, 2, 'We expect two hits' ); + ok( exists $marc_subfield_structure->[0]->{tagsubfield}, 'Testing a random column for existence in 1st hash' ); + ok( exists $marc_subfield_structure->[1]->{hidden}, 'Testing a random column for existence in 2nd hash' ); - # foo.bar does not exist so this should return undef + # foo.bar does not exist so this should return [] $marc_subfield_structure = GetMarcSubfieldStructureFromKohaField('foo.bar'); - is($marc_subfield_structure, undef, "invalid kohafield returns undef"); + is_deeply( $marc_subfield_structure, [], "invalid kohafield returns empty arrayref" ); }; -- 2.30.2