diff --git a/cms/djangoapps/contentstore/views/preview.py b/cms/djangoapps/contentstore/views/preview.py index 5e1beeb5c4..6cb518ffe8 100644 --- a/cms/djangoapps/contentstore/views/preview.py +++ b/cms/djangoapps/contentstore/views/preview.py @@ -207,7 +207,6 @@ def _preview_module_system(request, descriptor, field_data): wrappers=wrappers, wrappers_asides=wrappers_asides, error_descriptor_class=ErrorBlock, - get_user_role=lambda: get_user_role(request.user, course_id), # Get the raw DescriptorSystem, not the CombinedSystem descriptor_runtime=descriptor._runtime, # pylint: disable=protected-access services={ @@ -215,7 +214,11 @@ def _preview_module_system(request, descriptor, field_data): "i18n": ModuleI18nService, 'mako': mako_service, "settings": SettingsService(), - "user": DjangoXBlockUserService(request.user, anonymous_user_id='student'), + "user": DjangoXBlockUserService( + request.user, + anonymous_user_id='student', + user_role=get_user_role(request.user, course_id), + ), "partitions": StudioPartitionService(course_id=course_id), "teams_configuration": TeamsConfigurationService(), }, diff --git a/cms/djangoapps/contentstore/views/tests/test_preview.py b/cms/djangoapps/contentstore/views/tests/test_preview.py index 3a7f29472f..91b77523b8 100644 --- a/cms/djangoapps/contentstore/views/tests/test_preview.py +++ b/cms/djangoapps/contentstore/views/tests/test_preview.py @@ -230,6 +230,16 @@ class CmsModuleSystemShimTest(ModuleStoreTestCase): self.request = RequestFactory().get('/dummy-url') self.request.user = self.user self.request.session = {} + self.descriptor = ItemFactory(category="video", parent=self.course) + self.field_data = mock.Mock() + self.runtime = _preview_module_system( + self.request, + self.descriptor, + self.field_data, + ) + + def test_get_user_role(self): + assert self.runtime.get_user_role() == 'staff' @XBlock.register_temp_plugin(PureXBlock, identifier='pure') def test_render_template(self): diff --git a/common/djangoapps/xblock_django/constants.py b/common/djangoapps/xblock_django/constants.py index 3d44db7877..cb528f5222 100644 --- a/common/djangoapps/xblock_django/constants.py +++ b/common/djangoapps/xblock_django/constants.py @@ -4,8 +4,10 @@ Constants used by DjangoXBlockUserService # Optional attributes stored on the XBlockUser ATTR_KEY_ANONYMOUS_USER_ID = 'edx-platform.anonymous_user_id' +ATTR_KEY_REQUEST_COUNTRY_CODE = 'edx-platform.request_country_code' ATTR_KEY_IS_AUTHENTICATED = 'edx-platform.is_authenticated' ATTR_KEY_USER_ID = 'edx-platform.user_id' ATTR_KEY_USERNAME = 'edx-platform.username' ATTR_KEY_USER_IS_STAFF = 'edx-platform.user_is_staff' ATTR_KEY_USER_PREFERENCES = 'edx-platform.user_preferences' +ATTR_KEY_USER_ROLE = 'edx-platform.user_role' diff --git a/common/djangoapps/xblock_django/tests/test_user_service.py b/common/djangoapps/xblock_django/tests/test_user_service.py index 7954a09dc1..33cfa9aad4 100644 --- a/common/djangoapps/xblock_django/tests/test_user_service.py +++ b/common/djangoapps/xblock_django/tests/test_user_service.py @@ -14,9 +14,11 @@ from common.djangoapps.student.tests.factories import AnonymousUserFactory, User from common.djangoapps.xblock_django.user_service import ( ATTR_KEY_IS_AUTHENTICATED, ATTR_KEY_ANONYMOUS_USER_ID, + ATTR_KEY_REQUEST_COUNTRY_CODE, ATTR_KEY_USER_ID, ATTR_KEY_USER_IS_STAFF, ATTR_KEY_USER_PREFERENCES, + ATTR_KEY_USER_ROLE, ATTR_KEY_USERNAME, USER_PREFERENCES_WHITE_LIST, DjangoXBlockUserService @@ -37,15 +39,22 @@ class UserServiceTestCase(TestCase): set_user_preference(self.user, 'not_white_listed', 'hidden_value') self.anon_user = AnonymousUserFactory() - def assert_is_anon_xb_user(self, xb_user): + def assert_is_anon_xb_user(self, xb_user, request_country_code): """ A set of assertions for an anonymous XBlockUser. """ assert not xb_user.opt_attrs[ATTR_KEY_IS_AUTHENTICATED] + assert xb_user.opt_attrs[ATTR_KEY_REQUEST_COUNTRY_CODE] == request_country_code assert xb_user.full_name is None self.assertListEqual(xb_user.emails, []) - def assert_xblock_user_matches_django(self, xb_user, dj_user, user_is_staff=False, anonymous_user_id=None): + def assert_xblock_user_matches_django( + self, xb_user, dj_user, + user_is_staff=False, + user_role=None, + anonymous_user_id=None, + request_country_code=None, + ): """ A set of assertions for comparing a XBlockUser to a django User """ @@ -55,37 +64,49 @@ class UserServiceTestCase(TestCase): assert xb_user.opt_attrs[ATTR_KEY_USERNAME] == dj_user.username assert xb_user.opt_attrs[ATTR_KEY_USER_ID] == dj_user.id assert xb_user.opt_attrs[ATTR_KEY_USER_IS_STAFF] == user_is_staff + assert xb_user.opt_attrs[ATTR_KEY_USER_ROLE] == user_role assert xb_user.opt_attrs[ATTR_KEY_ANONYMOUS_USER_ID] == anonymous_user_id + assert xb_user.opt_attrs[ATTR_KEY_REQUEST_COUNTRY_CODE] == request_country_code assert all((pref in USER_PREFERENCES_WHITE_LIST) for pref in xb_user.opt_attrs[ATTR_KEY_USER_PREFERENCES]) def test_convert_anon_user(self): """ Tests for convert_django_user_to_xblock_user behavior when django user is AnonymousUser. """ - django_user_service = DjangoXBlockUserService(self.anon_user) + country_code = 'UK' + django_user_service = DjangoXBlockUserService(self.anon_user, request_country_code=country_code) xb_user = django_user_service.get_current_user() assert xb_user.is_current_user - self.assert_is_anon_xb_user(xb_user) + self.assert_is_anon_xb_user(xb_user, request_country_code=country_code) @ddt.data( - (False, None), - (True, None), - (False, 'abcdef0123'), - (True, 'abcdef0123'), + (False, None, None, None), + (True, 'instructor', None, None), + (True, 'staff', None, None), + (False, 'student', 'abcdef0123', None), + (True, 'student', 'abcdef0123', 'uk'), ) @ddt.unpack - def test_convert_authenticate_user(self, user_is_staff, anonymous_user_id): + def test_convert_authenticate_user(self, user_is_staff, user_role, anonymous_user_id, request_country_code): """ Tests for convert_django_user_to_xblock_user behavior when django user is User. """ django_user_service = DjangoXBlockUserService( self.user, user_is_staff=user_is_staff, + user_role=user_role, anonymous_user_id=anonymous_user_id, + request_country_code=request_country_code, ) xb_user = django_user_service.get_current_user() assert xb_user.is_current_user - self.assert_xblock_user_matches_django(xb_user, self.user, user_is_staff, anonymous_user_id) + self.assert_xblock_user_matches_django( + xb_user, self.user, + user_is_staff, + user_role, + anonymous_user_id, + request_country_code, + ) def test_get_anonymous_user_id_returns_none_for_non_staff_users(self): """ @@ -126,6 +147,27 @@ class UserServiceTestCase(TestCase): assert anonymous_user_id == anon_user_id + def test_get_user_by_anonymous_id(self): + """ + Tests that get_user_by_anonymous_id returns the expected user. + """ + course_key = CourseKey.from_string('edX/toy/2012_Fall') + anon_user_id = anonymous_id_for_user( + user=self.user, + course_id=course_key + ) + + django_user_service = DjangoXBlockUserService(self.user) + user = django_user_service.get_user_by_anonymous_id(anon_user_id) + assert user == self.user + + def test_get_user_by_anonymous_id_not_found(self): + """ + Tests that get_user_by_anonymous_id returns None for an unassigned anonymous user id. + """ + django_user_service = DjangoXBlockUserService(self.user) + assert django_user_service.get_user_by_anonymous_id('invalid-anon-id') is None + def test_external_id(self): """ Tests that external ids differ based on type. @@ -138,3 +180,17 @@ class UserServiceTestCase(TestCase): assert ext_id1 != ext_id2 with pytest.raises(ValueError): django_user_service.get_external_user_id('unknown') + + def test_get_user_by_anonymous_id_assume_id(self): + """ + Tests that get_user_by_anonymous_id uses the anonymous user ID given to the service if none is provided. + """ + course_key = CourseKey.from_string('edX/toy/2012_Fall') + anon_user_id = anonymous_id_for_user( + user=self.user, + course_id=course_key + ) + + django_user_service = DjangoXBlockUserService(self.user, anonymous_user_id=anon_user_id) + user = django_user_service.get_user_by_anonymous_id() + assert user == self.user diff --git a/common/djangoapps/xblock_django/user_service.py b/common/djangoapps/xblock_django/user_service.py index 19eff4d224..e9d8e74992 100644 --- a/common/djangoapps/xblock_django/user_service.py +++ b/common/djangoapps/xblock_django/user_service.py @@ -9,15 +9,17 @@ from xblock.reference.user_service import UserService, XBlockUser from openedx.core.djangoapps.external_user_ids.models import ExternalId from openedx.core.djangoapps.user_api.preferences.api import get_user_preferences -from common.djangoapps.student.models import anonymous_id_for_user, get_user_by_username_or_email +from common.djangoapps.student.models import anonymous_id_for_user, get_user_by_username_or_email, user_by_anonymous_id from .constants import ( ATTR_KEY_ANONYMOUS_USER_ID, ATTR_KEY_IS_AUTHENTICATED, + ATTR_KEY_REQUEST_COUNTRY_CODE, ATTR_KEY_USER_ID, ATTR_KEY_USERNAME, ATTR_KEY_USER_IS_STAFF, ATTR_KEY_USER_PREFERENCES, + ATTR_KEY_USER_ROLE, ) @@ -34,12 +36,16 @@ class DjangoXBlockUserService(UserService): Args: user_is_staff(bool): optional - whether the user is staff in the course + user_role(str): optional -- user's role in the course ('staff', 'instructor', or 'student') anonymous_user_id(str): optional - anonymous_user_id for the user in the course + request_country_code(str): optional -- country code determined from the user's request IP address. """ super().__init__(**kwargs) self._django_user = django_user self._user_is_staff = kwargs.get('user_is_staff', False) + self._user_role = kwargs.get('user_role', 'student') self._anonymous_user_id = kwargs.get('anonymous_user_id', None) + self._request_country_code = kwargs.get('request_country_code', None) def get_current_user(self): """ @@ -80,6 +86,16 @@ class DjangoXBlockUserService(UserService): course_id = CourseKey.from_string(course_id) return anonymous_id_for_user(user=user, course_id=course_id) + def get_user_by_anonymous_id(self, uid=None): + """ + Returns the Django User object corresponding to the given anonymous user id. + + Returns None if there is no user with the given anonymous user id. + + If no `uid` is provided, then the current anonymous user ID is used. + """ + return user_by_anonymous_id(uid or self._anonymous_user_id) + def _convert_django_user_to_xblock_user(self, django_user): """ A function that returns an XBlockUser from the current Django request.user @@ -96,9 +112,11 @@ class DjangoXBlockUserService(UserService): xblock_user.emails = [django_user.email] xblock_user.opt_attrs[ATTR_KEY_ANONYMOUS_USER_ID] = self._anonymous_user_id xblock_user.opt_attrs[ATTR_KEY_IS_AUTHENTICATED] = True + xblock_user.opt_attrs[ATTR_KEY_REQUEST_COUNTRY_CODE] = self._request_country_code xblock_user.opt_attrs[ATTR_KEY_USER_ID] = django_user.id xblock_user.opt_attrs[ATTR_KEY_USERNAME] = django_user.username xblock_user.opt_attrs[ATTR_KEY_USER_IS_STAFF] = self._user_is_staff + xblock_user.opt_attrs[ATTR_KEY_USER_ROLE] = self._user_role user_preferences = get_user_preferences(django_user) xblock_user.opt_attrs[ATTR_KEY_USER_PREFERENCES] = { pref: user_preferences.get(pref) @@ -107,5 +125,6 @@ class DjangoXBlockUserService(UserService): } else: xblock_user.opt_attrs[ATTR_KEY_IS_AUTHENTICATED] = False + xblock_user.opt_attrs[ATTR_KEY_REQUEST_COUNTRY_CODE] = self._request_country_code return xblock_user diff --git a/common/lib/xmodule/xmodule/tests/__init__.py b/common/lib/xmodule/xmodule/tests/__init__.py index 533bff5c69..2dd1a71326 100644 --- a/common/lib/xmodule/xmodule/tests/__init__.py +++ b/common/lib/xmodule/xmodule/tests/__init__.py @@ -92,6 +92,7 @@ def get_test_system( course_id=CourseKey.from_string('/'.join(['org', 'course', 'run'])), user=None, user_is_staff=False, + user_location=None, render_template=None, ): """ @@ -102,10 +103,14 @@ def get_test_system( """ if not user: user = Mock(name='get_test_system.user', is_staff=False) + if not user_location: + user_location = Mock(name='get_test_system.user_location') user_service = StubUserService( user=user, anonymous_user_id='student', user_is_staff=user_is_staff, + user_role='student', + request_country_code=user_location, ) mako_service = StubMakoService(render_template=render_template) @@ -133,7 +138,6 @@ def get_test_system( track_function=Mock(name='get_test_system.track_function'), get_module=get_module, replace_urls=str, - get_real_user=lambda __: user, filestore=Mock(name='get_test_system.filestore', root_path='.'), debug=True, hostname="edx.org", @@ -151,8 +155,6 @@ def get_test_system( node_path=os.environ.get("NODE_PATH", "/usr/local/lib/node_modules"), course_id=course_id, error_descriptor_class=ErrorBlock, - get_user_role=Mock(name='get_test_system.get_user_role', is_staff=False), - user_location=Mock(name='get_test_system.user_location'), descriptor_runtime=descriptor_system, ) diff --git a/common/lib/xmodule/xmodule/tests/helpers.py b/common/lib/xmodule/xmodule/tests/helpers.py index 22dc868924..f57f67fe97 100644 --- a/common/lib/xmodule/xmodule/tests/helpers.py +++ b/common/lib/xmodule/xmodule/tests/helpers.py @@ -4,7 +4,6 @@ Utility methods for unit tests. import filecmp -from unittest.mock import Mock import pprint from path import Path as path @@ -61,10 +60,18 @@ class StubUserService(UserService): Stub UserService for testing the sequence module. """ - def __init__(self, user=None, user_is_staff=False, anonymous_user_id=None, **kwargs): - self.user = user or Mock(name='StubUserService.user') + def __init__(self, + user=None, + user_is_staff=False, + user_role=None, + anonymous_user_id=None, + request_country_code=None, + **kwargs): + self.user = user self.user_is_staff = user_is_staff + self.user_role = user_role self.anonymous_user_id = anonymous_user_id + self.request_country_code = request_country_code super().__init__(**kwargs) def get_current_user(self): @@ -72,12 +79,21 @@ class StubUserService(UserService): Implements abstract method for getting the current user. """ user = XBlockUser() - if self.user.is_authenticated: + if self.user and self.user.is_authenticated: user.opt_attrs['edx-platform.anonymous_user_id'] = self.anonymous_user_id + user.opt_attrs['edx-platform.request_country_code'] = self.request_country_code user.opt_attrs['edx-platform.user_is_staff'] = self.user_is_staff user.opt_attrs['edx-platform.user_id'] = self.user.id + user.opt_attrs['edx-platform.user_role'] = self.user_role user.opt_attrs['edx-platform.username'] = self.user.username else: user.opt_attrs['edx-platform.username'] = 'anonymous' + user.opt_attrs['edx-platform.request_country_code'] = self.request_country_code user.opt_attrs['edx-platform.is_authenticated'] = False return user + + def get_user_by_anonymous_id(self, uid=None): # pylint: disable=unused-argument + """ + Return the original user passed into the service. + """ + return self.user diff --git a/common/lib/xmodule/xmodule/tests/test_lti20_unit.py b/common/lib/xmodule/xmodule/tests/test_lti20_unit.py index 7dc69c9ad3..aca04ea955 100644 --- a/common/lib/xmodule/xmodule/tests/test_lti20_unit.py +++ b/common/lib/xmodule/xmodule/tests/test_lti20_unit.py @@ -11,6 +11,7 @@ from xblock.field_data import DictFieldData from xmodule.lti_2_util import LTIError from xmodule.lti_module import LTIBlock +from xmodule.tests.helpers import StubUserService from . import get_test_system @@ -18,11 +19,13 @@ from . import get_test_system class LTI20RESTResultServiceTest(unittest.TestCase): """Logic tests for LTI module. LTI2.0 REST ResultService""" + USER_STANDIN = Mock() + USER_STANDIN.id = 999 + def setUp(self): super().setUp() - self.system = get_test_system() + self.system = get_test_system(user=self.USER_STANDIN) self.environ = {'wsgi.url_scheme': 'http', 'REQUEST_METHOD': 'POST'} - self.system.get_real_user = Mock() self.system.publish = Mock() self.system.rebind_noauth_module_to_user = Mock() @@ -223,14 +226,10 @@ class LTI20RESTResultServiceTest(unittest.TestCase): mock_request.body = body return mock_request - USER_STANDIN = Mock() - USER_STANDIN.id = 999 - def setup_system_xmodule_mocks_for_lti20_request_test(self): """ Helper fn to set up mocking for lti 2.0 request test """ - self.system.get_real_user = Mock(return_value=self.USER_STANDIN) self.xmodule.max_score = Mock(return_value=1.0) self.xmodule.get_client_key_secret = Mock(return_value=('test_client_key', 'test_client_secret')) self.xmodule.verify_oauth_body_sign = Mock() @@ -371,7 +370,7 @@ class LTI20RESTResultServiceTest(unittest.TestCase): Test that we get a 404 when the supplied user does not exist """ self.setup_system_xmodule_mocks_for_lti20_request_test() - self.system.get_real_user = Mock(return_value=None) + self.system._services['user'] = StubUserService(user=None) # pylint: disable=protected-access mock_request = self.get_signed_lti20_mock_request(self.GOOD_JSON_PUT) response = self.xmodule.lti_2_0_result_rest_handler(mock_request, "user/abcd") assert response.status_code == 404 diff --git a/common/lib/xmodule/xmodule/tests/test_lti_unit.py b/common/lib/xmodule/xmodule/tests/test_lti_unit.py index 3f13666e98..103cdaeb53 100644 --- a/common/lib/xmodule/xmodule/tests/test_lti_unit.py +++ b/common/lib/xmodule/xmodule/tests/test_lti_unit.py @@ -20,6 +20,7 @@ from common.djangoapps.xblock_django.constants import ATTR_KEY_ANONYMOUS_USER_ID from xmodule.fields import Timedelta from xmodule.lti_2_util import LTIError from xmodule.lti_module import LTIBlock +from xmodule.tests.helpers import StubUserService from . import get_test_system @@ -57,7 +58,6 @@ class LTIBlockTest(unittest.TestCase): """) self.system = get_test_system() - self.system.get_real_user = Mock() self.system.publish = Mock() self.system.rebind_noauth_module_to_user = Mock() @@ -171,9 +171,9 @@ class LTIBlockTest(unittest.TestCase): """ If we have no real user, we should send back failure response. """ + self.system._services['user'] = StubUserService(user=None) # pylint: disable=protected-access self.xmodule.verify_oauth_body_sign = Mock() self.xmodule.has_score = True - self.system.get_real_user = Mock(return_value=None) request = Request(self.environ) request.body = self.get_request_body() response = self.xmodule.grade_handler(request, '') diff --git a/common/lib/xmodule/xmodule/tests/test_sequence.py b/common/lib/xmodule/xmodule/tests/test_sequence.py index c51a642616..bf0e16fb1e 100644 --- a/common/lib/xmodule/xmodule/tests/test_sequence.py +++ b/common/lib/xmodule/xmodule/tests/test_sequence.py @@ -98,7 +98,7 @@ class SequenceBlockTestCase(XModuleXmlImportTest): block.xmodule_runtime._services['completion'] = Mock( # pylint: disable=protected-access return_value=Mock(vertical_is_complete=Mock(return_value=True)) ) - block.xmodule_runtime._services['user'] = StubUserService() # pylint: disable=protected-access + block.xmodule_runtime._services['user'] = StubUserService(user=Mock()) # pylint: disable=protected-access block.parent = parent.location return block diff --git a/common/lib/xmodule/xmodule/tests/test_vertical.py b/common/lib/xmodule/xmodule/tests/test_vertical.py index b24e561429..22b06d3548 100644 --- a/common/lib/xmodule/xmodule/tests/test_vertical.py +++ b/common/lib/xmodule/xmodule/tests/test_vertical.py @@ -190,7 +190,7 @@ class VerticalBlockTestCase(BaseVerticalBlockTest): Test the rendering of the student and public view. """ self.module_system._services['bookmarks'] = Mock() - self.module_system._services['user'] = StubUserService() + self.module_system._services['user'] = StubUserService(user=Mock()) self.module_system._services['completion'] = StubCompletionService(enabled=True, completion_value=0) now = datetime.now(pytz.UTC) @@ -212,7 +212,7 @@ class VerticalBlockTestCase(BaseVerticalBlockTest): def test_render_access_denied_blocks(self, node_has_access_error, child_has_access_error): """ Tests access denied blocks are not rendered when hide_access_error_blocks is True """ self.module_system._services['bookmarks'] = Mock() - self.module_system._services['user'] = StubUserService() + self.module_system._services['user'] = StubUserService(user=Mock()) self.vertical.due = datetime.now(pytz.UTC) + timedelta(days=-1) self.problem_block.has_access_error = node_has_access_error self.nested_problem_block.has_access_error = child_has_access_error diff --git a/common/lib/xmodule/xmodule/x_module.py b/common/lib/xmodule/xmodule/x_module.py index 056528ebd4..607c1b2ecf 100644 --- a/common/lib/xmodule/xmodule/x_module.py +++ b/common/lib/xmodule/xmodule/x_module.py @@ -44,8 +44,10 @@ from xmodule.util.xmodule_django import add_webpack_to_fragment from common.djangoapps.xblock_django.constants import ( ATTR_KEY_ANONYMOUS_USER_ID, + ATTR_KEY_REQUEST_COUNTRY_CODE, ATTR_KEY_USER_ID, ATTR_KEY_USER_IS_STAFF, + ATTR_KEY_USER_ROLE, ) @@ -1809,6 +1811,59 @@ class ModuleSystemShim: return self._services['user'].get_current_user().opt_attrs.get(ATTR_KEY_USER_IS_STAFF) return None + @property + def user_location(self): + """ + Returns the "country code" associated with the current user's request IP address. + + Deprecated in favor of the user service. + """ + warnings.warn( + 'runtime.user_location is deprecated. Please use the user service instead.', + DeprecationWarning, stacklevel=3, + ) + user_service = self._services.get('user') + if user_service: + return self._services['user'].get_current_user().opt_attrs.get(ATTR_KEY_REQUEST_COUNTRY_CODE) + return None + + @property + def get_real_user(self): + """ + Returns a function that takes `anonymous_student_id` and returns the Django User object + associated with `anonymous_student_id`. + + If no `anonymous_student_id` is provided as an argument to this function, then the user service's anonymous user + ID is used instead. + + Deprecated in favor of the user service. + """ + warnings.warn( + 'runtime.get_real_user is deprecated. Please use the user service instead.', + DeprecationWarning, stacklevel=3, + ) + user_service = self._services.get('user') + if user_service: + return self._services['user'].get_user_by_anonymous_id + return None + + @property + def get_user_role(self): + """ + Returns a function that returns the user's role in the course. + + Implementation is different for LMS and Studio. + + Deprecated in favor of the user service. + """ + warnings.warn( + 'runtime.get_user_role is deprecated. Please use the user service instead.', + DeprecationWarning, stacklevel=3, + ) + user_service = self._services.get('user') + if user_service: + return partial(self._services['user'].get_current_user().opt_attrs.get, ATTR_KEY_USER_ROLE) + @property def render_template(self): """ @@ -1846,9 +1901,9 @@ class ModuleSystem(MetricsMixin, ConfigurableFragmentWrapper, ModuleSystemShim, debug=False, hostname="", xqueue=None, publish=None, node_path="", course_id=None, cache=None, can_execute_unsafe_code=None, replace_course_urls=None, - replace_jump_to_id_urls=None, error_descriptor_class=None, get_real_user=None, - field_data=None, get_user_role=None, rebind_noauth_module_to_user=None, - user_location=None, get_python_lib_zip=None, **kwargs): + replace_jump_to_id_urls=None, error_descriptor_class=None, + field_data=None, rebind_noauth_module_to_user=None, + get_python_lib_zip=None, **kwargs): """ Create a closure around the system environment. @@ -1895,12 +1950,6 @@ class ModuleSystem(MetricsMixin, ConfigurableFragmentWrapper, ModuleSystemShim, error_descriptor_class - The class to use to render XModules with errors - get_real_user - function that takes `anonymous_student_id` and returns real user_id, - associated with `anonymous_student_id`. - - get_user_role - A function that returns user role. Implementation is different - for LMS and Studio. - field_data - the `FieldData` to use for backing XBlock storage. rebind_noauth_module_to_user - rebinds module bound to AnonymousUser to a real user...used in LTI @@ -1935,10 +1984,6 @@ class ModuleSystem(MetricsMixin, ConfigurableFragmentWrapper, ModuleSystemShim, self.error_descriptor_class = error_descriptor_class self.xmodule_instance = None - self.get_real_user = get_real_user - self.user_location = user_location - - self.get_user_role = get_user_role self.descriptor_runtime = descriptor_runtime self.rebind_noauth_module_to_user = rebind_noauth_module_to_user diff --git a/lms/djangoapps/courseware/module_render.py b/lms/djangoapps/courseware/module_render.py index 6929cea8a8..00ffa2a9b8 100644 --- a/lms/djangoapps/courseware/module_render.py +++ b/lms/djangoapps/courseware/module_render.py @@ -84,7 +84,7 @@ from openedx.core.lib.xblock_utils import wrap_xblock from openedx.features.course_duration_limits.access import course_expiration_wrapper from openedx.features.discounts.utils import offer_banner_wrapper from openedx.features.content_type_gating.services import ContentTypeGatingService -from common.djangoapps.student.models import anonymous_id_for_user, user_by_anonymous_id +from common.djangoapps.student.models import anonymous_id_for_user from common.djangoapps.student.roles import CourseBetaTesterRole from common.djangoapps.track import contexts from common.djangoapps.util import milestones_helpers @@ -553,7 +553,9 @@ def get_module_system_for_user( user_service = DjangoXBlockUserService( user, user_is_staff=user_is_staff, + user_role=get_user_role(user, course_id), anonymous_user_id=anonymous_student_id, + request_country_code=user_location, ) def publish(block, event_type, event): @@ -806,7 +808,6 @@ def get_module_system_for_user( # TODO: When we merge the descriptor and module systems, we can stop reaching into the mixologist (cpennington) mixins=descriptor.runtime.mixologist._mixins, # pylint: disable=protected-access wrappers=block_wrappers, - get_real_user=user_by_anonymous_id, services={ 'fs': FSService(), 'field-data': field_data, @@ -822,10 +823,8 @@ def get_module_system_for_user( 'user_state': UserStateService(), 'content_type_gating': ContentTypeGatingService(), }, - get_user_role=lambda: get_user_role(user, course_id), descriptor_runtime=descriptor._runtime, # pylint: disable=protected-access rebind_noauth_module_to_user=rebind_noauth_module_to_user, - user_location=user_location, request_token=request_token, ) diff --git a/lms/djangoapps/courseware/tests/helpers.py b/lms/djangoapps/courseware/tests/helpers.py index feb57dd065..e92326934a 100644 --- a/lms/djangoapps/courseware/tests/helpers.py +++ b/lms/djangoapps/courseware/tests/helpers.py @@ -64,18 +64,18 @@ class BaseTestXmodule(ModuleStoreTestCase): METADATA = {} MODEL_DATA = {'data': ''} - def new_module_runtime(self, render_template=None): + def new_module_runtime(self, **kwargs): """ Generate a new ModuleSystem that is minimally set up for testing """ - return get_test_system(course_id=self.course.id, render_template=render_template) + return get_test_system(course_id=self.course.id, **kwargs) - def new_descriptor_runtime(self): - runtime = get_test_descriptor_system() + def new_descriptor_runtime(self, **kwargs): + runtime = get_test_descriptor_system(**kwargs) runtime.get_block = modulestore().get_item return runtime - def initialize_module(self, **kwargs): # lint-amnesty, pylint: disable=missing-function-docstring + def initialize_module(self, runtime_kwargs=None, **kwargs): # lint-amnesty, pylint: disable=missing-function-docstring kwargs.update({ 'parent_location': self.section.location, 'category': self.CATEGORY @@ -90,7 +90,9 @@ class BaseTestXmodule(ModuleStoreTestCase): student_data = DictFieldData(field_data) self.item_descriptor._field_data = LmsFieldData(self.item_descriptor._field_data, student_data) # lint-amnesty, pylint: disable=protected-access - self.item_descriptor.xmodule_runtime = self.new_module_runtime() + if runtime_kwargs is None: + runtime_kwargs = {} + self.item_descriptor.xmodule_runtime = self.new_module_runtime(**runtime_kwargs) self.item_url = str(self.item_descriptor.location) @@ -144,13 +146,13 @@ class BaseTestXmodule(ModuleStoreTestCase): class XModuleRenderingTestBase(BaseTestXmodule): # lint-amnesty, pylint: disable=missing-class-docstring - def new_module_runtime(self, render_template=None): + def new_module_runtime(self, **kwargs): """ Create a runtime that actually does html rendering """ - if not render_template: - render_template = render_to_string - runtime = super().new_module_runtime(render_template=render_template) + if 'render_template' not in kwargs: + kwargs['render_template'] = render_to_string + runtime = super().new_module_runtime(**kwargs) runtime.modulestore = Mock() return runtime diff --git a/lms/djangoapps/courseware/tests/test_module_render.py b/lms/djangoapps/courseware/tests/test_module_render.py index 5f76c05803..bdbf449a12 100644 --- a/lms/djangoapps/courseware/tests/test_module_render.py +++ b/lms/djangoapps/courseware/tests/test_module_render.py @@ -2624,6 +2624,21 @@ class LmsModuleSystemShimTest(SharedModuleStoreTestCase): course=self.course, ) assert runtime.user_is_staff + assert runtime.get_user_role() == 'student' + + @patch('lms.djangoapps.courseware.module_render.get_user_role', Mock(return_value='instructor', autospec=True)) + def test_get_user_role(self): + runtime, _ = render.get_module_system_for_user( + self.user, + self.student_data, + self.descriptor, + self.course.id, + self.track_function, + self.xqueue_callback_url_prefix, + self.request_token, + course=self.course, + ) + assert runtime.get_user_role() == 'instructor' def test_anonymous_student_id(self): runtime, _ = render.get_module_system_for_user( @@ -2687,6 +2702,27 @@ class LmsModuleSystemShimTest(SharedModuleStoreTestCase): assert runtime.seed == 0 assert runtime.user_id is None assert not runtime.user_is_staff + assert not runtime.get_user_role() + + def test_get_real_user(self): + runtime, _ = render.get_module_system_for_user( + self.user, + self.student_data, + self.descriptor, + self.course.id, + self.track_function, + self.xqueue_callback_url_prefix, + self.request_token, + course=self.course, + ) + course_anonymous_student_id = anonymous_id_for_user(self.user, self.course.id) + assert runtime.get_real_user(course_anonymous_student_id) == self.user # pylint: disable=not-callable + + no_course_anonymous_student_id = anonymous_id_for_user(self.user, None) + assert runtime.get_real_user(no_course_anonymous_student_id) == self.user # pylint: disable=not-callable + + # Tests that the default is to use the user service's anonymous_student_id + assert runtime.get_real_user() == self.user # pylint: disable=not-callable def test_render_template(self): runtime, _ = render.get_module_system_for_user( diff --git a/lms/djangoapps/courseware/tests/test_video_mongo.py b/lms/djangoapps/courseware/tests/test_video_mongo.py index d48dd16811..7f33db339d 100644 --- a/lms/djangoapps/courseware/tests/test_video_mongo.py +++ b/lms/djangoapps/courseware/tests/test_video_mongo.py @@ -883,8 +883,10 @@ class TestGetHtmlMethod(BaseTestVideoXBlock): sources=data['sources'], edx_video_id=data['edx_video_id'], ) - self.initialize_block(data=DATA) - self.item_descriptor.xmodule_runtime.user_location = 'CN' + self.initialize_block(data=DATA, runtime_kwargs={ + 'user_location': 'CN', + }) + assert self.item_descriptor.xmodule_runtime.user_location == 'CN' context = self.item_descriptor.render('student_view').content expected_context = dict(initial_context) expected_context['metadata'].update({ diff --git a/lms/djangoapps/lms_xblock/test/test_runtime.py b/lms/djangoapps/lms_xblock/test/test_runtime.py index 5a21fca1f2..eed0279c71 100644 --- a/lms/djangoapps/lms_xblock/test/test_runtime.py +++ b/lms/djangoapps/lms_xblock/test/test_runtime.py @@ -176,10 +176,6 @@ class TestBadgingService(ModuleStoreTestCase): """ Create the testing runtime. """ - def mock_get_real_user(_anon_id): - """Just returns the test user""" - return self.user - return LmsModuleSystem( static_url='/static', track_function=Mock(), @@ -187,7 +183,6 @@ class TestBadgingService(ModuleStoreTestCase): replace_urls=str, course_id=self.course_id, user=self.user, - get_real_user=mock_get_real_user, descriptor_runtime=Mock(), )