From 7e261ecba8341c9491c3be7dc0537cfd0395a1f7 Mon Sep 17 00:00:00 2001 From: Jonathan Druart 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) --- 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 248856e42b2..ea70377373f 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 3f26cbc0274..cb0be0e86e1 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 35ab4b7f91d..e91f02f91df 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 933b52f79dd..4a26918b668 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 9d0ae806e20..735baf16f7f 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 2fcce67e7d2..6bb127fa729 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 %]
  • [% 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 %]