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

fix: first language visit #790

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

fix: first language visit #790

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions openedx/core/djangoapps/lang_pref/middleware.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
Middleware for Language Preferences
"""


from django.conf import settings
from django.utils.deprecation import MiddlewareMixin
from django.utils.translation import LANGUAGE_SESSION_KEY
from django.utils.translation.trans_real import parse_accept_lang_header
Expand All @@ -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 •
edited
Loading

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.

if cookie_lang:
if request.user.is_authenticated:
# DarkLangMiddleware will take care of this so don't change anything
Expand Down
Loading