diff --git a/cms/djangoapps/contentstore/views/course.py b/cms/djangoapps/contentstore/views/course.py index 7bad397cbaf712341f695317f62d2fa1f251bc2c..6b2e40bde1c8ae15876b71fb0db206c0837f0f4c 100644 --- a/cms/djangoapps/contentstore/views/course.py +++ b/cms/djangoapps/contentstore/views/course.py @@ -72,7 +72,7 @@ from common.djangoapps.util.string_utils import _has_non_ascii_characters from common.djangoapps.xblock_django.api import deprecated_xblocks from xmodule.contentstore.content import StaticContent from xmodule.course_module import DEFAULT_START_DATE, CourseFields -from xmodule.error_module import ErrorDescriptor +from xmodule.error_module import ErrorBlock from xmodule.modulestore import EdxJSONEncoder from xmodule.modulestore.django import modulestore from xmodule.modulestore.exceptions import DuplicateCourseError, ItemNotFoundError @@ -412,7 +412,7 @@ def _accessible_courses_iter(request): """ Filter out unusable and inaccessible courses """ - if isinstance(course, ErrorDescriptor): + if isinstance(course, ErrorBlock): return False # Custom Courses for edX (CCX) is an edX feature for re-using course content. @@ -499,7 +499,7 @@ def _accessible_libraries_iter(user, org=None): libraries = [] if org == '' else modulestore().get_libraries(org=org) else: libraries = modulestore().get_library_summaries() - # No need to worry about ErrorDescriptors - split's get_libraries() never returns them. + # No need to worry about ErrorBlocks - split's get_libraries() never returns them. return (lib for lib in libraries if has_studio_read_access(user, lib.location.library_key)) @@ -751,7 +751,7 @@ def _process_courses_list(courses_iter, in_process_course_actions, split_archive archived_courses = [] for course in courses_iter: - if isinstance(course, ErrorDescriptor) or (course.id in in_process_action_course_keys): + if isinstance(course, ErrorBlock) or (course.id in in_process_action_course_keys): continue formatted_course = format_course_for_view(course) diff --git a/cms/djangoapps/contentstore/views/preview.py b/cms/djangoapps/contentstore/views/preview.py index 81b1f813da29e1cf1fce87a7c65b54d84326c9e2..fde3acbdc01d8de74c6bd4fce6d708ab560bce41 100644 --- a/cms/djangoapps/contentstore/views/preview.py +++ b/cms/djangoapps/contentstore/views/preview.py @@ -32,7 +32,7 @@ from openedx.core.lib.xblock_utils import ( ) from common.djangoapps.xblock_django.user_service import DjangoXBlockUserService from xmodule.contentstore.django import contentstore -from xmodule.error_module import ErrorDescriptor +from xmodule.error_module import ErrorBlock from xmodule.exceptions import NotFoundError, ProcessingError from xmodule.modulestore.django import ModuleI18nService, modulestore from xmodule.partitions.partitions_service import PartitionService @@ -209,7 +209,7 @@ def _preview_module_system(request, descriptor, field_data): # Set up functions to modify the fragment produced by student_view wrappers=wrappers, wrappers_asides=wrappers_asides, - error_descriptor_class=ErrorDescriptor, + error_descriptor_class=ErrorBlock, get_user_role=lambda: get_user_role(request.user, course_id), # Get the raw DescriptorSystem, not the CombinedSystem descriptor_runtime=descriptor._runtime, # pylint: disable=protected-access diff --git a/common/djangoapps/student/tests/test_course_listing.py b/common/djangoapps/student/tests/test_course_listing.py index 00edc920eeca213c00cfbf40fe40b9d1c6d91cd0..ac40dc500d5f7328ea8bf30a5901b6e12bbfbfb2 100644 --- a/common/djangoapps/student/tests/test_course_listing.py +++ b/common/djangoapps/student/tests/test_course_listing.py @@ -18,7 +18,7 @@ from common.djangoapps.student.roles import GlobalStaff from common.djangoapps.student.tests.factories import UserFactory from common.djangoapps.student.views import get_course_enrollments from common.djangoapps.util.milestones_helpers import get_pre_requisite_courses_not_completed, set_prerequisite_courses -from xmodule.error_module import ErrorDescriptor +from xmodule.error_module import ErrorBlock from xmodule.modulestore import ModuleStoreEnum from xmodule.modulestore.django import modulestore from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase @@ -104,7 +104,7 @@ class TestCourseListing(ModuleStoreTestCase, MilestonesTestCaseMixin): def test_errored_course_regular_access(self): """ - Test the course list for regular staff when get_course returns an ErrorDescriptor + Test the course list for regular staff when get_course returns an ErrorBlock """ # pylint: disable=protected-access mongo_store = modulestore()._get_modulestore_by_type(ModuleStoreEnum.Type.mongo) @@ -112,7 +112,7 @@ class TestCourseListing(ModuleStoreTestCase, MilestonesTestCaseMixin): self._create_course_with_access_groups(course_key, default_store=ModuleStoreEnum.Type.mongo) with mock.patch('xmodule.modulestore.mongo.base.MongoKeyValueStore', mock.Mock(side_effect=Exception)): - self.assertIsInstance(modulestore().get_course(course_key), ErrorDescriptor) + self.assertIsInstance(modulestore().get_course(course_key), ErrorBlock) # Invalidate (e.g., delete) the corresponding CourseOverview, forcing get_course to be called. CourseOverview.objects.filter(id=course_key).delete() diff --git a/common/lib/xmodule/setup.py b/common/lib/xmodule/setup.py index 09d193d04930912c7c66172220e1050b5f16e5ad..75ca6b95213b50573beb49d4c5cfaac62028645e 100644 --- a/common/lib/xmodule/setup.py +++ b/common/lib/xmodule/setup.py @@ -9,8 +9,6 @@ XMODULES = [ "customtag = xmodule.template_module:CustomTagDescriptor", "discuss = xmodule.backcompat_module:TranslateCustomTagDescriptor", "image = xmodule.backcompat_module:TranslateCustomTagDescriptor", - "error = xmodule.error_module:ErrorDescriptor", - "nonstaff_error = xmodule.error_module:NonStaffErrorDescriptor", "poll_question = xmodule.poll_module:PollDescriptor", "problemset = xmodule.seq_module:SequenceDescriptor", "section = xmodule.backcompat_module:SemanticSectionDescriptor", @@ -28,10 +26,12 @@ XBLOCKS = [ "annotatable = xmodule.annotatable_module:AnnotatableBlock", "conditional = xmodule.conditional_module:ConditionalBlock", "course_info = xmodule.html_module:CourseInfoBlock", + "error = xmodule.error_module:ErrorBlock", "html = xmodule.html_module:HtmlBlock", "library = xmodule.library_root_xblock:LibraryRoot", "library_content = xmodule.library_content_module:LibraryContentBlock", "library_sourced = xmodule.library_sourced_block:LibrarySourcedBlock", + "nonstaff_error = xmodule.error_module:NonStaffErrorBlock", "problem = xmodule.capa_module:ProblemBlock", "randomize = xmodule.randomize_module:RandomizeBlock", "split_test = xmodule.split_test_module:SplitTestBlock", diff --git a/common/lib/xmodule/xmodule/conditional_module.py b/common/lib/xmodule/xmodule/conditional_module.py index fb017339303b91a0814d668413cdbee587915dbb..1b842d545597bd93f8ed2a308b2a57f509dfddae 100644 --- a/common/lib/xmodule/xmodule/conditional_module.py +++ b/common/lib/xmodule/xmodule/conditional_module.py @@ -216,7 +216,7 @@ class ConditionalBlock( if not hasattr(module, attr_name): # We don't throw an exception here because it is possible for # the descriptor of a required module to have a property but - # for the resulting module to be a (flavor of) ErrorModule. + # for the resulting module to be a (flavor of) ErrorBlock. # So just log and return false. if module is not None: # We do not want to log when module is None, and it is when requester diff --git a/common/lib/xmodule/xmodule/error_module.py b/common/lib/xmodule/xmodule/error_module.py index 604ba71f7ddb226864131358123730df8d44fd92..a6b52aa5eb0a0d8a2262f51b9783e1c22fb38048 100644 --- a/common/lib/xmodule/xmodule/error_module.py +++ b/common/lib/xmodule/xmodule/error_module.py @@ -11,18 +11,25 @@ import sys import six from lxml import etree +from web_fragments.fragment import Fragment from xblock.field_data import DictFieldData from xblock.fields import Scope, ScopeIds, String from xmodule.errortracker import exc_info_to_str from xmodule.modulestore import EdxJSONEncoder -from xmodule.x_module import XModule, XModuleDescriptor +from xmodule.x_module import ( + HTMLSnippet, + ResourceTemplates, + XModuleMixin, + XModuleDescriptorToXBlockMixin, + XModuleToXBlockMixin, +) log = logging.getLogger(__name__) # NOTE: This is not the most beautiful design in the world, but there's no good # way to tell if the module is being used in a staff context or not. Errors that get discovered -# at course load time are turned into ErrorDescriptor objects, and automatically hidden from students. +# at course load time are turned into ErrorBlock objects, and automatically hidden from students. # Unfortunately, we can also have errors when loading modules mid-request, and then we need to decide # what to show, and the logic for that belongs in the LMS (e.g. in get_module), so the error handler # decides whether to create a staff or not-staff module. @@ -30,62 +37,49 @@ log = logging.getLogger(__name__) class ErrorFields(object): """ - XBlock fields used by the ErrorModules + XBlock fields used by the ErrorBlocks """ contents = String(scope=Scope.content) error_msg = String(scope=Scope.content) display_name = String(scope=Scope.settings) -class ErrorModule(ErrorFields, XModule): +class ErrorBlock( + ErrorFields, + XModuleDescriptorToXBlockMixin, + XModuleToXBlockMixin, + HTMLSnippet, + ResourceTemplates, + XModuleMixin, +): # pylint: disable=abstract-method """ Module that gets shown to staff when there has been an error while loading or rendering other modules """ - def get_html(self): - '''Show an error to staff. - TODO (vshnayder): proper style, divs, etc. - ''' - # staff get to see all the details - return self.system.render_template('module-error.html', { + resources_dir = None + + def student_view(self, _context): + """ + Return a fragment that contains the html for the student view. + """ + fragment = Fragment(self.system.render_template('module-error.html', { 'staff_access': True, 'data': self.contents, 'error': self.error_msg, - }) - - -class NonStaffErrorModule(ErrorFields, XModule): - """ - Module that gets shown to students when there has been an error while - loading or rendering other modules - """ - def get_html(self): - '''Show an error to a student. - TODO (vshnayder): proper style, divs, etc. - ''' - # staff get to see all the details - return self.system.render_template('module-error.html', { - 'staff_access': False, - 'data': "", - 'error': "", - }) - - -class ErrorDescriptor(ErrorFields, XModuleDescriptor): - """ - Module that provides a raw editing view of broken xml. - """ - module_class = ErrorModule - resources_dir = None + })) + return fragment - def get_html(self): - return u'' + def studio_view(self, _context): + """ + Show empty edit view since this is not editable. + """ + return Fragment('') @classmethod def _construct(cls, system, contents, error_msg, location, for_parent=None): """ - Build a new ErrorDescriptor. using ``system``. + Build a new ErrorBlock using ``system``. Arguments: system (:class:`DescriptorSystem`): The :class:`DescriptorSystem` used @@ -106,7 +100,7 @@ class ErrorDescriptor(ErrorFields, XModuleDescriptor): # Pick a unique url_name -- the sha1 hash of the contents. # NOTE: We could try to pull out the url_name of the errored descriptor, # but url_names aren't guaranteed to be unique between descriptor types, - # and ErrorDescriptor can wrap any type. When the wrapped module is fixed, + # and ErrorBlock can wrap any type. When the wrapped module is fixed, # it will be written out with the original url_name. name=hashlib.sha1(contents.encode('utf8')).hexdigest() ) @@ -207,8 +201,18 @@ class ErrorDescriptor(ErrorFields, XModuleDescriptor): return etree.tostring(root, encoding='unicode') -class NonStaffErrorDescriptor(ErrorDescriptor): +class NonStaffErrorBlock(ErrorBlock): # pylint: disable=abstract-method """ - Module that provides non-staff error messages. + Block that gets shown to students when there has been an error while + loading or rendering other blocks. """ - module_class = NonStaffErrorModule + def student_view(self, _context): + """ + Return a fragment that contains the html for the student view. + """ + fragment = Fragment(self.system.render_template('module-error.html', { + 'staff_access': False, + 'data': '', + 'error': '', + })) + return fragment diff --git a/common/lib/xmodule/xmodule/modulestore/mongo/base.py b/common/lib/xmodule/xmodule/modulestore/mongo/base.py index 2aedf6f1e63611a00a447588f84bbcd03d6cc911..ded0ad545fe5da1886fd371e54e4a3d9feef1395 100644 --- a/common/lib/xmodule/xmodule/modulestore/mongo/base.py +++ b/common/lib/xmodule/xmodule/modulestore/mongo/base.py @@ -38,7 +38,7 @@ from xblock.runtime import KvsFieldData from xmodule.assetstore import AssetMetadata, CourseAssetsFromStorage from xmodule.course_module import CourseSummary -from xmodule.error_module import ErrorDescriptor +from xmodule.error_module import ErrorBlock from xmodule.errortracker import exc_info_to_str, null_error_tracker from xmodule.exceptions import HeartbeatFailure from xmodule.mako_module import MakoDescriptorSystem @@ -330,7 +330,7 @@ class CachingDescriptorSystem(MakoDescriptorSystem, EditInfoRuntimeMixin): return module except Exception: # pylint: disable=broad-except log.warning("Failed to load descriptor from %s", json_data, exc_info=True) - return ErrorDescriptor.from_json( + return ErrorBlock.from_json( json_data, self, location, @@ -1071,7 +1071,7 @@ class MongoModuleStore(ModuleStoreDraftAndPublished, ModuleStoreWriteBase, Mongo ], [] ) - return [course for course in base_list if not isinstance(course, ErrorDescriptor)] + return [course for course in base_list if not isinstance(course, ErrorBlock)] @autoretry_read() def _find_one(self, location): diff --git a/common/lib/xmodule/xmodule/modulestore/split_mongo/caching_descriptor_system.py b/common/lib/xmodule/xmodule/modulestore/split_mongo/caching_descriptor_system.py index e0a06cb0824e6fd4b51435d85454b76724128e88..06a21feb9b0963b15798063224cf534d63566e5f 100644 --- a/common/lib/xmodule/xmodule/modulestore/split_mongo/caching_descriptor_system.py +++ b/common/lib/xmodule/xmodule/modulestore/split_mongo/caching_descriptor_system.py @@ -12,7 +12,7 @@ from xblock.core import XBlock from xblock.fields import ScopeIds from xblock.runtime import KeyValueStore, KvsFieldData -from xmodule.error_module import ErrorDescriptor +from xmodule.error_module import ErrorBlock from xmodule.errortracker import exc_info_to_str from xmodule.library_tools import LibraryToolsService from xmodule.mako_module import MakoDescriptorSystem @@ -258,7 +258,7 @@ class CachingDescriptorSystem(MakoDescriptorSystem, EditInfoRuntimeMixin): ) except Exception: # pylint: disable=broad-except log.warning("Failed to load descriptor", exc_info=True) - return ErrorDescriptor.from_json( + return ErrorBlock.from_json( block_data, self, course_entry_override.course_key.make_usage_key( diff --git a/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py b/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py index f8321d20b65bb6e85058d9361e2cc1df05ec49ac..455c9568b6d8dbea7312a7aab7607e0b20bdd969 100644 --- a/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py +++ b/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py @@ -83,7 +83,7 @@ from xblock.fields import Reference, ReferenceList, ReferenceValueDict, Scope from xmodule.assetstore import AssetMetadata from xmodule.course_module import CourseSummary -from xmodule.error_module import ErrorDescriptor +from xmodule.error_module import ErrorBlock from xmodule.errortracker import null_error_tracker from xmodule.library_content_module import LibrarySummary from xmodule.modulestore import ( @@ -988,7 +988,7 @@ class SplitMongoModuleStore(SplitBulkWriteMixin, ModuleStoreWriteBase): envelope = CourseEnvelope(locator, entry) root = entry['root'] structures_list = self._load_items(envelope, [root], depth=0, **kwargs) - if not isinstance(structures_list[0], ErrorDescriptor): + if not isinstance(structures_list[0], ErrorBlock): result.append(structures_list[0]) return result diff --git a/common/lib/xmodule/xmodule/modulestore/xml.py b/common/lib/xmodule/xmodule/modulestore/xml.py index 60c22ef628990595cce7e54695e40257db76ca98..ddf627b89c0a77b5423c99824fae482afc684817 100644 --- a/common/lib/xmodule/xmodule/modulestore/xml.py +++ b/common/lib/xmodule/xmodule/modulestore/xml.py @@ -27,7 +27,7 @@ from xblock.field_data import DictFieldData from xblock.fields import ScopeIds from xblock.runtime import DictKeyValueStore -from xmodule.error_module import ErrorDescriptor +from xmodule.error_module import ErrorBlock from xmodule.errortracker import exc_info_to_str, make_error_tracker from xmodule.mako_module import MakoDescriptorSystem from xmodule.modulestore import COURSE_ROOT, LIBRARY_ROOT, ModuleStoreEnum, ModuleStoreReadBase @@ -194,7 +194,7 @@ class ImportSystem(XMLParsingSystem, MakoDescriptorSystem): self.error_tracker(msg) err_msg = msg + "\n" + exc_info_to_str(sys.exc_info()) - descriptor = ErrorDescriptor.from_xml( + descriptor = ErrorBlock.from_xml( xml, self, id_manager, @@ -390,7 +390,7 @@ class XMLModuleStore(ModuleStoreReadBase): if course_descriptor is None: pass - elif isinstance(course_descriptor, ErrorDescriptor): + elif isinstance(course_descriptor, ErrorBlock): # Didn't load course. Instead, save the errors elsewhere. self.errored_courses[course_dir] = errorlog else: @@ -529,7 +529,7 @@ class XMLModuleStore(ModuleStoreReadBase): ) course_descriptor = system.process_xml(etree.tostring(course_data, encoding='unicode')) # If we fail to load the course, then skip the rest of the loading steps - if isinstance(course_descriptor, ErrorDescriptor): + if isinstance(course_descriptor, ErrorBlock): return course_descriptor self.content_importers(system, course_descriptor, course_dir, url_name) @@ -783,7 +783,7 @@ class XMLModuleStore(ModuleStoreReadBase): def get_courses(self, **kwargs): """ Returns a list of course descriptors. If there were errors on loading, - some of these may be ErrorDescriptors instead. + some of these may be ErrorBlock instead. """ return list(self.courses.values()) diff --git a/common/lib/xmodule/xmodule/tests/__init__.py b/common/lib/xmodule/xmodule/tests/__init__.py index 9c954d20514660edd439ccfcc2bd36b3bf49b54a..24b6b50bf02f47a89cc3c27198ce91f458385dd6 100644 --- a/common/lib/xmodule/xmodule/tests/__init__.py +++ b/common/lib/xmodule/xmodule/tests/__init__.py @@ -29,7 +29,7 @@ from xblock.field_data import DictFieldData from xblock.fields import Reference, ReferenceList, ReferenceValueDict, ScopeIds from xmodule.assetstore import AssetMetadata -from xmodule.error_module import ErrorDescriptor +from xmodule.error_module import ErrorBlock from xmodule.mako_module import MakoDescriptorSystem from xmodule.modulestore import ModuleStoreEnum from xmodule.modulestore.draft_and_published import ModuleStoreDraftAndPublished @@ -146,7 +146,7 @@ def get_test_system( node_path=os.environ.get("NODE_PATH", "/usr/local/lib/node_modules"), anonymous_student_id='student', course_id=course_id, - error_descriptor_class=ErrorDescriptor, + error_descriptor_class=ErrorBlock, get_user_role=Mock(name='get_test_system.get_user_role', is_staff=False), user_location=Mock(name='get_test_system.user_location'), descriptor_runtime=descriptor_system, diff --git a/common/lib/xmodule/xmodule/tests/test_conditional.py b/common/lib/xmodule/xmodule/tests/test_conditional.py index 5155a0f787b245db1d152c08c7e445fe50f9e50d..d2616fc647908e4014f1b1876d7b3eb144e1ad31 100644 --- a/common/lib/xmodule/xmodule/tests/test_conditional.py +++ b/common/lib/xmodule/xmodule/tests/test_conditional.py @@ -14,7 +14,7 @@ from xblock.field_data import DictFieldData from xblock.fields import ScopeIds from xmodule.conditional_module import ConditionalBlock -from xmodule.error_module import NonStaffErrorDescriptor +from xmodule.error_module import NonStaffErrorBlock from xmodule.modulestore.xml import CourseLocationManager, ImportSystem, XMLModuleStore from xmodule.tests import DATA_DIR, get_test_descriptor_system, get_test_system from xmodule.tests.xml import XModuleXmlImportTest @@ -63,7 +63,7 @@ class ConditionalFactory(object): return a dict of modules: the conditional with a single source and a single child. Keys are 'cond_module', 'source_module', and 'child_module'. - if the source_is_error_module flag is set, create a real ErrorModule for the source. + if the source_is_error_module flag is set, create a real ErrorBlock for the source. """ descriptor_system = get_test_descriptor_system() @@ -72,7 +72,7 @@ class ConditionalFactory(object): "problem", "SampleProblem", deprecated=True) if source_is_error_module: # Make an error descriptor and module - source_descriptor = NonStaffErrorDescriptor.from_xml( + source_descriptor = NonStaffErrorBlock.from_xml( 'some random xml data', system, id_generator=CourseLocationManager(source_location.course_key), @@ -192,7 +192,7 @@ class ConditionalBlockBasicTest(unittest.TestCase): def test_error_as_source(self): ''' - Check that handle_ajax works properly if the source is really an ErrorModule, + Check that handle_ajax works properly if the source is really an ErrorBlock, and that the condition is not satisfied. ''' modules = ConditionalFactory.create(self.test_system, source_is_error_module=True) diff --git a/common/lib/xmodule/xmodule/tests/test_error_module.py b/common/lib/xmodule/xmodule/tests/test_error_module.py index 4a9f751ad123b85a29094cad436cd39cb1dad4b6..dce77fcf3ff80e36a38861204263d31089d286b5 100644 --- a/common/lib/xmodule/xmodule/tests/test_error_module.py +++ b/common/lib/xmodule/xmodule/tests/test_error_module.py @@ -1,5 +1,5 @@ """ -Tests for ErrorModule and NonStaffErrorModule +Tests for ErrorBlock and NonStaffErrorBlock """ @@ -12,17 +12,17 @@ from xblock.fields import ScopeIds from xblock.runtime import IdReader, Runtime from xblock.test.tools import unabc -from xmodule.error_module import ErrorDescriptor, ErrorModule, NonStaffErrorDescriptor +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 -class SetupTestErrorModules(unittest.TestCase): - """Common setUp for use in ErrorModule tests.""" +class SetupTestErrorBlock(unittest.TestCase): + """Common setUp for use in ErrorBlock tests.""" def setUp(self): - super(SetupTestErrorModules, self).setUp() + super().setUp() self.system = get_test_system() self.course_id = CourseLocator('org', 'course', 'run') self.location = self.course_id.make_usage_key('foo', 'bar') @@ -30,55 +30,55 @@ class SetupTestErrorModules(unittest.TestCase): self.error_msg = "Error" -class TestErrorModule(SetupTestErrorModules): +class TestErrorBlock(SetupTestErrorBlock): """ - Tests for ErrorModule and ErrorDescriptor + Tests for ErrorBlock """ - def test_error_module_xml_rendering(self): - descriptor = ErrorDescriptor.from_xml( + def test_error_block_xml_rendering(self): + descriptor = ErrorBlock.from_xml( self.valid_xml, self.system, CourseLocationManager(self.course_id), self.error_msg ) - self.assertIsInstance(descriptor, ErrorDescriptor) + self.assertIsInstance(descriptor, ErrorBlock) descriptor.xmodule_runtime = self.system context_repr = self.system.render(descriptor, STUDENT_VIEW).content self.assertIn(self.error_msg, context_repr) self.assertIn(repr(self.valid_xml), context_repr) - def test_error_module_from_descriptor(self): + def test_error_block_from_descriptor(self): descriptor = MagicMock( spec=XModuleDescriptor, runtime=self.system, location=self.location, ) - error_descriptor = ErrorDescriptor.from_descriptor( + error_descriptor = ErrorBlock.from_descriptor( descriptor, self.error_msg) - self.assertIsInstance(error_descriptor, ErrorDescriptor) + self.assertIsInstance(error_descriptor, ErrorBlock) error_descriptor.xmodule_runtime = self.system context_repr = self.system.render(error_descriptor, STUDENT_VIEW).content self.assertIn(self.error_msg, context_repr) self.assertIn(repr(descriptor), context_repr) -class TestNonStaffErrorModule(SetupTestErrorModules): +class TestNonStaffErrorBlock(SetupTestErrorBlock): """ - Tests for NonStaffErrorModule and NonStaffErrorDescriptor + Tests for NonStaffErrorBlock. """ - def test_non_staff_error_module_create(self): - descriptor = NonStaffErrorDescriptor.from_xml( + def test_non_staff_error_block_create(self): + descriptor = NonStaffErrorBlock.from_xml( self.valid_xml, self.system, CourseLocationManager(self.course_id) ) - self.assertIsInstance(descriptor, NonStaffErrorDescriptor) + self.assertIsInstance(descriptor, NonStaffErrorBlock) def test_from_xml_render(self): - descriptor = NonStaffErrorDescriptor.from_xml( + descriptor = NonStaffErrorBlock.from_xml( self.valid_xml, self.system, CourseLocationManager(self.course_id) @@ -88,16 +88,16 @@ class TestNonStaffErrorModule(SetupTestErrorModules): self.assertNotIn(self.error_msg, context_repr) self.assertNotIn(repr(self.valid_xml), context_repr) - def test_error_module_from_descriptor(self): + def test_error_block_from_descriptor(self): descriptor = MagicMock( spec=XModuleDescriptor, runtime=self.system, location=self.location, ) - error_descriptor = NonStaffErrorDescriptor.from_descriptor( + error_descriptor = NonStaffErrorBlock.from_descriptor( descriptor, self.error_msg) - self.assertIsInstance(error_descriptor, ErrorDescriptor) + self.assertIsInstance(error_descriptor, ErrorBlock) error_descriptor.xmodule_runtime = self.system context_repr = self.system.render(error_descriptor, STUDENT_VIEW).content self.assertNotIn(self.error_msg, context_repr) @@ -124,14 +124,14 @@ class TestRuntime(Runtime): pass -class TestErrorModuleConstruction(unittest.TestCase): +class TestErrorBlockConstruction(unittest.TestCase): """ - Test that error module construction happens correctly + Test that error block construction happens correctly """ def setUp(self): # pylint: disable=abstract-class-instantiated - super(TestErrorModuleConstruction, self).setUp() + super().setUp() field_data = DictFieldData({}) self.descriptor = BrokenDescriptor( TestRuntime(Mock(spec=IdReader), field_data), @@ -140,29 +140,29 @@ class TestErrorModuleConstruction(unittest.TestCase): BlockUsageLocator(CourseLocator('org', 'course', 'run'), 'broken', 'name')) ) self.descriptor.xmodule_runtime = TestRuntime(Mock(spec=IdReader), field_data) - self.descriptor.xmodule_runtime.error_descriptor_class = ErrorDescriptor + self.descriptor.xmodule_runtime.error_descriptor_class = ErrorBlock self.descriptor.xmodule_runtime.xmodule_instance = None - def test_broken_module(self): + def test_broken_block(self): """ - Test that when an XModule throws an error during __init__, we - get an ErrorModule back from XModuleDescriptor._xmodule + Test that when an XModule throws an block during __init__, we + get an ErrorBlock back from XModuleDescriptor._xmodule """ module = self.descriptor._xmodule - self.assertIsInstance(module, ErrorModule) + self.assertIsInstance(module, ErrorBlock) - @patch.object(ErrorDescriptor, '__init__', Mock(side_effect=TestException)) + @patch.object(ErrorBlock, '__init__', Mock(side_effect=TestException)) def test_broken_error_descriptor(self): """ - Test that a broken error descriptor doesn't cause an infinite loop + Test that a broken block descriptor doesn't cause an infinite loop """ with self.assertRaises(TestException): module = self.descriptor._xmodule - @patch.object(ErrorModule, '__init__', Mock(side_effect=TestException)) - def test_broken_error_module(self): + @patch.object(ErrorBlock, '__init__', Mock(side_effect=TestException)) + def test_broken_error_block(self): """ - Test that a broken error module doesn't cause an infinite loop + Test that a broken block module doesn't cause an infinite loop """ with self.assertRaises(TestException): module = self.descriptor._xmodule diff --git a/common/lib/xmodule/xmodule/tests/test_import.py b/common/lib/xmodule/xmodule/tests/test_import.py index 12d08cebac1b9fc9f8c48b83afcd346c76375b5f..1b98052398d524d2dad053bdf6a52b7822487cb1 100644 --- a/common/lib/xmodule/xmodule/tests/test_import.py +++ b/common/lib/xmodule/xmodule/tests/test_import.py @@ -122,7 +122,7 @@ class ImportTestCase(BaseCourseTestCase): date = Date() def test_fallback(self): - '''Check that malformed xml loads as an ErrorDescriptor.''' + '''Check that malformed xml loads as an ErrorBlock.''' # Use an exotic character to also flush out Unicode issues. bad_xml = u'''<sequential display_name="oops\N{SNOWMAN}"><video url="hi"></sequential>''' @@ -130,7 +130,7 @@ class ImportTestCase(BaseCourseTestCase): descriptor = system.process_xml(bad_xml) - self.assertEqual(descriptor.__class__.__name__, 'ErrorDescriptorWithMixins') + self.assertEqual(descriptor.__class__.__name__, 'ErrorBlockWithMixins') def test_unique_url_names(self): '''Check that each error gets its very own url_name''' @@ -164,7 +164,7 @@ class ImportTestCase(BaseCourseTestCase): descriptor.add_xml_to_node(node) re_import_descriptor = system.process_xml(etree.tostring(node)) - self.assertEqual(re_import_descriptor.__class__.__name__, 'ErrorDescriptorWithMixins') + self.assertEqual(re_import_descriptor.__class__.__name__, 'ErrorBlockWithMixins') self.assertEqual(descriptor.contents, re_import_descriptor.contents) self.assertEqual(descriptor.error_msg, re_import_descriptor.error_msg) @@ -627,7 +627,7 @@ class ImportTestCase(BaseCourseTestCase): ) def test_error_on_import(self): - '''Check that when load_error_module is false, an exception is raised, rather than returning an ErrorModule''' + '''Check that when load_error_module is false, an exception is raised, rather than returning an ErrorBlock''' bad_xml = '''<sequential display_name="oops"><video url="hi"></sequential>''' system = self.get_system(False) diff --git a/common/lib/xmodule/xmodule/x_module.py b/common/lib/xmodule/xmodule/x_module.py index 308bd90779148bd1fa552264cfad899bf0be7853..6f411a615388106ffe43e0f40e5c3bca55b61035 100644 --- a/common/lib/xmodule/xmodule/x_module.py +++ b/common/lib/xmodule/xmodule/x_module.py @@ -1304,11 +1304,11 @@ class XModuleDescriptor(XModuleDescriptorToXBlockMixin, HTMLSnippet, ResourceTem 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 ErrorModule instead + # 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 ErrorModule from an ErrorDescriptor') + log.exception('Error creating an ErrorBlock from an ErrorBlock') raise log.exception('Error creating xmodule') @@ -1317,7 +1317,7 @@ class XModuleDescriptor(XModuleDescriptorToXBlockMixin, HTMLSnippet, ResourceTem error_msg=exc_info_to_str(sys.exc_info()) ) descriptor.xmodule_runtime = self.xmodule_runtime - self.xmodule_runtime.xmodule_instance = descriptor._xmodule # pylint: disable=protected-access + self.xmodule_runtime.xmodule_instance = descriptor return self.xmodule_runtime.xmodule_instance course_id = module_attr('course_id') diff --git a/lms/djangoapps/ccx/models.py b/lms/djangoapps/ccx/models.py index 16d85ed26fdd978599ab2973270af34b373579fc..534f66512f73d61d605a5e8fe26766635f394bff 100644 --- a/lms/djangoapps/ccx/models.py +++ b/lms/djangoapps/ccx/models.py @@ -15,7 +15,7 @@ from lazy import lazy from opaque_keys.edx.django.models import CourseKeyField, UsageKeyField from pytz import utc -from xmodule.error_module import ErrorDescriptor +from xmodule.error_module import ErrorBlock from xmodule.modulestore.django import modulestore log = logging.getLogger("edx.ccx") @@ -43,7 +43,7 @@ class CustomCourseForEdX(models.Model): store = modulestore() with store.bulk_operations(self.course_id): course = store.get_course(self.course_id) - if not course or isinstance(course, ErrorDescriptor): + if not course or isinstance(course, ErrorBlock): log.error("CCX {0} from {2} course {1}".format( # pylint: disable=logging-format-interpolation self.display_name, self.course_id, "broken" if course else "non-existent" )) diff --git a/lms/djangoapps/courseware/access.py b/lms/djangoapps/courseware/access.py index ed389ca575a9baf6c9c27bef4fea187a6dde0c94..a9831b57ada1a9b9b96f7df332909720b912eba8 100644 --- a/lms/djangoapps/courseware/access.py +++ b/lms/djangoapps/courseware/access.py @@ -65,7 +65,7 @@ from common.djangoapps.util.milestones_helpers import ( is_prerequisite_courses_enabled ) from xmodule.course_module import CATALOG_VISIBILITY_ABOUT, CATALOG_VISIBILITY_CATALOG_AND_ABOUT, CourseDescriptor -from xmodule.error_module import ErrorDescriptor +from xmodule.error_module import ErrorBlock from xmodule.partitions.partitions import NoSuchUserPartitionError, NoSuchUserPartitionGroupError from xmodule.x_module import XModule @@ -149,7 +149,7 @@ def has_access(user, action, obj, course_key=None): if isinstance(obj, CourseOverview): return _has_access_course(user, action, obj) - if isinstance(obj, ErrorDescriptor): + if isinstance(obj, ErrorBlock): return _has_access_error_desc(user, action, obj, course_key) if isinstance(obj, XModule): diff --git a/lms/djangoapps/courseware/module_render.py b/lms/djangoapps/courseware/module_render.py index a645e514ef9226832fb5846b4e01b6b96b536e83..8ce25b9666d35143edcf3c19de808002e448c6aa 100644 --- a/lms/djangoapps/courseware/module_render.py +++ b/lms/djangoapps/courseware/module_render.py @@ -94,7 +94,7 @@ from common.djangoapps.util import milestones_helpers from common.djangoapps.util.json_request import JsonResponse from common.djangoapps.xblock_django.user_service import DjangoXBlockUserService from xmodule.contentstore.django import contentstore -from xmodule.error_module import ErrorDescriptor, NonStaffErrorDescriptor +from xmodule.error_module import ErrorBlock, NonStaffErrorBlock from xmodule.exceptions import NotFoundError, ProcessingError from xmodule.lti_module import LTIModule from xmodule.modulestore.django import modulestore @@ -322,7 +322,7 @@ def get_module(user, request, usage_key, field_data_cache, to the user. Returns: xmodule instance, or None if the user does not have access to the - module. If there's an error, will try to return an instance of ErrorModule + module. If there's an error, will try to return an instance of ErrorBlock if possible. If not possible, return None. """ try: @@ -845,11 +845,11 @@ def get_module_system_for_user( system.set(u'user_is_beta_tester', CourseBetaTesterRole(course_id).has_user(user)) system.set(u'days_early_for_beta', descriptor.days_early_for_beta) - # make an ErrorDescriptor -- assuming that the descriptor's system is ok + # make an ErrorBlock -- assuming that the descriptor's system is ok if has_access(user, u'staff', descriptor.location, course_id): - system.error_descriptor_class = ErrorDescriptor + system.error_descriptor_class = ErrorBlock else: - system.error_descriptor_class = NonStaffErrorDescriptor + system.error_descriptor_class = NonStaffErrorBlock return system, field_data diff --git a/lms/djangoapps/courseware/rules.py b/lms/djangoapps/courseware/rules.py index 7bf777b38928aef5e424fbd537a955a6fdae33f5..2e54ef771bce140b430ccd2df93fed982f9cbf17 100644 --- a/lms/djangoapps/courseware/rules.py +++ b/lms/djangoapps/courseware/rules.py @@ -21,7 +21,7 @@ from openedx.core.djangoapps.content.course_overviews.models import CourseOvervi from common.djangoapps.student.models import CourseAccessRole, CourseEnrollment from common.djangoapps.student.roles import CourseRole, OrgRole from xmodule.course_module import CourseDescriptor -from xmodule.error_module import ErrorDescriptor +from xmodule.error_module import ErrorBlock from xmodule.x_module import XModule @@ -113,7 +113,7 @@ class HasStaffAccessToContent(Rule): # (start with more specific types, then get more general) if isinstance(instance, (CourseDescriptor, CourseOverview)): course_key = instance.id - elif isinstance(instance, (ErrorDescriptor, XModule, XBlock)): + elif isinstance(instance, (ErrorBlock, XModule, XBlock)): course_key = instance.scope_ids.usage_id.course_key elif isinstance(instance, CourseKey): course_key = instance @@ -171,7 +171,7 @@ class HasRolesRule(Rule): course_key = instance elif isinstance(instance, (CourseDescriptor, CourseOverview)): course_key = instance.id - elif isinstance(instance, (ErrorDescriptor, XModule, XBlock)): + elif isinstance(instance, (ErrorBlock, XModule, 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/tests.py b/lms/djangoapps/courseware/tests/tests.py index 87ecf388874a01060021a815534cbc97d489cea5..3bad50f5a7bd7b7ae29618661d96a3eb94b930f3 100644 --- a/lms/djangoapps/courseware/tests/tests.py +++ b/lms/djangoapps/courseware/tests/tests.py @@ -13,7 +13,7 @@ from six import text_type from lms.djangoapps.courseware.tests.helpers import LoginEnrollmentTestCase from lms.djangoapps.lms_xblock.field_data import LmsFieldData -from xmodule.error_module import ErrorDescriptor +from xmodule.error_module import ErrorBlock from xmodule.modulestore.django import modulestore from xmodule.modulestore.tests.django_utils import TEST_DATA_MIXED_MODULESTORE, ModuleStoreTestCase from xmodule.modulestore.tests.factories import ToyCourseFactory @@ -119,7 +119,7 @@ class PageLoaderTestCase(LoginEnrollmentTestCase): if check_content: self.assertNotContains(response, "this module is temporarily unavailable") - self.assertNotIsInstance(descriptor, ErrorDescriptor) + self.assertNotIsInstance(descriptor, ErrorBlock) class TestMongoCoursesLoad(ModuleStoreTestCase, PageLoaderTestCase): diff --git a/openedx/core/djangoapps/content/course_overviews/models.py b/openedx/core/djangoapps/content/course_overviews/models.py index 416c86ed6e9b3a551ede13e3bf122f8a8e9583e6..92922d5d1afc9da0c918b9e374b86b781619bf8d 100644 --- a/openedx/core/djangoapps/content/course_overviews/models.py +++ b/openedx/core/djangoapps/content/course_overviews/models.py @@ -32,7 +32,7 @@ from openedx.core.lib.cache_utils import request_cached, RequestCache from common.djangoapps.static_replace.models import AssetBaseUrlConfig from xmodule import block_metadata_utils, course_metadata_utils from xmodule.course_module import DEFAULT_START_DATE, CourseDescriptor -from xmodule.error_module import ErrorDescriptor +from xmodule.error_module import ErrorBlock from xmodule.modulestore.django import modulestore from xmodule.tabs import CourseTab @@ -310,7 +310,7 @@ class CourseOverview(TimeStampedModel): "Error while loading CourseOverview for course {} " "from the module store: {}", six.text_type(course_id), - course.error_msg if isinstance(course, ErrorDescriptor) else six.text_type(course) + course.error_msg if isinstance(course, ErrorBlock) else six.text_type(course) ) else: log.info( diff --git a/openedx/core/djangoapps/content/course_overviews/tests/test_course_overviews.py b/openedx/core/djangoapps/content/course_overviews/tests/test_course_overviews.py index 6db935d35d7fdf3d4fcfee16e583dc0bbbe5f434..96eb908749f3227c6d33cb5581afac9254c0f5f9 100644 --- a/openedx/core/djangoapps/content/course_overviews/tests/test_course_overviews.py +++ b/openedx/core/djangoapps/content/course_overviews/tests/test_course_overviews.py @@ -36,7 +36,7 @@ from xmodule.course_module import ( CATALOG_VISIBILITY_CATALOG_AND_ABOUT, CATALOG_VISIBILITY_NONE ) -from xmodule.error_module import ErrorDescriptor +from xmodule.error_module import ErrorBlock from xmodule.modulestore import ModuleStoreEnum from xmodule.modulestore.django import modulestore from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase @@ -356,13 +356,13 @@ class CourseOverviewTestCase(CatalogIntegrationMixin, ModuleStoreTestCase, Cache def test_get_errored_course(self): """ - Test that getting an ErrorDescriptor back from the module store causes + Test that getting an ErrorBlock back from the module store causes load_from_module_store to raise an IOError. """ - mock_get_course = mock.Mock(return_value=ErrorDescriptor) + mock_get_course = mock.Mock(return_value=ErrorBlock) with mock.patch('xmodule.modulestore.mixed.MixedModuleStore.get_course', mock_get_course): # This mock makes it so when the module store tries to load course data, - # an exception is thrown, which causes get_course to return an ErrorDescriptor, + # an exception is thrown, which causes get_course to return an ErrorBlock, # which causes get_from_id to raise an IOError. with self.assertRaises(IOError): CourseOverview.load_from_module_store(self.store.make_course_key('Non', 'Existent', 'Course'))