Attachment 163896 Details for Bug 35138 – Bug 35138: Add the ability to manage ES facets

[patch] Bug 35138: Add the ability to manage ES facets

Bug-35138-Add-the-ability-to-manage-ES-facets.patch (text/plain), 18.18 KB, created by Clemens Tubach on 2024-03-26 10:51:02 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Clemens Tubach

Created: 2024-03-26 10:51:02 UTC

Size: 18.18 KB

patch

obsolete

>From d5e0529229167454cff156793294a142df1c3b14 Mon Sep 17 00:00:00 2001
>From: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
>Date: Thu, 14 Mar 2024 11:41:42 +0100
>Subject: [PATCH] Bug 35138: Add the ability to manage ES facets
>
>This new feature allows to manage facets for Elasticsearch from the
>administration page.
>Prior to this the facet fields were hardcoded in the codebase.
>
>Librarians can then add/remove facet fields and modify their label.
>
>Test plan:
>1. Create a new search field and set it a label different than its name.
>2. Save
>3. Go the bibliographic mapping tab
>4. Add 1+ mapping for this search field (Searchable and facetable must be "Yes")
>5. Add, reorder, remove new facets
>6. Save and reindex your records
>7. Search and notice the new facet list
>
>QA: There are several wrong things in this area (ES + facets code,
>everything, pm, pl, tt AND on this administration page). I have done my
>best to clean the code as much as I could and keep the code cleaner
>after than before. But there are still a lot to do.
>There are still inconsistencies on this page (like we need to save to
>see the changes applied to the other tables), but this is clearly out of
>the scope of this bug report.
>Another enhancement would be to move the facet list to a different DB
>table, that could bring more flexibility:
> * display or not (could be opac/staff/both/none)
> * define the size per field (number of facet to display)
> * order: move search_field.facet_order to this new table.
>But, again, it's a lot more work.
>
>More work is done in this area, please see related bug reports.
>
>Sponsored-by: The Research University in the Helmholtz Association (KIT)
>Signed-off-by: Clemens Tubach <clemens.tubach@kit.edu>
>---
> C4/Search.pm | 1 +
> Koha/SearchEngine/Elasticsearch.pm | 21 ++++-------
> .../Elasticsearch/QueryBuilder.pm | 33 ++++++++---------
> Koha/SearchEngine/Elasticsearch/Search.pm | 26 +++-----------
> admin/searchengine/elasticsearch/mappings.pl | 6 ++--
> .../intranet-tmpl/prog/en/includes/facets.inc | 35 ++++++++-----------
> .../searchengine/elasticsearch/mappings.tt | 33 +++++++----------
> 7 files changed, 56 insertions(+), 99 deletions(-)
>
>diff --git a/C4/Search.pm b/C4/Search.pm
>index 248856e42b..ea70377373 100644
>--- a/C4/Search.pm
>+++ b/C4/Search.pm
>@@ -579,6 +579,7 @@ sub getRecords {
> "type_label_"
> . $facets_info->{$link_value}->{'label_value'} =>
> 1,
>+ label => $facets_info->{$link_value}->{'label_value'},
> facets => \@this_facets_array,
> }
> unless (
>diff --git a/Koha/SearchEngine/Elasticsearch.pm b/Koha/SearchEngine/Elasticsearch.pm
>index 3f26cbc027..cb0be0e86e 100644
>--- a/Koha/SearchEngine/Elasticsearch.pm
>+++ b/Koha/SearchEngine/Elasticsearch.pm
>@@ -28,6 +28,7 @@ use Koha::Filter::MARC::EmbedSeeFromHeadings;
> use Koha::SearchFields;
> use Koha::SearchMarcMaps;
> use Koha::Caches;
>+use Koha::AuthorisedValueCategories;
> use C4::Heading;
> use C4::AuthoritiesMarc qw( GuessAuthTypeCode );
> use C4::Biblio;
>@@ -1391,28 +1392,18 @@ sub _read_configuration {
> return $configuration;
> }
> 
>-=head2 get_facetable_fields
>+=head2 get_facet_fields
> 
>-my @facetable_fields = Koha::SearchEngine::Elasticsearch->get_facetable_fields();
>+my @facet_fields = Koha::SearchEngine::Elasticsearch->get_facet_fields();
> 
>-Returns the list of Koha::SearchFields marked to be faceted in the ES configuration
>+Returns the list of Koha::SearchFields marked to be faceted.
> 
> =cut
> 
>-sub get_facetable_fields {
>+sub get_facet_fields {
> my ($self) = @_;
> 
>- # These should correspond to the ES field names, as opposed to the CCL
>- # things that zebra uses.
>- my @search_field_names = qw( author itype location su-geo title-series subject ccode holdingbranch homebranch ln );
>- my @faceted_fields = Koha::SearchFields->search(
>- { name => { -in => \@search_field_names }, facet_order => { '!=' => undef } }, { order_by => ['facet_order'] }
>- )->as_list;
>- my @not_faceted_fields = Koha::SearchFields->search(
>- { name => { -in => \@search_field_names }, facet_order => undef }, { order_by => ['facet_order'] }
>- )->as_list;
>- # This could certainly be improved
>- return ( @faceted_fields, @not_faceted_fields );
>+ return Koha::SearchFields->search( { facet_order => { '!=' => undef } }, { order_by => ['facet_order'] } )->as_list;
> }
> 
> =head2 clear_search_fields_cache
>diff --git a/Koha/SearchEngine/Elasticsearch/QueryBuilder.pm b/Koha/SearchEngine/Elasticsearch/QueryBuilder.pm
>index 35ab4b7f91..e91f02f91d 100644
>--- a/Koha/SearchEngine/Elasticsearch/QueryBuilder.pm
>+++ b/Koha/SearchEngine/Elasticsearch/QueryBuilder.pm
>@@ -48,6 +48,7 @@ use URI::Escape qw( uri_escape_utf8 );
> use C4::Context;
> use Koha::Exceptions;
> use Koha::Caches;
>+use Koha::AuthorisedValueCategories;
> 
> our %index_field_convert = (
> 'kw' => '',
>@@ -225,26 +226,22 @@ sub build_query {
> # See _convert_facets in Search.pm for how these get turned into
> # things that Koha can use.
> my $size = C4::Context->preference('FacetMaxCount');
>- $res->{aggregations} = {
>- author => { terms => { field => "author__facet" , size => $size } },
>- subject => { terms => { field => "subject__facet", size => $size } },
>- itype => { terms => { field => "itype__facet", size => $size} },
>- location => { terms => { field => "location__facet", size => $size } },
>- 'su-geo' => { terms => { field => "su-geo__facet", size => $size} },
>- 'title-series' => { terms => { field => "title-series__facet", size => $size } },
>- ccode => { terms => { field => "ccode__facet", size => $size } },
>- ln => { terms => { field => "ln__facet", size => $size } },
>+ my @facets = Koha::SearchEngine::Elasticsearch->get_facet_fields;
>+ for my $f ( @facets ) {
>+ my $name = $f->name;
>+ $res->{aggregations}->{$name} = { terms => { field => "${name}__facet" , size => $size } };
> };
> 
>- my $display_library_facets = C4::Context->preference('DisplayLibraryFacets');
>- if ( $display_library_facets eq 'both'
>- or $display_library_facets eq 'home' ) {
>- $res->{aggregations}{homebranch} = { terms => { field => "homebranch__facet", size => $size } };
>- }
>- if ( $display_library_facets eq 'both'
>- or $display_library_facets eq 'holding' ) {
>- $res->{aggregations}{holdingbranch} = { terms => { field => "holdingbranch__facet", size => $size } };
>- }
>+ # FIXME We need a way to show/hide the facet individually
>+ #my $display_library_facets = C4::Context->preference('DisplayLibraryFacets');
>+ #if ( $display_library_facets eq 'both'
>+ # or $display_library_facets eq 'home' ) {
>+ # $res->{aggregations}{homebranch} = { terms => { field => "homebranch__facet", size => $size } };
>+ #}
>+ #if ( $display_library_facets eq 'both'
>+ # or $display_library_facets eq 'holding' ) {
>+ # $res->{aggregations}{holdingbranch} = { terms => { field => "holdingbranch__facet", size => $size } };
>+ #}
> return $res;
> }
> 
>diff --git a/Koha/SearchEngine/Elasticsearch/Search.pm b/Koha/SearchEngine/Elasticsearch/Search.pm
>index 933b52f79d..4a26918b66 100644
>--- a/Koha/SearchEngine/Elasticsearch/Search.pm
>+++ b/Koha/SearchEngine/Elasticsearch/Search.pm
>@@ -45,6 +45,7 @@ use C4::Context;
> use C4::AuthoritiesMarc;
> use Koha::ItemTypes;
> use Koha::AuthorisedValues;
>+use Koha::AuthorisedValueCategories;
> use Koha::SearchEngine::QueryBuilder;
> use Koha::SearchEngine::Search;
> use Koha::Exceptions::Elasticsearch;
>@@ -440,28 +441,8 @@ sub _convert_facets {
> 
> return if !$es;
> 
>- # These should correspond to the ES field names, as opposed to the CCL
>- # things that zebra uses.
>- my %type_to_label;
>- my %label = (
>- author => 'Authors',
>- itype => 'ItemTypes',
>- location => 'Location',
>- 'su-geo' => 'Places',
>- 'title-series' => 'Series',
>- subject => 'Topics',
>- ccode => 'CollectionCodes',
>- holdingbranch => 'HoldingLibrary',
>- homebranch => 'HomeLibrary',
>- ln => 'Language',
>- );
>- my @facetable_fields =
>- Koha::SearchEngine::Elasticsearch->get_facetable_fields;
>- for my $f (@facetable_fields) {
>- next unless defined $f->facet_order;
>- $type_to_label{ $f->name } =
>- { order => $f->facet_order, label => $label{ $f->name } };
>- }
>+ my %type_to_label = map { $_->name => { order => $_->facet_order, label => $_->label } }
>+ Koha::SearchEngine::Elasticsearch->get_facet_fields;
> 
> # We also have some special cases, e.g. itypes that need to show the
> # value rather than the code.
>@@ -486,6 +467,7 @@ sub _convert_facets {
> my $facet = {
> type_id => $type . '_id',
> "type_label_$type_to_label{$type}{label}" => 1,
>+ label => $type_to_label{$type}{label},
> type_link_value => $type,
> order => $type_to_label{$type}{order},
> };
>diff --git a/admin/searchengine/elasticsearch/mappings.pl b/admin/searchengine/elasticsearch/mappings.pl
>index 9d0ae806e2..735baf16f7 100755
>--- a/admin/searchengine/elasticsearch/mappings.pl
>+++ b/admin/searchengine/elasticsearch/mappings.pl
>@@ -105,7 +105,7 @@ if ( $op eq 'cud-edit' ) {
> my @mapping_search = $input->multi_param('mapping_search');
> my @mapping_filter = $input->multi_param('mapping_filter');
> my @mapping_marc_field = $input->multi_param('mapping_marc_field');
>- my @faceted_field_names = $input->multi_param('display_facet');
>+ my @faceted_field_names = $input->multi_param('facet_name');
> 
> eval {
> 
>@@ -143,7 +143,7 @@ if ( $op eq 'cud-edit' ) {
> }
> 
> Koha::SearchMarcMaps->search( { marc_type => $marc_type, } )->delete;
>- my @facetable_fields = Koha::SearchEngine::Elasticsearch->get_facetable_fields();
>+ my @facetable_fields = Koha::SearchEngine::Elasticsearch->get_facet_fields();
> my @facetable_field_names = map { $_->name } @facetable_fields;
> 
> my $mandatory_before = Koha::SearchFields->search({mandatory=>1})->count;
>@@ -232,7 +232,7 @@ for my $index_name (@index_names) {
> }
> }
> 
>-my @facetable_fields = Koha::SearchEngine::Elasticsearch->get_facetable_fields();
>+my @facetable_fields = Koha::SearchEngine::Elasticsearch->get_facet_fields();
> for my $index_name (@index_names) {
> my $search_fields = Koha::SearchFields->search(
> {
>diff --git a/koha-tmpl/intranet-tmpl/prog/en/includes/facets.inc b/koha-tmpl/intranet-tmpl/prog/en/includes/facets.inc
>index 2fcce67e7d..6bb127fa72 100644
>--- a/koha-tmpl/intranet-tmpl/prog/en/includes/facets.inc
>+++ b/koha-tmpl/intranet-tmpl/prog/en/includes/facets.inc
>@@ -51,38 +51,31 @@
> [% IF facets_loo.facets.size > 0 %]
> <li id="[% facets_loo.type_id | html %]">
> [% facets_loo.type_label | html %]
>- [% IF facets_loo.type_label_Authors %]
>+ [% SWITCH facets_loo.label %]
>+ [% CASE 'Authors' %]
> Authors
>- [% END %]
>- [% IF facets_loo.type_label_Titles %]
>+ [% CASE 'Titles' %]
> Titles
>- [% END %]
>- [% IF facets_loo.type_label_Topics %]
>+ [% CASE 'Topics' %]
> Topics
>- [% END %]
>- [% IF facets_loo.type_label_Places %]
>+ [% CASE 'Places' %]
> Places
>- [% END %]
>- [% IF facets_loo.type_label_Series %]
>+ [% CASE 'Series' %]
> Series
>- [% END %]
>- [% IF facets_loo.type_label_ItemTypes %]
>+ [% CASE 'ItemTypes' %]
> Item types
>- [% END %]
>- [% IF ( facets_loo.type_label_HomeLibrary ) %]
>+ [% CASE 'HomeLibrary' %]
> Home libraries
>- [% END %]
>- [% IF ( facets_loo.type_label_HoldingLibrary ) %]
>+ [% CASE 'HoldingLibrary' %]
> Holding libraries
>- [% END %]
>- [% IF facets_loo.type_label_Location %]
>+ [% CASE 'Location' %]
> Locations
>- [% END %]
>- [% IF facets_loo.type_label_CollectionCodes %]
>+ [% CASE 'CollectionCodes' %]
> Collections
>- [% END %]
>- [% IF facets_loo.type_label_Language %]
>+ [% CASE 'Languages' %]
> Languages
>+ [% CASE %]
>+ [% facets_loo.label | html %]
> [% END %]
> <ul>
> [% SET url = "/cgi-bin/koha/catalogue/search.pl?" _ query_cgi _ limit_cgi %]
>diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/searchengine/elasticsearch/mappings.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/searchengine/elasticsearch/mappings.tt
>index 9d0022a261..a3513d0d2a 100644
>--- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/searchengine/elasticsearch/mappings.tt
>+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/searchengine/elasticsearch/mappings.tt
>@@ -493,7 +493,6 @@ a.add, a.delete {
> <th>Order</th>
> <th class="NoSort">Search field</th>
> <th class="NoSort">Label</th>
>- <th class="NoSort">Display</th>
> </tr>
> </thead>
> <tbody>
>@@ -504,26 +503,20 @@ a.add, a.delete {
> [% f.name | html %]
> </td>
> <td>
>- [% SWITCH f.name %]
>- [% CASE 'author' %]Authors
>- [% CASE 'itype' %]Item types
>- [% CASE 'location' %]Locations
>- [% CASE 'su-geo' %]Places
>- [% CASE 'title-series' %]Series
>- [% CASE 'subject' %]Topics
>- [% CASE 'ccode' %]Collections
>- [% CASE 'holdingbranch' %]Holding libraries
>- [% CASE 'homebranch' %]Home libraries
>- [% CASE 'ln' %]Language
>- [% CASE %][% f | html %]
>- [% END %]
>- </td>
>- <td>
>- [% IF f.facet_order %]
>- <input type="checkbox" name="display_facet" value="[% f.name | html %]" checked="checked" />
>- [% ELSE %]
>- <input type="checkbox" name="display_facet" value="[% f.name | html %]" />
>+ [% SWITCH f.label %]
>+ [% CASE 'Authors' %]Authors
>+ [% CASE 'Item types' %]Item types
>+ [% CASE 'Locations' %]Locations
>+ [% CASE 'Places' %]Places
>+ [% CASE 'Series' %]Series
>+ [% CASE 'Topics' %]Topics
>+ [% CASE 'Collections' %]Collections
>+ [% CASE 'Holding libraries' %]Holding libraries
>+ [% CASE 'Home libraries' %]Home libraries
>+ [% CASE 'Languages' %]Languages
>+ [% CASE %][% f.label | html %]
> [% END %]
>+ <input type="hidden" name="facet_name" value="[% f.name | html %]" />
> </td>
> </tr>
> [% END %]
>-- 
>2.30.2

Actions: View | Diff | Splinter Review

Attachments on bug 35138: 163133 | 163680 | 163681 | 163682 | 163683 | 163684 | 163896 | 163897 | 163898 | 163899 | 163900 | 164811 | 164812 | 164813 | 164814 | 164815 | 164816 | 165811 | 165812 | 165813 | 165814 | 165815 | 165816 | 165817 | 165818 | 166402