-
Notifications
You must be signed in to change notification settings - Fork 7
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’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: first language visit #790
base: ednx-release/mango.master.nelp
Are you sure you want to change the base?
fix: first language visit #790
Conversation
This fix is based in the method `get_language_cookie` default value. https://github.com/openedx/edx-platform/blob/master/openedx/core/djangoapps/lang_pref/helpers.py#L9 So in the first visit if the cookie doesnt exist, the language would be the configure by settings using `LANGUAGE_CODE`.If it is not defined, then would be `None` that is the default of the method.
@andrey-canon I was trying to replicate this error in my local env and stage and I didn't reached out. And if that header is 'empty' then Django in the locale middleware uses the setting BeforeAfter |
@@ -29,7 +29,7 @@ def process_request(self, request): | |||
If a user's UserPreference contains a language preference, use the user's preference. | |||
Save the current language preference cookie as the user's preferred language. | |||
""" | |||
cookie_lang = lang_pref_helpers.get_language_cookie(request) | |||
cookie_lang = lang_pref_helpers.get_language_cookie(request, getattr(settings, "LANGUAGE_CODE", None)) |
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.
Thanks @johanv26. I believe you've spotted the right place.
If the model of dark lhas is configurable empty in released languages that header become ''.
I'm not sure if this is an option because DGA requires two languages with Arabic by default. We also might have sites that are English by default. What do you think?
So I had to read the other middleware of dark_lang and I found that this language issues could be available if the clean-headers clean the header "HTTP_ACCEPT_LANGUAGE".
I believe this is a good solution. Replacing HTTP_ACCEPT_LANGUAGE
with settings.LANGUAGE_CODE
is what we need, but only for non-API URLs because mobile would be Arabic-only otherwise.
I think lang_pref_helpers.get_language_cookie(request, getattr(settings, "LANGUAGE_CODE", None))
is better than dropping HTTP_ACCEPT_LANGUAGE
because it affects only web.
As an optional refactoring note: My understanding that this is either should be an Open edX plugin by upgrading LocalizerX (Hawthorn) or putting it inside https://github.com/eduNEXT/eox-nelp but not in the eduNEXT/edunext-platform
. But I'll leave that decision up to you.
Description
This fix is based in the method
get_language_cookie
default value. https://github.com/openedx/edx-platform/blob/master/openedx/core/djangoapps/lang_pref/helpers.py#L9So in the first visit if the cookie doesnt exist, the language would be the configure by settings using
LANGUAGE_CODE
.If it is not defined, then would beNone
that is the default of the method.Supporting information
Jira ticket:
https://edunext.atlassian.net/jira/software/c/projects/FUTUREX/boards/36?selectedIssue=FUTUREX-640
Testing instructions
Please provide detailed step-by-step instructions for testing this change.