TRUNK-6341: Improved locale matching by using Locale.filter() for bes… #5485
+57
−24
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ibacher
reviewed
Nov 12, 2025
Member
ibacher
left a comment
ibacher
left a comment
There was a problem hiding this comment.
Is it possible to add some tests for this?
| import java.util.ResourceBundle; | ||
| import java.util.Set; | ||
|
|
||
| import java.util.List; |
Member
There was a problem hiding this comment.
Please make sure your IDE is configured to use the standard OpenMRS coding scheme. That should eliminate the blank line changes and put this in the right place or you can do that manually.
| * @param request the incoming HTTP request containing the Accept-Language header | ||
| * @return the best matching available locale based on the requests 'Accept-Language' header | ||
| */ | ||
| public Locale findBestLocale(HttpServletRequest request) { |
Member
There was a problem hiding this comment.
We should also use this in the update filter, I suppose...
Comment on lines
+997
to
+1001
| CustomResourceLoader resourceLoader = CustomResourceLoader.getInstance(httpRequest); | ||
|
|
||
| Locale bestLocale = resourceLoader.findBestLocale(httpRequest); | ||
|
|
||
| httpRequest.getSession().setAttribute(FilterUtil.LOCALE_ATTRIBUTE, bestLocale.toString()); |
Member
There was a problem hiding this comment.
Feels like this could be simplified or at least the added whitespace lines removed.
| httpRequest.getSession().setAttribute(FilterUtil.LOCALE_ATTRIBUTE, Locale.ENGLISH.toString()); | ||
| } | ||
| } | ||
| public void checkLocaleAttributesForFirstTime(HttpServletRequest httpRequest) { |
Member
There was a problem hiding this comment.
Why is the indentation changed here?
Member
|
Also, in the future, the PR template is there to help reviewers review your PR. Please do not ignore it. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR improves locale selection logic used during initialization.Previously, the filter only checked if the user’s preferred locale was directly present in available locales using contains().This caused issues for regional variants like fr-BE, which fell back to English even when fr translations existed.The new implementation uses Locale.LanguageRange.parse() and Locale.filter() to find the best available match, falling back to English only if no match is found.
No new tests added since the issue is low priority and no existing test class for this component.