Bug 18235 - Elastic search - Configurable facets
Summary: Elastic search - Configurable facets
Status: Signed Off
Alias: None
Product: Koha
Classification: Unclassified
Component: Searching - Elasticsearch (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Jonathan Druart
QA Contact: Marcel de Rooy
URL:
Whiteboard:
Keywords:
Depends on: 14899 17169 18174
Blocks: 18213
  Show dependency treegraph
 
Reported: 2017-03-08 18:57 UTC by Jonathan Druart
Modified: 2018-10-22 16:37 UTC (History)
10 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:


Attachments
Bug 18235: DBIx::Class schema changes (1.77 KB, patch)
2017-03-08 19:07 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18235: DB changes (2.11 KB, patch)
2017-03-08 19:07 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18235: ES - Facets configurable (16.20 KB, patch)
2017-03-08 19:07 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18235: DBIx::Class schema changes (1.83 KB, patch)
2017-03-23 12:11 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 18235: DB changes (2.17 KB, patch)
2017-03-23 12:11 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 18235: ES - Facets configurable (16.25 KB, patch)
2017-03-23 12:11 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 18235: DBIx schema changes (1.53 KB, patch)
2017-06-30 08:45 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 18235: DB changes (2.18 KB, patch)
2017-12-04 18:30 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18235: ES - Facets configurable (16.30 KB, patch)
2017-12-04 18:30 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18235: DBIx schema changes (1.50 KB, patch)
2017-12-04 18:30 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18235: DB changes (2.18 KB, patch)
2018-03-15 11:56 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 18235: ES - Facets configurable (16.42 KB, patch)
2018-03-15 11:56 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 18235: DBIx schema changes (1.49 KB, patch)
2018-03-15 11:56 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 18235: DB changes (2.24 KB, patch)
2018-03-16 09:37 UTC, Nicolas Legrand
Details | Diff | Splinter Review
Bug 18235: ES - Facets configurable (16.31 KB, patch)
2018-03-16 09:37 UTC, Nicolas Legrand
Details | Diff | Splinter Review
Bug 18235: DBIx schema changes (1.55 KB, patch)
2018-03-16 09:37 UTC, Nicolas Legrand
Details | Diff | Splinter Review
Bug 18235 - Add unit tests for get_facetable_fields (4.19 KB, patch)
2018-03-23 15:56 UTC, Alex Arnaud
Details | Diff | Splinter Review
Bug 18235: Add unit tests for get_facetable_fields (3.74 KB, patch)
2018-04-20 13:45 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18235: Add missing POD to get_facetable_fields (864 bytes, patch)
2018-04-20 13:45 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18235: DB changes (2.39 KB, patch)
2018-06-12 14:38 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18235: Add unit tests for get_facetable_fields (3.72 KB, patch)
2018-06-12 14:39 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 Jonathan Druart 2017-03-08 18:57:38 UTC
From the Elastic mappings configuration page, the librarians should be able to manage facets (display/hide and order).
Comment 1 Jonathan Druart 2017-03-08 19:07:34 UTC
Created attachment 60931 [details] [review]
Bug 18235: DBIx::Class schema changes
Comment 2 Jonathan Druart 2017-03-08 19:07:39 UTC
Created attachment 60932 [details] [review]
Bug 18235: DB changes
Comment 3 Jonathan Druart 2017-03-08 19:07:46 UTC
Created attachment 60933 [details] [review]
Bug 18235: ES - Facets configurable

This patch adds a new section 'Facet order' in the Biblio tab of the
'Search engine configuration' admin page of the Elastic mappings.
The idea is to let the librarians define the facet to display and order
them as their needs.

The ergonomic is not perfect and I am open to any suggestions.

Test plan:
Move up and down the field list to order the facets
Hide/show some facets
At the OPAC and the staff interface you should see the changes on the
search result page.
Comment 4 Nick Clemens 2017-03-23 12:11:28 UTC
Created attachment 61528 [details] [review]
Bug 18235: DBIx::Class schema changes

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 5 Nick Clemens 2017-03-23 12:11:41 UTC
Created attachment 61529 [details] [review]
Bug 18235: DB changes

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 6 Nick Clemens 2017-03-23 12:11:45 UTC
Created attachment 61530 [details] [review]
Bug 18235: ES - Facets configurable

This patch adds a new section 'Facet order' in the Biblio tab of the
'Search engine configuration' admin page of the Elastic mappings.
The idea is to let the librarians define the facet to display and order
them as their needs.

The ergonomic is not perfect and I am open to any suggestions.

Test plan:
Move up and down the field list to order the facets
Hide/show some facets
At the OPAC and the staff interface you should see the changes on the
search result page.

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 7 Nick Clemens 2017-03-23 12:14:34 UTC
I like this alot - many libraries want to control the facets a bit more and this is helpful.

Still a few tings that would be nice (but I am not suggesting they are needed in this patchset)
-labels shouldn't be hardcoded - staff shoul eb able to set them
-we should add more options to what can be faceted- could be out of control but technically any field should be able to be used
-ES has better handling for faceting (can provide subsets of existing/current results and allow to step in/out of facets)

FYI not complaints
Comment 8 Jonathan Druart 2017-03-23 14:35:53 UTC
Nick, yes you are right of course. This is only the first step.

However there are two problems:
1. Translatability: if staff changes or add new labels, they will not be translated (we could cover some in the template)
2. Shared includes files: What I said for 2 years now... We are using the same files for elastic and Zebra, so we have to stick to ugly structure the template expects. That's very bad, we need to create separate files to allow more flexibility.
Comment 9 Jonathan Druart 2017-06-16 15:27:17 UTC
Marcel, They apply correctly, just skip the first patch and regenerate the schema files.
Comment 10 Marcel de Rooy 2017-06-30 08:44:50 UTC
If I select say Author and set them all to Facetable No in the Search fields list on the Biblios tab, I would not expect Author to be still in the Facet order list?

Changing status for feedback
Comment 11 Marcel de Rooy 2017-06-30 08:45:58 UTC
Created attachment 64747 [details] [review]
Bug 18235: DBIx schema changes

Fresh changes for SearchField, facet_order.
Comment 12 Nick Clemens 2017-07-03 16:32:14 UTC
(In reply to Marcel de Rooy from comment #10)
> If I select say Author and set them all to Facetable No in the Search fields
> list on the Biblios tab, I would not expect Author to be still in the Facet
> order list?

I agree, this is not ideal, but I think that comes when we stop hardcoding the displayable facets all together - I would say beyond the scope of this bug
Comment 13 Jonathan Druart 2017-07-05 18:28:51 UTC
Yes indeed you are right, this is not ideal and might be confusing.
We should add more JS code to handle that correctly, but I am not willing to write it.
However I am still thinking that this feature is needed.
Comment 14 David Bourgault 2017-11-10 21:50:48 UTC
Also patch does not apply.
Comment 15 Jonathan Druart 2017-12-04 18:30:29 UTC
Created attachment 69485 [details] [review]
Bug 18235: DB changes

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 16 Jonathan Druart 2017-12-04 18:30:35 UTC
Created attachment 69486 [details] [review]
Bug 18235: ES - Facets configurable

This patch adds a new section 'Facet order' in the Biblio tab of the
'Search engine configuration' admin page of the Elastic mappings.
The idea is to let the librarians define the facet to display and order
them as their needs.

The ergonomic is not perfect and I am open to any suggestions.

Test plan:
Move up and down the field list to order the facets
Hide/show some facets
At the OPAC and the staff interface you should see the changes on the
search result page.

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 17 Jonathan Druart 2017-12-04 18:30:40 UTC
Created attachment 69487 [details] [review]
Bug 18235: DBIx schema changes

Fresh changes for SearchField, facet_order.
Comment 18 Nick Clemens 2018-03-15 11:56:31 UTC
Created attachment 72936 [details] [review]
Bug 18235: DB changes

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>


Current status: Signed Off
Comment 19 Nick Clemens 2018-03-15 11:56:37 UTC
Created attachment 72937 [details] [review]
Bug 18235: ES - Facets configurable

This patch adds a new section 'Facet order' in the Biblio tab of the
'Search engine configuration' admin page of the Elastic mappings.
The idea is to let the librarians define the facet to display and order
them as their needs.

The ergonomic is not perfect and I am open to any suggestions.

Test plan:
Move up and down the field list to order the facets
Hide/show some facets
At the OPAC and the staff interface you should see the changes on the
search result page.

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 20 Nick Clemens 2018-03-15 11:56:41 UTC
Created attachment 72938 [details] [review]
Bug 18235: DBIx schema changes

Fresh changes for SearchField, facet_order.
Comment 21 Nicolas Legrand 2018-03-16 09:37:11 UTC
Created attachment 72999 [details] [review]
Bug 18235: DB changes

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Signed-off-by: Nicolas Legrand <nicolas.legrand@bulac.fr>
Comment 22 Nicolas Legrand 2018-03-16 09:37:15 UTC
Created attachment 73000 [details] [review]
Bug 18235: ES - Facets configurable

This patch adds a new section 'Facet order' in the Biblio tab of the
'Search engine configuration' admin page of the Elastic mappings.
The idea is to let the librarians define the facet to display and order
them as their needs.

The ergonomic is not perfect and I am open to any suggestions.

Test plan:
Move up and down the field list to order the facets
Hide/show some facets
Rebuild index
At the OPAC and the staff interface you should see the changes on the
search result page.

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Signed-off-by: Nicolas Legrand <nicolas.legrand@bulac.fr>
Comment 23 Nicolas Legrand 2018-03-16 09:37:18 UTC
Created attachment 73001 [details] [review]
Bug 18235: DBIx schema changes

Fresh changes for SearchField, facet_order.

Signed-off-by: Nicolas Legrand <nicolas.legrand@bulac.fr>
Comment 24 Nicolas Legrand 2018-03-16 09:42:55 UTC
Added missing curly, removed a <<<<<HEAD thingy, added index rebuild in test plan.
Comment 25 Alex Arnaud 2018-03-22 09:14:45 UTC
Ergonomic suggestion:
  - move the the facet table in a new tab (discussed with some interested libraries). Currently it is not visible at the first glance.

QA suggestion:
  - rename bug_xxxxx.sql with the appropriate bug number,
  - add unit test for get_facetable_fields

Then, nice feature and i would like this to move forward.
Comment 26 Séverine Queune 2018-03-23 10:06:20 UTC
+1 for the new 'Facet order' tab !
Comment 27 Jonathan Druart 2018-03-23 13:30:12 UTC
I did not create a new tab because we already have 3: 
    Search fields
    Biblios
    Authorities
and I do not think it makes sense to have a 4th "Facet order" which will refer to biblio search only.
Comment 28 Alex Arnaud 2018-03-23 15:56:12 UTC
Created attachment 73193 [details] [review]
Bug 18235 - Add unit tests for get_facetable_fields

rename atomic update file
Comment 29 Marcel de Rooy 2018-04-13 07:59:46 UTC
QA: Looking here
Comment 30 Marcel de Rooy 2018-04-13 08:01:54 UTC
There is a lot of noise from YAML while running qa tools and a git manipulation fail too.
Comment 31 Jonathan Druart 2018-04-13 13:58:17 UTC
(In reply to Marcel de Rooy from comment #30)
> There is a lot of noise from YAML while running qa tools and a git
> manipulation fail too.

Noise appears before and after the patch, so not related.
The git manipulation is on the atomic update (it should not have been moved in the last patch), I will fix it on pushing.
Comment 32 Marcel de Rooy 2018-04-20 08:45:01 UTC
Resuming here
Comment 33 Marcel de Rooy 2018-04-20 08:49:24 UTC
* Commit title does not start with 'Bug XXXXX: ' - 9400c92
POD is missing for get_facetable_fields
Comment 34 Jonathan Druart 2018-04-20 13:38:47 UTC
(In reply to Marcel de Rooy from comment #33)
> * Commit title does not start with 'Bug XXXXX: ' - 9400c92
> POD is missing for get_facetable_fields

That's all?
Comment 35 Jonathan Druart 2018-04-20 13:45:26 UTC
Created attachment 74642 [details] [review]
Bug 18235: Add unit tests for get_facetable_fields

rename atomic update file
Comment 36 Jonathan Druart 2018-04-20 13:45:30 UTC
Created attachment 74643 [details] [review]
Bug 18235: Add missing POD to get_facetable_fields
Comment 37 Alex Arnaud 2018-06-12 14:17:50 UTC
(In reply to Jonathan Druart from comment #35)
> Created attachment 74642 [details] [review] [review]
> Bug 18235: Add unit tests for get_facetable_fields
> 
> rename atomic update file

Looks like this patch does not what it was named for. Atomic update file's name is still Bug_xxxxxxx.
Comment 38 Jonathan Druart 2018-06-12 14:38:26 UTC
Created attachment 75995 [details] [review]
Bug 18235: DB changes

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Signed-off-by: Nicolas Legrand <nicolas.legrand@bulac.fr>
Comment 39 Jonathan Druart 2018-06-12 14:39:35 UTC
Created attachment 75996 [details] [review]
Bug 18235: Add unit tests for get_facetable_fields
Comment 40 Jonathan Druart 2018-06-12 14:41:07 UTC
I have amended the last 2 patches: 1. rename atomic update file and 2. remove the line from commit message that was not related to the patch.