Attachment 170489 Details for Bug 36947 – Bug 36947: Do a locale-based sort for ES facet names

[patch] Bug 36947: Do a locale-based sort for ES facet names

Bug-36947-Do-a-locale-based-sort-for-ES-facet-name.patch (text/plain), 6.96 KB, created by David Cook on 2024-08-20 03:35:25 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: David Cook

Created: 2024-08-20 03:35:25 UTC

Size: 6.96 KB

patch

obsolete

>From 94e7bbe48bfd35bdd6d62c37e58642d4242ae29d Mon Sep 17 00:00:00 2001
>From: David Cook <dcook@prosentient.com.au>
>Date: Mon, 27 May 2024 23:37:01 +0000
>Subject: [PATCH] Bug 36947: Do a locale-based sort for ES facet names
>MIME-Version: 1.0
>Content-Type: text/plain; charset=UTF-8
>Content-Transfer-Encoding: 8bit
>
>This change uses a configurable locale-based collator to sort
>the ES facet names.
>
>Test plan:
>0. Apply the patch
>1. vi /etc/locale.gen
>2. Uncomment the locale you want to generate (e.g. fi_FI.UTF-8 UTF-8)
>3. locale-gen
>4. vi "/etc/default/koha-common"
>5. Add the following to the bottom of the file:
>export LC_ALL=fi_FI.UTF-8
>6. koha-plack --restart koha-common
>7. Setup some test records with authors with accented and unaccented names,
>    and different cases for the lead letter
>e.g. Aa author, Ãa author2, aa author, Ã©tienne
>8. Switch to using Elasticsearch and reindex
>koha-elasticsearch -b -v --rebuild kohadev
>9. Do a test search
>    e.g. http://localhost:8081/cgi-bin/koha/catalogue/search.pl?q=test
>10. Confirm the facet names are sorted in ascending order following
>Finnish collation rules
>    e.g.
>aa author
>Aa author
>Ã©tienne
>Farley, David
>Humble, Jez
>Martin, Robert C.
>Ãa author
>
>NOTE: Any collation and language can be used. Finnish is just
>an example of a Latin-based script which has a different
>alphabetical ordering than just A-Z
>---
> Koha/SearchEngine/Elasticsearch/Search.pm  | 40 ++++++++++++-
> t/Koha/SearchEngine/Elasticsearch/Search.t | 69 +++++++++++++++++++++-
> 2 files changed, 106 insertions(+), 3 deletions(-)
>
>diff --git a/Koha/SearchEngine/Elasticsearch/Search.pm b/Koha/SearchEngine/Elasticsearch/Search.pm
>index 42326e3573..b57d506575 100644
>--- a/Koha/SearchEngine/Elasticsearch/Search.pm
>+++ b/Koha/SearchEngine/Elasticsearch/Search.pm
>@@ -54,6 +54,9 @@ use MARC::File::XML;
> use MIME::Base64 qw( decode_base64 );
> use JSON;
> 
>+use POSIX qw(setlocale LC_COLLATE);
>+use Unicode::Collate::Locale;
>+
> Koha::SearchEngine::Elasticsearch::Search->mk_accessors(qw( store ));
> 
> =head2 search
>@@ -427,6 +430,37 @@ sub max_result_window {
>     return $max_result_window;
> }
> 
>+
>+=head2 _sort_facets
>+
>+    my $facets = _sort_facets($facets);
>+
>+Sorts facets using a locale.
>+
>+=cut
>+
>+sub _sort_facets {
>+    my ( $self, $args ) = @_;
>+    my $facets = $args->{facets};
>+    my $locale = $args->{locale};
>+    if ( !$locale ) {
>+
>+        #NOTE: When setlocale is run with only the 1st parameter, it is a "get" not a "set" function.
>+        $locale = setlocale(LC_COLLATE);
>+    }
>+    my $collator = Unicode::Collate::Locale->new( locale => $locale );
>+    if ( $collator && $facets ) {
>+        my @sorted_facets = sort { $collator->cmp( $a->{facet_label_value}, $b->{facet_label_value} ) } @{$facets};
>+        if (@sorted_facets) {
>+            return \@sorted_facets;
>+        }
>+    }
>+
>+    #NOTE: If there was a problem, at least return the not sorted facets
>+    return $facets;
>+}
>+
>+
> =head2 _convert_facets
> 
>     my $koha_facets = _convert_facets($es_facets);
>@@ -504,8 +538,10 @@ sub _convert_facets {
>             };
>         }
>         if( C4::Context->preference('FacetOrder') eq 'Alphabetical' ){
>-            @{ $facet->{facets} } =
>-                sort { $a->{facet_label_value} cmp $b->{facet_label_value} } @{ $facet->{facets} };
>+            my $sorted_facets = $self->_sort_facets( { facets => $facet->{facets} } );
>+            if ($sorted_facets) {
>+                $facet->{facets} = $sorted_facets;
>+            }
>         }
>         push @facets, $facet if exists $facet->{facets};
>     }
>diff --git a/t/Koha/SearchEngine/Elasticsearch/Search.t b/t/Koha/SearchEngine/Elasticsearch/Search.t
>index cae1b2a96e..1353f1ee1c 100755
>--- a/t/Koha/SearchEngine/Elasticsearch/Search.t
>+++ b/t/Koha/SearchEngine/Elasticsearch/Search.t
>@@ -17,12 +17,79 @@
> 
> use Modern::Perl;
> 
>-use Test::More tests => 2;
>+use Test::More tests => 3;
> use Test::MockModule;
> use t::lib::Mocks;
> 
>+use utf8;
>+
> use_ok('Koha::SearchEngine::Elasticsearch::Search');
> 
>+subtest '_sort_facets' => sub {
>+    plan tests => 2;
>+    t::lib::Mocks::mock_preference( 'SearchEngine', 'Elasticsearch' );
>+    my $facets = [
>+        { facet_label_value => 'Mary' },
>+        { facet_label_value => 'Harry' },
>+        { facet_label_value => 'Fairy' },
>+        { facet_label_value => 'Ari' },
>+        { facet_label_value => 'mary' },
>+        { facet_label_value => 'harry' },
>+        { facet_label_value => 'fairy' },
>+        { facet_label_value => 'ari' },
>+        { facet_label_value => 'Ã©tienne' },
>+        { facet_label_value => 'Ãuthor' },
>+    ];
>+
>+    my @normal_sort_facets     = sort { $a->{facet_label_value} cmp $b->{facet_label_value} } @$facets;
>+    my @normal_expected_facets = (
>+        { facet_label_value => 'Ari' },
>+        { facet_label_value => 'Fairy' },
>+        { facet_label_value => 'Harry' },
>+        { facet_label_value => 'Mary' },
>+        { facet_label_value => 'ari' },
>+        { facet_label_value => 'fairy' },
>+        { facet_label_value => 'harry' },
>+        { facet_label_value => 'mary' },
>+        { facet_label_value => 'Ãuthor' },
>+        { facet_label_value => 'Ã©tienne' },
>+    );
>+
>+    #NOTE: stringwise/bytewise is not UTF-8 friendly
>+    is_deeply( \@normal_sort_facets, \@normal_expected_facets, "Perl's built-in sort is stringwise/bytewise." );
>+
>+    my $search = Koha::SearchEngine::Elasticsearch::Search->new(
>+        { index => $Koha::SearchEngine::Elasticsearch::AUTHORITIES_INDEX } );
>+
>+    #NOTE: The 'default' locale uses the Default Unicode Collation Element Table, which
>+    #is used for the locales of English (en) and French (fr).
>+    my $sorted_facets = $search->_sort_facets( { facets => $facets, locale => 'default' } );
>+    my $expected      = [
>+        { facet_label_value => 'ari' },
>+        { facet_label_value => 'Ari' },
>+        { facet_label_value => 'Ãuthor' },
>+        { facet_label_value => 'Ã©tienne' },
>+        { facet_label_value => 'fairy' },
>+        { facet_label_value => 'Fairy' },
>+        { facet_label_value => 'harry' },
>+        { facet_label_value => 'Harry' },
>+        { facet_label_value => 'mary' },
>+        { facet_label_value => 'Mary' },
>+    ];
>+    is_deeply( $sorted_facets, $expected, "Facets sorted correctly with default locale" );
>+
>+    #NOTE: If "locale" is not provided to _sort_facets, it will look up the LC_COLLATE
>+    #for the local system. This is what allows this function to work well in production.
>+    #However, since LC_COLLATE could vary from system to system running these unit tests,
>+    #we can't test arbitrary locales here.
>+
>+    #TODO: It would be great to explicitly use a fi_FI locale for another test, but we cannot guarantee
>+    #that every system has that locale. That said, perhaps we could do some conditional unit
>+    #testing if certain locales are available when running tests. This could allow for the Koha community
>+    #to thoroughly test this functionality, while letting anyone run the unit tests regardless of their
>+    #installed locales.
>+};
>+
> subtest 'search_auth_compat' => sub {
>     plan tests => 4;
> 
>-- 
>2.39.2

Actions: View | Diff | Splinter Review

Attachments on bug 36947: 167110 | 167125 | 167141 | 167142 | 167143 | 167145 | 167146 | 167188 | 167198 | 167880 | 168488 | 170489