Bugzilla – Attachment 99714 Details for
Bug 23719
Record matching for authorities using defined fields is broken under ES
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 23719: [19.05.x] Allow searching specific fields for matching authorities in ES
Bug-23719-1905x-Allow-searching-specific-fields-fo.patch (text/plain), 6.72 KB, created by
Nick Clemens (kidclamp)
on 2020-02-27 18:57:01 UTC
(
hide
)
Description:
Bug 23719: [19.05.x] Allow searching specific fields for matching authorities in ES
Filename:
MIME Type:
Creator:
Nick Clemens (kidclamp)
Created:
2020-02-27 18:57:01 UTC
Size:
6.72 KB
patch
obsolete
>From e311559bc61a29b34d3e6d4d987f041082d3276d Mon Sep 17 00:00:00 2001 >From: Nick <nick@bywatersolutions.com> >Date: Wed, 2 Oct 2019 11:17:13 +0000 >Subject: [PATCH] Bug 23719: [19.05.x] Allow searching specific fields for > matching authorities in ES > >To test: >1 - Export your authorities via Tools->Export data >2 - Define a record matching rule in Admin->Record matchign rules > Use index: LC-card-number > field: 010$a >3 - Stage the exported records for import and use the rule created above for matching >4 - The process does not complete >5 - Check intranet error logs - exception on unknown marclist >6 - Apply patch >7 - Repeat >8 - Success! >9 - prove -v t/db_dependent/Koha/SearchEngine/Elasticsearch/QueryBuilder.t > >Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com> > >Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> > >Bug 23719: (follow-up) Add warn when passed invalid search field in marclist > >Note: I also remove warnings for undefined operation in this patch, is a trivial fix > >To test: >prove -v t/db_dependent/Koha/SearchEngine/Elasticsearch/QueryBuilder.t > >Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> >--- > Koha/SearchEngine/Elasticsearch/QueryBuilder.pm | 14 ++++--- > .../Koha/SearchEngine/Elasticsearch/QueryBuilder.t | 46 +++++++++++++++++----- > 2 files changed, 46 insertions(+), 14 deletions(-) > >diff --git a/Koha/SearchEngine/Elasticsearch/QueryBuilder.pm b/Koha/SearchEngine/Elasticsearch/QueryBuilder.pm >index 6bee35f8cb..0f88e7fbab 100644 >--- a/Koha/SearchEngine/Elasticsearch/QueryBuilder.pm >+++ b/Koha/SearchEngine/Elasticsearch/QueryBuilder.pm >@@ -315,7 +315,7 @@ sub build_authorities_query { > foreach my $s ( @{ $search->{searches} } ) { > my ( $wh, $op, $val ) = @{$s}{qw(where operator value)}; > $wh = '_all' if $wh eq ''; >- if ( $op eq 'is' || $op eq '=' || $op eq 'exact' ) { >+ if ( defined $op && ($op eq 'is' || $op eq '=' || $op eq 'exact') ) { > > # look for something that matches a term completely > # note, '=' is about numerical vals. May need special handling. >@@ -324,7 +324,7 @@ sub build_authorities_query { > # search analyzer applied to them. > push @query_parts, { match_phrase => {"$wh.phrase" => lc $val} }; > } >- elsif ( $op eq 'start' ) { >+ elsif ( defined $op && $op eq 'start' ) { > # startswith search, uses lowercase untokenized version of heading > push @query_parts, { match_phrase_prefix => {"$wh.phrase" => lc $val} }; > } >@@ -449,21 +449,25 @@ sub build_authorities_query_compat { > # This turns the old-style many-options argument form into a more > # extensible hash form that is understood by L<build_authorities_query>. > my @searches; >+ my $mappings = $self->get_elasticsearch_mappings(); > > # Convert to lower case > $marclist = [map(lc, @{$marclist})]; > $orderby = lc $orderby; > >+ my @indexes; > # Make sure everything exists > foreach my $m (@$marclist) { >- Koha::Exceptions::WrongParameter->throw("Invalid marclist field provided: $m") >- unless exists $koha_to_index_name->{$m}; >+ >+ $m = exists $koha_to_index_name->{$m} ? $koha_to_index_name->{$m} : $m; >+ push @indexes, $m; >+ warn "Unknown search field $m in marclist" unless (defined $mappings->{data}->{properties}->{$m} || $m eq ''); > } > for ( my $i = 0 ; $i < @$value ; $i++ ) { > next unless $value->[$i]; #clean empty form values, ES doesn't like undefined searches > push @searches, > { >- where => $koha_to_index_name->{$marclist->[$i]}, >+ where => $indexes[$i], > operator => $operator->[$i], > value => $value->[$i], > }; >diff --git a/t/db_dependent/Koha/SearchEngine/Elasticsearch/QueryBuilder.t b/t/db_dependent/Koha/SearchEngine/Elasticsearch/QueryBuilder.t >index f0231f8476..2e9a356ba9 100644 >--- a/t/db_dependent/Koha/SearchEngine/Elasticsearch/QueryBuilder.t >+++ b/t/db_dependent/Koha/SearchEngine/Elasticsearch/QueryBuilder.t >@@ -19,6 +19,7 @@ use Modern::Perl; > > use C4::Context; > use Test::Exception; >+use Test::Warn; > use t::lib::Mocks; > use t::lib::TestBuilder; > use Test::More tests => 6; >@@ -47,6 +48,10 @@ $se->mock( 'get_elasticsearch_mappings', sub { > subject => { > type => 'text' > }, >+ 'subject-heading-thesaurus' => { >+ type => 'text', >+ facet => 1 >+ }, > itemnumber => { > type => 'integer' > }, >@@ -56,12 +61,24 @@ $se->mock( 'get_elasticsearch_mappings', sub { > sortablenumber__sort => { > type => 'integer' > }, >- Heading => { >+ heading => { > type => 'text' > }, >- Heading__sort => { >+ 'heading-main' => { > type => 'text' >- } >+ }, >+ heading__sort => { >+ type => 'text' >+ }, >+ match => { >+ type => 'text' >+ }, >+ 'match-heading' => { >+ type => 'text' >+ }, >+ 'match-heading-see-from' => { >+ type => 'text' >+ }, > } > } > }; >@@ -82,7 +99,7 @@ $se->mock( 'get_elasticsearch_mappings', sub { > }); > > subtest 'build_authorities_query_compat() tests' => sub { >- plan tests => 45; >+ plan tests => 47; > > my $qb; > >@@ -169,12 +186,23 @@ subtest 'build_authorities_query_compat() tests' => sub { > "authorities type code is used as filter" > ); > >- # Failing case >- throws_ok { >- $qb->build_authorities_query_compat( [ 'tomas' ], undef, undef, ['contains'], [$search_term], 'AUTH_TYPE', 'asc' ); >+ # Authorities marclist check >+ warning_like { >+ $query = $qb->build_authorities_query_compat( [ 'tomas','mainentry' ], undef, undef, ['contains'], [$search_term,$search_term], 'AUTH_TYPE', 'asc' ) > } >- 'Koha::Exceptions::WrongParameter', >- 'Exception thrown on invalid value in the marclist param'; >+ qr/Unknown search field tomas/, >+ "Warning for unknown field in marclist"; >+ is_deeply( >+ $query->{query}->{bool}->{must}[0]->{query_string}->{default_field}, >+ 'tomas', >+ "If no mapping for marclist the index is passed through as defined" >+ ); >+ is_deeply( >+ $query->{query}->{bool}->{must}[1]->{query_string}{default_field}, >+ 'heading', >+ "If mapping found for marclist the index is passed through converted" >+ ); >+ > }; > > subtest 'build_query tests' => sub { >-- >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 23719
:
93414
|
94680
|
94913
|
95001
|
95090
|
95091
| 99714