Attachment 148229 Details for Bug 33159 – Bug 33159: Simplify ES handling and fix zebra handling

[patch] Bug 33159: Simplify ES handling and fix zebra handling

Bug-33159-Simplify-ES-handling-and-fix-zebra-handl.patch (text/plain), 6.59 KB, created by Martin Renvoize (ashimema) on 2023-03-15 16:14:34 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Martin Renvoize (ashimema)

Created: 2023-03-15 16:14:34 UTC

Size: 6.59 KB

patch

obsolete

>From 01b96456d8ee8df6f80d4739f479a3caecd9f90a Mon Sep 17 00:00:00 2001
>From: Nick Clemens <nick@bywatersolutions.com>
>Date: Wed, 8 Mar 2023 18:03:05 +0000
>Subject: [PATCH] Bug 33159: Simplify ES handling and fix zebra handling
>
>Before this patch we used two indexes for the thesaurus values, we can
>simply index both needed fields into a single index and just form the
>search correctly.
>
>This patch also ensures we pass the 'thesaurus' vlaue for the heading
>directly to the query builder - for zebra it goes through, and for ES
>we convert it to the expected code.
>
>This patch also moves the necessary mappings out of the user definable
>mappings and hardcodes them. There is precedent for this with
>'match-heading', it ensures matching works as expected
>
>To test:
>1 - Follow previous test plan in Zebra and ES
>
>Signed-off-by: Phil Ringnalda <phil@chetcolibrary.org>
>Signed-off-by: Frank Hansen <frank.hansen@ub.lu.se>
>Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
>---
> C4/Heading.pm                                 | 25 +------------------
> Koha/SearchEngine/Elasticsearch.pm            | 10 +++++++-
> .../Elasticsearch/QueryBuilder.pm             | 12 +++++++++
> .../searchengine/elasticsearch/mappings.yaml  | 18 -------------
> t/db_dependent/Heading.t                      |  3 +--
> 5 files changed, 23 insertions(+), 45 deletions(-)
>
>diff --git a/C4/Heading.pm b/C4/Heading.pm
>index cce3abf6fc..8b8ece8aac 100644
>--- a/C4/Heading.pm
>+++ b/C4/Heading.pm
>@@ -210,34 +210,11 @@ sub _search {
>     }
> 
>     if ( $thesaurus ) {
>-    # This is calculated in C4/Heading/MARC21.pm - not used for UNIMARC
>-        if ($thesaurus eq 'lcsh') {
>-            $subject_heading_thesaurus = 'a';
>-        } elsif ($thesaurus eq 'lcac') {
>-            $subject_heading_thesaurus = 'b';
>-        } elsif ($thesaurus eq 'mesh') {
>-            $subject_heading_thesaurus = 'c';
>-        } elsif ($thesaurus eq 'nal') {
>-            $subject_heading_thesaurus = 'd';
>-        } elsif ($thesaurus eq 'notspecified') {
>-            $subject_heading_thesaurus = 'n';
>-        } elsif ($thesaurus eq 'cash') {
>-            $subject_heading_thesaurus = 'k';
>-        } elsif ($thesaurus eq 'rvm') {
>-            $subject_heading_thesaurus = 'v';
>-        } else { # We stored the value from $7 as the thesaurus if there was one
>-            $subject_heading_thesaurus = 'z';
>-            push @marclist, 'thesaurus-conventions';
>-            push @and_or, 'and';
>-            push @excluding, '';
>-            push @operator, 'is';
>-            push @value, $self->{'thesaurus'};
>-        }
>         push @marclist, 'thesaurus';
>         push @and_or, 'and';
>         push @excluding, '';
>         push @operator, 'is';
>-        push @value, $subject_heading_thesaurus;
>+        push @value, $thesaurus;
>     }
> 
>     require Koha::SearchEngine::QueryBuilder;
>diff --git a/Koha/SearchEngine/Elasticsearch.pm b/Koha/SearchEngine/Elasticsearch.pm
>index 1fe6fb1c6f..9463aed1d3 100644
>--- a/Koha/SearchEngine/Elasticsearch.pm
>+++ b/Koha/SearchEngine/Elasticsearch.pm
>@@ -233,7 +233,10 @@ sub get_elasticsearch_mappings {
>                 }
>             }
>         );
>-        $mappings->{properties}{ 'match-heading' } = _get_elasticsearch_field_config('search', 'text') if $self->index eq 'authorities';
>+        if( $self->index eq 'authorities' ){
>+            $mappings->{properties}{ 'match-heading' } = _get_elasticsearch_field_config('search', 'text');
>+            $mappings->{properties}{ 'subject-heading-thesaurus' } = _get_elasticsearch_field_config('search', 'text');
>+        }
>         $all_mappings{$self->index} = $mappings;
>     }
>     $self->sort_fields(\%{$sort_fields{$self->index}});
>@@ -1166,6 +1169,11 @@ sub _get_marc_mapping_rules {
>         }
>     }
> 
>+    if( $self->index eq 'authorities' ){
>+        push @{$rules->{control_fields}->{'008'}}, ['subject-heading-thesaurus', { 'substr' => [ 11, 1 ] } ];
>+        push @{$rules->{data_fields}->{'040'}->{subfields}->{f}}, ['subject-heading-thesaurus', { } ];
>+    }
>+
>     return $rules;
> }
> 
>diff --git a/Koha/SearchEngine/Elasticsearch/QueryBuilder.pm b/Koha/SearchEngine/Elasticsearch/QueryBuilder.pm
>index 53042b4303..d5dbdee886 100644
>--- a/Koha/SearchEngine/Elasticsearch/QueryBuilder.pm
>+++ b/Koha/SearchEngine/Elasticsearch/QueryBuilder.pm
>@@ -564,6 +564,16 @@ our $koha_to_index_name = {
>     all             => ''
> };
> 
>+our $thesaurus_to_value = {
>+   lcsh => 'a',
>+   lcac => 'b',
>+   mesh => 'c',
>+   nal  => 'd',
>+   notspecified => 'n',
>+   cash => 'k',
>+   rvm => 'v',
>+};
>+
> sub build_authorities_query_compat {
>     my ( $self, $marclist, $and_or, $excluding, $operator, $value,
>         $authtypecode, $orderby )
>@@ -588,6 +598,8 @@ sub build_authorities_query_compat {
>     }
>     for ( my $i = 0 ; $i < @$value ; $i++ ) {
>         next unless $value->[$i]; #clean empty form values, ES doesn't like undefined searches
>+        $value->[$i] = $thesaurus_to_value->{ $value->[$i] }
>+            if( defined $thesaurus_to_value->{ $value->[$i] } && $indexes[$i] eq 'subject-heading-thesaurus' );
>         push @searches,
>           {
>             where    => $indexes[$i],
>diff --git a/admin/searchengine/elasticsearch/mappings.yaml b/admin/searchengine/elasticsearch/mappings.yaml
>index e892f9126c..2644bea859 100644
>--- a/admin/searchengine/elasticsearch/mappings.yaml
>+++ b/admin/searchengine/elasticsearch/mappings.yaml
>@@ -805,24 +805,6 @@ authorities:
>         sort: ~
>         suggestible: ''
>     type: ''
>-  Subject-heading-thesaurus:
>-    label: Subject-heading-thesaurus
>-    mappings:
>-      - facet: ''
>-        marc_field: 008_/11
>-        marc_type: marc21
>-        sort: ~
>-        suggestible: ''
>-    type: ''
>-  Subject-heading-thesaurus-conventions:
>-    label: Subject-heading-thesaurus-conventions
>-    mappings:
>-      - facet: ''
>-        marc_field: 040f
>-        marc_type: marc21
>-        sort: ~
>-        suggestible: ''
>-    type: ''
>   authtype:
>     label: authtype
>     mappings:
>diff --git a/t/db_dependent/Heading.t b/t/db_dependent/Heading.t
>index 6b47065573..6c05101635 100755
>--- a/t/db_dependent/Heading.t
>+++ b/t/db_dependent/Heading.t
>@@ -104,8 +104,7 @@ subtest "_search tests" => sub {
>     $terms = $search_query->{query}->{bool}->{must};
>     $expected_terms = [
>         { term => { 'match-heading.ci_raw' => 'Uncles generalsubdiv Fiction' } },
>-        { term => { 'subject-heading-thesaurus-conventions.ci_raw' => 'special_sauce' } },
>-        { term => { 'subject-heading-thesaurus.ci_raw' => 'z' } },
>+        { term => { 'subject-heading-thesaurus.ci_raw' => 'special_sauce' } },
>     ];
>     is_deeply( $terms, $expected_terms, "Search formed as expected with second indicator 7 and subfield 2");
> 
>-- 
>2.39.2

Actions: View | Diff | Splinter Review

Attachments on bug 33159: 147881 | 147882 | 147970 | 147980 | 147986 | 147987 | 147988 | 148032 | 148033 | 148034 | 148050 | 148051 | 148052 | 148227 | 148228 | 148229 | 148414 | 148415 | 148416