Bug 18068 - Elasticsearch (ES): Location and (home|holding)branch facets mixed
Summary: Elasticsearch (ES): Location and (home|holding)branch facets mixed
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Searching (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal (vote)
Assignee: Tomás Cohen Arazi
QA Contact: Jonathan Druart
URL:
Keywords:
Depends on:
Blocks: 17169 18322
  Show dependency treegraph
 
Reported: 2017-02-06 19:20 UTC by Tomás Cohen Arazi
Modified: 2017-12-07 22:20 UTC (History)
5 users (show)

See Also:
Change sponsored?: Sponsored
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 18068: ES - Fix location and (home|holding)branch facets (7.68 KB, patch)
2017-02-06 19:54 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 18068: ES - Fix location and (home|holding)branch facets (7.71 KB, patch)
2017-02-07 17:53 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 18068: ES - Fix location and (home|holding)branch facets (7.78 KB, patch)
2017-02-16 11:52 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Tomás Cohen Arazi 2017-02-06 19:20:08 UTC

    
Comment 1 Tomás Cohen Arazi 2017-02-06 19:21:37 UTC
To reproduce:
- Do a search on a DB with items having a homebranch set
=> FAIL: the 'Locations' facet shows branchcodes instead of the content from 952$c (MARC21)
Comment 2 Tomás Cohen Arazi 2017-02-06 19:54:11 UTC
Created attachment 59948 [details] [review]
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.
Comment 3 Nick Clemens 2017-02-07 17:53:13 UTC
Created attachment 59991 [details] [review]
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>
Comment 4 Jonathan Druart 2017-02-16 11:42:34 UTC
I do not see the "Holding branch" facet, even when mapped with 952$b (default mappings).
What did I miss?
Comment 5 Tomás Cohen Arazi 2017-02-16 11:44:09 UTC
(In reply to Jonathan Druart from comment #4)
> I do not see the "Holding branch" facet, even when mapped with 952$b
> (default mappings).
> What did I miss?

DisplayLibraryFacets?
Comment 6 Jonathan Druart 2017-02-16 11:49:46 UTC
(In reply to Jonathan Druart from comment #4)
> I do not see the "Holding branch" facet, even when mapped with 952$b
> (default mappings).
> What did I miss?

I forgot to reset the mappings...
Comment 7 Jonathan Druart 2017-02-16 11:52:42 UTC
Created attachment 60330 [details] [review]
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>
Comment 8 Kyle M Hall 2017-02-17 11:35:18 UTC
Pushed to master for 17.05, thanks Tomas!
Comment 9 Katrin Fischer 2017-02-19 20:15:39 UTC
This patch has been pushed to 16.11.x and will be in 16.11.04.
Comment 10 Mason James 2017-02-23 20:25:28 UTC
Pushed to 16.05.x, for 16.05.10 release