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.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.
Already on GitHub? Sign in to your account
Thread-safe locale switching #4237
Conversation
locale = if params["locale"] | ||
params["locale"] | ||
elsif current_user && current_user.locale.present? | ||
current_user.locale | ||
end | ||
|
||
I18n.locale = available_locales.include?(locale) ? locale : default_locale | ||
I18n.with_locale(available_locales.include?(locale) ? locale : default_locale, &action) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In case someone checks this code and wonders (like me) where are we changing the user locale, we currently do this here:
def create | |
enforce_permission_to :create, :locales | |
current_user.update!(locale: params["locale"]) if current_user && params["locale"] && available_locales.include?(params["locale"]) | |
redirect_to referer_with_new_locale | |
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense, thanks for the PR! 馃槃
I don't think this requires a Changelog entry. @deivid-rodriguez is this ready for merge? 馃槃 |
Actually I wrote a changelog entry, but forgot to push it! Since this might fix a real end user problem, I think a changelog entry is good! |
06033a2
to
2f0b0db
Compare
Cool, thanks! Merging this! 馃槃 |
馃帺 What? Why?
Decidim is not changing locales in a thread-safe manner, as explained in this rails issue. So after one request changes the locale, another request could accidentally use the changed value.
馃搶 Related Issues
馃搵 Subtasks
CHANGELOG
entry馃摲 Screenshots (optional)
None.