Bug 35138 - Enable configuration of facets with Elasticsearch
Summary: Enable configuration of facets with Elasticsearch
Status: Passed QA
Alias: None
Product: Koha
Classification: Unclassified
Component: Searching - Elasticsearch (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement with 10 votes (vote)
Assignee: Jonathan Druart
QA Contact:
URL:
Keywords: release-notes-needed
Depends on: 20388 35249 36394 18235
Blocks: 36396
  Show dependency treegraph
 
Reported: 2023-10-23 17:49 UTC by Michaela Sieber
Modified: 2024-04-16 07:18 UTC (History)
7 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 35138: Make ES facetable fields configurable - POC (12.54 KB, patch)
2024-03-14 11:00 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 35138: Add the ability to manage ES facets (18.14 KB, patch)
2024-03-22 10:34 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 35138: Adjust existing data (3.91 KB, patch)
2024-03-22 10:35 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 35138: Mark DisplayLibraryFacets as zebra only (3.54 KB, patch)
2024-03-22 10:35 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 35138: UI changes to add/remove a facet (4.98 KB, patch)
2024-03-22 10:35 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 35138: A bit of cleaning - UI (10.65 KB, patch)
2024-03-22 10:35 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 35138: Add the ability to manage ES facets (18.18 KB, patch)
2024-03-26 10:51 UTC, Clemens Tubach
Details | Diff | Splinter Review
Bug 35138: Adjust existing data (3.96 KB, patch)
2024-03-26 10:51 UTC, Clemens Tubach
Details | Diff | Splinter Review
Bug 35138: Mark DisplayLibraryFacets as zebra only (3.59 KB, patch)
2024-03-26 10:51 UTC, Clemens Tubach
Details | Diff | Splinter Review
Bug 35138: UI changes to add/remove a facet (5.03 KB, patch)
2024-03-26 10:51 UTC, Clemens Tubach
Details | Diff | Splinter Review
Bug 35138: A bit of cleaning - UI (10.70 KB, patch)
2024-03-26 10:51 UTC, Clemens Tubach
Details | Diff | Splinter Review
Bug 35138: Add the ability to manage ES facets (18.25 KB, patch)
2024-04-12 09:04 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 35138: Adjust existing data (4.02 KB, patch)
2024-04-12 09:04 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 35138: Mark DisplayLibraryFacets as zebra only (3.65 KB, patch)
2024-04-12 09:04 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 35138: UI changes to add/remove a facet (5.09 KB, patch)
2024-04-12 09:04 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 35138: A bit of cleaning - UI (10.76 KB, patch)
2024-04-12 09:04 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 35138: (QA followup) Tidy update (2.41 KB, patch)
2024-04-12 09:04 UTC, Nick Clemens
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Michaela Sieber 2023-10-23 17:49:16 UTC
We would like to create a new facet for a 9xx-MARC field

As Nick mentioned in Bug 18235, Comment 7:

-labels shouldn't be hardcoded - staff should be 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)


Problems / Challenges:
1. Translatability: if staff changes or add new labels, they will not be translated
2. same files for elastic and Zebra
Comment 1 Jonathan Druart 2024-03-14 11:00:31 UTC
Created attachment 163133 [details] [review]
Bug 35138: Make ES facetable fields configurable - POC

Move the ES facet fields that are currently hardcoded in the codebase to a new
ES_FACETS authorised value category.

Librarians can then add/remove facet fields and modify their lib/description.

Remaining work:
1. Adjust OPAC templates
2. AV is not ideal here, we should certainly have a dedicated DB table for that.
It will bring more flexibility.
We can image:
  * 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. In my
    testing this is not worker, they are displayed alpha (??)
3. Adjust the UI

Still no translatability however.
Comment 2 Jonathan Druart 2024-03-14 11:08:42 UTC
(In reply to Jonathan Druart from comment #1)
> Still no translatability however.

Translating using JS should be trivial as there will be an id on the label:

facets.inc
<span id="facet-[% facets_loo.type_link_value | html %]">[% facets_loo.label | html %]</span>
Comment 3 Michaela Sieber 2024-03-14 16:06:47 UTC
Thanks Jonathan for your proof of concept. 
We testet the patch that uses an authorized value for the configuration of ElasticSearch facets. 
Everything in the patch worked as expected.
Comment 4 Jonathan Druart 2024-03-15 08:22:12 UTC
(In reply to Jonathan Druart from comment #1)
>   * order: move search_field.facet_order to this new table. In my
>     testing this is not worker, they are displayed alpha (??)

*not working*

It's actually broken in master but is going to be fixed by bug 36190.
Comment 5 Jonathan Druart 2024-03-22 10:34:58 UTC
Created attachment 163680 [details] [review]
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)
Comment 6 Jonathan Druart 2024-03-22 10:35:00 UTC
Created attachment 163681 [details] [review]
Bug 35138: Adjust existing data

We want the label from the DB displayed on the UI so we are adjusting
the yaml and the DB.

Sponsored-by: The Research University in the Helmholtz Association (KIT)
Comment 7 Jonathan Druart 2024-03-22 10:35:03 UTC
Created attachment 163682 [details] [review]
Bug 35138: Mark DisplayLibraryFacets as zebra only

We can know add/remove homelibrary and holdinglibray from the config
page, this syspref is no longer needed for ES

Sponsored-by: The Research University in the Helmholtz Association (KIT)
Comment 8 Jonathan Druart 2024-03-22 10:35:06 UTC
Created attachment 163683 [details] [review]
Bug 35138: UI changes to add/remove a facet

Sponsored-by: The Research University in the Helmholtz Association (KIT)
Comment 9 Jonathan Druart 2024-03-22 10:35:09 UTC
Created attachment 163684 [details] [review]
Bug 35138: A bit of cleaning - UI

This can be moved to its own bug report, it's only a bit of cleaning
trying to keep the code cleaner after than before.
No behaviour changes expected.

Sponsored-by: The Research University in the Helmholtz Association (KIT)
Comment 10 Clemens Tubach 2024-03-26 10:51:02 UTC
Created attachment 163896 [details] [review]
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 <clemens.tubach@kit.edu>
Comment 11 Clemens Tubach 2024-03-26 10:51:05 UTC
Created attachment 163897 [details] [review]
Bug 35138: Adjust existing data

We want the label from the DB displayed on the UI so we are adjusting
the yaml and the DB.

Sponsored-by: The Research University in the Helmholtz Association (KIT)
Signed-off-by: Clemens Tubach <clemens.tubach@kit.edu>
Comment 12 Clemens Tubach 2024-03-26 10:51:07 UTC
Created attachment 163898 [details] [review]
Bug 35138: Mark DisplayLibraryFacets as zebra only

We can know add/remove homelibrary and holdinglibray from the config
page, this syspref is no longer needed for ES

Sponsored-by: The Research University in the Helmholtz Association (KIT)
Signed-off-by: Clemens Tubach <clemens.tubach@kit.edu>
Comment 13 Clemens Tubach 2024-03-26 10:51:10 UTC
Created attachment 163899 [details] [review]
Bug 35138: UI changes to add/remove a facet

Sponsored-by: The Research University in the Helmholtz Association (KIT)
Signed-off-by: Clemens Tubach <clemens.tubach@kit.edu>
Comment 14 Clemens Tubach 2024-03-26 10:51:13 UTC
Created attachment 163900 [details] [review]
Bug 35138: A bit of cleaning - UI

This can be moved to its own bug report, it's only a bit of cleaning
trying to keep the code cleaner after than before.
No behaviour changes expected.

Sponsored-by: The Research University in the Helmholtz Association (KIT)
Signed-off-by: Clemens Tubach <clemens.tubach@kit.edu>
Comment 15 Nick Clemens 2024-04-12 09:04:36 UTC
Created attachment 164811 [details] [review]
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 <clemens.tubach@kit.edu>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 16 Nick Clemens 2024-04-12 09:04:39 UTC
Created attachment 164812 [details] [review]
Bug 35138: Adjust existing data

We want the label from the DB displayed on the UI so we are adjusting
the yaml and the DB.

Sponsored-by: The Research University in the Helmholtz Association (KIT)
Signed-off-by: Clemens Tubach <clemens.tubach@kit.edu>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 17 Nick Clemens 2024-04-12 09:04:42 UTC
Created attachment 164813 [details] [review]
Bug 35138: Mark DisplayLibraryFacets as zebra only

We can know add/remove homelibrary and holdinglibray from the config
page, this syspref is no longer needed for ES

Sponsored-by: The Research University in the Helmholtz Association (KIT)
Signed-off-by: Clemens Tubach <clemens.tubach@kit.edu>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 18 Nick Clemens 2024-04-12 09:04:45 UTC
Created attachment 164814 [details] [review]
Bug 35138: UI changes to add/remove a facet

Sponsored-by: The Research University in the Helmholtz Association (KIT)
Signed-off-by: Clemens Tubach <clemens.tubach@kit.edu>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 19 Nick Clemens 2024-04-12 09:04:47 UTC
Created attachment 164815 [details] [review]
Bug 35138: A bit of cleaning - UI

This can be moved to its own bug report, it's only a bit of cleaning
trying to keep the code cleaner after than before.
No behaviour changes expected.

Sponsored-by: The Research University in the Helmholtz Association (KIT)
Signed-off-by: Clemens Tubach <clemens.tubach@kit.edu>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 20 Nick Clemens 2024-04-12 09:04:50 UTC
Created attachment 164816 [details] [review]
Bug 35138: (QA followup) Tidy update

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 21 Nick Clemens 2024-04-12 09:06:30 UTC
I think this patch makes the best of a difficult code area, and makes the management of facets in ES for more intuitive, and uses some of the things in the editor as they were intended.

The change to DisplayLibraryFacets will need to be highlighted in release notes so that libraries are aware they will need to reconfiugre
Comment 22 Jonathan Druart 2024-04-16 07:18:24 UTC
(In reply to Nick Clemens from comment #21)
> The change to DisplayLibraryFacets will need to be highlighted in release
> notes so that libraries are aware they will need to reconfiugre

They won't need to reconfigure anything. We are dealing with that in the atomic update. Or are you talking about something else?