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.
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

diff --git a/Koha/SearchEngine/Elasticsearch/QueryBuilder.pm b/Koha/SearchEngine/Elasticsearch/QueryBuilder.pm
index d0aa916..5946a27 100644 (file)
--- 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 d3cd294..ee38dae 100644 (file)
--- 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 b56e2b4..50582a8 100644 (file)
--- 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 2a77dad..f83bd8d 100644 (file)
--- 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;