From 4ab77adecc5438bbdb314e5982f04253941ff081 Mon Sep 17 00:00:00 2001 From: Kaustav Banerjee Date: Thu, 22 Dec 2022 13:58:03 +0530 Subject: [PATCH 01/13] chore: move ModuleSystemShim above DescriptorSystem --- xmodule/x_module.py | 522 ++++++++++++++++++++++---------------------- 1 file changed, 261 insertions(+), 261 deletions(-) diff --git a/xmodule/x_module.py b/xmodule/x_module.py index bc19c1456e..5f4d54d7e0 100644 --- a/xmodule/x_module.py +++ b/xmodule/x_module.py @@ -1043,266 +1043,6 @@ class MetricsMixin: ) -class DescriptorSystem(MetricsMixin, ConfigurableFragmentWrapper, Runtime): - """ - Base class for :class:`Runtime`s to be used with :class:`XModuleDescriptor`s - """ - def __init__( - self, load_item, resources_fs, error_tracker, get_policy=None, disabled_xblock_types=lambda: [], **kwargs - ): - """ - load_item: Takes a Location and returns an XModuleDescriptor - - resources_fs: A Filesystem object that contains all of the - resources needed for the course - - error_tracker: A hook for tracking errors in loading the descriptor. - Used for example to get a list of all non-fatal problems on course - load, and display them to the user. - - See errortracker.py for more documentation - - get_policy: a function that takes a usage id and returns a dict of - policy to apply. - - local_resource_url: an implementation of :meth:`xblock.runtime.Runtime.local_resource_url` - - """ - kwargs.setdefault('id_reader', OpaqueKeyReader()) - kwargs.setdefault('id_generator', AsideKeyGenerator()) - super().__init__(**kwargs) - - # This is used by XModules to write out separate files during xml export - self.export_fs = None - - self.load_item = load_item - self.resources_fs = resources_fs - self.error_tracker = error_tracker - if get_policy: - self.get_policy = get_policy - else: - self.get_policy = lambda u: {} - - self.disabled_xblock_types = disabled_xblock_types - - def get_block(self, usage_id, for_parent=None): - """See documentation for `xblock.runtime:Runtime.get_block`""" - return self.load_item(usage_id, for_parent=for_parent) - - def load_block_type(self, block_type): - """ - Returns a subclass of :class:`.XBlock` that corresponds to the specified `block_type`. - """ - if block_type in self.disabled_xblock_types(): - return self.default_class - return super().load_block_type(block_type) - - def get_field_provenance(self, xblock, field): - """ - For the given xblock, return a dict for the field's current state: - { - 'default_value': what json'd value will take effect if field is unset: either the field default or - inherited value, - 'explicitly_set': boolean for whether the current value is set v default/inherited, - } - :param xblock: - :param field: - """ - # pylint: disable=protected-access - # in runtime b/c runtime contains app-specific xblock behavior. Studio's the only app - # which needs this level of introspection right now. runtime also is 'allowed' to know - # about the kvs, dbmodel, etc. - - result = {} - result['explicitly_set'] = xblock._field_data.has(xblock, field.name) - try: - result['default_value'] = xblock._field_data.default(xblock, field.name) - except KeyError: - result['default_value'] = field.to_json(field.default) - return result - - def handler_url(self, block, handler_name, suffix='', query='', thirdparty=False): - # Currently, Modulestore is responsible for instantiating DescriptorSystems - # This means that LMS/CMS don't have a way to define a subclass of DescriptorSystem - # that implements the correct handler url. So, for now, instead, we will reference a - # global function that the application can override. - return descriptor_global_handler_url(block, handler_name, suffix, query, thirdparty) - - def local_resource_url(self, block, uri): - """ - See :meth:`xblock.runtime.Runtime:local_resource_url` for documentation. - """ - # Currently, Modulestore is responsible for instantiating DescriptorSystems - # This means that LMS/CMS don't have a way to define a subclass of DescriptorSystem - # that implements the correct local_resource_url. So, for now, instead, we will reference a - # global function that the application can override. - return descriptor_global_local_resource_url(block, uri) - - def applicable_aside_types(self, block): - """ - See :meth:`xblock.runtime.Runtime:applicable_aside_types` for documentation. - """ - potential_set = set(super().applicable_aside_types(block)) - if getattr(block, 'xmodule_runtime', None) is not None: - if hasattr(block.xmodule_runtime, 'applicable_aside_types'): - application_set = set(block.xmodule_runtime.applicable_aside_types(block)) - return list(potential_set.intersection(application_set)) - return list(potential_set) - - def resource_url(self, resource): - """ - See :meth:`xblock.runtime.Runtime:resource_url` for documentation. - """ - raise NotImplementedError("edX Platform doesn't currently implement XBlock resource urls") - - def add_block_as_child_node(self, block, node): - child = etree.SubElement(node, block.category) - child.set('url_name', block.url_name) - block.add_xml_to_node(child) - - def publish(self, block, event_type, event): # lint-amnesty, pylint: disable=arguments-differ - # A stub publish method that doesn't emit any events from XModuleDescriptors. - pass - - def service(self, block, service_name): - """ - Runtime-specific override for the XBlock service manager. If a service is not currently - instantiated and is declared as a critical requirement, an attempt is made to load the - module. - - Arguments: - block (an XBlock): this block's class will be examined for service - decorators. - service_name (string): the name of the service requested. - - Returns: - An object implementing the requested service, or None. - """ - # getting the service from parent module. making sure of block service declarations. - service = super().service(block=block, service_name=service_name) - # Passing the block to service if it is callable e.g. XBlockI18nService. It is the responsibility of calling - # service to handle the passing argument. - if callable(service): - return service(block) - return service - - -class XMLParsingSystem(DescriptorSystem): # lint-amnesty, pylint: disable=abstract-method, missing-class-docstring - def __init__(self, process_xml, **kwargs): - """ - process_xml: Takes an xml string, and returns a XModuleDescriptor - created from that xml - """ - - super().__init__(**kwargs) - self.process_xml = process_xml - - def _usage_id_from_node(self, node, parent_id, id_generator=None): - """Create a new usage id from an XML dom node. - - Args: - node (lxml.etree.Element): The DOM node to interpret. - parent_id: The usage ID of the parent block - id_generator (IdGenerator): The :class:`.IdGenerator` to use - for creating ids - Returns: - UsageKey: the usage key for the new xblock - """ - return self.xblock_from_node(node, parent_id, id_generator).scope_ids.usage_id - - def xblock_from_node(self, node, parent_id, id_generator=None): - """ - Create an XBlock instance from XML data. - - Args: - xml_data (string): A string containing valid xml. - system (XMLParsingSystem): The :class:`.XMLParsingSystem` used to connect the block - to the outside world. - id_generator (IdGenerator): An :class:`~xblock.runtime.IdGenerator` that - will be used to construct the usage_id and definition_id for the block. - - Returns: - XBlock: The fully instantiated :class:`~xblock.core.XBlock`. - - """ - id_generator = id_generator or self.id_generator - # leave next line commented out - useful for low-level debugging - # log.debug('[_usage_id_from_node] tag=%s, class=%s' % (node.tag, xblock_class)) - - block_type = node.tag - # remove xblock-family from elements - node.attrib.pop('xblock-family', None) - - url_name = node.get('url_name') # difference from XBlock.runtime - def_id = id_generator.create_definition(block_type, url_name) - usage_id = id_generator.create_usage(def_id) - - keys = ScopeIds(None, block_type, def_id, usage_id) - block_class = self.mixologist.mix(self.load_block_type(block_type)) - - aside_children = self.parse_asides(node, def_id, usage_id, id_generator) - asides_tags = [x.tag for x in aside_children] - - block = block_class.parse_xml(node, self, keys, id_generator) - self._convert_reference_fields_to_keys(block) # difference from XBlock.runtime - block.parent = parent_id - block.save() - - asides = self.get_asides(block) - for asd in asides: - if asd.scope_ids.block_type in asides_tags: - block.add_aside(asd) - - return block - - def parse_asides(self, node, def_id, usage_id, id_generator): - """pull the asides out of the xml payload and instantiate them""" - aside_children = [] - for child in node.iterchildren(): - # get xblock-family from node - xblock_family = child.attrib.pop('xblock-family', None) - if xblock_family: - xblock_family = self._family_id_to_superclass(xblock_family) - if issubclass(xblock_family, XBlockAside): - aside_children.append(child) - # now process them & remove them from the xml payload - for child in aside_children: - self._aside_from_xml(child, def_id, usage_id, id_generator) - node.remove(child) - return aside_children - - def _make_usage_key(self, course_key, value): - """ - Makes value into a UsageKey inside the specified course. - If value is already a UsageKey, returns that. - """ - if isinstance(value, UsageKey): - return value - usage_key = UsageKey.from_string(value) - return usage_key.map_into_course(course_key) - - def _convert_reference_fields_to_keys(self, xblock): - """ - Find all fields of type reference and convert the payload into UsageKeys - """ - course_key = xblock.scope_ids.usage_id.course_key - - for field in xblock.fields.values(): - if field.is_set_on(xblock): - field_value = getattr(xblock, field.name) - if field_value is None: - continue - elif isinstance(field, Reference): - setattr(xblock, field.name, self._make_usage_key(course_key, field_value)) - elif isinstance(field, ReferenceList): - setattr(xblock, field.name, [self._make_usage_key(course_key, ele) for ele in field_value]) - elif isinstance(field, ReferenceValueDict): - for key, subvalue in field_value.items(): - assert isinstance(subvalue, str) - field_value[key] = self._make_usage_key(course_key, subvalue) - setattr(xblock, field.name, field_value) - - class ModuleSystemShim: """ This shim provides the properties formerly available from ModuleSystem which are now being provided by services. @@ -1645,6 +1385,266 @@ class ModuleSystemShim: return self.descriptor_runtime.course_id.for_branch(None) +class DescriptorSystem(MetricsMixin, ConfigurableFragmentWrapper, Runtime): + """ + Base class for :class:`Runtime`s to be used with :class:`XModuleDescriptor`s + """ + def __init__( + self, load_item, resources_fs, error_tracker, get_policy=None, disabled_xblock_types=lambda: [], **kwargs + ): + """ + load_item: Takes a Location and returns an XModuleDescriptor + + resources_fs: A Filesystem object that contains all of the + resources needed for the course + + error_tracker: A hook for tracking errors in loading the descriptor. + Used for example to get a list of all non-fatal problems on course + load, and display them to the user. + + See errortracker.py for more documentation + + get_policy: a function that takes a usage id and returns a dict of + policy to apply. + + local_resource_url: an implementation of :meth:`xblock.runtime.Runtime.local_resource_url` + + """ + kwargs.setdefault('id_reader', OpaqueKeyReader()) + kwargs.setdefault('id_generator', AsideKeyGenerator()) + super().__init__(**kwargs) + + # This is used by XModules to write out separate files during xml export + self.export_fs = None + + self.load_item = load_item + self.resources_fs = resources_fs + self.error_tracker = error_tracker + if get_policy: + self.get_policy = get_policy + else: + self.get_policy = lambda u: {} + + self.disabled_xblock_types = disabled_xblock_types + + def get_block(self, usage_id, for_parent=None): + """See documentation for `xblock.runtime:Runtime.get_block`""" + return self.load_item(usage_id, for_parent=for_parent) + + def load_block_type(self, block_type): + """ + Returns a subclass of :class:`.XBlock` that corresponds to the specified `block_type`. + """ + if block_type in self.disabled_xblock_types(): + return self.default_class + return super().load_block_type(block_type) + + def get_field_provenance(self, xblock, field): + """ + For the given xblock, return a dict for the field's current state: + { + 'default_value': what json'd value will take effect if field is unset: either the field default or + inherited value, + 'explicitly_set': boolean for whether the current value is set v default/inherited, + } + :param xblock: + :param field: + """ + # pylint: disable=protected-access + # in runtime b/c runtime contains app-specific xblock behavior. Studio's the only app + # which needs this level of introspection right now. runtime also is 'allowed' to know + # about the kvs, dbmodel, etc. + + result = {} + result['explicitly_set'] = xblock._field_data.has(xblock, field.name) + try: + result['default_value'] = xblock._field_data.default(xblock, field.name) + except KeyError: + result['default_value'] = field.to_json(field.default) + return result + + def handler_url(self, block, handler_name, suffix='', query='', thirdparty=False): + # Currently, Modulestore is responsible for instantiating DescriptorSystems + # This means that LMS/CMS don't have a way to define a subclass of DescriptorSystem + # that implements the correct handler url. So, for now, instead, we will reference a + # global function that the application can override. + return descriptor_global_handler_url(block, handler_name, suffix, query, thirdparty) + + def local_resource_url(self, block, uri): + """ + See :meth:`xblock.runtime.Runtime:local_resource_url` for documentation. + """ + # Currently, Modulestore is responsible for instantiating DescriptorSystems + # This means that LMS/CMS don't have a way to define a subclass of DescriptorSystem + # that implements the correct local_resource_url. So, for now, instead, we will reference a + # global function that the application can override. + return descriptor_global_local_resource_url(block, uri) + + def applicable_aside_types(self, block): + """ + See :meth:`xblock.runtime.Runtime:applicable_aside_types` for documentation. + """ + potential_set = set(super().applicable_aside_types(block)) + if getattr(block, 'xmodule_runtime', None) is not None: + if hasattr(block.xmodule_runtime, 'applicable_aside_types'): + application_set = set(block.xmodule_runtime.applicable_aside_types(block)) + return list(potential_set.intersection(application_set)) + return list(potential_set) + + def resource_url(self, resource): + """ + See :meth:`xblock.runtime.Runtime:resource_url` for documentation. + """ + raise NotImplementedError("edX Platform doesn't currently implement XBlock resource urls") + + def add_block_as_child_node(self, block, node): + child = etree.SubElement(node, block.category) + child.set('url_name', block.url_name) + block.add_xml_to_node(child) + + def publish(self, block, event_type, event): # lint-amnesty, pylint: disable=arguments-differ + # A stub publish method that doesn't emit any events from XModuleDescriptors. + pass + + def service(self, block, service_name): + """ + Runtime-specific override for the XBlock service manager. If a service is not currently + instantiated and is declared as a critical requirement, an attempt is made to load the + module. + + Arguments: + block (an XBlock): this block's class will be examined for service + decorators. + service_name (string): the name of the service requested. + + Returns: + An object implementing the requested service, or None. + """ + # getting the service from parent module. making sure of block service declarations. + service = super().service(block=block, service_name=service_name) + # Passing the block to service if it is callable e.g. XBlockI18nService. It is the responsibility of calling + # service to handle the passing argument. + if callable(service): + return service(block) + return service + + +class XMLParsingSystem(DescriptorSystem): # lint-amnesty, pylint: disable=abstract-method, missing-class-docstring + def __init__(self, process_xml, **kwargs): + """ + process_xml: Takes an xml string, and returns a XModuleDescriptor + created from that xml + """ + + super().__init__(**kwargs) + self.process_xml = process_xml + + def _usage_id_from_node(self, node, parent_id, id_generator=None): + """Create a new usage id from an XML dom node. + + Args: + node (lxml.etree.Element): The DOM node to interpret. + parent_id: The usage ID of the parent block + id_generator (IdGenerator): The :class:`.IdGenerator` to use + for creating ids + Returns: + UsageKey: the usage key for the new xblock + """ + return self.xblock_from_node(node, parent_id, id_generator).scope_ids.usage_id + + def xblock_from_node(self, node, parent_id, id_generator=None): + """ + Create an XBlock instance from XML data. + + Args: + xml_data (string): A string containing valid xml. + system (XMLParsingSystem): The :class:`.XMLParsingSystem` used to connect the block + to the outside world. + id_generator (IdGenerator): An :class:`~xblock.runtime.IdGenerator` that + will be used to construct the usage_id and definition_id for the block. + + Returns: + XBlock: The fully instantiated :class:`~xblock.core.XBlock`. + + """ + id_generator = id_generator or self.id_generator + # leave next line commented out - useful for low-level debugging + # log.debug('[_usage_id_from_node] tag=%s, class=%s' % (node.tag, xblock_class)) + + block_type = node.tag + # remove xblock-family from elements + node.attrib.pop('xblock-family', None) + + url_name = node.get('url_name') # difference from XBlock.runtime + def_id = id_generator.create_definition(block_type, url_name) + usage_id = id_generator.create_usage(def_id) + + keys = ScopeIds(None, block_type, def_id, usage_id) + block_class = self.mixologist.mix(self.load_block_type(block_type)) + + aside_children = self.parse_asides(node, def_id, usage_id, id_generator) + asides_tags = [x.tag for x in aside_children] + + block = block_class.parse_xml(node, self, keys, id_generator) + self._convert_reference_fields_to_keys(block) # difference from XBlock.runtime + block.parent = parent_id + block.save() + + asides = self.get_asides(block) + for asd in asides: + if asd.scope_ids.block_type in asides_tags: + block.add_aside(asd) + + return block + + def parse_asides(self, node, def_id, usage_id, id_generator): + """pull the asides out of the xml payload and instantiate them""" + aside_children = [] + for child in node.iterchildren(): + # get xblock-family from node + xblock_family = child.attrib.pop('xblock-family', None) + if xblock_family: + xblock_family = self._family_id_to_superclass(xblock_family) + if issubclass(xblock_family, XBlockAside): + aside_children.append(child) + # now process them & remove them from the xml payload + for child in aside_children: + self._aside_from_xml(child, def_id, usage_id, id_generator) + node.remove(child) + return aside_children + + def _make_usage_key(self, course_key, value): + """ + Makes value into a UsageKey inside the specified course. + If value is already a UsageKey, returns that. + """ + if isinstance(value, UsageKey): + return value + usage_key = UsageKey.from_string(value) + return usage_key.map_into_course(course_key) + + def _convert_reference_fields_to_keys(self, xblock): + """ + Find all fields of type reference and convert the payload into UsageKeys + """ + course_key = xblock.scope_ids.usage_id.course_key + + for field in xblock.fields.values(): + if field.is_set_on(xblock): + field_value = getattr(xblock, field.name) + if field_value is None: + continue + elif isinstance(field, Reference): + setattr(xblock, field.name, self._make_usage_key(course_key, field_value)) + elif isinstance(field, ReferenceList): + setattr(xblock, field.name, [self._make_usage_key(course_key, ele) for ele in field_value]) + elif isinstance(field, ReferenceValueDict): + for key, subvalue in field_value.items(): + assert isinstance(subvalue, str) + field_value[key] = self._make_usage_key(course_key, subvalue) + setattr(xblock, field.name, field_value) + + class ModuleSystem(MetricsMixin, ConfigurableFragmentWrapper, ModuleSystemShim, Runtime): """ This is an abstraction such that x_modules can function independent @@ -1857,4 +1857,4 @@ class DoNothingCache: return None def set(self, key, value, timeout=None): - pass + pass \ No newline at end of file From 017f8469de4504178361547a3f409f3842c7007f Mon Sep 17 00:00:00 2001 From: Kaustav Banerjee Date: Thu, 5 Jan 2023 11:19:17 +0530 Subject: [PATCH 02/13] feat: merge ModuleSystem with DescriptorSystem --- cms/djangoapps/contentstore/views/preview.py | 7 +- lms/djangoapps/courseware/block_render.py | 3 + lms/djangoapps/lms_xblock/runtime.py | 4 +- xmodule/x_module.py | 169 +++++++------------ 4 files changed, 73 insertions(+), 110 deletions(-) diff --git a/cms/djangoapps/contentstore/views/preview.py b/cms/djangoapps/contentstore/views/preview.py index c65e00a0f3..de217dbc24 100644 --- a/cms/djangoapps/contentstore/views/preview.py +++ b/cms/djangoapps/contentstore/views/preview.py @@ -24,7 +24,7 @@ from xmodule.services import SettingsService, TeamsConfigurationService from xmodule.studio_editable import has_author_view from xmodule.util.sandboxing import SandboxService from xmodule.util.xmodule_django import add_webpack_to_fragment -from xmodule.x_module import AUTHOR_VIEW, PREVIEW_VIEWS, STUDENT_VIEW, ModuleSystem +from xmodule.x_module import AUTHOR_VIEW, PREVIEW_VIEWS, STUDENT_VIEW, DescriptorSystem from cms.djangoapps.xblock_config.models import StudioConfig from cms.djangoapps.contentstore.toggles import individualize_anonymous_user_id, ENABLE_COPY_PASTE_FEATURE from cms.lib.xblock.field_data import CmsFieldData @@ -94,7 +94,7 @@ def preview_handler(request, usage_key_string, handler, suffix=''): return webob_to_django_response(resp) -class PreviewModuleSystem(ModuleSystem): # pylint: disable=abstract-method +class PreviewModuleSystem(DescriptorSystem): # pylint: disable=abstract-method """ An XModule ModuleSystem for use in Studio previews """ @@ -207,6 +207,9 @@ def _preview_module_system(request, descriptor, field_data): preview_anonymous_user_id = anonymous_id_for_user(request.user, course_id) return PreviewModuleSystem( + load_item=descriptor._runtime.load_item, + resources_fs=descriptor._runtime.resources_fs, + error_tracker=descriptor._runtime.error_tracker, get_block=partial(_load_preview_block, request), mixins=settings.XBLOCK_MIXINS, diff --git a/lms/djangoapps/courseware/block_render.py b/lms/djangoapps/courseware/block_render.py index 9d80d8365a..87ea56c517 100644 --- a/lms/djangoapps/courseware/block_render.py +++ b/lms/djangoapps/courseware/block_render.py @@ -588,6 +588,9 @@ def get_module_system_for_user( store = modulestore() system = LmsModuleSystem( + load_item=descriptor._runtime.load_item, + resources_fs=descriptor._runtime.resources_fs, + error_tracker=descriptor._runtime.error_tracker, get_block=inner_get_block, # TODO: When we merge the descriptor and module systems, we can stop reaching into the mixologist (cpennington) mixins=descriptor.runtime.mixologist._mixins, # pylint: disable=protected-access diff --git a/lms/djangoapps/lms_xblock/runtime.py b/lms/djangoapps/lms_xblock/runtime.py index f79f75da08..ac7f2aa568 100644 --- a/lms/djangoapps/lms_xblock/runtime.py +++ b/lms/djangoapps/lms_xblock/runtime.py @@ -9,7 +9,7 @@ from lms.djangoapps.lms_xblock.models import XBlockAsidesConfig from openedx.core.djangoapps.user_api.course_tag import api as user_course_tag_api from openedx.core.lib.url_utils import quote_slashes from openedx.core.lib.xblock_utils import wrap_xblock_aside, xblock_local_resource_url -from xmodule.x_module import ModuleSystem # lint-amnesty, pylint: disable=wrong-import-order +from xmodule.x_module import DescriptorSystem # lint-amnesty, pylint: disable=wrong-import-order def handler_url(block, handler_name, suffix='', query='', thirdparty=False): @@ -116,7 +116,7 @@ class UserTagsService: ) -class LmsModuleSystem(ModuleSystem): # pylint: disable=abstract-method +class LmsModuleSystem(DescriptorSystem): # pylint: disable=abstract-method """ ModuleSystem specialized to the LMS """ diff --git a/xmodule/x_module.py b/xmodule/x_module.py index 5f4d54d7e0..d99a549f3e 100644 --- a/xmodule/x_module.py +++ b/xmodule/x_module.py @@ -1177,11 +1177,27 @@ class ModuleSystemShim: 'Use MakoService.render_template or a JavaScript-based template instead.', DeprecationWarning, stacklevel=2, ) + if hasattr(self, '_deprecated_render_template'): + return self._deprecated_render_template render_service = self._services.get('mako') if render_service: return render_service.render_template return None + @render_template.setter + def render_template(self, render_template): + """ + Set render_template. + + Deprecated in favor of the mako service. + """ + warnings.warn( + 'Use of runtime.render_template is deprecated. ' + 'Use MakoService.render_template or a JavaScript-based template instead.', + DeprecationWarning, stacklevel=2, + ) + self._deprecated_render_template = render_template + @property def xqueue(self): """ @@ -1382,15 +1398,39 @@ class ModuleSystemShim: "`runtime.course_id` is deprecated. Use `context_key` instead: `runtime.scope_ids.usage_id.context_key`.", DeprecationWarning, stacklevel=3, ) + if hasattr(self, '_deprecated_course_id'): + return self._deprecated_course_id return self.descriptor_runtime.course_id.for_branch(None) + @course_id.setter + def course_id(self, course_id): + """ + Set course_id. -class DescriptorSystem(MetricsMixin, ConfigurableFragmentWrapper, Runtime): + Deprecated in favor of `runtime.scope_ids.usage_id.context_key`. + """ + warnings.warn( + "`runtime.course_id` is deprecated. Use `context_key` instead: `runtime.scope_ids.usage_id.context_key`.", + DeprecationWarning, stacklevel=3, + ) + self._deprecated_course_id = course_id + + +class DescriptorSystem(MetricsMixin, ConfigurableFragmentWrapper, ModuleSystemShim, Runtime): """ Base class for :class:`Runtime`s to be used with :class:`XModuleDescriptor`s """ + + def get(self, attr): + """ provide uniform access to attributes (like etree).""" + return self.__dict__.get(attr) + + def set(self, attr, val): + """provide uniform access to attributes (like etree)""" + self.__dict__[attr] = val + def __init__( - self, load_item, resources_fs, error_tracker, get_policy=None, disabled_xblock_types=lambda: [], **kwargs + self, load_item, resources_fs, error_tracker, descriptor_runtime=None, get_policy=None, disabled_xblock_types=lambda: [], get_module=None, **kwargs ): """ load_item: Takes a Location and returns an XModuleDescriptor @@ -1426,9 +1466,23 @@ class DescriptorSystem(MetricsMixin, ConfigurableFragmentWrapper, Runtime): self.get_policy = lambda u: {} self.disabled_xblock_types = disabled_xblock_types + self.get_module = get_module + self.descriptor_runtime = descriptor_runtime + + def get(self, attr): + """ provide uniform access to attributes (like etree).""" + return self.__dict__.get(attr) + + def set(self, attr, val): + """provide uniform access to attributes (like etree)""" + self.__dict__[attr] = val def get_block(self, usage_id, for_parent=None): """See documentation for `xblock.runtime:Runtime.get_block`""" + if self.get_module: + # return self.get_module(block) + return self.get_module(self.descriptor_runtime.get_block(usage_id, for_parent=for_parent)) + return self.load_item(usage_id, for_parent=for_parent) def load_block_type(self, block_type): @@ -1503,8 +1557,12 @@ class DescriptorSystem(MetricsMixin, ConfigurableFragmentWrapper, Runtime): block.add_xml_to_node(child) def publish(self, block, event_type, event): # lint-amnesty, pylint: disable=arguments-differ - # A stub publish method that doesn't emit any events from XModuleDescriptors. - pass + """ + Publish events through the `EventPublishingService`. + This ensures that the correct track method is used for Instructor tasks. + """ + if publish_service := self._services.get('publish'): + publish_service.publish(block, event_type, event) def service(self, block, service_name): """ @@ -1644,107 +1702,6 @@ class XMLParsingSystem(DescriptorSystem): # lint-amnesty, pylint: disable=abstr field_value[key] = self._make_usage_key(course_key, subvalue) setattr(xblock, field.name, field_value) - -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, - or if we want to have a sandbox server for user-contributed content) - - ModuleSystem objects are passed to x_modules to provide access to system - functionality. - - Note that these functions can be closures over e.g. a django request - and user, or other environment-specific info. - """ - - def __init__( - self, - get_block, - descriptor_runtime, - **kwargs, - ): - """ - Create a closure around the system environment. - - get_block - function that takes a descriptor and returns a corresponding - block 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 - """ - - kwargs.setdefault('id_reader', getattr(descriptor_runtime, 'id_reader', OpaqueKeyReader())) - kwargs.setdefault('id_generator', getattr(descriptor_runtime, 'id_generator', AsideKeyGenerator())) - super().__init__(**kwargs) - - self.get_block_for_descriptor = get_block - - self.xmodule_instance = None - - self.descriptor_runtime = descriptor_runtime - - def get(self, attr): - """ provide uniform access to attributes (like etree).""" - return self.__dict__.get(attr) - - def set(self, attr, val): - """provide uniform access to attributes (like etree)""" - self.__dict__[attr] = val - - def __repr__(self): - kwargs = self.__dict__.copy() - - # Remove value set transiently by XBlock - kwargs.pop('_view_name') - - return f"{self.__class__.__name__}{kwargs}" - - @property - def ajax_url(self): - """ - The url prefix to be used by XModules to call into handle_ajax - """ - 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_block_for_descriptor(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") - - def publish(self, block, event_type, event): # lint-amnesty, pylint: disable=arguments-differ - """ - Publish events through the `EventPublishingService`. - This ensures that the correct track method is used for Instructor tasks. - """ - if publish_service := self._services.get('publish'): - publish_service.publish(block, event_type, event) - - def service(self, block, service_name): - """ - Runtime-specific override for the XBlock service manager. If a service is not currently - instantiated and is declared as a critical requirement, an attempt is made to load the - module. - - Arguments: - block (an XBlock): this block's class will be examined for service - decorators. - service_name (string): the name of the service requested. - - Returns: - An object implementing the requested service, or None. - """ - # getting the service from parent module. making sure of block service declarations. - service = super().service(block=block, service_name=service_name) - # Passing the block to service if it is callable e.g. XBlockI18nService. It is the responsibility of calling - # service to handle the passing argument. - if callable(service): - return service(block) - return service - - class CombinedSystem: """ This class is a shim to allow both pure XBlocks and XModuleDescriptors @@ -1857,4 +1814,4 @@ class DoNothingCache: return None def set(self, key, value, timeout=None): - pass \ No newline at end of file + pass From 18fea868a9914854fd614d147ad9b0bbbe7616c5 Mon Sep 17 00:00:00 2001 From: Kaustav Banerjee Date: Sun, 8 Jan 2023 22:56:04 +0530 Subject: [PATCH 03/13] feat: remove usage of LmsModuleSystem and PreviewModuleSystem --- cms/djangoapps/contentstore/views/preview.py | 261 ++++++++++++++----- lms/djangoapps/courseware/block_render.py | 146 +++++++---- lms/djangoapps/lms_xblock/runtime.py | 217 ++++++++++----- xmodule/x_module.py | 39 +-- 4 files changed, 462 insertions(+), 201 deletions(-) diff --git a/cms/djangoapps/contentstore/views/preview.py b/cms/djangoapps/contentstore/views/preview.py index de217dbc24..937bda7a89 100644 --- a/cms/djangoapps/contentstore/views/preview.py +++ b/cms/djangoapps/contentstore/views/preview.py @@ -41,8 +41,7 @@ from openedx.core.lib.xblock_utils import ( request_token, wrap_fragment, wrap_xblock, - wrap_xblock_aside, - xblock_local_resource_url + wrap_xblock_aside ) from ..utils import get_visibility_partition_info @@ -94,61 +93,144 @@ def preview_handler(request, usage_key_string, handler, suffix=''): return webob_to_django_response(resp) +def handler_url(block, handler_name, suffix='', query='', thirdparty=False): + """ + Handler URL function for Preview + """ + return reverse('preview_handler', kwargs={ + 'usage_key_string': str(block.scope_ids.usage_id), + 'handler': handler_name, + 'suffix': suffix, + }) + '?' + query + + +def preview_applicable_aside_types(block, applicable_aside_types=None): + """ + Remove acid_aside and honor the config record + """ + if not StudioConfig.asides_enabled(block.scope_ids.block_type): + return [] + + # TODO: aside_type != 'acid_aside' check should be removed once AcidBlock is only installed during tests + # (see https://openedx.atlassian.net/browse/TE-811) + return [ + aside_type + for aside_type in applicable_aside_types(block) + if aside_type != 'acid_aside' + ] + + +def render_child_placeholder(block, view_name, context, wrap_xblock=None): + """ + Renders a placeholder XBlock. + """ + return wrap_xblock(block, view_name, Fragment(), context) + + +def preview_layout_asides(block, context, frag, view_name, aside_frag_fns, wrap_aside=None): + position_for_asides = '' + result = Fragment() + result.add_fragment_resources(frag) + + for aside, aside_fn in aside_frag_fns: + aside_frag = aside_fn(block, context) + if aside_frag.content != '': + aside_frag_wrapped = wrap_aside(block, aside, view_name, aside_frag, context) + aside.save() + result.add_fragment_resources(aside_frag_wrapped) + replacement = position_for_asides + aside_frag_wrapped.content + frag.content = frag.content.replace(position_for_asides, replacement) + + result.add_content(frag.content) + return result + + class PreviewModuleSystem(DescriptorSystem): # pylint: disable=abstract-method """ An XModule ModuleSystem for use in Studio previews """ # xblocks can check for this attribute during rendering to determine if # they are being rendered for preview (i.e. in Studio) - is_author_mode = True + ####################### + ####################### + ## Set directly to system below + ####################### + ####################### + # is_author_mode = True - def handler_url(self, block, handler_name, suffix='', query='', thirdparty=False): - return reverse('preview_handler', kwargs={ - 'usage_key_string': str(block.scope_ids.usage_id), - 'handler': handler_name, - 'suffix': suffix, - }) + '?' + query + ####################### + ####################### + ## Implemented as handler_url above + ####################### + ####################### + # def handler_url(self, block, handler_name, suffix='', query='', thirdparty=False): + # return reverse('preview_handler', kwargs={ + # 'usage_key_string': str(block.scope_ids.usage_id), + # 'handler': handler_name, + # 'suffix': suffix, + # }) + '?' + query - def local_resource_url(self, block, uri): - return xblock_local_resource_url(block, uri) + ####################### + ####################### + ## Being monkey patched in Descriptor system from cms.djangoapps.xblock_config.apps.py + ####################### + ####################### + # def local_resource_url(self, block, uri): + # return xblock_local_resource_url(block, uri) - def applicable_aside_types(self, block): - """ - Remove acid_aside and honor the config record - """ - if not StudioConfig.asides_enabled(block.scope_ids.block_type): - return [] + ####################### + ####################### + ## Implemented as preview_applicable_aside_types above + ####################### + ####################### + # def applicable_aside_types(self, block): + # """ + # Remove acid_aside and honor the config record + # """ + # if not StudioConfig.asides_enabled(block.scope_ids.block_type): + # return [] - # TODO: aside_type != 'acid_aside' check should be removed once AcidBlock is only installed during tests - # (see https://openedx.atlassian.net/browse/TE-811) - return [ - aside_type - for aside_type in super().applicable_aside_types(block) - if aside_type != 'acid_aside' - ] + # # TODO: aside_type != 'acid_aside' check should be removed once AcidBlock is only installed during tests + # # (see https://openedx.atlassian.net/browse/TE-811) + # return [ + # aside_type + # for aside_type in super().applicable_aside_types(block) + # if aside_type != 'acid_aside' + # ] - def render_child_placeholder(self, block, view_name, context): - """ - Renders a placeholder XBlock. - """ - return self.wrap_xblock(block, view_name, Fragment(), context) + ####################### + ####################### + ## Implemented as render_child_placeholder above + ####################### + ####################### + # def render_child_placeholder(self, block, view_name, context): + # """ + # Renders a placeholder XBlock. + # """ + # return self.wrap_xblock(block, view_name, Fragment(), context) - def layout_asides(self, block, context, frag, view_name, aside_frag_fns): - position_for_asides = '' - result = Fragment() - result.add_fragment_resources(frag) + + ####################### + ####################### + ## Implemented as preview_layout_asides above + ####################### + ####################### + # def layout_asides(self, block, context, frag, view_name, aside_frag_fns): + # position_for_asides = '' + # result = Fragment() + # result.add_fragment_resources(frag) - for aside, aside_fn in aside_frag_fns: - aside_frag = aside_fn(block, context) - if aside_frag.content != '': - aside_frag_wrapped = self.wrap_aside(block, aside, view_name, aside_frag, context) - aside.save() - result.add_fragment_resources(aside_frag_wrapped) - replacement = position_for_asides + aside_frag_wrapped.content - frag.content = frag.content.replace(position_for_asides, replacement) + # for aside, aside_fn in aside_frag_fns: + # aside_frag = aside_fn(block, context) + # if aside_frag.content != '': + # aside_frag_wrapped = self.wrap_aside(block, aside, view_name, aside_frag, context) + # aside.save() + # result.add_fragment_resources(aside_frag_wrapped) + # replacement = position_for_asides + aside_frag_wrapped.content + # frag.content = frag.content.replace(position_for_asides, replacement) - result.add_content(frag.content) - return result + # result.add_content(frag.content) + # return result def _preview_module_system(request, descriptor, field_data): @@ -206,35 +288,74 @@ def _preview_module_system(request, descriptor, field_data): else: preview_anonymous_user_id = anonymous_id_for_user(request.user, course_id) - return PreviewModuleSystem( - load_item=descriptor._runtime.load_item, - resources_fs=descriptor._runtime.resources_fs, - error_tracker=descriptor._runtime.error_tracker, - get_block=partial(_load_preview_block, request), - mixins=settings.XBLOCK_MIXINS, + services={ + "field-data": field_data, + "i18n": XBlockI18nService, + 'mako': mako_service, + "settings": SettingsService(), + "user": DjangoXBlockUserService( + request.user, + user_role=get_user_role(request.user, course_id), + anonymous_user_id=preview_anonymous_user_id, + ), + "partitions": StudioPartitionService(course_id=course_id), + "teams_configuration": TeamsConfigurationService(), + "sandbox": SandboxService(contentstore=contentstore, course_id=course_id), + "cache": CacheService(cache), + 'replace_urls': replace_url_service + } - # Set up functions to modify the fragment produced by student_view - wrappers=wrappers, - wrappers_asides=wrappers_asides, - # Get the raw DescriptorSystem, not the CombinedSystem - descriptor_runtime=descriptor._runtime, # pylint: disable=protected-access - services={ - "field-data": field_data, - "i18n": XBlockI18nService, - 'mako': mako_service, - "settings": SettingsService(), - "user": DjangoXBlockUserService( - request.user, - user_role=get_user_role(request.user, course_id), - anonymous_user_id=preview_anonymous_user_id, - ), - "partitions": StudioPartitionService(course_id=course_id), - "teams_configuration": TeamsConfigurationService(), - "sandbox": SandboxService(contentstore=contentstore, course_id=course_id), - "cache": CacheService(cache), - 'replace_urls': replace_url_service - }, + # system = PreviewModuleSystem( + # load_item=descriptor._runtime.load_item, + # resources_fs=descriptor._runtime.resources_fs, + # error_tracker=descriptor._runtime.error_tracker, + # # get_module=partial(_load_preview_module, request), + # # mixins=settings.XBLOCK_MIXINS, + + # # Set up functions to modify the fragment produced by student_view + # # wrappers=wrappers, + # # wrappers_asides=wrappers_asides, + # # Get the raw DescriptorSystem, not the CombinedSystem + # # descriptor_runtime=descriptor._runtime, # pylint: disable=protected-access + # # services={ + # # "field-data": field_data, + # # "i18n": ModuleI18nService, + # # 'mako': mako_service, + # # "settings": SettingsService(), + # # "user": DjangoXBlockUserService( + # # request.user, + # # user_role=get_user_role(request.user, course_id), + # # anonymous_user_id=preview_anonymous_user_id, + # # ), + # # "partitions": StudioPartitionService(course_id=course_id), + # # "teams_configuration": TeamsConfigurationService(), + # # "sandbox": SandboxService(contentstore=contentstore, course_id=course_id), + # # "cache": CacheService(cache), + # # 'replace_urls': replace_url_service + # # }, + # ) + + descriptor._runtime.get_block = partial(_load_preview_block, request), + descriptor._runtime.mixins = settings.XBLOCK_MIXINS + + # Set up functions to modify the fragment produced by student_view + descriptor._runtime.wrappers = wrappers + descriptor._runtime.wrappers_asides=wrappers_asides + descriptor._runtime._services.update(services) + + descriptor._runtime.is_author_mode = True + descriptor._runtime.handler_url_override = handler_url + descriptor._runtime.applicable_aside_types_override = preview_applicable_aside_types + descriptor._runtime.render_child_placeholder = partial( + render_child_placeholder, + wrap_xblock = descriptor._runtime.wrap_xblock ) + descriptor._runtime.layout_asides_override = partial( + preview_layout_asides, + wrap_aside = descriptor._runtime.wrap_aside + ) + + return descriptor._runtime class StudioPartitionService(PartitionService): diff --git a/lms/djangoapps/courseware/block_render.py b/lms/djangoapps/courseware/block_render.py index 87ea56c517..05b8fec291 100644 --- a/lms/djangoapps/courseware/block_render.py +++ b/lms/djangoapps/courseware/block_render.py @@ -67,7 +67,7 @@ from lms.djangoapps.courseware.field_overrides import OverrideFieldData from lms.djangoapps.courseware.services import UserStateService from lms.djangoapps.grades.api import GradesUtilService from lms.djangoapps.lms_xblock.field_data import LmsFieldData -from lms.djangoapps.lms_xblock.runtime import LmsModuleSystem, UserTagsService +from lms.djangoapps.lms_xblock.runtime import UserTagsService, lms_wrappers_aside, lms_applicable_aside_types from lms.djangoapps.verify_student.services import XBlockVerificationService from openedx.core.djangoapps.bookmarks.api import BookmarksService from openedx.core.djangoapps.crawlers.models import CrawlersConfig @@ -587,50 +587,94 @@ def get_module_system_for_user( store = modulestore() - system = LmsModuleSystem( - load_item=descriptor._runtime.load_item, - resources_fs=descriptor._runtime.resources_fs, - error_tracker=descriptor._runtime.error_tracker, - get_block=inner_get_block, - # TODO: When we merge the descriptor and module systems, we can stop reaching into the mixologist (cpennington) - mixins=descriptor.runtime.mixologist._mixins, # pylint: disable=protected-access - wrappers=block_wrappers, - services={ - 'fs': FSService(), - 'field-data': field_data, - 'mako': mako_service, - 'user': user_service, - 'verification': XBlockVerificationService(), - 'proctoring': ProctoringService(), - 'milestones': milestones_helpers.get_service(), - 'credit': CreditService(), - 'bookmarks': BookmarksService(user=user), - 'gating': GatingService(), - 'grade_utils': GradesUtilService(course_id=course_id), - 'user_state': UserStateService(), - 'content_type_gating': ContentTypeGatingService(), - 'cache': CacheService(cache), - 'sandbox': SandboxService(contentstore=contentstore, course_id=course_id), - 'xqueue': xqueue_service, - 'replace_urls': replace_url_service, - 'rebind_user': rebind_user_service, - 'completion': CompletionService(user=user, context_key=course_id) - if user and user.is_authenticated - else None, - 'i18n': XBlockI18nService, - 'library_tools': LibraryToolsService(store, user_id=user.id if user else None), - 'partitions': PartitionService(course_id=course_id, cache=DEFAULT_REQUEST_CACHE.data), - 'settings': SettingsService(), - 'user_tags': UserTagsService(user=user, course_id=course_id), - 'badging': BadgingService(course_id=course_id, modulestore=store) if badges_enabled() else None, - 'teams': TeamsService(), - 'teams_configuration': TeamsConfigurationService(), - 'call_to_action': CallToActionService(), - 'publish': EventPublishingService(user, course_id, track_function), - }, - descriptor_runtime=descriptor._runtime, # pylint: disable=protected-access - request_token=request_token, - ) + services={ + 'fs': FSService(), + 'field-data': field_data, + 'mako': mako_service, + 'user': user_service, + 'verification': XBlockVerificationService(), + 'proctoring': ProctoringService(), + 'milestones': milestones_helpers.get_service(), + 'credit': CreditService(), + 'bookmarks': BookmarksService(user=user), + 'gating': GatingService(), + 'grade_utils': GradesUtilService(course_id=course_id), + 'user_state': UserStateService(), + 'content_type_gating': ContentTypeGatingService(), + 'cache': CacheService(cache), + 'sandbox': SandboxService(contentstore=contentstore, course_id=course_id), + 'xqueue': xqueue_service, + 'replace_urls': replace_url_service, + 'rebind_user': rebind_user_service, + 'completion': CompletionService(user=user, context_key=course_id) + if user and user.is_authenticated + else None, + 'i18n': XBlockI18nService, + 'library_tools': LibraryToolsService(store, user_id=user.id if user else None), + 'partitions': PartitionService(course_id=course_id, cache=DEFAULT_REQUEST_CACHE.data), + 'settings': SettingsService(), + 'user_tags': UserTagsService(user=user, course_id=course_id), + 'badging': BadgingService(course_id=course_id, modulestore=store) if badges_enabled() else None, + 'teams': TeamsService(), + 'teams_configuration': TeamsConfigurationService(), + 'call_to_action': CallToActionService(), + 'publish': EventPublishingService(user, course_id, track_function), + } + + + # system = LmsModuleSystem( + # load_item=descriptor._runtime.load_item, + # resources_fs=descriptor._runtime.resources_fs, + # error_tracker=descriptor._runtime.error_tracker, + # # get_module=inner_get_module, + # # TODO: When we merge the descriptor and module systems, we can stop reaching into the mixologist (cpennington) + # # mixins=descriptor.runtime.mixologist._mixins, # pylint: disable=protected-access + # # wrappers=block_wrappers, + # # services={ + # # 'fs': FSService(), + # # 'field-data': field_data, + # # 'mako': mako_service, + # # 'user': user_service, + # # 'verification': XBlockVerificationService(), + # # 'proctoring': ProctoringService(), + # # 'milestones': milestones_helpers.get_service(), + # # 'credit': CreditService(), + # # 'bookmarks': BookmarksService(user=user), + # # 'gating': GatingService(), + # # 'grade_utils': GradesUtilService(course_id=course_id), + # # 'user_state': UserStateService(), + # # 'content_type_gating': ContentTypeGatingService(), + # # 'cache': CacheService(cache), + # # 'sandbox': SandboxService(contentstore=contentstore, course_id=course_id), + # # 'xqueue': xqueue_service, + # # 'replace_urls': replace_url_service, + # # 'rebind_user': rebind_user_service, + # # 'completion': CompletionService(user=user, context_key=course_id) + # # if user and user.is_authenticated + # # else None, + # # 'i18n': ModuleI18nService, + # # 'library_tools': LibraryToolsService(store, user_id=user.id if user else None), + # # 'partitions': PartitionService(course_id=course_id, cache=DEFAULT_REQUEST_CACHE.data), + # # 'settings': SettingsService(), + # # 'user_tags': UserTagsService(user=user, course_id=course_id), + # # 'badging': BadgingService(course_id=course_id, modulestore=store) if badges_enabled() else None, + # # 'teams': TeamsService(), + # # 'teams_configuration': TeamsConfigurationService(), + # # 'call_to_action': CallToActionService(), + # # 'publish': EventPublishingService(user, course_id, track_function), + # # }, + # # descriptor_runtime=descriptor._runtime, # pylint: disable=protected-access + # # request_token=request_token, + # ) + + descriptor._runtime.get_block = inner_get_block + descriptor._runtime.mixins = descriptor.runtime.mixologist._mixins + descriptor._runtime.wrappers = block_wrappers + descriptor._runtime._services.update(services) + descriptor._runtime.request_token = request_token + + descriptor._runtime.wrap_asides_override = lms_wrappers_aside + descriptor._runtime.applicable_aside_types_override = lms_applicable_aside_types # pass position specified in URL to module through ModuleSystem if position is not None: @@ -640,14 +684,14 @@ def get_module_system_for_user( log.exception('Non-integer %r passed as position.', position) position = None - system.set('position', position) + descriptor._runtime.set('position', position) - system.set('user_is_staff', user_is_staff) - system.set('user_is_admin', bool(has_access(user, 'staff', 'global'))) - system.set('user_is_beta_tester', CourseBetaTesterRole(course_id).has_user(user)) - system.set('days_early_for_beta', descriptor.days_early_for_beta) + descriptor._runtime.set('user_is_staff', user_is_staff) + descriptor._runtime.set('user_is_admin', bool(has_access(user, 'staff', 'global'))) + descriptor._runtime.set('user_is_beta_tester', CourseBetaTesterRole(course_id).has_user(user)) + descriptor._runtime.set('days_early_for_beta', descriptor.days_early_for_beta) - return system, field_data + return descriptor._runtime, field_data # TODO: Find all the places that this method is called and figure out how to diff --git a/lms/djangoapps/lms_xblock/runtime.py b/lms/djangoapps/lms_xblock/runtime.py index ac7f2aa568..9b5aacd9c2 100644 --- a/lms/djangoapps/lms_xblock/runtime.py +++ b/lms/djangoapps/lms_xblock/runtime.py @@ -72,6 +72,64 @@ def local_resource_url(block, uri): return xblock_local_resource_url(block, uri) +def lms_wrappers_aside(block, aside, view, frag, context, request_token=None): + """ + Creates a div which identifies the aside, points to the original block, + and writes out the json_init_args into a script tag. + + The default implementation creates a frag to wraps frag w/ a div identifying the xblock. If you have + javascript, you'll need to override this impl + """ + if not frag.content: + return frag + + runtime_class = 'LmsRuntime' + extra_data = { + 'block-id': quote_slashes(str(block.scope_ids.usage_id)), + 'course-id': quote_slashes(str(block.scope_ids.usage_id.context_key)), + 'url-selector': 'asideBaseUrl', + 'runtime-class': runtime_class, + } + if request_token: + extra_data['request-token'] = request_token + + return wrap_xblock_aside( + runtime_class, + aside, + view, + frag, + context, + usage_id_serializer=str, + request_token=request_token, + extra_data=extra_data, + ) + + +def lms_applicable_aside_types(block, applicable_aside_types=None): + """ + Return all of the asides which might be decorating this `block`. + + Arguments: + block (:class:`.XBlock`): The block to render retrieve asides for. + """ + + config = XBlockAsidesConfig.current() + + if not config.enabled: + return [] + + if block.scope_ids.block_type in config.disabled_blocks.split(): + return [] + + # TODO: aside_type != 'acid_aside' check should be removed once AcidBlock is only installed during tests + # (see https://openedx.atlassian.net/browse/TE-811) + return [ + aside_type + for aside_type in applicable_aside_types(block) + if aside_type != 'acid_aside' + ] + + class UserTagsService: """ A runtime class that provides an interface to the user service. It handles filling in @@ -121,85 +179,114 @@ class LmsModuleSystem(DescriptorSystem): # pylint: disable=abstract-method ModuleSystem specialized to the LMS """ def __init__(self, **kwargs): - self.request_token = kwargs.pop('request_token', None) + ####################### + ####################### + ## Set Directly from module_render + ####################### + ####################### + # self.request_token = kwargs.pop('request_token', None) super().__init__(**kwargs) - def handler_url(self, *args, **kwargs): # lint-amnesty, pylint: disable=signature-differs - """ - Implement the XBlock runtime handler_url interface. + ####################### + ####################### + ## Being monkey patched in Descriptor system from lms.djangoapps.lms_xblock.apps.py + ####################### + ####################### - This is mostly just proxying to the module level `handler_url` function - defined higher up in this file. + # def handler_url(self, *args, **kwargs): # lint-amnesty, pylint: disable=signature-differs + # """ + # Implement the XBlock runtime handler_url interface. - We're doing this indirection because the module level `handler_url` - logic is also needed by the `DescriptorSystem`. The particular - `handler_url` that a `DescriptorSystem` needs will be different when - running an LMS process or a CMS/Studio process. That's accomplished by - monkey-patching a global. It's a long story, but please know that you - can't just refactor and fold that logic into here without breaking - things. + # This is mostly just proxying to the module level `handler_url` function + # defined higher up in this file. - https://openedx.atlassian.net/wiki/display/PLAT/Convert+from+Storage-centric+runtimes+to+Application-centric+runtimes + # We're doing this indirection because the module level `handler_url` + # logic is also needed by the `DescriptorSystem`. The particular + # `handler_url` that a `DescriptorSystem` needs will be different when + # running an LMS process or a CMS/Studio process. That's accomplished by + # monkey-patching a global. It's a long story, but please know that you + # can't just refactor and fold that logic into here without breaking + # things. - See :method:`xblock.runtime:Runtime.handler_url` - """ - return handler_url(*args, **kwargs) + # https://openedx.atlassian.net/wiki/display/PLAT/Convert+from+Storage-centric+runtimes+to+Application-centric+runtimes - def local_resource_url(self, *args, **kwargs): - return local_resource_url(*args, **kwargs) + # See :method:`xblock.runtime:Runtime.handler_url` + # """ + # return handler_url(*args, **kwargs) - def wrap_aside(self, block, aside, view, frag, context): - """ - Creates a div which identifies the aside, points to the original block, - and writes out the json_init_args into a script tag. + ####################### + ####################### + ## Being monkey patched in Descriptor system from lms.djangoapps.lms_xblock.apps.py + ####################### + ####################### - The default implementation creates a frag to wraps frag w/ a div identifying the xblock. If you have - javascript, you'll need to override this impl - """ - if not frag.content: - return frag + # def local_resource_url(self, *args, **kwargs): + # return local_resource_url(*args, **kwargs) - runtime_class = 'LmsRuntime' - extra_data = { - 'block-id': quote_slashes(str(block.scope_ids.usage_id)), - 'course-id': quote_slashes(str(block.course_id)), - 'url-selector': 'asideBaseUrl', - 'runtime-class': runtime_class, - } - if self.request_token: - extra_data['request-token'] = self.request_token + ####################### + ####################### + ## Implemented as lms_wrappers_aside above + ####################### + ####################### - return wrap_xblock_aside( - runtime_class, - aside, - view, - frag, - context, - usage_id_serializer=str, - request_token=self.request_token, - extra_data=extra_data, - ) + # def wrap_aside(self, block, aside, view, frag, context): + # """ + # Creates a div which identifies the aside, points to the original block, + # and writes out the json_init_args into a script tag. - def applicable_aside_types(self, block): - """ - Return all of the asides which might be decorating this `block`. + # The default implementation creates a frag to wraps frag w/ a div identifying the xblock. If you have + # javascript, you'll need to override this impl + # """ + # if not frag.content: + # return frag - Arguments: - block (:class:`.XBlock`): The block to render retrieve asides for. - """ + # runtime_class = 'LmsRuntime' + # extra_data = { + # 'block-id': quote_slashes(str(block.scope_ids.usage_id)), + # 'course-id': quote_slashes(str(block.course_id)), + # 'url-selector': 'asideBaseUrl', + # 'runtime-class': runtime_class, + # } + # if self.request_token: + # extra_data['request-token'] = self.request_token - config = XBlockAsidesConfig.current() + # return wrap_xblock_aside( + # runtime_class, + # aside, + # view, + # frag, + # context, + # usage_id_serializer=str, + # request_token=self.request_token, + # extra_data=extra_data, + # ) - if not config.enabled: - return [] + ####################### + ####################### + ## Implemented as lms_applicable_aside_types above + ####################### + ####################### - if block.scope_ids.block_type in config.disabled_blocks.split(): - return [] + # def applicable_aside_types(self, block): + # """ + # Return all of the asides which might be decorating this `block`. - # TODO: aside_type != 'acid_aside' check should be removed once AcidBlock is only installed during tests - # (see https://openedx.atlassian.net/browse/TE-811) - return [ - aside_type - for aside_type in super().applicable_aside_types(block) - if aside_type != 'acid_aside' - ] + # Arguments: + # block (:class:`.XBlock`): The block to render retrieve asides for. + # """ + + # config = XBlockAsidesConfig.current() + + # if not config.enabled: + # return [] + + # if block.scope_ids.block_type in config.disabled_blocks.split(): + # return [] + + # # TODO: aside_type != 'acid_aside' check should be removed once AcidBlock is only installed during tests + # # (see https://openedx.atlassian.net/browse/TE-811) + # return [ + # aside_type + # for aside_type in super().applicable_aside_types(block) + # if aside_type != 'acid_aside' + # ] diff --git a/xmodule/x_module.py b/xmodule/x_module.py index d99a549f3e..04d4d41970 100644 --- a/xmodule/x_module.py +++ b/xmodule/x_module.py @@ -1420,17 +1420,8 @@ class DescriptorSystem(MetricsMixin, ConfigurableFragmentWrapper, ModuleSystemSh """ Base class for :class:`Runtime`s to be used with :class:`XModuleDescriptor`s """ - - def get(self, attr): - """ provide uniform access to attributes (like etree).""" - return self.__dict__.get(attr) - - def set(self, attr, val): - """provide uniform access to attributes (like etree)""" - self.__dict__[attr] = val - def __init__( - self, load_item, resources_fs, error_tracker, descriptor_runtime=None, get_policy=None, disabled_xblock_types=lambda: [], get_module=None, **kwargs + self, load_item, resources_fs, error_tracker, get_policy=None, disabled_xblock_types=lambda: [], get_module=None, **kwargs ): """ load_item: Takes a Location and returns an XModuleDescriptor @@ -1467,7 +1458,10 @@ class DescriptorSystem(MetricsMixin, ConfigurableFragmentWrapper, ModuleSystemSh self.disabled_xblock_types = disabled_xblock_types self.get_module = get_module - self.descriptor_runtime = descriptor_runtime + # self.handler_url_override = None + # self.applicable_aside_types_override = None + # self.wrap_asides_override = None + # self.layout_asides_override = None def get(self, attr): """ provide uniform access to attributes (like etree).""" @@ -1479,11 +1473,10 @@ class DescriptorSystem(MetricsMixin, ConfigurableFragmentWrapper, ModuleSystemSh def get_block(self, usage_id, for_parent=None): """See documentation for `xblock.runtime:Runtime.get_block`""" + block = self.load_item(usage_id, for_parent=for_parent) if self.get_module: - # return self.get_module(block) - return self.get_module(self.descriptor_runtime.get_block(usage_id, for_parent=for_parent)) - - return self.load_item(usage_id, for_parent=for_parent) + return self.get_module(block) + return block def load_block_type(self, block_type): """ @@ -1522,6 +1515,8 @@ class DescriptorSystem(MetricsMixin, ConfigurableFragmentWrapper, ModuleSystemSh # This means that LMS/CMS don't have a way to define a subclass of DescriptorSystem # that implements the correct handler url. So, for now, instead, we will reference a # global function that the application can override. + if getattr(self, 'handler_url_override', None): + return self.handler_url_override(block, handler_name, suffix, query, thirdparty) return descriptor_global_handler_url(block, handler_name, suffix, query, thirdparty) def local_resource_url(self, block, uri): @@ -1538,6 +1533,9 @@ class DescriptorSystem(MetricsMixin, ConfigurableFragmentWrapper, ModuleSystemSh """ See :meth:`xblock.runtime.Runtime:applicable_aside_types` for documentation. """ + if getattr(self, 'applicable_aside_types_override', None): + return self.applicable_aside_types_override(block, applicable_aside_types=super().applicable_aside_types) + potential_set = set(super().applicable_aside_types(block)) if getattr(block, 'xmodule_runtime', None) is not None: if hasattr(block.xmodule_runtime, 'applicable_aside_types'): @@ -1586,6 +1584,16 @@ class DescriptorSystem(MetricsMixin, ConfigurableFragmentWrapper, ModuleSystemSh return service(block) return service + def wrap_aside(self, block, aside, view, frag, context): + if getattr(self, 'wrap_asides_override', None): + return self.wrap_asides_override(block, aside, view, frag, context, request_token=self.request_token) + return super().wrap_aside(block, aside, view, frag, context) + + def layout_asides(self, block, context, frag, view_name, aside_frag_fns): + if getattr(self, 'layout_asides_override', None): + return self.layout_asides_override(block, context, frag, view_name, aside_frag_fns) + return super().layout_asides(block, context, frag, view_name, aside_frag_fns) + class XMLParsingSystem(DescriptorSystem): # lint-amnesty, pylint: disable=abstract-method, missing-class-docstring def __init__(self, process_xml, **kwargs): @@ -1702,6 +1710,7 @@ class XMLParsingSystem(DescriptorSystem): # lint-amnesty, pylint: disable=abstr field_value[key] = self._make_usage_key(course_key, subvalue) setattr(xblock, field.name, field_value) + class CombinedSystem: """ This class is a shim to allow both pure XBlocks and XModuleDescriptors From 248c090eee82acbfb84c21c0b47f1c02d9b5a258 Mon Sep 17 00:00:00 2001 From: Kaustav Banerjee Date: Mon, 9 Jan 2023 12:29:33 +0530 Subject: [PATCH 04/13] fix: add namespace prefix to mako_service for redering in studio --- xmodule/studio_editable.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/xmodule/studio_editable.py b/xmodule/studio_editable.py index 69d3126338..2795bf31bc 100644 --- a/xmodule/studio_editable.py +++ b/xmodule/studio_editable.py @@ -33,7 +33,10 @@ class StudioEditableBlock(XBlockMixin): 'content': rendered_child.content }) - fragment.add_content(self.runtime.service(self, 'mako').render_template("studio_render_children_view.html", { # pylint: disable=no-member + # 'lms.' namespace_prefix is required for rendering in studio + mako_service = self.runtime.service(self, 'mako') + mako_service.namespace_prefix = 'lms.' + fragment.add_content(mako_service.render_template("studio_render_children_view.html", { # pylint: disable=no-member 'items': contents, 'xblock_context': context, 'can_add': can_add, From 09e1197053121f809c1efab1254f7e26db27deec Mon Sep 17 00:00:00 2001 From: Kaustav Banerjee Date: Sat, 4 Feb 2023 12:47:46 +0530 Subject: [PATCH 05/13] feat: remove CombinedSystem --- cms/djangoapps/contentstore/views/preview.py | 157 +++--------------- .../contentstore/views/tests/test_block.py | 3 +- lms/djangoapps/courseware/block_render.py | 79 ++------- xmodule/seq_block.py | 4 +- xmodule/services.py | 6 +- xmodule/x_module.py | 148 +++-------------- 6 files changed, 63 insertions(+), 334 deletions(-) diff --git a/cms/djangoapps/contentstore/views/preview.py b/cms/djangoapps/contentstore/views/preview.py index 937bda7a89..d627f64d60 100644 --- a/cms/djangoapps/contentstore/views/preview.py +++ b/cms/djangoapps/contentstore/views/preview.py @@ -24,7 +24,7 @@ from xmodule.services import SettingsService, TeamsConfigurationService from xmodule.studio_editable import has_author_view from xmodule.util.sandboxing import SandboxService from xmodule.util.xmodule_django import add_webpack_to_fragment -from xmodule.x_module import AUTHOR_VIEW, PREVIEW_VIEWS, STUDENT_VIEW, DescriptorSystem +from xmodule.x_module import AUTHOR_VIEW, PREVIEW_VIEWS, STUDENT_VIEW from cms.djangoapps.xblock_config.models import StudioConfig from cms.djangoapps.contentstore.toggles import individualize_anonymous_user_id, ENABLE_COPY_PASTE_FEATURE from cms.lib.xblock.field_data import CmsFieldData @@ -145,101 +145,13 @@ def preview_layout_asides(block, context, frag, view_name, aside_frag_fns, wrap_ return result -class PreviewModuleSystem(DescriptorSystem): # pylint: disable=abstract-method - """ - An XModule ModuleSystem for use in Studio previews - """ - # xblocks can check for this attribute during rendering to determine if - # they are being rendered for preview (i.e. in Studio) - ####################### - ####################### - ## Set directly to system below - ####################### - ####################### - # is_author_mode = True - - ####################### - ####################### - ## Implemented as handler_url above - ####################### - ####################### - # def handler_url(self, block, handler_name, suffix='', query='', thirdparty=False): - # return reverse('preview_handler', kwargs={ - # 'usage_key_string': str(block.scope_ids.usage_id), - # 'handler': handler_name, - # 'suffix': suffix, - # }) + '?' + query - - ####################### - ####################### - ## Being monkey patched in Descriptor system from cms.djangoapps.xblock_config.apps.py - ####################### - ####################### - # def local_resource_url(self, block, uri): - # return xblock_local_resource_url(block, uri) - - ####################### - ####################### - ## Implemented as preview_applicable_aside_types above - ####################### - ####################### - # def applicable_aside_types(self, block): - # """ - # Remove acid_aside and honor the config record - # """ - # if not StudioConfig.asides_enabled(block.scope_ids.block_type): - # return [] - - # # TODO: aside_type != 'acid_aside' check should be removed once AcidBlock is only installed during tests - # # (see https://openedx.atlassian.net/browse/TE-811) - # return [ - # aside_type - # for aside_type in super().applicable_aside_types(block) - # if aside_type != 'acid_aside' - # ] - - ####################### - ####################### - ## Implemented as render_child_placeholder above - ####################### - ####################### - # def render_child_placeholder(self, block, view_name, context): - # """ - # Renders a placeholder XBlock. - # """ - # return self.wrap_xblock(block, view_name, Fragment(), context) - - - ####################### - ####################### - ## Implemented as preview_layout_asides above - ####################### - ####################### - # def layout_asides(self, block, context, frag, view_name, aside_frag_fns): - # position_for_asides = '' - # result = Fragment() - # result.add_fragment_resources(frag) - - # for aside, aside_fn in aside_frag_fns: - # aside_frag = aside_fn(block, context) - # if aside_frag.content != '': - # aside_frag_wrapped = self.wrap_aside(block, aside, view_name, aside_frag, context) - # aside.save() - # result.add_fragment_resources(aside_frag_wrapped) - # replacement = position_for_asides + aside_frag_wrapped.content - # frag.content = frag.content.replace(position_for_asides, replacement) - - # result.add_content(frag.content) - # return result - - def _preview_module_system(request, descriptor, field_data): """ - Returns a ModuleSystem for the specified descriptor that is specialized for - rendering block previews. + Sets properties in the runtime of the specified descriptor that is + required for rendering block previews. request: The active django request - descriptor: An XModuleDescriptor + field_data: Wrapped field data for previews """ course_id = descriptor.location.course_key @@ -305,58 +217,28 @@ def _preview_module_system(request, descriptor, field_data): 'replace_urls': replace_url_service } - # system = PreviewModuleSystem( - # load_item=descriptor._runtime.load_item, - # resources_fs=descriptor._runtime.resources_fs, - # error_tracker=descriptor._runtime.error_tracker, - # # get_module=partial(_load_preview_module, request), - # # mixins=settings.XBLOCK_MIXINS, - - # # Set up functions to modify the fragment produced by student_view - # # wrappers=wrappers, - # # wrappers_asides=wrappers_asides, - # # Get the raw DescriptorSystem, not the CombinedSystem - # # descriptor_runtime=descriptor._runtime, # pylint: disable=protected-access - # # services={ - # # "field-data": field_data, - # # "i18n": ModuleI18nService, - # # 'mako': mako_service, - # # "settings": SettingsService(), - # # "user": DjangoXBlockUserService( - # # request.user, - # # user_role=get_user_role(request.user, course_id), - # # anonymous_user_id=preview_anonymous_user_id, - # # ), - # # "partitions": StudioPartitionService(course_id=course_id), - # # "teams_configuration": TeamsConfigurationService(), - # # "sandbox": SandboxService(contentstore=contentstore, course_id=course_id), - # # "cache": CacheService(cache), - # # 'replace_urls': replace_url_service - # # }, - # ) - - descriptor._runtime.get_block = partial(_load_preview_block, request), - descriptor._runtime.mixins = settings.XBLOCK_MIXINS + descriptor.runtime.get_block_for_descriptor = partial(_load_preview_block, request), + descriptor.runtime.mixins = settings.XBLOCK_MIXINS # Set up functions to modify the fragment produced by student_view - descriptor._runtime.wrappers = wrappers - descriptor._runtime.wrappers_asides=wrappers_asides - descriptor._runtime._services.update(services) + descriptor.runtime.wrappers = wrappers + descriptor.runtime.wrappers_asides = wrappers_asides + descriptor.runtime._services.update(services) - descriptor._runtime.is_author_mode = True - descriptor._runtime.handler_url_override = handler_url - descriptor._runtime.applicable_aside_types_override = preview_applicable_aside_types - descriptor._runtime.render_child_placeholder = partial( + # xmodules can check for this attribute during rendering to determine if + # they are being rendered for preview (i.e. in Studio) + descriptor.runtime.is_author_mode = True + descriptor.runtime.handler_url_override = handler_url + descriptor.runtime.applicable_aside_types_override = preview_applicable_aside_types + descriptor.runtime.render_child_placeholder = partial( render_child_placeholder, - wrap_xblock = descriptor._runtime.wrap_xblock + wrap_xblock = descriptor.runtime.wrap_xblock ) - descriptor._runtime.layout_asides_override = partial( + descriptor.runtime.layout_asides_override = partial( preview_layout_asides, - wrap_aside = descriptor._runtime.wrap_aside + wrap_aside = descriptor.runtime.wrap_aside ) - return descriptor._runtime - class StudioPartitionService(PartitionService): """ @@ -387,10 +269,9 @@ def _load_preview_block(request, descriptor): # wrap the _field_data upfront to pass to _preview_module_system wrapped_field_data = wrapper(descriptor._field_data) # pylint: disable=protected-access - preview_runtime = _preview_module_system(request, descriptor, wrapped_field_data) + _preview_module_system(request, descriptor, wrapped_field_data) descriptor.bind_for_student( - preview_runtime, request.user.id, [wrapper] ) diff --git a/cms/djangoapps/contentstore/views/tests/test_block.py b/cms/djangoapps/contentstore/views/tests/test_block.py index 7f5cbfe2be..863da2e02f 100644 --- a/cms/djangoapps/contentstore/views/tests/test_block.py +++ b/cms/djangoapps/contentstore/views/tests/test_block.py @@ -2128,8 +2128,7 @@ class TestEditSplitModule(ItemTest): group_id_to_child = split_test.group_id_to_child.copy() self.assertEqual(2, len(group_id_to_child)) - # Test environment and Studio use different module systems - # (CachingDescriptorSystem is used in tests, PreviewModuleSystem in Studio). + # CachingDescriptorSystem is used in tests. # CachingDescriptorSystem doesn't have user service, that's needed for # SplitTestBlock. So, in this line of code we add this service manually. split_test.runtime._services['user'] = DjangoXBlockUserService(self.user) # pylint: disable=protected-access diff --git a/lms/djangoapps/courseware/block_render.py b/lms/djangoapps/courseware/block_render.py index 05b8fec291..b77f4392a3 100644 --- a/lms/djangoapps/courseware/block_render.py +++ b/lms/djangoapps/courseware/block_render.py @@ -115,7 +115,7 @@ class LmsModuleRenderError(Exception): def make_track_function(request): ''' Make a tracking function that logs what happened. - For use in ModuleSystem. + For use in DescriptorSystem. ''' from common.djangoapps.track import views as track_views @@ -621,60 +621,18 @@ def get_module_system_for_user( 'publish': EventPublishingService(user, course_id, track_function), } + descriptor.runtime.get_block_for_descriptor = inner_get_block - # system = LmsModuleSystem( - # load_item=descriptor._runtime.load_item, - # resources_fs=descriptor._runtime.resources_fs, - # error_tracker=descriptor._runtime.error_tracker, - # # get_module=inner_get_module, - # # TODO: When we merge the descriptor and module systems, we can stop reaching into the mixologist (cpennington) - # # mixins=descriptor.runtime.mixologist._mixins, # pylint: disable=protected-access - # # wrappers=block_wrappers, - # # services={ - # # 'fs': FSService(), - # # 'field-data': field_data, - # # 'mako': mako_service, - # # 'user': user_service, - # # 'verification': XBlockVerificationService(), - # # 'proctoring': ProctoringService(), - # # 'milestones': milestones_helpers.get_service(), - # # 'credit': CreditService(), - # # 'bookmarks': BookmarksService(user=user), - # # 'gating': GatingService(), - # # 'grade_utils': GradesUtilService(course_id=course_id), - # # 'user_state': UserStateService(), - # # 'content_type_gating': ContentTypeGatingService(), - # # 'cache': CacheService(cache), - # # 'sandbox': SandboxService(contentstore=contentstore, course_id=course_id), - # # 'xqueue': xqueue_service, - # # 'replace_urls': replace_url_service, - # # 'rebind_user': rebind_user_service, - # # 'completion': CompletionService(user=user, context_key=course_id) - # # if user and user.is_authenticated - # # else None, - # # 'i18n': ModuleI18nService, - # # 'library_tools': LibraryToolsService(store, user_id=user.id if user else None), - # # 'partitions': PartitionService(course_id=course_id, cache=DEFAULT_REQUEST_CACHE.data), - # # 'settings': SettingsService(), - # # 'user_tags': UserTagsService(user=user, course_id=course_id), - # # 'badging': BadgingService(course_id=course_id, modulestore=store) if badges_enabled() else None, - # # 'teams': TeamsService(), - # # 'teams_configuration': TeamsConfigurationService(), - # # 'call_to_action': CallToActionService(), - # # 'publish': EventPublishingService(user, course_id, track_function), - # # }, - # # descriptor_runtime=descriptor._runtime, # pylint: disable=protected-access - # # request_token=request_token, - # ) + # TODO: When we merge the descriptor and module systems, we can stop reaching into the mixologist (cpennington) + # mixins=descriptor.runtime.mixologist._mixins, # pylint: disable=protected-access + descriptor.runtime.mixins = descriptor.runtime.mixologist._mixins + + descriptor.runtime.wrappers = block_wrappers + descriptor.runtime._services.update(services) + descriptor.runtime.request_token = request_token - descriptor._runtime.get_block = inner_get_block - descriptor._runtime.mixins = descriptor.runtime.mixologist._mixins - descriptor._runtime.wrappers = block_wrappers - descriptor._runtime._services.update(services) - descriptor._runtime.request_token = request_token - - descriptor._runtime.wrap_asides_override = lms_wrappers_aside - descriptor._runtime.applicable_aside_types_override = lms_applicable_aside_types + descriptor.runtime.wrap_asides_override = lms_wrappers_aside + descriptor.runtime.applicable_aside_types_override = lms_applicable_aside_types # pass position specified in URL to module through ModuleSystem if position is not None: @@ -684,14 +642,14 @@ def get_module_system_for_user( log.exception('Non-integer %r passed as position.', position) position = None - descriptor._runtime.set('position', position) + descriptor.runtime.set('position', position) - descriptor._runtime.set('user_is_staff', user_is_staff) - descriptor._runtime.set('user_is_admin', bool(has_access(user, 'staff', 'global'))) - descriptor._runtime.set('user_is_beta_tester', CourseBetaTesterRole(course_id).has_user(user)) - descriptor._runtime.set('days_early_for_beta', descriptor.days_early_for_beta) + descriptor.runtime.set('user_is_staff', user_is_staff) + descriptor.runtime.set('user_is_admin', bool(has_access(user, 'staff', 'global'))) + descriptor.runtime.set('user_is_beta_tester', CourseBetaTesterRole(course_id).has_user(user)) + descriptor.runtime.set('days_early_for_beta', descriptor.days_early_for_beta) - return descriptor._runtime, field_data + return field_data # TODO: Find all the places that this method is called and figure out how to @@ -709,7 +667,7 @@ def get_block_for_descriptor_internal(user, descriptor, student_data, course_id, request_token (str): A unique token for this request, used to isolate xblock rendering """ - (system, student_data) = get_module_system_for_user( + student_data = get_module_system_for_user( user=user, student_data=student_data, # These have implicit user bindings, the rest of args are considered not to descriptor=descriptor, @@ -727,7 +685,6 @@ def get_block_for_descriptor_internal(user, descriptor, student_data, course_id, ) descriptor.bind_for_student( - system, user.id, [ partial(DateLookupFieldData, course_id=course_id, user=user), diff --git a/xmodule/seq_block.py b/xmodule/seq_block.py index fc7202bfe0..eab754ba06 100644 --- a/xmodule/seq_block.py +++ b/xmodule/seq_block.py @@ -300,11 +300,11 @@ class SequenceBlock( self.gated_sequence_paywall = None - def bind_for_student(self, xmodule_runtime, user_id, wrappers=None): + def bind_for_student(self, user_id, wrappers=None): # The position of the child XBlock to select can also be passed in via the URL. # In such cases the value is set on the ModuleSystem in get_module_system_for_user() # and needs to be read here after the ModuleSystem has been set on the XBlock. - super().bind_for_student(xmodule_runtime, user_id, wrappers) + super().bind_for_student(user_id, wrappers) # If position is specified in system, then use that instead. position = getattr(self.runtime, 'position', None) if position is not None: diff --git a/xmodule/services.py b/xmodule/services.py index 5418ea4f19..d53d72fc32 100644 --- a/xmodule/services.py +++ b/xmodule/services.py @@ -202,7 +202,7 @@ class RebindUserService(Service): with modulestore().bulk_operations(self.course_id): course = modulestore().get_course(course_key=self.course_id) - (inner_system, inner_student_data) = self._ref["get_module_system_for_user"]( + inner_student_data = self._ref["get_module_system_for_user"]( user=real_user, student_data=student_data_real_user, # These have implicit user bindings, rest of args considered not to descriptor=block, @@ -212,7 +212,6 @@ class RebindUserService(Service): ) block.bind_for_student( - inner_system, real_user.id, [ partial(DateLookupFieldData, course_id=self.course_id, user=self.user), @@ -223,8 +222,7 @@ class RebindUserService(Service): block.scope_ids = block.scope_ids._replace(user_id=real_user.id) # now bind the module to the new ModuleSystem instance and vice-versa - block.runtime = inner_system - inner_system.xmodule_instance = block + block.runtime.xmodule_instance = block class EventPublishingService(Service): diff --git a/xmodule/x_module.py b/xmodule/x_module.py index 04d4d41970..8f04b284a2 100644 --- a/xmodule/x_module.py +++ b/xmodule/x_module.py @@ -313,19 +313,31 @@ class XModuleMixin(XModuleFields, XBlock): icon_class = 'other' def __init__(self, *args, **kwargs): - self.xmodule_runtime = None self._asides = [] super().__init__(*args, **kwargs) @property def runtime(self): - return CombinedSystem(self.xmodule_runtime, self._runtime) + return self._runtime @runtime.setter def runtime(self, value): self._runtime = value + @property + def xmodule_runtime(self): + """ + Shim to maintain backward compatibility. + + Deprecated in favor of the runtime property. + """ + warnings.warn( + 'xmodule_runtime property is deprecated. Please use the runtime property instead.', + DeprecationWarning, stacklevel=3, + ) + return self._runtime + @property def system(self): """ @@ -593,12 +605,11 @@ class XModuleMixin(XModuleFields, XBlock): """ return None - def bind_for_student(self, xmodule_runtime, user_id, wrappers=None): + def bind_for_student(self, user_id, wrappers=None): """ Set up this XBlock to act as an XModule instead of an XModuleDescriptor. Arguments: - xmodule_runtime (:class:`ModuleSystem'): the runtime to use when accessing student facing methods user_id: The user_id to set in scope_ids wrappers: These are a list functions that put a wrapper, such as LmsFieldData or OverrideFieldData, around the field_data. @@ -608,8 +619,8 @@ class XModuleMixin(XModuleFields, XBlock): # Skip rebinding if we're already bound a user, and it's this user. if self.scope_ids.user_id is not None and user_id == self.scope_ids.user_id: - if getattr(xmodule_runtime, 'position', None): - self.position = xmodule_runtime.position # update the position of the tab + if getattr(self._runtime, 'position', None): + self.position = self._runtime.position # update the position of the tab return # If we are switching users mid-request, save the data from the old user. @@ -634,9 +645,6 @@ class XModuleMixin(XModuleFields, XBlock): if field in self._dirty_fields: del self._dirty_fields[field] - # Set the new xmodule_runtime and field_data (which are user-specific) - self.xmodule_runtime = xmodule_runtime - if wrappers is None: wrappers = [] @@ -1013,6 +1021,7 @@ class MetricsMixin: """ def render(self, block, view_name, context=None): # lint-amnesty, pylint: disable=missing-function-docstring + context = context or {} start_time = time.time() try: return super().render(block, view_name, context=context) @@ -1421,7 +1430,7 @@ class DescriptorSystem(MetricsMixin, ConfigurableFragmentWrapper, ModuleSystemSh Base class for :class:`Runtime`s to be used with :class:`XModuleDescriptor`s """ def __init__( - self, load_item, resources_fs, error_tracker, get_policy=None, disabled_xblock_types=lambda: [], get_module=None, **kwargs + self, load_item, resources_fs, error_tracker, get_policy=None, disabled_xblock_types=lambda: [], **kwargs ): """ load_item: Takes a Location and returns an XModuleDescriptor @@ -1457,11 +1466,6 @@ class DescriptorSystem(MetricsMixin, ConfigurableFragmentWrapper, ModuleSystemSh self.get_policy = lambda u: {} self.disabled_xblock_types = disabled_xblock_types - self.get_module = get_module - # self.handler_url_override = None - # self.applicable_aside_types_override = None - # self.wrap_asides_override = None - # self.layout_asides_override = None def get(self, attr): """ provide uniform access to attributes (like etree).""" @@ -1474,8 +1478,8 @@ class DescriptorSystem(MetricsMixin, ConfigurableFragmentWrapper, ModuleSystemSh def get_block(self, usage_id, for_parent=None): """See documentation for `xblock.runtime:Runtime.get_block`""" block = self.load_item(usage_id, for_parent=for_parent) - if self.get_module: - return self.get_module(block) + if self.get_block_for_descriptor: + return self.get_block_for_descriptor(block) return block def load_block_type(self, block_type): @@ -1537,10 +1541,6 @@ class DescriptorSystem(MetricsMixin, ConfigurableFragmentWrapper, ModuleSystemSh return self.applicable_aside_types_override(block, applicable_aside_types=super().applicable_aside_types) potential_set = set(super().applicable_aside_types(block)) - if getattr(block, 'xmodule_runtime', None) is not None: - if hasattr(block.xmodule_runtime, 'applicable_aside_types'): - application_set = set(block.xmodule_runtime.applicable_aside_types(block)) - return list(potential_set.intersection(application_set)) return list(potential_set) def resource_url(self, resource): @@ -1711,112 +1711,6 @@ class XMLParsingSystem(DescriptorSystem): # lint-amnesty, pylint: disable=abstr setattr(xblock, field.name, field_value) -class CombinedSystem: - """ - This class is a shim to allow both pure XBlocks and XModuleDescriptors - that have been bound as XModules to access both the attributes of ModuleSystem - and of DescriptorSystem as a single runtime. - """ - - __slots__ = ('_module_system', '_descriptor_system') - - # This system doesn't override a number of methods that are provided by ModuleSystem and DescriptorSystem, - # namely handler_url, local_resource_url, query, and resource_url. - # - # At runtime, the ModuleSystem and/or DescriptorSystem will define those methods - # - def __init__(self, module_system, descriptor_system): - # These attributes are set directly to __dict__ below to avoid a recursion in getattr/setattr. - self._module_system = module_system - self._descriptor_system = descriptor_system - - def render(self, block, view_name, context=None): - """ - Render a block by invoking its view. - - Finds the view named `view_name` on `block`. The default view will be - used if a specific view hasn't be registered. If there is no default - view, an exception will be raised. - - The view is invoked, passing it `context`. The value returned by the - view is returned, with possible modifications by the runtime to - integrate it into a larger whole. - - """ - context = context or {} - return self.__getattr__('render')(block, view_name, context) # pylint: disable=unnecessary-dunder-call - - def service(self, block, service_name): - """Return a service, or None. - - Services are objects implementing arbitrary other interfaces. They are - requested by agreed-upon names, see [XXX TODO] for a list of possible - services. The object returned depends on the service requested. - - XBlocks must announce their intention to request services with the - `XBlock.needs` or `XBlock.wants` decorators. Use `needs` if you assume - that the service is available, or `wants` if your code is flexible and - can accept a None from this method. - - Runtimes can override this method if they have different techniques for - finding and delivering services. - - Arguments: - block (an XBlock): this block's class will be examined for service - decorators. - service_name (string): the name of the service requested. - - Returns: - An object implementing the requested service, or None. - - """ - service = None - - if self._module_system: - service = self._module_system.service(block, service_name) - - if service is None: - service = self._descriptor_system.service(block, service_name) - - return service - - def __getattr__(self, name): - """ - If the ModuleSystem doesn't have an attribute, try returning the same attribute from the - DescriptorSystem, instead. This allows XModuleDescriptors that are bound as XModules - to still function as XModuleDescriptors. - """ - # First we try a lookup in the module system... - try: - return getattr(self._module_system, name) - except AttributeError: - return getattr(self._descriptor_system, name) - - def __setattr__(self, name, value): - """ - If the ModuleSystem is set, set the attr on it. - Always set the attr on the DescriptorSystem. - """ - if name in self.__slots__: - return super().__setattr__(name, value) - - if self._module_system: - setattr(self._module_system, name, value) - setattr(self._descriptor_system, name, value) - - def __delattr__(self, name): - """ - If the ModuleSystem is set, delete the attribute from it. - Always delete the attribute from the DescriptorSystem. - """ - if self._module_system: - delattr(self._module_system, name) - delattr(self._descriptor_system, name) - - def __repr__(self): - return f"CombinedSystem({self._module_system!r}, {self._descriptor_system!r})" - - class DoNothingCache: """A duck-compatible object to use in ModuleSystem when there's no cache.""" def get(self, _key): From dc1078fa1e1f052ce24e8dece924bbae096d3fdd Mon Sep 17 00:00:00 2001 From: Kaustav Banerjee Date: Sat, 4 Feb 2023 17:57:59 +0530 Subject: [PATCH 06/13] feat: removed StudioEditModuleRuntime --- cms/djangoapps/contentstore/views/block.py | 47 +++++++------------ .../contentstore/views/component.py | 4 +- 2 files changed, 18 insertions(+), 33 deletions(-) diff --git a/cms/djangoapps/contentstore/views/block.py b/cms/djangoapps/contentstore/views/block.py index c65aaace82..2167e8bdc3 100644 --- a/cms/djangoapps/contentstore/views/block.py +++ b/cms/djangoapps/contentstore/views/block.py @@ -293,38 +293,23 @@ class StudioPermissionsService: return has_studio_write_access(self._user, course_key) -class StudioEditModuleRuntime: +def load_services_for_studio(runtime, user): """ - An extremely minimal ModuleSystem shim used for XBlock edits and studio_view. + Function to set some required services used for XBlock edits and studio_view. (i.e. whenever we're not using PreviewModuleSystem.) This is required to make information about the current user (especially permissions) available via services as needed. """ + services={ + "user": DjangoXBlockUserService(user), + "studio_user_permissions": StudioPermissionsService(user), + "mako": MakoService(), + "settings": SettingsService(), + "lti-configuration": ConfigurationService(CourseAllowPIISharingInLTIFlag), + "teams_configuration": TeamsConfigurationService(), + "library_tools": LibraryToolsService(modulestore(), user.id) + } - def __init__(self, user): - self._user = user - - def service(self, block, service_name): - """ - This block is not bound to a user but some blocks (LibraryContentBlock) may need - user-specific services to check for permissions, etc. - If we return None here, CombinedSystem will load services from the descriptor runtime. - """ - if block.service_declaration(service_name) is not None: - if service_name == "user": - return DjangoXBlockUserService(self._user) - if service_name == "studio_user_permissions": - return StudioPermissionsService(self._user) - if service_name == "mako": - return MakoService() - if service_name == "settings": - return SettingsService() - if service_name == "lti-configuration": - return ConfigurationService(CourseAllowPIISharingInLTIFlag) - if service_name == "teams_configuration": - return TeamsConfigurationService() - if service_name == "library_tools": - return LibraryToolsService(modulestore(), self._user.id) - return None + runtime._services.update(services) @require_http_methods("GET") @@ -368,8 +353,8 @@ def xblock_view_handler(request, usage_key_string, view_name): )) if view_name in (STUDIO_VIEW, VISIBILITY_VIEW): - if view_name == STUDIO_VIEW and xblock.xmodule_runtime is None: - xblock.xmodule_runtime = StudioEditModuleRuntime(request.user) + if view_name == STUDIO_VIEW: + load_services_for_studio(xblock.runtime, request.user) try: fragment = xblock.render(view_name) @@ -524,7 +509,7 @@ def _update_with_callback(xblock, user, old_metadata=None, old_content=None): old_metadata = own_metadata(xblock) if old_content is None: old_content = xblock.get_explicitly_set_fields_by_scope(Scope.content) - xblock.xmodule_runtime = StudioEditModuleRuntime(user) + load_services_for_studio(xblock.runtime, user) xblock.editor_saved(user, old_metadata, old_content) # Update after the callback so any changes made in the callback will get persisted. @@ -937,7 +922,7 @@ def _duplicate_block(parent_usage_key, duplicate_source_usage_key, user, display # Allow an XBlock to do anything fancy it may need to when duplicated from another block. # These blocks may handle their own children or parenting if needed. Let them return booleans to # let us know if we need to handle these or not. - dest_block.xmodule_runtime = StudioEditModuleRuntime(user) + load_services_for_studio(dest_block.runtime, user) children_handled = dest_block.studio_post_duplicate(store, source_item) # Children are not automatically copied over (and not all xblocks have a 'children' attribute). diff --git a/cms/djangoapps/contentstore/views/component.py b/cms/djangoapps/contentstore/views/component.py index 457f84793e..7f774ab161 100644 --- a/cms/djangoapps/contentstore/views/component.py +++ b/cms/djangoapps/contentstore/views/component.py @@ -32,7 +32,7 @@ from xmodule.modulestore.exceptions import ItemNotFoundError # lint-amnesty, py from ..utils import get_lms_link_for_item, get_sibling_urls, reverse_course_url from .helpers import get_parent_xblock, is_unit, xblock_type_display_name -from .block import StudioEditModuleRuntime, add_container_page_publishing_info, create_xblock_info +from .block import add_container_page_publishing_info, create_xblock_info, load_services_for_studio __all__ = [ 'container_handler', @@ -560,7 +560,7 @@ def component_handler(request, usage_key_string, handler, suffix=''): descriptor = modulestore().get_item(usage_key) handler_descriptor = descriptor asides = [] - handler_descriptor.xmodule_runtime = StudioEditModuleRuntime(request.user) + load_services_for_studio(handler_descriptor.runtime, request.user) resp = handler_descriptor.handle(handler, req, suffix) except NoSuchHandlerError: log.info("XBlock %s attempted to access missing handler %r", handler_descriptor, handler, exc_info=True) From 7d8f54fa88b0d2088510500dae8110f7bf583519 Mon Sep 17 00:00:00 2001 From: Kaustav Banerjee Date: Sat, 4 Feb 2023 18:54:06 +0530 Subject: [PATCH 07/13] chore: cleanup LmsModuleSystem and PreviewModuleSystem --- cms/djangoapps/contentstore/views/block.py | 2 +- lms/djangoapps/courseware/block_render.py | 3 +- lms/djangoapps/lms_xblock/runtime.py | 119 --------------------- 3 files changed, 2 insertions(+), 122 deletions(-) diff --git a/cms/djangoapps/contentstore/views/block.py b/cms/djangoapps/contentstore/views/block.py index 2167e8bdc3..1a1c37e5c5 100644 --- a/cms/djangoapps/contentstore/views/block.py +++ b/cms/djangoapps/contentstore/views/block.py @@ -296,7 +296,7 @@ class StudioPermissionsService: def load_services_for_studio(runtime, user): """ Function to set some required services used for XBlock edits and studio_view. - (i.e. whenever we're not using PreviewModuleSystem.) This is required to make information + (i.e. whenever we're not loading preview module system.) This is required to make information about the current user (especially permissions) available via services as needed. """ services={ diff --git a/lms/djangoapps/courseware/block_render.py b/lms/djangoapps/courseware/block_render.py index b77f4392a3..d7afe5bd11 100644 --- a/lms/djangoapps/courseware/block_render.py +++ b/lms/djangoapps/courseware/block_render.py @@ -437,7 +437,7 @@ def get_module_system_for_user( request_token (str): A token unique to the request use by xblock initialization Returns: - (LmsModuleSystem, KvsFieldData): (module system, student_data) bound to, primarily, the user and descriptor + KvsFieldData: student_data bound to, primarily, the user and descriptor """ def make_xqueue_callback(dispatch='score_update'): @@ -624,7 +624,6 @@ def get_module_system_for_user( descriptor.runtime.get_block_for_descriptor = inner_get_block # TODO: When we merge the descriptor and module systems, we can stop reaching into the mixologist (cpennington) - # mixins=descriptor.runtime.mixologist._mixins, # pylint: disable=protected-access descriptor.runtime.mixins = descriptor.runtime.mixologist._mixins descriptor.runtime.wrappers = block_wrappers diff --git a/lms/djangoapps/lms_xblock/runtime.py b/lms/djangoapps/lms_xblock/runtime.py index 9b5aacd9c2..c786271876 100644 --- a/lms/djangoapps/lms_xblock/runtime.py +++ b/lms/djangoapps/lms_xblock/runtime.py @@ -9,7 +9,6 @@ from lms.djangoapps.lms_xblock.models import XBlockAsidesConfig from openedx.core.djangoapps.user_api.course_tag import api as user_course_tag_api from openedx.core.lib.url_utils import quote_slashes from openedx.core.lib.xblock_utils import wrap_xblock_aside, xblock_local_resource_url -from xmodule.x_module import DescriptorSystem # lint-amnesty, pylint: disable=wrong-import-order def handler_url(block, handler_name, suffix='', query='', thirdparty=False): @@ -172,121 +171,3 @@ class UserTagsService: self._user, self._course_id, key, value ) - - -class LmsModuleSystem(DescriptorSystem): # pylint: disable=abstract-method - """ - ModuleSystem specialized to the LMS - """ - def __init__(self, **kwargs): - ####################### - ####################### - ## Set Directly from module_render - ####################### - ####################### - # self.request_token = kwargs.pop('request_token', None) - super().__init__(**kwargs) - - ####################### - ####################### - ## Being monkey patched in Descriptor system from lms.djangoapps.lms_xblock.apps.py - ####################### - ####################### - - # def handler_url(self, *args, **kwargs): # lint-amnesty, pylint: disable=signature-differs - # """ - # Implement the XBlock runtime handler_url interface. - - # This is mostly just proxying to the module level `handler_url` function - # defined higher up in this file. - - # We're doing this indirection because the module level `handler_url` - # logic is also needed by the `DescriptorSystem`. The particular - # `handler_url` that a `DescriptorSystem` needs will be different when - # running an LMS process or a CMS/Studio process. That's accomplished by - # monkey-patching a global. It's a long story, but please know that you - # can't just refactor and fold that logic into here without breaking - # things. - - # https://openedx.atlassian.net/wiki/display/PLAT/Convert+from+Storage-centric+runtimes+to+Application-centric+runtimes - - # See :method:`xblock.runtime:Runtime.handler_url` - # """ - # return handler_url(*args, **kwargs) - - ####################### - ####################### - ## Being monkey patched in Descriptor system from lms.djangoapps.lms_xblock.apps.py - ####################### - ####################### - - # def local_resource_url(self, *args, **kwargs): - # return local_resource_url(*args, **kwargs) - - ####################### - ####################### - ## Implemented as lms_wrappers_aside above - ####################### - ####################### - - # def wrap_aside(self, block, aside, view, frag, context): - # """ - # Creates a div which identifies the aside, points to the original block, - # and writes out the json_init_args into a script tag. - - # The default implementation creates a frag to wraps frag w/ a div identifying the xblock. If you have - # javascript, you'll need to override this impl - # """ - # if not frag.content: - # return frag - - # runtime_class = 'LmsRuntime' - # extra_data = { - # 'block-id': quote_slashes(str(block.scope_ids.usage_id)), - # 'course-id': quote_slashes(str(block.course_id)), - # 'url-selector': 'asideBaseUrl', - # 'runtime-class': runtime_class, - # } - # if self.request_token: - # extra_data['request-token'] = self.request_token - - # return wrap_xblock_aside( - # runtime_class, - # aside, - # view, - # frag, - # context, - # usage_id_serializer=str, - # request_token=self.request_token, - # extra_data=extra_data, - # ) - - ####################### - ####################### - ## Implemented as lms_applicable_aside_types above - ####################### - ####################### - - # def applicable_aside_types(self, block): - # """ - # Return all of the asides which might be decorating this `block`. - - # Arguments: - # block (:class:`.XBlock`): The block to render retrieve asides for. - # """ - - # config = XBlockAsidesConfig.current() - - # if not config.enabled: - # return [] - - # if block.scope_ids.block_type in config.disabled_blocks.split(): - # return [] - - # # TODO: aside_type != 'acid_aside' check should be removed once AcidBlock is only installed during tests - # # (see https://openedx.atlassian.net/browse/TE-811) - # return [ - # aside_type - # for aside_type in super().applicable_aside_types(block) - # if aside_type != 'acid_aside' - # ] From 20ed3d64ecfd494221b6400f423159d344667e8e Mon Sep 17 00:00:00 2001 From: Kaustav Banerjee Date: Mon, 6 Feb 2023 12:25:24 +0530 Subject: [PATCH 08/13] feat: add _runtime_services attribute to DescriptorSystem --- cms/djangoapps/contentstore/views/preview.py | 2 +- lms/djangoapps/courseware/block_render.py | 2 +- xmodule/x_module.py | 48 +++++++++++--------- 3 files changed, 28 insertions(+), 24 deletions(-) diff --git a/cms/djangoapps/contentstore/views/preview.py b/cms/djangoapps/contentstore/views/preview.py index d627f64d60..8fb72e3c61 100644 --- a/cms/djangoapps/contentstore/views/preview.py +++ b/cms/djangoapps/contentstore/views/preview.py @@ -223,7 +223,7 @@ def _preview_module_system(request, descriptor, field_data): # Set up functions to modify the fragment produced by student_view descriptor.runtime.wrappers = wrappers descriptor.runtime.wrappers_asides = wrappers_asides - descriptor.runtime._services.update(services) + descriptor.runtime._runtime_services.update(services) # lint-amnesty, pylint: disable=protected-access # xmodules can check for this attribute during rendering to determine if # they are being rendered for preview (i.e. in Studio) diff --git a/lms/djangoapps/courseware/block_render.py b/lms/djangoapps/courseware/block_render.py index d7afe5bd11..025b15894e 100644 --- a/lms/djangoapps/courseware/block_render.py +++ b/lms/djangoapps/courseware/block_render.py @@ -627,7 +627,7 @@ def get_module_system_for_user( descriptor.runtime.mixins = descriptor.runtime.mixologist._mixins descriptor.runtime.wrappers = block_wrappers - descriptor.runtime._services.update(services) + descriptor.runtime._runtime_services.update(services) # lint-amnesty, pylint: disable=protected-access descriptor.runtime.request_token = request_token descriptor.runtime.wrap_asides_override = lms_wrappers_aside diff --git a/xmodule/x_module.py b/xmodule/x_module.py index 8f04b284a2..c2fd4e0085 100644 --- a/xmodule/x_module.py +++ b/xmodule/x_module.py @@ -1070,7 +1070,7 @@ class ModuleSystemShim: 'runtime.anonymous_student_id is deprecated. Please use the user service instead.', DeprecationWarning, stacklevel=3, ) - user_service = self._services.get('user') + user_service = self._runtime_services.get('user') or self._services.get('user') if user_service: return user_service.get_current_user().opt_attrs.get(ATTR_KEY_ANONYMOUS_USER_ID) return None @@ -1100,7 +1100,7 @@ class ModuleSystemShim: 'runtime.user_id is deprecated. Please use the user service instead.', DeprecationWarning, stacklevel=3, ) - user_service = self._services.get('user') + user_service = self._runtime_services.get('user') or self._services.get('user') if user_service: return user_service.get_current_user().opt_attrs.get(ATTR_KEY_USER_ID) return None @@ -1116,9 +1116,9 @@ class ModuleSystemShim: 'runtime.user_is_staff is deprecated. Please use the user service instead.', DeprecationWarning, stacklevel=3, ) - user_service = self._services.get('user') + user_service = self._runtime_services.get('user') or self._services.get('user') if user_service: - return self._services['user'].get_current_user().opt_attrs.get(ATTR_KEY_USER_IS_STAFF) + return user_service.get_current_user().opt_attrs.get(ATTR_KEY_USER_IS_STAFF) return None @property @@ -1132,9 +1132,9 @@ class ModuleSystemShim: 'runtime.user_location is deprecated. Please use the user service instead.', DeprecationWarning, stacklevel=3, ) - user_service = self._services.get('user') + user_service = self._runtime_services.get('user') or self._services.get('user') if user_service: - return self._services['user'].get_current_user().opt_attrs.get(ATTR_KEY_REQUEST_COUNTRY_CODE) + return user_service.get_current_user().opt_attrs.get(ATTR_KEY_REQUEST_COUNTRY_CODE) return None @property @@ -1152,9 +1152,9 @@ class ModuleSystemShim: 'runtime.get_real_user is deprecated. Please use the user service instead.', DeprecationWarning, stacklevel=3, ) - user_service = self._services.get('user') + user_service = self._runtime_services.get('user') or self._services.get('user') if user_service: - return self._services['user'].get_user_by_anonymous_id + return user_service.get_user_by_anonymous_id return None @property @@ -1170,9 +1170,9 @@ class ModuleSystemShim: 'runtime.get_user_role is deprecated. Please use the user service instead.', DeprecationWarning, stacklevel=3, ) - user_service = self._services.get('user') + user_service = self._runtime_services.get('user') or self._services.get('user') if user_service: - return partial(self._services['user'].get_current_user().opt_attrs.get, ATTR_KEY_USER_ROLE) + return partial(user_service.get_current_user().opt_attrs.get, ATTR_KEY_USER_ROLE) @property def render_template(self): @@ -1188,7 +1188,7 @@ class ModuleSystemShim: ) if hasattr(self, '_deprecated_render_template'): return self._deprecated_render_template - render_service = self._services.get('mako') + render_service = self._runtime_services.get('mako') or self._services.get('mako') if render_service: return render_service.render_template return None @@ -1222,7 +1222,7 @@ class ModuleSystemShim: 'runtime.xqueue is deprecated. Please use the xqueue service instead.', DeprecationWarning, stacklevel=3, ) - xqueue_service = self._services.get('xqueue') + xqueue_service = self._runtime_services.get('xqueue') or self._services.get('xqueue') if xqueue_service: return { 'interface': xqueue_service.interface, @@ -1244,7 +1244,7 @@ class ModuleSystemShim: 'runtime.can_execute_unsafe_code is deprecated. Please use the sandbox service instead.', DeprecationWarning, stacklevel=3, ) - sandbox_service = self._services.get('sandbox') + sandbox_service = self._runtime_services.get('sandbox') or self._services.get('sandbox') if sandbox_service: return sandbox_service.can_execute_unsafe_code # Default to saying "no unsafe code". @@ -1264,7 +1264,7 @@ class ModuleSystemShim: 'runtime.get_python_lib_zip is deprecated. Please use the sandbox service instead.', DeprecationWarning, stacklevel=3, ) - sandbox_service = self._services.get('sandbox') + sandbox_service = self._runtime_services.get('sandbox') or self._services.get('sandbox') if sandbox_service: return sandbox_service.get_python_lib_zip # Default to saying "no lib data" @@ -1283,7 +1283,7 @@ class ModuleSystemShim: 'runtime.cache is deprecated. Please use the cache service instead.', DeprecationWarning, stacklevel=3, ) - return self._services.get('cache') or DoNothingCache() + return self._runtime_services.get('cache') or self._services.get('cache') or DoNothingCache() @property def replace_urls(self): @@ -1296,7 +1296,7 @@ class ModuleSystemShim: 'runtime.replace_urls is deprecated. Please use the replace_urls service instead.', DeprecationWarning, stacklevel=3, ) - replace_urls_service = self._services.get('replace_urls') + replace_urls_service = self._runtime_services.get('replace_urls') or self._services.get('replace_urls') if replace_urls_service: return partial(replace_urls_service.replace_urls, static_replace_only=True) @@ -1311,7 +1311,7 @@ class ModuleSystemShim: 'runtime.replace_course_urls is deprecated. Please use the replace_urls service instead.', DeprecationWarning, stacklevel=3, ) - replace_urls_service = self._services.get('replace_urls') + replace_urls_service = self._runtime_services.get('replace_urls') or self._services.get('replace_urls') if replace_urls_service: return partial(replace_urls_service.replace_urls) @@ -1326,7 +1326,7 @@ class ModuleSystemShim: 'runtime.replace_jump_to_id_urls is deprecated. Please use the replace_urls service instead.', DeprecationWarning, stacklevel=3, ) - replace_urls_service = self._services.get('replace_urls') + replace_urls_service = self._runtime_services.get('replace_urls') or self._services.get('replace_urls') if replace_urls_service: return partial(replace_urls_service.replace_urls) @@ -1379,7 +1379,7 @@ class ModuleSystemShim: "rebind_noauth_module_to_user is deprecated. Please use the 'rebind_user' service instead.", DeprecationWarning, stacklevel=3 ) - rebind_user_service = self._services.get('rebind_user') + rebind_user_service = self._runtime_services.get('rebind_user') or self._services.get('rebind_user') if rebind_user_service: return partial(rebind_user_service.rebind_noauth_module_to_user) @@ -1466,6 +1466,7 @@ class DescriptorSystem(MetricsMixin, ConfigurableFragmentWrapper, ModuleSystemSh self.get_policy = lambda u: {} self.disabled_xblock_types = disabled_xblock_types + self._runtime_services = {} def get(self, attr): """ provide uniform access to attributes (like etree).""" @@ -1559,7 +1560,7 @@ class DescriptorSystem(MetricsMixin, ConfigurableFragmentWrapper, ModuleSystemSh Publish events through the `EventPublishingService`. This ensures that the correct track method is used for Instructor tasks. """ - if publish_service := self._services.get('publish'): + if publish_service := self._runtime_services.get('publish') or self._services.get('publish'): publish_service.publish(block, event_type, event) def service(self, block, service_name): @@ -1576,8 +1577,11 @@ class DescriptorSystem(MetricsMixin, ConfigurableFragmentWrapper, ModuleSystemSh Returns: An object implementing the requested service, or None. """ - # getting the service from parent module. making sure of block service declarations. - service = super().service(block=block, service_name=service_name) + declaration = block.service_declaration(service_name) + service = self._runtime_services.get(service_name) + if declaration is None or service is None: + # getting the service from parent module. making sure of block service declarations. + service = super().service(block=block, service_name=service_name) # Passing the block to service if it is callable e.g. XBlockI18nService. It is the responsibility of calling # service to handle the passing argument. if callable(service): From d0fa2d65e3c1d5ec0cf5667357f9fee08bfea199 Mon Sep 17 00:00:00 2001 From: Kaustav Banerjee Date: Sun, 5 Feb 2023 12:00:29 +0530 Subject: [PATCH 09/13] test: fix test cases and lint issues --- .../contentstore/tests/test_i18n.py | 6 +- cms/djangoapps/contentstore/views/block.py | 4 +- cms/djangoapps/contentstore/views/preview.py | 17 +- .../contentstore/views/tests/test_preview.py | 41 ++-- lms/djangoapps/courseware/block_render.py | 6 +- lms/djangoapps/courseware/tests/helpers.py | 9 +- .../courseware/tests/test_block_render.py | 136 ++++++------- .../tests/test_discussion_xblock.py | 1 - .../courseware/tests/test_lti_integration.py | 8 +- .../courseware/tests/test_video_handlers.py | 7 +- .../courseware/tests/test_video_mongo.py | 32 +-- .../courseware/tests/test_word_cloud.py | 5 + .../tests/test_tasks_helper.py | 5 + .../lms_xblock/test/test_runtime.py | 12 +- .../completion_integration/test_services.py | 17 +- xmodule/tests/__init__.py | 184 +++++++++++++----- xmodule/tests/helpers.py | 17 ++ xmodule/tests/test_conditional.py | 42 ++-- xmodule/tests/test_error_block.py | 2 +- xmodule/tests/test_import.py | 3 - xmodule/tests/test_library_content.py | 25 ++- xmodule/tests/test_library_sourced_block.py | 9 +- xmodule/tests/test_lti20_unit.py | 2 +- xmodule/tests/test_lti_unit.py | 4 +- xmodule/tests/test_randomize_block.py | 8 +- xmodule/tests/test_sequence.py | 41 ++-- xmodule/tests/test_split_test_block.py | 30 ++- xmodule/tests/test_studio_editable.py | 2 +- xmodule/tests/test_vertical.py | 75 +++---- xmodule/x_module.py | 5 +- 30 files changed, 442 insertions(+), 313 deletions(-) diff --git a/cms/djangoapps/contentstore/tests/test_i18n.py b/cms/djangoapps/contentstore/tests/test_i18n.py index ebd5616b9d..980490b7e4 100644 --- a/cms/djangoapps/contentstore/tests/test_i18n.py +++ b/cms/djangoapps/contentstore/tests/test_i18n.py @@ -70,7 +70,7 @@ class TestXBlockI18nService(ModuleStoreTestCase): self.course = CourseFactory.create() self.field_data = mock.Mock() self.descriptor = BlockFactory(category="pure", parent=self.course) - self.runtime = _preview_module_system( + _preview_module_system( self.request, self.descriptor, self.field_data, @@ -81,7 +81,7 @@ class TestXBlockI18nService(ModuleStoreTestCase): """ return the block i18n service. """ - i18n_service = self.runtime.service(descriptor, 'i18n') + i18n_service = self.descriptor.runtime.service(descriptor, 'i18n') self.assertIsNotNone(i18n_service) self.assertIsInstance(i18n_service, XBlockI18nService) return i18n_service @@ -171,7 +171,7 @@ class TestXBlockI18nService(ModuleStoreTestCase): """ Test: i18n service should be callable in studio. """ - self.assertTrue(callable(self.runtime._services.get('i18n'))) # pylint: disable=protected-access + self.assertTrue(callable(self.descriptor.runtime._services.get('i18n'))) # pylint: disable=protected-access class InternationalizationTest(ModuleStoreTestCase): diff --git a/cms/djangoapps/contentstore/views/block.py b/cms/djangoapps/contentstore/views/block.py index 1a1c37e5c5..eb330da26a 100644 --- a/cms/djangoapps/contentstore/views/block.py +++ b/cms/djangoapps/contentstore/views/block.py @@ -299,7 +299,7 @@ def load_services_for_studio(runtime, user): (i.e. whenever we're not loading preview module system.) This is required to make information about the current user (especially permissions) available via services as needed. """ - services={ + services = { "user": DjangoXBlockUserService(user), "studio_user_permissions": StudioPermissionsService(user), "mako": MakoService(), @@ -309,7 +309,7 @@ def load_services_for_studio(runtime, user): "library_tools": LibraryToolsService(modulestore(), user.id) } - runtime._services.update(services) + runtime._services.update(services) # lint-amnesty, pylint: disable=protected-access @require_http_methods("GET") diff --git a/cms/djangoapps/contentstore/views/preview.py b/cms/djangoapps/contentstore/views/preview.py index 8fb72e3c61..29900af507 100644 --- a/cms/djangoapps/contentstore/views/preview.py +++ b/cms/djangoapps/contentstore/views/preview.py @@ -93,7 +93,7 @@ def preview_handler(request, usage_key_string, handler, suffix=''): return webob_to_django_response(resp) -def handler_url(block, handler_name, suffix='', query='', thirdparty=False): +def handler_url(block, handler_name, suffix='', query='', thirdparty=False): # lint-amnesty, pylint: disable=unused-argument """ Handler URL function for Preview """ @@ -120,14 +120,17 @@ def preview_applicable_aside_types(block, applicable_aside_types=None): ] -def render_child_placeholder(block, view_name, context, wrap_xblock=None): +def render_child_placeholder(block, view_name, context, wrap_block=None): """ Renders a placeholder XBlock. """ - return wrap_xblock(block, view_name, Fragment(), context) + return wrap_block(block, view_name, Fragment(), context) def preview_layout_asides(block, context, frag, view_name, aside_frag_fns, wrap_aside=None): + """ + Custom layout of asides for preview + """ position_for_asides = '' result = Fragment() result.add_fragment_resources(frag) @@ -200,7 +203,7 @@ def _preview_module_system(request, descriptor, field_data): else: preview_anonymous_user_id = anonymous_id_for_user(request.user, course_id) - services={ + services = { "field-data": field_data, "i18n": XBlockI18nService, 'mako': mako_service, @@ -217,7 +220,7 @@ def _preview_module_system(request, descriptor, field_data): 'replace_urls': replace_url_service } - descriptor.runtime.get_block_for_descriptor = partial(_load_preview_block, request), + descriptor.runtime.get_block_for_descriptor = partial(_load_preview_block, request) descriptor.runtime.mixins = settings.XBLOCK_MIXINS # Set up functions to modify the fragment produced by student_view @@ -232,11 +235,11 @@ def _preview_module_system(request, descriptor, field_data): descriptor.runtime.applicable_aside_types_override = preview_applicable_aside_types descriptor.runtime.render_child_placeholder = partial( render_child_placeholder, - wrap_xblock = descriptor.runtime.wrap_xblock + wrap_block=descriptor.runtime.wrap_xblock ) descriptor.runtime.layout_asides_override = partial( preview_layout_asides, - wrap_aside = descriptor.runtime.wrap_aside + wrap_aside=descriptor.runtime.wrap_aside ) diff --git a/cms/djangoapps/contentstore/views/tests/test_preview.py b/cms/djangoapps/contentstore/views/tests/test_preview.py index 41cfdbeeb4..944cb4b9bc 100644 --- a/cms/djangoapps/contentstore/views/tests/test_preview.py +++ b/cms/djangoapps/contentstore/views/tests/test_preview.py @@ -214,12 +214,12 @@ class StudioXBlockServiceBindingTest(ModuleStoreTestCase): Tests that the 'user' and 'i18n' services are provided by the Studio runtime. """ descriptor = BlockFactory(category="pure", parent=self.course) - runtime = _preview_module_system( + _preview_module_system( self.request, descriptor, self.field_data, ) - service = runtime.service(descriptor, expected_service) + service = descriptor.runtime.service(descriptor, expected_service) self.assertIsNotNone(service) @@ -245,15 +245,16 @@ class CmsModuleSystemShimTest(ModuleStoreTestCase): self.descriptor = BlockFactory(category="video", parent=course) self.field_data = mock.Mock() self.contentstore = contentstore() - self.runtime = _preview_module_system( + self.descriptor = BlockFactory(category="problem", parent=course) + _preview_module_system( self.request, - descriptor=BlockFactory(category="problem", parent=course), + descriptor=self.descriptor, field_data=mock.Mock(), ) self.course = self.store.get_item(course.location) def test_get_user_role(self): - assert self.runtime.get_user_role() == 'staff' + assert self.descriptor.runtime.get_user_role() == 'staff' @XBlock.register_temp_plugin(PureXBlock, identifier='pure') def test_render_template(self): @@ -263,10 +264,10 @@ class CmsModuleSystemShimTest(ModuleStoreTestCase): @override_settings(COURSES_WITH_UNSAFE_CODE=[r'course-v1:edX\+LmsModuleShimTest\+2021_Fall']) def test_can_execute_unsafe_code(self): - assert self.runtime.can_execute_unsafe_code() + assert self.descriptor.runtime.can_execute_unsafe_code() def test_cannot_execute_unsafe_code(self): - assert not self.runtime.can_execute_unsafe_code() + assert not self.descriptor.runtime.can_execute_unsafe_code() @override_settings(PYTHON_LIB_FILENAME=PYTHON_LIB_FILENAME) def test_get_python_lib_zip(self): @@ -276,7 +277,7 @@ class CmsModuleSystemShimTest(ModuleStoreTestCase): source_file=self.PYTHON_LIB_SOURCE_FILE, target_filename=self.PYTHON_LIB_FILENAME, ) - assert self.runtime.get_python_lib_zip() == zipfile + assert self.descriptor.runtime.get_python_lib_zip() == zipfile def test_no_get_python_lib_zip(self): zipfile = upload_file_to_course( @@ -285,38 +286,40 @@ class CmsModuleSystemShimTest(ModuleStoreTestCase): source_file=self.PYTHON_LIB_SOURCE_FILE, target_filename=self.PYTHON_LIB_FILENAME, ) - assert self.runtime.get_python_lib_zip() is None + assert self.descriptor.runtime.get_python_lib_zip() is None def test_cache(self): - assert hasattr(self.runtime.cache, 'get') - assert hasattr(self.runtime.cache, 'set') + assert hasattr(self.descriptor.runtime.cache, 'get') + assert hasattr(self.descriptor.runtime.cache, 'set') def test_replace_urls(self): html = '' - assert self.runtime.replace_urls(html) == \ + assert self.descriptor.runtime.replace_urls(html) == \ static_replace.replace_static_urls(html, course_id=self.course.id) def test_anonymous_user_id_preview(self): - assert self.runtime.anonymous_student_id == 'student' + assert self.descriptor.runtime.anonymous_student_id == 'student' @override_waffle_flag(INDIVIDUALIZE_ANONYMOUS_USER_ID, active=True) def test_anonymous_user_id_individual_per_student(self): """Test anonymous_user_id on a block which uses per-student anonymous IDs""" # Create the runtime with the flag turned on. - runtime = _preview_module_system( + descriptor = BlockFactory(category="problem", parent=self.course) + _preview_module_system( self.request, - descriptor=BlockFactory(category="problem", parent=self.course), + descriptor=descriptor, field_data=mock.Mock(), ) - assert runtime.anonymous_student_id == '26262401c528d7c4a6bbeabe0455ec46' + assert descriptor.runtime.anonymous_student_id == '26262401c528d7c4a6bbeabe0455ec46' @override_waffle_flag(INDIVIDUALIZE_ANONYMOUS_USER_ID, active=True) def test_anonymous_user_id_individual_per_course(self): """Test anonymous_user_id on a block which uses per-course anonymous IDs""" # Create the runtime with the flag turned on. - runtime = _preview_module_system( + descriptor = BlockFactory(category="lti", parent=self.course) + _preview_module_system( self.request, - descriptor=BlockFactory(category="lti", parent=self.course), + descriptor=descriptor, field_data=mock.Mock(), ) - assert runtime.anonymous_student_id == 'ad503f629b55c531fed2e45aa17a3368' + assert descriptor.runtime.anonymous_student_id == 'ad503f629b55c531fed2e45aa17a3368' diff --git a/lms/djangoapps/courseware/block_render.py b/lms/djangoapps/courseware/block_render.py index 025b15894e..fb28034aaa 100644 --- a/lms/djangoapps/courseware/block_render.py +++ b/lms/djangoapps/courseware/block_render.py @@ -587,7 +587,7 @@ def get_module_system_for_user( store = modulestore() - services={ + services = { 'fs': FSService(), 'field-data': field_data, 'mako': mako_service, @@ -624,12 +624,10 @@ def get_module_system_for_user( descriptor.runtime.get_block_for_descriptor = inner_get_block # TODO: When we merge the descriptor and module systems, we can stop reaching into the mixologist (cpennington) - descriptor.runtime.mixins = descriptor.runtime.mixologist._mixins - + descriptor.runtime.mixins = descriptor.runtime.mixologist._mixins # lint-amnesty, pylint: disable=protected-access descriptor.runtime.wrappers = block_wrappers descriptor.runtime._runtime_services.update(services) # lint-amnesty, pylint: disable=protected-access descriptor.runtime.request_token = request_token - descriptor.runtime.wrap_asides_override = lms_wrappers_aside descriptor.runtime.applicable_aside_types_override = lms_applicable_aside_types diff --git a/lms/djangoapps/courseware/tests/helpers.py b/lms/djangoapps/courseware/tests/helpers.py index 6a38691098..5f8eadf8a3 100644 --- a/lms/djangoapps/courseware/tests/helpers.py +++ b/lms/djangoapps/courseware/tests/helpers.py @@ -33,7 +33,7 @@ from common.djangoapps.util.date_utils import strftime_localized_html from xmodule.modulestore.django import modulestore # lint-amnesty, pylint: disable=wrong-import-order from xmodule.modulestore.tests.django_utils import TEST_DATA_MONGO_MODULESTORE, ModuleStoreTestCase # lint-amnesty, pylint: disable=wrong-import-order from xmodule.modulestore.tests.factories import CourseFactory, BlockFactory # lint-amnesty, pylint: disable=wrong-import-order -from xmodule.tests import get_test_descriptor_system, get_test_system # lint-amnesty, pylint: disable=wrong-import-order +from xmodule.tests import get_test_descriptor_system, get_test_system, prepare_block_runtime # lint-amnesty, pylint: disable=wrong-import-order class BaseTestXmodule(ModuleStoreTestCase): @@ -66,10 +66,12 @@ class BaseTestXmodule(ModuleStoreTestCase): METADATA = {} MODEL_DATA = {'data': ''} - def new_module_runtime(self, **kwargs): + def new_module_runtime(self, runtime=None, **kwargs): """ Generate a new ModuleSystem that is minimally set up for testing """ + if runtime: + return prepare_block_runtime(runtime, course_id=self.course.id, **kwargs) return get_test_system(course_id=self.course.id, **kwargs) def new_descriptor_runtime(self, **kwargs): @@ -94,7 +96,7 @@ class BaseTestXmodule(ModuleStoreTestCase): if runtime_kwargs is None: runtime_kwargs = {} - self.item_descriptor.xmodule_runtime = self.new_module_runtime(**runtime_kwargs) + self.new_module_runtime(runtime=self.item_descriptor.runtime, **runtime_kwargs) self.item_url = str(self.item_descriptor.location) @@ -148,6 +150,7 @@ class BaseTestXmodule(ModuleStoreTestCase): class XModuleRenderingTestBase(BaseTestXmodule): # lint-amnesty, pylint: disable=missing-class-docstring + # lint-amnesty, pylint: disable=arguments-differ def new_module_runtime(self, **kwargs): """ Create a runtime that actually does html rendering diff --git a/lms/djangoapps/courseware/tests/test_block_render.py b/lms/djangoapps/courseware/tests/test_block_render.py index 53e10b5c51..36a674dff1 100644 --- a/lms/djangoapps/courseware/tests/test_block_render.py +++ b/lms/djangoapps/courseware/tests/test_block_render.py @@ -37,7 +37,7 @@ from xblock.core import XBlock, XBlockAside # lint-amnesty, pylint: disable=wro from xblock.exceptions import NoSuchServiceError from xblock.field_data import FieldData # lint-amnesty, pylint: disable=wrong-import-order from xblock.fields import ScopeIds # lint-amnesty, pylint: disable=wrong-import-order -from xblock.runtime import DictKeyValueStore, KvsFieldData, Runtime # lint-amnesty, pylint: disable=wrong-import-order +from xblock.runtime import DictKeyValueStore, KvsFieldData # lint-amnesty, pylint: disable=wrong-import-order from xblock.test.tools import TestRuntime # lint-amnesty, pylint: disable=wrong-import-order from xmodule.capa.tests.response_xml_factory import OptionResponseXMLFactory # lint-amnesty, pylint: disable=reimported @@ -58,7 +58,7 @@ from xmodule.modulestore.tests.factories import CourseFactory, BlockFactory, Toy from xmodule.modulestore.tests.test_asides import AsideTestType # lint-amnesty, pylint: disable=wrong-import-order from xmodule.services import RebindUserServiceError from xmodule.video_block import VideoBlock # lint-amnesty, pylint: disable=wrong-import-order -from xmodule.x_module import STUDENT_VIEW, CombinedSystem # lint-amnesty, pylint: disable=wrong-import-order +from xmodule.x_module import STUDENT_VIEW, DescriptorSystem # lint-amnesty, pylint: disable=wrong-import-order from common.djangoapps import static_replace from common.djangoapps.course_modes.models import CourseMode # lint-amnesty, pylint: disable=reimported from common.djangoapps.student.tests.factories import GlobalStaffFactory @@ -1894,9 +1894,10 @@ class TestAnonymousStudentId(SharedModuleStoreTestCase, LoginEnrollmentTestCase) location=location, static_asset_path=None, _runtime=Mock( - spec=Runtime, + spec=DescriptorSystem, resources_fs=None, mixologist=Mock(_mixins=(), name='mixologist'), + _services={}, name='runtime', ), scope_ids=Mock(spec=ScopeIds), @@ -1906,7 +1907,7 @@ class TestAnonymousStudentId(SharedModuleStoreTestCase, LoginEnrollmentTestCase) fields={}, days_early_for_beta=None, ) - descriptor.runtime = CombinedSystem(descriptor._runtime, None) # pylint: disable=protected-access + descriptor.runtime = DescriptorSystem(None, None, None) # Use the xblock_class's bind_for_student method descriptor.bind_for_student = partial(xblock_class.bind_for_student, descriptor) @@ -1922,7 +1923,7 @@ class TestAnonymousStudentId(SharedModuleStoreTestCase, LoginEnrollmentTestCase) request_token='request_token', course=self.course, ) - current_user = block.xmodule_runtime.service(block, 'user').get_current_user() + current_user = block.runtime.service(block, 'user').get_current_user() return current_user.opt_attrs.get(ATTR_KEY_ANONYMOUS_USER_ID) @ddt.data(*PER_STUDENT_ANONYMIZED_XBLOCKS) @@ -1976,7 +1977,7 @@ class TestModuleTrackingContext(SharedModuleStoreTestCase): @XBlockAside.register_temp_plugin(AsideTestType, 'test_aside') @patch('xmodule.modulestore.mongo.base.CachingDescriptorSystem.applicable_aside_types', lambda self, block: ['test_aside']) - @patch('lms.djangoapps.lms_xblock.runtime.LmsModuleSystem.applicable_aside_types', + @patch('xmodule.x_module.DescriptorSystem.applicable_aside_types', lambda self, block: ['test_aside']) def test_context_contains_aside_info(self, mock_tracker): """ @@ -2185,7 +2186,7 @@ class TestRebindBlock(TestSubmittingProblems): # Bind the block to another student, which will remove "correct_map" # from the block's _field_data_cache and _dirty_fields. user2 = UserFactory.create() - block.bind_for_student(block.runtime, user2.id) + block.bind_for_student(user2.id) # XBlock's save method assumes that if a field is in _dirty_fields, # then it's also in _field_data_cache. If this assumption @@ -2194,7 +2195,7 @@ class TestRebindBlock(TestSubmittingProblems): # _field_data cache, but not _dirty_fields, when we bound # this block to the second student. (TNL-2640) user3 = UserFactory.create() - block.bind_for_student(block.runtime, user3.id) + block.bind_for_student(user3.id) def test_rebind_noauth_block_to_user_not_anonymous(self): """ @@ -2263,13 +2264,13 @@ class TestEventPublishing(ModuleStoreTestCase, LoginEnrollmentTestCase): class LMSXBlockServiceMixin(SharedModuleStoreTestCase): """ - Helper class that initializes the LmsModuleSystem. + Helper class that initializes the runtime. """ def _prepare_runtime(self): """ - Instantiate the LmsModuleSystem. + Instantiate the runtem. """ - self.runtime, _ = render.get_module_system_for_user( + _ = render.get_module_system_for_user( self.user, self.student_data, self.descriptor, @@ -2333,7 +2334,7 @@ class LMSXBlockServiceBindingTest(LMSXBlockServiceMixin): """ Tests that the 'user', 'i18n', and 'fs' services are provided by the LMS runtime. """ - service = self.runtime.service(self.descriptor, expected_service) + service = self.descriptor.runtime.service(self.descriptor, expected_service) assert service is not None def test_beta_tester_fields_added(self): @@ -2344,8 +2345,8 @@ class LMSXBlockServiceBindingTest(LMSXBlockServiceMixin): self._prepare_runtime() # pylint: disable=no-member - assert not self.runtime.user_is_beta_tester - assert self.runtime.days_early_for_beta == 5 + assert not self.descriptor.runtime.user_is_beta_tester + assert self.descriptor.runtime.days_early_for_beta == 5 def test_get_set_tag(self): """ @@ -2355,23 +2356,23 @@ class LMSXBlockServiceBindingTest(LMSXBlockServiceMixin): key = 'key1' # test for when we haven't set the tag yet - tag = self.runtime.service(self.descriptor, 'user_tags').get_tag(scope, key) + tag = self.descriptor.runtime.service(self.descriptor, 'user_tags').get_tag(scope, key) assert tag is None # set the tag set_value = 'value' - self.runtime.service(self.descriptor, 'user_tags').set_tag(scope, key, set_value) - tag = self.runtime.service(self.descriptor, 'user_tags').get_tag(scope, key) + self.descriptor.runtime.service(self.descriptor, 'user_tags').set_tag(scope, key, set_value) + tag = self.descriptor.runtime.service(self.descriptor, 'user_tags').get_tag(scope, key) assert tag == set_value # Try to set tag in wrong scope with pytest.raises(ValueError): - self.runtime.service(self.descriptor, 'user_tags').set_tag('fake_scope', key, set_value) + self.descriptor.runtime.service(self.descriptor, 'user_tags').set_tag('fake_scope', key, set_value) # Try to get tag in wrong scope with pytest.raises(ValueError): - self.runtime.service(self.descriptor, 'user_tags').get_tag('fake_scope', key) + self.descriptor.runtime.service(self.descriptor, 'user_tags').get_tag('fake_scope', key) @ddt.ddt @@ -2381,23 +2382,23 @@ class TestBadgingService(LMSXBlockServiceMixin): @patch.dict(settings.FEATURES, {'ENABLE_OPENBADGES': True}) def test_service_rendered(self): self._prepare_runtime() - assert self.runtime.service(self.descriptor, 'badging') + assert self.descriptor.runtime.service(self.descriptor, 'badging') def test_no_service_rendered(self): with pytest.raises(NoSuchServiceError): - self.runtime.service(self.descriptor, 'badging') + self.descriptor.runtime.service(self.descriptor, 'badging') @ddt.data(True, False) @patch.dict(settings.FEATURES, {'ENABLE_OPENBADGES': True}) def test_course_badges_toggle(self, toggle): self.course = CourseFactory.create(metadata={'issue_badges': toggle}) self._prepare_runtime() - assert self.runtime.service(self.descriptor, 'badging').course_badges_enabled is toggle + assert self.descriptor.runtime.service(self.descriptor, 'badging').course_badges_enabled is toggle @patch.dict(settings.FEATURES, {'ENABLE_OPENBADGES': True}) def test_get_badge_class(self): self._prepare_runtime() - badge_service = self.runtime.service(self.descriptor, 'badging') + badge_service = self.descriptor.runtime.service(self.descriptor, 'badging') premade_badge_class = BadgeClassFactory.create() # Ignore additional parameters. This class already exists. # We should get back the first class we created, rather than a new one. @@ -2421,7 +2422,7 @@ class TestI18nService(LMSXBlockServiceMixin): """ Test: module i18n service in LMS """ - i18n_service = self.runtime.service(self.descriptor, 'i18n') + i18n_service = self.descriptor.runtime.service(self.descriptor, 'i18n') assert i18n_service is not None assert isinstance(i18n_service, XBlockI18nService) @@ -2431,27 +2432,30 @@ class TestI18nService(LMSXBlockServiceMixin): """ self.descriptor.service_declaration = Mock(return_value=None) with pytest.raises(NoSuchServiceError): - self.runtime.service(self.descriptor, 'i18n') + self.descriptor.runtime.service(self.descriptor, 'i18n') def test_no_service_exception_(self): """ Test: NoSuchServiceError should be raised if i18n service is none. """ - self.runtime._services['i18n'] = None # pylint: disable=protected-access + i18nService = self.descriptor.runtime._services['i18n'] # pylint: disable=protected-access + self.descriptor.runtime._runtime_services['i18n'] = None # pylint: disable=protected-access + self.descriptor.runtime._services['i18n'] = None # pylint: disable=protected-access with pytest.raises(NoSuchServiceError): - self.runtime.service(self.descriptor, 'i18n') + self.descriptor.runtime.service(self.descriptor, 'i18n') + self.descriptor.runtime._services['i18n'] = i18nService # pylint: disable=protected-access def test_i18n_service_callable(self): """ Test: _services dict should contain the callable i18n service in LMS. """ - assert callable(self.runtime._services.get('i18n')) # pylint: disable=protected-access + assert callable(self.descriptor.runtime._services.get('i18n')) # pylint: disable=protected-access def test_i18n_service_not_callable(self): """ Test: i18n service should not be callable in LMS after initialization. """ - assert not callable(self.runtime.service(self.descriptor, 'i18n')) + assert not callable(self.descriptor.runtime.service(self.descriptor, 'i18n')) class PureXBlockWithChildren(PureXBlock): @@ -2666,7 +2670,7 @@ class LmsModuleSystemShimTest(SharedModuleStoreTestCase): self.track_function = Mock() self.request_token = Mock() self.contentstore = contentstore() - self.runtime, _ = render.get_module_system_for_user( + _ = render.get_module_system_for_user( self.user, self.student_data, self.descriptor, @@ -2686,11 +2690,11 @@ class LmsModuleSystemShimTest(SharedModuleStoreTestCase): """ Tests that the deprecated attributes provided by the user service match expected values. """ - assert getattr(self.runtime, attribute) == expected_value + assert getattr(self.descriptor.runtime, attribute) == expected_value @patch('lms.djangoapps.courseware.block_render.has_access', Mock(return_value=True, autospec=True)) def test_user_is_staff(self): - runtime, _ = render.get_module_system_for_user( + _ = render.get_module_system_for_user( self.user, self.student_data, self.descriptor, @@ -2699,12 +2703,12 @@ class LmsModuleSystemShimTest(SharedModuleStoreTestCase): self.request_token, course=self.course, ) - assert runtime.user_is_staff - assert runtime.get_user_role() == 'student' + assert self.descriptor.runtime.user_is_staff + assert self.descriptor.runtime.get_user_role() == 'student' @patch('lms.djangoapps.courseware.block_render.get_user_role', Mock(return_value='instructor', autospec=True)) def test_get_user_role(self): - runtime, _ = render.get_module_system_for_user( + _ = render.get_module_system_for_user( self.user, self.student_data, self.descriptor, @@ -2713,17 +2717,17 @@ class LmsModuleSystemShimTest(SharedModuleStoreTestCase): self.request_token, course=self.course, ) - assert runtime.get_user_role() == 'instructor' + assert self.descriptor.runtime.get_user_role() == 'instructor' def test_anonymous_student_id(self): - assert self.runtime.anonymous_student_id == anonymous_id_for_user(self.user, self.course.id) + assert self.descriptor.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( + _ = render.get_module_system_for_user( self.user, self.student_data, self.problem_descriptor, @@ -2733,9 +2737,9 @@ class LmsModuleSystemShimTest(SharedModuleStoreTestCase): 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) + assert self.problem_descriptor.runtime.anonymous_student_id == anonymous_id_for_user(self.user, None) - vertical_runtime, _ = render.get_module_system_for_user( + _ = render.get_module_system_for_user( self.user, self.student_data, self.descriptor, @@ -2745,13 +2749,13 @@ class LmsModuleSystemShimTest(SharedModuleStoreTestCase): 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) + assert self.descriptor.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) + assert self.problem_descriptor.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( + _ = render.get_module_system_for_user( AnonymousUser(), self.student_data, self.descriptor, @@ -2760,14 +2764,14 @@ class LmsModuleSystemShimTest(SharedModuleStoreTestCase): 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 - assert not runtime.get_user_role() + assert self.descriptor.runtime.anonymous_student_id is None + assert self.descriptor.runtime.seed == 0 + assert self.descriptor.runtime.user_id is None + assert not self.descriptor.runtime.user_is_staff + assert not self.descriptor.runtime.get_user_role() def test_get_real_user(self): - runtime, _ = render.get_module_system_for_user( + _ = render.get_module_system_for_user( self.user, self.student_data, self.descriptor, @@ -2777,20 +2781,20 @@ class LmsModuleSystemShimTest(SharedModuleStoreTestCase): course=self.course, ) course_anonymous_student_id = anonymous_id_for_user(self.user, self.course.id) - assert runtime.get_real_user(course_anonymous_student_id) == self.user # pylint: disable=not-callable + assert self.descriptor.runtime.get_real_user(course_anonymous_student_id) == self.user # pylint: disable=not-callable no_course_anonymous_student_id = anonymous_id_for_user(self.user, None) - assert runtime.get_real_user(no_course_anonymous_student_id) == self.user # pylint: disable=not-callable + assert self.descriptor.runtime.get_real_user(no_course_anonymous_student_id) == self.user # pylint: disable=not-callable # Tests that the default is to use the user service's anonymous_student_id - assert runtime.get_real_user() == self.user # pylint: disable=not-callable + assert self.descriptor.runtime.get_real_user() == self.user # pylint: disable=not-callable def test_render_template(self): - rendered = self.runtime.render_template('templates/edxmako.html', {'element_id': 'hi'}) # pylint: disable=not-callable + rendered = self.descriptor.runtime.render_template('templates/edxmako.html', {'element_id': 'hi'}) # pylint: disable=not-callable assert rendered == '
Testing the MakoService
\n' def test_xqueue(self): - xqueue = self.runtime.xqueue + xqueue = self.descriptor.runtime.xqueue assert isinstance(xqueue['interface'], XQueueInterface) assert xqueue['interface'].url == 'http://sandbox-xqueue.edx.org' assert xqueue['default_queuename'] == 'edX-LmsModuleShimTest' @@ -2812,7 +2816,7 @@ class LmsModuleSystemShimTest(SharedModuleStoreTestCase): XQUEUE_WAITTIME_BETWEEN_REQUESTS=15, ) def test_xqueue_settings(self): - runtime, _ = render.get_module_system_for_user( + _ = render.get_module_system_for_user( self.user, self.student_data, self.descriptor, @@ -2821,7 +2825,7 @@ class LmsModuleSystemShimTest(SharedModuleStoreTestCase): self.request_token, course=self.course, ) - xqueue = runtime.xqueue + xqueue = self.descriptor.runtime.xqueue assert isinstance(xqueue['interface'], XQueueInterface) assert xqueue['interface'].url == 'http://xqueue.url' assert xqueue['default_queuename'] == 'edX-LmsModuleShimTest' @@ -2832,14 +2836,14 @@ class LmsModuleSystemShimTest(SharedModuleStoreTestCase): @override_settings(COURSES_WITH_UNSAFE_CODE=[r'course-v1:edX\+LmsModuleShimTest\+2021_Fall']) def test_can_execute_unsafe_code_when_allowed(self): - assert self.runtime.can_execute_unsafe_code() + assert self.descriptor.runtime.can_execute_unsafe_code() @override_settings(COURSES_WITH_UNSAFE_CODE=[r'course-v1:edX\+full\+2021_Fall']) def test_cannot_execute_unsafe_code_when_disallowed(self): - assert not self.runtime.can_execute_unsafe_code() + assert not self.descriptor.runtime.can_execute_unsafe_code() def test_cannot_execute_unsafe_code(self): - assert not self.runtime.can_execute_unsafe_code() + assert not self.descriptor.runtime.can_execute_unsafe_code() @override_settings(PYTHON_LIB_FILENAME=PYTHON_LIB_FILENAME) def test_get_python_lib_zip(self): @@ -2849,7 +2853,7 @@ class LmsModuleSystemShimTest(SharedModuleStoreTestCase): source_file=self.PYTHON_LIB_SOURCE_FILE, target_filename=self.PYTHON_LIB_FILENAME, ) - assert self.runtime.get_python_lib_zip() == zipfile + assert self.descriptor.runtime.get_python_lib_zip() == zipfile def test_no_get_python_lib_zip(self): zipfile = upload_file_to_course( @@ -2858,26 +2862,26 @@ class LmsModuleSystemShimTest(SharedModuleStoreTestCase): source_file=self.PYTHON_LIB_SOURCE_FILE, target_filename=self.PYTHON_LIB_FILENAME, ) - assert self.runtime.get_python_lib_zip() is None + assert self.descriptor.runtime.get_python_lib_zip() is None def test_cache(self): - assert hasattr(self.runtime.cache, 'get') - assert hasattr(self.runtime.cache, 'set') + assert hasattr(self.descriptor.runtime.cache, 'get') + assert hasattr(self.descriptor.runtime.cache, 'set') def test_replace_urls(self): html = '' - assert self.runtime.replace_urls(html) == \ + assert self.descriptor.runtime.replace_urls(html) == \ static_replace.replace_static_urls(html, course_id=self.course.id) def test_replace_course_urls(self): html = '' - assert self.runtime.replace_course_urls(html) == \ + assert self.descriptor.runtime.replace_course_urls(html) == \ static_replace.replace_course_urls(html, course_key=self.course.id) def test_replace_jump_to_id_urls(self): html = '' jump_to_id_base_url = reverse('jump_to_id', kwargs={'course_id': str(self.course.id), 'module_id': ''}) - assert self.runtime.replace_jump_to_id_urls(html) == \ + assert self.descriptor.runtime.replace_jump_to_id_urls(html) == \ static_replace.replace_jump_to_id_urls(html, self.course.id, jump_to_id_base_url) @XBlock.register_temp_plugin(PureXBlock, 'pure') diff --git a/lms/djangoapps/courseware/tests/test_discussion_xblock.py b/lms/djangoapps/courseware/tests/test_discussion_xblock.py index 49f76be0f4..159add3e05 100644 --- a/lms/djangoapps/courseware/tests/test_discussion_xblock.py +++ b/lms/djangoapps/courseware/tests/test_discussion_xblock.py @@ -55,7 +55,6 @@ class TestDiscussionXBlock(XModuleRenderingTestBase): field_data=self.data, scope_ids=scope_ids ) - self.block.xmodule_runtime = mock.Mock() if self.PATCH_DJANGO_USER: self.django_user_canary = UserFactory() diff --git a/lms/djangoapps/courseware/tests/test_lti_integration.py b/lms/djangoapps/courseware/tests/test_lti_integration.py index bf85e7db61..e7d7f856ba 100644 --- a/lms/djangoapps/courseware/tests/test_lti_integration.py +++ b/lms/djangoapps/courseware/tests/test_lti_integration.py @@ -41,7 +41,7 @@ class TestLTI(BaseTestXmodule): # 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_service = self.item_descriptor.runtime.service(self.item_descriptor, 'user') user_id = str(user_service.get_current_user().opt_attrs.get(ATTR_KEY_ANONYMOUS_USER_ID)) hostname = settings.LMS_BASE resource_link_id = str(urllib.parse.quote(f'{hostname}-{self.item_descriptor.location.html_id()}')) @@ -81,8 +81,10 @@ class TestLTI(BaseTestXmodule): 'element_id': self.item_descriptor.location.html_id(), 'launch_url': 'http://www.example.com', # default value 'open_in_a_new_page': True, - 'form_url': self.item_descriptor.xmodule_runtime.handler_url(self.item_descriptor, - 'preview_handler').rstrip('/?'), + 'form_url': self.item_descriptor.runtime.handler_url( + self.item_descriptor, + 'preview_handler' + ).rstrip('/?'), 'hide_launch': False, 'has_score': False, 'module_score': None, diff --git a/lms/djangoapps/courseware/tests/test_video_handlers.py b/lms/djangoapps/courseware/tests/test_video_handlers.py index d2a260bf86..737c6c25b9 100644 --- a/lms/djangoapps/courseware/tests/test_video_handlers.py +++ b/lms/djangoapps/courseware/tests/test_video_handlers.py @@ -22,6 +22,8 @@ from xmodule.contentstore.django import contentstore # lint-amnesty, pylint: di from xmodule.exceptions import NotFoundError # lint-amnesty, pylint: disable=wrong-import-order from xmodule.modulestore import ModuleStoreEnum # lint-amnesty, pylint: disable=wrong-import-order from xmodule.modulestore.django import modulestore # lint-amnesty, pylint: disable=wrong-import-order +# noinspection PyUnresolvedReferences +from xmodule.tests.helpers import override_descriptor_system # pylint: disable=unused-import from xmodule.video_block import VideoBlock # lint-amnesty, pylint: disable=wrong-import-order from xmodule.video_block.transcripts_utils import ( # lint-amnesty, pylint: disable=wrong-import-order Transcript, @@ -29,7 +31,7 @@ from xmodule.video_block.transcripts_utils import ( # lint-amnesty, pylint: dis get_transcript, subs_filename, ) -from xmodule.x_module import STUDENT_VIEW # lint-amnesty, pylint: disable=wrong-import-order +from xmodule.x_module import STUDENT_VIEW from .helpers import BaseTestXmodule from .test_video_xml import SOURCE_XML @@ -131,6 +133,7 @@ def attach_bumper_transcript(item, filename, lang="en"): item.video_bumper["transcripts"][lang] = filename +@pytest.mark.usefixtures("override_descriptor_system") class BaseTestVideoXBlock(BaseTestXmodule): """Base class for VideoXBlock tests.""" @@ -232,7 +235,7 @@ class TestVideo(BaseTestVideoXBlock): """ Return the URL for the specified handler on self.item_descriptor. """ - return self.item_descriptor.xmodule_runtime.handler_url( + return self.item_descriptor.runtime.handler_url( self.item_descriptor, handler, suffix ).rstrip('/?') diff --git a/lms/djangoapps/courseware/tests/test_video_mongo.py b/lms/djangoapps/courseware/tests/test_video_mongo.py index 6c5bb5e8f0..88fb132e6b 100644 --- a/lms/djangoapps/courseware/tests/test_video_mongo.py +++ b/lms/djangoapps/courseware/tests/test_video_mongo.py @@ -39,6 +39,8 @@ from xmodule.exceptions import NotFoundError from xmodule.modulestore import ModuleStoreEnum from xmodule.modulestore.inheritance import own_metadata from xmodule.modulestore.tests.django_utils import TEST_DATA_MONGO_MODULESTORE, TEST_DATA_SPLIT_MODULESTORE +# noinspection PyUnresolvedReferences +from xmodule.tests.helpers import override_descriptor_system # pylint: disable=unused-import from xmodule.tests.test_import import DummySystem from xmodule.tests.test_video import VideoBlockTestBase from xmodule.video_block import VideoBlock, bumper_utils, video_utils @@ -136,7 +138,7 @@ class TestVideoYouTube(TestVideo): # lint-amnesty, pylint: disable=missing-clas 'public_video_url': None, } - mako_service = self.item_descriptor.xmodule_runtime.service(self.item_descriptor, 'mako') + mako_service = self.item_descriptor.runtime.service(self.item_descriptor, 'mako') assert get_context_dict_from_string(context) ==\ get_context_dict_from_string(mako_service.render_template('video.html', expected_context)) @@ -220,7 +222,7 @@ class TestVideoNonYouTube(TestVideo): # pylint: disable=test-inherits-tests 'public_video_url': None, } - mako_service = self.item_descriptor.xmodule_runtime.service(self.item_descriptor, 'mako') + mako_service = self.item_descriptor.runtime.service(self.item_descriptor, 'mako') expected_result = get_context_dict_from_string( mako_service.render_template('video.html', expected_context) ) @@ -305,7 +307,7 @@ class TestGetHtmlMethod(BaseTestVideoXBlock): Return the URL for the specified handler on the block represented by self.item_descriptor. """ - return self.item_descriptor.xmodule_runtime.handler_url( + return self.item_descriptor.runtime.handler_url( self.item_descriptor, handler, suffix ).rstrip('/?') @@ -422,7 +424,7 @@ class TestGetHtmlMethod(BaseTestVideoXBlock): 'metadata': json.dumps(metadata) }) - mako_service = self.item_descriptor.xmodule_runtime.service(self.item_descriptor, 'mako') + mako_service = self.item_descriptor.runtime.service(self.item_descriptor, 'mako') assert get_context_dict_from_string(context) ==\ get_context_dict_from_string(mako_service.render_template('video.html', expected_context)) @@ -533,7 +535,7 @@ class TestGetHtmlMethod(BaseTestVideoXBlock): 'metadata': json.dumps(expected_context['metadata']) }) - mako_service = self.item_descriptor.xmodule_runtime.service(self.item_descriptor, 'mako') + mako_service = self.item_descriptor.runtime.service(self.item_descriptor, 'mako') assert get_context_dict_from_string(context) ==\ get_context_dict_from_string(mako_service.render_template('video.html', expected_context)) @@ -676,7 +678,7 @@ class TestGetHtmlMethod(BaseTestVideoXBlock): 'metadata': json.dumps(expected_context['metadata']) }) - mako_service = self.item_descriptor.xmodule_runtime.service(self.item_descriptor, 'mako') + mako_service = self.item_descriptor.runtime.service(self.item_descriptor, 'mako') assert get_context_dict_from_string(context) ==\ get_context_dict_from_string(mako_service.render_template('video.html', expected_context)) @@ -704,7 +706,7 @@ class TestGetHtmlMethod(BaseTestVideoXBlock): # context returned by get_html when provided with above data # expected_context, a dict to assert with context context, expected_context = self.helper_get_html_with_edx_video_id(data) - mako_service = self.item_descriptor.xmodule_runtime.service(self.item_descriptor, 'mako') + mako_service = self.item_descriptor.runtime.service(self.item_descriptor, 'mako') assert get_context_dict_from_string(context) ==\ get_context_dict_from_string(mako_service.render_template('video.html', expected_context)) @@ -735,7 +737,7 @@ class TestGetHtmlMethod(BaseTestVideoXBlock): # expected_context, a dict to assert with context context, expected_context = self.helper_get_html_with_edx_video_id(data) - mako_service = self.item_descriptor.xmodule_runtime.service(self.item_descriptor, 'mako') + mako_service = self.item_descriptor.runtime.service(self.item_descriptor, 'mako') assert get_context_dict_from_string(context) ==\ get_context_dict_from_string(mako_service.render_template('video.html', expected_context)) @@ -936,7 +938,7 @@ class TestGetHtmlMethod(BaseTestVideoXBlock): self.initialize_block(data=DATA, runtime_kwargs={ 'user_location': 'CN', }) - user_service = self.item_descriptor.xmodule_runtime.service(self.item_descriptor, 'user') + user_service = self.item_descriptor.runtime.service(self.item_descriptor, 'user') user_location = user_service.get_current_user().opt_attrs[ATTR_KEY_REQUEST_COUNTRY_CODE] assert user_location == 'CN' context = self.item_descriptor.render('student_view').content @@ -954,7 +956,7 @@ class TestGetHtmlMethod(BaseTestVideoXBlock): 'metadata': json.dumps(expected_context['metadata']) }) - mako_service = self.item_descriptor.xmodule_runtime.service(self.item_descriptor, 'mako') + mako_service = self.item_descriptor.runtime.service(self.item_descriptor, 'mako') assert get_context_dict_from_string(context) ==\ get_context_dict_from_string(mako_service.render_template('video.html', expected_context)) @@ -1059,7 +1061,7 @@ class TestGetHtmlMethod(BaseTestVideoXBlock): 'metadata': json.dumps(expected_context['metadata']) }) - mako_service = self.item_descriptor.xmodule_runtime.service(self.item_descriptor, 'mako') + mako_service = self.item_descriptor.runtime.service(self.item_descriptor, 'mako') assert get_context_dict_from_string(context) ==\ get_context_dict_from_string(mako_service.render_template('video.html', expected_context)) @@ -2312,7 +2314,7 @@ class TestVideoWithBumper(TestVideo): # pylint: disable=test-inherits-tests })) } - mako_service = self.item_descriptor.xmodule_runtime.service(self.item_descriptor, 'mako') + mako_service = self.item_descriptor.runtime.service(self.item_descriptor, 'mako') expected_content = mako_service.render_template('video.html', expected_context) assert get_context_dict_from_string(content) == get_context_dict_from_string(expected_content) @@ -2368,10 +2370,10 @@ class TestAutoAdvanceVideo(TestVideo): # lint-amnesty, pylint: disable=test-inh 'ytTestTimeout': 1500, 'ytApiUrl': 'https://www.youtube.com/iframe_api', 'lmsRootURL': settings.LMS_ROOT_URL, - 'transcriptTranslationUrl': self.item_descriptor.xmodule_runtime.handler_url( + 'transcriptTranslationUrl': self.item_descriptor.runtime.handler_url( self.item_descriptor, 'transcript', 'translation/__lang__' ).rstrip('/?'), - 'transcriptAvailableTranslationsUrl': self.item_descriptor.xmodule_runtime.handler_url( + 'transcriptAvailableTranslationsUrl': self.item_descriptor.runtime.handler_url( self.item_descriptor, 'transcript', 'available_translations' ).rstrip('/?'), 'autohideHtml5': False, @@ -2407,7 +2409,7 @@ class TestAutoAdvanceVideo(TestVideo): # lint-amnesty, pylint: disable=test-inh autoadvance_flag=autoadvance_must_be, ) - mako_service = self.item_descriptor.xmodule_runtime.service(self.item_descriptor, 'mako') + mako_service = self.item_descriptor.runtime.service(self.item_descriptor, 'mako') with override_settings(FEATURES=self.FEATURES): expected_content = mako_service.render_template('video.html', expected_context) diff --git a/lms/djangoapps/courseware/tests/test_word_cloud.py b/lms/djangoapps/courseware/tests/test_word_cloud.py index 2ba9ba8374..dd367b2023 100644 --- a/lms/djangoapps/courseware/tests/test_word_cloud.py +++ b/lms/djangoapps/courseware/tests/test_word_cloud.py @@ -1,14 +1,19 @@ """Word cloud integration tests using mongo modulestore.""" +import pytest + import json from operator import itemgetter +# noinspection PyUnresolvedReferences +from xmodule.tests.helpers import override_descriptor_system # pylint: disable=unused-import from xmodule.x_module import STUDENT_VIEW from .helpers import BaseTestXmodule +@pytest.mark.usefixtures("override_descriptor_system") class TestWordCloud(BaseTestXmodule): """Integration test for Word Cloud Block.""" CATEGORY = "word_cloud" diff --git a/lms/djangoapps/instructor_task/tests/test_tasks_helper.py b/lms/djangoapps/instructor_task/tests/test_tasks_helper.py index d5e1ef28b1..e3c2901745 100644 --- a/lms/djangoapps/instructor_task/tests/test_tasks_helper.py +++ b/lms/djangoapps/instructor_task/tests/test_tasks_helper.py @@ -16,6 +16,7 @@ from datetime import datetime, timedelta from unittest.mock import ANY, MagicMock, Mock, patch import ddt +import pytest import unicodecsv from django.conf import settings from django.test.utils import override_settings @@ -70,6 +71,8 @@ from openedx.core.lib.teams_config import TeamsConfig from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase # lint-amnesty, pylint: disable=wrong-import-order from xmodule.modulestore.tests.factories import CourseFactory, BlockFactory, check_mongo_calls # lint-amnesty, pylint: disable=wrong-import-order from xmodule.partitions.partitions import Group, UserPartition # lint-amnesty, pylint: disable=wrong-import-order +# noinspection PyUnresolvedReferences +from xmodule.tests.helpers import override_descriptor_system # pylint: disable=unused-import from ..models import ReportStore from ..tasks_helper.utils import UPDATE_STATUS_FAILED, UPDATE_STATUS_SUCCEEDED @@ -1080,6 +1083,7 @@ class TestProblemReportSplitTestContent(TestReportMixin, TestConditionalContent, @ddt.ddt +@pytest.mark.usefixtures("override_descriptor_system") class TestProblemReportCohortedContent(TestReportMixin, ContentGroupTestCase, InstructorTaskModuleTestCase): """ Test the problem report on a course that has cohorted content. @@ -1729,6 +1733,7 @@ class TestCohortStudents(TestReportMixin, InstructorTaskCourseTestCase): @ddt.ddt @patch('lms.djangoapps.instructor_task.tasks_helper.misc.DefaultStorage', new=MockDefaultStorage) +@pytest.mark.usefixtures("override_descriptor_system") class TestGradeReport(TestReportMixin, InstructorTaskModuleTestCase): """ Test that grade report has correct grade values. diff --git a/lms/djangoapps/lms_xblock/test/test_runtime.py b/lms/djangoapps/lms_xblock/test/test_runtime.py index 046c2139ef..6c9cf69f49 100644 --- a/lms/djangoapps/lms_xblock/test/test_runtime.py +++ b/lms/djangoapps/lms_xblock/test/test_runtime.py @@ -11,7 +11,8 @@ from django.test import TestCase from opaque_keys.edx.locations import BlockUsageLocator, CourseLocator from xblock.fields import ScopeIds -from lms.djangoapps.lms_xblock.runtime import LmsModuleSystem +from xmodule.x_module import DescriptorSystem +from lms.djangoapps.lms_xblock.runtime import handler_url class BlockMock(Mock): @@ -50,10 +51,13 @@ class TestHandlerUrl(TestCase): def setUp(self): super().setUp() self.block = BlockMock(name='block') - self.runtime = LmsModuleSystem( - get_block=Mock(), - descriptor_runtime=Mock(), + self.runtime = DescriptorSystem( + load_item=Mock(name='get_test_descriptor_system.load_item'), + resources_fs=Mock(name='get_test_descriptor_system.resources_fs'), + error_tracker=Mock(name='get_test_descriptor_system.error_tracker') ) + self.runtime.get_block_for_descriptor = Mock() + self.runtime.handler_url_override = handler_url def test_trailing_characters(self): assert not self.runtime.handler_url(self.block, 'handler').endswith('?') diff --git a/openedx/tests/completion_integration/test_services.py b/openedx/tests/completion_integration/test_services.py index d7bc197d95..a15ad307db 100644 --- a/openedx/tests/completion_integration/test_services.py +++ b/openedx/tests/completion_integration/test_services.py @@ -13,7 +13,7 @@ from opaque_keys.edx.keys import CourseKey from xmodule.library_tools import LibraryToolsService from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase from xmodule.modulestore.tests.factories import CourseFactory, BlockFactory, LibraryFactory -from xmodule.tests import get_test_system +from xmodule.tests import prepare_block_runtime from openedx.core.djangolib.testing.utils import skip_unless_lms from common.djangoapps.student.tests.factories import UserFactory @@ -121,20 +121,17 @@ class CompletionServiceTestCase(CompletionWaffleTestMixin, SharedModuleStoreTest """ Bind a block (part of self.course) so we can access student-specific data. """ - module_system = get_test_system(course_id=block.location.course_key) - module_system.descriptor_runtime = block.runtime._descriptor_system # pylint: disable=protected-access - module_system._services['library_tools'] = LibraryToolsService(self.store, self.user.id) # pylint: disable=protected-access + prepare_block_runtime(block.runtime, course_id=block.location.course_key) + block.runtime._services.update({'library_tools': LibraryToolsService(self.store, self.user.id)}) # lint-amnesty, pylint: disable=protected-access def get_block(descriptor): """Mocks module_system get_block_for_descriptor function""" - sub_module_system = get_test_system(course_id=block.location.course_key) - sub_module_system.get_block_for_descriptor = get_block - sub_module_system.descriptor_runtime = descriptor._runtime # pylint: disable=protected-access - descriptor.bind_for_student(sub_module_system, self.user.id) + prepare_block_runtime(descriptor.runtime, course_id=block.location.course_key) + descriptor.runtime.get_block_for_descriptor = get_block + descriptor.bind_for_student(self.user.id) return descriptor - module_system.get_block_for_descriptor = get_block - block.xmodule_runtime = module_system + block.runtime.get_block_for_descriptor = get_block def test_completion_service(self): # Only the completions for the user and course specified for the CompletionService diff --git a/xmodule/tests/__init__.py b/xmodule/tests/__init__.py index 7770988f64..e31aee0216 100644 --- a/xmodule/tests/__init__.py +++ b/xmodule/tests/__init__.py @@ -36,7 +36,7 @@ from xmodule.modulestore.inheritance import InheritanceMixin from xmodule.modulestore.xml import CourseLocationManager from xmodule.tests.helpers import StubReplaceURLService, mock_render_template, StubMakoService, StubUserService from xmodule.util.sandboxing import SandboxService -from xmodule.x_module import DoNothingCache, ModuleSystem, XModuleMixin +from xmodule.x_module import DoNothingCache, XModuleMixin from openedx.core.lib.cache_utils import CacheService @@ -46,9 +46,35 @@ MODULE_DIR = path(__file__).dirname() DATA_DIR = MODULE_DIR.parent.parent / "common" / "test" / "data" -class TestModuleSystem(ModuleSystem): # pylint: disable=abstract-method +def handler_url(block, handler, suffix='', query='', thirdparty=False): # lint-amnesty, pylint: disable=arguments-differ + return '{usage_id}/{handler}{suffix}?{query}'.format( + usage_id=str(block.scope_ids.usage_id), + handler=handler, + suffix=suffix, + query=query, + ) + + +def local_resource_url(block, uri): + return 'resource/{usage_id}/{uri}'.format( + usage_id=str(block.scope_ids.usage_id), + uri=uri, + ) + + +# Disable XBlockAsides in most tests +def get_asides(block): + return [] + + +@property +def resources_fs(): + return Mock(name='TestDescriptorSystem.resources_fs', root_path='.') + + +class TestDescriptorSystem(MakoDescriptorSystem): # pylint: disable=abstract-method """ - ModuleSystem for testing + DescriptorSystem for testing """ def handler_url(self, block, handler, suffix='', query='', thirdparty=False): # lint-amnesty, pylint: disable=arguments-differ return '{usage_id}/{handler}{suffix}?{query}'.format( @@ -68,9 +94,8 @@ class TestModuleSystem(ModuleSystem): # pylint: disable=abstract-method def get_asides(self, block): return [] - @property - def resources_fs(self): - return Mock(name='TestModuleSystem.resources_fs', root_path='.') + def resources_fs(self): # lint-amnesty, pylint: disable=method-hidden + return Mock(name='TestDescriptorSystem.resources_fs', root_path='.') def __repr__(self): """ @@ -94,13 +119,19 @@ def get_test_system( user_is_staff=False, user_location=None, render_template=None, + add_get_block_overrides=False ): """ - Construct a test ModuleSystem instance. + Construct a test DescriptorSystem instance. By default, the descriptor system's render_template() method simply returns the repr of the context it is passed. You can override this by passing in a different render_template argument. """ + + id_manager = CourseLocationManager(course_id) + + descriptor_system = get_test_descriptor_system(id_reader=id_manager, id_generator=id_manager) + if not user: user = Mock(name='get_test_system.user', is_staff=False) if not user_location: @@ -117,63 +148,128 @@ def get_test_system( replace_url_service = StubReplaceURLService() - descriptor_system = get_test_descriptor_system() - - id_manager = CourseLocationManager(course_id) - - def get_block(descriptor): + def get_block(block): """Mocks module_system get_block function""" - # Unlike XBlock Runtimes or DescriptorSystems, - # each XModule is provided with a new ModuleSystem. - # Construct one for the new XModule. - module_system = get_test_system() + prepare_block_runtime(block.runtime, add_overrides=add_get_block_overrides) + block.runtime.get_block_for_descriptor = get_block + block.bind_for_student(user.id) - # Descriptors can all share a single DescriptorSystem. - # So, bind to the same one as the current descriptor. - module_system.descriptor_runtime = descriptor._runtime # pylint: disable=protected-access + return block - descriptor.bind_for_student(module_system, user.id) + services = { + 'user': user_service, + 'mako': mako_service, + 'xqueue': XQueueService( + url='http://xqueue.url', + django_auth={}, + basic_auth=[], + default_queuename='testqueue', + waittime=10, + construct_callback=Mock(name='get_test_system.xqueue.construct_callback', side_effect="/"), + ), + 'replace_urls': replace_url_service, + 'cache': CacheService(DoNothingCache()), + 'field-data': DictFieldData({}), + 'sandbox': SandboxService(contentstore, course_id), + } - return descriptor + descriptor_system.get_block_for_descriptor = get_block # lint-amnesty, pylint: disable=attribute-defined-outside-init + descriptor_system._services.update(services) # lint-amnesty, pylint: disable=protected-access - return TestModuleSystem( - get_block=get_block, - services={ - 'user': user_service, - 'mako': mako_service, - 'xqueue': XQueueService( - url='http://xqueue.url', - django_auth={}, - basic_auth=[], - default_queuename='testqueue', - waittime=10, - construct_callback=Mock(name='get_test_system.xqueue.construct_callback', side_effect="/"), - ), - 'replace_urls': replace_url_service, - 'cache': CacheService(DoNothingCache()), - 'field-data': DictFieldData({}), - 'sandbox': SandboxService(contentstore, course_id), - }, - descriptor_runtime=descriptor_system, - id_reader=id_manager, - id_generator=id_manager, + return descriptor_system + + +def prepare_block_runtime( + runtime, + course_id=CourseKey.from_string('/'.join(['org', 'course', 'run'])), + user=None, + user_is_staff=False, + user_location=None, + render_template=None, + add_overrides=False, + add_get_block=False, +): + """ + Sets properties in the runtime of the specified descriptor that is + required for tests. + """ + + if not user: + user = Mock(name='get_test_system.user', is_staff=False) + if not user_location: + user_location = Mock(name='get_test_system.user_location') + user_service = StubUserService( + user=user, + anonymous_user_id='student', + user_is_staff=user_is_staff, + user_role='student', + request_country_code=user_location, ) + mako_service = StubMakoService(render_template=render_template) -def get_test_descriptor_system(render_template=None): + replace_url_service = StubReplaceURLService() + + def get_block(block): + """Mocks module_system get_block function""" + + prepare_block_runtime(block.runtime) + block.bind_for_student(user.id) + + return block + + services = { + 'user': user_service, + 'mako': mako_service, + 'xqueue': XQueueService( + url='http://xqueue.url', + django_auth={}, + basic_auth=[], + default_queuename='testqueue', + waittime=10, + construct_callback=Mock(name='get_test_system.xqueue.construct_callback', side_effect="/"), + ), + 'replace_urls': replace_url_service, + 'cache': CacheService(DoNothingCache()), + 'field-data': DictFieldData({}), + 'sandbox': SandboxService(contentstore, course_id), + } + + if add_overrides: + runtime.handler_url_override = handler_url + runtime.local_resource_url = local_resource_url + runtime.get_asides = get_asides + runtime.resources_fs = resources_fs + + if add_get_block: + runtime.get_block_for_descriptor = get_block + + runtime._services.update(services) # lint-amnesty, pylint: disable=protected-access + + # runtime.load_item=Mock(name='get_test_descriptor_system.load_item') + # runtime.resources_fs=Mock(name='get_test_descriptor_system.resources_fs') + # runtime.error_tracker=Mock(name='get_test_descriptor_system.error_tracker') + # runtime.render_template=render_template or mock_render_template, + # runtime.mixins=(InheritanceMixin, XModuleMixin) + + return runtime + + +def get_test_descriptor_system(render_template=None, **kwargs): """ Construct a test DescriptorSystem instance. """ field_data = DictFieldData({}) - descriptor_system = MakoDescriptorSystem( + descriptor_system = TestDescriptorSystem( load_item=Mock(name='get_test_descriptor_system.load_item'), resources_fs=Mock(name='get_test_descriptor_system.resources_fs'), error_tracker=Mock(name='get_test_descriptor_system.error_tracker'), render_template=render_template or mock_render_template, mixins=(InheritanceMixin, XModuleMixin), services={'field-data': field_data}, + **kwargs ) descriptor_system.get_asides = lambda block: [] return descriptor_system diff --git a/xmodule/tests/helpers.py b/xmodule/tests/helpers.py index 2f7a399ab0..d69be6bc00 100644 --- a/xmodule/tests/helpers.py +++ b/xmodule/tests/helpers.py @@ -6,8 +6,10 @@ Utility methods for unit tests. import filecmp import pprint +import pytest from path import Path as path from xblock.reference.user_service import UserService, XBlockUser +from xmodule.x_module import DescriptorSystem def directories_equal(directory1, directory2): @@ -72,6 +74,7 @@ class StubUserService(UserService): self.user_role = user_role self.anonymous_user_id = anonymous_user_id self.request_country_code = request_country_code + self._django_user = user super().__init__(**kwargs) def get_current_user(self): @@ -109,3 +112,17 @@ class StubReplaceURLService: Invokes the configured render_template method. """ return text + + +@pytest.fixture +def override_descriptor_system(monkeypatch): + """ + Fixture to override get_block method of DescriptorSystem + """ + + def get_block(self, usage_id, for_parent=None): + """See documentation for `xblock.runtime:Runtime.get_block`""" + block = self.load_item(usage_id, for_parent=for_parent) + return block + + monkeypatch.setattr(DescriptorSystem, "get_block", get_block) diff --git a/xmodule/tests/test_conditional.py b/xmodule/tests/test_conditional.py index b36ae92b63..0413842557 100644 --- a/xmodule/tests/test_conditional.py +++ b/xmodule/tests/test_conditional.py @@ -16,7 +16,7 @@ from xblock.fields import ScopeIds from xmodule.conditional_block import ConditionalBlock from xmodule.error_block import ErrorBlock from xmodule.modulestore.xml import CourseLocationManager, ImportSystem, XMLModuleStore -from xmodule.tests import DATA_DIR, get_test_descriptor_system, get_test_system +from xmodule.tests import DATA_DIR, get_test_system, prepare_block_runtime from xmodule.tests.xml import XModuleXmlImportTest from xmodule.tests.xml import factories as xml from xmodule.validation import StudioValidationMessage @@ -41,7 +41,8 @@ class DummySystem(ImportSystem): # lint-amnesty, pylint: disable=abstract-metho load_error_blocks=load_error_blocks, ) - def render_template(self, template, context): # lint-amnesty, pylint: disable=method-hidden + @property + def render_template(self): # lint-amnesty, pylint: disable=method-hidden raise Exception("Shouldn't be called") @@ -65,7 +66,6 @@ class ConditionalFactory: if the source_is_error_block flag is set, create a real ErrorBlock for the source. """ - descriptor_system = get_test_descriptor_system() # construct source descriptor and module: source_location = BlockUsageLocator(CourseLocator("edX", "conditional_test", "test_run", deprecated=True), @@ -83,17 +83,16 @@ class ConditionalFactory: source_descriptor.location = source_location source_descriptor.visible_to_staff_only = source_visible_to_staff_only - source_descriptor.runtime = descriptor_system - source_descriptor.render = lambda view, context=None: descriptor_system.render(source_descriptor, view, context) + source_descriptor.runtime = system + source_descriptor.render = lambda view, context=None: system.render(source_descriptor, view, context) # construct other descriptors: child_descriptor = Mock(name='child_descriptor') child_descriptor.visible_to_staff_only = False child_descriptor._xmodule.student_view.return_value = Fragment(content='

This is a secret

') # lint-amnesty, pylint: disable=protected-access child_descriptor.student_view = child_descriptor._xmodule.student_view # lint-amnesty, pylint: disable=protected-access - child_descriptor.runtime = descriptor_system - child_descriptor.xmodule_runtime = get_test_system() - child_descriptor.render = lambda view, context=None: descriptor_system.render(child_descriptor, view, context) + child_descriptor.runtime = system + child_descriptor.render = lambda view, context=None: system.render(child_descriptor, view, context) child_descriptor.location = source_location.replace(category='html', name='child') def visible_to_nonstaff_users(desc): @@ -109,9 +108,7 @@ class ConditionalFactory: source_location: source_descriptor }.get(usage_id) - descriptor_system.load_item = load_item - - system.descriptor_runtime = descriptor_system + system.load_item = load_item # construct conditional block: cond_location = BlockUsageLocator(CourseLocator("edX", "conditional_test", "test_run", deprecated=True), @@ -125,11 +122,10 @@ class ConditionalFactory: }) cond_descriptor = ConditionalBlock( - descriptor_system, + system, field_data, ScopeIds(None, None, cond_location, cond_location) ) - cond_descriptor.xmodule_runtime = system system.get_block_for_descriptor = lambda desc: desc if visible_to_nonstaff_users(desc) else None cond_descriptor.get_required_blocks = [ system.get_block_for_descriptor(source_descriptor), @@ -165,7 +161,7 @@ class ConditionalBlockBasicTest(unittest.TestCase): # because get_test_system returns the repr of the context dict passed to render_template, # we reverse it here html = blocks['cond_block'].render(STUDENT_VIEW).content - mako_service = blocks['cond_block'].xmodule_runtime.service(blocks['cond_block'], 'mako') + mako_service = blocks['cond_block'].runtime.service(blocks['cond_block'], 'mako') expected = mako_service.render_template('conditional_ajax.html', { 'ajax_url': blocks['cond_block'].ajax_url, 'element_id': 'i4x-edX-conditional_test-conditional-SampleConditional', @@ -220,7 +216,12 @@ class ConditionalBlockXmlTest(unittest.TestCase): def setUp(self): super().setUp() - self.test_system = get_test_system() + + def add_block_as_child_node(block, node): + child = etree.SubElement(node, "unknown") + block.add_xml_to_node(child) + self.test_system = get_test_system(add_get_block_overrides=True) + self.test_system.add_block_as_child_node = add_block_as_child_node self.modulestore = XMLModuleStore(DATA_DIR, source_dirs=['conditional_and_poll']) courses = self.modulestore.get_courses() assert len(courses) == 1 @@ -241,7 +242,7 @@ class ConditionalBlockXmlTest(unittest.TestCase): block = self.get_block_for_location(location) html = block.render(STUDENT_VIEW).content - mako_service = block.xmodule_runtime.service(block, 'mako') + mako_service = block.runtime.service(block, 'mako') html_expect = mako_service.render_template( 'conditional_ajax.html', { @@ -355,12 +356,9 @@ class ConditionalBlockStudioTest(XModuleXmlImportTest): self.sequence = self.course.get_children()[0] self.conditional = self.sequence.get_children()[0] - self.module_system = get_test_system() - self.module_system.descriptor_runtime = self.course._runtime # pylint: disable=protected-access - user = Mock(username='ma', email='ma@edx.org', is_staff=False, is_active=True) + self.conditional.runtime = prepare_block_runtime(self.course.runtime) self.conditional.bind_for_student( - self.module_system, user.id ) @@ -380,11 +378,11 @@ class ConditionalBlockStudioTest(XModuleXmlImportTest): } context = create_studio_context(self.conditional, False) - html = self.module_system.render(self.conditional, AUTHOR_VIEW, context).content + html = self.course.runtime.render(self.conditional, AUTHOR_VIEW, context).content assert 'This is a secret HTML' in html context = create_studio_context(self.sequence, True) - html = self.module_system.render(self.conditional, AUTHOR_VIEW, context).content + html = self.course.runtime.render(self.conditional, AUTHOR_VIEW, context).content assert 'This is a secret HTML' not in html def test_non_editable_settings(self): diff --git a/xmodule/tests/test_error_block.py b/xmodule/tests/test_error_block.py index 5df3bcb61f..f80fb905cb 100644 --- a/xmodule/tests/test_error_block.py +++ b/xmodule/tests/test_error_block.py @@ -38,7 +38,7 @@ class TestErrorBlock(SetupTestErrorBlock): self.error_msg ) assert isinstance(descriptor, ErrorBlock) - descriptor.xmodule_runtime = self.system + descriptor.runtime = self.system context_repr = self.system.render(descriptor, STUDENT_VIEW).content assert self.error_msg in context_repr assert repr(self.valid_xml) in context_repr diff --git a/xmodule/tests/test_import.py b/xmodule/tests/test_import.py index 570b484ef0..4dc2404d55 100644 --- a/xmodule/tests/test_import.py +++ b/xmodule/tests/test_import.py @@ -52,9 +52,6 @@ class DummySystem(ImportSystem): # lint-amnesty, pylint: disable=abstract-metho services={'field-data': KvsFieldData(DictKeyValueStore())}, ) - def render_template(self, _template, _context): # lint-amnesty, pylint: disable=method-hidden - raise Exception("Shouldn't be called") - class BaseCourseTestCase(TestCase): '''Make sure block imports work properly, including for malformed inputs''' diff --git a/xmodule/tests/test_library_content.py b/xmodule/tests/test_library_content.py index 09f176ce63..957ed72927 100644 --- a/xmodule/tests/test_library_content.py +++ b/xmodule/tests/test_library_content.py @@ -19,7 +19,7 @@ from xmodule.library_tools import LibraryToolsService from xmodule.modulestore import ModuleStoreEnum from xmodule.modulestore.tests.factories import CourseFactory, LibraryFactory from xmodule.modulestore.tests.utils import MixedSplitTestCase -from xmodule.tests import get_test_system +from xmodule.tests import prepare_block_runtime from xmodule.validation import StudioValidationMessage from xmodule.x_module import AUTHOR_VIEW from xmodule.capa_block import ProblemBlock @@ -58,20 +58,17 @@ class LibraryContentTest(MixedSplitTestCase): """ Bind a block (part of self.course) so we can access student-specific data. """ - module_system = get_test_system(course_id=block.location.course_key) - module_system.descriptor_runtime = block.runtime._descriptor_system # pylint: disable=protected-access - module_system._services['library_tools'] = self.tools # pylint: disable=protected-access + prepare_block_runtime(block.runtime, course_id=block.location.course_key) + block.runtime._runtime_services.update({'library_tools': self.tools}) # lint-amnesty, pylint: disable=protected-access def get_block(descriptor): """Mocks module_system get_block function""" - sub_module_system = get_test_system(course_id=block.location.course_key) - sub_module_system.get_block_for_descriptor = get_block - sub_module_system.descriptor_runtime = descriptor._runtime # pylint: disable=protected-access - descriptor.bind_for_student(sub_module_system, self.user_id) + prepare_block_runtime(descriptor.runtime, course_id=block.location.course_key) + descriptor.runtime.get_block_for_descriptor = get_block + descriptor.bind_for_student(self.user_id) return descriptor - module_system.get_block_for_descriptor = get_block - block.xmodule_runtime = module_system + block.runtime.get_block_for_descriptor = get_block class TestLibraryContentExportImport(LibraryContentTest): @@ -99,7 +96,7 @@ class TestLibraryContentExportImport(LibraryContentTest): # Set the virtual FS to export the olx to. self.export_fs = MemoryFS() - self.lc_block.runtime._descriptor_system.export_fs = self.export_fs # pylint: disable=protected-access + self.lc_block.runtime.export_fs = self.export_fs # pylint: disable=protected-access # Prepare runtime for the import. self.runtime = TestImportSystem(load_error_blocks=True, course_id=self.lc_block.location.course_key) @@ -517,7 +514,7 @@ class TestLibraryContentAnalytics(LibraryContentTest): self.lc_block.refresh_children() self.lc_block = self.store.get_item(self.lc_block.location) self._bind_course_block(self.lc_block) - self.lc_block.xmodule_runtime.publish = self.publisher + self.lc_block.runtime.publish = self.publisher def _assert_event_was_published(self, event_type): """ @@ -571,7 +568,7 @@ class TestLibraryContentAnalytics(LibraryContentTest): with self.store.branch_setting(ModuleStoreEnum.Branch.published_only): self.lc_block = self.store.get_item(self.lc_block.location) self._bind_course_block(self.lc_block) - self.lc_block.xmodule_runtime.publish = self.publisher + self.lc_block.runtime.publish = self.publisher self.test_assigned_event() def test_assigned_descendants(self): @@ -590,7 +587,7 @@ class TestLibraryContentAnalytics(LibraryContentTest): # Reload lc_block and set it up for a student: self.lc_block = self.store.get_item(self.lc_block.location) self._bind_course_block(self.lc_block) - self.lc_block.xmodule_runtime.publish = self.publisher + self.lc_block.runtime.publish = self.publisher # Get the keys of each of our blocks, as they appear in the course: course_usage_main_vertical = self.lc_block.children[0] diff --git a/xmodule/tests/test_library_sourced_block.py b/xmodule/tests/test_library_sourced_block.py index 12acc31d94..5ec6291542 100644 --- a/xmodule/tests/test_library_sourced_block.py +++ b/xmodule/tests/test_library_sourced_block.py @@ -6,7 +6,7 @@ from openedx.core.djangoapps.content_libraries.tests.base import ContentLibrarie from common.djangoapps.student.roles import CourseInstructorRole from xmodule.modulestore.django import modulestore from xmodule.modulestore.tests.factories import CourseFactory, BlockFactory -from xmodule.tests import get_test_system +from xmodule.tests import prepare_block_runtime from xmodule.x_module import STUDENT_VIEW # lint-amnesty, pylint: disable=unused-import @@ -66,7 +66,6 @@ class LibrarySourcedBlockTestCase(ContentLibrariesRestApiTest): """ context = context or {} block = self.store.get_item(block.location) - module_system = get_test_system(block) - module_system.descriptor_runtime = block._runtime # pylint: disable=protected-access - block.bind_for_student(module_system, self.user.id) - return module_system.render(block, view, context).content + prepare_block_runtime(block.runtime) + block.bind_for_student(self.user.id) + return block.runtime.render(block, view, context).content diff --git a/xmodule/tests/test_lti20_unit.py b/xmodule/tests/test_lti20_unit.py index 8923f4b474..2c876f25f0 100644 --- a/xmodule/tests/test_lti20_unit.py +++ b/xmodule/tests/test_lti20_unit.py @@ -370,7 +370,7 @@ class LTI20RESTResultServiceTest(unittest.TestCase): Test that we get a 404 when the supplied user does not exist """ self.setup_system_xblock_mocks_for_lti20_request_test() - self.runtime._services['user'] = StubUserService(user=None) # pylint: disable=protected-access + self.runtime._runtime_services['user'] = StubUserService(user=None) # pylint: disable=protected-access mock_request = self.get_signed_lti20_mock_request(self.GOOD_JSON_PUT) response = self.xblock.lti_2_0_result_rest_handler(mock_request, "user/abcd") assert response.status_code == 404 diff --git a/xmodule/tests/test_lti_unit.py b/xmodule/tests/test_lti_unit.py index 47d480d5bf..76e4fc66e1 100644 --- a/xmodule/tests/test_lti_unit.py +++ b/xmodule/tests/test_lti_unit.py @@ -65,7 +65,7 @@ class LTIBlockTest(TestCase): self.course_id = CourseKey.from_string('org/course/run') self.system = get_test_system(self.course_id) self.system.publish = Mock() - self.system._services['rebind_user'] = Mock() # pylint: disable=protected-access + self.system._runtime_services['rebind_user'] = Mock() # pylint: disable=protected-access self.xblock = LTIBlock( self.system, @@ -178,7 +178,7 @@ class LTIBlockTest(TestCase): """ If we have no real user, we should send back failure response. """ - self.system._services['user'] = StubUserService(user=None) # pylint: disable=protected-access + self.system._runtime_services['user'] = StubUserService(user=None) # pylint: disable=protected-access self.xblock.verify_oauth_body_sign = Mock() self.xblock.has_score = True request = Request(self.environ) diff --git a/xmodule/tests/test_randomize_block.py b/xmodule/tests/test_randomize_block.py index 8848c4dded..d39eed84c4 100644 --- a/xmodule/tests/test_randomize_block.py +++ b/xmodule/tests/test_randomize_block.py @@ -9,7 +9,7 @@ from lxml import etree from xmodule.modulestore.tests.factories import CourseFactory from xmodule.modulestore.tests.utils import MixedSplitTestCase from xmodule.randomize_block import RandomizeBlock -from xmodule.tests import get_test_system +from xmodule.tests import prepare_block_runtime from .test_course_block import DummySystem as TestImportSystem @@ -42,9 +42,7 @@ class RandomizeBlockTest(MixedSplitTestCase): Bind module system to block so we can access student-specific data. """ user = Mock(name='get_test_system.user', id=user_id, is_staff=False) - module_system = get_test_system(course_id=block.location.course_key, user=user) - module_system.descriptor_runtime = block.runtime._descriptor_system # pylint: disable=protected-access - block.xmodule_runtime = module_system + prepare_block_runtime(block.runtime, course_id=block.location.course_key, user=user) def test_xml_export_import_cycle(self): """ @@ -64,7 +62,7 @@ class RandomizeBlockTest(MixedSplitTestCase): export_fs = MemoryFS() # Set the virtual FS to export the olx to. - randomize_block.runtime._descriptor_system.export_fs = export_fs # pylint: disable=protected-access + randomize_block.runtime.export_fs = export_fs # pylint: disable=protected-access # Export the olx. node = etree.Element("unknown_root") diff --git a/xmodule/tests/test_sequence.py b/xmodule/tests/test_sequence.py index 4447bb2aed..9a81ef3280 100644 --- a/xmodule/tests/test_sequence.py +++ b/xmodule/tests/test_sequence.py @@ -19,7 +19,7 @@ from web_fragments.fragment import Fragment from edx_toggles.toggles.testutils import override_waffle_flag from openedx.features.content_type_gating.models import ContentTypeGatingConfig from xmodule.seq_block import TIMED_EXAM_GATING_WAFFLE_FLAG, SequenceBlock -from xmodule.tests import get_test_system +from xmodule.tests import get_test_system, prepare_block_runtime from xmodule.tests.helpers import StubUserService from xmodule.tests.xml import XModuleXmlImportTest from xmodule.tests.xml import factories as xml @@ -94,11 +94,8 @@ class SequenceBlockTestCase(XModuleXmlImportTest): self._set_up_module_system(block) - block.xmodule_runtime._services['bookmarks'] = Mock() # pylint: disable=protected-access - block.xmodule_runtime._services['completion'] = Mock( # pylint: disable=protected-access - return_value=Mock(vertical_is_complete=Mock(return_value=True)) - ) - block.xmodule_runtime._services['user'] = StubUserService(user=Mock()) # pylint: disable=protected-access + block.runtime._runtime_services['bookmarks'] = Mock() # pylint: disable=protected-access + block.runtime._runtime_services['user'] = StubUserService(user=Mock()) # pylint: disable=protected-access block.parent = parent.location return block @@ -106,14 +103,12 @@ class SequenceBlockTestCase(XModuleXmlImportTest): """ Sets up the test module system for the given block. """ - module_system = get_test_system() - module_system.descriptor_runtime = block._runtime # pylint: disable=protected-access - block.xmodule_runtime = module_system + prepare_block_runtime(block.runtime) # The render operation will ask modulestore for the current course to get some data. As these tests were # originally not written to be compatible with a real modulestore, we've mocked out the relevant return values. - module_system.modulestore = Mock() - module_system.modulestore.get_course.return_value = self.course + block.runtime.modulestore = Mock() + block.runtime.modulestore.get_course.return_value = self.course def _get_rendered_view(self, sequence, @@ -130,7 +125,7 @@ class SequenceBlockTestCase(XModuleXmlImportTest): context.update(extra_context) self.course.self_paced = self_paced - return sequence.xmodule_runtime.render(sequence, view, context).content + return sequence.runtime.render(sequence, view, context).content def _assert_view_at_position(self, rendered_html, expected_position): """ @@ -139,10 +134,10 @@ class SequenceBlockTestCase(XModuleXmlImportTest): assert f"'position': {expected_position}" in rendered_html def test_student_view_init(self): - module_system = get_test_system() - module_system.position = 2 - seq_block = SequenceBlock(runtime=module_system, scope_ids=Mock()) - seq_block.bind_for_student(module_system, 34) + runtime = get_test_system() + runtime.position = 2 + seq_block = SequenceBlock(runtime=runtime, scope_ids=Mock()) + seq_block.bind_for_student(34) assert seq_block.position == 2 # matches position set in the runtime @@ -335,7 +330,7 @@ class SequenceBlockTestCase(XModuleXmlImportTest): False, {'url': 'PrereqUrl', 'display_name': 'PrereqSectionName', 'id': 'mockId'} ] - self.sequence_1_2.xmodule_runtime._services['gating'] = gating_mock_1_2 # pylint: disable=protected-access + self.sequence_1_2.runtime._services['gating'] = gating_mock_1_2 # pylint: disable=protected-access self.sequence_1_2.display_name = 'sequence_1_2' html = self._get_rendered_view( @@ -373,6 +368,9 @@ class SequenceBlockTestCase(XModuleXmlImportTest): def test_xblock_handler_get_completion_success(self): """Test that the completion data is returned successfully on targeted vertical through ajax call""" + self.sequence_3_1.runtime._runtime_services['completion'] = Mock( # pylint: disable=protected-access + return_value=Mock(vertical_is_complete=Mock(return_value=True)) + ) for child in self.sequence_3_1.get_children(): usage_key = str(child.location) request = RequestFactory().post( @@ -382,6 +380,7 @@ class SequenceBlockTestCase(XModuleXmlImportTest): ) completion_return = self.sequence_3_1.handle('get_completion', request) assert completion_return.json == {'complete': True} + self.sequence_3_1.runtime._runtime_services['completion'] = None # pylint: disable=protected-access def test_xblock_handler_get_completion_bad_key(self): """Test that the completion data is returned as False when usage key is None through ajax call""" @@ -395,10 +394,14 @@ class SequenceBlockTestCase(XModuleXmlImportTest): def test_handle_ajax_get_completion_success(self): """Test that the old-style ajax handler for completion still works""" + self.sequence_3_1.runtime._runtime_services['completion'] = Mock( # pylint: disable=protected-access + return_value=Mock(vertical_is_complete=Mock(return_value=True)) + ) for child in self.sequence_3_1.get_children(): usage_key = str(child.location) completion_return = self.sequence_3_1.handle_ajax('get_completion', {'usage_key': usage_key}) assert json.loads(completion_return) == {'complete': True} + self.sequence_3_1.runtime._runtime_services['completion'] = None # pylint: disable=protected-access def test_xblock_handler_goto_position_success(self): """Test that we can set position through ajax call""" @@ -434,7 +437,7 @@ class SequenceBlockTestCase(XModuleXmlImportTest): """Test that the sequence metadata is returned correctly""" # rather than dealing with json serialization of the Mock object, # let's just disable the bookmarks service - self.sequence_3_1.xmodule_runtime._services['bookmarks'] = None # lint-amnesty, pylint: disable=protected-access + self.sequence_3_1.runtime._services['bookmarks'] = None # lint-amnesty, pylint: disable=protected-access metadata = self.sequence_3_1.get_metadata() assert len(metadata['items']) == 3 assert metadata['tag'] == 'sequential' @@ -445,7 +448,7 @@ class SequenceBlockTestCase(XModuleXmlImportTest): )) def test_get_metadata_content_type_gated_content(self): """The contains_content_type_gated_content field tells whether the item contains content type gated content""" - self.sequence_5_1.xmodule_runtime._services['bookmarks'] = None # pylint: disable=protected-access + self.sequence_5_1.runtime._services['bookmarks'] = None # pylint: disable=protected-access ContentTypeGatingConfig.objects.create(enabled=True, enabled_as_of=datetime(2018, 1, 1)) metadata = self.sequence_5_1.get_metadata() assert metadata['items'][0]['contains_content_type_gated_content'] is False diff --git a/xmodule/tests/test_split_test_block.py b/xmodule/tests/test_split_test_block.py index 61ddd6f8c2..5de2e15ee7 100644 --- a/xmodule/tests/test_split_test_block.py +++ b/xmodule/tests/test_split_test_block.py @@ -18,7 +18,7 @@ from xmodule.split_test_block import ( get_split_user_partitions, user_partition_values, ) -from xmodule.tests import get_test_system +from xmodule.tests import prepare_block_runtime from xmodule.tests.test_course_block import DummySystem as TestImportSystem from xmodule.tests.xml import XModuleXmlImportTest from xmodule.tests.xml import factories as xml @@ -85,9 +85,9 @@ class SplitTestBlockTest(XModuleXmlImportTest, PartitionTestCase): self.course = self.process_xml(course) self.course_sequence = self.course.get_children()[0] - self.module_system = get_test_system() + user = Mock(username='ma', email='ma@edx.org', is_staff=False, is_active=True) + prepare_block_runtime(self.course.runtime, user=user, add_get_block=True) - self.module_system.descriptor_runtime = self.course._runtime # pylint: disable=protected-access self.course.runtime.export_fs = MemoryFS() # Create mock partition service, as these tests are running with XML in-memory system. @@ -106,19 +106,15 @@ class SplitTestBlockTest(XModuleXmlImportTest, PartitionTestCase): self.course, course_id=self.course.id, ) - self.module_system._services['partitions'] = partitions_service # pylint: disable=protected-access + self.course.runtime._runtime_services['partitions'] = partitions_service # pylint: disable=protected-access # Mock user_service user user_service = Mock() - user = Mock(username='ma', email='ma@edx.org', is_staff=False, is_active=True) user_service._django_user = user # lint-amnesty, pylint: disable=protected-access - self.module_system._services['user'] = user_service # pylint: disable=protected-access self.split_test_block = self.course_sequence.get_children()[0] - self.split_test_block.bind_for_student( - self.module_system, - user.id - ) + self.split_test_block.runtime = self.course.runtime + self.split_test_block.bind_for_student(user.id) # Create mock modulestore for getting the course. Needed for rendering the HTML # view, since mock services exist and the rendering code will not short-circuit. @@ -152,7 +148,7 @@ class SplitTestBlockLMSTest(SplitTestBlockTest): @ddt.unpack def test_get_html(self, user_tag, child_content): self.user_partition.scheme.current_group = self.user_partition.groups[user_tag] - assert child_content in self.module_system.render(self.split_test_block, STUDENT_VIEW).content + assert child_content in self.course.runtime.render(self.split_test_block, STUDENT_VIEW).content @ddt.data(0, 1) def test_child_missing_tag_value(self, _user_tag): @@ -175,7 +171,7 @@ class SplitTestBlockLMSTest(SplitTestBlockTest): # Mock out the process_xml # Expect it to return a child descriptor for the SplitTestDescriptor when called. - self.module_system.process_xml = Mock() + self.course.runtime.process_xml = Mock() # Write out the xml. xml_obj = self.split_test_block.definition_to_xml(MemoryFS()) @@ -184,7 +180,7 @@ class SplitTestBlockLMSTest(SplitTestBlockTest): assert xml_obj.get('group_id_to_child') is not None # Read the xml back in. - fields, children = SplitTestBlock.definition_from_xml(xml_obj, self.module_system) + fields, children = SplitTestBlock.definition_from_xml(xml_obj, self.course.runtime) assert fields.get('user_partition_id') == '0' assert fields.get('group_id_to_child') is not None assert len(children) == 2 @@ -212,13 +208,13 @@ class SplitTestBlockStudioTest(SplitTestBlockTest): # The split_test block should render both its groups when it is the root context = create_studio_context(self.split_test_block) - html = self.module_system.render(self.split_test_block, AUTHOR_VIEW, context).content + html = self.course.runtime.render(self.split_test_block, AUTHOR_VIEW, context).content assert 'HTML FOR GROUP 0' in html assert 'HTML FOR GROUP 1' in html # When rendering as a child, it shouldn't render either of its groups context = create_studio_context(self.course_sequence) - html = self.module_system.render(self.split_test_block, AUTHOR_VIEW, context).content + html = self.course.runtime.render(self.split_test_block, AUTHOR_VIEW, context).content assert 'HTML FOR GROUP 0' not in html assert 'HTML FOR GROUP 1' not in html @@ -228,7 +224,7 @@ class SplitTestBlockStudioTest(SplitTestBlockTest): UserPartition(0, 'first_partition', 'First Partition', [Group("0", 'alpha'), Group("1", 'beta'), Group("2", 'gamma')]) ] - html = self.module_system.render(self.split_test_block, AUTHOR_VIEW, context).content + html = self.course.runtime.render(self.split_test_block, AUTHOR_VIEW, context).content assert 'HTML FOR GROUP 0' in html assert 'HTML FOR GROUP 1' in html @@ -569,7 +565,7 @@ class SplitTestBlockExportImportTest(MixedSplitTestCase): ) export_fs = MemoryFS() # Set the virtual FS to export the olx to. - split_test_block.runtime._descriptor_system.export_fs = export_fs # pylint: disable=protected-access + split_test_block.runtime.export_fs = export_fs # pylint: disable=protected-access # Export the olx. node = lxml.etree.Element("unknown_root") diff --git a/xmodule/tests/test_studio_editable.py b/xmodule/tests/test_studio_editable.py index f697cf8249..a8f9f7d4e1 100644 --- a/xmodule/tests/test_studio_editable.py +++ b/xmodule/tests/test_studio_editable.py @@ -24,6 +24,6 @@ class StudioEditableBlockTestCase(BaseVerticalBlockTest): } # Both children of the vertical should be rendered as reorderable - self.module_system.render(self.vertical, AUTHOR_VIEW, context).content # pylint: disable=expression-not-assigned + self.course.runtime.render(self.vertical, AUTHOR_VIEW, context).content # pylint: disable=expression-not-assigned assert self.vertical.get_children()[0].location in reorderable_items assert self.vertical.get_children()[1].location in reorderable_items diff --git a/xmodule/tests/test_vertical.py b/xmodule/tests/test_vertical.py index 1717354b47..f4012125e2 100644 --- a/xmodule/tests/test_vertical.py +++ b/xmodule/tests/test_vertical.py @@ -18,7 +18,7 @@ from django.test import override_settings from openedx_filters import PipelineStep from openedx_filters.learning.filters import VerticalBlockChildRenderStarted, VerticalBlockRenderCompleted -from . import get_test_system +from . import prepare_block_runtime from .helpers import StubUserService from .xml import XModuleXmlImportTest from .xml import factories as xml @@ -164,13 +164,12 @@ class BaseVerticalBlockTest(XModuleXmlImportTest): self.course = self.process_xml(course) course_seq = self.course.get_children()[0] - self.module_system = get_test_system() + prepare_block_runtime(self.course.runtime) - self.module_system.descriptor_runtime = self.course._runtime self.course.runtime.export_fs = MemoryFS() self.vertical = course_seq.get_children()[0] - self.vertical.xmodule_runtime = self.module_system + self.vertical.runtime = self.course.runtime self.html_block = self.vertical.get_children()[0] self.problem_block = self.vertical.get_children()[1] @@ -213,17 +212,19 @@ class VerticalBlockTestCase(BaseVerticalBlockTest): """ Test the rendering of the student and public view. """ - self.module_system._services['bookmarks'] = Mock() + self.course.runtime._runtime_services['bookmarks'] = Mock() 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['completion'] = StubCompletionService(enabled=True, - completion_value=completion_value) + self.course.runtime._runtime_services['user'] = StubUserService(user=Mock(username=self.username)) + self.course.runtime._runtime_services['completion'] = StubCompletionService( + enabled=True, + completion_value=completion_value + ) elif view == PUBLIC_VIEW: - self.module_system._services['user'] = StubUserService(user=AnonymousUser()) + self.course.runtime._runtime_services['user'] = StubUserService(user=AnonymousUser()) - html = self.module_system.render( + html = self.course.runtime.render( self.vertical, view, self.default_context if context is None else context ).content assert self.test_html in html @@ -248,15 +249,15 @@ class VerticalBlockTestCase(BaseVerticalBlockTest): """ Test the rendering of the student and public view. """ - self.module_system._services['bookmarks'] = Mock() - self.module_system._services['user'] = StubUserService(user=Mock()) - self.module_system._services['completion'] = StubCompletionService(enabled=True, completion_value=0) + self.course.runtime._services['bookmarks'] = Mock() + self.course.runtime._services['user'] = StubUserService(user=Mock()) + self.course.runtime._services['completion'] = StubCompletionService(enabled=True, completion_value=0) now = datetime.now(pytz.UTC) self.vertical.due = now + timedelta(days=-1) self.problem_block.has_score = has_score - html = self.module_system.render(self.vertical, STUDENT_VIEW, self.default_context).content + html = self.course.runtime.render(self.vertical, STUDENT_VIEW, self.default_context).content if has_score: assert "'has_assignments': True" in html assert "'completed': False" in html @@ -270,14 +271,14 @@ class VerticalBlockTestCase(BaseVerticalBlockTest): @ddt.unpack def test_render_access_denied_blocks(self, node_has_access_error, child_has_access_error): """ Tests access denied blocks are not rendered when hide_access_error_blocks is True """ - self.module_system._services['bookmarks'] = Mock() - self.module_system._services['user'] = StubUserService(user=Mock()) + self.course.runtime._services['bookmarks'] = Mock() + self.course.runtime._services['user'] = StubUserService(user=Mock()) self.vertical.due = datetime.now(pytz.UTC) + timedelta(days=-1) self.problem_block.has_access_error = node_has_access_error self.nested_problem_block.has_access_error = child_has_access_error context = {'username': self.username, 'hide_access_error_blocks': True} - html = self.module_system.render(self.vertical, STUDENT_VIEW, context).content + html = self.course.runtime.render(self.vertical, STUDENT_VIEW, context).content if node_has_access_error and child_has_access_error: assert self.test_problem not in html @@ -316,11 +317,11 @@ class VerticalBlockTestCase(BaseVerticalBlockTest): Test that mark-completed-on-view-after-delay is only set for relevant child Xblocks. """ with patch.object(self.html_block, 'render') as mock_student_view: - self.module_system._services['completion'] = StubCompletionService( + self.course.runtime._services['completion'] = StubCompletionService( enabled=completion_enabled, completion_value=completion_value, ) - self.module_system.render(self.vertical, STUDENT_VIEW, self.default_context) + self.course.runtime.render(self.vertical, STUDENT_VIEW, self.default_context) if mark_completed_enabled: assert mock_student_view.call_args[0][1]['wrap_xblock_data']['mark-completed-on-view-after-delay'] ==\ 9876 @@ -335,7 +336,7 @@ class VerticalBlockTestCase(BaseVerticalBlockTest): context = { 'is_unit_page': True } - html = self.module_system.render(self.vertical, AUTHOR_VIEW, context).content + html = self.course.runtime.render(self.vertical, AUTHOR_VIEW, context).content assert self.test_html not in html assert self.test_problem not in html @@ -345,7 +346,7 @@ class VerticalBlockTestCase(BaseVerticalBlockTest): 'is_unit_page': False, 'reorderable_items': reorderable_items, } - html = self.module_system.render(self.vertical, AUTHOR_VIEW, context).content + html = self.course.runtime.render(self.vertical, AUTHOR_VIEW, context).content assert self.test_html in html assert self.test_problem in html @@ -363,11 +364,11 @@ class VerticalBlockTestCase(BaseVerticalBlockTest): """ Test the VerticalBlockChildRenderStarted filter's effects on student view. """ - self.module_system._services['bookmarks'] = Mock() - self.module_system._services['user'] = StubUserService(user=Mock()) - self.module_system._services['completion'] = StubCompletionService(enabled=True, completion_value=0) + self.course.runtime._services['bookmarks'] = Mock() + self.course.runtime._services['user'] = StubUserService(user=Mock()) + self.course.runtime._services['completion'] = StubCompletionService(enabled=True, completion_value=0) - html = self.module_system.render(self.vertical, STUDENT_VIEW, self.default_context).content + html = self.course.runtime.render(self.vertical, STUDENT_VIEW, self.default_context).content assert TestVerticalBlockChildRenderStep.filter_content in html @@ -385,11 +386,11 @@ class VerticalBlockTestCase(BaseVerticalBlockTest): """ Test VerticalBlockChildRenderStarted filter can be used to skip child blocks. """ - self.module_system._services['bookmarks'] = Mock() - self.module_system._services['user'] = StubUserService(user=Mock()) - self.module_system._services['completion'] = StubCompletionService(enabled=True, completion_value=0) + self.course.runtime._services['bookmarks'] = Mock() + self.course.runtime._services['user'] = StubUserService(user=Mock()) + self.course.runtime._services['completion'] = StubCompletionService(enabled=True, completion_value=0) - html = self.module_system.render(self.vertical, STUDENT_VIEW, self.default_context).content + html = self.course.runtime.render(self.vertical, STUDENT_VIEW, self.default_context).content assert self.test_html not in html assert self.test_html_nested not in html @@ -408,11 +409,11 @@ class VerticalBlockTestCase(BaseVerticalBlockTest): """ Test the VerticalBlockRenderCompleted filter's execution. """ - self.module_system._services['bookmarks'] = Mock() - self.module_system._services['user'] = StubUserService(user=Mock()) - self.module_system._services['completion'] = StubCompletionService(enabled=True, completion_value=0) + self.course.runtime._services['bookmarks'] = Mock() + self.course.runtime._services['user'] = StubUserService(user=Mock()) + self.course.runtime._services['completion'] = StubCompletionService(enabled=True, completion_value=0) - html = self.module_system.render(self.vertical, STUDENT_VIEW, self.default_context).content + html = self.course.runtime.render(self.vertical, STUDENT_VIEW, self.default_context).content assert TestVerticalBlockRenderCompletedStep.filter_content in html @@ -430,10 +431,10 @@ class VerticalBlockTestCase(BaseVerticalBlockTest): """ Test VerticalBlockRenderCompleted filter can be used to prevent vertical block from rendering. """ - self.module_system._services['bookmarks'] = Mock() - self.module_system._services['user'] = StubUserService(user=Mock()) - self.module_system._services['completion'] = StubCompletionService(enabled=True, completion_value=0) + self.course.runtime._services['bookmarks'] = Mock() + self.course.runtime._services['user'] = StubUserService(user=Mock()) + self.course.runtime._services['completion'] = StubCompletionService(enabled=True, completion_value=0) - html = self.module_system.render(self.vertical, STUDENT_VIEW, self.default_context).content + html = self.course.runtime.render(self.vertical, STUDENT_VIEW, self.default_context).content assert TestPreventVerticalBlockRenderStep.filter_content == html diff --git a/xmodule/x_module.py b/xmodule/x_module.py index c2fd4e0085..727a77840e 100644 --- a/xmodule/x_module.py +++ b/xmodule/x_module.py @@ -1408,8 +1408,7 @@ class ModuleSystemShim: DeprecationWarning, stacklevel=3, ) if hasattr(self, '_deprecated_course_id'): - return self._deprecated_course_id - return self.descriptor_runtime.course_id.for_branch(None) + return self._deprecated_course_id.for_branch(None) @course_id.setter def course_id(self, course_id): @@ -1479,7 +1478,7 @@ class DescriptorSystem(MetricsMixin, ConfigurableFragmentWrapper, ModuleSystemSh def get_block(self, usage_id, for_parent=None): """See documentation for `xblock.runtime:Runtime.get_block`""" block = self.load_item(usage_id, for_parent=for_parent) - if self.get_block_for_descriptor: + if getattr(self, 'get_block_for_descriptor', None): return self.get_block_for_descriptor(block) return block From c5439221cb22c2180bd463344d3b2cb47204b7b1 Mon Sep 17 00:00:00 2001 From: Kaustav Banerjee Date: Wed, 15 Feb 2023 16:25:37 +0530 Subject: [PATCH 10/13] chore: address review comments --- .../contentstore/tests/test_i18n.py | 4 +- cms/djangoapps/contentstore/views/block.py | 2 +- cms/djangoapps/contentstore/views/preview.py | 38 ++++++------ .../contentstore/views/tests/test_preview.py | 16 ++--- lms/djangoapps/courseware/block_render.py | 61 +++++++++---------- lms/djangoapps/courseware/tests/helpers.py | 2 +- .../courseware/tests/test_block_render.py | 20 +++--- lms/djangoapps/courseware/views/views.py | 2 +- .../tasks_helper/module_state.py | 4 +- .../core/djangoapps/xblock/runtime/runtime.py | 4 +- xmodule/capa/capa_problem.py | 2 +- xmodule/mako_block.py | 7 +-- xmodule/seq_block.py | 4 +- xmodule/services.py | 25 ++++---- xmodule/x_module.py | 30 ++++++--- 15 files changed, 113 insertions(+), 108 deletions(-) diff --git a/cms/djangoapps/contentstore/tests/test_i18n.py b/cms/djangoapps/contentstore/tests/test_i18n.py index 980490b7e4..e38094a2d3 100644 --- a/cms/djangoapps/contentstore/tests/test_i18n.py +++ b/cms/djangoapps/contentstore/tests/test_i18n.py @@ -15,7 +15,7 @@ from xmodule.modulestore.tests.factories import CourseFactory, BlockFactory from xmodule.tests.test_export import PureXBlock from cms.djangoapps.contentstore.tests.utils import AjaxEnabledTestClient -from cms.djangoapps.contentstore.views.preview import _preview_module_system +from cms.djangoapps.contentstore.views.preview import _prepare_runtime_for_preview from common.djangoapps.student.tests.factories import UserFactory from openedx.core.lib.edx_six import get_gettext @@ -70,7 +70,7 @@ class TestXBlockI18nService(ModuleStoreTestCase): self.course = CourseFactory.create() self.field_data = mock.Mock() self.descriptor = BlockFactory(category="pure", parent=self.course) - _preview_module_system( + _prepare_runtime_for_preview( self.request, self.descriptor, self.field_data, diff --git a/cms/djangoapps/contentstore/views/block.py b/cms/djangoapps/contentstore/views/block.py index eb330da26a..7c28f00501 100644 --- a/cms/djangoapps/contentstore/views/block.py +++ b/cms/djangoapps/contentstore/views/block.py @@ -296,7 +296,7 @@ class StudioPermissionsService: def load_services_for_studio(runtime, user): """ Function to set some required services used for XBlock edits and studio_view. - (i.e. whenever we're not loading preview module system.) This is required to make information + (i.e. whenever we're not loading _prepare_runtime_for_preview.) This is required to make information about the current user (especially permissions) available via services as needed. """ services = { diff --git a/cms/djangoapps/contentstore/views/preview.py b/cms/djangoapps/contentstore/views/preview.py index 29900af507..12b00d16e6 100644 --- a/cms/djangoapps/contentstore/views/preview.py +++ b/cms/djangoapps/contentstore/views/preview.py @@ -148,17 +148,17 @@ def preview_layout_asides(block, context, frag, view_name, aside_frag_fns, wrap_ return result -def _preview_module_system(request, descriptor, field_data): +def _prepare_runtime_for_preview(request, block, field_data): """ - Sets properties in the runtime of the specified descriptor that is + Sets properties in the runtime of the specified block that is required for rendering block previews. request: The active django request field_data: Wrapped field data for previews """ - course_id = descriptor.location.course_key - display_name_only = (descriptor.category == 'static_tab') + course_id = block.location.course_key + display_name_only = (block.category == 'static_tab') replace_url_service = ReplaceURLService(course_id=course_id) @@ -198,7 +198,7 @@ def _preview_module_system(request, descriptor, field_data): # the anonymous_user_id to specific courses. These are captured in the # block attribute 'requires_per_student_anonymous_id'. Please note, # the course_id field in AnynomousUserID model is blank if value is None. - if getattr(descriptor, 'requires_per_student_anonymous_id', False): + if getattr(block, 'requires_per_student_anonymous_id', False): preview_anonymous_user_id = anonymous_id_for_user(request.user, None) else: preview_anonymous_user_id = anonymous_id_for_user(request.user, course_id) @@ -220,26 +220,26 @@ def _preview_module_system(request, descriptor, field_data): 'replace_urls': replace_url_service } - descriptor.runtime.get_block_for_descriptor = partial(_load_preview_block, request) - descriptor.runtime.mixins = settings.XBLOCK_MIXINS + block.runtime.get_block_for_descriptor = partial(_load_preview_block, request) + block.runtime.mixins = settings.XBLOCK_MIXINS # Set up functions to modify the fragment produced by student_view - descriptor.runtime.wrappers = wrappers - descriptor.runtime.wrappers_asides = wrappers_asides - descriptor.runtime._runtime_services.update(services) # lint-amnesty, pylint: disable=protected-access + block.runtime.wrappers = wrappers + block.runtime.wrappers_asides = wrappers_asides + block.runtime._runtime_services.update(services) # lint-amnesty, pylint: disable=protected-access # xmodules can check for this attribute during rendering to determine if # they are being rendered for preview (i.e. in Studio) - descriptor.runtime.is_author_mode = True - descriptor.runtime.handler_url_override = handler_url - descriptor.runtime.applicable_aside_types_override = preview_applicable_aside_types - descriptor.runtime.render_child_placeholder = partial( + block.runtime.is_author_mode = True + block.runtime.handler_url_override = handler_url + block.runtime.applicable_aside_types_override = preview_applicable_aside_types + block.runtime.render_child_placeholder = partial( render_child_placeholder, - wrap_block=descriptor.runtime.wrap_xblock + wrap_block=block.runtime.wrap_xblock ) - descriptor.runtime.layout_asides_override = partial( + block.runtime.layout_asides_override = partial( preview_layout_asides, - wrap_aside=descriptor.runtime.wrap_aside + wrap_aside=block.runtime.wrap_aside ) @@ -270,9 +270,9 @@ def _load_preview_block(request, descriptor): else: wrapper = partial(LmsFieldData, student_data=student_data) - # wrap the _field_data upfront to pass to _preview_module_system + # wrap the _field_data upfront to pass to _prepare_runtime_for_preview wrapped_field_data = wrapper(descriptor._field_data) # pylint: disable=protected-access - _preview_module_system(request, descriptor, wrapped_field_data) + _prepare_runtime_for_preview(request, descriptor, wrapped_field_data) descriptor.bind_for_student( request.user.id, diff --git a/cms/djangoapps/contentstore/views/tests/test_preview.py b/cms/djangoapps/contentstore/views/tests/test_preview.py index 944cb4b9bc..c8d1266853 100644 --- a/cms/djangoapps/contentstore/views/tests/test_preview.py +++ b/cms/djangoapps/contentstore/views/tests/test_preview.py @@ -26,7 +26,7 @@ from cms.djangoapps.xblock_config.models import StudioConfig 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 _prepare_runtime_for_preview, get_preview_fragment @ddt.ddt @@ -214,7 +214,7 @@ class StudioXBlockServiceBindingTest(ModuleStoreTestCase): Tests that the 'user' and 'i18n' services are provided by the Studio runtime. """ descriptor = BlockFactory(category="pure", parent=self.course) - _preview_module_system( + _prepare_runtime_for_preview( self.request, descriptor, self.field_data, @@ -246,9 +246,9 @@ class CmsModuleSystemShimTest(ModuleStoreTestCase): self.field_data = mock.Mock() self.contentstore = contentstore() self.descriptor = BlockFactory(category="problem", parent=course) - _preview_module_system( + _prepare_runtime_for_preview( self.request, - descriptor=self.descriptor, + block=self.descriptor, field_data=mock.Mock(), ) self.course = self.store.get_item(course.location) @@ -305,9 +305,9 @@ class CmsModuleSystemShimTest(ModuleStoreTestCase): """Test anonymous_user_id on a block which uses per-student anonymous IDs""" # Create the runtime with the flag turned on. descriptor = BlockFactory(category="problem", parent=self.course) - _preview_module_system( + _prepare_runtime_for_preview( self.request, - descriptor=descriptor, + block=descriptor, field_data=mock.Mock(), ) assert descriptor.runtime.anonymous_student_id == '26262401c528d7c4a6bbeabe0455ec46' @@ -317,9 +317,9 @@ class CmsModuleSystemShimTest(ModuleStoreTestCase): """Test anonymous_user_id on a block which uses per-course anonymous IDs""" # Create the runtime with the flag turned on. descriptor = BlockFactory(category="lti", parent=self.course) - _preview_module_system( + _prepare_runtime_for_preview( self.request, - descriptor=descriptor, + block=descriptor, field_data=mock.Mock(), ) assert descriptor.runtime.anonymous_student_id == 'ad503f629b55c531fed2e45aa17a3368' diff --git a/lms/djangoapps/courseware/block_render.py b/lms/djangoapps/courseware/block_render.py index fb28034aaa..6cf435f5a0 100644 --- a/lms/djangoapps/courseware/block_render.py +++ b/lms/djangoapps/courseware/block_render.py @@ -403,11 +403,11 @@ def get_block_for_descriptor(user, request, descriptor, field_data_cache, course ) -def get_module_system_for_user( +def prepare_runtime_for_user( user, student_data, # TODO # Arguments preceding this comment have user binding, those following don't - descriptor, + block, course_id, track_function, request_token, @@ -421,14 +421,13 @@ def get_module_system_for_user( will_recheck_access=False, ): """ - Helper function that returns a module system and student_data bound to a user and a descriptor. + Helper function that binds the given xblock to a user and student_data to a user and the block. The purpose of this function is to factor out everywhere a user is implicitly bound when creating a module, - to allow an existing block to be re-bound to a user. Most of the user bindings happen when creating the - closures that feed the instantiation of ModuleSystem. + to allow an existing block to be re-bound to a user. The arguments fall into two categories: those that have explicit or implicit user binding, which are user - and student_data, and those don't and are just present so that ModuleSystem can be instantiated, which + and student_data, and those don't and are used to instantiate the service required in LMS, which are all the other arguments. Ultimately, this isn't too different than how get_block_for_descriptor_internal was before refactoring. @@ -437,7 +436,7 @@ def get_module_system_for_user( request_token (str): A token unique to the request use by xblock initialization Returns: - KvsFieldData: student_data bound to, primarily, the user and descriptor + KvsFieldData: student_data bound to, primarily, the user and block """ def make_xqueue_callback(dispatch='score_update'): @@ -449,7 +448,7 @@ def get_module_system_for_user( kwargs=dict( course_id=str(course_id), userid=str(user.id), - mod_id=str(descriptor.location), + mod_id=str(block.location), dispatch=dispatch ), ) @@ -459,7 +458,7 @@ def get_module_system_for_user( # Default queuename is course-specific and is derived from the course that # contains the current block. # TODO: Queuename should be derived from 'course_settings.json' of each course - xqueue_default_queuename = descriptor.location.org + '-' + descriptor.location.course + xqueue_default_queuename = block.location.org + '-' + block.location.course xqueue_service = XQueueService( construct_callback=make_xqueue_callback, @@ -500,12 +499,12 @@ def get_module_system_for_user( # 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): + if getattr(block, '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_is_staff = bool(has_access(user, 'staff', block.location, course_id)) user_service = DjangoXBlockUserService( user, user_is_staff=user_is_staff, @@ -518,7 +517,7 @@ def get_module_system_for_user( rebind_user_service = RebindUserService( user, course_id, - get_module_system_for_user, + prepare_runtime_for_user, track_function=track_function, position=position, wrap_xblock_display=wrap_xblock_display, @@ -552,9 +551,9 @@ def get_module_system_for_user( )) replace_url_service = ReplaceURLService( - data_directory=getattr(descriptor, 'data_dir', None), + data_directory=getattr(block, 'data_dir', None), course_id=course_id, - static_asset_path=static_asset_path or descriptor.static_asset_path, + static_asset_path=static_asset_path or block.static_asset_path, jump_to_id_base_url=reverse('jump_to_id', kwargs={'course_id': str(course_id), 'module_id': ''}) ) @@ -578,11 +577,11 @@ def get_module_system_for_user( del user.real_user.masquerade_settings user.real_user.masquerade_settings = masquerade_settings else: - staff_access = has_access(user, 'staff', descriptor, course_id) + staff_access = has_access(user, 'staff', block, course_id) if staff_access: block_wrappers.append(partial(add_staff_markup, user, disable_staff_debug_info)) - field_data = DateLookupFieldData(descriptor._field_data, course_id, user) # pylint: disable=protected-access + field_data = DateLookupFieldData(block._field_data, course_id, user) # pylint: disable=protected-access field_data = LmsFieldData(field_data, student_data) store = modulestore() @@ -621,17 +620,15 @@ def get_module_system_for_user( 'publish': EventPublishingService(user, course_id, track_function), } - descriptor.runtime.get_block_for_descriptor = inner_get_block + block.runtime.get_block_for_descriptor = inner_get_block - # TODO: When we merge the descriptor and module systems, we can stop reaching into the mixologist (cpennington) - descriptor.runtime.mixins = descriptor.runtime.mixologist._mixins # lint-amnesty, pylint: disable=protected-access - descriptor.runtime.wrappers = block_wrappers - descriptor.runtime._runtime_services.update(services) # lint-amnesty, pylint: disable=protected-access - descriptor.runtime.request_token = request_token - descriptor.runtime.wrap_asides_override = lms_wrappers_aside - descriptor.runtime.applicable_aside_types_override = lms_applicable_aside_types + block.runtime.wrappers = block_wrappers + block.runtime._runtime_services.update(services) # lint-amnesty, pylint: disable=protected-access + block.runtime.request_token = request_token + block.runtime.wrap_asides_override = lms_wrappers_aside + block.runtime.applicable_aside_types_override = lms_applicable_aside_types - # pass position specified in URL to module through ModuleSystem + # pass position specified in URL to runtime if position is not None: try: position = int(position) @@ -639,12 +636,12 @@ def get_module_system_for_user( log.exception('Non-integer %r passed as position.', position) position = None - descriptor.runtime.set('position', position) + block.runtime.set('position', position) - descriptor.runtime.set('user_is_staff', user_is_staff) - descriptor.runtime.set('user_is_admin', bool(has_access(user, 'staff', 'global'))) - descriptor.runtime.set('user_is_beta_tester', CourseBetaTesterRole(course_id).has_user(user)) - descriptor.runtime.set('days_early_for_beta', descriptor.days_early_for_beta) + block.runtime.set('user_is_staff', user_is_staff) + block.runtime.set('user_is_admin', bool(has_access(user, 'staff', 'global'))) + block.runtime.set('user_is_beta_tester', CourseBetaTesterRole(course_id).has_user(user)) + block.runtime.set('days_early_for_beta', block.days_early_for_beta) return field_data @@ -664,10 +661,10 @@ def get_block_for_descriptor_internal(user, descriptor, student_data, course_id, request_token (str): A unique token for this request, used to isolate xblock rendering """ - student_data = get_module_system_for_user( + student_data = prepare_runtime_for_user( user=user, student_data=student_data, # These have implicit user bindings, the rest of args are considered not to - descriptor=descriptor, + block=descriptor, course_id=course_id, track_function=track_function, position=position, diff --git a/lms/djangoapps/courseware/tests/helpers.py b/lms/djangoapps/courseware/tests/helpers.py index 5f8eadf8a3..99dbf75250 100644 --- a/lms/djangoapps/courseware/tests/helpers.py +++ b/lms/djangoapps/courseware/tests/helpers.py @@ -68,7 +68,7 @@ class BaseTestXmodule(ModuleStoreTestCase): def new_module_runtime(self, runtime=None, **kwargs): """ - Generate a new ModuleSystem that is minimally set up for testing + Generate a new DescriptorSystem that is minimally set up for testing """ if runtime: return prepare_block_runtime(runtime, course_id=self.course.id, **kwargs) diff --git a/lms/djangoapps/courseware/tests/test_block_render.py b/lms/djangoapps/courseware/tests/test_block_render.py index 36a674dff1..37c3848671 100644 --- a/lms/djangoapps/courseware/tests/test_block_render.py +++ b/lms/djangoapps/courseware/tests/test_block_render.py @@ -2270,7 +2270,7 @@ class LMSXBlockServiceMixin(SharedModuleStoreTestCase): """ Instantiate the runtem. """ - _ = render.get_module_system_for_user( + _ = render.prepare_runtime_for_user( self.user, self.student_data, self.descriptor, @@ -2670,7 +2670,7 @@ class LmsModuleSystemShimTest(SharedModuleStoreTestCase): self.track_function = Mock() self.request_token = Mock() self.contentstore = contentstore() - _ = render.get_module_system_for_user( + _ = render.prepare_runtime_for_user( self.user, self.student_data, self.descriptor, @@ -2694,7 +2694,7 @@ class LmsModuleSystemShimTest(SharedModuleStoreTestCase): @patch('lms.djangoapps.courseware.block_render.has_access', Mock(return_value=True, autospec=True)) def test_user_is_staff(self): - _ = render.get_module_system_for_user( + _ = render.prepare_runtime_for_user( self.user, self.student_data, self.descriptor, @@ -2708,7 +2708,7 @@ class LmsModuleSystemShimTest(SharedModuleStoreTestCase): @patch('lms.djangoapps.courseware.block_render.get_user_role', Mock(return_value='instructor', autospec=True)) def test_get_user_role(self): - _ = render.get_module_system_for_user( + _ = render.prepare_runtime_for_user( self.user, self.student_data, self.descriptor, @@ -2724,10 +2724,10 @@ class LmsModuleSystemShimTest(SharedModuleStoreTestCase): 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 + Verifies that subsequent calls to prepare_runtime_for_user have no effect on each block runtime's anonymous_student_id value. """ - _ = render.get_module_system_for_user( + _ = render.prepare_runtime_for_user( self.user, self.student_data, self.problem_descriptor, @@ -2739,7 +2739,7 @@ class LmsModuleSystemShimTest(SharedModuleStoreTestCase): # Ensure the problem block returns a per-user anonymous id assert self.problem_descriptor.runtime.anonymous_student_id == anonymous_id_for_user(self.user, None) - _ = render.get_module_system_for_user( + _ = render.prepare_runtime_for_user( self.user, self.student_data, self.descriptor, @@ -2755,7 +2755,7 @@ class LmsModuleSystemShimTest(SharedModuleStoreTestCase): assert self.problem_descriptor.runtime.anonymous_student_id == anonymous_id_for_user(self.user, None) def test_user_service_with_anonymous_user(self): - _ = render.get_module_system_for_user( + _ = render.prepare_runtime_for_user( AnonymousUser(), self.student_data, self.descriptor, @@ -2771,7 +2771,7 @@ class LmsModuleSystemShimTest(SharedModuleStoreTestCase): assert not self.descriptor.runtime.get_user_role() def test_get_real_user(self): - _ = render.get_module_system_for_user( + _ = render.prepare_runtime_for_user( self.user, self.student_data, self.descriptor, @@ -2816,7 +2816,7 @@ class LmsModuleSystemShimTest(SharedModuleStoreTestCase): XQUEUE_WAITTIME_BETWEEN_REQUESTS=15, ) def test_xqueue_settings(self): - _ = render.get_module_system_for_user( + _ = render.prepare_runtime_for_user( self.user, self.student_data, self.descriptor, diff --git a/lms/djangoapps/courseware/views/views.py b/lms/djangoapps/courseware/views/views.py index 79d2976d2c..bd549324fc 100644 --- a/lms/djangoapps/courseware/views/views.py +++ b/lms/djangoapps/courseware/views/views.py @@ -1487,7 +1487,7 @@ def enclosing_sequence_for_gating_checks(block): if ancestor: # get_parent() returns a parent block instance cached on the block which does not - # have the ModuleSystem bound to it so we need to get it again with get_block() which will set up everything. + # have user data bound to it so we need to get it again with get_block() which will set up everything. return block.runtime.get_block(ancestor.location) return None diff --git a/lms/djangoapps/instructor_task/tasks_helper/module_state.py b/lms/djangoapps/instructor_task/tasks_helper/module_state.py index ddec49cf8a..ec7d8b89d4 100644 --- a/lms/djangoapps/instructor_task/tasks_helper/module_state.py +++ b/lms/djangoapps/instructor_task/tasks_helper/module_state.py @@ -351,7 +351,7 @@ def _get_module_instance_for_task(course_id, student, module_descriptor, xblock_ ''' Make a tracking function that logs what happened. - For insertion into ModuleSystem, and used by CapaModule, which will + For insertion into runtime, and used by CapaModule, which will provide the event_type (as string) and event (as dict) as arguments. The request_info and task_info (and page) are provided here. ''' @@ -375,7 +375,7 @@ def _get_track_function_for_task(student, xblock_instance_args=None, source_page """ Make a tracking function that logs what happened. - For insertion into ModuleSystem, and used by CapaModule, which will + For insertion into runtime, and used by CapaModule, which will provide the event_type (as string) and event (as dict) as arguments. The request_info and task_info (and page) are provided here. """ diff --git a/openedx/core/djangoapps/xblock/runtime/runtime.py b/openedx/core/djangoapps/xblock/runtime/runtime.py index cadc7f4e33..132e3b9728 100644 --- a/openedx/core/djangoapps/xblock/runtime/runtime.py +++ b/openedx/core/djangoapps/xblock/runtime/runtime.py @@ -256,13 +256,13 @@ class XBlockRuntime(RuntimeShim, Runtime): elif service_name == 'replace_urls': return ReplaceURLService(xblock=block, lookup_asset_url=self._lookup_asset_url) elif service_name == 'rebind_user': - # this service should ideally be initialized with all the arguments of get_module_system_for_user + # this service should ideally be initialized with all the arguments of prepare_runtime_for_user # but only the positional arguments are passed here as the other arguments are too # specific to the lms.block_render module return RebindUserService( self.user, context_key, - block_render.get_module_system_for_user, + block_render.prepare_runtime_for_user, track_function=make_track_function(), request_token=request_token(crum.get_current_request()), ) diff --git a/xmodule/capa/capa_problem.py b/xmodule/capa/capa_problem.py index 7a6592e095..9dd17cc253 100644 --- a/xmodule/capa/capa_problem.py +++ b/xmodule/capa/capa_problem.py @@ -93,7 +93,7 @@ class LoncapaSystem(object): i18n: an object implementing the `gettext.Translations` interface so that we can use `.ugettext` to localize strings. - See :class:`ModuleSystem` for documentation of other attributes. + See :class:`DescriptorSystem` for documentation of other attributes. """ def __init__( diff --git a/xmodule/mako_block.py b/xmodule/mako_block.py index 017b277e03..6e6a789104 100644 --- a/xmodule/mako_block.py +++ b/xmodule/mako_block.py @@ -19,12 +19,11 @@ class MakoDescriptorSystem(DescriptorSystem): # lint-amnesty, pylint: disable=a # Add the MakoService to the descriptor system. # - # This is not needed by most XBlocks, because they are initialized with a full runtime ModuleSystem that already - # has the MakoService. - # However, there are a few cases where the XBlock only has the descriptor system instead of the full module + # This is not needed by most XBlocks, because the MakoService is added to their runtimes. + # However, there are a few cases where the MakoService is not added to the XBlock's # runtime. Specifically: # * in the Instructor Dashboard bulk emails tab, when rendering the HtmlBlock for its WYSIWYG editor. - # * during testing, when using the ModuleSystemTestCase to fetch factory-created blocks. + # * during testing, when fetching factory-created blocks. from common.djangoapps.edxmako.services import MakoService self._services['mako'] = MakoService() diff --git a/xmodule/seq_block.py b/xmodule/seq_block.py index eab754ba06..0d8be411fd 100644 --- a/xmodule/seq_block.py +++ b/xmodule/seq_block.py @@ -302,8 +302,8 @@ class SequenceBlock( def bind_for_student(self, user_id, wrappers=None): # The position of the child XBlock to select can also be passed in via the URL. - # In such cases the value is set on the ModuleSystem in get_module_system_for_user() - # and needs to be read here after the ModuleSystem has been set on the XBlock. + # In such cases the value is set in the runtime inside prepare_runtime_for_user() + # and needs to be read here after the value has been set. super().bind_for_student(user_id, wrappers) # If position is specified in system, then use that instead. position = getattr(self.runtime, 'position', None) diff --git a/xmodule/services.py b/xmodule/services.py index d53d72fc32..c4d00a7390 100644 --- a/xmodule/services.py +++ b/xmodule/services.py @@ -147,9 +147,8 @@ class RebindUserService(Service): """ An XBlock Service that allows modules to get rebound to real users if it was previously bound to an AnonymousUser. - This used to be a local function inside the `lms.djangoapps.courseware.block_render.get_module_system_for_user` - method, and was passed as a constructor argument to x_module.ModuleSystem. This has been refactored out into a - service to simplify the ModuleSystem and lives in this module temporarily. + This used to be a local function inside the `lms.djangoapps.courseware.block_render.prepare_runtime_for_user` + method. This has been refactored out into a service and lives in this module temporarily. TODO: Only the old LTI XBlock uses it in 2 places for LTI 2.0 integration. As the LTI XBlock is deprecated in favour of the LTI Consumer XBlock, this should be removed when the LTI XBlock is removed. @@ -158,18 +157,18 @@ class RebindUserService(Service): user (User) - A Django User object course_id (str) - Course ID course (Course) - Course Object - get_module_system_for_user (function) - The helper function that will be called to create a module system + prepare_runtime_for_user (function) - The helper function that will be called to create a module system for a specfic user. This is the parent function from which this service was reactored out. - `lms.djangoapps.courseware.block_render.get_module_system_for_user` - kwargs (dict) - all the keyword arguments that need to be passed to the `get_module_system_for_user` + `lms.djangoapps.courseware.block_render.prepare_runtime_for_user` + kwargs (dict) - all the keyword arguments that need to be passed to the `prepare_runtime_for_user` function when it is called during rebinding """ - def __init__(self, user, course_id, get_module_system_for_user, **kwargs): + def __init__(self, user, course_id, prepare_runtime_for_user, **kwargs): super().__init__(**kwargs) self.user = user self.course_id = course_id self._ref = { - "get_module_system_for_user": get_module_system_for_user + "prepare_runtime_for_user": prepare_runtime_for_user } self._kwargs = kwargs @@ -202,10 +201,10 @@ class RebindUserService(Service): with modulestore().bulk_operations(self.course_id): course = modulestore().get_course(course_key=self.course_id) - inner_student_data = self._ref["get_module_system_for_user"]( + inner_student_data = self._ref["prepare_runtime_for_user"]( user=real_user, student_data=student_data_real_user, # These have implicit user bindings, rest of args considered not to - descriptor=block, + block=block, course_id=self.course_id, course=course, **self._kwargs @@ -221,16 +220,14 @@ class RebindUserService(Service): ) block.scope_ids = block.scope_ids._replace(user_id=real_user.id) - # now bind the module to the new ModuleSystem instance and vice-versa - block.runtime.xmodule_instance = block class EventPublishingService(Service): """ An XBlock Service that allows XModules to publish events (e.g. grading, completion). - We have separated it from the ModuleSystem to be able to alter its behavior when using a different context: - LMS, Studio, or Instructor tasks. + We have implemented it as a seperate service to be able to alter its behavior when using + a different context: LMS, Studio, or Instructor tasks. """ def __init__(self, user, course_id, track_function, **kwargs): super().__init__(**kwargs) diff --git a/xmodule/x_module.py b/xmodule/x_module.py index 727a77840e..031a895b0f 100644 --- a/xmodule/x_module.py +++ b/xmodule/x_module.py @@ -336,13 +336,19 @@ class XModuleMixin(XModuleFields, XBlock): 'xmodule_runtime property is deprecated. Please use the runtime property instead.', DeprecationWarning, stacklevel=3, ) - return self._runtime + return self.runtime @property def system(self): """ Return the XBlock runtime (backwards compatibility alias provided for XModules). + + Deprecated in favor of the runtime property. """ + warnings.warn( + 'system property is deprecated. Please use the runtime property instead.', + DeprecationWarning, stacklevel=3, + ) return self.runtime @property @@ -619,8 +625,8 @@ class XModuleMixin(XModuleFields, XBlock): # Skip rebinding if we're already bound a user, and it's this user. if self.scope_ids.user_id is not None and user_id == self.scope_ids.user_id: - if getattr(self._runtime, 'position', None): - self.position = self._runtime.position # update the position of the tab + if getattr(self.runtime, 'position', None): + self.position = self.runtime.position # update the position of the tab return # If we are switching users mid-request, save the data from the old user. @@ -1017,7 +1023,7 @@ def descriptor_global_local_resource_url(block, uri): class MetricsMixin: """ - Mixin for adding metric logging for render and handle methods in the DescriptorSystem and ModuleSystem. + Mixin for adding metric logging for render and handle methods in the DescriptorSystem. """ def render(self, block, view_name, context=None): # lint-amnesty, pylint: disable=missing-function-docstring @@ -1478,6 +1484,9 @@ class DescriptorSystem(MetricsMixin, ConfigurableFragmentWrapper, ModuleSystemSh def get_block(self, usage_id, for_parent=None): """See documentation for `xblock.runtime:Runtime.get_block`""" block = self.load_item(usage_id, for_parent=for_parent) + # get_block_for_descriptor property is used to bind additional data such as user data + # to the XBlock and to check if the user has access to the block as may be required for + # the LMS or Preview. if getattr(self, 'get_block_for_descriptor', None): return self.get_block_for_descriptor(block) return block @@ -1515,10 +1524,9 @@ class DescriptorSystem(MetricsMixin, ConfigurableFragmentWrapper, ModuleSystemSh return result def handler_url(self, block, handler_name, suffix='', query='', thirdparty=False): - # Currently, Modulestore is responsible for instantiating DescriptorSystems - # This means that LMS/CMS don't have a way to define a subclass of DescriptorSystem - # that implements the correct handler url. So, for now, instead, we will reference a - # global function that the application can override. + # When the Modulestore instantiates DescriptorSystems, we will reference a + # global function that the application can override, unless a specific function is + # defined for LMS/CMS through the handler_url_override property. if getattr(self, 'handler_url_override', None): return self.handler_url_override(block, handler_name, suffix, query, thirdparty) return descriptor_global_handler_url(block, handler_name, suffix, query, thirdparty) @@ -1537,6 +1545,8 @@ class DescriptorSystem(MetricsMixin, ConfigurableFragmentWrapper, ModuleSystemSh """ See :meth:`xblock.runtime.Runtime:applicable_aside_types` for documentation. """ + # applicable_aside_types_override property can be used by LMS/CMS to define specific filters + # and conditions as may be applicable. if getattr(self, 'applicable_aside_types_override', None): return self.applicable_aside_types_override(block, applicable_aside_types=super().applicable_aside_types) @@ -1588,11 +1598,13 @@ class DescriptorSystem(MetricsMixin, ConfigurableFragmentWrapper, ModuleSystemSh return service def wrap_aside(self, block, aside, view, frag, context): + # LMS/CMS can define custom wrap aside using wrap_asides_override as required. if getattr(self, 'wrap_asides_override', None): return self.wrap_asides_override(block, aside, view, frag, context, request_token=self.request_token) return super().wrap_aside(block, aside, view, frag, context) def layout_asides(self, block, context, frag, view_name, aside_frag_fns): + # LMS/CMS can define custom layout aside using layout_asides_override as required. if getattr(self, 'layout_asides_override', None): return self.layout_asides_override(block, context, frag, view_name, aside_frag_fns) return super().layout_asides(block, context, frag, view_name, aside_frag_fns) @@ -1715,7 +1727,7 @@ class XMLParsingSystem(DescriptorSystem): # lint-amnesty, pylint: disable=abstr class DoNothingCache: - """A duck-compatible object to use in ModuleSystem when there's no cache.""" + """A duck-compatible object to use in ModuleSystemShim when there's no cache.""" def get(self, _key): return None From 522c48c137df489aa874068f85b3c63f13f0e99a Mon Sep 17 00:00:00 2001 From: Agrendalath Date: Wed, 22 Mar 2023 18:56:08 +0100 Subject: [PATCH 11/13] refactor: do not rely on `LmsModuleSystem` in Video XBlock It was introduced in 159b707. We are removing this class. This also adds a missing test case. --- lms/djangoapps/courseware/tests/test_video_mongo.py | 6 ++++-- xmodule/video_block/video_block.py | 10 ++-------- 2 files changed, 6 insertions(+), 10 deletions(-) diff --git a/lms/djangoapps/courseware/tests/test_video_mongo.py b/lms/djangoapps/courseware/tests/test_video_mongo.py index 88fb132e6b..9887c10059 100644 --- a/lms/djangoapps/courseware/tests/test_video_mongo.py +++ b/lms/djangoapps/courseware/tests/test_video_mongo.py @@ -243,14 +243,16 @@ class TestVideoPublicAccess(BaseTestVideoXBlock): @ddt.data( (True, False), (False, False), + (False, True), (True, True), ) @ddt.unpack def test_public_video_url(self, is_lms_platform, enable_public_share): """Test public video url.""" assert self.item_descriptor.public_access is True - with patch.object(self.item_descriptor, '_is_lms_platform', return_value=is_lms_platform), \ - patch.object(PUBLIC_VIDEO_SHARE, 'is_enabled', return_value=enable_public_share): + if not is_lms_platform: + self.item_descriptor.runtime.is_author_mode = True + with patch.object(PUBLIC_VIDEO_SHARE, 'is_enabled', return_value=enable_public_share): context = self.item_descriptor.render(STUDENT_VIEW).content # public video url iif PUBLIC_VIDEO_SHARE waffle and is_lms_platform, public_access are true assert bool(get_context_dict_from_string(context)['public_video_url']) \ diff --git a/xmodule/video_block/video_block.py b/xmodule/video_block/video_block.py index 1b9b441f37..772a2fbcd1 100644 --- a/xmodule/video_block/video_block.py +++ b/xmodule/video_block/video_block.py @@ -30,7 +30,6 @@ from xblock.completable import XBlockCompletionMode from xblock.core import XBlock from xblock.fields import ScopeIds from xblock.runtime import KvsFieldData -from lms.djangoapps.lms_xblock.runtime import LmsModuleSystem from common.djangoapps.xblock_django.constants import ATTR_KEY_REQUEST_COUNTRY_CODE from openedx.core.djangoapps.video_config.models import HLSPlaybackEnabledFlag, CourseYoutubeBlockedFlag @@ -481,17 +480,12 @@ class VideoBlock( return self.runtime.service(self, 'mako').render_template('video.html', template_context) - def _is_lms_platform(self): - """ - Returns True if the platform is LMS. - """ - return isinstance(self.xmodule_runtime, LmsModuleSystem) - def _get_public_video_url(self): """ Returns the public video url """ - if self.public_access and self._is_lms_platform(): + is_studio = getattr(self.runtime, "is_author_mode", False) + if self.public_access and not is_studio: from openedx.core.djangoapps.video_config.toggles import PUBLIC_VIDEO_SHARE if PUBLIC_VIDEO_SHARE.is_enabled(self.location.course_key): return urljoin( From c74f3825e8668a7eeaf88f026ae7e4be0a64b83a Mon Sep 17 00:00:00 2001 From: Agrendalath Date: Thu, 23 Mar 2023 12:55:19 +0100 Subject: [PATCH 12/13] test: remove `only_xmodules` This function was only used by tests. It was breaking tests from the `xmodule` package on the devstack because the dist.key of its entry_points is `open-edx`. --- xmodule/modulestore/__init__.py | 7 ------- xmodule/tests/test_import.py | 2 -- xmodule/tests/xml/factories.py | 2 -- 3 files changed, 11 deletions(-) diff --git a/xmodule/modulestore/__init__.py b/xmodule/modulestore/__init__.py index d02d62e142..5077480c58 100644 --- a/xmodule/modulestore/__init__.py +++ b/xmodule/modulestore/__init__.py @@ -1386,10 +1386,3 @@ class ModuleStoreWriteBase(ModuleStoreReadBase, ModuleStoreWrite): """ if self.signal_handler: self.signal_handler.send("item_deleted", usage_key=usage_key, user_id=user_id) - - -def only_xmodules(identifier, entry_points): - """Only use entry_points that are supplied by the xmodule package""" - from_xmodule = [entry_point for entry_point in entry_points if entry_point.dist.key == 'xmodule'] - - return default_select(identifier, from_xmodule) diff --git a/xmodule/tests/test_import.py b/xmodule/tests/test_import.py index 4dc2404d55..0c66f68ad9 100644 --- a/xmodule/tests/test_import.py +++ b/xmodule/tests/test_import.py @@ -17,7 +17,6 @@ from xblock.fields import Integer, Scope, String from xblock.runtime import DictKeyValueStore, KvsFieldData from xmodule.fields import Date -from xmodule.modulestore import only_xmodules from xmodule.modulestore.inheritance import InheritanceMixin, compute_inherited_metadata from xmodule.modulestore.xml import ImportSystem, LibraryXMLModuleStore, XMLModuleStore from xmodule.tests import DATA_DIR @@ -69,7 +68,6 @@ class BaseCourseTestCase(TestCase): DATA_DIR, source_dirs=[name], xblock_mixins=(InheritanceMixin,), - xblock_select=only_xmodules, ) courses = modulestore.get_courses() assert len(courses) == 1 diff --git a/xmodule/tests/xml/factories.py b/xmodule/tests/xml/factories.py index 7409b0ac6d..0c183ebd72 100644 --- a/xmodule/tests/xml/factories.py +++ b/xmodule/tests/xml/factories.py @@ -11,7 +11,6 @@ from fs.osfs import OSFS from lxml import etree from xblock.mixins import HierarchyMixin -from xmodule.modulestore import only_xmodules from xmodule.modulestore.inheritance import InheritanceMixin from xmodule.x_module import XModuleMixin @@ -72,7 +71,6 @@ class XmlImportFactory(Factory): filesystem = OSFS(mkdtemp()) xblock_mixins = (InheritanceMixin, XModuleMixin, HierarchyMixin) - xblock_select = only_xmodules url_name = Sequence(str) attribs = {} policy = {} From 44a960f04dc8e3a91ba3280711491d6ef6079c52 Mon Sep 17 00:00:00 2001 From: Agrendalath Date: Fri, 21 Apr 2023 14:10:09 +0200 Subject: [PATCH 13/13] chore: update DnDv2 XBlock --- requirements/edx/base.txt | 2 +- requirements/edx/development.txt | 2 +- requirements/edx/testing.txt | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/requirements/edx/base.txt b/requirements/edx/base.txt index e71a1b77a1..be6448097f 100644 --- a/requirements/edx/base.txt +++ b/requirements/edx/base.txt @@ -1186,7 +1186,7 @@ xblock[django]==1.6.2 # xblock-google-drive # xblock-poll # xblock-utils -xblock-drag-and-drop-v2==3.1.2 +xblock-drag-and-drop-v2==3.2.0 # via -r requirements/edx/base.in xblock-google-drive==0.3.0 # via -r requirements/edx/base.in diff --git a/requirements/edx/development.txt b/requirements/edx/development.txt index d67b11d698..326dd52d52 100644 --- a/requirements/edx/development.txt +++ b/requirements/edx/development.txt @@ -1715,7 +1715,7 @@ xblock[django]==1.6.2 # xblock-google-drive # xblock-poll # xblock-utils -xblock-drag-and-drop-v2==3.1.2 +xblock-drag-and-drop-v2==3.2.0 # via -r requirements/edx/testing.txt xblock-google-drive==0.3.0 # via -r requirements/edx/testing.txt diff --git a/requirements/edx/testing.txt b/requirements/edx/testing.txt index 07618efd55..d46e811797 100644 --- a/requirements/edx/testing.txt +++ b/requirements/edx/testing.txt @@ -1583,7 +1583,7 @@ xblock[django]==1.6.2 # xblock-google-drive # xblock-poll # xblock-utils -xblock-drag-and-drop-v2==3.1.2 +xblock-drag-and-drop-v2==3.2.0 # via -r requirements/edx/base.txt xblock-google-drive==0.3.0 # via -r requirements/edx/base.txt