diff --git a/cms/djangoapps/contentstore/views/preview.py b/cms/djangoapps/contentstore/views/preview.py index 8b05d8786f..c87bf5ac0c 100644 --- a/cms/djangoapps/contentstore/views/preview.py +++ b/cms/djangoapps/contentstore/views/preview.py @@ -25,7 +25,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, XModule, XModuleDescriptor +from xmodule.x_module import AUTHOR_VIEW, PREVIEW_VIEWS, STUDENT_VIEW, ModuleSystem from cms.djangoapps.xblock_config.models import StudioConfig from cms.lib.xblock.field_data import CmsFieldData from common.djangoapps.static_replace.services import ReplaceURLService @@ -307,11 +307,6 @@ def _studio_wrap_xblock(xblock, view, frag, context, display_name_only=False): 'language': getattr(course, 'language', None) } - if isinstance(xblock, (XModule, XModuleDescriptor)): - # Add the webpackified asset tags - class_name = getattr(xblock.__class__, 'unmixed_class', xblock.__class__).__name__ - add_webpack_to_fragment(frag, class_name) - add_webpack_to_fragment(frag, "js/factories/xblock_validation") html = render_to_string('studio_xblock_wrapper.html', template_context) diff --git a/common/lib/xmodule/xmodule/mako_module.py b/common/lib/xmodule/xmodule/mako_module.py index b34e040798..238d7d7449 100644 --- a/common/lib/xmodule/xmodule/mako_module.py +++ b/common/lib/xmodule/xmodule/mako_module.py @@ -4,8 +4,9 @@ Code to handle mako templating for XModules and XBlocks. from web_fragments.fragment import Fragment +from xblock.core import XBlock -from .x_module import DescriptorSystem, XModuleDescriptor, shim_xmodule_js +from .x_module import DescriptorSystem, shim_xmodule_js, XModuleMixin class MakoDescriptorSystem(DescriptorSystem): # lint-amnesty, pylint: disable=abstract-method @@ -71,7 +72,8 @@ class MakoTemplateBlockBase: return fragment -class MakoModuleDescriptor(MakoTemplateBlockBase, XModuleDescriptor): # pylint: disable=abstract-method +@XBlock.needs("i18n") +class MakoModuleDescriptor(MakoTemplateBlockBase, XModuleMixin): # pylint: disable=abstract-method """ Mixin to use for XModule descriptors. """ diff --git a/common/lib/xmodule/xmodule/modulestore/xml_importer.py b/common/lib/xmodule/xmodule/modulestore/xml_importer.py index 493e333bb4..78e1901a55 100644 --- a/common/lib/xmodule/xmodule/modulestore/xml_importer.py +++ b/common/lib/xmodule/xmodule/modulestore/xml_importer.py @@ -51,7 +51,7 @@ from xmodule.modulestore.store_utilities import draft_node_constructor, get_draf from xmodule.modulestore.xml import ImportSystem, LibraryXMLModuleStore, XMLModuleStore from xmodule.tabs import CourseTabList from xmodule.util.misc import escape_invalid_characters -from xmodule.x_module import XModuleDescriptor, XModuleMixin +from xmodule.x_module import XModuleMixin from .inheritance import own_metadata from .store_utilities import rewrite_nonportable_content_links @@ -1346,14 +1346,11 @@ def _update_module_location(module, new_location): """ # Retrieve the content and settings fields that have been explicitly set # to ensure that they are properly re-keyed in the XBlock field data. - if isinstance(module, XModuleDescriptor): - rekey_fields = [] - else: - rekey_fields = ( - list(module.get_explicitly_set_fields_by_scope(Scope.content).keys()) + - list(module.get_explicitly_set_fields_by_scope(Scope.settings).keys()) + - list(module.get_explicitly_set_fields_by_scope(Scope.children).keys()) - ) + rekey_fields = ( + list(module.get_explicitly_set_fields_by_scope(Scope.content).keys()) + + list(module.get_explicitly_set_fields_by_scope(Scope.settings).keys()) + + list(module.get_explicitly_set_fields_by_scope(Scope.children).keys()) + ) module.location = new_location diff --git a/common/lib/xmodule/xmodule/seq_module.py b/common/lib/xmodule/xmodule/seq_module.py index d46ca96485..32edb0ae5b 100644 --- a/common/lib/xmodule/xmodule/seq_module.py +++ b/common/lib/xmodule/xmodule/seq_module.py @@ -39,7 +39,7 @@ from .exceptions import NotFoundError from .fields import Date from .mako_module import MakoTemplateBlockBase from .progress import Progress -from .x_module import AUTHOR_VIEW, PUBLIC_VIEW, STUDENT_VIEW, XModule # pylint: disable=unused-import +from .x_module import AUTHOR_VIEW, PUBLIC_VIEW, STUDENT_VIEW from .xml_module import XmlMixin diff --git a/common/lib/xmodule/xmodule/static_content.py b/common/lib/xmodule/xmodule/static_content.py index bf48f37883..3c891cd750 100755 --- a/common/lib/xmodule/xmodule/static_content.py +++ b/common/lib/xmodule/xmodule/static_content.py @@ -30,7 +30,7 @@ from xmodule.seq_module import SequenceBlock from xmodule.split_test_module import SplitTestBlock from xmodule.template_module import CustomTagBlock from xmodule.word_cloud_module import WordCloudBlock -from xmodule.x_module import XModuleDescriptor, HTMLSnippet +from xmodule.x_module import HTMLSnippet LOG = logging.getLogger(__name__) @@ -91,44 +91,22 @@ XBLOCK_CLASSES = [ def write_module_styles(output_root): """Write all registered XModule css, sass, and scss files to output root.""" - return _write_styles('.xmodule_display', output_root, _list_modules(), 'get_preview_view_css') + return _write_styles('.xmodule_display', output_root, XBLOCK_CLASSES, 'get_preview_view_css') def write_module_js(output_root): """Write all registered XModule js and coffee files to output root.""" - return _write_js(output_root, _list_modules(), 'get_preview_view_js') + return _write_js(output_root, XBLOCK_CLASSES, 'get_preview_view_js') def write_descriptor_styles(output_root): """Write all registered XModuleDescriptor css, sass, and scss files to output root.""" - return _write_styles('.xmodule_edit', output_root, _list_descriptors(), 'get_studio_view_css') + return _write_styles('.xmodule_edit', output_root, XBLOCK_CLASSES, 'get_studio_view_css') def write_descriptor_js(output_root): """Write all registered XModuleDescriptor js and coffee files to output root.""" - return _write_js(output_root, _list_descriptors(), 'get_studio_view_js') - - -def _list_descriptors(): - """Return a list of all registered XModuleDescriptor classes.""" - return sorted( - [ - desc for (_, desc) in XModuleDescriptor.load_classes() - ] + XBLOCK_CLASSES, - key=str - ) - - -def _list_modules(): - """Return a list of all registered XModule classes.""" - return sorted( - [ - desc.module_class for desc in [ - desc for (_, desc) in XModuleDescriptor.load_classes() - ] - ] + XBLOCK_CLASSES, - key=str - ) + return _write_js(output_root, XBLOCK_CLASSES, 'get_studio_view_js') def _ensure_dir(directory): diff --git a/common/lib/xmodule/xmodule/studio_editable.py b/common/lib/xmodule/xmodule/studio_editable.py index e9176cd8a0..f222851dc3 100644 --- a/common/lib/xmodule/xmodule/studio_editable.py +++ b/common/lib/xmodule/xmodule/studio_editable.py @@ -2,7 +2,7 @@ Mixin to support editing in Studio. """ from xblock.core import XBlock, XBlockMixin -from xmodule.x_module import AUTHOR_VIEW, STUDENT_VIEW, module_attr +from xmodule.x_module import AUTHOR_VIEW, STUDENT_VIEW @XBlock.needs('mako') @@ -51,17 +51,6 @@ class StudioEditableBlock(XBlockMixin): StudioEditableModule = StudioEditableBlock -class StudioEditableDescriptor: - """ - Helper mixin for supporting Studio editing of xmodules. - - This class is only intended to be used with an XModule Descriptor. This class assumes that the associated - XModule will have an "author_view" method for returning an editable preview view of the module. - """ - author_view = module_attr(AUTHOR_VIEW) - has_author_view = True - - def has_author_view(descriptor): """ Returns True if the xmodule linked to the descriptor supports "author_view". diff --git a/common/lib/xmodule/xmodule/tests/__init__.py b/common/lib/xmodule/xmodule/tests/__init__.py index 02d4c8f980..fd38c6f64d 100644 --- a/common/lib/xmodule/xmodule/tests/__init__.py +++ b/common/lib/xmodule/xmodule/tests/__init__.py @@ -37,7 +37,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, XModuleDescriptor, XModuleMixin +from xmodule.x_module import DoNothingCache, ModuleSystem, XModuleMixin from openedx.core.lib.cache_utils import CacheService diff --git a/common/lib/xmodule/xmodule/tests/test_content.py b/common/lib/xmodule/xmodule/tests/test_content.py index 93a50e4e35..dabd6d7527 100644 --- a/common/lib/xmodule/xmodule/tests/test_content.py +++ b/common/lib/xmodule/xmodule/tests/test_content.py @@ -11,7 +11,7 @@ from opaque_keys.edx.locator import AssetLocator, CourseLocator from path import Path as path from xmodule.contentstore.content import ContentStore, StaticContent, StaticContentStream -from xmodule.static_content import _list_descriptors, _write_js +from xmodule.static_content import XBLOCK_CLASSES, _write_js SAMPLE_STRING = """ This is a sample string with more than 1024 bytes, the default STREAM_DATA_CHUNK_SIZE @@ -211,7 +211,7 @@ class ContentTest(unittest.TestCase): # lint-amnesty, pylint: disable=missing-c Test that only one filename starts with 000. """ output_root = path('common/static/xmodule/descriptors/js') - file_owners = _write_js(output_root, _list_descriptors(), 'get_studio_view_js') + file_owners = _write_js(output_root, XBLOCK_CLASSES, 'get_studio_view_js') js_file_paths = {file_path for file_path in sum(list(file_owners.values()), []) if os.path.basename(file_path).startswith('000-')} # lint-amnesty, pylint: disable=line-too-long assert len(js_file_paths) == 1 assert 'XModule.Descriptor = (function() {' in open(js_file_paths.pop()).read() diff --git a/common/lib/xmodule/xmodule/tests/test_error_module.py b/common/lib/xmodule/xmodule/tests/test_error_module.py index 7e266bd95c..066f48aed7 100644 --- a/common/lib/xmodule/xmodule/tests/test_error_module.py +++ b/common/lib/xmodule/xmodule/tests/test_error_module.py @@ -4,19 +4,15 @@ Tests for ErrorBlock and NonStaffErrorBlock import unittest -from unittest.mock import MagicMock, Mock, patch import pytest -from opaque_keys.edx.locator import BlockUsageLocator, CourseLocator -from xblock.field_data import DictFieldData -from xblock.fields import ScopeIds -from xblock.runtime import IdReader, Runtime +from opaque_keys.edx.locator import CourseLocator from xblock.test.tools import unabc from xmodule.error_module import ErrorBlock, NonStaffErrorBlock from xmodule.modulestore.xml import CourseLocationManager from xmodule.tests import get_test_system -from xmodule.x_module import STUDENT_VIEW, XModule, XModuleDescriptor +from xmodule.x_module import STUDENT_VIEW class SetupTestErrorBlock(unittest.TestCase): @@ -49,21 +45,6 @@ class TestErrorBlock(SetupTestErrorBlock): assert self.error_msg in context_repr assert repr(self.valid_xml) in context_repr - def test_error_block_from_descriptor(self): - descriptor = MagicMock( - spec=XModuleDescriptor, - runtime=self.system, - location=self.location, - ) - - error_descriptor = ErrorBlock.from_descriptor( - descriptor, self.error_msg) - assert isinstance(error_descriptor, ErrorBlock) - error_descriptor.xmodule_runtime = self.system - context_repr = self.system.render(error_descriptor, STUDENT_VIEW).content - assert self.error_msg in context_repr - assert repr(descriptor) in context_repr - class TestNonStaffErrorBlock(SetupTestErrorBlock): """ @@ -88,82 +69,3 @@ class TestNonStaffErrorBlock(SetupTestErrorBlock): context_repr = self.system.render(descriptor, STUDENT_VIEW).content assert self.error_msg not in context_repr assert repr(self.valid_xml) not in context_repr - - def test_error_block_from_descriptor(self): - descriptor = MagicMock( - spec=XModuleDescriptor, - runtime=self.system, - location=self.location, - ) - - error_descriptor = NonStaffErrorBlock.from_descriptor( - descriptor, self.error_msg) - assert isinstance(error_descriptor, ErrorBlock) - error_descriptor.xmodule_runtime = self.system - context_repr = self.system.render(error_descriptor, STUDENT_VIEW).content - assert self.error_msg not in context_repr - assert str(descriptor) not in context_repr - - -class BrokenModule(XModule): # lint-amnesty, pylint: disable=abstract-method - def __init__(self, *args, **kwargs): - super().__init__(*args, **kwargs) - raise Exception("This is a broken xmodule") - - -class BrokenDescriptor(XModuleDescriptor): # lint-amnesty, pylint: disable=abstract-method - module_class = BrokenModule - - -class TestException(Exception): - """An exception type to use to verify raises in tests""" - pass # lint-amnesty, pylint: disable=unnecessary-pass - - -@unabc("Tests should not call {}") -class TestRuntime(Runtime): # lint-amnesty, pylint: disable=abstract-method - pass - - -class TestErrorBlockConstruction(unittest.TestCase): - """ - Test that error block construction happens correctly - """ - - def setUp(self): - # pylint: disable=abstract-class-instantiated - super().setUp() - field_data = DictFieldData({}) - self.descriptor = BrokenDescriptor( - TestRuntime(Mock(spec=IdReader), field_data), - field_data, - ScopeIds(None, None, None, - BlockUsageLocator(CourseLocator('org', 'course', 'run'), 'broken', 'name')) - ) - self.descriptor.xmodule_runtime = TestRuntime(Mock(spec=IdReader), field_data) - self.descriptor.xmodule_runtime.error_descriptor_class = ErrorBlock - self.descriptor.xmodule_runtime.xmodule_instance = None - - def test_broken_block(self): - """ - Test that when an XModule throws an block during __init__, we - get an ErrorBlock back from XModuleDescriptor._xmodule - """ - module = self.descriptor._xmodule # lint-amnesty, pylint: disable=protected-access - assert isinstance(module, ErrorBlock) - - @patch.object(ErrorBlock, '__init__', Mock(side_effect=TestException)) - def test_broken_error_descriptor(self): - """ - Test that a broken block descriptor doesn't cause an infinite loop - """ - with pytest.raises(TestException): - module = self.descriptor._xmodule # lint-amnesty, pylint: disable=protected-access, unused-variable - - @patch.object(ErrorBlock, '__init__', Mock(side_effect=TestException)) - def test_broken_error_block(self): - """ - Test that a broken block module doesn't cause an infinite loop - """ - with pytest.raises(TestException): - module = self.descriptor._xmodule # lint-amnesty, pylint: disable=protected-access, unused-variable diff --git a/common/lib/xmodule/xmodule/tests/test_progress.py b/common/lib/xmodule/xmodule/tests/test_progress.py index 0b77510dfa..17cdc2b19a 100644 --- a/common/lib/xmodule/xmodule/tests/test_progress.py +++ b/common/lib/xmodule/xmodule/tests/test_progress.py @@ -2,15 +2,9 @@ import unittest -from unittest.mock import Mock -from xblock.field_data import DictFieldData - -from xmodule import x_module from xmodule.progress import Progress -from . import get_test_system - class ProgressTest(unittest.TestCase): ''' Test that basic Progress objects work. A Progress represents a @@ -113,14 +107,3 @@ class ProgressTest(unittest.TestCase): # Check != while we're at it assert prg1 != prg2 assert prg1 == prg3 - - -class ModuleProgressTest(unittest.TestCase): - ''' Test that get_progress() does the right thing for the different modules - ''' - - def test_xmodule_default(self): - '''Make sure default get_progress exists, returns None''' - xmod = x_module.XModule(Mock(), get_test_system(), DictFieldData({'location': 'a://b/c/d/e'}), Mock()) - prg = xmod.get_progress() # lint-amnesty, pylint: disable=assignment-from-none - assert prg is None diff --git a/common/lib/xmodule/xmodule/tests/test_xblock_wrappers.py b/common/lib/xmodule/xmodule/tests/test_xblock_wrappers.py deleted file mode 100644 index 74f9fd96b8..0000000000 --- a/common/lib/xmodule/xmodule/tests/test_xblock_wrappers.py +++ /dev/null @@ -1,453 +0,0 @@ -""" -Tests for the wrapping layer that provides the XBlock API using XModule/Descriptor -functionality -""" -# For tests, ignore access to protected members -# pylint: disable=protected-access - - -from unittest.case import SkipTest, TestCase -from unittest.mock import Mock - -import ddt -import webob -from webob.multidict import MultiDict -from factory import ( - BUILD_STRATEGY, - Factory, - LazyAttributeSequence, - SubFactory, - lazy_attribute, - post_generation, - use_strategy -) -from fs.memoryfs import MemoryFS -from lxml import etree -from opaque_keys.edx.locator import BlockUsageLocator, CourseLocator -from xblock.core import XBlock -from xblock.field_data import DictFieldData -from xblock.fields import ScopeIds - -from xmodule.annotatable_module import AnnotatableBlock -from xmodule.conditional_module import ConditionalBlock -from xmodule.course_module import CourseBlock -from xmodule.html_module import HtmlBlock -from xmodule.poll_module import PollBlock -from xmodule.randomize_module import RandomizeBlock -from xmodule.seq_module import SequenceBlock -from xmodule.tests import get_test_descriptor_system, get_test_system -from xmodule.vertical_block import VerticalBlock -from xmodule.word_cloud_module import WordCloudBlock -from xmodule.wrapper_module import WrapperBlock -from xmodule.x_module import ( - PUBLIC_VIEW, - STUDENT_VIEW, - STUDIO_VIEW, - DescriptorSystem, - ModuleSystem, - XModule, - XModuleDescriptor -) - -# A dictionary that maps specific XModuleDescriptor classes without children -# to a list of sample field values to test with. -# TODO: Add more types of sample data -LEAF_XMODULES = { - AnnotatableBlock: [{}], - HtmlBlock: [{}], - PollBlock: [{'display_name': 'Poll Display Name'}], - WordCloudBlock: [{}], -} - - -# A dictionary that maps specific XModuleDescriptor classes with children -# to a list of sample field values to test with. -# TODO: Add more types of sample data -CONTAINER_XMODULES = { - ConditionalBlock: [{}], - CourseBlock: [{}], - RandomizeBlock: [{'display_name': 'Test String Display'}], - SequenceBlock: [{'display_name': 'Test Unicode हिंदी Display'}], - VerticalBlock: [{}], - WrapperBlock: [{}], -} - -# These modules are not editable in studio yet -NOT_STUDIO_EDITABLE = ( - PollBlock, -) - - -def flatten(class_dict): - """ - Flatten a dict from cls -> [fields, ...] and yields values of the form (cls, fields) - for each entry in the dictionary value. - """ - for cls in sorted(class_dict, key=lambda err: err.__name__): - fields_list = class_dict[cls] - for fields in fields_list: - yield (cls, fields) - - -@use_strategy(BUILD_STRATEGY) -class ModuleSystemFactory(Factory): - """ - Factory to build a test ModuleSystem. Creation is - performed by :func:`xmodule.tests.get_test_system`, so - arguments for that function are valid factory attributes. - """ - class Meta: - model = ModuleSystem - - @classmethod - def _build(cls, target_class, *args, **kwargs): # lint-amnesty, pylint: disable=arguments-differ, unused-argument - """See documentation from :meth:`factory.Factory._build`""" - return get_test_system(*args, **kwargs) - - -@use_strategy(BUILD_STRATEGY) -class DescriptorSystemFactory(Factory): - """ - Factory to build a test DescriptorSystem. Creation is - performed by :func:`xmodule.tests.get_test_descriptor_system`, so - arguments for that function are valid factory attributes. - """ - class Meta: - model = DescriptorSystem - - @classmethod - def _build(cls, target_class, *args, **kwargs): # lint-amnesty, pylint: disable=arguments-differ, unused-argument - """See documentation from :meth:`factory.Factory._build`""" - return get_test_descriptor_system(*args, **kwargs) - - -class ContainerModuleRuntimeFactory(ModuleSystemFactory): - """ - Factory to generate a ModuleRuntime that generates children when asked - for them, for testing container XModules. - """ - @post_generation - def depth(self, create, depth, **kwargs): # pylint: disable=unused-argument - """ - When `depth` is specified as a Factory parameter, creates a - tree of children with that many levels. - """ - # pylint: disable=no-member - if depth == 0: - self.get_module.side_effect = lambda x: LeafModuleFactory(descriptor_cls=HtmlBlock) - else: - self.get_module.side_effect = lambda x: ContainerModuleFactory( - descriptor_cls=VerticalBlock, - depth=depth - 1 - ) - - @post_generation - def position(self, create, position=2, **kwargs): # pylint: disable=unused-argument, method-hidden - """ - Update the position attribute of the generated ModuleRuntime. - """ - self.position = position - - -class ContainerDescriptorRuntimeFactory(DescriptorSystemFactory): - """ - Factory to generate a DescriptorRuntime that generates children when asked - for them, for testing container XModuleDescriptors. - """ - @post_generation - def depth(self, create, depth, **kwargs): # pylint: disable=unused-argument - """ - When `depth` is specified as a Factory parameter, creates a - tree of children with that many levels. - """ - # pylint: disable=no-member - if depth == 0: - self.load_item.side_effect = lambda x: LeafModuleFactory(descriptor_cls=HtmlBlock) - else: - self.load_item.side_effect = lambda x: ContainerModuleFactory( - descriptor_cls=VerticalBlock, - depth=depth - 1 - ) - - @post_generation - def position(self, create, position=2, **kwargs): # pylint: disable=unused-argument, method-hidden - """ - Update the position attribute of the generated ModuleRuntime. - """ - self.position = position - - -@use_strategy(BUILD_STRATEGY) -class LeafDescriptorFactory(Factory): - """ - Factory to generate leaf XModuleDescriptors. - """ - - class Meta: - model = XModuleDescriptor - - runtime = SubFactory(DescriptorSystemFactory) - url_name = LazyAttributeSequence('{.block_type}_{}'.format) - - @lazy_attribute - def location(self): - return BlockUsageLocator(CourseLocator('org', 'course', 'run'), 'category', self.url_name) - - @lazy_attribute - def block_type(self): - return self.descriptor_cls.__name__ # pylint: disable=no-member - - @lazy_attribute - def definition_id(self): - return self.location - - @lazy_attribute - def usage_id(self): - return self.location - - @classmethod - def _build(cls, target_class, *args, **kwargs): # lint-amnesty, pylint: disable=arguments-differ, unused-argument - runtime = kwargs.pop('runtime') - desc_cls = kwargs.pop('descriptor_cls') - block_type = kwargs.pop('block_type') - def_id = kwargs.pop('definition_id') - usage_id = kwargs.pop('usage_id') - - block = runtime.construct_xblock_from_class( - desc_cls, - ScopeIds(None, block_type, def_id, usage_id), - DictFieldData(dict(**kwargs)) - ) - block.save() - return block - - -class LeafModuleFactory(LeafDescriptorFactory): - """ - Factory to generate leaf XModuleDescriptors that are prepped to be - used as XModules. - """ - @post_generation - def xmodule_runtime(self, create, xmodule_runtime, **kwargs): # pylint: disable=method-hidden, unused-argument - """ - Set the xmodule_runtime to make this XModuleDescriptor usable - as an XModule. - """ - if xmodule_runtime is None: - xmodule_runtime = ModuleSystemFactory() - - self.xmodule_runtime = xmodule_runtime - - -class ContainerDescriptorFactory(LeafDescriptorFactory): - """ - Factory to generate XModuleDescriptors that are containers. - """ - runtime = SubFactory(ContainerDescriptorRuntimeFactory) - children = list(range(3)) - - -class ContainerModuleFactory(LeafModuleFactory): - """ - Factory to generate XModuleDescriptors that are containers - and are ready to act as XModules. - """ - @lazy_attribute - def xmodule_runtime(self): # lint-amnesty, pylint: disable=arguments-differ - return ContainerModuleRuntimeFactory(depth=self.depth) # pylint: disable=no-member - - -@ddt.ddt -class XBlockWrapperTestMixin: - """ - This is a mixin for building tests of the implementation of the XBlock - api by wrapping XModule native functions. - - You can create an actual test case by inheriting from this class and UnitTest, - and implement skip_if_invalid and check_property. - """ - - def skip_if_invalid(self, descriptor_cls): - """ - Raise SkipTest if this descriptor_cls shouldn't be tested. - """ - pass # lint-amnesty, pylint: disable=unnecessary-pass - - def check_property(self, descriptor): - """ - Execute assertions to verify that the property under test is true for - the supplied descriptor. - """ - raise SkipTest("check_property not defined") - - # Test that for all of the leaf XModule Descriptors, - # the test property holds - @ddt.data(*flatten(LEAF_XMODULES)) - def test_leaf_node(self, cls_and_fields): - descriptor_cls, fields = cls_and_fields - self.skip_if_invalid(descriptor_cls) - descriptor = LeafModuleFactory(descriptor_cls=descriptor_cls, **fields) - mocked_course = Mock() - modulestore = Mock() - modulestore.get_course.return_value = mocked_course - # pylint: disable=no-member - descriptor.runtime.id_reader.get_definition_id = Mock(return_value='a') - descriptor.runtime.modulestore = modulestore - if hasattr(descriptor, '_xmodule'): - descriptor._xmodule.graded = 'False' - self.check_property(descriptor) - - # Test that when an xmodule is generated from descriptor_cls - # with only xmodule children, the test property holds - @ddt.data(*flatten(CONTAINER_XMODULES)) - def test_container_node_xmodules_only(self, cls_and_fields): - descriptor_cls, fields = cls_and_fields - self.skip_if_invalid(descriptor_cls) - descriptor = ContainerModuleFactory(descriptor_cls=descriptor_cls, depth=2, **fields) - descriptor.runtime.id_reader.get_definition_id = Mock(return_value='a') - self.check_property(descriptor) - - # Test that when an xmodule is generated from descriptor_cls - # with mixed xmodule and xblock children, the test property holds - @ddt.data(*flatten(CONTAINER_XMODULES)) - def test_container_node_mixed(self, cls_and_fields): - raise SkipTest("XBlock support in XDescriptor not yet fully implemented") - - # Test that when an xmodule is generated from descriptor_cls - # with only xblock children, the test property holds - @ddt.data(*flatten(CONTAINER_XMODULES)) - def test_container_node_xblocks_only(self, cls_and_fields): - raise SkipTest("XBlock support in XModules not yet fully implemented") - - -class TestStudentView(XBlockWrapperTestMixin, TestCase): - """ - This tests that student_view and XModule.get_html produce the same results. - """ - - def skip_if_invalid(self, descriptor_cls): - pure_xblock_class = issubclass(descriptor_cls, XBlock) and not issubclass(descriptor_cls, XModuleDescriptor) - if pure_xblock_class: - student_view = descriptor_cls.student_view - else: - student_view = descriptor_cls.module_class.student_view - if student_view != XModule.student_view: - raise SkipTest(descriptor_cls.__name__ + " implements student_view") - - def check_property(self, descriptor): - """ - Assert that both student_view and get_html render the same. - """ - assert descriptor._xmodule.get_html() == descriptor.render(STUDENT_VIEW).content - - -class TestStudioView(XBlockWrapperTestMixin, TestCase): - """ - This tests that studio_view and XModuleDescriptor.get_html produce the same results - """ - - def skip_if_invalid(self, descriptor_cls): - if descriptor_cls in NOT_STUDIO_EDITABLE: - raise SkipTest(descriptor_cls.__name__ + " is not editable in studio") - - pure_xblock_class = issubclass(descriptor_cls, XBlock) and not issubclass(descriptor_cls, XModuleDescriptor) - if pure_xblock_class: # lint-amnesty, pylint: disable=no-else-raise - raise SkipTest(descriptor_cls.__name__ + " is a pure XBlock and implements studio_view") - elif descriptor_cls.studio_view != XModuleDescriptor.studio_view: - raise SkipTest(descriptor_cls.__name__ + " implements studio_view") - - def check_property(self, descriptor): - """ - Assert that studio_view and get_html render the same. - """ - html = descriptor.get_html() - rendered_content = descriptor.render(STUDIO_VIEW).content - assert html == rendered_content - - -@ddt.ddt -class TestXModuleHandler(TestCase): - """ - Tests that the xmodule_handler function correctly wraps handle_ajax - """ - - def setUp(self): - super().setUp() - self.module = XModule(descriptor=Mock(), field_data=Mock(), runtime=Mock(), scope_ids=Mock()) - self.module.handle_ajax = Mock(return_value='{}') - self.request = webob.Request({}) - - def test_xmodule_handler_passed_data(self): - self.module.xmodule_handler(self.request) - self.module.handle_ajax.assert_called_with(None, MultiDict(self.request.POST)) - - def test_xmodule_handler_dispatch(self): - self.module.xmodule_handler(self.request, 'dispatch') - self.module.handle_ajax.assert_called_with('dispatch', MultiDict(self.request.POST)) - - def test_xmodule_handler_return_value(self): - response = self.module.xmodule_handler(self.request) - assert isinstance(response, webob.Response) - assert response.body.decode('utf-8') == '{}' - - @ddt.data( - '{"test_key": "test_value"}', - '{"test_key": "test_value"}', - ) - def test_xmodule_handler_with_data(self, response_data): - """ - Tests that xmodule_handler function correctly wraps handle_ajax when handle_ajax response is either - str or unicode. - """ - - self.module.handle_ajax = Mock(return_value=response_data) - response = self.module.xmodule_handler(self.request) - assert isinstance(response, webob.Response) - assert response.body.decode('utf-8') == '{"test_key": "test_value"}' - - -class TestXmlExport(XBlockWrapperTestMixin, TestCase): - """ - This tests that XModuleDescriptor.export_course_to_xml and add_xml_to_node produce the same results. - """ - - def skip_if_invalid(self, descriptor_cls): - if descriptor_cls.add_xml_to_node != XModuleDescriptor.add_xml_to_node: - raise SkipTest(descriptor_cls.__name__ + " implements add_xml_to_node") - - def check_property(self, descriptor): - xmodule_api_fs = MemoryFS() - xblock_api_fs = MemoryFS() - - descriptor.runtime.export_fs = xblock_api_fs - xblock_node = etree.Element('unknown') - descriptor.add_xml_to_node(xblock_node) - - xmodule_node = etree.fromstring(descriptor.export_to_xml(xmodule_api_fs)) - - assert list(xmodule_api_fs.walk()) == list(xblock_api_fs.walk()) - assert etree.tostring(xmodule_node) == etree.tostring(xblock_node) - - -class TestPublicView(XBlockWrapperTestMixin, TestCase): - """ - This tests that default public_view shows the correct message. - """ - - def skip_if_invalid(self, descriptor_cls): - pure_xblock_class = issubclass(descriptor_cls, XBlock) and not issubclass(descriptor_cls, XModuleDescriptor) - if pure_xblock_class: - public_view = descriptor_cls.public_view - else: - public_view = descriptor_cls.module_class.public_view - if public_view != XModule.public_view: - raise SkipTest(descriptor_cls.__name__ + " implements public_view") - - def check_property(self, descriptor): - """ - Assert that public_view contains correct message. - """ - if descriptor.display_name: - assert descriptor.display_name in descriptor.render(PUBLIC_VIEW).content - else: - assert 'This content is only accessible' in descriptor.render(PUBLIC_VIEW).content diff --git a/common/lib/xmodule/xmodule/x_module.py b/common/lib/xmodule/xmodule/x_module.py index f122f4663a..307d7c8155 100644 --- a/common/lib/xmodule/xmodule/x_module.py +++ b/common/lib/xmodule/xmodule/x_module.py @@ -251,26 +251,18 @@ class HTMLSnippet: @classmethod def get_preview_view_js(cls): - if issubclass(cls, XModule): - return cls.get_javascript() return cls.preview_view_js @classmethod def get_preview_view_js_bundle_name(cls): - if issubclass(cls, XModule): - return cls.__name__ return cls.__name__ + 'Preview' @classmethod def get_studio_view_js(cls): - if issubclass(cls, XModuleDescriptor): - return cls.get_javascript() return cls.studio_view_js @classmethod def get_studio_view_js_bundle_name(cls): - if issubclass(cls, XModuleDescriptor): - return cls.__name__ return cls.__name__ + 'Studio' @classmethod @@ -291,14 +283,10 @@ class HTMLSnippet: @classmethod def get_preview_view_css(cls): - if issubclass(cls, XModule): - return cls.get_css() return cls.preview_view_css @classmethod def get_studio_view_css(cls): - if issubclass(cls, XModuleDescriptor): - return cls.get_css() return cls.studio_view_css def get_html(self): @@ -834,54 +822,6 @@ class XModuleMixin(XModuleFields, XBlock): return Fragment(alert_html.format(display_text)) -class ProxyAttribute: - """ - A (python) descriptor that proxies attribute access. - - For example: - - class Foo(object): - def __init__(self, value): - self.foo_attr = value - - class Bar(object): - foo = Foo('x') - foo_attr = ProxyAttribute('foo', 'foo_attr') - - bar = Bar() - - assert bar.foo_attr == 'x' - bar.foo_attr = 'y' - assert bar.foo.foo_attr == 'y' - del bar.foo_attr - assert not hasattr(bar.foo, 'foo_attr') - """ - def __init__(self, source, name): - """ - :param source: The name of the attribute to proxy to - :param name: The name of the attribute to proxy - """ - self._source = source - self._name = name - - def __get__(self, instance, owner): - if instance is None: - return self - - return getattr(getattr(instance, self._source), self._name) - - def __set__(self, instance, value): - setattr(getattr(instance, self._source), self._name, value) - - def __delete__(self, instance): - delattr(getattr(instance, self._source), self._name) - - -module_attr = partial(ProxyAttribute, '_xmodule') # pylint: disable=invalid-name -descriptor_attr = partial(ProxyAttribute, 'descriptor') # pylint: disable=invalid-name -module_runtime_attr = partial(ProxyAttribute, 'xmodule_runtime') # pylint: disable=invalid-name - - class XModuleToXBlockMixin: """ Common code needed by XModule and XBlocks converted from XModules. @@ -926,108 +866,6 @@ class XModuleToXBlockMixin: return Response(response_data, content_type='application/json', charset='UTF-8') -@XBlock.needs("i18n") -class XModule(XModuleToXBlockMixin, HTMLSnippet, XModuleMixin): # lint-amnesty, pylint: disable=abstract-method - """ Implements a generic learning module. - - Subclasses must at a minimum provide a definition for get_html in order - to be displayed to users. - - See the HTML module for a simple example. - """ - - entry_point = "xmodule.v1" - - has_score = descriptor_attr('has_score') - max_score = descriptor_attr('max_score') - show_in_read_only_mode = descriptor_attr('show_in_read_only_mode') - _field_data_cache = descriptor_attr('_field_data_cache') - _field_data = descriptor_attr('_field_data') - _dirty_fields = descriptor_attr('_dirty_fields') - - def __init__(self, descriptor, *args, **kwargs): - """ - Construct a new xmodule - - runtime: An XBlock runtime allowing access to external resources - - descriptor: the XModuleDescriptor that this module is an instance of. - - field_data: A dictionary-like object that maps field names to values - for those fields. - """ - - # Set the descriptor first so that we can proxy to it - self.descriptor = descriptor - self._runtime = None - super().__init__(*args, **kwargs) - self.runtime.xmodule_instance = self - - @property - def runtime(self): - return CombinedSystem(self._runtime, self.descriptor._runtime) # pylint: disable=protected-access - - @runtime.setter - def runtime(self, value): - self._runtime = value - - def __str__(self): - # xss-lint: disable=python-wrap-html - return f'' # lint-amnesty, pylint: disable=no-member - - def handle_ajax(self, _dispatch, _data): - """ dispatch is last part of the URL. - data is a dictionary-like object with the content of the request""" - return "" - - def get_child(self, usage_id): - if usage_id in self._child_cache: - return self._child_cache[usage_id] - - # Take advantage of the children cache that the descriptor might have - child_descriptor = self.descriptor.get_child(usage_id) - child_block = None - if child_descriptor is not None: - child_block = self.system.get_module(child_descriptor) - - self._child_cache[usage_id] = child_block - return child_block - - def get_child_descriptors(self): - """ - Returns the descriptors of the child modules - - Overriding this changes the behavior of get_children and - anything that uses get_children, such as get_display_items. - - This method will not instantiate the modules of the children - unless absolutely necessary, so it is cheaper to call than get_children - - These children will be the same children returned by the - descriptor unless descriptor.has_dynamic_children() is true. - """ - return self.descriptor.get_children() - - def displayable_items(self): - """ - Returns list of displayable modules contained by this module. If this - module is visible, should return [self]. - """ - return [self.descriptor] - - # ~~~~~~~~~~~~~~~ XBlock API Wrappers ~~~~~~~~~~~~~~~~ - def student_view(self, context): # lint-amnesty, pylint: disable=unused-argument - """ - Return a fragment with the html from this XModule - - Doesn't yet add any of the javascript to the fragment, nor the css. - Also doesn't expect any javascript binding, yet. - - Makes no use of the context parameter - """ - return Fragment(self.get_html()) - - def policy_key(location): """ Get the key for a location in a policy file. (Since the policy file is @@ -1104,161 +942,6 @@ class ResourceTemplates: return template -@XBlock.needs("i18n") -class XModuleDescriptor(HTMLSnippet, ResourceTemplates, XModuleMixin): # lint-amnesty, pylint: disable=abstract-method - """ - An XModuleDescriptor is a specification for an element of a course. This - could be a problem, an organizational element (a group of content), or a - segment of video, for example. - - XModuleDescriptors are independent and agnostic to the current student state - on a problem. They handle the editing interface used by instructors to - create a problem, and can generate XModules (which do know about student - state). - """ - - entry_point = "xmodule.v1" - - module_class = XModule - - # ============================= STRUCTURAL MANIPULATION =================== - def __init__(self, *args, **kwargs): - """ - Construct a new XModuleDescriptor. The only required arguments are the - system, used for interaction with external resources, and the - definition, which specifies all the data needed to edit and display the - problem (but none of the associated metadata that handles recordkeeping - around the problem). - - This allows for maximal flexibility to add to the interface while - preserving backwards compatibility. - - runtime: A DescriptorSystem for interacting with external resources - - field_data: A dictionary-like object that maps field names to values - for those fields. - - XModuleDescriptor.__init__ takes the same arguments as xblock.core:XBlock.__init__ - """ - super().__init__(*args, **kwargs) - # update_version is the version which last updated this xblock v prev being the penultimate updater - # leaving off original_version since it complicates creation w/o any obv value yet and is computable - # by following previous until None - # definition_locator is only used by mongostores which separate definitions from blocks - self.previous_version = self.update_version = self.definition_locator = None - self.xmodule_runtime = None - - def editor_saved(self, user, old_metadata, old_content): - """ - This method is called when "Save" is pressed on the Studio editor. - - Note that after this method is called, the modulestore update_item method will - be called on this xmodule. Therefore, any modifications to the xmodule that are - performed in editor_saved will automatically be persisted (calling update_item - from implementors of this method is not necessary). - - Args: - user: the user who requested the save (as obtained from the request) - old_metadata (dict): the values of the fields with Scope.settings before the save was performed - old_content (dict): the values of the fields with Scope.content before the save was performed. - This will include 'data'. - """ - pass # lint-amnesty, pylint: disable=unnecessary-pass - - # =============================== BUILTIN METHODS ========================== - def __eq__(self, other): - """ - Is this XModule effectively equal to the other instance? - """ - return (hasattr(other, 'scope_ids') and - self.scope_ids == other.scope_ids and - set(self.fields.keys()) == set(other.fields.keys()) and # lint-amnesty, pylint: disable=no-member - all(getattr(self, field.name) == getattr(other, field.name) - for field in self.fields.values())) # lint-amnesty, pylint: disable=no-member - - def __hash__(self): # pylint: disable=useless-super-delegation - """ - This isn't technically appropriate since descriptors are actually mutable, - but in practice we rarely modify them after creation or instantiate two - equivalent descriptors in the same process. And we perform graph - operations on large collections of XBlocks that have simply unacceptable - performance if we have to rely on lists and equality rather than sets, - dictionaries, and identity-based hash functions. - """ - return super().__hash__() - - def __repr__(self): - return ( - "{0.__class__.__name__}(" - "{0.runtime!r}, " - "{0._field_data!r}, " - "{0.scope_ids!r}" - ")".format(self) - ) - - # ~~~~~~~~~~~~~~~ XModule Indirection ~~~~~~~~~~~~~~~~ - @property - def _xmodule(self): - """ - Returns the XModule corresponding to this descriptor. Expects that the system - already supports all of the attributes needed by xmodules - """ - if self.xmodule_runtime is None: - raise UndefinedContext() - assert self.xmodule_runtime.error_descriptor_class is not None - if self.xmodule_runtime.xmodule_instance is None: - try: - self.xmodule_runtime.construct_xblock_from_class( - self.module_class, - descriptor=self, - scope_ids=self.scope_ids, - field_data=self._field_data, - for_parent=self.get_parent() if self.has_cached_parent else None - ) - self.xmodule_runtime.xmodule_instance.save() - except Exception: # pylint: disable=broad-except - # xmodule_instance is set by the XModule.__init__. If we had an error after that, - # we need to clean it out so that we can set up the ErrorBlock instead - self.xmodule_runtime.xmodule_instance = None - - if isinstance(self, self.xmodule_runtime.error_descriptor_class): - log.exception('Error creating an ErrorBlock from an ErrorBlock') - raise - - log.exception('Error creating xmodule') - descriptor = self.xmodule_runtime.error_descriptor_class.from_descriptor( - self, - error_msg=exc_info_to_str(sys.exc_info()) - ) - descriptor.xmodule_runtime = self.xmodule_runtime - self.xmodule_runtime.xmodule_instance = descriptor - return self.xmodule_runtime.xmodule_instance - - course_id = module_attr('course_id') - displayable_items = module_attr('displayable_items') - get_display_items = module_attr('get_display_items') - get_icon_class = module_attr('get_icon_class') - get_progress = module_attr('get_progress') - get_score = module_attr('get_score') - handle_ajax = module_attr('handle_ajax') - student_view = module_attr(STUDENT_VIEW) - public_view = module_attr(PUBLIC_VIEW) - get_child_descriptors = module_attr('get_child_descriptors') - xmodule_handler = module_attr('xmodule_handler') - - # ~~~~~~~~~~~~~~~ XBlock API Wrappers ~~~~~~~~~~~~~~~~ - def studio_view(self, _context): - """ - Return a fragment with the html from this XModuleDescriptor's editing view - - Doesn't yet add any of the javascript to the fragment, nor the css. - Also doesn't expect any javascript binding, yet. - - Makes no use of the context parameter - """ - return Fragment(self.get_html()) - - class ConfigurableFragmentWrapper: """ Runtime mixin that allows for composition of many `wrap_xblock` wrappers @@ -2071,18 +1754,6 @@ class CombinedSystem: self._module_system = module_system self._descriptor_system = descriptor_system - def _get_student_block(self, block): - """ - If block is an XModuleDescriptor that has been bound to a student, return - the corresponding XModule, instead of the XModuleDescriptor. - - Otherwise, return block. - """ - if isinstance(block, XModuleDescriptor) and block.xmodule_runtime: - return block._xmodule # pylint: disable=protected-access - else: - return block - def render(self, block, view_name, context=None): """ Render a block by invoking its view. @@ -2097,9 +1768,6 @@ class CombinedSystem: """ context = context or {} - if view_name in PREVIEW_VIEWS: - block = self._get_student_block(block) - return self.__getattr__('render')(block, view_name, context) def service(self, block, service_name): diff --git a/common/lib/xmodule/xmodule/xml_module.py b/common/lib/xmodule/xmodule/xml_module.py index 8afc6a5695..488d6b1919 100644 --- a/common/lib/xmodule/xmodule/xml_module.py +++ b/common/lib/xmodule/xmodule/xml_module.py @@ -8,12 +8,13 @@ import os from lxml import etree from lxml.etree import Element, ElementTree, XMLParser -from xblock.core import XML_NAMESPACES +from xblock.core import XBlock, XML_NAMESPACES from xblock.fields import Dict, Scope, ScopeIds from xblock.runtime import KvsFieldData from xmodule.modulestore import EdxJSONEncoder from xmodule.modulestore.inheritance import InheritanceKeyValueStore, own_metadata -from xmodule.x_module import XModuleDescriptor # lint-amnesty, pylint: disable=unused-import + +from .x_module import XModuleMixin log = logging.getLogger(__name__) @@ -604,5 +605,6 @@ class XmlMixin(XmlParserMixin): # lint-amnesty, pylint: disable=abstract-method super().add_xml_to_node(node) -class XmlDescriptor(XmlMixin, XModuleDescriptor): # lint-amnesty, pylint: disable=abstract-method +@XBlock.needs("i18n") +class XmlDescriptor(XmlMixin, XModuleMixin): # lint-amnesty, pylint: disable=abstract-method pass diff --git a/lms/djangoapps/courseware/access.py b/lms/djangoapps/courseware/access.py index 46668f1aa8..fe9db5db56 100644 --- a/lms/djangoapps/courseware/access.py +++ b/lms/djangoapps/courseware/access.py @@ -33,7 +33,6 @@ from lms.djangoapps.courseware.access_response import ( from lms.djangoapps.courseware.access_utils import ( ACCESS_DENIED, ACCESS_GRANTED, - adjust_start_date, check_course_open_for_learner, check_start_date, debug, @@ -67,7 +66,6 @@ from common.djangoapps.util.milestones_helpers import ( from xmodule.course_module import CATALOG_VISIBILITY_ABOUT, CATALOG_VISIBILITY_CATALOG_AND_ABOUT, CourseBlock # lint-amnesty, pylint: disable=wrong-import-order from xmodule.error_module import ErrorBlock # lint-amnesty, pylint: disable=wrong-import-order from xmodule.partitions.partitions import NoSuchUserPartitionError, NoSuchUserPartitionGroupError # lint-amnesty, pylint: disable=wrong-import-order -from xmodule.x_module import XModule # lint-amnesty, pylint: disable=wrong-import-order log = logging.getLogger(__name__) @@ -152,9 +150,6 @@ def has_access(user, action, obj, course_key=None): if isinstance(obj, ErrorBlock): return _has_access_error_desc(user, action, obj, course_key) - if isinstance(obj, XModule): - return _has_access_xmodule(user, action, obj, course_key) - # NOTE: any descriptor access checkers need to go above this if isinstance(obj, XBlock): return _has_access_descriptor(user, action, obj, course_key) @@ -700,29 +695,6 @@ def _dispatch(table, action, user, obj): type(obj), action)) -def _adjust_start_date_for_beta_testers(user, descriptor, course_key): - """ - If user is in a beta test group, adjust the start date by the appropriate number of - days. - - Arguments: - user: A django user. May be anonymous. - descriptor: the XModuleDescriptor the user is trying to get access to, with a - non-None start date. - - Returns: - A datetime. Either the same as start, or earlier for beta testers. - - NOTE: number of days to adjust should be cached to avoid looking it up thousands of - times per query. - - NOTE: For now, this function assumes that the descriptor's location is in the course - the user is looking at. Once we have proper usages and definitions per the XBlock - design, this should use the course the usage is in. - """ - return adjust_start_date(user, descriptor.days_early_for_beta, descriptor.start, course_key) - - def _has_instructor_access_to_location(user, location, course_key=None): if course_key is None: course_key = location.course_key diff --git a/lms/djangoapps/courseware/rules.py b/lms/djangoapps/courseware/rules.py index 25931b6a89..76e30b64a8 100644 --- a/lms/djangoapps/courseware/rules.py +++ b/lms/djangoapps/courseware/rules.py @@ -21,7 +21,6 @@ from common.djangoapps.student.models import CourseAccessRole from common.djangoapps.student.roles import CourseRole, OrgRole from xmodule.course_module import CourseBlock # lint-amnesty, pylint: disable=wrong-import-order from xmodule.error_module import ErrorBlock # lint-amnesty, pylint: disable=wrong-import-order -from xmodule.x_module import XModule # lint-amnesty, pylint: disable=wrong-import-order from .access import has_access @@ -106,7 +105,7 @@ class HasStaffAccessToContent(Rule): # (start with more specific types, then get more general) if isinstance(instance, (CourseBlock, CourseOverview)): course_key = instance.id - elif isinstance(instance, (ErrorBlock, XModule, XBlock)): + elif isinstance(instance, (ErrorBlock, XBlock)): course_key = instance.scope_ids.usage_id.course_key elif isinstance(instance, CourseKey): course_key = instance @@ -164,7 +163,7 @@ class HasRolesRule(Rule): # lint-amnesty, pylint: disable=abstract-method, miss course_key = instance elif isinstance(instance, (CourseBlock, CourseOverview)): course_key = instance.id - elif isinstance(instance, (ErrorBlock, XModule, XBlock)): + elif isinstance(instance, (ErrorBlock, XBlock)): course_key = instance.scope_ids.usage_id.course_key else: course_key = CourseKey.from_string(str(instance)) diff --git a/lms/djangoapps/courseware/tests/test_module_render.py b/lms/djangoapps/courseware/tests/test_module_render.py index 1cf23dd81e..d9d8aeae0e 100644 --- a/lms/djangoapps/courseware/tests/test_module_render.py +++ b/lms/djangoapps/courseware/tests/test_module_render.py @@ -3,7 +3,6 @@ Test for lms courseware app, module render unit """ -import itertools import json import textwrap from datetime import datetime @@ -57,7 +56,7 @@ from xmodule.modulestore.tests.django_utils import ( from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory, ToyCourseFactory, check_mongo_calls # lint-amnesty, pylint: disable=wrong-import-order from xmodule.modulestore.tests.test_asides import AsideTestType # lint-amnesty, pylint: disable=wrong-import-order from xmodule.video_module import VideoBlock # lint-amnesty, pylint: disable=wrong-import-order -from xmodule.x_module import STUDENT_VIEW, CombinedSystem, XModule, XModuleDescriptor # lint-amnesty, pylint: disable=wrong-import-order +from xmodule.x_module import STUDENT_VIEW, CombinedSystem # 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 @@ -103,20 +102,6 @@ class PureXBlock(XBlock): pass # lint-amnesty, pylint: disable=unnecessary-pass -class EmptyXModule(XModule): # pylint: disable=abstract-method - """ - Empty XModule for testing with no dependencies. - """ - pass # lint-amnesty, pylint: disable=unnecessary-pass - - -class EmptyXModuleDescriptor(XModuleDescriptor): # pylint: disable=abstract-method - """ - Empty XModule for testing with no dependencies. - """ - module_class = EmptyXModule - - class GradedStatelessXBlock(XBlock): """ This XBlock exists to test grade storage for blocks that don't store @@ -1857,11 +1842,6 @@ PER_STUDENT_ANONYMIZED_XBLOCKS = [ VideoBlock, ] -# The "set" here is to work around the bug that load_classes returns duplicates for multiply-declared classes. -PER_STUDENT_ANONYMIZED_DESCRIPTORS = sorted(set([ - class_ for (name, class_) in XModuleDescriptor.load_classes() -] + PER_STUDENT_ANONYMIZED_XBLOCKS), key=str) - @ddt.ddt class TestAnonymousStudentId(SharedModuleStoreTestCase, LoginEnrollmentTestCase): @@ -1919,7 +1899,7 @@ class TestAnonymousStudentId(SharedModuleStoreTestCase, LoginEnrollmentTestCase) current_user = module.xmodule_runtime.service(module, 'user').get_current_user() return current_user.opt_attrs.get(ATTR_KEY_ANONYMOUS_USER_ID) - @ddt.data(*PER_STUDENT_ANONYMIZED_DESCRIPTORS) + @ddt.data(*PER_STUDENT_ANONYMIZED_XBLOCKS) def test_per_student_anonymized_id(self, descriptor_class): for course_id in ('MITx/6.00x/2012_Fall', 'MITx/6.00x/2013_Spring'): assert 'de619ab51c7f4e9c7216b4644c24f3b5' == \ @@ -2231,15 +2211,13 @@ class TestEventPublishing(ModuleStoreTestCase, LoginEnrollmentTestCase): self.mock_user.id = 1 self.request_factory = RequestFactoryNoCsrf() - @ddt.data('xblock', 'xmodule') @XBlock.register_temp_plugin(PureXBlock, identifier='xblock') - @XBlock.register_temp_plugin(EmptyXModuleDescriptor, identifier='xmodule') @patch.object(render, 'make_track_function') - def test_event_publishing(self, block_type, mock_track_function): + def test_event_publishing(self, mock_track_function): request = self.request_factory.get('') request.user = self.mock_user course = CourseFactory() - descriptor = ItemFactory(category=block_type, parent=course) + descriptor = ItemFactory(category='xblock', parent=course) field_data_cache = FieldDataCache([course, descriptor], course.id, self.mock_user) block = render.get_module(self.mock_user, request, descriptor.location, field_data_cache) @@ -2318,22 +2296,6 @@ class PureXBlockWithChildren(PureXBlock): has_children = True -class EmptyXModuleWithChildren(EmptyXModule): # pylint: disable=abstract-method - """ - Empty XModule for testing with no dependencies. - """ - has_children = True - - -class EmptyXModuleDescriptorWithChildren(EmptyXModuleDescriptor): # pylint: disable=abstract-method - """ - Empty XModule for testing with no dependencies. - """ - module_class = EmptyXModuleWithChildren - has_children = True - - -BLOCK_TYPES = ['xblock', 'xmodule'] USER_NUMBERS = list(range(2)) @@ -2354,81 +2316,58 @@ class TestFilteredChildren(SharedModuleStoreTestCase): patcher.start() self.addCleanup(patcher.stop) - @ddt.data(*BLOCK_TYPES) @XBlock.register_temp_plugin(PureXBlockWithChildren, identifier='xblock') - @XBlock.register_temp_plugin(EmptyXModuleDescriptorWithChildren, identifier='xmodule') - def test_unbound(self, block_type): - block = self._load_block(block_type) + def test_unbound(self): + block = self._load_block() self.assertUnboundChildren(block) - @ddt.data(*itertools.product(BLOCK_TYPES, USER_NUMBERS)) - @ddt.unpack + @ddt.data(*USER_NUMBERS) @XBlock.register_temp_plugin(PureXBlockWithChildren, identifier='xblock') - @XBlock.register_temp_plugin(EmptyXModuleDescriptorWithChildren, identifier='xmodule') - def test_unbound_then_bound_as_descriptor(self, block_type, user_number): + def test_unbound_then_bound_as_descriptor(self, user_number): user = self.users[user_number] - block = self._load_block(block_type) + block = self._load_block() self.assertUnboundChildren(block) self._bind_block(block, user) self.assertBoundChildren(block, user) - @ddt.data(*itertools.product(BLOCK_TYPES, USER_NUMBERS)) - @ddt.unpack + @ddt.data(*USER_NUMBERS) @XBlock.register_temp_plugin(PureXBlockWithChildren, identifier='xblock') - @XBlock.register_temp_plugin(EmptyXModuleDescriptorWithChildren, identifier='xmodule') - def test_unbound_then_bound_as_xmodule(self, block_type, user_number): + def test_unbound_then_bound_as_xmodule(self, user_number): user = self.users[user_number] - block = self._load_block(block_type) + block = self._load_block() self.assertUnboundChildren(block) self._bind_block(block, user) - - # Validate direct XModule access as well - if isinstance(block, XModuleDescriptor): - self.assertBoundChildren(block._xmodule, user) # pylint: disable=protected-access - else: - self.assertBoundChildren(block, user) - - @ddt.data(*itertools.product(BLOCK_TYPES, USER_NUMBERS)) - @ddt.unpack - @XBlock.register_temp_plugin(PureXBlockWithChildren, identifier='xblock') - @XBlock.register_temp_plugin(EmptyXModuleDescriptorWithChildren, identifier='xmodule') - def test_bound_only_as_descriptor(self, block_type, user_number): - user = self.users[user_number] - block = self._load_block(block_type) - self._bind_block(block, user) self.assertBoundChildren(block, user) - @ddt.data(*itertools.product(BLOCK_TYPES, USER_NUMBERS)) - @ddt.unpack + @ddt.data(*USER_NUMBERS) @XBlock.register_temp_plugin(PureXBlockWithChildren, identifier='xblock') - @XBlock.register_temp_plugin(EmptyXModuleDescriptorWithChildren, identifier='xmodule') - def test_bound_only_as_xmodule(self, block_type, user_number): + def test_bound_only_as_descriptor(self, user_number): user = self.users[user_number] - block = self._load_block(block_type) + block = self._load_block() self._bind_block(block, user) + self.assertBoundChildren(block, user) - # Validate direct XModule access as well - if isinstance(block, XModuleDescriptor): - self.assertBoundChildren(block._xmodule, user) # pylint: disable=protected-access - else: - self.assertBoundChildren(block, user) + @ddt.data(*USER_NUMBERS) + @XBlock.register_temp_plugin(PureXBlockWithChildren, identifier='xblock') + def test_bound_only_as_xmodule(self, user_number): + user = self.users[user_number] + block = self._load_block() + self._bind_block(block, user) + self.assertBoundChildren(block, user) - def _load_block(self, block_type): + def _load_block(self): """ - Instantiate an XBlock of `block_type` with the appropriate set of children. + Instantiate an XBlock with the appropriate set of children. """ - self.parent = ItemFactory(category=block_type, parent=self.course) + self.parent = ItemFactory(category='xblock', parent=self.course) - # Create a child of each block type for each user + # Create a child for each user self.children_for_user = { - user: [ - ItemFactory(category=child_type, parent=self.parent).scope_ids.usage_id # lint-amnesty, pylint: disable=no-member - for child_type in BLOCK_TYPES - ] + user: ItemFactory(category='xblock', parent=self.parent).scope_ids.usage_id # lint-amnesty, pylint: disable=no-member for user in self.users.values() } - self.all_children = sum(list(self.children_for_user.values()), []) + self.all_children = self.children_for_user.values() return modulestore().get_item(self.parent.scope_ids.usage_id) # lint-amnesty, pylint: disable=no-member @@ -2465,13 +2404,13 @@ class TestFilteredChildren(SharedModuleStoreTestCase): key = obj if key == self.parent.scope_ids.usage_id: # lint-amnesty, pylint: disable=no-member return AccessResponse(True) - return AccessResponse(key in self.children_for_user[user]) + return AccessResponse(key == self.children_for_user[user]) def assertBoundChildren(self, block, user): """ Ensure the bound children are indeed children. """ - self.assertChildren(block, self.children_for_user[user]) + self.assertChildren(block, [self.children_for_user[user]]) def assertUnboundChildren(self, block): """ diff --git a/lms/djangoapps/lms_xblock/runtime.py b/lms/djangoapps/lms_xblock/runtime.py index d32e0951e3..859d01c156 100644 --- a/lms/djangoapps/lms_xblock/runtime.py +++ b/lms/djangoapps/lms_xblock/runtime.py @@ -47,12 +47,6 @@ def handler_url(block, handler_name, suffix='', query='', thirdparty=False): if not func: raise ValueError(f"{handler_name!r} is not a function name") - # Is the following necessary? ProxyAttribute causes an UndefinedContext error - # if trying this without the module system. - # - #if not getattr(func, "_is_xblock_handler", False): - # raise ValueError("{!r} is not a handler name".format(handler_name)) - if thirdparty: view_name = 'xblock_handler_noauth' diff --git a/openedx/core/lib/xblock_utils/__init__.py b/openedx/core/lib/xblock_utils/__init__.py index fafa203a4e..43ec280348 100644 --- a/openedx/core/lib/xblock_utils/__init__.py +++ b/openedx/core/lib/xblock_utils/__init__.py @@ -32,8 +32,7 @@ from xmodule.seq_module import SequenceBlock # lint-amnesty, pylint: disable=wr from xmodule.util.xmodule_django import add_webpack_to_fragment # lint-amnesty, pylint: disable=wrong-import-order from xmodule.vertical_block import VerticalBlock # lint-amnesty, pylint: disable=wrong-import-order from xmodule.x_module import ( # lint-amnesty, pylint: disable=wrong-import-order - PREVIEW_VIEWS, STUDENT_VIEW, STUDIO_VIEW, - XModule, XModuleDescriptor, shim_xmodule_js, + PREVIEW_VIEWS, STUDENT_VIEW, STUDIO_VIEW, shim_xmodule_js, ) log = logging.getLogger(__name__) @@ -114,7 +113,7 @@ def wrap_xblock( if view == STUDENT_VIEW and getattr(block, 'HIDDEN', False): css_classes.append('is-hidden') - if isinstance(block, (XModule, XModuleDescriptor)) or getattr(block, 'uses_xmodule_styles_setup', False): + if getattr(block, 'uses_xmodule_styles_setup', False): if view in PREVIEW_VIEWS: # The block is acting as an XModule css_classes.append('xmodule_display') @@ -124,10 +123,6 @@ def wrap_xblock( css_classes.append('xmodule_' + markupsafe.escape(class_name)) - if isinstance(block, (XModule, XModuleDescriptor)): - data['type'] = block.js_module_name - shim_xmodule_js(frag, block.js_module_name) - if frag.js_init_fn: data['init'] = frag.js_init_fn data['runtime-class'] = runtime_class @@ -155,10 +150,6 @@ def wrap_xblock( else: template_context['js_init_parameters'] = "" - if isinstance(block, (XModule, XModuleDescriptor)): - # Add the webpackified asset tags - add_webpack_to_fragment(frag, class_name) - return wrap_fragment(frag, render_to_string('xblock_wrapper.html', template_context))