From 33fee20c3a84f313f31d2e8ff5b5a4a23d53e019 Mon Sep 17 00:00:00 2001 From: Calen Pennington <cale@edx.org> Date: Wed, 26 Apr 2017 11:41:39 -0400 Subject: [PATCH] Reapply "Merge pull request #14868 from cpennington/learner-542" This reverts commit 65f13ddec4da6b6c872c5408a6294f9352fb9a08. --- cms/envs/aws.py | 2 ++ cms/envs/common.py | 2 ++ common/djangoapps/student/cookies.py | 6 +++--- lms/djangoapps/courseware/tests/test_course_info.py | 4 ++-- lms/djangoapps/courseware/tests/test_i18n.py | 24 ++++++++++++++---------- lms/envs/aws.py | 2 ++ lms/envs/common.py | 2 ++ lms/envs/devstack_docker.py | 1 + openedx/core/djangoapps/lang_pref/__init__.py | 4 ++++ openedx/core/djangoapps/lang_pref/middleware.py | 58 ++++++++++++++++++++++++++++++++++++++++++++++------------ openedx/core/djangoapps/lang_pref/tests/test_middleware.py | 192 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++--------------------------------------------------------------- openedx/core/djangoapps/lang_pref/views.py | 11 +++++++++-- openedx/core/djangoapps/user_api/helpers.py | 8 ++++++-- openedx/core/djangoapps/user_api/serializers.py | 7 +------ openedx/core/djangoapps/user_api/tests/test_helpers.py | 22 ++++++++++++++++++---- openedx/core/djangoapps/user_api/views.py | 6 +++--- 16 files changed, 244 insertions(+), 107 deletions(-) diff --git a/cms/envs/aws.py b/cms/envs/aws.py index 9285afe..68f6237 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 eecd597..7424af8 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/common/djangoapps/student/cookies.py b/common/djangoapps/student/cookies.py index 454ed4e..d204020 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 9de1a6e..0013f5a 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 1d52ae0..cafa3ed 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,8 +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 @@ -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,9 +168,17 @@ 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): + """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 +188,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 +203,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/aws.py b/lms/envs/aws.py index ee35092..526b733 100644 --- a/lms/envs/aws.py +++ b/lms/envs/aws.py @@ -315,6 +315,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 3cbd9ca..cf4fed8 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -863,6 +863,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/lms/envs/devstack_docker.py b/lms/envs/devstack_docker.py index 4a9b5e0..58ecb1f 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 4182bb6..a72ca0b 100644 --- a/openedx/core/djangoapps/lang_pref/__init__.py +++ b/openedx/core/djangoapps/lang_pref/__init__.py @@ -4,3 +4,7 @@ Useful information for setting the language preference # this is the UserPreference key for the user's preferred language LANGUAGE_KEY = 'pref-lang' + +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 e2352f0..a0d89ac 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_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(settings.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( + settings.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( + settings.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 2664073..e70f84e 100644 --- a/openedx/core/djangoapps/lang_pref/tests/test_middleware.py +++ b/openedx/core/djangoapps/lang_pref/tests/test_middleware.py @@ -2,17 +2,22 @@ Tests for lang_pref middleware. """ +import itertools 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 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, 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 +39,134 @@ 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 - - @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) + def test_logout_shouldnt_remove_cookie(self): - self.assertEquals(self.request.session[LANGUAGE_SESSION_KEY], 'eo') # pylint: disable=no-member + self.middleware.process_request(self.request) - @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): + response = mock.Mock(spec=HttpResponse) + self.middleware.process_response(self.request, response) + + response.delete_cookie.assert_not_called() + + @ddt.data(None, 'es', 'en') + def test_preference_setting_changes_cookie(self, lang_pref_out): """ - test: browser language should not be set in user session if it is not supported by system. + 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. """ - self.request.user = self.anonymous_user + 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( + settings.LANGUAGE_COOKIE, + value=lang_pref_out, + domain=settings.SESSION_COOKIE_DOMAIN, + max_age=COOKIE_DURATION, + ) + else: + response.delete_cookie.assert_called_with( + settings.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[settings.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.assertNotEqual(self.request.session.get(LANGUAGE_SESSION_KEY), 'ar') # pylint: disable=no-member - @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): - """ - test: user preferred language has been removed from user preferences model if it is not supported by system - for authenticated users. - """ - set_user_preference(self.user, LANGUAGE_KEY, 'eo') + 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[settings.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) - self.assertEqual(get_user_preference(self.request.user, LANGUAGE_KEY), None) + + 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) + + @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) + + 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 0331ff2..cd47008 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, 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( + settings.LANGUAGE_COOKIE, + language, + domain=settings.SESSION_COOKIE_DOMAIN, + max_age=COOKIE_DURATION + ) + return response diff --git a/openedx/core/djangoapps/user_api/helpers.py b/openedx/core/djangoapps/user_api/helpers.py index c2c62d6..851bf49 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/serializers.py b/openedx/core/djangoapps/user_api/serializers.py index f46197c..2ba351f 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/tests/test_helpers.py b/openedx/core/djangoapps/user_api/tests/test_helpers.py index a0983a0..00576d5 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': <class '" + exception + u"'>}': FakeInputException()" - ) + u"keyword arguments '{{'raise_error': <class '{}'>}}' " + 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): diff --git a/openedx/core/djangoapps/user_api/views.py b/openedx/core/djangoapps/user_api/views.py index fbe3e7c..0627bf7 100644 --- a/openedx/core/djangoapps/user_api/views.py +++ b/openedx/core/djangoapps/user_api/views.py @@ -1002,7 +1002,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" @@ -1028,7 +1028,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 @@ -1057,7 +1057,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): -- libgit2 0.26.0