From d5e0529229167454cff156793294a142df1c3b14 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) Signed-off-by: Clemens Tubach --- 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 %]
  • [% 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 %]
      [% 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 { Order Search field Label - Display @@ -504,26 +503,20 @@ a.add, a.delete { [% f.name | html %] - [% 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 %] - - - [% IF f.facet_order %] - - [% ELSE %] - + [% 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 %] + [% END %] -- 2.30.2