Bugzilla – Attachment 68205 Details for
Bug 19096
Koha to MARC mappings (Part 2): Make Default authoritative
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 19096: Make Default authoritative in core modules
Bug-19096-Make-Default-authoritative-in-core-modul.patch (text/plain), 19.09 KB, created by
Marcel de Rooy
on 2017-10-17 13:00:14 UTC
(
hide
)
Description:
Bug 19096: Make Default authoritative in core modules
Filename:
MIME Type:
Creator:
Marcel de Rooy
Created:
2017-10-17 13:00:14 UTC
Size:
19.09 KB
patch
obsolete
>From 5785200c53798636bb4a1e6e2a1e58b9c97adbe2 Mon Sep 17 00:00:00 2001 >From: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> >Date: Thu, 10 Aug 2017 10:59:07 +0200 >Subject: [PATCH] Bug 19096: Make Default authoritative in core modules >Content-Type: text/plain; charset=utf-8 > >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 <josef.moravec@gmail.com> >--- > 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 6d4fd2e..774d4f9 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 6281393..a7154be 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}; >@@ -1500,7 +1499,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 9cda7f2..f3d202f 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
<b>You cannot view the attachment while viewing its details because your browser does not support IFRAMEs. <a href="attachment.cgi?id=68205">View the attachment on a separate page</a>.</b>
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 19096
:
66333
|
66334
|
66335
|
66336
|
66337
|
66338
|
66406
|
66407
|
66408
|
66409
|
66410
|
66411
|
67508
|
67509
|
67510
|
67511
|
67512
|
67513
|
67590
|
67591
|
67592
|
67593
|
67594
|
67595
|
68205
|
68206
|
68207
|
68208
|
68209
|
68210
|
68774
|
68775
|
68776
|
68777
|
68778
|
68779
|
69629
|
69646
|
69647
|
69681