Bugzilla – Attachment 97357 Details for
Bug 24269
Authority matching in Elasticsearch is broken when authority has subdivisions
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 24269: Adjust C4::Heading to generate headings from auth tags
Bug-24269-Adjust-C4Heading-to-generate-headings-fr.patch (text/plain), 16.39 KB, created by
Nick Clemens (kidclamp)
on 2020-01-14 15:58:19 UTC
(
hide
)
Description:
Bug 24269: Adjust C4::Heading to generate headings from auth tags
Filename:
MIME Type:
Creator:
Nick Clemens (kidclamp)
Created:
2020-01-14 15:58:19 UTC
Size:
16.39 KB
patch
obsolete
>From 0675471650d09aa358b93f809815d6f7d3090ce8 Mon Sep 17 00:00:00 2001 >From: Nick Clemens <nick@bywatersolutions.com> >Date: Tue, 14 Jan 2020 13:00:03 +0000 >Subject: [PATCH] Bug 24269: Adjust C4::Heading to generate headings from auth > tags > >--- > C4/Biblio.pm | 5 +-- > C4/Heading.pm | 30 +++++++++-------- > C4/Heading/MARC21.pm | 66 +++++++++++++++++++++++++++++++++----- > C4/Heading/UNIMARC.pm | 6 ++-- > C4/Linker/Default.pm | 2 +- > Koha/SearchEngine/Elasticsearch.pm | 14 +++++++- > t/db_dependent/Heading.t | 33 ++++++++++--------- > t/db_dependent/Heading_MARC21.t | 2 +- > t/db_dependent/Linker_Default.t | 6 ++-- > t/db_dependent/Linker_FirstMatch.t | 2 +- > 10 files changed, 117 insertions(+), 49 deletions(-) > >diff --git a/C4/Biblio.pm b/C4/Biblio.pm >index ce0d4fbdb6..753692ac75 100644 >--- a/C4/Biblio.pm >+++ b/C4/Biblio.pm >@@ -502,8 +502,9 @@ sub LinkBibHeadingsToAuthorities { > $allowrelink = 1 unless defined $allowrelink; > my $num_headings_changed = 0; > foreach my $field ( $bib->fields() ) { >- my $heading = C4::Heading->new_from_bib_field( $field, $frameworkcode ); >+ my $heading = C4::Heading->new_from_field( $field, $frameworkcode ); > next unless defined $heading; >+ warn $heading->search_form; > > # check existing $9 > my $current_link = $field->subfield('9'); >@@ -549,7 +550,7 @@ sub LinkBibHeadingsToAuthorities { > map { > $authfield->add_subfields( $_->[0] => $_->[1] ) > if ( $_->[0] =~ /[A-z]/ && $_->[0] ne "a" >- && C4::Heading::valid_bib_heading_subfield( >+ && C4::Heading::valid_heading_subfield( > $authority_type->auth_tag_to_report, $_->[0] ) > ); > } $field->subfields(); >diff --git a/C4/Heading.pm b/C4/Heading.pm >index 5dfd0fadb0..9d4cc668e1 100644 >--- a/C4/Heading.pm >+++ b/C4/Heading.pm >@@ -33,7 +33,7 @@ C4::Heading > =head1 SYNOPSIS > > use C4::Heading; >- my $heading = C4::Heading->new_from_bib_field($field, $frameworkcode); >+ my $heading = C4::Heading->new_from_field($field, $frameworkcode); > my $thesaurus = $heading->thesaurus(); > my $type = $heading->type(); > my $display_heading = $heading->display_form(); >@@ -46,9 +46,9 @@ headings found in bibliographic and authority records. > > =head1 METHODS > >-=head2 new_from_bib_field >+=head2 new_from_field > >- my $heading = C4::Heading->new_from_bib_field($field, $frameworkcode, [, $marc_flavour]); >+ my $heading = C4::Heading->new_from_field($field, $frameworkcode, [, $marc_flavour]); > > Given a C<MARC::Field> object containing a heading from a > bib record, create a C<C4::Heading> object. >@@ -57,21 +57,23 @@ The optional second parameter is the MARC flavour (i.e., MARC21 > or UNIMARC); if this parameter is not supplied, it is > taken from the Koha application context. > >+The optional third parameter is 'auth' - it is handled as boolean. If supplied we treat the field as an auth record field. Otherwise if it is a bib field. >+ > If the MARC field supplied is not a valid heading, undef > is returned. > > =cut > >-sub new_from_bib_field { >+sub new_from_field { > my $class = shift; > my $field = shift; > my $frameworkcode = shift; >- my $marcflavour = @_ ? shift : C4::Context->preference('marcflavour'); >- >+ my $auth = shift; >+ my $marcflavour = C4::Context->preference('marcflavour'); > my $marc_handler = _marc_format_handler($marcflavour); > > my $tag = $field->tag(); >- return unless $marc_handler->valid_bib_heading_tag( $tag, $frameworkcode ); >+ return unless $marc_handler->valid_heading_tag( $tag, $frameworkcode, $auth ); > my $self = {}; > > $self->{'field'} = $field; >@@ -79,7 +81,7 @@ sub new_from_bib_field { > $self->{'auth_type'}, $self->{'thesaurus'}, > $self->{'search_form'}, $self->{'display_form'}, > $self->{'match_type'} >- ) = $marc_handler->parse_heading($field); >+ ) = $marc_handler->parse_heading($field, $auth ); > > bless $self, $class; > return $self; >@@ -170,20 +172,20 @@ sub preferred_authorities { > return $results; > } > >-=head2 valid_bib_heading_subfield >+=head2 valid_heading_subfield > >- if (C4::Heading::valid_bib_heading_subfield('100', 'e', '')) ... >+ if (C4::Heading::valid_heading_subfield('100', 'e', '')) ... > > =cut > >-sub valid_bib_heading_subfield { >+sub valid_heading_subfield { > my $tag = shift; > my $subfield = shift; >- my $marcflavour = @_ ? shift : C4::Context->preference('marcflavour'); >+ my $marcflavour = C4::Context->preference('marcflavour'); >+ my $auth = shift; > > my $marc_handler = _marc_format_handler($marcflavour); >- >- return $marc_handler->valid_bib_heading_subfield( $tag, $subfield ); >+ return $marc_handler->valid_heading_subfield( $tag, $subfield, $auth ); > } > > =head1 INTERNAL METHODS >diff --git a/C4/Heading/MARC21.pm b/C4/Heading/MARC21.pm >index fc4d576a45..604c1811aa 100644 >--- a/C4/Heading/MARC21.pm >+++ b/C4/Heading/MARC21.pm >@@ -119,6 +119,49 @@ my $bib_heading_fields = { > { auth_type => 'UNIF_TITLE', subfields => 'adfghklmnoprst', series => 1 }, > }; > >+my $auth_heading_fields = { >+ '100' => { >+ auth_type => 'PERSO_NAME', >+ subfields => 'abcdfghjklmnopqrstvxyz', >+ main_entry => 1 >+ }, >+ '110' => { >+ auth_type => 'CORPO_NAME', >+ subfields => 'abcdfghklmnoprstvxyz', >+ main_entry => 1 >+ }, >+ '111' => { >+ auth_type => 'MEETI_NAME', >+ subfields => 'acdfghjklnpqstvxyz', >+ main_entry => 1 >+ }, >+ '130' => { >+ auth_type => 'UNIF_TITLE', >+ subfields => 'adfghklmnoprstvxyz', >+ main_entry => 1 >+ }, >+ '148' => { >+ auth_type => 'CHRON_TERM', >+ subfields => 'avxyz', >+ main_entry => 1 >+ }, >+ '150' => { >+ auth_type => 'TOPIC_TERM', >+ subfields => 'abgvxyz', >+ main_entry => 1 >+ }, >+ '151' => { >+ auth_type => 'GEOG_NAME', >+ subfields => 'agvxyz', >+ main_entry => 1 >+ }, >+ '155' => { >+ auth_type => 'GENRE/FORM', >+ subfields => 'agvxyz', >+ main_entry => 1 >+ } >+}; >+ > =head2 subdivisions > > =cut >@@ -143,16 +186,18 @@ sub new { > return bless {}, $class; > } > >-=head2 valid_bib_heading_tag >+=head2 valid_heading_tag > > =cut > >-sub valid_bib_heading_tag { >+sub valid_heading_tag { > my $self = shift; > my $tag = shift; > my $frameworkcode = shift; >+ my $auth = shift; >+ my $heading_fields = $auth ? { %$auth_heading_fields } : { %$bib_heading_fields }; > >- if ( exists $bib_heading_fields->{$tag} ) { >+ if ( exists $heading_fields->{$tag} ) { > return 1; > } > else { >@@ -161,17 +206,20 @@ sub valid_bib_heading_tag { > > } > >-=head2 valid_bib_heading_subfield >+=head2 valid_heading_subfield > > =cut > >-sub valid_bib_heading_subfield { >+sub valid_heading_subfield { > my $self = shift; > my $tag = shift; > my $subfield = shift; >+ my $auth = shift; >+ >+ my $heading_fields = $auth ? { %$auth_heading_fields } : { %$bib_heading_fields }; > >- if ( exists $bib_heading_fields->{$tag} ) { >- return 1 if ($bib_heading_fields->{$tag}->{subfields} =~ /$subfield/); >+ if ( exists $heading_fields->{$tag} ) { >+ return 1 if ($heading_fields->{$tag}->{subfields} =~ /$subfield/); > } > return 0; > } >@@ -183,10 +231,12 @@ sub valid_bib_heading_subfield { > sub parse_heading { > my $self = shift; > my $field = shift; >+ my $auth = shift; > > my $tag = $field->tag; >- my $field_info = $bib_heading_fields->{$tag}; >+ my $heading_fields = $auth ? { %$auth_heading_fields } : { %$bib_heading_fields }; > >+ my $field_info = $heading_fields->{$tag}; > my $auth_type = $field_info->{'auth_type'}; > my $thesaurus = > $tag =~ m/6../ >diff --git a/C4/Heading/UNIMARC.pm b/C4/Heading/UNIMARC.pm >index 81dafaef83..e72ec0076c 100644 >--- a/C4/Heading/UNIMARC.pm >+++ b/C4/Heading/UNIMARC.pm >@@ -91,16 +91,16 @@ sub new { > > =cut > >-sub valid_bib_heading_tag { >+sub valid_heading_tag { > my ( $self, $tag ) = @_; > return $bib_heading_fields->{$tag}; > } > >-=head2 valid_bib_heading_subfield >+=head2 valid_heading_subfield > > =cut > >-sub valid_bib_heading_subfield { >+sub valid_heading_subfield { > my $self = shift; > my $tag = shift; > my $subfield = shift; >diff --git a/C4/Linker/Default.pm b/C4/Linker/Default.pm >index 1fa395c19a..bfe1d72308 100644 >--- a/C4/Linker/Default.pm >+++ b/C4/Linker/Default.pm >@@ -68,7 +68,7 @@ sub get_link { > map { $_->[0] => $_->[1] } @subfields > ); > ( $authid, $fuzzy ) = >- $self->get_link( C4::Heading->new_from_bib_field($field), >+ $self->get_link( C4::Heading->new_from_field($field), > $behavior ); > } > } >diff --git a/Koha/SearchEngine/Elasticsearch.pm b/Koha/SearchEngine/Elasticsearch.pm >index 7322f84178..bfb86d7903 100644 >--- a/Koha/SearchEngine/Elasticsearch.pm >+++ b/Koha/SearchEngine/Elasticsearch.pm >@@ -26,6 +26,7 @@ use Koha::Exceptions::Config; > use Koha::Exceptions::Elasticsearch; > use Koha::SearchFields; > use Koha::SearchMarcMaps; >+use C4::Heading; > > use Carp; > use Clone qw(clone); >@@ -470,7 +471,6 @@ sub marc_records_to_documents { > } > > my $data_field_rules = $data_fields_rules->{$tag}; >- > if ($data_field_rules) { > my $subfields_mappings = $data_field_rules->{subfields}; > my $wildcard_mappings = $subfields_mappings->{'*'}; >@@ -483,6 +483,12 @@ sub marc_records_to_documents { > if (@{$mappings}) { > $self->_process_mappings($mappings, $data, $record_document, $altscript); > } >+ if ( grep /match-heading/, @{$mappings} ){ >+ my $matcher = C4::Heading->new_from_field( $field, undef, 1 ); #new auth heading >+ next unless $matcher; >+ push @{$record_document->{'match-heading'}}, $matcher->search_form; >+ push @{$record_document->{'display_form'}}, $matcher->display_form; >+ } > } > > my $subfields_join_mappings = $data_field_rules->{subfields_join}; >@@ -499,6 +505,12 @@ sub marc_records_to_documents { > if ($data) { > $self->_process_mappings($subfields_join_mappings->{$subfields_group}, $data, $record_document, $altscript); > } >+ if ( grep { $_->[0] eq 'match-heading' } @{$subfields_join_mappings->{$subfields_group}} ){ >+ my $matcher = C4::Heading->new_from_field( $field, undef, 1 ); #new auth heading >+ next unless $matcher; >+ push @{$record_document->{'match-heading'}}, $matcher->search_form; >+ push @{$record_document->{'display_form'}}, $matcher->display_form; >+ } > } > } > } >diff --git a/t/db_dependent/Heading.t b/t/db_dependent/Heading.t >index bcbbe515ec..e2f7064667 100755 >--- a/t/db_dependent/Heading.t >+++ b/t/db_dependent/Heading.t >@@ -27,20 +27,23 @@ BEGIN { > } > > subtest "MARC21 tests" => sub { >- plan tests => 7; >+ plan tests => 9; > > t::lib::Mocks::mock_preference('marcflavour', 'MARC21'); > >- ok(C4::Heading::valid_bib_heading_subfield('100', 'a'), '100a valid'); >- ok(!C4::Heading::valid_bib_heading_subfield('100', 'e'), '100e not valid'); >+ ok(C4::Heading::valid_heading_subfield('100', 'a'), '100a valid for bib'); >+ ok(!C4::Heading::valid_heading_subfield('100', 'e'), '100e not valid for bib'); >+ >+ ok(C4::Heading::valid_heading_subfield('100', 'a', 1), '100a valid for authority'); >+ ok(!C4::Heading::valid_heading_subfield('100', 'e', 1), '100e not valid for authority'); > >- ok(C4::Heading::valid_bib_heading_subfield('110', 'a'), '110a valid'); >- ok(!C4::Heading::valid_bib_heading_subfield('110', 'e'), '110e not valid'); >+ ok(C4::Heading::valid_heading_subfield('110', 'a'), '110a valid for bib'); >+ ok(!C4::Heading::valid_heading_subfield('110', 'e'), '110e not valid for bib'); > >- ok(C4::Heading::valid_bib_heading_subfield('600', 'a'), '600a valid'); >- ok(!C4::Heading::valid_bib_heading_subfield('600', 'e'), '600e not valid'); >+ ok(C4::Heading::valid_heading_subfield('600', 'a'), '600a valid for bib'); >+ ok(!C4::Heading::valid_heading_subfield('600', 'e'), '600e not valid for bib'); > >- ok(!C4::Heading::valid_bib_heading_subfield('012', 'a'), '012a invalid field'); >+ ok(!C4::Heading::valid_heading_subfield('012', 'a'), '012a invalid field for bib'); > }; > > subtest "UNIMARC tests" => sub { >@@ -48,14 +51,14 @@ subtest "UNIMARC tests" => sub { > > t::lib::Mocks::mock_preference('marcflavour', 'UNIMARC'); > >- ok(C4::Heading::valid_bib_heading_subfield('100', 'a'), '100a valid'); >- ok(!C4::Heading::valid_bib_heading_subfield('100', 'i'), '100i not valid'); >+ ok(C4::Heading::valid_heading_subfield('100', 'a'), '100a valid for bib'); >+ ok(!C4::Heading::valid_heading_subfield('100', 'i'), '100i not valid fir bib'); > >- ok(C4::Heading::valid_bib_heading_subfield('110', 'a'), '110a valid'); >- ok(!C4::Heading::valid_bib_heading_subfield('110', 'i'), '110i not valid'); >+ ok(C4::Heading::valid_heading_subfield('110', 'a'), '110a valid for bib'); >+ ok(!C4::Heading::valid_heading_subfield('110', 'i'), '110i not valid for bib'); > >- ok(C4::Heading::valid_bib_heading_subfield('600', 'a'), '600a valid'); >- ok(!C4::Heading::valid_bib_heading_subfield('600', 'i'), '600i not valid'); >+ ok(C4::Heading::valid_heading_subfield('600', 'a'), '600a valid for bib'); >+ ok(!C4::Heading::valid_heading_subfield('600', 'i'), '600i not valid for bib'); > >- ok(!C4::Heading::valid_bib_heading_subfield('012', 'a'), '012a invalid field'); >+ ok(!C4::Heading::valid_heading_subfield('012', 'a'), '012a invalid field for bib'); > } >diff --git a/t/db_dependent/Heading_MARC21.t b/t/db_dependent/Heading_MARC21.t >index 0e1f1c9a72..55acc030e7 100755 >--- a/t/db_dependent/Heading_MARC21.t >+++ b/t/db_dependent/Heading_MARC21.t >@@ -16,7 +16,7 @@ BEGIN { > SKIP: { > skip "MARC21 heading tests not applicable to UNIMARC", 2 if C4::Context->preference('marcflavour') eq 'UNIMARC'; > my $field = MARC::Field->new( '650', ' ', '0', a => 'Uncles', x => 'Fiction' ); >- my $heading = C4::Heading->new_from_bib_field($field); >+ my $heading = C4::Heading->new_from_field($field); > is($heading->display_form(), 'Uncles--Fiction', 'Display form generation'); > is($heading->search_form(), 'Uncles generalsubdiv Fiction', 'Search form generation'); > } >diff --git a/t/db_dependent/Linker_Default.t b/t/db_dependent/Linker_Default.t >index 597e86b39e..19f421373f 100755 >--- a/t/db_dependent/Linker_Default.t >+++ b/t/db_dependent/Linker_Default.t >@@ -50,9 +50,9 @@ subtest 'Test caching in get_link and update_cache' => sub { > my $subject_field2 = MARC::Field->new($tags[0],0,2,$tags[1]=>'Science fiction'); > my $genre_field = MARC::Field->new($tags[2],0,2,$tags[3]=>'Science fiction'); > # Can we build a heading from it? >- my $subject_heading = C4::Heading->new_from_bib_field( $subject_field, q{} ); >- my $subject_heading2 = C4::Heading->new_from_bib_field( $subject_field, q{} ); >- my $genre_heading = C4::Heading->new_from_bib_field( $genre_field, q{} ); >+ my $subject_heading = C4::Heading->new_from_field( $subject_field, q{} ); >+ my $subject_heading2 = C4::Heading->new_from_field( $subject_field, q{} ); >+ my $genre_heading = C4::Heading->new_from_field( $genre_field, q{} ); > > > # Now test to see if C4::Linker can find it. >diff --git a/t/db_dependent/Linker_FirstMatch.t b/t/db_dependent/Linker_FirstMatch.t >index 04cc6a664b..cfcd752437 100755 >--- a/t/db_dependent/Linker_FirstMatch.t >+++ b/t/db_dependent/Linker_FirstMatch.t >@@ -110,7 +110,7 @@ sub run_tests { > my $heading; > ok( > defined( >- $heading = C4::Heading->new_from_bib_field( $new_bibfield, q{} ) >+ $heading = C4::Heading->new_from_field( $new_bibfield, q{} ) > ), > 'Creating heading from bib field' > ); >-- >2.11.0
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 24269
:
97357
|
97981
|
97982
|
97996
|
99039
|
99246