diff --git a/cms/djangoapps/contentstore/views/preview.py b/cms/djangoapps/contentstore/views/preview.py index b64a77ddb2a7..e2fc1deaf38a 100644 --- a/cms/djangoapps/contentstore/views/preview.py +++ b/cms/djangoapps/contentstore/views/preview.py @@ -35,6 +35,8 @@ from common.djangoapps.edxmako.shortcuts import render_to_string from common.djangoapps.edxmako.services import MakoService from common.djangoapps.xblock_django.user_service import DjangoXBlockUserService +from common.lib.xmodule.xmodule.x_module import AsideKeyGenerator, OpaqueKeyReader +from lms.djangoapps.courseware.services import ModuleService from lms.djangoapps.lms_xblock.field_data import LmsFieldData from openedx.core.lib.license import wrap_with_license from openedx.core.lib.cache_utils import CacheService @@ -207,11 +209,12 @@ def _preview_module_system(request, descriptor, field_data): else: preview_anonymous_user_id = anonymous_id_for_user(request.user, course_id) + module_service = ModuleService(partial(_load_preview_module, request=request)) + return PreviewModuleSystem( static_url=settings.STATIC_URL, # TODO (cpennington): Do we want to track how instructors are using the preview problems? track_function=lambda event_type, event: None, - get_module=partial(_load_preview_module, request), debug=True, mixins=settings.XBLOCK_MIXINS, course_id=course_id, @@ -221,7 +224,6 @@ def _preview_module_system(request, descriptor, field_data): wrappers_asides=wrappers_asides, error_descriptor_class=ErrorBlock, # Get the raw DescriptorSystem, not the CombinedSystem - descriptor_runtime=descriptor._runtime, # pylint: disable=protected-access services={ "field-data": field_data, "i18n": ModuleI18nService, @@ -236,8 +238,11 @@ def _preview_module_system(request, descriptor, field_data): "teams_configuration": TeamsConfigurationService(), "sandbox": SandboxService(contentstore=contentstore, course_id=course_id), "cache": CacheService(cache), - 'replace_urls': replace_url_service + 'replace_urls': replace_url_service, + 'module': module_service }, + id_reader=getattr(descriptor._runtime, 'id_reader', OpaqueKeyReader()), # pylint: disable=protected-access + id_generator=getattr(descriptor._runtime, 'id_generator', AsideKeyGenerator()), # pylint: disable=protected-access ) diff --git a/cms/djangoapps/contentstore/views/tests/test_preview.py b/cms/djangoapps/contentstore/views/tests/test_preview.py index 66e934911f06..ed476b91f887 100644 --- a/cms/djangoapps/contentstore/views/tests/test_preview.py +++ b/cms/djangoapps/contentstore/views/tests/test_preview.py @@ -28,7 +28,7 @@ from common.djangoapps import static_replace from common.djangoapps.student.tests.factories import UserFactory -from ..preview import _preview_module_system, get_preview_fragment +from ..preview import _preview_module_system, get_preview_fragment, _load_preview_module @ddt.ddt @@ -178,6 +178,7 @@ def test_block_branch_not_changed_by_preview_handler(self, default_store): @XBlock.needs("field-data") @XBlock.needs("i18n") @XBlock.needs("mako") +@XBlock.needs("module") @XBlock.needs("replace_urls") @XBlock.needs("user") @XBlock.needs("teams_configuration") @@ -210,7 +211,7 @@ def setUp(self): self.field_data = mock.Mock() @XBlock.register_temp_plugin(PureXBlock, identifier='pure') - @ddt.data("user", "i18n", "field-data", "teams_configuration", "replace_urls") + @ddt.data("user", "i18n", "field-data", "teams_configuration", "replace_urls", "module") def test_expected_services_exist(self, expected_service): """ Tests that the 'user' and 'i18n' services are provided by the Studio runtime. @@ -298,6 +299,9 @@ def test_replace_urls(self): assert self.runtime.replace_urls(html) == \ static_replace.replace_static_urls(html, course_id=self.runtime.course_id) + def test_get_module(self): + assert self.runtime.get_module(self.descriptor) == _load_preview_module(self.request, self.descriptor) + def test_anonymous_user_id_preview(self): assert self.runtime.anonymous_student_id == 'student' diff --git a/common/lib/xmodule/xmodule/conditional_module.py b/common/lib/xmodule/xmodule/conditional_module.py index 238d5564fffd..399da809b914 100644 --- a/common/lib/xmodule/xmodule/conditional_module.py +++ b/common/lib/xmodule/xmodule/conditional_module.py @@ -39,6 +39,7 @@ @XBlock.needs('mako') +@XBlock.needs('module') class ConditionalBlock( SequenceMixin, MakoTemplateBlockBase, @@ -317,7 +318,8 @@ def get_required_blocks(self): """ Returns a list of bound XBlocks instances upon which XBlock depends. """ - return [self.system.get_module(descriptor) for descriptor in self.get_required_module_descriptors()] + module_service = self.runtime.service(self, 'module') + return [module_service.get_module(descriptor) for descriptor in self.get_required_module_descriptors()] def get_required_module_descriptors(self): """ diff --git a/common/lib/xmodule/xmodule/library_sourced_block.py b/common/lib/xmodule/xmodule/library_sourced_block.py index d96741d6e447..8a917bc800c0 100644 --- a/common/lib/xmodule/xmodule/library_sourced_block.py +++ b/common/lib/xmodule/xmodule/library_sourced_block.py @@ -101,7 +101,13 @@ def student_view(self, context): Renders the view that learners see. """ result = Fragment() - child_frags = self.runtime.render_children(self, context=context) + + # TODO: uncomment this line and remove _render_children() once + # merger of ModuleSystem and DescriptorSystem is complete + + # child_frags = self.runtime.render_children(self, context=context) + child_frags = self._render_children(context) + result.add_resources(child_frags) result.add_content('
') for frag in child_frags: @@ -109,6 +115,26 @@ def student_view(self, context): result.add_content('
') return result + def _render_children(self, context): + """ + Use CombinedSystem runtime to get block and render each child individually. + + The runtime.render_children() method was earlier used to render each child + However, this caused a problem, when we removed the get_block() and + descriptor_runtime properties from the ModuleSystem, as the render_children + method ran in the context of LmsModuleSystem which is a child class of + ModuleSystem. + + This is intended to be a temporary method until deprecation of all properties + of ModuleSystem and merger of ModuleSystem and DescriptorSystem is complete. + """ + results = [] + for child_id in self.children: + child = self.runtime.get_block(child_id) + result = self.runtime.render_child(child, context=context) + results.append(result) + return results + def validate_field_data(self, validation, data): """ Validate this block's field data. Instead of checking fields like self.name, check the diff --git a/common/lib/xmodule/xmodule/split_test_module.py b/common/lib/xmodule/xmodule/split_test_module.py index 069f9995481f..d269646d5c44 100644 --- a/common/lib/xmodule/xmodule/split_test_module.py +++ b/common/lib/xmodule/xmodule/split_test_module.py @@ -125,6 +125,7 @@ def get_split_user_partitions(user_partitions): @XBlock.needs('mako') @XBlock.needs('partitions') @XBlock.needs('user') +@XBlock.needs('module') class SplitTestBlock( # lint-amnesty, pylint: disable=abstract-method SplitTestFields, SequenceMixin, @@ -192,7 +193,7 @@ def child(self): Return the user bound child block for the partition or None. """ if self.child_descriptor is not None: - return self.system.get_module(self.child_descriptor) + return self.runtime.service(self, 'module').get_module(self.child_descriptor) else: return None @@ -272,7 +273,7 @@ def _staff_view(self, context): for child_location in self.children: # pylint: disable=no-member child_descriptor = self.get_child_descriptor_by_location(child_location) - child = self.system.get_module(child_descriptor) + child = self.runtime.service(self, 'module').get_module(child_descriptor) rendered_child = child.render(STUDENT_VIEW, context) fragment.add_fragment_resources(rendered_child) group_name, updated_group_id = self.get_data_for_vertical(child) @@ -347,7 +348,7 @@ def studio_render_children(self, fragment, children, context): """ html = "" for active_child_descriptor in children: - active_child = self.system.get_module(active_child_descriptor) + active_child = self.runtime.service(self, 'module').get_module(active_child_descriptor) rendered_child = active_child.render(StudioEditableBlock.get_preview_view_name(active_child), context) if active_child.category == 'vertical': group_name, group_id = self.get_data_for_vertical(active_child) diff --git a/common/lib/xmodule/xmodule/tests/__init__.py b/common/lib/xmodule/xmodule/tests/__init__.py index fd38c6f64d45..4683b9ca3e7d 100644 --- a/common/lib/xmodule/xmodule/tests/__init__.py +++ b/common/lib/xmodule/xmodule/tests/__init__.py @@ -15,10 +15,11 @@ import traceback import unittest from contextlib import contextmanager -from functools import wraps +from functools import partial, wraps from unittest.mock import Mock from django.test import TestCase +from lms.djangoapps.courseware.services import ModuleService from opaque_keys.edx.keys import CourseKey from path import Path as path @@ -152,7 +153,6 @@ def get_module(descriptor): return TestModuleSystem( static_url='/static', track_function=Mock(name='get_test_system.track_function'), - get_module=get_module, debug=True, hostname="edx.org", services={ @@ -166,12 +166,12 @@ def get_module(descriptor): waittime=10, construct_callback=Mock(name='get_test_system.xqueue.construct_callback', side_effect="/"), ), - 'replace_urls': replace_url_service + 'replace_urls': replace_url_service, + 'module': ModuleService(partial(get_module)) }, node_path=os.environ.get("NODE_PATH", "/usr/local/lib/node_modules"), course_id=course_id, error_descriptor_class=ErrorBlock, - descriptor_runtime=descriptor_system, ) diff --git a/common/lib/xmodule/xmodule/tests/test_conditional.py b/common/lib/xmodule/xmodule/tests/test_conditional.py index ac862add812c..e4831942651b 100644 --- a/common/lib/xmodule/xmodule/tests/test_conditional.py +++ b/common/lib/xmodule/xmodule/tests/test_conditional.py @@ -1,11 +1,13 @@ # lint-amnesty, pylint: disable=missing-module-docstring +from functools import partial import json import unittest from unittest.mock import Mock, patch from django.conf import settings from fs.memoryfs import MemoryFS +from lms.djangoapps.courseware.services import ModuleService from lxml import etree from opaque_keys.edx.keys import CourseKey from opaque_keys.edx.locator import BlockUsageLocator, CourseLocator @@ -131,9 +133,15 @@ def load_item(usage_id, for_parent=None): # pylint: disable=unused-argument ScopeIds(None, None, cond_location, cond_location) ) cond_descriptor.xmodule_runtime = system - system.get_module = lambda desc: desc if visible_to_nonstaff_users(desc) else None + + def _check_module_user_access(descriptor): + if visible_to_nonstaff_users(descriptor): + return descriptor + return None + + system._services['module'] = ModuleService(partial(_check_module_user_access)) cond_descriptor.get_required_blocks = [ - system.get_module(source_descriptor), + system._services['module'].get_module(source_descriptor), ] # return dict: @@ -229,7 +237,7 @@ def setUp(self): def get_module_for_location(self, location): descriptor = self.modulestore.get_item(location, depth=None) - return self.test_system.get_module(descriptor) + return self.test_system._services['module'].get_module(descriptor) @patch('xmodule.x_module.descriptor_global_local_resource_url') @patch.dict(settings.FEATURES, {'ENABLE_EDXNOTES': False}) diff --git a/common/lib/xmodule/xmodule/tests/test_library_content.py b/common/lib/xmodule/xmodule/tests/test_library_content.py index 628f471e88eb..a900b4c0fe03 100644 --- a/common/lib/xmodule/xmodule/tests/test_library_content.py +++ b/common/lib/xmodule/xmodule/tests/test_library_content.py @@ -3,10 +3,12 @@ Higher-level tests are in `cms/djangoapps/contentstore/tests/test_libraries.py`. """ +from functools import partial from unittest.mock import Mock, patch from bson.objectid import ObjectId from fs.memoryfs import MemoryFS +from lms.djangoapps.courseware.services import ModuleService from lxml import etree from search.search_engine_base import SearchEngine from web_fragments.fragment import Fragment @@ -61,12 +63,12 @@ def _bind_course_module(self, module): def get_module(descriptor): """Mocks module_system get_module function""" sub_module_system = get_test_system(course_id=module.location.course_key) - sub_module_system.get_module = get_module + sub_module_system._services['module'] = ModuleService(partial(get_module)) sub_module_system.descriptor_runtime = descriptor._runtime # pylint: disable=protected-access descriptor.bind_for_student(sub_module_system, self.user_id) return descriptor - module_system.get_module = get_module + module_system._services['module'] = ModuleService(partial(get_module)) module.xmodule_runtime = module_system diff --git a/common/lib/xmodule/xmodule/unit_block.py b/common/lib/xmodule/xmodule/unit_block.py index 2d3dacb65b8b..09d7cf69bdbe 100644 --- a/common/lib/xmodule/xmodule/unit_block.py +++ b/common/lib/xmodule/xmodule/unit_block.py @@ -44,7 +44,13 @@ class UnitBlock(XBlock): def student_view(self, context=None): """Provide default student view.""" result = Fragment() - child_frags = self.runtime.render_children(self, context=context) + + # TODO: uncomment this line and remove _render_children() once + # merger of ModuleSystem and DescriptorSystem is complete + + # child_frags = self.runtime.render_children(self, context=context) + child_frags = self._render_children(context) + result.add_resources(child_frags) result.add_content('
') for frag in child_frags: @@ -52,6 +58,26 @@ def student_view(self, context=None): result.add_content('
') return result + def _render_children(self, context): + """ + Use CombinedSystem runtime to get block and render each child individually. + + The runtime.render_children() method was earlier used to render each child + However, this caused a problem, when we removed the get_block() and + descriptor_runtime properties from the ModuleSystem, as the render_children + method ran in the context of LmsModuleSystem which is a child class of + ModuleSystem. + + This is intended to be a temporary method until deprecation of all properties + of ModuleSystem and merger of ModuleSystem and DescriptorSystem is complete. + """ + results = [] + for child_id in self.children: + child = self.runtime.get_block(child_id) + result = self.runtime.render_child(child, context=context) + results.append(result) + return results + public_view = student_view def index_dictionary(self): diff --git a/common/lib/xmodule/xmodule/x_module.py b/common/lib/xmodule/xmodule/x_module.py index 307d7c815557..678746d2c30b 100644 --- a/common/lib/xmodule/xmodule/x_module.py +++ b/common/lib/xmodule/xmodule/x_module.py @@ -1609,6 +1609,21 @@ def filestore(self): ) return self.resources_fs + @property + def get_module(self): + """ + Returns a function to bind module system and user data to given descriptor + + Deprecated in favor of module service. + """ + warnings.warn( + 'runtime.get_module is deprecated. Please use the module service instead.', + DeprecationWarning, stacklevel=3, + ) + module_service = self._services.get('module') + if module_service: + return partial(module_service.get_module) + class ModuleSystem(MetricsMixin, ConfigurableFragmentWrapper, ModuleSystemShim, Runtime): """ @@ -1624,8 +1639,8 @@ class ModuleSystem(MetricsMixin, ConfigurableFragmentWrapper, ModuleSystemShim, """ def __init__( - self, static_url, track_function, get_module, - descriptor_runtime, debug=False, hostname="", publish=None, + self, static_url, track_function, + debug=False, hostname="", publish=None, node_path="", course_id=None, error_descriptor_class=None, field_data=None, rebind_noauth_module_to_user=None, **kwargs): @@ -1639,12 +1654,6 @@ def __init__( TODO: Not used, and has inconsistent args in different files. Update or remove. - get_module - function that takes a descriptor and returns a corresponding - module instance object. If the current user does not have - access to that location, returns None. - - descriptor_runtime - A `DescriptorSystem` to use for loading xblocks by id - course_id - the course_id containing this module publish(event) - A function that allows XModules to publish events (such as grade changes) @@ -1659,13 +1668,10 @@ def __init__( # Usage_store is unused, and field_data is often supplanted with an # explicit field_data during construct_xblock. - kwargs.setdefault('id_reader', getattr(descriptor_runtime, 'id_reader', OpaqueKeyReader())) - kwargs.setdefault('id_generator', getattr(descriptor_runtime, 'id_generator', AsideKeyGenerator())) super().__init__(field_data=field_data, **kwargs) self.STATIC_URL = static_url self.track_function = track_function - self.get_module = get_module self.DEBUG = self.debug = debug self.HOSTNAME = self.hostname = hostname self.node_path = node_path @@ -1676,7 +1682,6 @@ def __init__( self.error_descriptor_class = error_descriptor_class self.xmodule_instance = None - self.descriptor_runtime = descriptor_runtime self.rebind_noauth_module_to_user = rebind_noauth_module_to_user def get(self, attr): @@ -1703,9 +1708,6 @@ def ajax_url(self): assert self.xmodule_instance is not None return self.handler_url(self.xmodule_instance, 'xmodule_handler', '', '').rstrip('/?') - def get_block(self, block_id, for_parent=None): # lint-amnesty, pylint: disable=arguments-differ - return self.get_module(self.descriptor_runtime.get_block(block_id, for_parent=for_parent)) - def resource_url(self, resource): raise NotImplementedError("edX Platform doesn't currently implement XBlock resource urls") @@ -1804,6 +1806,24 @@ def service(self, block, service_name): return service + def get_block(self, *args, **kwargs): # lint-amnesty, pylint: disable=arguments-differ + """ + Get the block using the provided arguments (usually block_id and for_parent). + + The method fetches the block from the DescriptorSystem using the given arguments. + If the ModuleSystem is initalized and contains the 'module' service, then the + user data and module system is bound to the block. + """ + block_descriptor = self._descriptor_system.get_block(*args, **kwargs) + + if (self._module_system and + hasattr(self._module_system, '_services') and + self._module_system._services.get('module')): + module_service = self._module_system._services.get('module') # pylint: disable=protected-access + return module_service.get_module(block_descriptor) + + return block_descriptor + def __getattr__(self, name): """ If the ModuleSystem doesn't have an attribute, try returning the same attribute from the diff --git a/lms/djangoapps/courseware/module_render.py b/lms/djangoapps/courseware/module_render.py index 62157ec648bf..c7705d075937 100644 --- a/lms/djangoapps/courseware/module_render.py +++ b/lms/djangoapps/courseware/module_render.py @@ -48,6 +48,7 @@ from common.djangoapps.static_replace.services import ReplaceURLService from common.djangoapps.static_replace.wrapper import replace_urls_wrapper from common.djangoapps.xblock_django.constants import ATTR_KEY_USER_ID +from common.lib.xmodule.xmodule.x_module import AsideKeyGenerator, OpaqueKeyReader from capa.xqueue_interface import XQueueService # lint-amnesty, pylint: disable=wrong-import-order 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 @@ -59,7 +60,7 @@ ) from lms.djangoapps.courseware.model_data import DjangoKeyValueStore, FieldDataCache from lms.djangoapps.courseware.field_overrides import OverrideFieldData -from lms.djangoapps.courseware.services import UserStateService +from lms.djangoapps.courseware.services import ModuleService, UserStateService from lms.djangoapps.grades.api import GradesUtilService from lms.djangoapps.grades.api import signals as grades_signals from lms.djangoapps.lms_xblock.field_data import LmsFieldData @@ -469,30 +470,6 @@ def make_xqueue_callback(dispatch='score_update'): waittime=settings.XQUEUE_WAITTIME_BETWEEN_REQUESTS, ) - def inner_get_module(descriptor): - """ - Delegate to get_module_for_descriptor_internal() with all values except `descriptor` set. - - Because it does an access check, it may return None. - """ - # TODO: fix this so that make_xqueue_callback uses the descriptor passed into - # inner_get_module, not the parent's callback. Add it as an argument.... - return get_module_for_descriptor_internal( - user=user, - descriptor=descriptor, - student_data=student_data, - course_id=course_id, - track_function=track_function, - position=position, - wrap_xmodule_display=wrap_xmodule_display, - grade_bucket_type=grade_bucket_type, - static_asset_path=static_asset_path, - user_location=user_location, - request_token=request_token, - course=course, - will_recheck_access=will_recheck_access, - ) - def get_event_handler(event_type): """ Return an appropriate function to handle the event. @@ -724,10 +701,25 @@ def rebind_noauth_module_to_user(module, real_user): field_data = DateLookupFieldData(descriptor._field_data, course_id, user) # pylint: disable=protected-access field_data = LmsFieldData(field_data, student_data) + module_service = ModuleService(partial( + get_module_for_descriptor_internal, + user=user, + student_data=student_data, + course_id=course_id, + track_function=track_function, + position=position, + wrap_xmodule_display=wrap_xmodule_display, + grade_bucket_type=grade_bucket_type, + static_asset_path=static_asset_path, + user_location=user_location, + request_token=request_token, + course=course, + will_recheck_access=will_recheck_access, + )) + system = LmsModuleSystem( track_function=track_function, static_url=settings.STATIC_URL, - get_module=inner_get_module, user=user, debug=settings.DEBUG, hostname=settings.SITE_NAME, @@ -754,11 +746,13 @@ def rebind_noauth_module_to_user(module, real_user): 'cache': CacheService(cache), 'sandbox': SandboxService(contentstore=contentstore, course_id=course_id), 'xqueue': xqueue_service, - 'replace_urls': replace_url_service + 'replace_urls': replace_url_service, + 'module': module_service }, - descriptor_runtime=descriptor._runtime, # pylint: disable=protected-access rebind_noauth_module_to_user=rebind_noauth_module_to_user, request_token=request_token, + id_reader=getattr(descriptor._runtime, 'id_reader', OpaqueKeyReader()), # pylint: disable=protected-access + id_generator=getattr(descriptor._runtime, 'id_generator', AsideKeyGenerator()), # pylint: disable=protected-access ) # pass position specified in URL to module through ModuleSystem diff --git a/lms/djangoapps/courseware/services.py b/lms/djangoapps/courseware/services.py index 3fa31a3e00fc..8895d2827fdc 100644 --- a/lms/djangoapps/courseware/services.py +++ b/lms/djangoapps/courseware/services.py @@ -39,3 +39,18 @@ def get_state_as_dict(self, username_or_email, block_id): return json.loads(student_module.state) except StudentModule.DoesNotExist: return {} + + +class ModuleService: + """ + A service to bind user data and module system to descriptor using the given callback functiion + """ + + def __init__(self, module_callback) -> None: + self.module_callback = module_callback + + def get_module(self, descriptor): + """ + Invokes the callback function to bind user data and module system to given descriptor + """ + return self.module_callback(descriptor=descriptor) diff --git a/lms/djangoapps/courseware/tests/test_module_render.py b/lms/djangoapps/courseware/tests/test_module_render.py index d9d8aeae0ef9..14272d14ccdb 100644 --- a/lms/djangoapps/courseware/tests/test_module_render.py +++ b/lms/djangoapps/courseware/tests/test_module_render.py @@ -2717,3 +2717,25 @@ def test_replace_jump_to_id_urls(self): jump_to_id_base_url = reverse('jump_to_id', kwargs={'course_id': str(self.runtime.course_id), 'module_id': ''}) assert self.runtime.replace_jump_to_id_urls(html) == \ static_replace.replace_jump_to_id_urls(html, self.runtime.course_id, jump_to_id_base_url) + + def test_get_module(self): + runtime, _ = render.get_module_system_for_user( + self.user, + self.student_data, + self.descriptor, + self.course.id, + self.track_function, + self.request_token, + course=self.course, + ) + module = render.get_module_for_descriptor_internal( + user=self.user, + descriptor=self.descriptor, + student_data=self.student_data, + course_id=self.course.id, + track_function=self.track_function, + request_token=self.request_token, + course=self.course + ) + + assert runtime.get_module(self.descriptor) == module diff --git a/lms/djangoapps/courseware/tests/test_services.py b/lms/djangoapps/courseware/tests/test_services.py index 1e68317d06b5..3ccd7239c470 100644 --- a/lms/djangoapps/courseware/tests/test_services.py +++ b/lms/djangoapps/courseware/tests/test_services.py @@ -3,16 +3,20 @@ """ +from functools import partial import itertools import json +from unittest.mock import Mock import ddt +from xmodule.contentstore.django import contentstore +from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase, SharedModuleStoreTestCase +from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory from common.djangoapps.student.tests.factories import UserFactory -from lms.djangoapps.courseware.services import UserStateService +from lms.djangoapps.courseware.module_render import get_module_for_descriptor_internal +from lms.djangoapps.courseware.services import ModuleService, UserStateService from lms.djangoapps.courseware.tests.factories import StudentModuleFactory -from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase # lint-amnesty, pylint: disable=wrong-import-order -from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory # lint-amnesty, pylint: disable=wrong-import-order @ddt.ddt @@ -124,3 +128,55 @@ def test_nonexistent_student_module_state(self, state_params, should_use_email): self._create_student_module({'key_1': 'value_1'}) state = UserStateService().get_state_as_dict(**params) assert not state + + +class TestModuleService(SharedModuleStoreTestCase): + """ + Tests the Module Service. + """ + COURSE_ID = 'edX/LmsModuleShimTest/2021_Fall' + + @classmethod + def setUpClass(cls): + """ + Set up the course and descriptor used to instantiate the runtime. + """ + super().setUpClass() + org, number, run = cls.COURSE_ID.split('/') + cls.course = CourseFactory.create(org=org, number=number, run=run) + 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.request_token = Mock() + self.contentstore = contentstore() + self.module = get_module_for_descriptor_internal( + user=self.user, + descriptor=self.descriptor, + student_data=self.student_data, + course_id=self.course.id, + track_function=self.track_function, + request_token=self.request_token, + course=self.course + ) + + def test_get_module(self): + """ + Test the get_module method of ModuleService + """ + module_service = ModuleService(partial( + get_module_for_descriptor_internal, + user=self.user, + student_data=self.student_data, + course_id=self.course.id, + track_function=self.track_function, + request_token=self.request_token, + course=self.course + )) + assert module_service.get_module(self.descriptor) == self.module diff --git a/lms/djangoapps/courseware/tests/test_views.py b/lms/djangoapps/courseware/tests/test_views.py index 0db2441ef0ac..925d52236124 100644 --- a/lms/djangoapps/courseware/tests/test_views.py +++ b/lms/djangoapps/courseware/tests/test_views.py @@ -2446,7 +2446,13 @@ def student_view(self, context): # pylint: disable=unused-argument """ msg = f"{self.state} != {self.scope_ids.usage_id}" assert self.state == str(self.scope_ids.usage_id), msg - fragments = self.runtime.render_children(self) + + # TODO: uncomment this line and remove _render_children() once + # merger of ModuleSystem and DescriptorSystem is complete + + # fragments = self.runtime.render_children(self) + fragments = self._render_children() + result = Fragment( content="

ViewCheckerPassed: {}

\n{}".format( str(self.scope_ids.usage_id), @@ -2455,6 +2461,26 @@ def student_view(self, context): # pylint: disable=unused-argument ) return result + def _render_children(self): + """ + Use CombinedSystem runtime to get block and render each child individually. + + The runtime.render_children() method was earlier used to render each child + However, this caused a problem, when we removed the get_block() and + descriptor_runtime properties from the ModuleSystem, as the render_children + method ran in the context of LmsModuleSystem which is a child class of + ModuleSystem. + + This is intended to be a temporary method until deprecation of all properties + of ModuleSystem and merger of ModuleSystem and DescriptorSystem is complete. + """ + results = [] + for child_id in self.children: + child = self.runtime.get_block(child_id) + result = self.runtime.render_child(child) + results.append(result) + return results + @ddt.ddt @set_preview_mode(True) diff --git a/lms/djangoapps/lms_xblock/test/test_runtime.py b/lms/djangoapps/lms_xblock/test/test_runtime.py index d85c9e5816ed..66917cdf9df9 100644 --- a/lms/djangoapps/lms_xblock/test/test_runtime.py +++ b/lms/djangoapps/lms_xblock/test/test_runtime.py @@ -18,6 +18,7 @@ from common.djangoapps.student.tests.factories import UserFactory from lms.djangoapps.badges.tests.factories import BadgeClassFactory from lms.djangoapps.badges.tests.test_models import get_image +from lms.djangoapps.courseware.services import ModuleService from lms.djangoapps.lms_xblock.runtime import LmsModuleSystem from xmodule.modulestore.django import ModuleI18nService # lint-amnesty, pylint: disable=wrong-import-order from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase # lint-amnesty, pylint: disable=wrong-import-order @@ -62,10 +63,13 @@ def setUp(self): self.runtime = LmsModuleSystem( static_url='/static', track_function=Mock(), - get_module=Mock(), + services={ + 'module': ModuleService(Mock()) + }, course_id=self.course_key, user=Mock(), - descriptor_runtime=Mock(), + id_reader=Mock(), + id_generator=Mock() ) def test_trailing_characters(self): @@ -127,10 +131,13 @@ def setUp(self): self.runtime = LmsModuleSystem( static_url='/static', track_function=Mock(), - get_module=Mock(), + services={ + 'module': ModuleService(Mock()) + }, user=self.user, course_id=self.course_id, - descriptor_runtime=Mock(), + id_reader=Mock(), + id_generator=Mock() ) self.scope = 'course' self.key = 'key1' @@ -177,10 +184,13 @@ def create_runtime(self): return LmsModuleSystem( static_url='/static', track_function=Mock(), - get_module=Mock(), + services={ + 'module': ModuleService(Mock()) + }, course_id=self.course_id, user=self.user, - descriptor_runtime=Mock(), + id_reader=Mock(), + id_generator=Mock() ) @patch.dict(settings.FEATURES, {'ENABLE_OPENBADGES': True}) @@ -230,10 +240,13 @@ def setUp(self): self.runtime = LmsModuleSystem( static_url='/static', track_function=Mock(), - get_module=Mock(), + services={ + 'module': ModuleService(Mock()) + }, course_id=self.course.id, user=Mock(), - descriptor_runtime=Mock(), + id_reader=Mock(), + id_generator=Mock() ) self.mock_block = Mock() diff --git a/openedx/tests/completion_integration/test_services.py b/openedx/tests/completion_integration/test_services.py index 0529ec7b7188..2752b53ef576 100644 --- a/openedx/tests/completion_integration/test_services.py +++ b/openedx/tests/completion_integration/test_services.py @@ -3,6 +3,7 @@ """ +from functools import partial import ddt from completion.models import BlockCompletion from completion.services import CompletionService @@ -15,6 +16,7 @@ from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory, LibraryFactory from xmodule.tests import get_test_system +from lms.djangoapps.courseware.services import ModuleService from openedx.core.djangolib.testing.utils import skip_unless_lms from common.djangoapps.student.tests.factories import UserFactory @@ -128,12 +130,12 @@ def _bind_course_module(self, module): def get_module(descriptor): """Mocks module_system get_module function""" sub_module_system = get_test_system(course_id=module.location.course_key) - sub_module_system.get_module = get_module + sub_module_system._services['module'] = ModuleService(partial(get_module)) # pylint: disable=protected-access sub_module_system.descriptor_runtime = descriptor._runtime # pylint: disable=protected-access descriptor.bind_for_student(sub_module_system, self.user.id) return descriptor - module_system.get_module = get_module + module_system._services['module'] = ModuleService(partial(get_module)) # pylint: disable=protected-access module.xmodule_runtime = module_system def test_completion_service(self):