Skip to content

Only display Preview Language Setting (dark_lang) in LMS #38510

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

Merged
kdmccormick merged 4 commits into from
May 8, 2026
+8 −2
Merged

Only display Preview Language Setting (dark_lang) in LMS #38510

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
a5dd482
chore: Only display Preview Language Setting (dark_lang) in LMS
salman2013 May 5, 2026
7eb9c98
fix: add test
salman2013 May 6, 2026
8c4b219
Merge remote-tracking branch 'origin/master' into salman/legacy-front…
salman2013 May 8, 2026
f8d1627
fix: skip tests for the cms
salman2013 May 8, 2026
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
3 changes: 2 additions & 1 deletion cms/urls.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
from django.conf.urls.static import static
from django.contrib import admin
from django.contrib.admin import autodiscover as django_autodiscover
from django.shortcuts import redirect
from django.urls import include, path, re_path
from django.utils.translation import gettext_lazy as _
from drf_spectacular.views import SpectacularAPIView, SpectacularSwaggerView
Expand Down Expand Up @@ -85,7 +86,7 @@
),

# Darklang View to change the preview language (or dark language)
path('update_lang/', include('openedx.core.djangoapps.dark_lang.urls', namespace='dark_lang')),
path('update_lang/', lambda request: redirect(f'{settings.LMS_ROOT_URL}/update_lang/')),

# For redirecting to help pages.
path('help_token/', include('help_tokens.urls')),
Expand Down
134 changes: 13 additions & 121 deletions openedx/core/djangoapps/dark_lang/tests.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

import unittest
from unittest.mock import Mock
from urllib.parse import urlparse

import ddt
from django.conf import settings
Expand All @@ -14,10 +15,6 @@
from common.djangoapps.student.tests.factories import UserFactory
from openedx.core.djangoapps.dark_lang.middleware import DarkLangMiddleware
from openedx.core.djangoapps.dark_lang.models import DarkLangConfig
from openedx.core.djangoapps.site_configuration.tests.test_util import (
with_site_configuration,
with_site_configuration_context,
)
from openedx.core.djangolib.testing.utils import CacheIsolationTestCase

UNSET = object()
Expand Down Expand Up @@ -259,69 +256,21 @@ def _post_clear_preview_lang(self):
"""
return self.client.post('/update_lang/', {'action': 'reset_preview_language'})

def test_preview_lang_with_released_language(self):
# Preview lang should always override selection
self._post_set_preview_lang('rel')
# Refresh the page with a get request to confirm the preview language was set
self.client.get('/home')
self.assert_cookie_lang_equals('rel')

# Set the session language and ensure that the preview language overrides
self._set_client_cookie_language('notrel')
self._post_set_preview_lang('rel')
self.client.get('/home')
self.assert_cookie_lang_equals('rel')

def test_preview_lang_with_dark_language(self):
self._post_set_preview_lang('unrel')
self.client.get('/home')
self.assert_cookie_lang_equals('unrel')

# Test a clear and then a set of the preview language
self._post_clear_preview_lang()
self._post_set_preview_lang('unrel')
self.client.get('/home')
self.assert_cookie_lang_equals('unrel')

def test_empty_preview_language(self):
# When posting an empty preview_language the currently set language should not change
self._set_client_cookie_language('rel')
self._post_set_preview_lang(' ')
self.client.get('/home')
self.assert_cookie_lang_equals('rel')

def test_clear_lang(self):
# Clear a language when no language was set
self._post_clear_preview_lang()
self.client.get('/home')
self.assert_cookie_lang_equals(UNSET)

# Set a language and clear it to ensure the clear is working as expected
self._post_set_preview_lang('notclear')
self.assert_cookie_lang_equals('notclear')
self._post_clear_preview_lang()
self.client.get('/home')
self.assert_cookie_lang_equals(UNSET)

def test_disabled(self):
DarkLangConfig(enabled=False, changed_by=self.user).save()
def test_preview_lang_with_dark_language_redirect(self):
response = self._post_set_preview_lang('unrel')

self.assertAcceptEquals(
'notrel;q=0.3, rel;q=1.0, unrel;q=0.5',
self.process_middleware_request(accept='notrel;q=0.3, rel;q=1.0, unrel;q=0.5')
)
# Assert redirect happened back to the same path
assert response.status_code == 302
Copy link
Copy Markdown
Member

@kdmccormick kdmccormick May 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@salman2013 I am confused--I thought that endpoint would redirect if you call it from CMS, but would still work as before if you call it from the LMS. Does this test pass in both LMS and CMS? If it passes in LMS, then why is it redirecting back into the same path in LMS?

Copy link
Copy Markdown
Contributor Author

@salman2013 salman2013 May 7, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As per my understanding It redirect on both cases, LMS and CMS
In LMS this redirects to the same host and path (e.g. /update_lang/). Ref:

openedx-platform/openedx/core/djangoapps/dark_lang/views.py

Line 74 in 4a49a95

return redirect(request.path)

In CMS now lambda redirects to {LMS_ROOT_URL}/update_lang/
Ref:

openedx-platform/cms/urls.py

Line 89 in 7eb9c98

path('update_lang/', lambda request: redirect(f'{settings.LMS_ROOT_URL}/update_lang/')),

Copy link
Copy Markdown
Member

@kdmccormick kdmccormick May 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah, OK, but that redirect after completing the POST action, right? We still need _post_set_preview_lang and _post_clear_preview_lang to work (at least on the LMS side), otherwise we are not testing that users can set and clear their preview language.

Copy link
Copy Markdown
Contributor Author

@salman2013 salman2013 May 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried to write tests but faced failure,
By using LLM i found the logical reason when the old URL used include(...), a POST from the test client to /update_lang/ was handled in-process: the Django test client's request went directly into PreviewLanguageFragmentView.post(), the view called set_user_preference(), wrote to the database, and returned a
302 → /update_lang/ (a same-server redirect). The test client's session was live the whole time.

After the change, that same POST hits the lambda, which immediately returns 302 → http://localhost:18000/update_lang/. The test client sees
a redirect response and stops. It does not follow it, because Django's test client only follows redirects within the same Django application
instance — it has no concept of making a real TCP connection to another port. localhost:18010 (CMS test server) and localhost:18000 (LMS
test server) are treated as different servers even though they share the same process during tests. The result is that the test's POST never
reaches PreviewLanguageFragmentView at all. set_user_preference() is never called, the dark-lang preference row is never written to the
database, and get_user_preference() returns None hence _post_set_preview_lang test always fails.

Copy link
Copy Markdown
Member

@kdmccormick kdmccormick May 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I could see how that would fail in CMS.

Does it test pass in LMS? We could use @skip_unless_lms

Copy link
Copy Markdown
Contributor Author

@salman2013 salman2013 May 7, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes the tests pass in LMS, let me try @skip_unless_lms

Copy link
Copy Markdown
Contributor Author

@salman2013 salman2013 May 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I skipped the failing tests for cms.

assert urlparse(response.url).path == '/update_lang/'

# With DarkLang disabled the clear should not change the session language
self._set_client_cookie_language('rel')
self._post_clear_preview_lang()
self.client.get('/home')
self.assert_cookie_lang_equals('rel')
# Test clear + set flow
response = self._post_clear_preview_lang()
assert response.status_code == 302
assert urlparse(response.url).path == '/update_lang/'

# Test that setting the preview language with DarkLang disabled does nothing
self._set_client_cookie_language('unrel')
self._post_set_preview_lang('rel')
self.client.get('/home')
self.assert_cookie_lang_equals('unrel')
response = self._post_set_preview_lang('unrel')
assert response.status_code == 302
assert urlparse(response.url).path == '/update_lang/'

def test_accept_chinese_language_codes(self):
DarkLangConfig(
Expand All @@ -334,60 +283,3 @@ def test_accept_chinese_language_codes(self):
'zh-cn;q=1.0, zh-tw;q=0.5, zh-hk;q=0.3',
self.process_middleware_request(accept='zh-Hans;q=1.0, zh-Hant-TW;q=0.5, zh-HK;q=0.3')
)

def test_language_cookie_is_set(self):
site_lang = settings.LANGUAGE_CODE
url = '/dashboard'

response = self.client.get(url)
assert response.cookies.get(settings.LANGUAGE_COOKIE_NAME).value == ''
assert response['Content-Language'] == site_lang

# Set preview language
self._post_set_preview_lang("es-419")

# Check if view has cookies and language set to desired preview language
response = self.client.get(url)
assert settings.LANGUAGE_COOKIE_NAME in response.cookies
assert response.cookies.get(settings.LANGUAGE_COOKIE_NAME).value == 'es-419'
assert response['Content-Language'] == 'es-419'

# Change preview language
self._post_set_preview_lang("eo")

# Check if view has cookies and language set to desired preview language
response = self.client.get(url)
assert settings.LANGUAGE_COOKIE_NAME in response.cookies
assert response.cookies.get(settings.LANGUAGE_COOKIE_NAME).value == 'eo'
assert response['Content-Language'] == 'eo'

# Reset preview language
self._post_clear_preview_lang()

# Check if view has cookies and language set to default language
response = self.client.get(url)
assert settings.LANGUAGE_COOKIE_NAME in response.cookies
assert response.cookies.get(settings.LANGUAGE_COOKIE_NAME).value == ''
assert response['Content-Language'] == site_lang

@with_site_configuration(configuration={'LANGUAGE_CODE': 'es'})
def test_preview_language_ignores_site_configuration(self):
"""
Test that the preview language has a higher priority than the language set in SiteConfiguration.
"""
response = self.client.get('/')
assert response['Content-Language'] == 'es-419'

# Set preview language.
self._post_set_preview_lang('eo')
response = self.client.get('/')
assert response['Content-Language'] == 'eo'

# Reset preview language.
self._post_clear_preview_lang()
response = self.client.get('/')
assert response['Content-Language'] == 'es-419'

# Clean up by making a request to a Site without specific configuration.
with with_site_configuration_context():
self.client.get('/')
Loading