diff --git a/cms/djangoapps/contentstore/views/preview.py b/cms/djangoapps/contentstore/views/preview.py index be7453ced5cb..1abafe5fb9bc 100644 --- a/cms/djangoapps/contentstore/views/preview.py +++ b/cms/djangoapps/contentstore/views/preview.py @@ -198,12 +198,10 @@ def _preview_module_system(request, descriptor, field_data): render_template=render_from_lms, debug=True, replace_urls=partial(static_replace.replace_static_urls, data_directory=None, course_id=course_id), - user=request.user, can_execute_unsafe_code=(lambda: can_execute_unsafe_code(course_id)), get_python_lib_zip=(lambda: get_python_lib_zip(contentstore, course_id)), mixins=settings.XBLOCK_MIXINS, course_id=course_id, - anonymous_student_id='student', # Set up functions to modify the fragment produced by student_view wrappers=wrappers, @@ -216,7 +214,7 @@ def _preview_module_system(request, descriptor, field_data): "field-data": field_data, "i18n": ModuleI18nService, "settings": SettingsService(), - "user": DjangoXBlockUserService(request.user), + "user": DjangoXBlockUserService(request.user, anonymous_user_id='student'), "partitions": StudioPartitionService(course_id=course_id), "teams_configuration": TeamsConfigurationService(), }, diff --git a/common/djangoapps/xblock_django/constants.py b/common/djangoapps/xblock_django/constants.py new file mode 100644 index 000000000000..3d44db7877b9 --- /dev/null +++ b/common/djangoapps/xblock_django/constants.py @@ -0,0 +1,11 @@ +""" +Constants used by DjangoXBlockUserService +""" + +# Optional attributes stored on the XBlockUser +ATTR_KEY_ANONYMOUS_USER_ID = 'edx-platform.anonymous_user_id' +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' diff --git a/common/djangoapps/xblock_django/tests/test_user_service.py b/common/djangoapps/xblock_django/tests/test_user_service.py index 6d090417c5f2..7954a09dc1d0 100644 --- a/common/djangoapps/xblock_django/tests/test_user_service.py +++ b/common/djangoapps/xblock_django/tests/test_user_service.py @@ -2,6 +2,7 @@ Tests for the DjangoXBlockUserService. """ +import ddt import pytest from django.test import TestCase from opaque_keys.edx.keys import CourseKey @@ -12,6 +13,7 @@ from common.djangoapps.student.tests.factories import AnonymousUserFactory, UserFactory from common.djangoapps.xblock_django.user_service import ( ATTR_KEY_IS_AUTHENTICATED, + ATTR_KEY_ANONYMOUS_USER_ID, ATTR_KEY_USER_ID, ATTR_KEY_USER_IS_STAFF, ATTR_KEY_USER_PREFERENCES, @@ -21,6 +23,7 @@ ) +@ddt.ddt class UserServiceTestCase(TestCase): """ Tests for the DjangoXBlockUserService. @@ -42,7 +45,7 @@ def assert_is_anon_xb_user(self, xb_user): assert xb_user.full_name is None self.assertListEqual(xb_user.emails, []) - def assert_xblock_user_matches_django(self, xb_user, dj_user): + def assert_xblock_user_matches_django(self, xb_user, dj_user, user_is_staff=False, anonymous_user_id=None): """ A set of assertions for comparing a XBlockUser to a django User """ @@ -51,7 +54,8 @@ def assert_xblock_user_matches_django(self, xb_user, dj_user): assert xb_user.full_name == dj_user.profile.name assert xb_user.opt_attrs[ATTR_KEY_USERNAME] == dj_user.username assert xb_user.opt_attrs[ATTR_KEY_USER_ID] == dj_user.id - assert not xb_user.opt_attrs[ATTR_KEY_USER_IS_STAFF] + assert xb_user.opt_attrs[ATTR_KEY_USER_IS_STAFF] == user_is_staff + assert xb_user.opt_attrs[ATTR_KEY_ANONYMOUS_USER_ID] == anonymous_user_id 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): @@ -63,14 +67,25 @@ def test_convert_anon_user(self): assert xb_user.is_current_user self.assert_is_anon_xb_user(xb_user) - def test_convert_authenticate_user(self): + @ddt.data( + (False, None), + (True, None), + (False, 'abcdef0123'), + (True, 'abcdef0123'), + ) + @ddt.unpack + def test_convert_authenticate_user(self, user_is_staff, anonymous_user_id): """ Tests for convert_django_user_to_xblock_user behavior when django user is User. """ - django_user_service = DjangoXBlockUserService(self.user) + django_user_service = DjangoXBlockUserService( + self.user, + user_is_staff=user_is_staff, + anonymous_user_id=anonymous_user_id, + ) xb_user = django_user_service.get_current_user() assert xb_user.is_current_user - self.assert_xblock_user_matches_django(xb_user, self.user) + self.assert_xblock_user_matches_django(xb_user, self.user, user_is_staff, anonymous_user_id) def test_get_anonymous_user_id_returns_none_for_non_staff_users(self): """ diff --git a/common/djangoapps/xblock_django/user_service.py b/common/djangoapps/xblock_django/user_service.py index 23af5ca273a8..19eff4d22423 100644 --- a/common/djangoapps/xblock_django/user_service.py +++ b/common/djangoapps/xblock_django/user_service.py @@ -11,11 +11,16 @@ 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 -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' +from .constants import ( + ATTR_KEY_ANONYMOUS_USER_ID, + ATTR_KEY_IS_AUTHENTICATED, + ATTR_KEY_USER_ID, + ATTR_KEY_USERNAME, + ATTR_KEY_USER_IS_STAFF, + ATTR_KEY_USER_PREFERENCES, +) + + USER_PREFERENCES_WHITE_LIST = ['pref-lang', 'time_zone'] @@ -24,10 +29,17 @@ class DjangoXBlockUserService(UserService): A user service that converts Django users to XBlockUser """ def __init__(self, django_user, **kwargs): + """ + Constructs a DjangoXBlockUserService object. + + Args: + user_is_staff(bool): optional - whether the user is staff in the course + anonymous_user_id(str): optional - anonymous_user_id for the user in the course + """ super().__init__(**kwargs) self._django_user = django_user - if self._django_user: - self._django_user.user_is_staff = kwargs.get('user_is_staff', False) + self._user_is_staff = kwargs.get('user_is_staff', False) + self._anonymous_user_id = kwargs.get('anonymous_user_id', None) def get_current_user(self): """ @@ -82,10 +94,11 @@ def _convert_django_user_to_xblock_user(self, django_user): full_name = None xblock_user.full_name = full_name 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_USER_ID] = django_user.id xblock_user.opt_attrs[ATTR_KEY_USERNAME] = django_user.username - xblock_user.opt_attrs[ATTR_KEY_USER_IS_STAFF] = django_user.user_is_staff + xblock_user.opt_attrs[ATTR_KEY_USER_IS_STAFF] = self._user_is_staff user_preferences = get_user_preferences(django_user) xblock_user.opt_attrs[ATTR_KEY_USER_PREFERENCES] = { pref: user_preferences.get(pref) diff --git a/common/lib/xmodule/xmodule/capa_module.py b/common/lib/xmodule/xmodule/capa_module.py index 3311e2eb85f8..25a133d19e1c 100644 --- a/common/lib/xmodule/xmodule/capa_module.py +++ b/common/lib/xmodule/xmodule/capa_module.py @@ -31,6 +31,11 @@ from capa.inputtypes import Status from capa.responsetypes import LoncapaProblemError, ResponseError, StudentInputError from capa.util import convert_files_to_filenames, get_inner_html_from_xpath +from common.djangoapps.xblock_django.constants import ( + ATTR_KEY_ANONYMOUS_USER_ID, + ATTR_KEY_USER_IS_STAFF, + ATTR_KEY_USER_ID, +) from openedx.core.djangolib.markup import HTML, Text from xmodule.contentstore.django import contentstore from xmodule.editing_module import EditingMixin @@ -113,7 +118,7 @@ def from_json(self, value): to_json = from_json -@XBlock.wants('user') +@XBlock.needs('user') @XBlock.needs('i18n') @XBlock.wants('call_to_action') class ProblemBlock( @@ -784,9 +789,10 @@ def choose_new_seed(self): """ if self.rerandomize == RANDOMIZATION.NEVER: self.seed = 1 - elif self.rerandomize == RANDOMIZATION.PER_STUDENT and hasattr(self.runtime, 'seed'): + elif self.rerandomize == RANDOMIZATION.PER_STUDENT: + user_id = self.runtime.service(self, 'user').get_current_user().opt_attrs.get(ATTR_KEY_USER_ID) or 0 # see comment on randomization_bin - self.seed = randomization_bin(self.runtime.seed, str(self.location).encode('utf-8')) + self.seed = randomization_bin(user_id, str(self.location).encode('utf-8')) else: self.seed = struct.unpack('i', os.urandom(4))[0] @@ -801,9 +807,13 @@ def new_lcp(self, state, text=None): if text is None: text = self.data + user_service = self.runtime.service(self, 'user') + anonymous_student_id = user_service.get_current_user().opt_attrs.get(ATTR_KEY_ANONYMOUS_USER_ID) + seed = user_service.get_current_user().opt_attrs.get(ATTR_KEY_USER_ID) or 0 + capa_system = LoncapaSystem( ajax_url=self.ajax_url, - anonymous_student_id=self.runtime.anonymous_student_id, + anonymous_student_id=anonymous_student_id, cache=self.runtime.cache, can_execute_unsafe_code=self.runtime.can_execute_unsafe_code, get_python_lib_zip=self.runtime.get_python_lib_zip, @@ -812,7 +822,7 @@ def new_lcp(self, state, text=None): i18n=self.runtime.service(self, "i18n"), node_path=self.runtime.node_path, render_template=self.runtime.render_template, - seed=self.runtime.seed, # Why do we do this if we have self.seed? + seed=seed, # Why do we do this if we have self.seed? STATIC_URL=self.runtime.STATIC_URL, xqueue=self.runtime.xqueue, matlab_api_key=self.matlab_api_key @@ -1412,6 +1422,7 @@ def answer_available(self): """ Is the user allowed to see an answer? """ + user_is_staff = self.runtime.service(self, 'user').get_current_user().opt_attrs.get(ATTR_KEY_USER_IS_STAFF) if not self.correctness_available(): # If correctness is being withheld, then don't show answers either. return False @@ -1419,7 +1430,7 @@ def answer_available(self): return False elif self.showanswer == SHOWANSWER.NEVER: return False - elif self.runtime.user_is_staff: + elif user_is_staff: # This is after the 'never' check because admins can see the answer # unless the problem explicitly prevents it return True @@ -1459,10 +1470,11 @@ def correctness_available(self): Limits access to the correct/incorrect flags, messages, and problem score. """ + user_is_staff = self.runtime.service(self, 'user').get_current_user().opt_attrs.get(ATTR_KEY_USER_IS_STAFF) return ShowCorrectness.correctness_available( show_correctness=self.show_correctness, due_date=self.close_date, - has_staff_access=self.runtime.user_is_staff, + has_staff_access=user_is_staff, ) def update_score(self, data): @@ -1777,7 +1789,8 @@ def submit_problem(self, data, override_time=False): # If the user is a staff member, include # the full exception, including traceback, # in the response - if self.runtime.user_is_staff: + user_is_staff = self.runtime.service(self, 'user').get_current_user().opt_attrs.get(ATTR_KEY_USER_IS_STAFF) + if user_is_staff: msg = f"Staff debug info: {traceback.format_exc()}" # Otherwise, display just an error message, diff --git a/common/lib/xmodule/xmodule/html_module.py b/common/lib/xmodule/xmodule/html_module.py index b72c61bc02c8..0329bd0bc2c0 100644 --- a/common/lib/xmodule/xmodule/html_module.py +++ b/common/lib/xmodule/xmodule/html_module.py @@ -17,6 +17,7 @@ from web_fragments.fragment import Fragment from xblock.core import XBlock from xblock.fields import Boolean, List, Scope, String +from common.djangoapps.xblock_django.constants import ATTR_KEY_ANONYMOUS_USER_ID from xmodule.contentstore.content import StaticContent from xmodule.editing_module import EditingMixin from xmodule.edxnotes_utils import edxnotes @@ -42,6 +43,7 @@ @XBlock.needs("i18n") +@XBlock.needs("user") class HtmlBlockMixin( # lint-amnesty, pylint: disable=abstract-method XmlMixin, EditingMixin, XModuleDescriptorToXBlockMixin, XModuleToXBlockMixin, HTMLSnippet, ResourceTemplates, XModuleMixin, @@ -117,8 +119,9 @@ def get_html(self): """ Returns html required for rendering the block. """ if self.data: data = self.data - if getattr(self.runtime, 'anonymous_student_id', None): - data = data.replace("%%USER_ID%%", self.runtime.anonymous_student_id) + user_id = self.runtime.service(self, 'user').get_current_user().opt_attrs.get(ATTR_KEY_ANONYMOUS_USER_ID) + if user_id: + data = data.replace("%%USER_ID%%", user_id) data = data.replace("%%COURSE_ID%%", str(self.scope_ids.usage_id.context_key)) return data return self.data diff --git a/common/lib/xmodule/xmodule/lti_module.py b/common/lib/xmodule/xmodule/lti_module.py index 99f7db03c762..f38cb482e8ee 100644 --- a/common/lib/xmodule/xmodule/lti_module.py +++ b/common/lib/xmodule/xmodule/lti_module.py @@ -78,6 +78,7 @@ from openedx.core.djangolib.markup import HTML, Text from xmodule.editing_module import EditingMixin +from common.djangoapps.xblock_django.constants import ATTR_KEY_ANONYMOUS_USER_ID from xmodule.lti_2_util import LTI20BlockMixin, LTIError from xmodule.raw_module import EmptyDataRawMixin from xmodule.util.xmodule_django import add_webpack_to_fragment @@ -269,6 +270,7 @@ class LTIFields: @XBlock.needs("i18n") +@XBlock.needs("user") class LTIBlock( LTIFields, LTI20BlockMixin, @@ -529,7 +531,10 @@ def preview_handler(self, _, __): return Response(template, content_type='text/html') def get_user_id(self): - user_id = self.runtime.anonymous_student_id + """ + Returns the current user ID, URL-escaped so it is safe to use as a URL component. + """ + user_id = self.runtime.service(self, 'user').get_current_user().opt_attrs.get(ATTR_KEY_ANONYMOUS_USER_ID) assert user_id is not None return str(parse.quote(user_id)) @@ -671,7 +676,8 @@ def oauth_params(self, custom_parameters, client_key, client_secret): # To test functionality test in LMS if callable(self.runtime.get_real_user): - real_user_object = self.runtime.get_real_user(self.runtime.anonymous_student_id) + user_id = self.runtime.service(self, 'user').get_current_user().opt_attrs.get(ATTR_KEY_ANONYMOUS_USER_ID) + real_user_object = self.runtime.get_real_user(user_id) try: self.user_email = real_user_object.email # lint-amnesty, pylint: disable=attribute-defined-outside-init except AttributeError: diff --git a/common/lib/xmodule/xmodule/modulestore/tests/django_utils.py b/common/lib/xmodule/xmodule/modulestore/tests/django_utils.py index 717618413ddb..f77e9b842259 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/django_utils.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/django_utils.py @@ -29,6 +29,7 @@ from xmodule.modulestore.tests.factories import XMODULE_FACTORY_LOCK from xmodule.modulestore.tests.mongo_connection import MONGO_HOST, MONGO_PORT_NUM + class CourseUserType(Enum): """ Types of users to be used when testing a course. diff --git a/common/lib/xmodule/xmodule/seq_module.py b/common/lib/xmodule/xmodule/seq_module.py index 7e0010562132..6ccd4fea8a39 100644 --- a/common/lib/xmodule/xmodule/seq_module.py +++ b/common/lib/xmodule/xmodule/seq_module.py @@ -33,6 +33,7 @@ XModuleToXBlockMixin, ) +from common.djangoapps.xblock_django.constants import ATTR_KEY_USER_ID, ATTR_KEY_USER_IS_STAFF from openedx.core.djangoapps.agreements.toggles import is_integrity_signature_enabled from .exceptions import NotFoundError @@ -378,7 +379,7 @@ def get_metadata(self, view=STUDENT_VIEW, context=None): is_hidden_after_due = False if self._required_prereq(): - if self.runtime.user_is_staff: + if self.runtime.service(self, 'user').get_current_user().opt_attrs.get(ATTR_KEY_USER_IS_STAFF): banner_text = _( 'This subsection is unlocked for learners when they meet the prerequisite requirements.' ) @@ -459,7 +460,7 @@ def student_view(self, context): prereq_met = True prereq_meta_info = {} if self._required_prereq(): - if self.runtime.user_is_staff: + if self.runtime.service(self, 'user').get_current_user().opt_attrs.get(ATTR_KEY_USER_IS_STAFF): banner_text = _( 'This subsection is unlocked for learners when they meet the prerequisite requirements.' ) @@ -553,7 +554,7 @@ def _can_user_view_content(self, course): """ hidden_date = course.end if course.self_paced else self.due return ( - self.runtime.user_is_staff or + self.runtime.service(self, 'user').get_current_user().opt_attrs.get(ATTR_KEY_USER_IS_STAFF) or self.verify_current_content_visibility(hidden_date, self.hide_after_due) ) @@ -643,8 +644,9 @@ def _is_gate_fulfilled(self): """ gating_service = self.runtime.service(self, 'gating') if gating_service: + user_id = self.runtime.service(self, 'user').get_current_user().opt_attrs.get(ATTR_KEY_USER_ID) fulfilled = gating_service.is_gate_fulfilled( - self.course_id, self.location, self.runtime.user_id + self.course_id, self.location, user_id ) return fulfilled @@ -692,7 +694,8 @@ def descendants_are_gated(self, context): comes to determining whether a student is allowed to access this, with other checks being done in has_access calls. """ - if self.runtime.user_is_staff or context.get('specific_masquerade', False): + user_is_staff = self.runtime.service(self, 'user').get_current_user().opt_attrs.get(ATTR_KEY_USER_IS_STAFF) + if user_is_staff or context.get('specific_masquerade', False): return False # We're not allowed to see it because of pre-reqs that haven't been @@ -723,7 +726,8 @@ def _compute_is_prereq_met(self, recalc_on_unmet): """ gating_service = self.runtime.service(self, 'gating') if gating_service: - return gating_service.compute_is_prereq_met(self.location, self.runtime.user_id, recalc_on_unmet) + user_id = self.runtime.service(self, 'user').get_current_user().opt_attrs.get(ATTR_KEY_USER_ID) + return gating_service.compute_is_prereq_met(self.location, user_id, recalc_on_unmet) return True, {} @@ -915,8 +919,10 @@ def _time_limited_student_view(self): self.is_time_limited ) if feature_enabled: - user_id = self.runtime.user_id - user_role_in_course = 'staff' if self.runtime.user_is_staff else 'student' + current_user = self.runtime.service(self, 'user').get_current_user() + user_id = current_user.opt_attrs.get(ATTR_KEY_USER_ID) + user_is_staff = current_user.opt_attrs.get(ATTR_KEY_USER_IS_STAFF) + user_role_in_course = 'staff' if user_is_staff else 'student' course_id = self.runtime.course_id content_id = self.location diff --git a/common/lib/xmodule/xmodule/tests/__init__.py b/common/lib/xmodule/xmodule/tests/__init__.py index 2ebc9787a604..c91da247d8da 100644 --- a/common/lib/xmodule/xmodule/tests/__init__.py +++ b/common/lib/xmodule/xmodule/tests/__init__.py @@ -34,8 +34,10 @@ from xmodule.modulestore.draft_and_published import ModuleStoreDraftAndPublished from xmodule.modulestore.inheritance import InheritanceMixin from xmodule.modulestore.xml import CourseLocationManager +from xmodule.tests.helpers import StubUserService from xmodule.x_module import ModuleSystem, XModuleDescriptor, XModuleMixin + MODULE_DIR = path(__file__).dirname() # Location of common test DATA directory # '../../../../edx-platform/common/test/data/' @@ -90,6 +92,7 @@ def __repr__(self): def get_test_system( course_id=CourseKey.from_string('/'.join(['org', 'course', 'run'])), user=None, + user_is_staff=False, ): """ Construct a test ModuleSystem instance. @@ -105,6 +108,11 @@ def get_test_system( """ if not user: user = Mock(name='get_test_system.user', is_staff=False) + user_service = StubUserService( + user=user, + anonymous_user_id='student', + user_is_staff=user_is_staff, + ) descriptor_system = get_test_descriptor_system() @@ -130,11 +138,13 @@ def get_module(descriptor): get_module=get_module, render_template=mock_render_template, replace_urls=str, - user=user, get_real_user=lambda __: user, filestore=Mock(name='get_test_system.filestore', root_path='.'), debug=True, hostname="edx.org", + services={ + 'user': user_service, + }, xqueue={ 'interface': None, 'callback_url': '/', @@ -143,7 +153,6 @@ def get_module(descriptor): 'construct_callback': Mock(name='get_test_system.xqueue.construct_callback', side_effect="/"), }, node_path=os.environ.get("NODE_PATH", "/usr/local/lib/node_modules"), - anonymous_student_id='student', course_id=course_id, error_descriptor_class=ErrorBlock, get_user_role=Mock(name='get_test_system.get_user_role', is_staff=False), diff --git a/common/lib/xmodule/xmodule/tests/helpers.py b/common/lib/xmodule/xmodule/tests/helpers.py index 42156049fdc5..4e9e9d1d14db 100644 --- a/common/lib/xmodule/xmodule/tests/helpers.py +++ b/common/lib/xmodule/xmodule/tests/helpers.py @@ -4,6 +4,7 @@ import filecmp +from unittest.mock import Mock from path import Path as path from xblock.reference.user_service import UserService, XBlockUser @@ -34,8 +35,10 @@ class StubUserService(UserService): Stub UserService for testing the sequence module. """ - def __init__(self, is_anonymous=False, **kwargs): - self.is_anonymous = is_anonymous + def __init__(self, user=None, user_is_staff=False, anonymous_user_id=None, **kwargs): + self.user = user or Mock(name='StubUserService.user') + self.user_is_staff = user_is_staff + self.anonymous_user_id = anonymous_user_id super().__init__(**kwargs) def get_current_user(self): @@ -43,9 +46,12 @@ def get_current_user(self): Implements abstract method for getting the current user. """ user = XBlockUser() - if self.is_anonymous: + if self.user.is_authenticated: + user.opt_attrs['edx-platform.anonymous_user_id'] = self.anonymous_user_id + 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.username'] = self.user.username + else: user.opt_attrs['edx-platform.username'] = 'anonymous' user.opt_attrs['edx-platform.is_authenticated'] = False - else: - user.opt_attrs['edx-platform.username'] = 'bilbo' return user diff --git a/common/lib/xmodule/xmodule/tests/test_capa_module.py b/common/lib/xmodule/xmodule/tests/test_capa_module.py index 30f2e5cd4bc5..58de4adc261e 100644 --- a/common/lib/xmodule/xmodule/tests/test_capa_module.py +++ b/common/lib/xmodule/xmodule/tests/test_capa_module.py @@ -113,8 +113,7 @@ def create(cls, attempts=None, problem_state=None, correct=False, xml=None, over # since everything else is a string. field_data['attempts'] = int(attempts) - system = get_test_system(course_id=location.course_key) - system.user_is_staff = kwargs.get('user_is_staff', False) + system = get_test_system(course_id=location.course_key, user_is_staff=kwargs.get('user_is_staff', False)) system.render_template = Mock(return_value="
Test Template HTML
") module = ProblemBlock( system, diff --git a/common/lib/xmodule/xmodule/tests/test_conditional.py b/common/lib/xmodule/xmodule/tests/test_conditional.py index 49c8603be320..13f2dc6a9504 100644 --- a/common/lib/xmodule/xmodule/tests/test_conditional.py +++ b/common/lib/xmodule/xmodule/tests/test_conditional.py @@ -178,7 +178,6 @@ def test_handle_ajax(self): modules['cond_module'].save() modules['source_module'].is_attempted = "false" ajax = json.loads(modules['cond_module'].handle_ajax('', '')) - print("ajax: ", ajax) fragments = ajax['fragments'] assert not any(('This is a secret' in item['content']) for item in fragments) @@ -186,7 +185,6 @@ def test_handle_ajax(self): modules['source_module'].is_attempted = "true" ajax = json.loads(modules['cond_module'].handle_ajax('', '')) modules['cond_module'].save() - print("post-attempt ajax: ", ajax) fragments = ajax['fragments'] assert any(('This is a secret' in item['content']) for item in fragments) @@ -220,63 +218,29 @@ class ConditionalBlockXmlTest(unittest.TestCase): Make sure ConditionalBlock works, by loading data in from an XML-defined course. """ - @staticmethod - def get_system(load_error_modules=True): - '''Get a dummy system''' - return DummySystem(load_error_modules) - def setUp(self): super().setUp() self.test_system = get_test_system() - - def get_course(self, name): - """Get a test course by directory name. If there's more than one, error.""" - print(f"Importing {name}") - - modulestore = XMLModuleStore(DATA_DIR, source_dirs=[name]) - courses = modulestore.get_courses() - self.modulestore = modulestore # lint-amnesty, pylint: disable=attribute-defined-outside-init + self.modulestore = XMLModuleStore(DATA_DIR, source_dirs=['conditional_and_poll']) + courses = self.modulestore.get_courses() assert len(courses) == 1 - return courses[0] + self.course = courses[0] + + def get_module_for_location(self, location): + descriptor = self.modulestore.get_item(location, depth=None) + return self.test_system.get_module(descriptor) @patch('xmodule.x_module.descriptor_global_local_resource_url') @patch.dict(settings.FEATURES, {'ENABLE_EDXNOTES': False}) def test_conditional_module(self, _): """Make sure that conditional module works""" - - print("Starting import") - course = self.get_course('conditional_and_poll') - - print("Course: ", course) - print("id: ", course.id) - - def inner_get_module(descriptor): - if isinstance(descriptor, BlockUsageLocator): - location = descriptor - descriptor = self.modulestore.get_item(location, depth=None) - descriptor.xmodule_runtime = get_test_system() - descriptor.xmodule_runtime.descriptor_runtime = descriptor._runtime # pylint: disable=protected-access - descriptor.xmodule_runtime.get_module = inner_get_module - return descriptor - # edx - HarvardX # cond_test - ER22x location = BlockUsageLocator(CourseLocator("HarvardX", "ER22x", "2013_Spring", deprecated=True), "conditional", "condone", deprecated=True) - def replace_urls(text, staticfiles_prefix=None, replace_prefix='/static/', course_namespace=None): # lint-amnesty, pylint: disable=unused-argument - return text - self.test_system.replace_urls = replace_urls - self.test_system.get_module = inner_get_module - - module = inner_get_module(location) - print("module: ", module) - print("module children: ", module.get_children()) - print("module display items (children): ", module.get_display_items()) - + module = self.get_module_for_location(location) html = module.render(STUDENT_VIEW).content - print("html type: ", type(html)) - print("html: ", html) html_expect = module.xmodule_runtime.render_template( 'conditional_ajax.html', { @@ -288,29 +252,20 @@ def replace_urls(text, staticfiles_prefix=None, replace_prefix='/static/', cours ) assert html == html_expect - gdi = module.get_display_items() - print("gdi=", gdi) - ajax = json.loads(module.handle_ajax('', '')) - module.save() - print("ajax: ", ajax) fragments = ajax['fragments'] assert not any(('This is a secret' in item['content']) for item in fragments) # Now change state of the capa problem to make it completed - inner_module = inner_get_module(location.replace(category="problem", name='choiceprob')) + inner_module = self.get_module_for_location(location.replace(category="problem", name='choiceprob')) inner_module.attempts = 1 # Save our modifications to the underlying KeyValueStore so they can be persisted inner_module.save() ajax = json.loads(module.handle_ajax('', '')) - module.save() - print("post-attempt ajax: ", ajax) fragments = ajax['fragments'] assert any(('This is a secret' in item['content']) for item in fragments) - maxDiff = None - def test_conditional_module_with_empty_sources_list(self): """ If a ConditionalBlock is initialized with an empty sources_list, we assert that the sources_list is set diff --git a/common/lib/xmodule/xmodule/tests/test_html_module.py b/common/lib/xmodule/xmodule/tests/test_html_module.py index 38f6be97721a..5695941d0b3e 100644 --- a/common/lib/xmodule/xmodule/tests/test_html_module.py +++ b/common/lib/xmodule/xmodule/tests/test_html_module.py @@ -4,6 +4,7 @@ from unittest.mock import Mock import ddt +from django.contrib.auth.models import AnonymousUser from django.test.utils import override_settings from opaque_keys.edx.locator import BlockUsageLocator, CourseLocator from xblock.field_data import DictFieldData @@ -135,8 +136,7 @@ def test_substitution_without_magic_string(self): def test_substitution_without_anonymous_student_id(self): sample_xml = '''%%USER_ID%%''' field_data = DictFieldData({'data': sample_xml}) - module_system = get_test_system() - module_system.anonymous_student_id = None + module_system = get_test_system(user=AnonymousUser()) module = HtmlBlock(module_system, field_data, Mock()) assert module.get_html() == sample_xml diff --git a/common/lib/xmodule/xmodule/tests/test_lti20_unit.py b/common/lib/xmodule/xmodule/tests/test_lti20_unit.py index c1c43fd17f04..7dc69c9ad3f0 100644 --- a/common/lib/xmodule/xmodule/tests/test_lti20_unit.py +++ b/common/lib/xmodule/xmodule/tests/test_lti20_unit.py @@ -37,8 +37,7 @@ def test_sanitize_get_context(self): mocked_course = Mock(name='mocked_course', lti_passports=['lti_id:test_client:test_secret']) modulestore = Mock(name='modulestore') modulestore.get_course.return_value = mocked_course - runtime = Mock(name='runtime', modulestore=modulestore, anonymous_student_id='student') - self.xmodule.runtime = runtime + self.xmodule.runtime.modulestore = modulestore self.xmodule.lti_id = "lti_id" test_cases = ( # (before sanitize, after sanitize) diff --git a/common/lib/xmodule/xmodule/tests/test_lti_unit.py b/common/lib/xmodule/xmodule/tests/test_lti_unit.py index 53d173bf3d27..3f13666e98b8 100644 --- a/common/lib/xmodule/xmodule/tests/test_lti_unit.py +++ b/common/lib/xmodule/xmodule/tests/test_lti_unit.py @@ -16,6 +16,7 @@ from xblock.field_data import DictFieldData from xblock.fields import ScopeIds +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 @@ -59,13 +60,14 @@ def setUp(self): self.system.get_real_user = Mock() self.system.publish = Mock() self.system.rebind_noauth_module_to_user = Mock() - self.user_id = self.system.anonymous_student_id self.xmodule = LTIBlock( self.system, DictFieldData({}), ScopeIds(None, None, None, BlockUsageLocator(self.system.course_id, 'lti', 'name')) ) + current_user = self.system.service(self.xmodule, 'user').get_current_user() + self.user_id = current_user.opt_attrs.get(ATTR_KEY_ANONYMOUS_USER_ID) self.lti_id = self.xmodule.lti_id self.unquoted_resource_link_id = '{}-i4x-2-3-lti-31de800015cf4afb973356dbe81496df'.format( self.xmodule.runtime.hostname diff --git a/common/lib/xmodule/xmodule/tests/test_vertical.py b/common/lib/xmodule/xmodule/tests/test_vertical.py index 5a31c16fbcd1..b24e561429d7 100644 --- a/common/lib/xmodule/xmodule/tests/test_vertical.py +++ b/common/lib/xmodule/xmodule/tests/test_vertical.py @@ -13,6 +13,7 @@ import pytz import ddt from fs.memoryfs import MemoryFS +from django.contrib.auth.models import AnonymousUser from . import get_test_system from .helpers import StubUserService @@ -157,11 +158,11 @@ def test_render_student_preview_view(self, context, view, completion_value, days now = datetime.now(pytz.UTC) self.vertical.due = now + timedelta(days=days) if view == STUDENT_VIEW: - self.module_system._services['user'] = StubUserService() + self.module_system._services['user'] = StubUserService(user=Mock(username=self.username)) self.module_system._services['completion'] = StubCompletionService(enabled=True, completion_value=completion_value) elif view == PUBLIC_VIEW: - self.module_system._services['user'] = StubUserService(is_anonymous=True) + self.module_system._services['user'] = StubUserService(user=AnonymousUser()) html = self.module_system.render( self.vertical, view, self.default_context if context is None else context diff --git a/common/lib/xmodule/xmodule/x_module.py b/common/lib/xmodule/xmodule/x_module.py index 9b8f898d1ab0..593d8f117665 100644 --- a/common/lib/xmodule/xmodule/x_module.py +++ b/common/lib/xmodule/xmodule/x_module.py @@ -4,6 +4,7 @@ import os import sys import time +import warnings from collections import namedtuple from functools import partial @@ -41,6 +42,13 @@ from xmodule.modulestore.exceptions import ItemNotFoundError from xmodule.util.xmodule_django import add_webpack_to_fragment +from common.djangoapps.xblock_django.constants import ( + ATTR_KEY_ANONYMOUS_USER_ID, + ATTR_KEY_USER_ID, + ATTR_KEY_USER_IS_STAFF, +) + + log = logging.getLogger(__name__) XMODULE_METRIC_NAME = 'edxapp.xmodule' @@ -1732,7 +1740,77 @@ def _convert_reference_fields_to_keys(self, xblock): setattr(xblock, field.name, field_value) -class ModuleSystem(MetricsMixin, ConfigurableFragmentWrapper, Runtime): +class ModuleSystemShim: + """ + This shim provides the properties formerly available from ModuleSystem which are now being provided by services. + + This shim will be removed, so all properties raise a deprecation warning. + """ + + @property + def anonymous_student_id(self): + """ + Returns the anonymous user ID for the current user and course. + + Deprecated in favor of the user service. + """ + warnings.warn( + 'runtime.anonymous_student_id is deprecated. Please use the user service instead.', + DeprecationWarning, stacklevel=3, + ) + user_service = self._services.get('user') + if user_service: + return user_service.get_current_user().opt_attrs.get(ATTR_KEY_ANONYMOUS_USER_ID) + return None + + @property + def seed(self): + """ + Returns the numeric current user id, for use as a random seed. + Returns 0 if there is no current user. + + Deprecated in favor of the user service. + """ + warnings.warn( + 'runtime.seed is deprecated. Please use the user service `user_id` instead.', + DeprecationWarning, stacklevel=3, + ) + return self.user_id or 0 + + @property + def user_id(self): + """ + Returns the current user id, or None if there is no current user. + + Deprecated in favor of the user service. + """ + warnings.warn( + 'runtime.user_id is deprecated. Please use the user service instead.', + DeprecationWarning, stacklevel=3, + ) + user_service = self._services.get('user') + if user_service: + return user_service.get_current_user().opt_attrs.get(ATTR_KEY_USER_ID) + return None + + @property + def user_is_staff(self): + """ + Returns whether the current user has staff access to the course. + + Deprecated in favor of the user service. + """ + warnings.warn( + 'runtime.user_is_staff 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_USER_IS_STAFF) + return None + + +class ModuleSystem(MetricsMixin, ConfigurableFragmentWrapper, ModuleSystemShim, Runtime): """ This is an abstraction such that x_modules can function independent of the courseware (e.g. import into other types of courseware, LMS, @@ -1747,9 +1825,9 @@ class ModuleSystem(MetricsMixin, ConfigurableFragmentWrapper, Runtime): def __init__( self, static_url, track_function, get_module, render_template, - replace_urls, descriptor_runtime, user=None, filestore=None, + replace_urls, descriptor_runtime, filestore=None, debug=False, hostname="", xqueue=None, publish=None, node_path="", - anonymous_student_id='', course_id=None, + 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, @@ -1771,9 +1849,6 @@ def __init__( render_template - a function that takes (template_file, context), and returns rendered html. - user - The user to base the random number generator seed off of for this - request - filestore - A filestore ojbect. Defaults to an instance of OSFS based at settings.DATA_DIR. @@ -1789,8 +1864,6 @@ def __init__( descriptor_runtime - A `DescriptorSystem` to use for loading xblocks by id - anonymous_student_id - Used for tracking modules with student id - course_id - the course_id containing this module publish(event) - A function that allows XModules to publish events (such as grade changes) @@ -1834,12 +1907,9 @@ def __init__( self.render_template = render_template self.DEBUG = self.debug = debug self.HOSTNAME = self.hostname = hostname - self.seed = user.id if user is not None else 0 self.replace_urls = replace_urls self.node_path = node_path - self.anonymous_student_id = anonymous_student_id self.course_id = course_id - self.user_is_staff = user is not None and user.is_staff if publish: self.publish = publish @@ -1859,9 +1929,6 @@ def __init__( self.descriptor_runtime = descriptor_runtime self.rebind_noauth_module_to_user = rebind_noauth_module_to_user - if user: - self.user_id = user.id - def get(self, attr): """ provide uniform access to attributes (like etree).""" return self.__dict__.get(attr) diff --git a/lms/djangoapps/courseware/module_render.py b/lms/djangoapps/courseware/module_render.py index bee106a48330..d6ac518e9519 100644 --- a/lms/djangoapps/courseware/module_render.py +++ b/lms/djangoapps/courseware/module_render.py @@ -35,13 +35,13 @@ from rest_framework.decorators import api_view from rest_framework.exceptions import APIException from web_fragments.fragment import Fragment -from xblock.core import XBlock from xblock.django.request import django_to_webob_request, webob_to_django_response from xblock.exceptions import NoSuchHandlerError, NoSuchViewError from xblock.reference.plugins import FSService from xblock.runtime import KvsFieldData from common.djangoapps import static_replace +from common.djangoapps.xblock_django.constants import ATTR_KEY_USER_ID from capa.xqueue_interface import XQueueInterface from lms.djangoapps.courseware.access import get_user_role, has_access from lms.djangoapps.courseware.entrance_exams import user_can_skip_entrance_exam, user_has_passed_entrance_exam @@ -97,7 +97,6 @@ from xmodule.modulestore.django import modulestore from xmodule.modulestore.exceptions import ItemNotFoundError from xmodule.util.sandboxing import can_execute_unsafe_code, get_python_lib_zip -from xmodule.x_module import XModuleDescriptor log = logging.getLogger(__name__) @@ -539,6 +538,24 @@ def get_event_handler(event_type): }) return handlers.get(event_type) + # These modules store data using the anonymous_student_id as a key. + # To prevent loss of data, we will continue to provide old modules with + # the per-student anonymized id (as we have in the past), + # while giving selected modules a per-course anonymized id. + # As we have the time to manually test more modules, we can add to the list + # of modules that get the per-course anonymized id. + if getattr(descriptor, 'requires_per_student_anonymous_id', False): + anonymous_student_id = anonymous_id_for_user(user, None) + else: + anonymous_student_id = anonymous_id_for_user(user, course_id) + + user_is_staff = bool(has_access(user, 'staff', descriptor.location, course_id)) + user_service = DjangoXBlockUserService( + user, + user_is_staff=user_is_staff, + anonymous_user_id=anonymous_student_id, + ) + def publish(block, event_type, event): """ A function that allows XModules to publish events. @@ -548,8 +565,10 @@ def publish(block, event_type, event): handle_event(block, event) else: context = contexts.course_context_from_course_id(course_id) - if block.runtime.user_id: - context['user_id'] = block.runtime.user_id + user_id = user_service.get_current_user().opt_attrs.get(ATTR_KEY_USER_ID) + if user_id: + context['user_id'] = user_id + context['asides'] = {} for aside in block.runtime.get_asides(block): if hasattr(aside, 'get_event_context'): @@ -746,23 +765,9 @@ def rebind_noauth_module_to_user(module, real_user): if staff_access: block_wrappers.append(partial(add_staff_markup, user, disable_staff_debug_info)) - # These modules store data using the anonymous_student_id as a key. - # To prevent loss of data, we will continue to provide old modules with - # the per-student anonymized id (as we have in the past), - # while giving selected modules a per-course anonymized id. - # As we have the time to manually test more modules, we can add to the list - # of modules that get the per-course anonymized id. - is_pure_xblock = isinstance(descriptor, XBlock) and not isinstance(descriptor, XModuleDescriptor) - if (is_pure_xblock and not getattr(descriptor, 'requires_per_student_anonymous_id', False)): - anonymous_student_id = anonymous_id_for_user(user, course_id) - else: - anonymous_student_id = anonymous_id_for_user(user, None) - field_data = DateLookupFieldData(descriptor._field_data, course_id, user) # pylint: disable=protected-access field_data = LmsFieldData(field_data, student_data) - user_is_staff = bool(has_access(user, 'staff', descriptor.location, course_id)) - system = LmsModuleSystem( track_function=track_function, render_template=render_to_string, @@ -794,7 +799,6 @@ def rebind_noauth_module_to_user(module, real_user): ), node_path=settings.NODE_PATH, publish=publish, - anonymous_student_id=anonymous_student_id, course_id=course_id, cache=cache, can_execute_unsafe_code=(lambda: can_execute_unsafe_code(course_id)), @@ -806,7 +810,7 @@ def rebind_noauth_module_to_user(module, real_user): services={ 'fs': FSService(), 'field-data': field_data, - 'user': DjangoXBlockUserService(user, user_is_staff=user_is_staff), + 'user': user_service, 'verification': XBlockVerificationService(), 'proctoring': ProctoringService(), 'milestones': milestones_helpers.get_service(), diff --git a/lms/djangoapps/courseware/tests/test_lti_integration.py b/lms/djangoapps/courseware/tests/test_lti_integration.py index e96386b0627d..70ac82167e49 100644 --- a/lms/djangoapps/courseware/tests/test_lti_integration.py +++ b/lms/djangoapps/courseware/tests/test_lti_integration.py @@ -10,6 +10,7 @@ from django.conf import settings from django.urls import reverse +from common.djangoapps.xblock_django.constants import ATTR_KEY_ANONYMOUS_USER_ID from lms.djangoapps.courseware.tests.helpers import BaseTestXmodule from lms.djangoapps.courseware.views.views import get_course_lti_endpoints from openedx.core.lib.url_utils import quote_slashes @@ -40,7 +41,8 @@ def setUp(self): # Note: this course_id is actually a course_key context_id = str(self.item_descriptor.course_id) - user_id = str(self.item_descriptor.xmodule_runtime.anonymous_student_id) + user_service = self.item_descriptor.xmodule_runtime.service(self.item_descriptor, 'user') + user_id = str(user_service.get_current_user().opt_attrs.get(ATTR_KEY_ANONYMOUS_USER_ID)) hostname = self.item_descriptor.xmodule_runtime.hostname resource_link_id = str(urllib.parse.quote(f'{hostname}-{self.item_descriptor.location.html_id()}')) diff --git a/lms/djangoapps/courseware/tests/test_module_render.py b/lms/djangoapps/courseware/tests/test_module_render.py index e6acde7d225d..bc19f06e8353 100644 --- a/lms/djangoapps/courseware/tests/test_module_render.py +++ b/lms/djangoapps/courseware/tests/test_module_render.py @@ -45,6 +45,7 @@ from common.djangoapps.student.tests.factories import GlobalStaffFactory from common.djangoapps.student.tests.factories import RequestFactoryNoCsrf from common.djangoapps.student.tests.factories import UserFactory +from common.djangoapps.xblock_django.constants import ATTR_KEY_ANONYMOUS_USER_ID from lms.djangoapps.courseware import module_render as render from lms.djangoapps.courseware.access_response import AccessResponse from lms.djangoapps.courseware.courses import get_course_info_section, get_course_with_access @@ -1923,7 +1924,7 @@ def _get_anonymous_id(self, course_id, xblock_class): # lint-amnesty, pylint: d if hasattr(xblock_class, 'module_class'): descriptor.module_class = xblock_class.module_class - return render.get_module_for_descriptor_internal( + module = render.get_module_for_descriptor_internal( user=self.user, descriptor=descriptor, student_data=Mock(spec=FieldData, name='student_data'), @@ -1932,7 +1933,9 @@ def _get_anonymous_id(self, course_id, xblock_class): # lint-amnesty, pylint: d xqueue_callback_url_prefix=Mock(name='xqueue_callback_url_prefix'), # XQueue Callback Url Prefix request_token='request_token', course=self.course, - ).xmodule_runtime.anonymous_student_id + ) + current_user = module.xmodule_runtime.service(module, 'user').get_current_user() + return current_user.opt_attrs.get(ATTR_KEY_ANONYMOUS_USER_ID) @ddt.data(*PER_STUDENT_ANONYMIZED_DESCRIPTORS) def test_per_student_anonymized_id(self, descriptor_class): @@ -2557,3 +2560,130 @@ def _verify_descriptor(self, category, course, descriptor, item_id=None): item = self.store.get_item(item_id) assert item.__class__.__name__ == descriptor return item_id + + +@ddt.ddt +class LmsModuleSystemShimTest(SharedModuleStoreTestCase): + """ + Tests that the deprecated attributes in the LMS Module System (XBlock Runtime) return the expected values. + """ + + @classmethod + def setUpClass(cls): + """ + Set up the course and descriptor used to instantiate the runtime. + """ + super().setUpClass() + cls.course = CourseFactory.create() + cls.descriptor = ItemFactory(category="vertical", parent=cls.course) + cls.problem_descriptor = ItemFactory(category="problem", parent=cls.course) + + def setUp(self): + """ + Set up the user and other fields that will be used to instantiate the runtime. + """ + super().setUp() + self.user = UserFactory(id=232) + self.student_data = Mock() + self.track_function = Mock() + self.xqueue_callback_url_prefix = Mock() + self.request_token = Mock() + + @ddt.data( + ('seed', 232), + ('user_id', 232), + ('user_is_staff', False), + ) + @ddt.unpack + def test_user_service_attributes(self, attribute, expected_value): + """ + Tests that the deprecated attributes provided by the user service match expected values. + """ + 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 getattr(runtime, attribute) == expected_value + + @patch('lms.djangoapps.courseware.module_render.has_access', Mock(return_value=True, autospec=True)) + def test_user_is_staff(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.user_is_staff + + def test_anonymous_student_id(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.anonymous_student_id == anonymous_id_for_user(self.user, self.course.id) + + def test_anonymous_student_id_bug(self): + """ + Verifies that subsequent calls to get_module_system_for_user have no effect on each block runtime's + anonymous_student_id value. + """ + problem_runtime, _ = render.get_module_system_for_user( + self.user, + self.student_data, + self.problem_descriptor, + self.course.id, + self.track_function, + self.xqueue_callback_url_prefix, + self.request_token, + course=self.course, + ) + # Ensure the problem block returns a per-user anonymous id + assert problem_runtime.anonymous_student_id == anonymous_id_for_user(self.user, None) + + vertical_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, + ) + # Ensure the vertical block returns a per-course+user anonymous id + assert vertical_runtime.anonymous_student_id == anonymous_id_for_user(self.user, self.course.id) + + # Ensure the problem runtime's anonymous student ID is unchanged after the above call. + assert problem_runtime.anonymous_student_id == anonymous_id_for_user(self.user, None) + + def test_user_service_with_anonymous_user(self): + runtime, _ = render.get_module_system_for_user( + AnonymousUser(), + self.student_data, + self.descriptor, + self.course.id, + self.track_function, + self.xqueue_callback_url_prefix, + self.request_token, + course=self.course, + ) + assert runtime.anonymous_student_id is None + assert runtime.seed == 0 + assert runtime.user_id is None + assert not runtime.user_is_staff diff --git a/lms/djangoapps/edxnotes/decorators.py b/lms/djangoapps/edxnotes/decorators.py index f92ca2939be5..6c5d0ca6d933 100644 --- a/lms/djangoapps/edxnotes/decorators.py +++ b/lms/djangoapps/edxnotes/decorators.py @@ -6,8 +6,10 @@ import json from django.conf import settings +from xblock.exceptions import NoSuchServiceError from common.djangoapps.edxmako.shortcuts import render_to_string +from common.djangoapps.xblock_django.constants import ATTR_KEY_ANONYMOUS_USER_ID def edxnotes(cls): @@ -40,7 +42,11 @@ def get_html(self, *args, **kwargs): # - Harvard Annotation Tool is enabled for the course # - the feature flag or `edxnotes` setting of the course is set to False # - the user is not authenticated - user = self.runtime.get_real_user(self.runtime.anonymous_student_id) + try: + user_id = self.runtime.service(self, 'user').get_current_user().opt_attrs.get(ATTR_KEY_ANONYMOUS_USER_ID) + user = self.runtime.get_real_user(user_id) + except NoSuchServiceError: + user = None if is_studio or not is_feature_enabled(course, user): return original_get_html(self, *args, **kwargs) diff --git a/lms/djangoapps/lms_xblock/runtime.py b/lms/djangoapps/lms_xblock/runtime.py index 57e048aca03f..5ab2fe946bdf 100644 --- a/lms/djangoapps/lms_xblock/runtime.py +++ b/lms/djangoapps/lms_xblock/runtime.py @@ -98,13 +98,9 @@ class UserTagsService: COURSE_SCOPE = user_course_tag_api.COURSE_SCOPE - def __init__(self, runtime): - self.runtime = runtime - - def _get_current_user(self): - """Returns the real, not anonymized, current user.""" - real_user = self.runtime.get_real_user(self.runtime.anonymous_student_id) - return real_user + def __init__(self, user, course_id): + self._user = user + self._course_id = course_id def get_tag(self, scope, key): """ @@ -117,8 +113,8 @@ def get_tag(self, scope, key): raise ValueError(f"unexpected scope {scope}") return user_course_tag_api.get_course_tag( - self._get_current_user(), - self.runtime.course_id, key + self._user, + self._course_id, key ) def set_tag(self, scope, key, value): @@ -133,8 +129,8 @@ def set_tag(self, scope, key, value): raise ValueError(f"unexpected scope {scope}") return user_course_tag_api.set_course_tag( - self._get_current_user(), - self.runtime.course_id, key, value + self._user, + self._course_id, key, value ) @@ -142,25 +138,28 @@ class LmsModuleSystem(ModuleSystem): # pylint: disable=abstract-method """ ModuleSystem specialized to the LMS """ - def __init__(self, **kwargs): + def __init__(self, user, **kwargs): request_cache_dict = DEFAULT_REQUEST_CACHE.data store = modulestore() + course_id = kwargs.get('course_id') services = kwargs.setdefault('services', {}) - user = kwargs.get('user') if user and user.is_authenticated: - services['completion'] = CompletionService(user=user, context_key=kwargs.get('course_id')) + services['completion'] = CompletionService(user=user, context_key=course_id) services['fs'] = xblock.reference.plugins.FSService() services['i18n'] = ModuleI18nService services['library_tools'] = LibraryToolsService(store, user_id=user.id if user else None) services['partitions'] = PartitionService( - course_id=kwargs.get('course_id'), + course_id=course_id, cache=request_cache_dict ) services['settings'] = SettingsService() - services['user_tags'] = UserTagsService(self) + services['user_tags'] = UserTagsService( + user=user, + course_id=course_id, + ) if badges_enabled(): - services['badging'] = BadgingService(course_id=kwargs.get('course_id'), modulestore=store) + services['badging'] = BadgingService(course_id=course_id, modulestore=store) self.request_token = kwargs.pop('request_token', None) services['teams'] = TeamsService() services['teams_configuration'] = TeamsConfigurationService() diff --git a/lms/djangoapps/lms_xblock/test/test_runtime.py b/lms/djangoapps/lms_xblock/test/test_runtime.py index 67632e7e0cf8..42c20de05b71 100644 --- a/lms/djangoapps/lms_xblock/test/test_runtime.py +++ b/lms/djangoapps/lms_xblock/test/test_runtime.py @@ -66,6 +66,7 @@ def setUp(self): render_template=Mock(), replace_urls=str, course_id=self.course_key, + user=Mock(), descriptor_runtime=Mock(), ) @@ -125,18 +126,14 @@ def setUp(self): self.course_id = CourseLocator("org", "course", "run") self.user = UserFactory.create() - def mock_get_real_user(_anon_id): - """Just returns the test user""" - return self.user - self.runtime = LmsModuleSystem( static_url='/static', track_function=Mock(), get_module=Mock(), render_template=Mock(), replace_urls=str, + user=self.user, course_id=self.course_id, - get_real_user=mock_get_real_user, descriptor_runtime=Mock(), ) self.scope = 'course' @@ -192,6 +189,7 @@ def mock_get_real_user(_anon_id): render_template=Mock(), replace_urls=str, course_id=self.course_id, + user=self.user, get_real_user=mock_get_real_user, descriptor_runtime=Mock(), ) @@ -247,6 +245,7 @@ def setUp(self): render_template=Mock(), replace_urls=str, course_id=self.course.id, + user=Mock(), descriptor_runtime=Mock(), ) diff --git a/openedx/core/djangoapps/xblock/runtime/runtime.py b/openedx/core/djangoapps/xblock/runtime/runtime.py index 4bf9f2f0f611..558259d99842 100644 --- a/openedx/core/djangoapps/xblock/runtime/runtime.py +++ b/openedx/core/djangoapps/xblock/runtime/runtime.py @@ -21,6 +21,7 @@ from common.djangoapps.track import contexts as track_contexts from common.djangoapps.track import views as track_views +from common.djangoapps.xblock_django.user_service import DjangoXBlockUserService from lms.djangoapps.courseware.model_data import DjangoKeyValueStore, FieldDataCache from lms.djangoapps.grades.api import signals as grades_signals from openedx.core.djangoapps.xblock.apps import get_xblock_app_config @@ -226,6 +227,14 @@ def service(self, block, service_name): elif service_name == "completion": context_key = block.scope_ids.usage_id.context_key return CompletionService(user=self.user, context_key=context_key) + elif service_name == "user": + return DjangoXBlockUserService( + self.user, + # The value should be updated to whether the user is staff in the context when Blockstore runtime adds + # support for courses. + user_is_staff=self.user.is_staff, + anonymous_user_id=self.anonymous_student_id, + ) elif service_name == "i18n": return ModuleI18nService(block=block) # Check if the XBlockRuntimeSystem wants to handle this: