OBPIH-5161 Set locale for every request #3763
Merged
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.
…olving the big spike), fix menu (translations) fetching when switching locales
After a long and exhausting round of investigation I have a very interesting summary of known for the long time issue with
<g:message>
tags not being translatable by the crowdin vs<warehouse:message>
being translatable.After we lost this "workaround" I found for translating
<g:message>
tags when merging the changes to transition in/out to/from localization mode, we again faced an issue with<g:message>
not being translatable by crowdin anywhere (because the “workaround” which we did not really know what it is about, we thought it’s just a random thing was lost). It was a big regression “issue”, because since we had a workaround to translate those and suddenly lost that workaround, it would mean that our previous work (especially Manon’s work with translating) had no-sense. Why I marked the “issue” word inside the quotation mark is that actually it was only a workaround which we didn’t really understand.After many attempts from each of us to find the real cause of
<g:message>
not always being translatable by the crowdin (the spike ticket), I finally found what really caused that issue of<g:message>
not being translatable by the crowdin.Let me explain:
What turned out to be this "workaround" was actually passing the
lang
param to theupdateAuthUserLocale
method (we called this method when changing the language via links in the footer on the GSP side) and it was changing "grails' language", so that the grails and all its tags including<g:message>
knew what the current language was.From what I've read from the
grails_i18n's
docs:source: link (point 3.2)
and since I changed logic a bit for changing the language, so that when changing the language to
Acoli
we were not usingupdateAuthUserLocale
, but actually calling theenableLocalizationMode
explicitly via footer language link/button, we were not passing thelang
param to that action, so the<g:message>
and wholegrails
still didn't know that the language is set toach
, as this information is stored somehow in a cookie. Thislang
param and our session.locale is NOT the same andgrails'
built-in tags did not "pull" the language from session.locale, but they were looking on actuallang
.This is also why I recommended not a long time ago to change the language to
Acoli
on the GSP side in order to make the "workaround" work (because on React side we did not pass thelang
as param when changing the language and that explains why when changing the language toAcoli
on the React side and then going to some GSP pages, the<g:message>
tags were still not translatable - because thelang
param was not passed to the method on the React side).Every issue that we've faced with crowdin so far, especially with this
<g:message>
stuff seems really logical to me after understanding what really caused the issue.Going to what actually needed to be done in the shortest way was to call the
RCU
and setting the locale viaInitializationFilters
(so we don't have to passlang
param for each request of language change).This line:
is actually equal to passing
?lang
as param to a request, so that's why I thought doing in inside theInitializationFilters
is the best idea to have this logic in one place.As for changing the language on obdev and having menu translated to the locale that is equal to user's default locale I couldn't spot it earlier locally, because I think (I “think”, because I do not have access to obdev storage to check it and as Justin was out today and Artur is out, I had to “blind guess” it basing on my experience and intuition) that on obdev this setting in config:
is set to true (it has to be overwritten via
openboxes-config.properties
, because by default it is set to false as I have locally on develop branch) and that's why the first if was not executed and in the further if's insideMessageTagLib
theuser.locale
was "higher" in order thansession.locale
which should be checked firstly in order to choose the proper translation.What needed to be done is just changing the order, so that
session.locale
is checked before thesession.user.locale
.With this fix I also answered for the spike we had for the
g:message
vswarehouse:message
stuff: so that we do not need any workarounds and can change the language/enable localization mode WHEREVER we want and everything should be working fine. I really do feel that this will improve the work with crowdin for Manon and the users, as it’s a big game changer going further.