Bugzilla – Attachment 183552 Details for
Bug 36947
Sort Elasticsearch/Zebra facets according to configurable locale instead of using Perl's stringwise/bytewise sort
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 36947: Add system preference for configurable locale-based facet sorting
Bug-36947-Add-system-preference-for-configurable-l.patch (text/plain), 19.60 KB, created by
Martin Renvoize (ashimema)
on 2025-06-26 16:45:12 UTC
(
hide
)
Description:
Bug 36947: Add system preference for configurable locale-based facet sorting
Filename:
MIME Type:
Creator:
Martin Renvoize (ashimema)
Created:
2025-06-26 16:45:12 UTC
Size:
19.60 KB
patch
obsolete
>From 4c8c0a5b60caa0f41bd0423024b4fabb4cfa837c Mon Sep 17 00:00:00 2001 >From: Martin Renvoize <martin.renvoize@openfifth.co.uk> >Date: Thu, 26 Jun 2025 17:28:55 +0100 >Subject: [PATCH] Bug 36947: Add system preference for configurable > locale-based facet sorting >MIME-Version: 1.0 >Content-Type: text/plain; charset=UTF-8 >Content-Transfer-Encoding: 8bit > >This follow-up adds two major enhancements to the locale-based facet sorting >implementation: > >1. **System Preference Integration** > - Adds FacetSortingLocale system preference for configurable locale selection > - Dynamically detects available system locales using Koha::I18N > - Provides user-friendly dropdown with locale descriptions > - Supports both Elasticsearch and Zebra search engines > >2. **Zebra Search Engine Support** > - Extends locale-based sorting to Zebra (in addition to existing Elasticsearch) > - Implements _sort_facets_zebra() function in C4::Search > - Maintains consistent Unicode-aware sorting behavior across search engines > >Key Features: >- Dynamic locale detection via Koha::I18N::available_locales() >- Proper module architecture with comprehensive unit tests >- Clean fallback chain: preference â system LC_COLLATE â default >- Admin UI integration in System Preferences â I18N/L10N >- Universal facet sorting for international library systems > >Test plan: >1. Apply patch and restart services >2. Install/run database update >3. Navigate to Administration â System Preferences â I18N/L10N >4. Verify "Sort facet names using" dropdown shows available system locales >5. Test facet sorting with different locales and both search engines >6. Run: prove t/Koha/I18N.t t/Koha/SearchEngine/Elasticsearch/Search.t t/db_dependent/Search_FacetSorting.t >--- > C4/Search.pm | 42 +++++++- > Koha/I18N.pm | 99 +++++++++++++++++++ > Koha/SearchEngine/Elasticsearch/Search.pm | 13 ++- > admin/preferences.pl | 6 ++ > .../data/mysql/atomicupdate/bug_36947.pl | 22 +++++ > installer/data/mysql/mandatory/sysprefs.sql | 1 + > .../modules/admin/preferences/i18n_l10n.pref | 5 + > t/Koha/I18N.t | 48 ++++++++- > t/Koha/SearchEngine/Elasticsearch/Search.t | 9 +- > t/db_dependent/Search_FacetSorting.t | 68 +++++++++++++ > 10 files changed, 305 insertions(+), 8 deletions(-) > create mode 100755 installer/data/mysql/atomicupdate/bug_36947.pl > create mode 100755 t/db_dependent/Search_FacetSorting.t > >diff --git a/C4/Search.pm b/C4/Search.pm >index 08903e7fc57..bf0a485286f 100644 >--- a/C4/Search.pm >+++ b/C4/Search.pm >@@ -40,6 +40,8 @@ use URI::Escape; > use Business::ISBN; > use MARC::Record; > use MARC::Field; >+use POSIX qw(setlocale LC_COLLATE); >+use Unicode::Collate::Locale; > > our ( @ISA, @EXPORT_OK ); > >@@ -291,6 +293,40 @@ See verbose embedded documentation. > > =cut > >+=head2 _sort_facets_zebra >+ >+ my $sorted_facets = _sort_facets_zebra($facets, $locale); >+ >+Sorts facets using a configurable locale for Zebra search engine. >+ >+=cut >+ >+sub _sort_facets_zebra { >+ my ( $facets, $locale ) = @_; >+ >+ if ( !$locale ) { >+ >+ # Get locale from system preference, falling back to system LC_COLLATE >+ $locale = C4::Context->preference('FacetSortingLocale') || 'default'; >+ if ( $locale eq 'default' || !$locale ) { >+ >+ #NOTE: When setlocale is run with only the 1st parameter, it is a "get" not a "set" function. >+ $locale = setlocale(LC_COLLATE) || 'default'; >+ } >+ } >+ >+ 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; >+} >+ > sub getRecords { > my ( > $koha_query, $simple_query, $sort_by_ref, $servers_ref, >@@ -554,8 +590,10 @@ sub getRecords { > if (@facets_loop) { > foreach my $f (@facets_loop) { > if ( C4::Context->preference('FacetOrder') eq 'Alphabetical' ) { >- $f->{facets} = >- [ sort { uc( $a->{facet_label_value} ) cmp uc( $b->{facet_label_value} ) } @{ $f->{facets} } ]; >+ my $sorted_facets = _sort_facets_zebra( $f->{facets} ); >+ if ($sorted_facets) { >+ $f->{facets} = $sorted_facets; >+ } > } > } > } >diff --git a/Koha/I18N.pm b/Koha/I18N.pm >index c957e9df404..0be3960ae05 100644 >--- a/Koha/I18N.pm >+++ b/Koha/I18N.pm >@@ -53,6 +53,10 @@ our @EXPORT = qw( > N__np > ); > >+our @EXPORT_OK = qw( >+ available_locales >+); >+ > our $textdomain = 'Koha'; > > sub init { >@@ -224,4 +228,99 @@ sub _expand { > return $text; > } > >+=head2 available_locales >+ >+ my $locales = Koha::I18N::available_locales(); >+ >+Returns an arrayref of available system locales for use in system preferences. >+ >+Each locale is a hashref with: >+ - C<value>: The locale identifier (e.g., 'en_US.utf8', 'default') >+ - C<text>: Human-readable description (e.g., 'English (United States) - en_US.utf8') >+ >+Always includes 'default' as the first option. Additional locales are detected >+from the system using C<locale -a> and filtered for UTF-8 locales. >+ >+=cut >+ >+sub available_locales { >+ my @available_locales = (); >+ >+ # Always include default option >+ push @available_locales, { >+ value => 'default', >+ text => 'Default Unicode collation' >+ }; >+ >+ # Get system locales using the same approach as init() >+ my @system_locales = grep { chomp; not( /^C/ || $_ eq 'POSIX' ) } qx/locale -a/; >+ >+ my @filtered_locales = (); >+ for my $locale (@system_locales) { >+ >+ # Filter for useful locales (UTF-8 ones and common patterns) >+ if ( $locale =~ /^[a-z]{2}_[A-Z]{2}\.utf8?$/i || $locale =~ /^[a-z]{2}_[A-Z]{2}$/i ) { >+ >+ # Create friendly display names >+ my $display_name = $locale; >+ if ( $locale =~ /^([a-z]{2})_([A-Z]{2})/ ) { >+ my %lang_names = ( >+ 'en' => 'English', >+ 'fr' => 'French', >+ 'de' => 'German', >+ 'es' => 'Spanish', >+ 'it' => 'Italian', >+ 'pt' => 'Portuguese', >+ 'nl' => 'Dutch', >+ 'pl' => 'Polish', >+ 'fi' => 'Finnish', >+ 'sv' => 'Swedish', >+ 'da' => 'Danish', >+ 'no' => 'Norwegian', >+ 'ru' => 'Russian', >+ 'ja' => 'Japanese', >+ 'zh' => 'Chinese', >+ 'ar' => 'Arabic', >+ 'hi' => 'Hindi' >+ ); >+ my %country_names = ( >+ 'US' => 'United States', >+ 'GB' => 'United Kingdom', >+ 'FR' => 'France', >+ 'DE' => 'Germany', >+ 'ES' => 'Spain', >+ 'IT' => 'Italy', >+ 'PT' => 'Portugal', >+ 'BR' => 'Brazil', >+ 'NL' => 'Netherlands', >+ 'PL' => 'Poland', >+ 'FI' => 'Finland', >+ 'SE' => 'Sweden', >+ 'DK' => 'Denmark', >+ 'NO' => 'Norway', >+ 'RU' => 'Russia', >+ 'JP' => 'Japan', >+ 'CN' => 'China', >+ 'TW' => 'Taiwan', >+ 'SA' => 'Saudi Arabia', >+ 'IN' => 'India' >+ ); >+ my $lang = $lang_names{$1} || uc($1); >+ my $country = $country_names{$2} || $2; >+ $display_name = "$lang ($country) - $locale"; >+ } >+ push @filtered_locales, { >+ value => $locale, >+ text => $display_name >+ }; >+ } >+ } >+ >+ # Sort locales by display name and add to available list >+ @filtered_locales = sort { $a->{text} cmp $b->{text} } @filtered_locales; >+ push @available_locales, @filtered_locales; >+ >+ return \@available_locales; >+} >+ > 1; >diff --git a/Koha/SearchEngine/Elasticsearch/Search.pm b/Koha/SearchEngine/Elasticsearch/Search.pm >index 50bcef898aa..f9383c379fb 100644 >--- a/Koha/SearchEngine/Elasticsearch/Search.pm >+++ b/Koha/SearchEngine/Elasticsearch/Search.pm >@@ -456,7 +456,6 @@ sub max_result_window { > return $max_result_window; > } > >- > =head2 _sort_facets > > my $facets = _sort_facets($facets); >@@ -469,11 +468,18 @@ 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); >+ # Get locale from system preference, falling back to system LC_COLLATE >+ $locale = C4::Context->preference('FacetSortingLocale') || 'default'; >+ if ( $locale eq 'default' || !$locale ) { >+ >+ #NOTE: When setlocale is run with only the 1st parameter, it is a "get" not a "set" function. >+ $locale = setlocale(LC_COLLATE) || 'default'; >+ } > } >+ > 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}; >@@ -486,7 +492,6 @@ sub _sort_facets { > return $facets; > } > >- > =head2 _convert_facets > > my $koha_facets = _convert_facets($es_facets); >diff --git a/admin/preferences.pl b/admin/preferences.pl >index 34ddca9497d..fdeefc33027 100755 >--- a/admin/preferences.pl >+++ b/admin/preferences.pl >@@ -122,6 +122,12 @@ sub _get_chunk { > } > $chunk->{'languages'} = getTranslatedLanguages( $interface, $theme ); > $chunk->{'type'} = 'languages'; >+ } elsif ( $options{'type'} && $options{'type'} eq 'locale-list' ) { >+ >+ # Dynamic locale detection for facet sorting using Koha::I18N >+ require Koha::I18N; >+ $chunk->{'choices'} = Koha::I18N::available_locales(); >+ $chunk->{'type'} = 'select'; > } elsif ( $options{'choices'} ) { > my $add_blank; > if ( $options{'choices'} && ref( $options{'choices'} ) eq '' ) { >diff --git a/installer/data/mysql/atomicupdate/bug_36947.pl b/installer/data/mysql/atomicupdate/bug_36947.pl >new file mode 100755 >index 00000000000..7f871c73885 >--- /dev/null >+++ b/installer/data/mysql/atomicupdate/bug_36947.pl >@@ -0,0 +1,22 @@ >+use Modern::Perl; >+use Koha::Installer::Output qw(say_warning say_success say_info); >+ >+return { >+ bug_number => "36947", >+ description => "Add FacetSortingLocale system preference for locale-based facet sorting", >+ up => sub { >+ my ($args) = @_; >+ my ( $dbh, $out ) = @$args{qw(dbh out)}; >+ >+ $dbh->do( >+ q{ >+ INSERT IGNORE INTO systempreferences (`variable`,`value`,`options`,`explanation`,`type`) >+ VALUES ('FacetSortingLocale','default','', >+ 'Choose the locale for sorting facet names when FacetOrder is set to Alphabetical. This enables proper Unicode-aware sorting of accented characters and locale-specific alphabetical ordering.', >+ 'Choice') >+ } >+ ); >+ >+ say_success( $out, "Added new system preference 'FacetSortingLocale'" ); >+ }, >+}; >diff --git a/installer/data/mysql/mandatory/sysprefs.sql b/installer/data/mysql/mandatory/sysprefs.sql >index b8f35657707..cc26290b81d 100644 >--- a/installer/data/mysql/mandatory/sysprefs.sql >+++ b/installer/data/mysql/mandatory/sysprefs.sql >@@ -278,6 +278,7 @@ INSERT INTO systempreferences ( `variable`, `value`, `options`, `explanation`, ` > ('FacetLabelTruncationLength','20',NULL,'Specify the facet max length in OPAC','Integer'), > ('FacetMaxCount','20',NULL,'Specify the max facet count for each category','Integer'), > ('FacetOrder','Alphabetical','Alphabetical|Usage','Specify the order of facets within each category','Choice'), >+('FacetSortingLocale','default','','Choose the locale for sorting facet names when FacetOrder is set to Alphabetical. This enables proper Unicode-aware sorting of accented characters and locale-specific alphabetical ordering.','Choice'), > ('FailedLoginAttempts','','','Number of login attempts before lockout the patron account','Integer'), > ('FallbackToSMSIfNoEmail', 0, 'Enable|Disable', 'Send messages by SMS if no patron email is defined', 'YesNo'), > ('FeeOnChangePatronCategory','1','','If set, when a patron changes to a category with enrolment fee, a fee is charged','YesNo'), >diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/i18n_l10n.pref b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/i18n_l10n.pref >index 34a9d542113..e95b3454347 100644 >--- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/i18n_l10n.pref >+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/i18n_l10n.pref >@@ -63,6 +63,11 @@ I18N/L10N: > de: German style ([Address] [Street number] - [ZIP/Postal Code] [City] - [Country]) > fr: French style ([Street number] [Address] - [ZIP/Postal Code] [City] - [Country]) > - . >+ - >+ - Sort facet names using >+ - pref: FacetSortingLocale >+ type: locale-list >+ - locale when FacetOrder is set to Alphabetical. This enables proper Unicode-aware sorting of accented characters and locale-specific alphabetical ordering. > - > - pref: TranslateNotices > choices: >diff --git a/t/Koha/I18N.t b/t/Koha/I18N.t >index 45a34d3a0c1..55e14256805 100755 >--- a/t/Koha/I18N.t >+++ b/t/Koha/I18N.t >@@ -2,7 +2,7 @@ > > use Modern::Perl; > use Test::NoWarnings; >-use Test::More tests => 36; >+use Test::More tests => 37; > use Test::MockModule; > use FindBin qw($Bin); > use Encode; >@@ -61,3 +61,49 @@ my @tests = ( > foreach my $test (@tests) { > is( $test->[0], decode_utf8( $test->[1] ), $test->[1] ); > } >+ >+subtest 'available_locales' => sub { >+ plan tests => 6; >+ >+ # Test basic functionality >+ my $locales = Koha::I18N::available_locales(); >+ >+ # Should return an arrayref >+ is( ref($locales), 'ARRAY', 'available_locales returns an arrayref' ); >+ >+ # Should have at least the default option >+ ok( scalar(@$locales) >= 1, 'At least one locale returned (default)' ); >+ >+ # First locale should be default >+ is( $locales->[0]->{value}, 'default', 'First locale is default' ); >+ is( $locales->[0]->{text}, 'Default Unicode collation', 'Default locale has correct text' ); >+ >+ # All locales should have value and text keys >+ my $all_have_keys = 1; >+ for my $locale (@$locales) { >+ unless ( exists $locale->{value} && exists $locale->{text} ) { >+ $all_have_keys = 0; >+ last; >+ } >+ } >+ ok( $all_have_keys, 'All locales have value and text keys' ); >+ >+ # Test structure for real system locales (if any) >+ my $system_locales = [ grep { $_->{value} ne 'default' } @$locales ]; >+ if (@$system_locales) { >+ >+ # Should have friendly display names for common locales >+ my $has_friendly_name = 0; >+ for my $locale (@$system_locales) { >+ if ( $locale->{text} =~ /^[A-Z][a-z]+ \([^)]+\) - / ) { >+ $has_friendly_name = 1; >+ last; >+ } >+ } >+ ok( $has_friendly_name, 'System locales have friendly display names' ) if @$system_locales; >+ } else { >+ >+ # If no system locales, just pass this test >+ ok( 1, 'No system locales found (test environment)' ); >+ } >+}; >diff --git a/t/Koha/SearchEngine/Elasticsearch/Search.t b/t/Koha/SearchEngine/Elasticsearch/Search.t >index 3e75460589f..cab8c844018 100755 >--- a/t/Koha/SearchEngine/Elasticsearch/Search.t >+++ b/t/Koha/SearchEngine/Elasticsearch/Search.t >@@ -29,7 +29,7 @@ use utf8; > use_ok('Koha::SearchEngine::Elasticsearch::Search'); > > subtest '_sort_facets' => sub { >- plan tests => 2; >+ plan tests => 3; > t::lib::Mocks::mock_preference( 'SearchEngine', 'Elasticsearch' ); > my $facets = [ > { facet_label_value => 'Mary' }, >@@ -81,6 +81,13 @@ subtest '_sort_facets' => sub { > ]; > is_deeply( $sorted_facets, $expected, "Facets sorted correctly with default locale" ); > >+ # Test system preference integration >+ t::lib::Mocks::mock_preference( 'FacetSortingLocale', 'en_US.utf8' ); >+ my $sorted_facets_syspref = $search->_sort_facets( { facets => $facets } ); >+ >+ # Should return sorted facets (exact order may vary by system locale availability) >+ is( ref($sorted_facets_syspref), 'ARRAY', "System preference integration works" ); >+ > #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, >diff --git a/t/db_dependent/Search_FacetSorting.t b/t/db_dependent/Search_FacetSorting.t >new file mode 100755 >index 00000000000..96383ad8a78 >--- /dev/null >+++ b/t/db_dependent/Search_FacetSorting.t >@@ -0,0 +1,68 @@ >+#!/usr/bin/perl >+ >+# This file is part of Koha. >+# >+# Koha is free software; you can redistribute it and/or modify it >+# under the terms of the GNU General Public License as published by >+# the Free Software Foundation; either version 3 of the License, or >+# (at your option) any later version. >+# >+# Koha is distributed in the hope that it will be useful, but >+# WITHOUT ANY WARRANTY; without even the implied warranty of >+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the >+# GNU General Public License for more details. >+# >+# You should have received a copy of the GNU General Public License >+# along with Koha; if not, see <http://www.gnu.org/licenses>. >+ >+use Modern::Perl; >+ >+use Test::More tests => 1; >+use Test::MockModule; >+use t::lib::Mocks; >+use C4::Search; >+ >+use utf8; >+ >+subtest '_sort_facets_zebra with system preference' => sub { >+ plan tests => 3; >+ >+ 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' }, >+ ]; >+ >+ # Test with explicit locale parameter >+ my $sorted_facets_explicit = C4::Search::_sort_facets_zebra( $facets, '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_explicit, $expected, "Zebra facets sorted correctly with explicit locale" ); >+ >+ # Test with system preference >+ t::lib::Mocks::mock_preference( 'FacetSortingLocale', 'default' ); >+ my $sorted_facets_syspref = C4::Search::_sort_facets_zebra($facets); >+ is_deeply( $sorted_facets_syspref, $expected, "Zebra facets sorted correctly with system preference" ); >+ >+ # Test fallback behavior >+ t::lib::Mocks::mock_preference( 'FacetSortingLocale', '' ); >+ my $sorted_facets_fallback = C4::Search::_sort_facets_zebra($facets); >+ is( ref($sorted_facets_fallback), 'ARRAY', "Zebra facets sorting fallback works" ); >+}; >-- >2.49.0
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 36947
:
167110
|
167125
|
167141
|
167142
|
167143
|
167145
|
167146
|
167188
|
167198
|
167880
|
168488
|
170489
|
183551
|
183552
|
183553
|
183563
|
183564
|
183565
|
183566
|
183567
|
183568
|
183726
|
183727