@@ -, +, @@ authority --- C4/AuthoritiesMarc.pm | 121 +++++++++--------- C4/Biblio.pm | 5 +- C4/Heading.pm | 2 + C4/Heading/MARC21.pm | 120 +++++++++++++++++ .../mandatory/authorities_normal_marc21.sql | 5 +- t/db_dependent/AuthoritiesMarc.t | 28 +++- t/db_dependent/AuthoritiesMarc/MARC21.t | 91 +++++++++++++ 7 files changed, 308 insertions(+), 64 deletions(-) create mode 100644 t/db_dependent/AuthoritiesMarc/MARC21.t --- a/C4/AuthoritiesMarc.pm +++ a/C4/AuthoritiesMarc.pm @@ -1,6 +1,7 @@ package C4::AuthoritiesMarc; # Copyright 2000-2002 Katipo Communications +# Copyright 2018 The National Library of Finland, University of Helsinki # # This file is part of Koha. # @@ -359,10 +360,12 @@ sub GuessAuthTypeCode { '110'=>{authtypecode=>'CORPO_NAME'}, '111'=>{authtypecode=>'MEETI_NAME'}, '130'=>{authtypecode=>'UNIF_TITLE'}, + '147'=>{authtypecode=>'NAME_EVENT'}, '148'=>{authtypecode=>'CHRON_TERM'}, '150'=>{authtypecode=>'TOPIC_TERM'}, '151'=>{authtypecode=>'GEOGR_NAME'}, '155'=>{authtypecode=>'GENRE/FORM'}, + '162'=>{authtypecode=>'MED_PERFRM'}, '180'=>{authtypecode=>'GEN_SUBDIV'}, '181'=>{authtypecode=>'GEO_SUBDIV'}, '182'=>{authtypecode=>'CHRON_SUBD'}, @@ -938,6 +941,8 @@ sub BuildSummary { # construct MARC21 summary # FIXME - looping over 1XX is questionable # since MARC21 authority should have only one 1XX + use C4::Heading::MARC21; + my $handler = C4::Heading::MARC21->new(); my $subfields_to_report; foreach my $field ($record->field('1..')) { my $tag = $field->tag(); @@ -945,31 +950,9 @@ sub BuildSummary { # FIXME - 152 is not a good tag to use # in MARC21 -- purely local tags really ought to be # 9XX - if ($tag eq '100') { - $subfields_to_report = 'abcdefghjklmnopqrstvxyz'; - } elsif ($tag eq '110') { - $subfields_to_report = 'abcdefghklmnoprstvxyz'; - } elsif ($tag eq '111') { - $subfields_to_report = 'acdefghklnpqstvxyz'; - } elsif ($tag eq '130') { - $subfields_to_report = 'adfghklmnoprstvxyz'; - } elsif ($tag eq '148') { - $subfields_to_report = 'abvxyz'; - } elsif ($tag eq '150') { - $subfields_to_report = 'abvxyz'; - } elsif ($tag eq '151') { - $subfields_to_report = 'avxyz'; - } elsif ($tag eq '155') { - $subfields_to_report = 'abvxyz'; - } elsif ($tag eq '180') { - $subfields_to_report = 'vxyz'; - } elsif ($tag eq '181') { - $subfields_to_report = 'vxyz'; - } elsif ($tag eq '182') { - $subfields_to_report = 'vxyz'; - } elsif ($tag eq '185') { - $subfields_to_report = 'vxyz'; - } + + $subfields_to_report = $handler->get_auth_heading_subfields_to_report($tag); + if ($subfields_to_report) { push @authorized, { heading => $field->as_string($subfields_to_report), @@ -1093,44 +1076,47 @@ sub GetAuthorizedHeading { return $field->as_string('abcdefghijlmnopqrstuvwxyz'); } } else { + use C4::Heading::MARC21; + my $handler = C4::Heading::MARC21->new(); + foreach my $field ($record->field('1..')) { - my $tag = $field->tag(); - next if "152" eq $tag; -# FIXME - 152 is not a good tag to use -# in MARC21 -- purely local tags really ought to be -# 9XX - if ($tag eq '100') { - return $field->as_string('abcdefghjklmnopqrstvxyz68'); - } elsif ($tag eq '110') { - return $field->as_string('abcdefghklmnoprstvxyz68'); - } elsif ($tag eq '111') { - return $field->as_string('acdefghklnpqstvxyz68'); - } elsif ($tag eq '130') { - return $field->as_string('adfghklmnoprstvxyz68'); - } elsif ($tag eq '148') { - return $field->as_string('abvxyz68'); - } elsif ($tag eq '150') { - return $field->as_string('abvxyz68'); - } elsif ($tag eq '151') { - return $field->as_string('avxyz68'); - } elsif ($tag eq '155') { - return $field->as_string('abvxyz68'); - } elsif ($tag eq '180') { - return $field->as_string('vxyz68'); - } elsif ($tag eq '181') { - return $field->as_string('vxyz68'); - } elsif ($tag eq '182') { - return $field->as_string('vxyz68'); - } elsif ($tag eq '185') { - return $field->as_string('vxyz68'); - } else { - return $field->as_string(); - } + my $subfields = $handler->get_valid_bib_heading_subfields($field->tag()); + return $field->as_string($subfields) if ($subfields); } } return; } +=head2 CompareFieldWithAuthority + + $match = &CompareFieldWithAuthority({ field => $field, authid => $authid }) + +Takes a MARC::Field from a bibliographic record and an authid, and returns true if they match. + +=cut + +sub CompareFieldWithAuthority { + my $args = shift; + + my $record = GetAuthority($args->{authid}); + return unless (ref $record eq 'MARC::Record'); + if (C4::Context->preference('marcflavour') eq 'UNIMARC') { + # UNIMARC has same subfields for bibs and authorities + foreach my $field ($record->field('2..')) { + return compare_fields($field, $args->{field}, 'abcdefghijlmnopqrstuvwxyz'); + } + } else { + use C4::Heading::MARC21; + my $handler = C4::Heading::MARC21->new(); + + foreach my $field ($record->field('1..')) { + my $subfields = $handler->get_valid_bib_heading_subfields($field->tag()); + return compare_fields($field, $args->{field}, $subfields) if ($subfields); + } + } + return 0; +} + =head2 BuildAuthHierarchies $text= &BuildAuthHierarchies( $authid, $force) @@ -1592,6 +1578,26 @@ sub get_auth_type_location { } } +=head2 compare_fields + + my match = compare_fields($field1, $field2, 'abcde'); + +Compares the listed subfields of both fields and return true if they all match + +=cut + +sub compare_fields { + my ($field1, $field2, $subfields) = @_; + + foreach my $subfield (split(//, $subfields)) { + my $subfield1 = $field1->subfield($subfield) // ''; + my $subfield2 = $field2->subfield($subfield) // ''; + return 0 unless $subfield1 eq $subfield2; + } + return 1; +} + + END { } # module clean-up code here (global destructor) 1; @@ -1602,6 +1608,7 @@ __END__ Koha Development Team Paul POULAIN paul.poulain@free.fr +Ere Maijala ere.maijala@helsinki.fi =cut --- a/C4/Biblio.pm +++ a/C4/Biblio.pm @@ -628,10 +628,7 @@ sub _check_valid_auth_link { require C4::AuthoritiesMarc; - my $authorized_heading = - C4::AuthoritiesMarc::GetAuthorizedHeading( { 'authid' => $authid } ) || ''; - - return ($field->as_string('abcdefghijklmnopqrstuvwxyz') eq $authorized_heading); + return C4::AuthoritiesMarc::CompareFieldWithAuthority( { 'field' => $field, 'authid' => $authid } ); } =head2 GetRecordValue --- a/C4/Heading.pm +++ a/C4/Heading.pm @@ -174,6 +174,8 @@ sub preferred_authorities { if (C4::Heading::valid_bib_heading_subfield('100', 'e', '')) ... +Check if the given subfield is valid for the given field. + =cut sub valid_bib_heading_subfield { --- a/C4/Heading/MARC21.pm +++ a/C4/Heading/MARC21.pm @@ -68,6 +68,52 @@ my $bib_heading_fields = { subfields => 'adfghklmnoprst', main_entry => 1 }, + '147' => { + auth_type => 'NAME_EVENT', + subfields => 'acdgvxyz68', + main_entry => 1 + }, + '148' => { + auth_type => 'CHRON_TERM', + subfields => 'abvxyz68', + main_entry => 1 + }, + '150' => { + auth_type => 'TOPIC_TERM', + subfields => 'abvxyz68', + main_entry => 1 + }, + '151' => { + auth_type => 'GEOGR_NAME', + subfields => 'avxyz68', + main_entry => 1 + }, + '155' => { + auth_type => 'GENRE/FORM', + subfields => 'abvxyz68', + main_entry => 1 + }, + '162' => { + auth_type => 'MED_PERFRM', + subfields => 'a68', + main_entry => 1 + }, + '180' => { + auth_type => 'TOPIC_TERM', + subfields => 'vxyz68' + }, + '181' => { + auth_type => 'GEOGR_NAME', + subfields => 'vxyz68' + }, + '182' => { + auth_type => 'CHRON_TERM', + subfields => 'vxyz68' + }, + '185' => { + auth_type => 'GENRE/FORM', + subfields => 'vxyz68' + }, '440' => { auth_type => 'UNIF_TITLE', subfields => 'anp', series => 1 }, '600' => { auth_type => 'PERSO_NAME', @@ -119,6 +165,56 @@ my $bib_heading_fields = { { auth_type => 'UNIF_TITLE', subfields => 'adfghklmnoprst', series => 1 }, }; + +=head2 auth_heading_fields + +=cut + +my $auth_heading_fields = { + '100' => { + subfields => 'abcdefghjklmnopqrstvxyz68', + }, + '110' => { + subfields => 'abcdefghklmnoprstvxyz68', + }, + '111' => { + subfields => 'acdefghklnpqstvxyz68', + }, + '130' => { + subfields => 'adfghklmnoprstvxyz68', + }, + '147' => { + subfields => 'acdgvxyz68', + }, + '148' => { + subfields => 'abvxyz68', + }, + '150' => { + subfields => 'abvxyz68', + }, + '151' => { + subfields => 'avxyz68', + }, + '155' => { + subfields => 'abvxyz68', + }, + '162' => { + subfields => 'a68', + }, + '180' => { + subfields => 'vxyz68', + }, + '181' => { + subfields => 'vxyz68', + }, + '182' => { + subfields => 'vxyz68', + }, + '185' => { + subfields => 'vxyz68', + }, +}; + =head2 subdivisions =cut @@ -176,6 +272,30 @@ sub valid_bib_heading_subfield { return 0; } +=head2 get_valid_bib_heading_subfields + +=cut + +sub get_valid_bib_heading_subfields { + my $self = shift; + my $tag = shift; + + return $bib_heading_fields->{$tag}->{subfields} // undef; +} + +=head2 get_auth_heading_subfields_to_report + +=cut + +sub get_auth_heading_subfields_to_report { + my $self = shift; + my $tag = shift; + + my $subfields = $auth_heading_fields->{$tag}->{subfields} // ''; + $subfields =~ s/[68]//; + return $subfields; +} + =head2 parse_heading =cut --- a/installer/data/mysql/en/marcflavour/marc21/mandatory/authorities_normal_marc21.sql +++ a/installer/data/mysql/en/marcflavour/marc21/mandatory/authorities_normal_marc21.sql @@ -28,11 +28,12 @@ INSERT INTO auth_types (authtypecode, authtypetext, auth_tag_to_report, summary) ('CORPO_NAME', 'Corporate Name', '110', 'Corporate Names'), ('MEETI_NAME', 'Meeting Name', '111', 'Meeting Name'), ('UNIF_TITLE', 'Uniform Title', '130', 'Uniform Title'), + ('NAME_EVENT', 'Named Event', '147', 'Named Event'), ('CHRON_TERM', 'Chronological Term', '148', 'Chronological Term'), ('TOPIC_TERM', 'Topical Term', '150', 'Topical Term'), ('GEOGR_NAME', 'Geographic Name', '151', 'Geographic Name'), - ('GENRE/FORM', 'Genre/Form Term', '155', 'Genre/Form Term'); - + ('GENRE/FORM', 'Genre/Form Term', '155', 'Genre/Form Term'), + ('MED_PERFRM', 'Medium of Performance', '162', 'Medium of Performance'); -- ****************************************************** -- KOHA UNSUPPORTED STANDARD MARC 21 AUTHORITY TYPES. --- a/t/db_dependent/AuthoritiesMarc.t +++ a/t/db_dependent/AuthoritiesMarc.t @@ -5,9 +5,10 @@ use Modern::Perl; -use Test::More tests => 9; +use Test::More tests => 10; use Test::MockModule; use Test::Warn; +use MARC::Field; use MARC::Record; use t::lib::Mocks; @@ -54,6 +55,11 @@ $module->mock('GetAuthority', sub { [ '151', ' ', ' ', a => 'New York (City)' ], [ '551', ' ', ' ', a => 'New York (State)', w => 'g' ] ); + } elsif ($authid eq '5') { + $record->add_fields( + [ '001', '5' ], + [ '100', ' ', ' ', a => 'Lastname, Firstname', b => 'b', c => 'c', i => 'i' ] + ); } else { undef $record; } @@ -214,4 +220,24 @@ subtest 'AddAuthority should respect AUTO_INCREMENT (BZ 18104)' => sub { is( $record->field('001')->data, $id3, 'Check updated 001' ); }; +subtest 'CompareFieldWithAuthority tests' => sub { + plan tests => 3; + + t::lib::Mocks::mock_preference('marcflavour', 'MARC21'); + + $builder->build({ source => 'AuthType', value => { authtypecode => 'PERSO_NAME' }}); + + my $field = MARC::Field->new('100', 0, 0, a => 'Lastname, Firstname', b => 'b', c => 'c'); + + ok(C4::AuthoritiesMarc::CompareFieldWithAuthority({'field' => $field, 'authid' => 5}), 'Authority matches'); + + $field->add_subfields(i => 'X'); + + ok(C4::AuthoritiesMarc::CompareFieldWithAuthority({'field' => $field, 'authid' => 5}), 'Compare ignores unlisted subfields'); + + $field->add_subfields(d => 'd'); + + ok(!C4::AuthoritiesMarc::CompareFieldWithAuthority({'field' => $field, 'authid' => 5}), 'Authority does not match'); +}; + $schema->storage->txn_rollback; --- a/t/db_dependent/AuthoritiesMarc/MARC21.t +++ a/t/db_dependent/AuthoritiesMarc/MARC21.t @@ -0,0 +1,91 @@ +#!/usr/bin/perl +# + +use strict; +use warnings; + +use Test::MockModule; +use Test::More tests => 2; +use MARC::Field; +use MARC::Record; + +use t::lib::Mocks; +use t::lib::TestBuilder; + +BEGIN { + use_ok('C4::AuthoritiesMarc::MARC21'); +} + +my $schema = Koha::Database->new->schema; +$schema->storage->txn_begin; +my $dbh = C4::Context->dbh; +my $builder = t::lib::TestBuilder->new; + +t::lib::Mocks::mock_preference('marcflavour', 'MARC21'); + +subtest 'CompareFieldWithAuthority tests' => sub { + plan tests => 3; + + # We are now going to be testing the authorities hierarchy code, and + # therefore need to pretend that we have consistent data in our database + my $module = new Test::MockModule('C4::AuthoritiesMarc'); + $module->mock('GetHeaderAuthority', sub { + return {'authtrees' => ''}; + }); + $module->mock('AddAuthorityTrees', sub { + return; + }); + $module->mock('GetAuthority', sub { + my ($authid) = @_; + my $record = MARC::Record->new(); + if ($authid eq '1') { + $record->add_fields( + [ '001', '1' ], + [ '151', ' ', ' ', a => 'United States' ] + ); + } elsif ($authid eq '2') { + $record->add_fields( + [ '001', '2' ], + [ '151', ' ', ' ', a => 'New York (State)' ], + [ '551', ' ', ' ', a => 'United States', w => 'g', 9 => '1' ] + ); + } elsif ($authid eq '3') { + $record->add_fields( + [ '001', '3' ], + [ '151', ' ', ' ', a => 'New York (City)' ], + [ '551', ' ', ' ', a => 'New York (State)', w => 'g', 9 => '2' ] + ); + } elsif ($authid eq '4') { + $record->add_fields( + [ '001', '4' ], + [ '151', ' ', ' ', a => 'New York (City)' ], + [ '551', ' ', ' ', a => 'New York (State)', w => 'g' ] + ); + } elsif ($authid eq '5') { + $record->add_fields( + [ '001', '5' ], + [ '100', ' ', ' ', a => 'Lastname, Firstname', b => 'b', c => 'c', i => 'i' ] + ); + } else { + undef $record; + } + return $record; + }); + + $dbh->do('DELETE FROM auth_types'); + $builder->build({ source => 'AuthType', value => { authtypecode => 'PERSO_NAME' }}); + + my $field = MARC::Field->new('100', 0, 0, a => 'Lastname, Firstname', b => 'b', c => 'c'); + + ok(C4::AuthoritiesMarc::CompareFieldWithAuthority({'field' => $field, 'authid' => 5}), 'Authority matches'); + + $field->add_subfields(i => 'X'); + + ok(C4::AuthoritiesMarc::CompareFieldWithAuthority({'field' => $field, 'authid' => 5}), 'Compare ignores unlisted subfields'); + + $field->add_subfields(d => 'd'); + + ok(!C4::AuthoritiesMarc::CompareFieldWithAuthority({'field' => $field, 'authid' => 5}), 'Authority does not match'); +}; + +$schema->storage->txn_rollback; --