diff --git a/cms/djangoapps/contentstore/views/preview.py b/cms/djangoapps/contentstore/views/preview.py index f6305cca543a..35bec14c3f05 100644 --- a/cms/djangoapps/contentstore/views/preview.py +++ b/cms/djangoapps/contentstore/views/preview.py @@ -198,10 +198,12 @@ 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, @@ -214,7 +216,7 @@ def _preview_module_system(request, descriptor, field_data): "field-data": field_data, "i18n": ModuleI18nService, "settings": SettingsService(), - "user": DjangoXBlockUserService(request.user, anonymous_user_id='student'), + "user": DjangoXBlockUserService(request.user), "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 deleted file mode 100644 index 3d44db7877b9..000000000000 --- a/common/djangoapps/xblock_django/constants.py +++ /dev/null @@ -1,11 +0,0 @@ -""" -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 7954a09dc1d0..6d090417c5f2 100644 --- a/common/djangoapps/xblock_django/tests/test_user_service.py +++ b/common/djangoapps/xblock_django/tests/test_user_service.py @@ -2,7 +2,6 @@ Tests for the DjangoXBlockUserService. """ -import ddt import pytest from django.test import TestCase from opaque_keys.edx.keys import CourseKey @@ -13,7 +12,6 @@ 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, @@ -23,7 +21,6 @@ ) -@ddt.ddt class UserServiceTestCase(TestCase): """ Tests for the DjangoXBlockUserService. @@ -45,7 +42,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, user_is_staff=False, anonymous_user_id=None): + def assert_xblock_user_matches_django(self, xb_user, dj_user): """ A set of assertions for comparing a XBlockUser to a django User """ @@ -54,8 +51,7 @@ def assert_xblock_user_matches_django(self, xb_user, dj_user, user_is_staff=Fals 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 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 not xb_user.opt_attrs[ATTR_KEY_USER_IS_STAFF] 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): @@ -67,25 +63,14 @@ def test_convert_anon_user(self): assert xb_user.is_current_user self.assert_is_anon_xb_user(xb_user) - @ddt.data( - (False, None), - (True, None), - (False, 'abcdef0123'), - (True, 'abcdef0123'), - ) - @ddt.unpack - def test_convert_authenticate_user(self, user_is_staff, anonymous_user_id): + def test_convert_authenticate_user(self): """ 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, - anonymous_user_id=anonymous_user_id, - ) + django_user_service = DjangoXBlockUserService(self.user) 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) 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 b11cb02d7786..23af5ca273a8 100644 --- a/common/djangoapps/xblock_django/user_service.py +++ b/common/djangoapps/xblock_django/user_service.py @@ -11,16 +11,11 @@ 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 .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, -) - - +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' USER_PREFERENCES_WHITE_LIST = ['pref-lang', 'time_zone'] @@ -29,18 +24,10 @@ 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._django_user.anonymous_user_id = kwargs.get('anonymous_user_id', None) def get_current_user(self): """ @@ -95,7 +82,6 @@ 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] = django_user.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 diff --git a/common/lib/xmodule/xmodule/capa_module.py b/common/lib/xmodule/xmodule/capa_module.py index 0db2eac7c320..1bc0ffb8e3d7 100644 --- a/common/lib/xmodule/xmodule/capa_module.py +++ b/common/lib/xmodule/xmodule/capa_module.py @@ -31,11 +31,6 @@ 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 @@ -118,7 +113,7 @@ def from_json(self, value): to_json = from_json -@XBlock.needs('user') +@XBlock.wants('user') @XBlock.needs('i18n') @XBlock.wants('call_to_action') class ProblemBlock( @@ -789,10 +784,9 @@ def choose_new_seed(self): """ if self.rerandomize == RANDOMIZATION.NEVER: self.seed = 1 - 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 + elif self.rerandomize == RANDOMIZATION.PER_STUDENT and hasattr(self.runtime, 'seed'): # see comment on randomization_bin - self.seed = randomization_bin(user_id, str(self.location).encode('utf-8')) + self.seed = randomization_bin(self.runtime.seed, str(self.location).encode('utf-8')) else: self.seed = struct.unpack('i', os.urandom(4))[0] @@ -807,13 +801,9 @@ 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=anonymous_student_id, + anonymous_student_id=self.runtime.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, @@ -822,7 +812,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=seed, # Why do we do this if we have self.seed? + seed=self.runtime.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 @@ -1422,7 +1412,6 @@ 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 @@ -1430,7 +1419,7 @@ def answer_available(self): return False elif self.showanswer == SHOWANSWER.NEVER: return False - elif user_is_staff: + elif self.runtime.user_is_staff: # This is after the 'never' check because admins can see the answer # unless the problem explicitly prevents it return True @@ -1470,11 +1459,10 @@ 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=user_is_staff, + has_staff_access=self.runtime.user_is_staff, ) def update_score(self, data): @@ -1789,8 +1777,7 @@ def submit_problem(self, data, override_time=False): # If the user is a staff member, include # the full exception, including traceback, # in the response - user_is_staff = self.runtime.service(self, 'user').get_current_user().opt_attrs.get(ATTR_KEY_USER_IS_STAFF) - if user_is_staff: + if self.runtime.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 0329bd0bc2c0..b72c61bc02c8 100644 --- a/common/lib/xmodule/xmodule/html_module.py +++ b/common/lib/xmodule/xmodule/html_module.py @@ -17,7 +17,6 @@ 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 @@ -43,7 +42,6 @@ @XBlock.needs("i18n") -@XBlock.needs("user") class HtmlBlockMixin( # lint-amnesty, pylint: disable=abstract-method XmlMixin, EditingMixin, XModuleDescriptorToXBlockMixin, XModuleToXBlockMixin, HTMLSnippet, ResourceTemplates, XModuleMixin, @@ -119,9 +117,8 @@ def get_html(self): """ Returns html required for rendering the block. """ if self.data: data = self.data - 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) + if getattr(self.runtime, 'anonymous_student_id', None): + data = data.replace("%%USER_ID%%", self.runtime.anonymous_student_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 f38cb482e8ee..99f7db03c762 100644 --- a/common/lib/xmodule/xmodule/lti_module.py +++ b/common/lib/xmodule/xmodule/lti_module.py @@ -78,7 +78,6 @@ 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 @@ -270,7 +269,6 @@ class LTIFields: @XBlock.needs("i18n") -@XBlock.needs("user") class LTIBlock( LTIFields, LTI20BlockMixin, @@ -531,10 +529,7 @@ def preview_handler(self, _, __): return Response(template, content_type='text/html') def get_user_id(self): - """ - 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) + user_id = self.runtime.anonymous_student_id assert user_id is not None return str(parse.quote(user_id)) @@ -676,8 +671,7 @@ def oauth_params(self, custom_parameters, client_key, client_secret): # To test functionality test in LMS if callable(self.runtime.get_real_user): - 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) + real_user_object = self.runtime.get_real_user(self.runtime.anonymous_student_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/seq_module.py b/common/lib/xmodule/xmodule/seq_module.py index 6ccd4fea8a39..7e0010562132 100644 --- a/common/lib/xmodule/xmodule/seq_module.py +++ b/common/lib/xmodule/xmodule/seq_module.py @@ -33,7 +33,6 @@ 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 @@ -379,7 +378,7 @@ def get_metadata(self, view=STUDENT_VIEW, context=None): is_hidden_after_due = False if self._required_prereq(): - if self.runtime.service(self, 'user').get_current_user().opt_attrs.get(ATTR_KEY_USER_IS_STAFF): + if self.runtime.user_is_staff: banner_text = _( 'This subsection is unlocked for learners when they meet the prerequisite requirements.' ) @@ -460,7 +459,7 @@ def student_view(self, context): prereq_met = True prereq_meta_info = {} if self._required_prereq(): - if self.runtime.service(self, 'user').get_current_user().opt_attrs.get(ATTR_KEY_USER_IS_STAFF): + if self.runtime.user_is_staff: banner_text = _( 'This subsection is unlocked for learners when they meet the prerequisite requirements.' ) @@ -554,7 +553,7 @@ def _can_user_view_content(self, course): """ hidden_date = course.end if course.self_paced else self.due return ( - self.runtime.service(self, 'user').get_current_user().opt_attrs.get(ATTR_KEY_USER_IS_STAFF) or + self.runtime.user_is_staff or self.verify_current_content_visibility(hidden_date, self.hide_after_due) ) @@ -644,9 +643,8 @@ 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, user_id + self.course_id, self.location, self.runtime.user_id ) return fulfilled @@ -694,8 +692,7 @@ 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. """ - 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): + if self.runtime.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 @@ -726,8 +723,7 @@ def _compute_is_prereq_met(self, recalc_on_unmet): """ 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) - return gating_service.compute_is_prereq_met(self.location, user_id, recalc_on_unmet) + return gating_service.compute_is_prereq_met(self.location, self.runtime.user_id, recalc_on_unmet) return True, {} @@ -919,10 +915,8 @@ def _time_limited_student_view(self): self.is_time_limited ) if feature_enabled: - 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' + user_id = self.runtime.user_id + user_role_in_course = 'staff' if self.runtime.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 c91da247d8da..2ebc9787a604 100644 --- a/common/lib/xmodule/xmodule/tests/__init__.py +++ b/common/lib/xmodule/xmodule/tests/__init__.py @@ -34,10 +34,8 @@ 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/' @@ -92,7 +90,6 @@ 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. @@ -108,11 +105,6 @@ 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() @@ -138,13 +130,11 @@ 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': '/', @@ -153,6 +143,7 @@ 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 4e9e9d1d14db..42156049fdc5 100644 --- a/common/lib/xmodule/xmodule/tests/helpers.py +++ b/common/lib/xmodule/xmodule/tests/helpers.py @@ -4,7 +4,6 @@ import filecmp -from unittest.mock import Mock from path import Path as path from xblock.reference.user_service import UserService, XBlockUser @@ -35,10 +34,8 @@ 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') - self.user_is_staff = user_is_staff - self.anonymous_user_id = anonymous_user_id + def __init__(self, is_anonymous=False, **kwargs): + self.is_anonymous = is_anonymous super().__init__(**kwargs) def get_current_user(self): @@ -46,12 +43,9 @@ def get_current_user(self): Implements abstract method for getting the current user. """ user = XBlockUser() - 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: + if self.is_anonymous: 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 318405fb3360..7ba5ffa6e021 100644 --- a/common/lib/xmodule/xmodule/tests/test_capa_module.py +++ b/common/lib/xmodule/xmodule/tests/test_capa_module.py @@ -113,7 +113,8 @@ 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, user_is_staff=kwargs.get('user_is_staff', False)) + system = get_test_system(course_id=location.course_key) + system.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 13f2dc6a9504..49c8603be320 100644 --- a/common/lib/xmodule/xmodule/tests/test_conditional.py +++ b/common/lib/xmodule/xmodule/tests/test_conditional.py @@ -178,6 +178,7 @@ 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) @@ -185,6 +186,7 @@ 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) @@ -218,29 +220,63 @@ 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() - self.modulestore = XMLModuleStore(DATA_DIR, source_dirs=['conditional_and_poll']) - courses = self.modulestore.get_courses() - assert len(courses) == 1 - 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) + 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 + assert len(courses) == 1 + return courses[0] @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) - module = self.get_module_for_location(location) + 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()) + html = module.render(STUDENT_VIEW).content + print("html type: ", type(html)) + print("html: ", html) html_expect = module.xmodule_runtime.render_template( 'conditional_ajax.html', { @@ -252,20 +288,29 @@ def test_conditional_module(self, _): ) 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 = self.get_module_for_location(location.replace(category="problem", name='choiceprob')) + inner_module = inner_get_module(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 5695941d0b3e..38f6be97721a 100644 --- a/common/lib/xmodule/xmodule/tests/test_html_module.py +++ b/common/lib/xmodule/xmodule/tests/test_html_module.py @@ -4,7 +4,6 @@ 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 @@ -136,7 +135,8 @@ 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(user=AnonymousUser()) + module_system = get_test_system() + module_system.anonymous_student_id = None 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 7dc69c9ad3f0..c1c43fd17f04 100644 --- a/common/lib/xmodule/xmodule/tests/test_lti20_unit.py +++ b/common/lib/xmodule/xmodule/tests/test_lti20_unit.py @@ -37,7 +37,8 @@ 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 - self.xmodule.runtime.modulestore = modulestore + runtime = Mock(name='runtime', modulestore=modulestore, anonymous_student_id='student') + self.xmodule.runtime = runtime 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 3f13666e98b8..53d173bf3d27 100644 --- a/common/lib/xmodule/xmodule/tests/test_lti_unit.py +++ b/common/lib/xmodule/xmodule/tests/test_lti_unit.py @@ -16,7 +16,6 @@ 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 @@ -60,14 +59,13 @@ 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 b24e561429d7..5a31c16fbcd1 100644 --- a/common/lib/xmodule/xmodule/tests/test_vertical.py +++ b/common/lib/xmodule/xmodule/tests/test_vertical.py @@ -13,7 +13,6 @@ 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 @@ -158,11 +157,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(user=Mock(username=self.username)) + self.module_system._services['user'] = StubUserService() self.module_system._services['completion'] = StubCompletionService(enabled=True, completion_value=completion_value) elif view == PUBLIC_VIEW: - self.module_system._services['user'] = StubUserService(user=AnonymousUser()) + self.module_system._services['user'] = StubUserService(is_anonymous=True) 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 593d8f117665..9b8f898d1ab0 100644 --- a/common/lib/xmodule/xmodule/x_module.py +++ b/common/lib/xmodule/xmodule/x_module.py @@ -4,7 +4,6 @@ import os import sys import time -import warnings from collections import namedtuple from functools import partial @@ -42,13 +41,6 @@ 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' @@ -1740,77 +1732,7 @@ def _convert_reference_fields_to_keys(self, xblock): setattr(xblock, field.name, field_value) -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): +class ModuleSystem(MetricsMixin, ConfigurableFragmentWrapper, Runtime): """ This is an abstraction such that x_modules can function independent of the courseware (e.g. import into other types of courseware, LMS, @@ -1825,9 +1747,9 @@ class ModuleSystem(MetricsMixin, ConfigurableFragmentWrapper, ModuleSystemShim, def __init__( self, static_url, track_function, get_module, render_template, - replace_urls, descriptor_runtime, filestore=None, + replace_urls, descriptor_runtime, user=None, filestore=None, debug=False, hostname="", xqueue=None, publish=None, node_path="", - course_id=None, + anonymous_student_id='', 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, @@ -1849,6 +1771,9 @@ 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. @@ -1864,6 +1789,8 @@ 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) @@ -1907,9 +1834,12 @@ 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 @@ -1929,6 +1859,9 @@ 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 d6ac518e9519..bee106a48330 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,6 +97,7 @@ 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__) @@ -538,24 +539,6 @@ 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. @@ -565,10 +548,8 @@ def publish(block, event_type, event): handle_event(block, event) else: context = contexts.course_context_from_course_id(course_id) - user_id = user_service.get_current_user().opt_attrs.get(ATTR_KEY_USER_ID) - if user_id: - context['user_id'] = user_id - + if block.runtime.user_id: + context['user_id'] = block.runtime.user_id context['asides'] = {} for aside in block.runtime.get_asides(block): if hasattr(aside, 'get_event_context'): @@ -765,9 +746,23 @@ 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, @@ -799,6 +794,7 @@ 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)), @@ -810,7 +806,7 @@ def rebind_noauth_module_to_user(module, real_user): services={ 'fs': FSService(), 'field-data': field_data, - 'user': user_service, + 'user': DjangoXBlockUserService(user, user_is_staff=user_is_staff), '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 70ac82167e49..e96386b0627d 100644 --- a/lms/djangoapps/courseware/tests/test_lti_integration.py +++ b/lms/djangoapps/courseware/tests/test_lti_integration.py @@ -10,7 +10,6 @@ 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 @@ -41,8 +40,7 @@ def setUp(self): # Note: this course_id is actually a course_key context_id = str(self.item_descriptor.course_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)) + user_id = str(self.item_descriptor.xmodule_runtime.anonymous_student_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 41a961625a60..a8166bdbd15a 100644 --- a/lms/djangoapps/courseware/tests/test_module_render.py +++ b/lms/djangoapps/courseware/tests/test_module_render.py @@ -45,7 +45,6 @@ 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 @@ -1921,7 +1920,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 - module = render.get_module_for_descriptor_internal( + return render.get_module_for_descriptor_internal( user=self.user, descriptor=descriptor, student_data=Mock(spec=FieldData, name='student_data'), @@ -1930,9 +1929,7 @@ 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, - ) - current_user = module.xmodule_runtime.service(module, 'user').get_current_user() - return current_user.opt_attrs.get(ATTR_KEY_ANONYMOUS_USER_ID) + ).xmodule_runtime.anonymous_student_id @ddt.data(*PER_STUDENT_ANONYMIZED_DESCRIPTORS) def test_per_student_anonymized_id(self, descriptor_class): @@ -2557,95 +2554,3 @@ 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) - - 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_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 6c5d0ca6d933..f92ca2939be5 100644 --- a/lms/djangoapps/edxnotes/decorators.py +++ b/lms/djangoapps/edxnotes/decorators.py @@ -6,10 +6,8 @@ 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): @@ -42,11 +40,7 @@ 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 - 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 + user = self.runtime.get_real_user(self.runtime.anonymous_student_id) 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 5ab2fe946bdf..57e048aca03f 100644 --- a/lms/djangoapps/lms_xblock/runtime.py +++ b/lms/djangoapps/lms_xblock/runtime.py @@ -98,9 +98,13 @@ class UserTagsService: COURSE_SCOPE = user_course_tag_api.COURSE_SCOPE - def __init__(self, user, course_id): - self._user = user - self._course_id = course_id + 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 get_tag(self, scope, key): """ @@ -113,8 +117,8 @@ def get_tag(self, scope, key): raise ValueError(f"unexpected scope {scope}") return user_course_tag_api.get_course_tag( - self._user, - self._course_id, key + self._get_current_user(), + self.runtime.course_id, key ) def set_tag(self, scope, key, value): @@ -129,8 +133,8 @@ def set_tag(self, scope, key, value): raise ValueError(f"unexpected scope {scope}") return user_course_tag_api.set_course_tag( - self._user, - self._course_id, key, value + self._get_current_user(), + self.runtime.course_id, key, value ) @@ -138,28 +142,25 @@ class LmsModuleSystem(ModuleSystem): # pylint: disable=abstract-method """ ModuleSystem specialized to the LMS """ - def __init__(self, user, **kwargs): + def __init__(self, **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=course_id) + services['completion'] = CompletionService(user=user, context_key=kwargs.get('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=course_id, + course_id=kwargs.get('course_id'), cache=request_cache_dict ) services['settings'] = SettingsService() - services['user_tags'] = UserTagsService( - user=user, - course_id=course_id, - ) + services['user_tags'] = UserTagsService(self) if badges_enabled(): - services['badging'] = BadgingService(course_id=course_id, modulestore=store) + services['badging'] = BadgingService(course_id=kwargs.get('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 42c20de05b71..67632e7e0cf8 100644 --- a/lms/djangoapps/lms_xblock/test/test_runtime.py +++ b/lms/djangoapps/lms_xblock/test/test_runtime.py @@ -66,7 +66,6 @@ def setUp(self): render_template=Mock(), replace_urls=str, course_id=self.course_key, - user=Mock(), descriptor_runtime=Mock(), ) @@ -126,14 +125,18 @@ 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' @@ -189,7 +192,6 @@ 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(), ) @@ -245,7 +247,6 @@ def setUp(self): render_template=Mock(), replace_urls=str, course_id=self.course.id, - user=Mock(), descriptor_runtime=Mock(), ) diff --git a/lms/djangoapps/mobile_api/course_info/tests.py b/lms/djangoapps/mobile_api/course_info/tests.py index 4aca674e38fe..6441f9f4219e 100644 --- a/lms/djangoapps/mobile_api/course_info/tests.py +++ b/lms/djangoapps/mobile_api/course_info/tests.py @@ -8,7 +8,10 @@ from django.urls import reverse from edx_toggles.toggles.testutils import override_waffle_flag from milestones.tests.utils import MilestonesTestCaseMixin +from rest_framework.test import APIClient # pylint: disable=unused-import +from common.djangoapps.student.models import CourseEnrollment # pylint: disable=unused-import +from common.djangoapps.student.tests.factories import UserFactory # pylint: disable=unused-import from lms.djangoapps.course_goals.toggles import RECORD_USER_ACTIVITY_FLAG from lms.djangoapps.mobile_api.testutils import MobileAPITestCase, MobileAuthTestMixin, MobileCourseAccessTestMixin from lms.djangoapps.mobile_api.utils import API_V1, API_V05 @@ -16,6 +19,7 @@ from xmodule.modulestore import ModuleStoreEnum from xmodule.modulestore.django import modulestore from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase +from xmodule.modulestore.tests.factories import CourseFactory # pylint: disable=unused-import from xmodule.modulestore.xml_importer import import_course_from_xml diff --git a/openedx/core/djangoapps/xblock/runtime/runtime.py b/openedx/core/djangoapps/xblock/runtime/runtime.py index 558259d99842..4bf9f2f0f611 100644 --- a/openedx/core/djangoapps/xblock/runtime/runtime.py +++ b/openedx/core/djangoapps/xblock/runtime/runtime.py @@ -21,7 +21,6 @@ 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 @@ -227,14 +226,6 @@ 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: