From 78d727b8fbd4e3e688531caa446aaa82db297577 Mon Sep 17 00:00:00 2001 From: Tomas Cohen Arazi Date: Mon, 6 Feb 2017 16:22:51 -0300 Subject: [PATCH] Bug 18068: ES - Fix location and (home|holding)branch facets This patch makes the 'Locations' facet work as expected (i.e. having the same behaviour it has for Zebra: picking the 952$c in MARC21 and 995e for UNIMARC). It also adds the code to handle holding and home library settings for facets and makes the facets show the library name instead of the branch code. The mappings are updated so the labels match what facets.inc expect to work properly. To test: - On master, do a search that returns biblios with items having homebranch set. => FAIL: Under the 'Locations' label on the facets you will notice branchcodes are shown. - Apply the patch - Restart memcached and plack (just in case, it was tricky) - Reset your mappings: http://localhost:8081/cgi-bin/koha/admin/searchengine/elasticsearch/mappings.pl?op=reset&i_know_what_i_am_doing=1 - Restart memcached and plack (again, not sure if needed) - Make sure this mappings are set: homebranch => HomeLibrary holdingbranch => HoldingLibrary (Note: it might not be set due to the place the yaml file is being picked) - Reindex your records: $ sudo koha-shell kohadev k$ cd kohaclone k$ perl misc/search_tools/rebuild_elastic_search.pl -d -v - Repeat the initial search => SUCCESS: 'Location' contains the right stuff, 'Home libraries' and 'Holding libraries' too. - Run k$ prove t/db_dependent/Koha_SearchEngine_Elasticsearch_Search.t => SUCCESS: Tests pass! - Sign off :-D Note: play with the 'DisplayLibraryFacets' syspref options. --- Koha/SearchEngine/Elasticsearch/QueryBuilder.pm | 12 +++++++++- Koha/SearchEngine/Elasticsearch/Search.pm | 6 +++++ admin/searchengine/elasticsearch/mappings.yaml | 27 ++++++++++++++++++---- .../Koha_SearchEngine_Elasticsearch_Search.t | 26 ++++++++++++++++++++- 4 files changed, 65 insertions(+), 6 deletions(-) diff --git a/Koha/SearchEngine/Elasticsearch/QueryBuilder.pm b/Koha/SearchEngine/Elasticsearch/QueryBuilder.pm index d0aa916c23..5946a272ac 100644 --- a/Koha/SearchEngine/Elasticsearch/QueryBuilder.pm +++ b/Koha/SearchEngine/Elasticsearch/QueryBuilder.pm @@ -115,10 +115,20 @@ sub build_query { author => { terms => { field => "author__facet" } }, subject => { terms => { field => "subject__facet" } }, itype => { terms => { field => "itype__facet" } }, - location => { terms => { field => "homebranch__facet" } }, + location => { terms => { field => "location__facet" } }, 'su-geo' => { terms => { field => "su-geo__facet" } }, se => { terms => { field => "se__facet" } }, }; + + 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" } }; + } + if ( $display_library_facets eq 'both' + or $display_library_facets eq 'holding' ) { + $res->{aggregations}{holdingbranch} = { terms => { field => "holdingbranch__facet" } }; + } if ( my $ef = $options{expanded_facet} ) { $res->{aggregations}{$ef}{terms}{size} = C4::Context->preference('FacetMaxCount'); }; diff --git a/Koha/SearchEngine/Elasticsearch/Search.pm b/Koha/SearchEngine/Elasticsearch/Search.pm index d3cd29460d..ee38daec17 100644 --- a/Koha/SearchEngine/Elasticsearch/Search.pm +++ b/Koha/SearchEngine/Elasticsearch/Search.pm @@ -408,16 +408,22 @@ sub _convert_facets { 'su-geo' => { order => 4, label => 'Places', }, se => { order => 5, label => 'Series', }, subject => { order => 6, label => 'Topics', }, + holdingbranch => { order => 8, label => 'HoldingLibrary' }, + homebranch => { order => 9, label => 'HomeLibrary' } ); # We also have some special cases, e.g. itypes that need to show the # value rather than the code. my @itypes = Koha::ItemTypes->search; + my @libraries = Koha::Libraries->search; + my $library_names = { map { $_->branchcode => $_->branchname } @libraries }; my @locations = Koha::AuthorisedValues->search( { category => 'LOC' } ); my $opac = C4::Context->interface eq 'opac' ; my %special = ( itype => { map { $_->itemtype => $_->description } @itypes }, location => { map { $_->authorised_value => ( $opac ? ( $_->lib_opac || $_->lib ) : $_->lib ) } @locations }, + holdingbranch => $library_names, + homebranch => $library_names ); my @facets; $exp_facet //= ''; diff --git a/admin/searchengine/elasticsearch/mappings.yaml b/admin/searchengine/elasticsearch/mappings.yaml index b56e2b438d..50582a8b4e 100644 --- a/admin/searchengine/elasticsearch/mappings.yaml +++ b/admin/searchengine/elasticsearch/mappings.yaml @@ -1425,14 +1425,14 @@ biblios: suggestible: '' type: '' holdingbranch: - label: holdingbranch + label: HoldingLibrary mappings: - - facet: '' + - facet: '1' marc_field: 952b marc_type: marc21 sort: ~ suggestible: '' - - facet: '' + - facet: '1' marc_field: 952b marc_type: normarc sort: ~ @@ -1444,7 +1444,7 @@ biblios: suggestible: '' type: string homebranch: - label: homebranch + label: HomeLibrary mappings: - facet: '1' marc_field: 952a @@ -1736,6 +1736,25 @@ biblios: sort: ~ suggestible: '1' type: '' + location: + label: Location + mappings: + - facet: '1' + marc_field: 952c + marc_type: marc21 + sort: ~ + suggestible: '' + - facet: '1' + marc_field: 952c + marc_type: normarc + sort: ~ + suggestible: '' + - facet: '1' + marc_field: 995e + marc_type: unimarc + sort: ~ + suggestible: '' + type: '' material-type: label: material-type mappings: diff --git a/t/db_dependent/Koha_SearchEngine_Elasticsearch_Search.t b/t/db_dependent/Koha_SearchEngine_Elasticsearch_Search.t index 2a77dad1c1..f83bd8d11c 100644 --- a/t/db_dependent/Koha_SearchEngine_Elasticsearch_Search.t +++ b/t/db_dependent/Koha_SearchEngine_Elasticsearch_Search.t @@ -17,7 +17,8 @@ use Modern::Perl; -use Test::More tests => 11; +use Test::More tests => 12; +use t::lib::Mocks; use Koha::SearchEngine::Elasticsearch::QueryBuilder; @@ -79,4 +80,27 @@ subtest 'json2marc' => sub { }; +subtest 'build_query tests' => sub { + plan tests => 6; + + t::lib::Mocks::mock_preference('DisplayLibraryFacets','both'); + my $query = $builder->build_query(); + ok( defined $query->{aggregations}{homebranch}, + 'homebranch added to facets if DisplayLibraryFacets=both' ); + ok( defined $query->{aggregations}{holdingbranch}, + 'holdingbranch added to facets if DisplayLibraryFacets=both' ); + t::lib::Mocks::mock_preference('DisplayLibraryFacets','holding'); + $query = $builder->build_query(); + ok( !defined $query->{aggregations}{homebranch}, + 'homebranch not added to facets if DisplayLibraryFacets=holding' ); + ok( defined $query->{aggregations}{holdingbranch}, + 'holdingbranch added to facets if DisplayLibraryFacets=holding' ); + t::lib::Mocks::mock_preference('DisplayLibraryFacets','home'); + $query = $builder->build_query(); + ok( defined $query->{aggregations}{homebranch}, + 'homebranch added to facets if DisplayLibraryFacets=home' ); + ok( !defined $query->{aggregations}{holdingbranch}, + 'holdingbranch not added to facets if DisplayLibraryFacets=home' ); +}; + 1; -- 2.11.1