From 4c8c0a5b60caa0f41bd0423024b4fabb4cfa837c Mon Sep 17 00:00:00 2001 From: Martin Renvoize 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: The locale identifier (e.g., 'en_US.utf8', 'default') + - C: 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 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 . + +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