From ef5307d254fc5ca51b3b4d6ab94828754483855f 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 Signed-off-by: Nick Clemens Signed-off-by: Katrin Fischer --- 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 c3c2d27e01..fc7ef4d789 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; @@ -1413,28 +1414,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 e8f28a9200..5c7c9b0d61 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' => '', @@ -226,27 +227,23 @@ 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 } }; + #} $res = _rebuild_to_es_advanced_query($res) if @$es_advanced_searches ; return $res; } diff --git a/Koha/SearchEngine/Elasticsearch/Search.pm b/Koha/SearchEngine/Elasticsearch/Search.pm index 12b90cd0ef..37b57d9f8c 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; @@ -441,28 +442,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. @@ -487,6 +468,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 4c840d5d14..4311ddddb0 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; @@ -235,7 +235,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 18baa39ad2..85095ec944 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 @@ -498,7 +498,6 @@ a.add, a.delete { Order Search field Label - Display @@ -509,26 +508,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.39.5