Bugzilla – Attachment 158231 Details for
Bug 19097
Koha to MARC mappings (Part 3): Correct remaining GetMarcFromKohaField calls
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 19097: Remove wantarray from GetMarcSubfieldStructureFromKohaField
Bug-19097-Remove-wantarray-from-GetMarcSubfieldStr.patch (text/plain), 6.53 KB, created by
Marcel de Rooy
on 2023-11-02 07:44:20 UTC
(
hide
)
Description:
Bug 19097: Remove wantarray from GetMarcSubfieldStructureFromKohaField
Filename:
MIME Type:
Creator:
Marcel de Rooy
Created:
2023-11-02 07:44:20 UTC
Size:
6.53 KB
patch
obsolete
>From fa598383c4d0399a5ff5ec11d22641ffb2b9cf8d Mon Sep 17 00:00:00 2001 >From: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> >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 <m.de.rooy@rijksmuseum.nl> >--- > 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
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 19097
:
158063
|
158231
|
158232
|
158233
|
158246
|
158247
|
158248
|
158249
|
165103
|
165104
|
165105
|
165106