Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[#212] Locale-unspecified message properties to default to 'en' messages. #213

Merged
merged 6 commits into from
Dec 21, 2022
Merged

[#212] Locale-unspecified message properties to default to 'en' messages. #213

merged 6 commits into from
Dec 21, 2022

Conversation

mogoodrich
Copy link
Contributor

…ffix as Englis

@mogoodrich mogoodrich mentioned this pull request Dec 16, 2022
Closed
@mogoodrich
Copy link
Contributor Author

Closing this, this needs to be tweaked/fixed

@mogoodrich mogoodrich closed this Dec 16, 2022
@mogoodrich mogoodrich reopened this Dec 16, 2022
@mogoodrich
Copy link
Contributor Author

My original PR preferred English over the default locale set via OpenMRS global property (and caused a test failure). New PR with what is hopefully the proper fix and a new test.

mseaton
mseaton requested changes Dec 17, 2022
View reviewed changes
Copy link
Collaborator

@mseaton mseaton left a comment

Choose a reason for hiding this comment

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

One minor comment to remove a bad import. Also should add a bullet point to the release notes describing the change.

api/src/test/java/org/openmrs/module/initializer/InitializerMessageSourceIntegrationTest.java Outdated Show resolved Hide resolved
@mogoodrich
Copy link
Contributor Author

FYI, looks like the build error is unrelated to this commit? "Error: Failed to execute goal on project initializer-api-bahmni: Could not resolve dependencies for project org.openmrs.module:initializer-api-bahmni:jar:2.4.1-SNAPSHOT: The following artifacts could not be resolved: org.bahmni.module:bahmnicore-api:jar:0.93-1.1.0, org.openmrs.module:episodes-api:jar:1.0-1.0.0: Could not find artifact org.bahmni.module:bahmnicore-api:jar:0.93-1.1.0 in openmrs-repo (https://mavenrepo.openmrs.org/public) -> [Help 1]"

@mogoodrich
Copy link
Contributor Author

This should be ready to go now @mks-d @mseaton @ibacher

mseaton
mseaton approved these changes Dec 20, 2022
View reviewed changes
Copy link
Collaborator

@mseaton mseaton left a comment

Choose a reason for hiding this comment

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

Looks good to me. @ibacher / @mks-d - happy to provide some context on this. Would be good to get this bug fix merged in sooner rather than later.

@mseaton mseaton requested a review from ibacher December 20, 2022 20:19
@mogoodrich
Copy link
Contributor Author

Thanks @mseaton ... yes @ibacher @mks-d it's a rather serious bug... ironically, it means that if your OS locale is something other than English, when people set their OpenMRS locale to that language, the site generally shows up in English instead.

@mks-d mks-d changed the title [#212] Message Source should interpret messages.properties with no su… [#212] Locale-unspecified message properties to default to 'en' messages. Dec 21, 2022
@mks-d mks-d removed the request for review from ibacher December 21, 2022 09:16
@mks-d mks-d merged commit e29810d into mekomsolutions:master Dec 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mseaton mseaton mseaton approved these changes

@mks-d mks-d mks-d approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@mogoodrich @mseaton @mks-d