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 <josef.moravec@gmail.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>

diff --git a/C4/Acquisition.pm b/C4/Acquisition.pm
index 4a83cb9..5dc9cb3 100644 (file)
--- 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 51479ec..d23fb7f 100644 (file)
--- 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 f334096..6981ebf 100644 (file)
--- 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 9e80489..b0c4570 100755 (executable)
--- 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 ca3b0d3..bc2c8d4 100755 (executable)
--- 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 e34557c..44d5907 100755 (executable)
--- 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 db09984..7a4031d 100755 (executable)
--- 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 8b72cb4..d1500a4 100755 (executable)
--- 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 336a5fa..f4ab2ab 100755 (executable)
--- 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 200260d..4b45f95 100755 (executable)
--- 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 758fa69..531aa78 100755 (executable)
--- 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 5674717..d62ef6d 100755 (executable)
--- 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