From 0625f3bc95c4714c740ae6d4ce04382808659eb5 Mon Sep 17 00:00:00 2001 From: Marcel de Rooy Date: Thu, 10 Aug 2017 10:59:07 +0200 Subject: [PATCH] Bug 19096: Make Default authoritative in core modules After feedback from the dev mailing list, it seems appropriate here to propose making the Default framework authoritative for Koha to MARC mappings. This implies checking only the Default framework in the routines: [1] GetMarcFromKohaField: The parameter frameworkcode is removed. A follow-up report (19097) will update the calls not adjusted here. This is safe since the parameter is silently ignored. [2] GetMarcSubfieldStructureFromKohaField: Framework parameter is removed and calls are adjusted. Includes acquisitions_stats.pl. [3] TransformKohaToMarc: The parameter is removed; all calls are verified or adjusted. [4] TransformMarcToKoha: The parameter is no longer used and will be removed in a follow-up report (19097). It always goes to Default now. [5] TransformMarcToKohaOneField: The parameter is removed and all calls are adjusted. Including: Breeding, XISBN and MetadataRecord modules. [6] C4::Koha::IsKohaFieldLinked: This routine was called only once (in C4::Items::_build_default_values_for_mod_marc. It can be replaced by calling GetMarcFromKohaField. If there is no kohafield linked, undef is returned. (Corresponding unit test is removed here.) [7] C4::Items::ModItemFromMarc: The helper routine _build_default_values_for_mod_marc does no longer have a framework parameter. The cache key default_value_for_mod_marc- is no longer combined with a frameworkcode. Three admin scripts are adjusted accordingly; some tests will be corrected in the next patch. Test plan: See next patch. That patch adjusts all tests involved. Signed-off-by: Josef Moravec --- C4/Biblio.pm | 85 +++++++++++++++++++-------------- C4/Breeding.pm | 2 +- C4/Items.pm | 15 +++--- C4/Koha.pm | 26 ---------- C4/XISBN.pm | 4 +- Koha/MetadataRecord.pm | 2 +- admin/biblio_framework.pl | 4 +- admin/marc_subfields_structure.pl | 4 +- admin/marctagstructure.pl | 4 +- reports/acquisitions_stats.pl | 2 +- t/db_dependent/Koha/IsKohaFieldLinked.t | 44 ----------------- 11 files changed, 67 insertions(+), 125 deletions(-) delete mode 100644 t/db_dependent/Koha/IsKohaFieldLinked.t diff --git a/C4/Biblio.pm b/C4/Biblio.pm index a982416..c8f2da6 100644 --- a/C4/Biblio.pm +++ b/C4/Biblio.pm @@ -1109,43 +1109,54 @@ sub GetMarcSubfieldStructure { =head2 GetMarcFromKohaField - ($MARCfield,$MARCsubfield)=GetMarcFromKohaField($kohafield,$frameworkcode); + ( $field,$subfield ) = GetMarcFromKohaField( $kohafield ); + @fields = GetMarcFromKohaField( $kohafield ); + $field = GetMarcFromKohaField( $kohafield ); -Returns the MARC fields & subfields mapped to the koha field -for the given frameworkcode or default framework if $frameworkcode is missing + Returns the MARC fields & 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. =cut sub GetMarcFromKohaField { - my ( $kohafield, $frameworkcode ) = @_; - return (0, undef) unless $kohafield; - my $mss = GetMarcSubfieldStructure( $frameworkcode ); + my ( $kohafield ) = @_; + return unless $kohafield; + # The next call uses the Default framework since it is AUTHORITATIVE + # for all Koha to MARC mappings. + my $mss = GetMarcSubfieldStructure( '' ); # Do not change framework my @retval; foreach( @{ $mss->{$kohafield} } ) { push @retval, $_->{tagfield}, $_->{tagsubfield}; } - return wantarray ? @retval : $retval[0]; + return wantarray ? @retval : ( @retval ? $retval[0] : undef ); } =head2 GetMarcSubfieldStructureFromKohaField - my $subfield_structure = &GetMarcSubfieldStructureFromKohaField($kohafield, $frameworkcode); - -Returns a hashref where keys are marc_subfield_structure column names for the -row where kohafield=$kohafield for the given framework code. -In list context returns a list of those hashrefs in case duplicate Koha to -MARC mappings exist. + my $str = GetMarcSubfieldStructureFromKohaField( $kohafield ); -$frameworkcode is optional. If not given, then the default framework is used. + 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. =cut sub GetMarcSubfieldStructureFromKohaField { - my ( $kohafield, $frameworkcode ) = @_; + my ( $kohafield ) = @_; return unless $kohafield; - my $mss = GetMarcSubfieldStructure( $frameworkcode ); + # The next call uses the Default framework since it is AUTHORITATIVE + # for all Koha to MARC mappings. + my $mss = GetMarcSubfieldStructure(''); # Do not change framework return unless $mss->{$kohafield}; return wantarray ? @{$mss->{$kohafield}} : $mss->{$kohafield}->[0]; } @@ -2158,24 +2169,26 @@ sub GetFrameworkCode { =head2 TransformKohaToMarc - $record = TransformKohaToMarc( $hash ) + $record = TransformKohaToMarc( $hash [, $params ] ) -This function builds partial MARC::Record from a hash -Hash entries can be from biblio or biblioitems. +This function builds a (partial) MARC::Record from a hash. +Hash entries can be from biblio, biblioitems or items. +The params hash includes the parameter no_split used in C4::Items. This function is called in acquisition module, to create a basic catalogue -entry from user entry +entry from user entry. =cut sub TransformKohaToMarc { - my ( $hash, $frameworkcode, $params ) = @_; + my ( $hash, $params ) = @_; my $record = MARC::Record->new(); SetMarcUnicodeFlag( $record, C4::Context->preference("marcflavour") ); - # NOTE: Many older calls do not include a frameworkcode. In that case - # we default to Default framework. - my $mss = GetMarcSubfieldStructure( $frameworkcode//'' ); + + # In the next call we use the Default framework, since it is considered + # authoritative for Koha to Marc mappings. + my $mss = GetMarcSubfieldStructure( '' ); # do not change framework my $tag_hr = {}; while ( my ($kohafield, $value) = each %$hash ) { foreach my $fld ( @{ $mss->{$kohafield} } ) { @@ -2560,19 +2573,19 @@ sub TransformHtmlToMarc { =head2 TransformMarcToKoha - $result = TransformMarcToKoha( $record, $frameworkcode, $limit ) + $result = TransformMarcToKoha( $record, undef, $limit ) Extract data from a MARC bib record into a hashref representing -Koha biblio, biblioitems, and items fields. +Koha biblio, biblioitems, and items fields. If passed an undefined record will log the error and return an empty -hash_ref +hash_ref. =cut sub TransformMarcToKoha { my ( $record, $frameworkcode, $limit_table ) = @_; - $frameworkcode //= q{}; + # FIXME Parameter $frameworkcode is obsolete and will be removed $limit_table //= q{}; my $result = {}; @@ -2586,13 +2599,13 @@ sub TransformMarcToKoha { %tables = ( items => 1 ); } - my $mss = GetMarcSubfieldStructure( $frameworkcode ); + # The next call acknowledges Default as the authoritative framework + # for Koha to MARC mappings. + my $mss = GetMarcSubfieldStructure(''); # Do not change framework foreach my $kohafield ( keys %{ $mss } ) { my ( $table, $column ) = split /[.]/, $kohafield, 2; next unless $tables{$table}; - my $val = TransformMarcToKohaOneField( - $kohafield, $record, $frameworkcode, - ); + my $val = TransformMarcToKohaOneField( $kohafield, $record ); next if !defined $val; my $key = _disambiguate( $table, $column ); $result->{$key} = $val; @@ -2641,15 +2654,17 @@ sub _disambiguate { =head2 TransformMarcToKohaOneField - $val = TransformMarcToKohaOneField( 'biblio.title', $marc, $frameworkcode ); + $val = TransformMarcToKohaOneField( 'biblio.title', $marc ); + + Note: The authoritative Default framework is used implicitly. =cut sub TransformMarcToKohaOneField { - my ( $kohafield, $marc, $frameworkcode ) = @_; + my ( $kohafield, $marc ) = @_; my ( @rv, $retval ); - my @mss = GetMarcSubfieldStructureFromKohaField($kohafield, $frameworkcode); + my @mss = GetMarcSubfieldStructureFromKohaField($kohafield); foreach my $fldhash ( @mss ) { my $tag = $fldhash->{tagfield}; my $sub = $fldhash->{tagsubfield}; diff --git a/C4/Breeding.pm b/C4/Breeding.pm index 98f46c4..a17406b 100644 --- a/C4/Breeding.pm +++ b/C4/Breeding.pm @@ -332,7 +332,7 @@ sub _add_rowdata { date2 => 'biblioitems.publicationyear', #UNIMARC ); foreach my $k (keys %fetch) { - $row->{$k} = C4::Biblio::TransformMarcToKohaOneField( $fetch{$k}, $record, q{} ); # default framework + $row->{$k} = C4::Biblio::TransformMarcToKohaOneField( $fetch{$k}, $record ); } $row->{date}//= $row->{date2}; $row->{isbn}=_isbn_replace($row->{isbn}); diff --git a/C4/Items.pm b/C4/Items.pm index b35801c..a8a06a2 100644 --- a/C4/Items.pm +++ b/C4/Items.pm @@ -441,10 +441,11 @@ Returns item record =cut sub _build_default_values_for_mod_marc { - my ($frameworkcode) = @_; + # Has no framework parameter anymore, since Default is authoritative + # for Koha to MARC mappings. my $cache = Koha::Caches->get_instance(); - my $cache_key = "default_value_for_mod_marc-$frameworkcode"; + my $cache_key = "default_value_for_mod_marc-"; my $cached = $cache->get_from_cache($cache_key); return $cached if $cached; @@ -483,10 +484,8 @@ sub _build_default_values_for_mod_marc { while ( my ( $field, $default_value ) = each %$default_values ) { my $kohafield = $field; $kohafield =~ s|^([^\.]+)$|items.$1|; - $default_values_for_mod_from_marc{$field} = - $default_value - if C4::Koha::IsKohaFieldLinked( - { kohafield => $kohafield, frameworkcode => $frameworkcode } ); + $default_values_for_mod_from_marc{$field} = $default_value + if C4::Biblio::GetMarcFromKohaField( $kohafield ); } $cache->set_in_cache($cache_key, \%default_values_for_mod_from_marc); @@ -505,7 +504,7 @@ sub ModItemFromMarc { my $localitemmarc = MARC::Record->new; $localitemmarc->append_fields( $item_marc->field($itemtag) ); my $item = &TransformMarcToKoha( $localitemmarc, $frameworkcode, 'items' ); - my $default_values = _build_default_values_for_mod_marc($frameworkcode); + my $default_values = _build_default_values_for_mod_marc(); foreach my $item_field ( keys %$default_values ) { $item->{$item_field} = $default_values->{$item_field} unless exists $item->{$item_field}; @@ -1495,7 +1494,7 @@ sub Item2Marc { }; my $framework = C4::Biblio::GetFrameworkCode( $biblionumber ); my $itemmarc = C4::Biblio::TransformKohaToMarc( - $mungeditem, $framework, { no_split => 1}, + $mungeditem, { no_split => 1}, ); my ( $itemtag, $itemsubfield ) = C4::Biblio::GetMarcFromKohaField( "items.itemnumber", $framework, diff --git a/C4/Koha.pm b/C4/Koha.pm index f547fe0..42c8af1 100644 --- a/C4/Koha.pm +++ b/C4/Koha.pm @@ -1002,32 +1002,6 @@ sub GetVariationsOfISSNs { return wantarray ? @issns : join( " | ", @issns ); } - -=head2 IsKohaFieldLinked - - my $is_linked = IsKohaFieldLinked({ - kohafield => $kohafield, - frameworkcode => $frameworkcode, - }); - - Return 1 if the field is linked - -=cut - -sub IsKohaFieldLinked { - my ( $params ) = @_; - my $kohafield = $params->{kohafield}; - my $frameworkcode = $params->{frameworkcode} || ''; - my $dbh = C4::Context->dbh; - my $is_linked = $dbh->selectcol_arrayref( q| - SELECT COUNT(*) - FROM marc_subfield_structure - WHERE frameworkcode = ? - AND kohafield = ? - |,{}, $frameworkcode, $kohafield ); - return $is_linked->[0]; -} - 1; __END__ diff --git a/C4/XISBN.pm b/C4/XISBN.pm index b955a6f..3d160f0 100644 --- a/C4/XISBN.pm +++ b/C4/XISBN.pm @@ -72,9 +72,7 @@ sub _get_biblio_from_xisbn { return unless ( !$errors && scalar @$results ); my $record = C4::Search::new_record_from_zebra( 'biblioserver', $results->[0] ); - my $biblionumber = C4::Biblio::TransformMarcToKohaOneField( - 'biblio.biblionumber', $record, # Default framework used - ); + my $biblionumber = C4::Biblio::TransformMarcToKohaOneField( 'biblio.biblionumber', $record ); return unless $biblionumber; my $biblio = Koha::Biblios->find( $biblionumber ); diff --git a/Koha/MetadataRecord.pm b/Koha/MetadataRecord.pm index e451a2b..789764e 100644 --- a/Koha/MetadataRecord.pm +++ b/Koha/MetadataRecord.pm @@ -118,7 +118,7 @@ sub createMergeHash { sub getKohaField { my ($self, $kohafield) = @_; if ($self->schema =~ m/marc/) { - return C4::Biblio::TransformMarcToKohaOneField($kohafield, $self->record); # Default framework used + return C4::Biblio::TransformMarcToKohaOneField($kohafield, $self->record); } } diff --git a/admin/biblio_framework.pl b/admin/biblio_framework.pl index dd4af97..d99351b 100755 --- a/admin/biblio_framework.pl +++ b/admin/biblio_framework.pl @@ -80,7 +80,7 @@ if ( $op eq 'add_form' ) { } $cache->clear_from_cache("MarcStructure-0-$frameworkcode"); $cache->clear_from_cache("MarcStructure-1-$frameworkcode"); - $cache->clear_from_cache("default_value_for_mod_marc-$frameworkcode"); + $cache->clear_from_cache("default_value_for_mod_marc-"); $cache->clear_from_cache("MarcSubfieldStructure-$frameworkcode"); $op = 'list'; } elsif ( $op eq 'delete_confirm' ) { @@ -111,7 +111,7 @@ if ( $op eq 'add_form' ) { } $cache->clear_from_cache("MarcStructure-0-$frameworkcode"); $cache->clear_from_cache("MarcStructure-1-$frameworkcode"); - $cache->clear_from_cache("default_value_for_mod_marc-$frameworkcode"); + $cache->clear_from_cache("default_value_for_mod_marc-"); $cache->clear_from_cache("MarcSubfieldStructure-$frameworkcode"); $op = 'list'; } diff --git a/admin/marc_subfields_structure.pl b/admin/marc_subfields_structure.pl index 077a58b..ebec042 100755 --- a/admin/marc_subfields_structure.pl +++ b/admin/marc_subfields_structure.pl @@ -338,7 +338,7 @@ elsif ( $op eq 'add_validate' ) { $sth_update->finish; $cache->clear_from_cache("MarcStructure-0-$frameworkcode"); $cache->clear_from_cache("MarcStructure-1-$frameworkcode"); - $cache->clear_from_cache("default_value_for_mod_marc-$frameworkcode"); + $cache->clear_from_cache("default_value_for_mod_marc-"); $cache->clear_from_cache("MarcSubfieldStructure-$frameworkcode"); print $input->redirect("/cgi-bin/koha/admin/marc_subfields_structure.pl?tagfield=$tagfield&frameworkcode=$frameworkcode"); @@ -384,7 +384,7 @@ elsif ( $op eq 'delete_confirmed' ) { } $cache->clear_from_cache("MarcStructure-0-$frameworkcode"); $cache->clear_from_cache("MarcStructure-1-$frameworkcode"); - $cache->clear_from_cache("default_value_for_mod_marc-$frameworkcode"); + $cache->clear_from_cache("default_value_for_mod_marc-"); $cache->clear_from_cache("MarcSubfieldStructure-$frameworkcode"); print $input->redirect("/cgi-bin/koha/admin/marc_subfields_structure.pl?tagfield=$tagfield&frameworkcode=$frameworkcode"); exit; diff --git a/admin/marctagstructure.pl b/admin/marctagstructure.pl index 4800b34..97f96f1 100755 --- a/admin/marctagstructure.pl +++ b/admin/marctagstructure.pl @@ -149,7 +149,7 @@ if ($op eq 'add_form') { } $cache->clear_from_cache("MarcStructure-0-$frameworkcode"); $cache->clear_from_cache("MarcStructure-1-$frameworkcode"); - $cache->clear_from_cache("default_value_for_mod_marc-$frameworkcode"); + $cache->clear_from_cache("default_value_for_mod_marc-"); $cache->clear_from_cache("MarcSubfieldStructure-$frameworkcode"); } print $input->redirect("/cgi-bin/koha/admin/marctagstructure.pl?searchfield=$tagfield&frameworkcode=$frameworkcode"); @@ -177,7 +177,7 @@ if ($op eq 'add_form') { $sth2->execute($searchfield, $frameworkcode); $cache->clear_from_cache("MarcStructure-0-$frameworkcode"); $cache->clear_from_cache("MarcStructure-1-$frameworkcode"); - $cache->clear_from_cache("default_value_for_mod_marc-$frameworkcode"); + $cache->clear_from_cache("default_value_for_mod_marc-"); $cache->clear_from_cache("MarcSubfieldStructure-$frameworkcode"); } $template->param( diff --git a/reports/acquisitions_stats.pl b/reports/acquisitions_stats.pl index ac86289..d7dea6a 100755 --- a/reports/acquisitions_stats.pl +++ b/reports/acquisitions_stats.pl @@ -186,7 +186,7 @@ else { my @branches = Koha::Libraries->search({}, { order_by => 'branchname' }); - my $ccode_subfield_structure = GetMarcSubfieldStructureFromKohaField('items.ccode', ''); + my $ccode_subfield_structure = GetMarcSubfieldStructureFromKohaField('items.ccode'); my $ccode_label; my $ccode_avlist; if($ccode_subfield_structure) { diff --git a/t/db_dependent/Koha/IsKohaFieldLinked.t b/t/db_dependent/Koha/IsKohaFieldLinked.t deleted file mode 100644 index 90a5704..0000000 --- a/t/db_dependent/Koha/IsKohaFieldLinked.t +++ /dev/null @@ -1,44 +0,0 @@ -use Modern::Perl; -use Test::More; - -use C4::Context; -use C4::Koha; - -my $dbh = C4::Context->dbh; -$dbh->{RaiseError} = 1; -$dbh->{AutoCommit} = 0; - -my $frameworkcode = 'FCUT'; -$dbh->do( qq| - INSERT INTO marc_subfield_structure( - tagfield, tagsubfield, liblibrarian, kohafield, frameworkcode - ) VALUES - ('952', 'p', 'Barcode', 'items.barcode', '$frameworkcode'), - ('952', '8', 'Collection code', 'items.ccode', '$frameworkcode'), - ('952', '7', 'Not for loan', 'items.notforloan', '$frameworkcode'), - ('952', 'y', 'Koha item type', 'items.itype', '$frameworkcode'), - ('952', 'c', 'Permanent location', '', '$frameworkcode') -|); - -is ( C4::Koha::IsKohaFieldLinked( { - kohafield => 'items.barcode', - frameworkcode => $frameworkcode, -}), 1, 'items.barcode is linked' ); - -is ( C4::Koha::IsKohaFieldLinked( { - kohafield => 'items.notforloan', - frameworkcode => $frameworkcode, -}), 1, 'items.notforloan is linked' ); - -is ( C4::Koha::IsKohaFieldLinked( { - kohafield => 'notforloan', - frameworkcode => $frameworkcode, -}), 0, 'notforloan is not linked' ); - -is ( C4::Koha::IsKohaFieldLinked( { - kohafield => 'items.permanent_location', - frameworkcode => $frameworkcode, -}), 0, 'items.permanent_location is not linked' ); - - -done_testing; -- 2.1.4