From 2237e0f871fa1bcf0009346022fa854aedf0b7f8 Mon Sep 17 00:00:00 2001 From: Jonathan Druart Date: Fri, 12 Feb 2016 12:36:16 +0000 Subject: [PATCH] Bug 5404: C4::Koha - remove subfield_is_koha_internal_p The commit b5ecefd485a75d54a5fa26fff5a0cc890541e2c3 Date: Mon Feb 3 18:46:00 2003 +0000 had a funny description: Added function to check if a MARC subfield name is "koha-internal" (instead of checking it for 'lib' and 'tag' everywhere); temporarily added to Koha.pm "Temporarily", since 2003, everything is relative, isn't it? :) The thing is that GetMarcStructure returns hash like field_200 => { subfield_a => { %attributes_of_subfield_a }, %attributes_of_field_200 } The attributes for field_200 can be 'repeatable', 'mandatory', 'tag', 'lib'. We don't want to loop on these values when looping on subfields. Since there are just { k => v } with v is a scalar (string), it's easier to test if we are processing a subfield testing the reference. At some places, we don't need to test that, we are looping on values from MARC::Field->subfields which are always valid subfields. Test plan: 1/ Edit items using the batch item mod tool 2/ display and edit items via the cataloguing module. You should not see any changes between before and after the patch applied. Tech notes: We need to check what we are processing when we loop on 'subfields' from GetMarcStructure, not from MARC::Field->subfields. Signed-off-by: Josef Moravec Signed-off-by: Marcel de Rooy Signed-off-by: Brendan A Gallagher --- C4/Acquisition.pm | 4 ++-- C4/Items.pm | 2 +- C4/Koha.pm | 12 ------------ authorities/authorities-home.pl | 2 +- authorities/authorities.pl | 2 +- cataloguing/addbiblio.pl | 4 ++-- cataloguing/additem.pl | 9 ++++----- labels/label-item-search.pl | 2 +- opac/opac-authorities-home.pl | 2 +- reports/guided_reports.pl | 2 +- t/db_dependent/Acquisition/FillWithDefaultValues.t | 8 ++++++-- tools/batchMod.pl | 6 +++--- 12 files changed, 23 insertions(+), 32 deletions(-) diff --git a/C4/Acquisition.pm b/C4/Acquisition.pm index 4a83cb908f..5dc9cb3f05 100644 --- a/C4/Acquisition.pm +++ b/C4/Acquisition.pm @@ -33,7 +33,7 @@ use Koha::Acquisition::Bookseller; use Koha::Number::Price; use Koha::Libraries; -use C4::Koha qw( subfield_is_koha_internal_p ); +use C4::Koha; use MARC::Field; use MARC::Record; @@ -2996,7 +2996,7 @@ sub FillWithDefaultValues { next unless $tag; next if $tag == $itemfield; for my $subfield ( sort keys %{ $tagslib->{$tag} } ) { - next if ( subfield_is_koha_internal_p($subfield) ); + next unless ref $tagslib->{$tag}{$subfield}; # Not a valid subfield (mandatory, tab, lib) my $defaultvalue = $tagslib->{$tag}{$subfield}{defaultvalue}; if ( defined $defaultvalue and $defaultvalue ne '' ) { my @fields = $record->field($tag); diff --git a/C4/Items.pm b/C4/Items.pm index 51479ecfeb..d23fb7fb1a 100644 --- a/C4/Items.pm +++ b/C4/Items.pm @@ -2943,7 +2943,7 @@ sub PrepareItemrecordDisplay { # loop through each subfield my $cntsubf; foreach my $subfield ( sort keys %{ $tagslib->{$tag} } ) { - next if ( subfield_is_koha_internal_p($subfield) ); + next unless ref $tagslib->{$tag}{$subfield}; # Not a valid subfield (mandatory, tab, lib) next if ( $tagslib->{$tag}->{$subfield}->{'tab'} ne "10" ); my %subfield_data; $subfield_data{tag} = $tag; diff --git a/C4/Koha.pm b/C4/Koha.pm index f334096801..6981ebf8da 100644 --- a/C4/Koha.pm +++ b/C4/Koha.pm @@ -39,7 +39,6 @@ BEGIN { require Exporter; @ISA = qw(Exporter); @EXPORT = qw( - &subfield_is_koha_internal_p &GetPrinters &GetPrinter &GetItemTypes &getitemtypeinfo &GetItemTypesCategorized &GetItemTypesByCategory @@ -94,17 +93,6 @@ Koha.pm provides many functions for Koha scripts. =cut -# FIXME.. this should be moved to a MARC-specific module -sub subfield_is_koha_internal_p { - my ($subfield) = @_; - - # We could match on 'lib' and 'tab' (and 'mandatory', & more to come!) - # But real MARC subfields are always single-character - # so it really is safer just to check the length - - return length $subfield != 1; -} - =head2 GetSupportName $itemtypename = &GetSupportName($codestring); diff --git a/authorities/authorities-home.pl b/authorities/authorities-home.pl index 9e80489903..b0c4570306 100755 --- a/authorities/authorities-home.pl +++ b/authorities/authorities-home.pl @@ -29,7 +29,7 @@ use C4::Auth; use C4::Output; use C4::AuthoritiesMarc; use C4::Acquisition; -use C4::Koha; # XXX subfield_is_koha_internal_p +use C4::Koha; use C4::Biblio; use C4::Search::History; diff --git a/authorities/authorities.pl b/authorities/authorities.pl index ca3b0d37d7..bc2c8d415e 100755 --- a/authorities/authorities.pl +++ b/authorities/authorities.pl @@ -26,7 +26,7 @@ use C4::Output; use C4::AuthoritiesMarc; use C4::ImportBatch; #GetImportRecordMarc use C4::Context; -use C4::Koha; # XXX subfield_is_koha_internal_p +use C4::Koha; use Date::Calc qw(Today); use MARC::File::USMARC; use MARC::File::XML; diff --git a/cataloguing/addbiblio.pl b/cataloguing/addbiblio.pl index e34557c4ae..44d5907b2f 100755 --- a/cataloguing/addbiblio.pl +++ b/cataloguing/addbiblio.pl @@ -30,8 +30,8 @@ use C4::AuthoritiesMarc; use C4::Context; use MARC::Record; use C4::Log; -use C4::Koha; # XXX subfield_is_koha_internal_p -use C4::Branch; # XXX subfield_is_koha_internal_p +use C4::Koha; +use C4::Branch; use C4::ClassSource; use C4::ImportBatch; use C4::Charset; diff --git a/cataloguing/additem.pl b/cataloguing/additem.pl index db099843a6..7a4031dd31 100755 --- a/cataloguing/additem.pl +++ b/cataloguing/additem.pl @@ -28,8 +28,8 @@ use C4::Biblio; use C4::Items; use C4::Context; use C4::Circulation; -use C4::Koha; # XXX subfield_is_koha_internal_p -use C4::Branch; # XXX subfield_is_koha_internal_p +use C4::Koha; +use C4::Branch; use C4::ClassSource; use Koha::DateUtils; use List::MoreUtils qw/any/; @@ -606,7 +606,7 @@ if ($op eq "additem") { my $tag = $field->{_tag}; foreach my $subfield ($field->subfields()){ my $subfieldtag = $subfield->[0]; - if (subfield_is_koha_internal_p($subfieldtag) || $tagslib->{$tag}->{$subfieldtag}->{'tab'} ne "10" + if ($tagslib->{$tag}->{$subfieldtag}->{'tab'} ne "10" || abs($tagslib->{$tag}->{$subfieldtag}->{hidden})>4 ){ my $fieldItem = $itemrecord->field($tag); $itemrecord->delete_field($fieldItem); @@ -871,7 +871,6 @@ if($itemrecord){ my $value = $subfield->[1]; my $subfieldlib = $tagslib->{$tag}->{$subfieldtag}; - next if subfield_is_koha_internal_p($subfieldtag); next if ($tagslib->{$tag}->{$subfieldtag}->{'tab'} ne "10"); my $subfield_data = generate_subfield_form($tag, $subfieldtag, $value, $tagslib, $subfieldlib, $branches, $biblionumber, $temp, \@loop_data, $i, $restrictededition); @@ -891,7 +890,7 @@ $itemrecord = $cookieitemrecord if ($prefillitem and not $justaddeditem and $op # We generate form, and fill with values if defined foreach my $tag ( keys %{$tagslib}){ foreach my $subtag (keys %{$tagslib->{$tag}}){ - next if subfield_is_koha_internal_p($subtag); + next unless ref $tagslib->{$tag}{$subtag}; # Not a valid subfield (mandatory, tab, lib) next if ($tagslib->{$tag}->{$subtag}->{'tab'} ne "10"); next if any { /^$tag$subtag$/ } @fields; diff --git a/labels/label-item-search.pl b/labels/label-item-search.pl index 8b72cb4ea2..d1500a4ac1 100755 --- a/labels/label-item-search.pl +++ b/labels/label-item-search.pl @@ -31,7 +31,7 @@ use C4::Context; use C4::Search qw(SimpleSearch); use C4::Biblio qw(TransformMarcToKoha); use C4::Items qw(GetItemInfosOf get_itemnumbers_of); -use C4::Koha qw(GetItemTypes); # XXX subfield_is_koha_internal_p +use C4::Koha qw(GetItemTypes); use C4::Creators::Lib qw(html_table); use C4::Debug; use Koha::DateUtils; diff --git a/opac/opac-authorities-home.pl b/opac/opac-authorities-home.pl index 336a5fa136..f4ab2ab222 100755 --- a/opac/opac-authorities-home.pl +++ b/opac/opac-authorities-home.pl @@ -28,7 +28,7 @@ use C4::Context; use C4::Auth; use C4::Output; use C4::AuthoritiesMarc; -use C4::Koha; # XXX subfield_is_koha_internal_p +use C4::Koha; use C4::Search::History; use Koha::Authority::Types; diff --git a/reports/guided_reports.pl b/reports/guided_reports.pl index 200260d2e4..4b45f95325 100755 --- a/reports/guided_reports.pl +++ b/reports/guided_reports.pl @@ -28,8 +28,8 @@ use C4::Reports::Guided; use C4::Auth qw/:DEFAULT get_session/; use C4::Output; use C4::Debug; -use C4::Branch; # XXX subfield_is_koha_internal_p use C4::Koha qw/GetFrameworksLoop/; +use C4::Branch; use C4::Context; use C4::Log; use Koha::DateUtils qw/dt_from_string output_pref/; diff --git a/t/db_dependent/Acquisition/FillWithDefaultValues.t b/t/db_dependent/Acquisition/FillWithDefaultValues.t index 758fa692e7..531aa78704 100755 --- a/t/db_dependent/Acquisition/FillWithDefaultValues.t +++ b/t/db_dependent/Acquisition/FillWithDefaultValues.t @@ -21,8 +21,12 @@ $biblio_module->mock( { # default value for an existing field '245' => { - c => { defaultvalue => $default_author }, - }, + c => { defaultvalue => $default_author }, + mandatory => 0, + repeatable => 0, + tab => 0, + lib => 'a lib', + }, # default for a nonexisting field '099' => { diff --git a/tools/batchMod.pl b/tools/batchMod.pl index 56747177a9..d62ef6db6b 100755 --- a/tools/batchMod.pl +++ b/tools/batchMod.pl @@ -27,8 +27,8 @@ use C4::Biblio; use C4::Items; use C4::Circulation; use C4::Context; -use C4::Koha; # XXX subfield_is_koha_internal_p -use C4::Branch; # XXX subfield_is_koha_internal_p +use C4::Koha; +use C4::Branch; use C4::BackgroundJob; use C4::ClassSource; use C4::Debug; @@ -308,7 +308,7 @@ my @subfieldsToAllow = split(/ /, $subfieldsToAllowForBatchmod); foreach my $tag (sort keys %{$tagslib}) { # loop through each subfield foreach my $subfield (sort keys %{$tagslib->{$tag}}) { - next if subfield_is_koha_internal_p($subfield); + next unless ref $tagslib->{$tag}{$subfield}; # Not a valid subfield (mandatory, tab, lib) next if (not $allowAllSubfields and $restrictededition && !grep { $tag . '$' . $subfield eq $_ } @subfieldsToAllow ); next if ($tagslib->{$tag}->{$subfield}->{'tab'} ne "10"); # barcode and stocknumber are not meant to be batch-modified -- 2.20.1