From e733179d49f68e96c80db9c28d7167d7f91a51c5 Mon Sep 17 00:00:00 2001 From: Calen Pennington Date: Wed, 12 Apr 2017 11:14:14 -0400 Subject: [PATCH 1/6] Store Learner language preferences into a cookie rather than the session This modifies the lang-pref django app to: a) Use the current value of the 'edx-language-preference' cookie to set the users Accept-Language header on an incoming request. b) At the end of the request, update the 'edx-language-preference' cookie to reflect the users current Language Preference choice, if any. [LEARNER-542] --- common/djangoapps/student/cookies.py | 6 +- .../courseware/tests/test_course_info.py | 4 +- lms/djangoapps/courseware/tests/test_i18n.py | 17 +- lms/envs/devstack_docker.py | 1 + openedx/core/djangoapps/lang_pref/__init__.py | 6 + .../core/djangoapps/lang_pref/middleware.py | 58 ++++-- .../lang_pref/tests/test_middleware.py | 174 +++++++++++------- openedx/core/djangoapps/lang_pref/views.py | 11 +- 8 files changed, 191 insertions(+), 86 deletions(-) diff --git a/common/djangoapps/student/cookies.py b/common/djangoapps/student/cookies.py index 454ed4e486..d204020ab9 100644 --- a/common/djangoapps/student/cookies.py +++ b/common/djangoapps/student/cookies.py @@ -17,7 +17,7 @@ from student.models import CourseEnrollment CREATE_LOGON_COOKIE = Signal(providing_args=['user', 'response']) -def _get_cookie_settings(request): +def standard_cookie_settings(request): """ Returns the common cookie settings (e.g. expiration time). """ if request.session.get_expire_at_browser_close(): @@ -73,7 +73,7 @@ def set_logged_in_cookies(request, response, user): HttpResponse """ - cookie_settings = _get_cookie_settings(request) + cookie_settings = standard_cookie_settings(request) # Backwards compatibility: set the cookie indicating that the user # is logged in. This is just a boolean value, so it's not very useful. @@ -96,7 +96,7 @@ def set_logged_in_cookies(request, response, user): def set_user_info_cookie(response, request): """ Sets the user info cookie on the response. """ - cookie_settings = _get_cookie_settings(request) + cookie_settings = standard_cookie_settings(request) # In production, TLS should be enabled so that this cookie is encrypted # when we send it. We also need to set "secure" to True so that the browser diff --git a/lms/djangoapps/courseware/tests/test_course_info.py b/lms/djangoapps/courseware/tests/test_course_info.py index 9de1a6e56b..0013f5ad02 100644 --- a/lms/djangoapps/courseware/tests/test_course_info.py +++ b/lms/djangoapps/courseware/tests/test_course_info.py @@ -367,7 +367,7 @@ class SelfPacedCourseInfoTestCase(LoginEnrollmentTestCase, SharedModuleStoreTest self.assertEqual(resp.status_code, 200) def test_num_queries_instructor_paced(self): - self.fetch_course_info_with_queries(self.instructor_paced_course, 21, 4) + self.fetch_course_info_with_queries(self.instructor_paced_course, 24, 4) def test_num_queries_self_paced(self): - self.fetch_course_info_with_queries(self.self_paced_course, 21, 4) + self.fetch_course_info_with_queries(self.self_paced_course, 24, 4) diff --git a/lms/djangoapps/courseware/tests/test_i18n.py b/lms/djangoapps/courseware/tests/test_i18n.py index 1d52ae06d7..d7d7fe6b8b 100644 --- a/lms/djangoapps/courseware/tests/test_i18n.py +++ b/lms/djangoapps/courseware/tests/test_i18n.py @@ -2,6 +2,7 @@ Tests i18n in courseware """ +import json import re from django.conf import settings @@ -14,7 +15,6 @@ from nose.plugins.attrib import attr from openedx.core.djangoapps.dark_lang.models import DarkLangConfig from openedx.core.djangoapps.lang_pref import LANGUAGE_KEY -from openedx.core.djangoapps.user_api.preferences.api import set_user_preference from student.models import UserProfile from student.tests.factories import UserFactory @@ -175,6 +175,15 @@ class I18nLangPrefTests(BaseI18nTestCase): UserProfile.objects.create(user=self.user) self.user_login() + def set_lang_preference(self, language): + """Sets the user's language preference, allowing the LangPref middleware to operate to set the preference cookie.""" + response = self.client.patch( + reverse('preferences_api', args=[self.user.username]), + json.dumps({LANGUAGE_KEY: language}), + content_type="application/merge-patch+json" + ) + self.assertEqual(response.status_code, 204) + def test_lang_preference(self): # Regression test; LOC-87 self.release_languages('ar, es-419') @@ -184,13 +193,13 @@ class I18nLangPrefTests(BaseI18nTestCase): self.assert_tag_has_attr(response.content, "html", "lang", self.site_lang) # Set user language preference - set_user_preference(self.user, LANGUAGE_KEY, 'ar') + self.set_lang_preference('ar') # and verify we now get an ar response response = self.client.get(self.url) self.assert_tag_has_attr(response.content, "html", "lang", 'ar') # Verify that switching language preference gives the right language - set_user_preference(self.user, LANGUAGE_KEY, 'es-419') + self.set_lang_preference('es-419') response = self.client.get(self.url) self.assert_tag_has_attr(response.content, "html", "lang", 'es-419') @@ -199,7 +208,7 @@ class I18nLangPrefTests(BaseI18nTestCase): self.release_languages('ar, es-419') # Set user language preference - set_user_preference(self.user, LANGUAGE_KEY, 'ar') + self.set_lang_preference('ar') # Verify preview-lang takes precedence self.client.post(self.preview_language_url, {'preview_lang': 'eo', 'set_language': 'set_language'}) response = self.client.get(self.url) diff --git a/lms/envs/devstack_docker.py b/lms/envs/devstack_docker.py index 4a9b5e00e9..58ecb1fff2 100644 --- a/lms/envs/devstack_docker.py +++ b/lms/envs/devstack_docker.py @@ -30,6 +30,7 @@ FEATURES.update({ 'ENABLE_COURSEWARE_SEARCH': False, 'ENABLE_COURSE_DISCOVERY': False, 'ENABLE_DASHBOARD_SEARCH': False, + 'SHOW_LANGUAGE_SELECTOR': True }) ENABLE_MKTG_SITE = os.environ.get('ENABLE_MARKETING_SITE', False) diff --git a/openedx/core/djangoapps/lang_pref/__init__.py b/openedx/core/djangoapps/lang_pref/__init__.py index 4182bb6978..a68c8a5252 100644 --- a/openedx/core/djangoapps/lang_pref/__init__.py +++ b/openedx/core/djangoapps/lang_pref/__init__.py @@ -4,3 +4,9 @@ Useful information for setting the language preference # this is the UserPreference key for the user's preferred language LANGUAGE_KEY = 'pref-lang' + +LANGUAGE_COOKIE = 'edx-language-preference' + +LANGUAGE_HEADER = 'HTTP_ACCEPT_LANGUAGE' + +COOKIE_DURATION = 14 * 24 * 60 * 60 # 14 days in seconds diff --git a/openedx/core/djangoapps/lang_pref/middleware.py b/openedx/core/djangoapps/lang_pref/middleware.py index e2352f0780..6bc1985434 100644 --- a/openedx/core/djangoapps/lang_pref/middleware.py +++ b/openedx/core/djangoapps/lang_pref/middleware.py @@ -2,12 +2,17 @@ Middleware for Language Preferences """ +from django.conf import settings 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 LANGUAGE_KEY -from openedx.core.djangoapps.lang_pref.api import released_languages -from openedx.core.djangoapps.user_api.preferences.api import get_user_preference, delete_user_preference +from openedx.core.djangoapps.lang_pref import ( + LANGUAGE_KEY, LANGUAGE_COOKIE, LANGUAGE_HEADER, COOKIE_DURATION +) +from openedx.core.djangoapps.user_api.preferences.api import ( + get_user_preference, delete_user_preference, set_user_preference +) +from openedx.core.djangoapps.user_api.errors import UserAPIInternalError, UserAPIRequestError class LanguagePreferenceMiddleware(object): @@ -21,17 +26,46 @@ class LanguagePreferenceMiddleware(object): 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. """ - languages = released_languages() - system_released_languages = [seq[0] for seq in languages] + cookie_lang = request.COOKIES.get(LANGUAGE_COOKIE, None) + if cookie_lang: + if request.user.is_authenticated(): + set_user_preference(request.user, LANGUAGE_KEY, cookie_lang) + accept_header = request.META.get(LANGUAGE_HEADER, None) + if accept_header: + current_langs = parse_accept_lang_header(accept_header) + # Promote the cookie_lang over any language currently in the accept header + current_langs = [(lang, qvalue) for (lang, qvalue) in current_langs if lang != cookie_lang] + current_langs.insert(0, (cookie_lang, 1)) + accept_header = ",".join("{};q={}".format(lang, qvalue) for (lang, qvalue) in current_langs) + else: + accept_header = cookie_lang + request.META[LANGUAGE_HEADER] = accept_header + + def process_response(self, request, response): # If the user is logged in, check for their language preference - if request.user.is_authenticated(): + if getattr(request, 'user', None) and request.user.is_authenticated(): # Get the user's language preference - user_pref = get_user_preference(request.user, LANGUAGE_KEY) - # Set it to the LANGUAGE_SESSION_KEY (Django-specific session setting governing language pref) - if user_pref: - if user_pref in system_released_languages: - request.session[LANGUAGE_SESSION_KEY] = user_pref + try: + user_pref = get_user_preference(request.user, LANGUAGE_KEY) + except (UserAPIRequestError, UserAPIInternalError): + # If we can't find the user preferences, then don't modify the cookie + pass + else: + # Set it in the LANGUAGE_COOKIE + if user_pref: + response.set_cookie( + LANGUAGE_COOKIE, + value=user_pref, + domain=settings.SESSION_COOKIE_DOMAIN, + max_age=COOKIE_DURATION, + ) else: - delete_user_preference(request.user, LANGUAGE_KEY) + response.delete_cookie( + LANGUAGE_COOKIE, + domain=settings.SESSION_COOKIE_DOMAIN + ) + + return response diff --git a/openedx/core/djangoapps/lang_pref/tests/test_middleware.py b/openedx/core/djangoapps/lang_pref/tests/test_middleware.py index 2664073fd4..1b67b015a6 100644 --- a/openedx/core/djangoapps/lang_pref/tests/test_middleware.py +++ b/openedx/core/djangoapps/lang_pref/tests/test_middleware.py @@ -2,17 +2,21 @@ Tests for lang_pref middleware. """ +import itertools import mock import ddt +from django.conf import settings from django.test import TestCase from django.test.client import RequestFactory +from django.http import HttpResponse from django.contrib.sessions.middleware import SessionMiddleware 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 LANGUAGE_KEY +from openedx.core.djangoapps.lang_pref import LANGUAGE_KEY, LANGUAGE_COOKIE, COOKIE_DURATION from openedx.core.djangoapps.lang_pref.middleware import LanguagePreferenceMiddleware -from openedx.core.djangoapps.user_api.preferences.api import set_user_preference, get_user_preference +from openedx.core.djangoapps.user_api.preferences.api import set_user_preference, get_user_preference, delete_user_preference from student.tests.factories import UserFactory from student.tests.factories import AnonymousUserFactory @@ -34,73 +38,117 @@ class TestUserPreferenceMiddleware(TestCase): self.request.META['HTTP_ACCEPT_LANGUAGE'] = 'ar;q=1.0' # pylint: disable=no-member self.session_middleware.process_request(self.request) - def test_no_language_set_in_session_or_prefs(self): - # nothing set in the session or the prefs - self.middleware.process_request(self.request) - self.assertNotIn(LANGUAGE_SESSION_KEY, self.request.session) # pylint: disable=no-member + def test_logout_shouldnt_remove_cookie(self): - @mock.patch( - 'openedx.core.djangoapps.lang_pref.middleware.released_languages', - mock.Mock(return_value=[('eo', 'esperanto')]) - ) - def test_language_in_user_prefs(self): - # language set in the user preferences and not the session - set_user_preference(self.user, LANGUAGE_KEY, 'eo') - self.middleware.process_request(self.request) - self.assertEquals(self.request.session[LANGUAGE_SESSION_KEY], 'eo') # pylint: disable=no-member - - @mock.patch( - 'openedx.core.djangoapps.lang_pref.middleware.released_languages', - mock.Mock(return_value=[('en', 'english'), ('eo', 'esperanto')]) - ) - def test_language_in_session(self): - # language set in both the user preferences and session, - # preference should get precedence. The session will hold the last value, - # which is probably the user's last preference. Look up the updated preference. - - # Dark lang middleware should run after this middleware, so it can - # set a session language as an override of the user's preference. - self.request.session[LANGUAGE_SESSION_KEY] = 'en' # pylint: disable=no-member - set_user_preference(self.user, LANGUAGE_KEY, 'eo') self.middleware.process_request(self.request) - self.assertEquals(self.request.session[LANGUAGE_SESSION_KEY], 'eo') # pylint: disable=no-member - - @mock.patch( - 'openedx.core.djangoapps.lang_pref.middleware.released_languages', - mock.Mock(return_value=[('eo', 'dummy Esperanto'), ('ar', 'arabic'), ('eu-es', 'euskara (Espainia)')]) - ) - @ddt.data('ar;q=1.0', 'eu;q=1.0', 'es-419;q=1.0') - def test_browser_language_in_session_for_unauthenticated_user(self, accept_language): - """ - test: browser language should not be set in user session for unauthenticated user. - """ - self.request.META['HTTP_ACCEPT_LANGUAGE'] = accept_language # pylint: disable=no-member self.request.user = self.anonymous_user - self.middleware.process_request(self.request) - self.assertNotIn(LANGUAGE_SESSION_KEY, self.request.session) # pylint: disable=no-member - @mock.patch( - 'openedx.core.djangoapps.lang_pref.middleware.released_languages', - mock.Mock(return_value=[('en', 'english')]) - ) - def test_browser_language_not_be_in_session(self): - """ - test: browser language should not be set in user session if it is not supported by system. - """ - self.request.user = self.anonymous_user - self.middleware.process_request(self.request) - self.assertNotEqual(self.request.session.get(LANGUAGE_SESSION_KEY), 'ar') # pylint: disable=no-member + response = mock.Mock(spec=HttpResponse) + self.middleware.process_response(self.request, response) - @mock.patch( - 'openedx.core.djangoapps.lang_pref.middleware.released_languages', - mock.Mock(return_value=[('en', 'english'), ('ar', 'arabic')]) - ) - def test_delete_user_lang_preference_not_supported_by_system(self): + response.delete_cookie.assert_not_called() + + @ddt.data(None, 'es', 'en') + def test_preference_setting_changes_cookie(self, lang_pref_out): """ - test: user preferred language has been removed from user preferences model if it is not supported by system - for authenticated users. + Test that the LANGUAGE_COOKIE is always set to the user's current language preferences + at the end of the request, with an expiry that's the same as the users current session cookie. """ - set_user_preference(self.user, LANGUAGE_KEY, 'eo') + if lang_pref_out: + set_user_preference(self.user, LANGUAGE_KEY, lang_pref_out) + else: + delete_user_preference(self.user, LANGUAGE_KEY) + + response = mock.Mock(spec=HttpResponse) + self.middleware.process_response(self.request, response) + + if lang_pref_out: + response.set_cookie.assert_called_with( + LANGUAGE_COOKIE, + value=lang_pref_out, + domain=settings.SESSION_COOKIE_DOMAIN, + max_age=COOKIE_DURATION, + ) + else: + response.delete_cookie.assert_called_with( + LANGUAGE_COOKIE, + domain=settings.SESSION_COOKIE_DOMAIN, + ) + + self.assertNotIn(LANGUAGE_SESSION_KEY, self.request.session) + + @ddt.data(*itertools.product( + (None, 'eo', 'es'), # LANGUAGE_COOKIE + (None, 'es', 'en'), # Language Preference In + )) + @ddt.unpack + @mock.patch('openedx.core.djangoapps.lang_pref.middleware.set_user_preference') + def test_preference_cookie_changes_setting(self, lang_cookie, lang_pref_in, mock_set_user_preference): + self.request.COOKIES[LANGUAGE_COOKIE] = lang_cookie + + if lang_pref_in: + set_user_preference(self.user, LANGUAGE_KEY, lang_pref_in) + else: + delete_user_preference(self.user, LANGUAGE_KEY) + self.middleware.process_request(self.request) - self.assertEqual(get_user_preference(self.request.user, LANGUAGE_KEY), None) + + if lang_cookie is None: + self.assertEqual(mock_set_user_preference.mock_calls, []) + else: + mock_set_user_preference.assert_called_with(self.user, LANGUAGE_KEY, lang_cookie) + + @ddt.data(*( + (logged_in, ) + test_def + for logged_in in (True, False) + for test_def in [ + # (LANGUAGE_COOKIE, LANGUAGE_SESSION_KEY, Accept-Language In, Accept-Language Out) + (None, None, None, None), + (None, 'eo', None, None), + (None, 'eo', 'en', 'en'), + (None, None, 'en', 'en'), + ('en', None, None, 'en'), + ('en', None, 'eo', 'en;q=1.0,eo'), + ('en', None, 'en', 'en'), + ('en', 'eo', 'en', 'en'), + ('en', 'eo', 'eo', 'en;q=1.0,eo') + ] + )) + @ddt.unpack + def test_preference_cookie_overrides_browser(self, logged_in, lang_cookie, lang_session, accept_lang_in, accept_lang_out): + if not logged_in: + self.request.user = self.anonymous_user + if lang_cookie: + self.request.COOKIES[LANGUAGE_COOKIE] = lang_cookie + if lang_session: + self.request.session[LANGUAGE_SESSION_KEY] = lang_session + if accept_lang_in: + self.request.META['HTTP_ACCEPT_LANGUAGE'] = accept_lang_in + else: + del self.request.META['HTTP_ACCEPT_LANGUAGE'] + + self.middleware.process_request(self.request) + + accept_lang_result = self.request.META.get('HTTP_ACCEPT_LANGUAGE') + if accept_lang_result: + accept_lang_result = parse_accept_lang_header(accept_lang_result) + + if accept_lang_out: + accept_lang_out = parse_accept_lang_header(accept_lang_out) + + if accept_lang_out and accept_lang_result: + self.assertItemsEqual(accept_lang_result, accept_lang_out) + else: + self.assertEqual(accept_lang_result, accept_lang_out) + + self.assertEquals(self.request.session.get(LANGUAGE_SESSION_KEY), lang_session) + + def test_process_response_no_user_noop(self): + del self.request.user + response = mock.Mock(spec=HttpResponse) + + result = self.middleware.process_response(self.request, response) + + self.assertIs(result, response) + self.assertEqual(response.mock_calls, []) diff --git a/openedx/core/djangoapps/lang_pref/views.py b/openedx/core/djangoapps/lang_pref/views.py index 0331ff2c8c..fb198876bb 100644 --- a/openedx/core/djangoapps/lang_pref/views.py +++ b/openedx/core/djangoapps/lang_pref/views.py @@ -9,7 +9,7 @@ from django.http import HttpResponse from django.views.decorators.csrf import ensure_csrf_cookie from django.utils.translation import LANGUAGE_SESSION_KEY -from openedx.core.djangoapps.lang_pref import LANGUAGE_KEY +from openedx.core.djangoapps.lang_pref import LANGUAGE_KEY, LANGUAGE_COOKIE, COOKIE_DURATION @ensure_csrf_cookie @@ -17,9 +17,16 @@ def update_session_language(request): """ Update the language session key. """ + response = HttpResponse(200) if request.method == 'PATCH': data = json.loads(request.body) language = data.get(LANGUAGE_KEY, settings.LANGUAGE_CODE) if request.session.get(LANGUAGE_SESSION_KEY, None) != language: request.session[LANGUAGE_SESSION_KEY] = unicode(language) - return HttpResponse(200) + response.set_cookie( + LANGUAGE_COOKIE, + language, + domain=settings.SESSION_COOKIE_DOMAIN, + max_age=COOKIE_DURATION + ) + return response From 4d21fbe6b915f9b0701503c6154872d21f6004eb Mon Sep 17 00:00:00 2001 From: Calen Pennington Date: Wed, 12 Apr 2017 14:24:44 -0400 Subject: [PATCH 2/6] Use UserFactory.create so that it creates a UserProfile for the user --- lms/djangoapps/courseware/tests/test_i18n.py | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/lms/djangoapps/courseware/tests/test_i18n.py b/lms/djangoapps/courseware/tests/test_i18n.py index d7d7fe6b8b..cafa3ed478 100644 --- a/lms/djangoapps/courseware/tests/test_i18n.py +++ b/lms/djangoapps/courseware/tests/test_i18n.py @@ -15,7 +15,6 @@ from nose.plugins.attrib import attr from openedx.core.djangoapps.dark_lang.models import DarkLangConfig from openedx.core.djangoapps.lang_pref import LANGUAGE_KEY -from student.models import UserProfile from student.tests.factories import UserFactory @@ -61,8 +60,7 @@ class BaseI18nTestCase(TestCase): """ # Create one user and save it to the database email = 'test@edx.org' - self.user = UserFactory.build(username='test', email=email, password=self.pwd) - self.user.save() + self.user = UserFactory.create(username='test', email=email, password=self.pwd) def user_login(self): """ @@ -119,7 +117,6 @@ class I18nRegressionTests(BaseI18nTestCase): def test_unreleased_lang_resolution(self): # Regression test; LOC-85 - UserProfile.objects.create(user=self.user) self.release_languages('fa') self.user_login() @@ -136,7 +133,6 @@ class I18nRegressionTests(BaseI18nTestCase): self.assert_tag_has_attr(response.content, "html", "lang", "fa-ir") def test_preview_lang(self): - UserProfile.objects.create(user=self.user) self.user_login() # Regression test; LOC-87 @@ -172,7 +168,6 @@ class I18nLangPrefTests(BaseI18nTestCase): """ def setUp(self): super(I18nLangPrefTests, self).setUp() - UserProfile.objects.create(user=self.user) self.user_login() def set_lang_preference(self, language): From 0c82bae91c19f88d6cec5672dfa1f4337a54dc77 Mon Sep 17 00:00:00 2001 From: Calen Pennington Date: Wed, 12 Apr 2017 15:37:38 -0400 Subject: [PATCH 3/6] Add the caller to user_api caught error logging --- openedx/core/djangoapps/user_api/helpers.py | 8 +++++-- .../djangoapps/user_api/tests/test_helpers.py | 22 +++++++++++++++---- 2 files changed, 24 insertions(+), 6 deletions(-) diff --git a/openedx/core/djangoapps/user_api/helpers.py b/openedx/core/djangoapps/user_api/helpers.py index c2c62d6618..851bf4914c 100644 --- a/openedx/core/djangoapps/user_api/helpers.py +++ b/openedx/core/djangoapps/user_api/helpers.py @@ -6,6 +6,7 @@ from collections import defaultdict from functools import wraps import logging import json +import traceback from django import forms from django.core.serializers.json import DjangoJSONEncoder @@ -65,16 +66,19 @@ def intercept_errors(api_error, ignore_errors=None): LOGGER.warning(msg) raise + caller = traceback.format_stack(limit=2)[0] + # Otherwise, log the error and raise the API-specific error msg = ( u"An unexpected error occurred when calling '{func_name}' " - u"with arguments '{args}' and keyword arguments '{kwargs}': " + u"with arguments '{args}' and keyword arguments '{kwargs}' from {caller}: " u"{exception}" ).format( func_name=func.func_name, args=args, kwargs=kwargs, - exception=ex.developer_message if hasattr(ex, 'developer_message') else repr(ex) + exception=ex.developer_message if hasattr(ex, 'developer_message') else repr(ex), + caller=caller.strip() ) LOGGER.exception(msg) raise api_error(msg) diff --git a/openedx/core/djangoapps/user_api/tests/test_helpers.py b/openedx/core/djangoapps/user_api/tests/test_helpers.py index a0983a0573..00576d588e 100644 --- a/openedx/core/djangoapps/user_api/tests/test_helpers.py +++ b/openedx/core/djangoapps/user_api/tests/test_helpers.py @@ -2,6 +2,7 @@ Tests for helper functions. """ import json +import re import mock import ddt from django import forms @@ -52,22 +53,35 @@ class InterceptErrorsTest(TestCase): @mock.patch('openedx.core.djangoapps.user_api.helpers.LOGGER') def test_logs_errors(self, mock_logger): + self.maxDiff = None exception = 'openedx.core.djangoapps.user_api.tests.test_helpers.FakeInputException' expected_log_msg = ( u"An unexpected error occurred when calling 'intercepted_function' with arguments '()' and " - u"keyword arguments '{'raise_error': }': FakeInputException()" - ) + u"keyword arguments '{{'raise_error': }}' " + u"from File \"{}\", line XXX, in test_logs_errors\n" + u" intercepted_function(raise_error=FakeInputException): FakeInputException()" + ).format(exception, __file__) # Verify that the raised exception has the error message try: intercepted_function(raise_error=FakeInputException) except FakeOutputException as ex: - self.assertEqual(ex.message, expected_log_msg) + actual_message = re.sub(r'line \d+', 'line XXX', ex.message, flags=re.MULTILINE) + self.assertEqual(actual_message, expected_log_msg) # Verify that the error logger is called # This will include the stack trace for the original exception # because it's called with log level "ERROR" - mock_logger.exception.assert_called_once_with(expected_log_msg) + calls = mock_logger.exception.mock_calls + self.assertEqual(len(calls), 1) + name, args, kwargs = calls[0] + + self.assertEqual(name, '') + self.assertEqual(len(args), 1) + self.assertEqual(kwargs, {}) + + actual_message = re.sub(r'line \d+', 'line XXX', args[0], flags=re.MULTILINE) + self.assertEqual(actual_message, expected_log_msg) class FormDescriptionTest(TestCase): From 4cc23bc6416f3892787d7c258690d04afa6549ae Mon Sep 17 00:00:00 2001 From: Calen Pennington Date: Wed, 12 Apr 2017 15:38:00 -0400 Subject: [PATCH 4/6] Attempt to reduce the number of database calls for user_api serialization --- openedx/core/djangoapps/user_api/serializers.py | 7 +------ openedx/core/djangoapps/user_api/views.py | 6 +++--- 2 files changed, 4 insertions(+), 9 deletions(-) diff --git a/openedx/core/djangoapps/user_api/serializers.py b/openedx/core/djangoapps/user_api/serializers.py index f46197cb95..2ba351f4f2 100644 --- a/openedx/core/djangoapps/user_api/serializers.py +++ b/openedx/core/djangoapps/user_api/serializers.py @@ -22,12 +22,7 @@ class UserSerializer(serializers.HyperlinkedModelSerializer): """ Return the name attribute from the user profile object if profile exists else none """ - try: - profile = UserProfile.objects.get(user=user) - except UserProfile.DoesNotExist: - return None - - return profile.name + return user.profile.name def get_preferences(self, user): """ diff --git a/openedx/core/djangoapps/user_api/views.py b/openedx/core/djangoapps/user_api/views.py index e6125d8e6b..5da932dad6 100644 --- a/openedx/core/djangoapps/user_api/views.py +++ b/openedx/core/djangoapps/user_api/views.py @@ -956,7 +956,7 @@ class UserViewSet(viewsets.ReadOnlyModelViewSet): """ authentication_classes = (authentication.SessionAuthentication,) permission_classes = (ApiKeyHeaderPermission,) - queryset = User.objects.all().prefetch_related("preferences") + queryset = User.objects.all().prefetch_related("preferences").select_related("profile") serializer_class = UserSerializer paginate_by = 10 paginate_by_param = "page_size" @@ -982,7 +982,7 @@ class ForumRoleUsersListView(generics.ListAPIView): raise ParseError('course_id must be specified') course_id = SlashSeparatedCourseKey.from_deprecated_string(course_id_string) role = Role.objects.get_or_create(course_id=course_id, name=name)[0] - users = role.users.all() + users = role.users.prefetch_related("preferences").select_related("profile").all() return users @@ -1011,7 +1011,7 @@ class PreferenceUsersListView(generics.ListAPIView): paginate_by_param = "page_size" def get_queryset(self): - return User.objects.filter(preferences__key=self.kwargs["pref_key"]).prefetch_related("preferences") + return User.objects.filter(preferences__key=self.kwargs["pref_key"]).prefetch_related("preferences").select_related("profile") class UpdateEmailOptInPreference(APIView): From f8b050a5d8fd2808e24b0e9c89a47a2d6996e4b0 Mon Sep 17 00:00:00 2001 From: Calen Pennington Date: Mon, 24 Apr 2017 13:03:17 -0400 Subject: [PATCH 5/6] Put LANGUAGE_COOKIE into settings --- cms/envs/aws.py | 2 ++ cms/envs/common.py | 2 ++ lms/envs/aws.py | 2 ++ lms/envs/common.py | 2 ++ openedx/core/djangoapps/lang_pref/__init__.py | 2 -- openedx/core/djangoapps/lang_pref/middleware.py | 8 ++++---- .../core/djangoapps/lang_pref/tests/test_middleware.py | 10 +++++----- openedx/core/djangoapps/lang_pref/views.py | 4 ++-- 8 files changed, 19 insertions(+), 13 deletions(-) diff --git a/cms/envs/aws.py b/cms/envs/aws.py index 9285afe7e5..68f6237cf3 100644 --- a/cms/envs/aws.py +++ b/cms/envs/aws.py @@ -221,6 +221,8 @@ GIT_REPO_EXPORT_DIR = ENV_TOKENS.get('GIT_REPO_EXPORT_DIR', '/edx/var/edxapp/exp # Translation overrides LANGUAGES = ENV_TOKENS.get('LANGUAGES', LANGUAGES) LANGUAGE_CODE = ENV_TOKENS.get('LANGUAGE_CODE', LANGUAGE_CODE) +LANGUAGE_COOKIE = ENV_TOKENS.get('LANGUAGE_COOKIE', LANGUAGE_COOKIE) + USE_I18N = ENV_TOKENS.get('USE_I18N', USE_I18N) ENV_FEATURES = ENV_TOKENS.get('FEATURES', {}) diff --git a/cms/envs/common.py b/cms/envs/common.py index eecd597497..7424af8803 100644 --- a/cms/envs/common.py +++ b/cms/envs/common.py @@ -547,6 +547,8 @@ TIME_ZONE = 'America/New_York' # http://en.wikipedia.org/wiki/List_of_tz_zones_ LANGUAGE_CODE = 'en' # http://www.i18nguy.com/unicode/language-identifiers.html LANGUAGES_BIDI = lms.envs.common.LANGUAGES_BIDI +LANGUAGE_COOKIE = lms.envs.common.LANGUAGE_COOKIE + LANGUAGES = lms.envs.common.LANGUAGES LANGUAGE_DICT = dict(LANGUAGES) diff --git a/lms/envs/aws.py b/lms/envs/aws.py index de1b4a800e..a88fcd1873 100644 --- a/lms/envs/aws.py +++ b/lms/envs/aws.py @@ -314,6 +314,8 @@ TIME_ZONE = ENV_TOKENS.get('TIME_ZONE', TIME_ZONE) LANGUAGES = ENV_TOKENS.get('LANGUAGES', LANGUAGES) LANGUAGE_DICT = dict(LANGUAGES) LANGUAGE_CODE = ENV_TOKENS.get('LANGUAGE_CODE', LANGUAGE_CODE) +LANGUAGE_COOKIE = ENV_TOKENS.get('LANGUAGE_COOKIE', LANGUAGE_COOKIE) + USE_I18N = ENV_TOKENS.get('USE_I18N', USE_I18N) # Additional installed apps diff --git a/lms/envs/common.py b/lms/envs/common.py index a8b6d94d50..1da86a7022 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -860,6 +860,8 @@ LANGUAGE_CODE = 'en' # http://www.i18nguy.com/unicode/language-identifiers.html # these languages display right to left LANGUAGES_BIDI = ("he", "ar", "fa", "ur", "fa-ir", "rtl") +LANGUAGE_COOKIE = "openedx-language-preference" + # Sourced from http://www.localeplanet.com/icu/ and wikipedia LANGUAGES = ( ('en', u'English'), diff --git a/openedx/core/djangoapps/lang_pref/__init__.py b/openedx/core/djangoapps/lang_pref/__init__.py index a68c8a5252..a72ca0bb3b 100644 --- a/openedx/core/djangoapps/lang_pref/__init__.py +++ b/openedx/core/djangoapps/lang_pref/__init__.py @@ -5,8 +5,6 @@ Useful information for setting the language preference # this is the UserPreference key for the user's preferred language LANGUAGE_KEY = 'pref-lang' -LANGUAGE_COOKIE = 'edx-language-preference' - LANGUAGE_HEADER = 'HTTP_ACCEPT_LANGUAGE' COOKIE_DURATION = 14 * 24 * 60 * 60 # 14 days in seconds diff --git a/openedx/core/djangoapps/lang_pref/middleware.py b/openedx/core/djangoapps/lang_pref/middleware.py index 6bc1985434..a0d89acfc4 100644 --- a/openedx/core/djangoapps/lang_pref/middleware.py +++ b/openedx/core/djangoapps/lang_pref/middleware.py @@ -7,7 +7,7 @@ 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 ( - LANGUAGE_KEY, LANGUAGE_COOKIE, LANGUAGE_HEADER, COOKIE_DURATION + LANGUAGE_KEY, LANGUAGE_HEADER, COOKIE_DURATION ) from openedx.core.djangoapps.user_api.preferences.api import ( get_user_preference, delete_user_preference, set_user_preference @@ -28,7 +28,7 @@ class LanguagePreferenceMiddleware(object): 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 = request.COOKIES.get(LANGUAGE_COOKIE, None) + cookie_lang = request.COOKIES.get(settings.LANGUAGE_COOKIE, None) if cookie_lang: if request.user.is_authenticated(): set_user_preference(request.user, LANGUAGE_KEY, cookie_lang) @@ -57,14 +57,14 @@ class LanguagePreferenceMiddleware(object): # Set it in the LANGUAGE_COOKIE if user_pref: response.set_cookie( - LANGUAGE_COOKIE, + settings.LANGUAGE_COOKIE, value=user_pref, domain=settings.SESSION_COOKIE_DOMAIN, max_age=COOKIE_DURATION, ) else: response.delete_cookie( - LANGUAGE_COOKIE, + settings.LANGUAGE_COOKIE, domain=settings.SESSION_COOKIE_DOMAIN ) diff --git a/openedx/core/djangoapps/lang_pref/tests/test_middleware.py b/openedx/core/djangoapps/lang_pref/tests/test_middleware.py index 1b67b015a6..88f5280efc 100644 --- a/openedx/core/djangoapps/lang_pref/tests/test_middleware.py +++ b/openedx/core/djangoapps/lang_pref/tests/test_middleware.py @@ -14,7 +14,7 @@ from django.contrib.sessions.middleware import SessionMiddleware 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 LANGUAGE_KEY, LANGUAGE_COOKIE, COOKIE_DURATION +from openedx.core.djangoapps.lang_pref import LANGUAGE_KEY, COOKIE_DURATION from openedx.core.djangoapps.lang_pref.middleware import LanguagePreferenceMiddleware from openedx.core.djangoapps.user_api.preferences.api import set_user_preference, get_user_preference, delete_user_preference from student.tests.factories import UserFactory @@ -65,14 +65,14 @@ class TestUserPreferenceMiddleware(TestCase): if lang_pref_out: response.set_cookie.assert_called_with( - LANGUAGE_COOKIE, + settings.LANGUAGE_COOKIE, value=lang_pref_out, domain=settings.SESSION_COOKIE_DOMAIN, max_age=COOKIE_DURATION, ) else: response.delete_cookie.assert_called_with( - LANGUAGE_COOKIE, + settings.LANGUAGE_COOKIE, domain=settings.SESSION_COOKIE_DOMAIN, ) @@ -85,7 +85,7 @@ class TestUserPreferenceMiddleware(TestCase): @ddt.unpack @mock.patch('openedx.core.djangoapps.lang_pref.middleware.set_user_preference') def test_preference_cookie_changes_setting(self, lang_cookie, lang_pref_in, mock_set_user_preference): - self.request.COOKIES[LANGUAGE_COOKIE] = lang_cookie + self.request.COOKIES[settings.LANGUAGE_COOKIE] = lang_cookie if lang_pref_in: set_user_preference(self.user, LANGUAGE_KEY, lang_pref_in) @@ -120,7 +120,7 @@ class TestUserPreferenceMiddleware(TestCase): if not logged_in: self.request.user = self.anonymous_user if lang_cookie: - self.request.COOKIES[LANGUAGE_COOKIE] = lang_cookie + self.request.COOKIES[settings.LANGUAGE_COOKIE] = lang_cookie if lang_session: self.request.session[LANGUAGE_SESSION_KEY] = lang_session if accept_lang_in: diff --git a/openedx/core/djangoapps/lang_pref/views.py b/openedx/core/djangoapps/lang_pref/views.py index fb198876bb..cd47008c8a 100644 --- a/openedx/core/djangoapps/lang_pref/views.py +++ b/openedx/core/djangoapps/lang_pref/views.py @@ -9,7 +9,7 @@ from django.http import HttpResponse from django.views.decorators.csrf import ensure_csrf_cookie from django.utils.translation import LANGUAGE_SESSION_KEY -from openedx.core.djangoapps.lang_pref import LANGUAGE_KEY, LANGUAGE_COOKIE, COOKIE_DURATION +from openedx.core.djangoapps.lang_pref import LANGUAGE_KEY, COOKIE_DURATION @ensure_csrf_cookie @@ -24,7 +24,7 @@ def update_session_language(request): if request.session.get(LANGUAGE_SESSION_KEY, None) != language: request.session[LANGUAGE_SESSION_KEY] = unicode(language) response.set_cookie( - LANGUAGE_COOKIE, + settings.LANGUAGE_COOKIE, language, domain=settings.SESSION_COOKIE_DOMAIN, max_age=COOKIE_DURATION From dffcb13c0e6642a42a413da4461db29cd9212f0e Mon Sep 17 00:00:00 2001 From: Calen Pennington Date: Tue, 25 Apr 2017 11:46:52 -0400 Subject: [PATCH 6/6] Validate that the language cookie is preserved during logout --- .../lang_pref/tests/test_middleware.py | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/openedx/core/djangoapps/lang_pref/tests/test_middleware.py b/openedx/core/djangoapps/lang_pref/tests/test_middleware.py index 88f5280efc..e70f84e9d9 100644 --- a/openedx/core/djangoapps/lang_pref/tests/test_middleware.py +++ b/openedx/core/djangoapps/lang_pref/tests/test_middleware.py @@ -8,6 +8,7 @@ import mock import ddt from django.conf import settings from django.test import TestCase +from django.core.urlresolvers import reverse from django.test.client import RequestFactory from django.http import HttpResponse from django.contrib.sessions.middleware import SessionMiddleware @@ -144,6 +145,23 @@ class TestUserPreferenceMiddleware(TestCase): self.assertEquals(self.request.session.get(LANGUAGE_SESSION_KEY), lang_session) + @ddt.data(None, 'es', 'en') + def test_logout_preserves_cookie(self, lang_cookie): + if lang_cookie: + self.client.cookies[settings.LANGUAGE_COOKIE] = lang_cookie + elif settings.LANGUAGE_COOKIE in self.client.cookies: + del self.client.cookies[settings.LANGUAGE_COOKIE] + # Use an actual call to the logout endpoint, because the logout function + # explicitly clears all cookies + self.client.get(reverse('logout')) + if lang_cookie: + self.assertEqual( + self.client.cookies[settings.LANGUAGE_COOKIE].value, + lang_cookie + ) + else: + self.assertNotIn(settings.LANGUAGE_COOKIE, self.client.cookies) + def test_process_response_no_user_noop(self): del self.request.user response = mock.Mock(spec=HttpResponse)