Skip to content

Sort filter values in a locale aware way#1262

Open
boo-code wants to merge 2 commits into
PrestaShop:devfrom
boo-code:fix/locale-aware-value-sort-1201
Open

Sort filter values in a locale aware way#1262
boo-code wants to merge 2 commits into
PrestaShop:devfrom
boo-code:fix/locale-aware-value-sort-1201

Conversation

@boo-code

Copy link
Copy Markdown
Contributor
Questions Answers
Description? Feature filter values were ordered with natcasesort() (in Filters\Block::sortFeatureBlock()), which compares strings byte by byte. Multi-byte accented characters (Czech č, š, ř, …) therefore sorted after every plain ASCII letter instead of next to their base letter, so e.g. Černá landed at the bottom of the list rather than right after Czech. The fix sorts with a Collator built from the current language locale (when the intl extension is available), using a secondary strength (case-insensitive, as natcasesort was) and numeric collation (to keep the natural 2-before-10 ordering). It falls back to natcasesort() when intl is unavailable.
Type? bug fix
BC breaks? no
Deprecations? no
Fixed ticket? Fixes #1201.
How to test? A unit test is included (BlockTest::testSortFeatureBlockSortsValuesLocaleAware): with locale cs-CZ, a feature whose values are Value, Else, Beta, Černá, Czech, Other, Alpha is sorted to Alpha, Beta, Czech, Černá, Else, Other, Value (red before / green after). Manually: create a feature with Czech accented values (č, š, ř, …), enable it as a filter, and check the storefront filter block — the values are now alphabetically ordered per the language instead of pushing accented ones to the end.
Sponsor company

Reported by @Hlavtox. Verified that Collator(cs-CZ) reproduces the exact expected order from the issue (Alpha, Beta, Czech, Černá, Else, Other, Value), whereas natcasesort() produces Alpha, Beta, Czech, Else, Other, Value, Černá.

Feature filter values were ordered with natcasesort(), which compares strings
byte by byte. Multi-byte accented characters (Czech č, š, ř, etc.) therefore
sorted after every plain ASCII letter instead of next to their base letter, so a
value like "Černá" ended up at the bottom of the list rather than near "Czech".

Use a Collator built from the current language locale when the intl extension is
available, with a secondary strength (case insensitive, like before) and numeric
collation (to keep the natural 2-before-10 ordering). Fall back to natcasesort()
when intl is not present.
@ps-jarvis

Copy link
Copy Markdown

Hello @boo-code!

This is your first pull request on ps_facetedsearch repository of the PrestaShop project.

Thank you, and welcome to this Open Source community!

@github-project-automation github-project-automation Bot moved this to Ready for review in PR Dashboard Jun 27, 2026
kpodemski
kpodemski previously approved these changes Jul 6, 2026
@ps-jarvis ps-jarvis added the Waiting for QA Status: Action required, Waiting for test feedback label Jul 6, 2026
@ps-jarvis ps-jarvis moved this from Ready for review to To be tested in PR Dashboard Jul 6, 2026
@kpodemski kpodemski added the Waiting for QA by Community Status: Action required, Waiting for test feedback by Community label Jul 6, 2026
@kpodemski kpodemski added this to the 5.1.0 milestone Jul 7, 2026
@kpodemski

kpodemski commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

thanks @boo-code

CI is red.

Details:

 @php -d date.timezone=UTC ./vendor/bin/phpunit -c tests/php/phpunit.xml
PHPUnit 5.7.27 by Sebastian Bergmann and contributors.

.......................................................E.......  63 / 101 ( 62%)
......................................                          101 / 101 (100%)

Time: 226 ms, Memory: 16.00MB

There was 1 error:

1) PrestaShop\Module\FacetedSearch\Tests\Filters\BlockTest::testGetFiltersBlockWithoutFeaturesWithoutSearchFilterAndFeatures
Undefined property: stdClass::$locale

/home/runner/work/ps_facetedsearch/ps_facetedsearch/src/Filters/Block.php:950
/home/runner/work/ps_facetedsearch/ps_facetedsearch/src/Filters/Block.php:909
/home/runner/work/ps_facetedsearch/ps_facetedsearch/src/Filters/Block.php:895
/home/runner/work/ps_facetedsearch/ps_facetedsearch/src/Filters/Block.php:149
/home/runner/work/ps_facetedsearch/ps_facetedsearch/tests/php/FacetedSearch/Filters/BlockTest.php:1043
phpvfscomposer:///home/runner/work/ps_facetedsearch/ps_facetedsearch/vendor/phpunit/phpunit/phpunit:51

ERRORS!
Tests: 101, Assertions: 923, Errors: 1.
Script @php -d date.timezone=UTC ./vendor/bin/phpunit -c tests/php/phpunit.xml handling the test event returned with error code 2

@kpodemski kpodemski added waiting for author Waiting for author's feedback and removed Waiting for QA Status: Action required, Waiting for test feedback Waiting for QA by Community Status: Action required, Waiting for test feedback by Community labels Jul 7, 2026
getValuesCollator() read $this->context->language->locale unconditionally, so
any call path where the language carries no locale raised an Undefined property
notice (surfaced by the existing BlockTest fixtures, whose context language has
no locale). Guard the access and fall back to natcasesort when no locale is set,
which also protects a real store whose language has an empty locale configured.
@boo-code

boo-code commented Jul 7, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for catching that. Fixed in f51ef4d: getValuesCollator() read $this->context->language->locale unconditionally, which raised an Undefined property notice on the fixtures whose language carries no locale. It now guards the access and falls back to natcasesort() when no locale is set. PHPUnit is green again.

@Hlavtox Hlavtox left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oooooh lalaaa, nice!!!!

@ps-jarvis ps-jarvis added the Waiting for QA Status: Action required, Waiting for test feedback label Jul 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

waiting for author Waiting for author's feedback Waiting for QA Status: Action required, Waiting for test feedback

Projects

Status: To be tested

Development

Successfully merging this pull request may close these issues.

[BUG] Sorting of values is broken

4 participants