From 7d3e62e930525d0662834dddbe1fc4db6d1b20df Mon Sep 17 00:00:00 2001 From: Agrendalath Date: Tue, 6 Dec 2022 17:13:54 +0100 Subject: [PATCH 1/2] fix: use language specified in SiteConfiguration This feature was implemented in b01544d690 to replace the session's language in the request. 44ddbdf925 moved the process from the request to the response, which made this feature unusable (because the request was already processed). 44ddbdf925 also made this feature set the language cookie. However, it is overwritten by user preferences. To fix this, we could overwrite the cookie of the response after it's set from user preferences. However, it is not an ideal solution because when users switch between Sites with different languages, the first response will use the language of the previous page. Therefore, this ignores user preferences and alters the cookie of a request instead. --- .../core/djangoapps/dark_lang/middleware.py | 11 ------ openedx/core/djangoapps/dark_lang/tests.py | 37 +++++++++++++------ .../core/djangoapps/lang_pref/middleware.py | 11 +++++- .../lang_pref/tests/test_middleware.py | 33 +++++++++++++++++ 4 files changed, 68 insertions(+), 24 deletions(-) diff --git a/openedx/core/djangoapps/dark_lang/middleware.py b/openedx/core/djangoapps/dark_lang/middleware.py index 36c4798a79..325c7b1e7c 100644 --- a/openedx/core/djangoapps/dark_lang/middleware.py +++ b/openedx/core/djangoapps/dark_lang/middleware.py @@ -17,7 +17,6 @@ from django.utils.deprecation import MiddlewareMixin from openedx.core.djangoapps.dark_lang import DARK_LANGUAGE_KEY from openedx.core.djangoapps.dark_lang.models import DarkLangConfig from openedx.core.djangoapps.lang_pref.helpers import set_language_cookie -from openedx.core.djangoapps.site_configuration.helpers import get_value from openedx.core.djangoapps.user_api.preferences.api import get_user_preference # If django 1.7 or higher is used, the right-side can be updated with new-style codes. @@ -98,20 +97,10 @@ class DarkLangMiddleware(MiddlewareMixin): Apply user's dark lang preference as a cookie for future requests. """ if DarkLangConfig.current().enabled: - self._set_site_or_microsite_language(request, response) self._activate_preview_language(request, response) return response - def _set_site_or_microsite_language(self, request, response): - """ - Apply language specified in site configuration. - """ - language = get_value('LANGUAGE_CODE', None) - if language: - request.session[LANGUAGE_SESSION_KEY] = language - set_language_cookie(request, response, language) - def _fuzzy_match(self, lang_code): """Returns a fuzzy match for lang_code""" match = None diff --git a/openedx/core/djangoapps/dark_lang/tests.py b/openedx/core/djangoapps/dark_lang/tests.py index 8ef114ad78..836376ee7d 100644 --- a/openedx/core/djangoapps/dark_lang/tests.py +++ b/openedx/core/djangoapps/dark_lang/tests.py @@ -14,7 +14,10 @@ from django.utils.translation import LANGUAGE_SESSION_KEY 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 +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 from common.djangoapps.student.tests.factories import UserFactory @@ -258,16 +261,6 @@ class DarkLangMiddlewareTests(CacheIsolationTestCase): session[LANGUAGE_SESSION_KEY] = session_language session.save() - @with_site_configuration(configuration={'LANGUAGE_CODE': 'rel'}) - def test_site_configuration_language(self): - # `LANGUAGE_CODE` in site configuration should override session lang - self._set_client_session_language('notrel') - self.client.get('/home') - self.assert_session_lang_equals( - 'rel', - self.client.session - ) - def test_preview_lang_with_released_language(self): # Preview lang should always override selection self._post_set_preview_lang('rel') @@ -404,3 +397,25 @@ class DarkLangMiddlewareTests(CacheIsolationTestCase): 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('/') diff --git a/openedx/core/djangoapps/lang_pref/middleware.py b/openedx/core/djangoapps/lang_pref/middleware.py index d0a787c5da..e36685fd0a 100644 --- a/openedx/core/djangoapps/lang_pref/middleware.py +++ b/openedx/core/djangoapps/lang_pref/middleware.py @@ -1,8 +1,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 @@ -11,6 +10,7 @@ from openedx.core.djangoapps.dark_lang import DARK_LANGUAGE_KEY from openedx.core.djangoapps.dark_lang.models import DarkLangConfig from openedx.core.djangoapps.lang_pref import LANGUAGE_HEADER, LANGUAGE_KEY from openedx.core.djangoapps.lang_pref import helpers as lang_pref_helpers +from openedx.core.djangoapps.site_configuration.helpers import get_value from openedx.core.djangoapps.user_api.errors import UserAPIInternalError, UserAPIRequestError from openedx.core.djangoapps.user_api.preferences.api import get_user_preference, set_user_preference from openedx.core.lib.mobile_utils import is_request_from_mobile_app @@ -28,6 +28,9 @@ class LanguagePreferenceMiddleware(MiddlewareMixin): """ 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. + + If you specify the LANGUAGE_CODE in SiteConfiguration, it will have a higher priority than the user's language + preference. """ cookie_lang = lang_pref_helpers.get_language_cookie(request) if cookie_lang: @@ -54,6 +57,10 @@ class LanguagePreferenceMiddleware(MiddlewareMixin): if LANGUAGE_SESSION_KEY in request.session and request.session[LANGUAGE_SESSION_KEY] != cookie_lang: del request.session[LANGUAGE_SESSION_KEY] + # Apply language specified in SiteConfiguration, ignoring user preferences. + if language := get_value('LANGUAGE_CODE'): + request.COOKIES[settings.LANGUAGE_COOKIE_NAME] = language + def process_response(self, request, response): # lint-amnesty, pylint: disable=missing-function-docstring # If the user is logged in, check for their language preference. Also check for real user # if current user is a masquerading user, diff --git a/openedx/core/djangoapps/lang_pref/tests/test_middleware.py b/openedx/core/djangoapps/lang_pref/tests/test_middleware.py index b62b05b7d9..6ab218b654 100644 --- a/openedx/core/djangoapps/lang_pref/tests/test_middleware.py +++ b/openedx/core/djangoapps/lang_pref/tests/test_middleware.py @@ -14,8 +14,13 @@ from django.test.client import Client, RequestFactory from django.urls import reverse from django.utils.translation import LANGUAGE_SESSION_KEY from django.utils.translation.trans_real import parse_accept_lang_header + from openedx.core.djangoapps.lang_pref import COOKIE_DURATION, LANGUAGE_KEY from openedx.core.djangoapps.lang_pref.middleware import LanguagePreferenceMiddleware +from openedx.core.djangoapps.site_configuration.tests.test_util import ( + with_site_configuration, + with_site_configuration_context, +) from openedx.core.djangoapps.user_api.preferences.api import ( delete_user_preference, get_user_preference, @@ -267,3 +272,31 @@ class TestUserPreferenceMiddleware(CacheIsolationTestCase): mock_is_mobile_request.return_value = True response = self.middleware.process_response(self.request, response) response.delete_cookie.assert_called() + + @with_site_configuration(configuration={'LANGUAGE_CODE': 'eo'}) + @ddt.data(None, 'es', 'en') + def test_site_language_ignores_user_preferences(self, user_preference): + """ + Test that the language set in SiteConfiguration has a higher priority than user preferences. + It also does not create or update user preferences. + """ + self.request.COOKIES[settings.LANGUAGE_COOKIE_NAME] = user_preference + self.middleware.process_request(self.request) + + # Do not alter user preferences. + assert get_user_preference(self.user, LANGUAGE_KEY) == user_preference + # Change the request's cookie instead. + assert self.request.COOKIES[settings.LANGUAGE_COOKIE_NAME] == 'eo' + + # Use an actual call to determine the language of the response. + response = self.client.get('/') + + assert get_user_preference(self.user, LANGUAGE_KEY) == user_preference + assert response['Content-Language'] == 'eo' + # `LocaleMiddleware` no longer looks for language in the session since Django 3.2. It checks the cookie instead. + # See: https://docs.djangoproject.com/en/3.2/releases/3.0/#miscellaneous + assert self.client.session.get(LANGUAGE_SESSION_KEY) is None + + # Clean up by making a request to a Site without specific configuration. + with with_site_configuration_context(): + self.client.get('/') From 4141fbfda90f525d0d756a9d580742894dc44cbe Mon Sep 17 00:00:00 2001 From: Agrendalath Date: Wed, 7 Dec 2022 19:53:56 +0100 Subject: [PATCH 2/2] test: fix `TestCourseStatusGET` This test stopped throwing the `TransactionManagementError` once we added the `site_configuration.get_value()` call to the language preferences middleware. --- lms/djangoapps/mobile_api/users/tests.py | 22 +++++++++------------- 1 file changed, 9 insertions(+), 13 deletions(-) diff --git a/lms/djangoapps/mobile_api/users/tests.py b/lms/djangoapps/mobile_api/users/tests.py index 9358966468..237bdea157 100644 --- a/lms/djangoapps/mobile_api/users/tests.py +++ b/lms/djangoapps/mobile_api/users/tests.py @@ -501,21 +501,17 @@ class TestCourseStatusGET(CourseStatusAPITestCase, MobileAuthUserTestMixin, assert response.data['last_visited_block_id'] == str(self.unit.location) # Since we are testing an non atomic view in atomic test case, therefore we are expecting error on failures - def api_error_response(self, reverse_args=None, data=None, **kwargs): + def api_atomic_response(self, reverse_args=None, data=None, **kwargs): """ - Same as api response from MobileAPITestCase but handle views which throw errors + Same as the api_response from MobileAPITestCase, but handles the view as an atomic transaction. """ url = self.reverse_url(reverse_args, **kwargs) - try: - with transaction.atomic(): - self.url_method(url, data=data, **kwargs) - assert False - except transaction.TransactionManagementError: - assert True + with transaction.atomic(): + self.url_method(url, data=data, **kwargs) def test_invalid_user(self): self.login_and_enroll() - self.api_error_response(username='no_user') + self.api_atomic_response(username='no_user') def test_other_user(self): # login and enroll as the test user @@ -530,22 +526,22 @@ class TestCourseStatusGET(CourseStatusAPITestCase, MobileAuthUserTestMixin, # now login and call the API as the test user self.login() - self.api_error_response(username=other.username) + self.api_atomic_response(username=other.username) def test_course_not_found(self): non_existent_course_id = CourseKey.from_string('a/b/c') self.init_course_access(course_id=non_existent_course_id) - self.api_error_response(course_id=non_existent_course_id) + self.api_atomic_response(course_id=non_existent_course_id) def test_unenrolled_user(self): self.login() self.unenroll() - self.api_error_response(expected_response_code=None) + self.api_atomic_response(expected_response_code=None) def test_no_auth(self): self.logout() - self.api_error_response() + self.api_atomic_response() class TestCourseStatusPATCH(CourseStatusAPITestCase, MobileAuthUserTestMixin,