Bug 36437 - Add publication year facet to Elasticsearch
Summary: Add publication year facet to Elasticsearch
Status: Needs Signoff
Alias: None
Product: Koha
Classification: Unclassified
Component: Searching - Elasticsearch (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement
Assignee: Michał
QA Contact:
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2024-03-27 08:00 UTC by Michał
Modified: 2024-10-30 23:09 UTC (History)
4 users (show)

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


Attachments
0001-Bug-36437-Add-publication-year-facet-to-Elasticsearc.patch (10.70 KB, patch)
2024-03-27 08:03 UTC, Michał
Details | Diff | Splinter Review
Bug 36437: add pub year facet to Elasticsearch (6.23 KB, patch)
2024-10-30 13:14 UTC, Michał
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Michał 2024-03-27 08:00:35 UTC
Similarly to Bug 18213, it'd be nice to add publication date facet. It seems to work quite nicely, so why not? Our librarians say it's super useful.

I've seen one library implement it in their fork, the only difference is that they sort the publication year facet in reverse incremental order (so from newest to oldest year instead of oldest to newest), but I think this would require an explicit check in the code, and I wasn't sure if that'd be fine...

To test, one should have Elasticsearch enabled and re-generate the mappings from file and reindex as described in Bug 18213.

Then confirm that the facet is visible and working:
- in OPAC search
- in intranet search
- in intranet settings, under Elasticsearch mapping settings page, biblio tab, publication year facet should be visible at the bottom where you set facet order, with correct label; you should be able to change the order normally as expected
Comment 1 Michał 2024-03-27 08:03:28 UTC
Created attachment 163976 [details] [review]
0001-Bug-36437-Add-publication-year-facet-to-Elasticsearc.patch
Comment 2 Jonathan Druart 2024-03-27 08:44:56 UTC
Hey, did you see bug 35138?
Comment 3 Michał 2024-03-27 09:11:12 UTC
Oh, I didn't. Honestly if they want to make facets configurable in the UI instead of hardcoding them in several places, then that's only for the better. Then that'd obsolete this bug and afterwards we could only argue for including the publication year on by default perhaps... (whether as this bug or by Bug 10734 and marking that one as solved and this one as not applying...)
Comment 4 Jonathan Druart 2024-03-27 12:39:24 UTC
It could still be added to the default but in my opinion the patch should be written on top of bug 35138.
Comment 5 Michał 2024-07-03 14:20:23 UTC
Reverting status to ASSIGNED until I rewrite this on top of bug 35138 in place of the current patch.
Comment 6 Michał 2024-10-30 13:14:13 UTC
Created attachment 173716 [details] [review]
Bug 36437: add pub year facet to Elasticsearch

Notes:
- I labeled it as "Year of publication" instead of "Date of publication" since it seems to be more occurate (since the field is a year and that's gonna be better to translate to other langauge)
- the third word is not capitalized, since I'm following the naming convention of existing facet labels
- facet_order was set to 3, since that should be a pretty common field and useful to display high up to get a quick idea for the range of years the results are from

Test plan:
0. Ensure Elasticsearch index is rebuilt (should be on fresh ktd startup?)
1. On intranet go to /cgi-bin/koha/admin/searchengine/elasticsearch/mappings.pl
2. Go to second tab there
3. Ensure date-of-publication is in the table, has label in parentheses, has facetable set to Yes
4. Scrolldown to the order of facets table
5. Ensure date-of-publication is there, has label, is third on the list
6. Search for something in intranet, ensure the facet is visible, its value is year, it's clickable and working
7. Search for something in OPAC, as above ensure it works (important to check both OPAC and intranet since facets use different templates to display in both...)
Comment 7 Michał 2024-10-30 13:18:11 UTC
Comment on attachment 163976 [details] [review]
0001-Bug-36437-Add-publication-year-facet-to-Elasticsearc.patch

>From a9ae44f2adff90c234f3b65e439c04c5da4f8dfa Mon Sep 17 00:00:00 2001
>From: MichaÅ Kula <148193449+mkibp@users.noreply.github.com>
>Date: Wed, 27 Mar 2024 09:00:43 +0100
>Subject: [PATCH] Bug 36437: Add publication year facet to Elasticsearch
>
>---
> Koha/SearchEngine/Elasticsearch.pm            |  2 +-
> .../Elasticsearch/QueryBuilder.pm             | 17 ++++++++-------
> Koha/SearchEngine/Elasticsearch/Search.pm     | 21 ++++++++++---------
> .../searchengine/elasticsearch/mappings.yaml  | 21 ++++++++++---------
> .../intranet-tmpl/prog/en/includes/facets.inc |  3 +++
> .../searchengine/elasticsearch/mappings.tt    |  3 ++-
> .../bootstrap/en/includes/opac-facets.inc     |  2 ++
> 7 files changed, 39 insertions(+), 30 deletions(-)
>
>diff --git a/Koha/SearchEngine/Elasticsearch.pm b/Koha/SearchEngine/Elasticsearch.pm
>index 3f26cbc027..b905f7726f 100644
>--- a/Koha/SearchEngine/Elasticsearch.pm
>+++ b/Koha/SearchEngine/Elasticsearch.pm
>@@ -1404,7 +1404,7 @@ sub get_facetable_fields {
> 
>     # These should correspond to the ES field names, as opposed to the CCL
>     # things that zebra uses.
>-    my @search_field_names = qw( author itype location su-geo title-series subject ccode holdingbranch homebranch ln );
>+    my @search_field_names = qw( author itype location su-geo title-series subject ccode holdingbranch homebranch ln date-of-publication );
>     my @faceted_fields = Koha::SearchFields->search(
>         { name => { -in => \@search_field_names }, facet_order => { '!=' => undef } }, { order_by => ['facet_order'] }
>     )->as_list;
>diff --git a/Koha/SearchEngine/Elasticsearch/QueryBuilder.pm b/Koha/SearchEngine/Elasticsearch/QueryBuilder.pm
>index 35ab4b7f91..758c8409ec 100644
>--- a/Koha/SearchEngine/Elasticsearch/QueryBuilder.pm
>+++ b/Koha/SearchEngine/Elasticsearch/QueryBuilder.pm
>@@ -226,14 +226,15 @@ sub build_query {
>     # things that Koha can use.
>     my $size = C4::Context->preference('FacetMaxCount');
>     $res->{aggregations} = {
>-        author         => { terms => { field => "author__facet" , size => $size } },
>-        subject        => { terms => { field => "subject__facet", size => $size } },
>-        itype          => { terms => { field => "itype__facet", size => $size} },
>-        location       => { terms => { field => "location__facet", size => $size } },
>-        'su-geo'       => { terms => { field => "su-geo__facet", size => $size} },
>-        'title-series' => { terms => { field => "title-series__facet", size => $size } },
>-        ccode          => { terms => { field => "ccode__facet", size => $size } },
>-        ln             => { terms => { field => "ln__facet", size => $size } },
>+        author                => { terms => { field => "author__facet" , size => $size } },
>+        subject               => { terms => { field => "subject__facet", size => $size } },
>+        itype                 => { terms => { field => "itype__facet", size => $size} },
>+        location              => { terms => { field => "location__facet", size => $size } },
>+        'su-geo'              => { terms => { field => "su-geo__facet", size => $size} },
>+        'title-series'        => { terms => { field => "title-series__facet", size => $size } },
>+        ccode                 => { terms => { field => "ccode__facet", size => $size } },
>+        ln                    => { terms => { field => "ln__facet", size => $size } },
>+        'date-of-publication' => { terms => { field => "date-of-publication__facet", size => $size } },
>     };
> 
>     my $display_library_facets = C4::Context->preference('DisplayLibraryFacets');
>diff --git a/Koha/SearchEngine/Elasticsearch/Search.pm b/Koha/SearchEngine/Elasticsearch/Search.pm
>index 933b52f79d..4c1d73a9b1 100644
>--- a/Koha/SearchEngine/Elasticsearch/Search.pm
>+++ b/Koha/SearchEngine/Elasticsearch/Search.pm
>@@ -444,16 +444,17 @@ sub _convert_facets {
>     # things that zebra uses.
>     my %type_to_label;
>     my %label = (
>-        author         => 'Authors',
>-        itype          => 'ItemTypes',
>-        location       => 'Location',
>-        'su-geo'       => 'Places',
>-        'title-series' => 'Series',
>-        subject        => 'Topics',
>-        ccode          => 'CollectionCodes',
>-        holdingbranch  => 'HoldingLibrary',
>-        homebranch     => 'HomeLibrary',
>-        ln             => 'Language',
>+        author                => 'Authors',
>+        itype                 => 'ItemTypes',
>+        location              => 'Location',
>+        'su-geo'              => 'Places',
>+        'title-series'        => 'Series',
>+        subject               => 'Topics',
>+        ccode                 => 'CollectionCodes',
>+        holdingbranch         => 'HoldingLibrary',
>+        homebranch            => 'HomeLibrary',
>+        ln                    => 'Language',
>+        'date-of-publication' => 'PubDate',
>     );
>     my @facetable_fields =
>       Koha::SearchEngine::Elasticsearch->get_facetable_fields;
>diff --git a/admin/searchengine/elasticsearch/mappings.yaml b/admin/searchengine/elasticsearch/mappings.yaml
>index f9f9a042c5..1e1bca315b 100644
>--- a/admin/searchengine/elasticsearch/mappings.yaml
>+++ b/admin/searchengine/elasticsearch/mappings.yaml
>@@ -1280,7 +1280,7 @@ biblios:
>     staff_client: 1
>     type: ''
>   ccode:
>-    facet_order: 7
>+    facet_order: 8
>     label: collection-code
>     mandatory: ~
>     mappings:
>@@ -1656,15 +1656,16 @@ biblios:
>     staff_client: 1
>     type: date
>   date-of-publication:
>+    facet_order: 3
>     label: date-of-publication
>     mandatory: ~
>     mappings:
>-    - facet: ''
>+    - facet: 1
>       marc_field: 008_/7-10
>       marc_type: marc21
>       sort: 1
>       suggestible: ''
>-    - facet: ''
>+    - facet: 1
>       marc_field: 100a_/9-12
>       marc_type: unimarc
>       sort: 1
>@@ -1856,7 +1857,7 @@ biblios:
>     staff_client: 1
>     type: ''
>   holdingbranch:
>-    facet_order: 8
>+    facet_order: 9
>     label: holdinglibrary
>     mandatory: ~
>     mappings:
>@@ -1874,7 +1875,7 @@ biblios:
>     staff_client: 1
>     type: string
>   homebranch:
>-    facet_order: 9
>+    facet_order: 10
>     label: homelibrary
>     mandatory: ~
>     mappings:
>@@ -2484,7 +2485,7 @@ biblios:
>     staff_client: 1
>     type: ''
>   ln:
>-    facet_order: 10
>+    facet_order: 11
>     label: ln
>     mandatory: ~
>     mappings:
>@@ -2565,7 +2566,7 @@ biblios:
>     staff_client: 1
>     type: number
>   location:
>-    facet_order: 3
>+    facet_order: 4
>     label: location
>     mandatory: ~
>     mappings:
>@@ -3351,7 +3352,7 @@ biblios:
>     staff_client: 1
>     type: ''
>   su-geo:
>-    facet_order: 4
>+    facet_order: 5
>     label: su-geo
>     mandatory: ~
>     mappings:
>@@ -3369,7 +3370,7 @@ biblios:
>     staff_client: 1
>     type: string
>   subject:
>-    facet_order: 6
>+    facet_order: 7
>     label: subject
>     mandatory: ~
>     mappings:
>@@ -4131,7 +4132,7 @@ biblios:
>     staff_client: 1
>     type: ''
>   title-series:
>-    facet_order: 5
>+    facet_order: 6
>     label: title-series
>     mandatory: ~
>     mappings:
>diff --git a/koha-tmpl/intranet-tmpl/prog/en/includes/facets.inc b/koha-tmpl/intranet-tmpl/prog/en/includes/facets.inc
>index 2fcce67e7d..fbbdb52ae8 100644
>--- a/koha-tmpl/intranet-tmpl/prog/en/includes/facets.inc
>+++ b/koha-tmpl/intranet-tmpl/prog/en/includes/facets.inc
>@@ -84,6 +84,9 @@
>                             [% IF facets_loo.type_label_Language %]
>                                 <span id="facet-languages">Languages</span>
>                             [% END %]
>+                            [% IF facets_loo.type_label_PubDate %]
>+                                <span id="facet-pubdates">Publication years</span>
>+                            [% END %]
>                             <ul>
>                                 [% SET url = "/cgi-bin/koha/catalogue/search.pl?" _ query_cgi _ limit_cgi %]
>                                 [% IF ( sort_by ) %]
>diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/searchengine/elasticsearch/mappings.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/searchengine/elasticsearch/mappings.tt
>index 625afa4998..233ea98126 100644
>--- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/searchengine/elasticsearch/mappings.tt
>+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/searchengine/elasticsearch/mappings.tt
>@@ -482,7 +482,8 @@ a.add, a.delete {
>                                                     [% CASE 'holdingbranch' %]<span>Holding libraries</span>
>                                                     [% CASE 'homebranch' %]<span>Home libraries</span>
>                                                     [% CASE 'ln' %]<span>Language</span>
>-                                                    [% CASE %][% f | html %]
>+                                                    [% CASE 'date-of-publication' %]<span>Publication year</span>
>+                                                    [% CASE %][% f.name | html %]
>                                                     [% END %]
>                                                 </td>
>                                                 <td>
>diff --git a/koha-tmpl/opac-tmpl/bootstrap/en/includes/opac-facets.inc b/koha-tmpl/opac-tmpl/bootstrap/en/includes/opac-facets.inc
>index 8a62c4cd93..3b4dbc810f 100644
>--- a/koha-tmpl/opac-tmpl/bootstrap/en/includes/opac-facets.inc
>+++ b/koha-tmpl/opac-tmpl/bootstrap/en/includes/opac-facets.inc
>@@ -51,6 +51,7 @@
>             [% more_less_labels.HomeLibrary     = { more = t("Show more home libraries"),    less = t("Show fewer home libraries") }    %]
>             [% more_less_labels.HoldingLibrary  = { more = t("Show more holding libraries"), less = t("Show fewer holding libraries") } %]
>             [% more_less_labels.Location        = { more = t("Show more locations"),         less = t("Show fewer locations") }         %]
>+            [% more_less_labels.PubDate         = { more = t("Show more publication years"), less = t("Show fewer publication years") } %]
>             [% FOREACH facets_loo IN facets_loop %]
>                 [% IF facets_loo.facets.size > 0 %]
>                     <li id="[% facets_loo.type_id | html %]">
>@@ -62,6 +63,7 @@
>                         [% IF facets_loo.type_label_ItemTypes %]<h3 id="facet-itemtypes">Item types</h3>[% END %]
>                         [% IF facets_loo.type_label_CollectionCodes %]<h3 id="facet-collections">Collections</h3>[% END %]
>                         [% IF facets_loo.type_label_Language %]<h3 id="facet-languages">Languages</h3>[% END %]
>+                        [% IF facets_loo.type_label_PubDate %]<h3 id="facet-pubdates">Publication years</h3>[% END %]
>                         [% UNLESS singleBranchMode %]
>                             [% IF ( facets_loo.type_label_HomeLibrary ) %]<h3 id="facet-home-libraries">Home libraries</h3>[% END %]
>                             [% IF ( facets_loo.type_label_HoldingLibrary ) %]<h3 id="facet-holding-libraries">Holding libraries</h3>[% END %]
>-- 
>2.42.0.windows.1
>
Comment 8 Michał 2024-10-30 13:21:27 UTC
Rewritten on top of bug 35138 (one that adds configurable facets). Now it adds year of publication as one of *default* facets.

The only thing that kind of sucks right now is that with C4::Context->preference('FacetOrder') eq 'Alphabetical' it's gonna sort the years from oldest rather than from newest. I'm not sure if it's worth adding some special case there in Search.pm to sort years from newest first instead in such case.
Comment 9 David Cook 2024-10-30 23:09:31 UTC
(In reply to Michał from comment #8)
> The only thing that kind of sucks right now is that with
> C4::Context->preference('FacetOrder') eq 'Alphabetical' it's gonna sort the
> years from oldest rather than from newest. I'm not sure if it's worth adding
> some special case there in Search.pm to sort years from newest first instead
> in such case.

I think we did a local version of this, and that's what we ended up doing. I think it's an important thing to include. 

--

I'll make a note to test this patch soon. It would be nice to have!