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.

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

Draft
wants to merge 1 commit into
base: ednx-release/mango.master.nelp
Choose a base branch
from

Conversation

johanseto
Copy link
Contributor

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#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.

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.

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.
@johanseto
Copy link
Contributor Author

johanseto commented Dec 7, 2023

@andrey-canon I was trying to replicate this error in my local env and stage and I didn't reached out.
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". If the model of dark lhas is configurable empty in released languages that header become ''.
new_accept_header

image

And if that header is 'empty' then Django in the locale middleware uses the setting LANGUAGE_CODE value.
https://github.com/django/django/blob/main/django/middleware/locale.py#L34

Before

Peek 2023-12-07 14-50

After

Peek 2023-12-07 14-54

@@ -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))
Copy link
Contributor

@OmarIthawi OmarIthawi Dec 8, 2023

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants