refactor: remove error_descriptor_class and NonStaffErrorBlock
It's safe to remove this because non-staff [1] users cannot access [2] an `ErrorBlock`. We were able to reproduce this with and without this commit with the following results: 1. Staff users were seeing the `ErrorBlock`. 2. Non-staff users were getting an empty `<div class="vert-mod"></div>`. In theory, error blocks should be hidden in the Learning MFE because of this option [3]. However, when we manually set `hide_access_error_blocks` to `False`, we kept getting identical results (with and without this commit), so it looks that the removal `NonStaffErrorBlock` was just omitted at some point. [1]a4ec4c1b8e/lms/djangoapps/courseware/access.py (L419-L436)[2]a4ec4c1b8e/lms/djangoapps/courseware/access.py (L150-L151)[3]92ca176fde/lms/djangoapps/courseware/views/views.py (L1547-L1551)
This commit is contained in:
@@ -17,7 +17,6 @@ from xblock.exceptions import NoSuchHandlerError
|
||||
from xblock.runtime import KvsFieldData
|
||||
|
||||
from xmodule.contentstore.django import contentstore
|
||||
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
|
||||
@@ -218,7 +217,6 @@ 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=ErrorBlock,
|
||||
# Get the raw DescriptorSystem, not the CombinedSystem
|
||||
descriptor_runtime=descriptor._runtime, # pylint: disable=protected-access
|
||||
services={
|
||||
|
||||
@@ -40,7 +40,6 @@ from xblock.reference.plugins import FSService
|
||||
from xblock.runtime import KvsFieldData
|
||||
|
||||
from xmodule.contentstore.django import contentstore
|
||||
from xmodule.error_module import ErrorBlock, NonStaffErrorBlock
|
||||
from xmodule.exceptions import NotFoundError, ProcessingError
|
||||
from xmodule.modulestore.django import modulestore
|
||||
from xmodule.modulestore.exceptions import ItemNotFoundError
|
||||
@@ -728,12 +727,6 @@ def get_module_system_for_user(
|
||||
system.set('user_is_beta_tester', CourseBetaTesterRole(course_id).has_user(user))
|
||||
system.set('days_early_for_beta', descriptor.days_early_for_beta)
|
||||
|
||||
# make an ErrorBlock -- assuming that the descriptor's system is ok
|
||||
if has_access(user, 'staff', descriptor.location, course_id):
|
||||
system.error_descriptor_class = ErrorBlock
|
||||
else:
|
||||
system.error_descriptor_class = NonStaffErrorBlock
|
||||
|
||||
return system, field_data
|
||||
|
||||
|
||||
|
||||
1
setup.py
1
setup.py
@@ -24,7 +24,6 @@ XBLOCKS = [
|
||||
"library_content = xmodule.library_content_module:LibraryContentBlock",
|
||||
"library_sourced = xmodule.library_sourced_block:LibrarySourcedBlock",
|
||||
"lti = xmodule.lti_module:LTIBlock",
|
||||
"nonstaff_error = xmodule.error_module:NonStaffErrorBlock",
|
||||
"poll_question = xmodule.poll_module:PollBlock",
|
||||
"problem = xmodule.capa_module:ProblemBlock",
|
||||
"randomize = xmodule.randomize_module:RandomizeBlock",
|
||||
|
||||
@@ -227,20 +227,3 @@ class ErrorBlock(
|
||||
node.set(key, value)
|
||||
|
||||
node.extend(list(exported_node))
|
||||
|
||||
|
||||
class NonStaffErrorBlock(ErrorBlock): # pylint: disable=abstract-method
|
||||
"""
|
||||
Block that gets shown to students when there has been an error while
|
||||
loading or rendering other blocks.
|
||||
"""
|
||||
def student_view(self, _context):
|
||||
"""
|
||||
Return a fragment that contains the html for the student view.
|
||||
"""
|
||||
fragment = Fragment(self.runtime.service(self, 'mako').render_template('module-error.html', {
|
||||
'staff_access': False,
|
||||
'data': '',
|
||||
'error': '',
|
||||
}))
|
||||
return fragment
|
||||
|
||||
@@ -29,7 +29,6 @@ from xblock.fields import Reference, ReferenceList, ReferenceValueDict, ScopeIds
|
||||
from capa.xqueue_interface import XQueueService
|
||||
from xmodule.assetstore import AssetMetadata
|
||||
from xmodule.contentstore.django import contentstore
|
||||
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
|
||||
@@ -167,7 +166,6 @@ def get_test_system(
|
||||
'replace_urls': replace_url_service
|
||||
},
|
||||
course_id=course_id,
|
||||
error_descriptor_class=ErrorBlock,
|
||||
descriptor_runtime=descriptor_system,
|
||||
)
|
||||
|
||||
|
||||
@@ -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 NonStaffErrorBlock
|
||||
from xmodule.error_module import ErrorBlock
|
||||
from xmodule.modulestore.xml import CourseLocationManager, ImportSystem, XMLModuleStore
|
||||
from xmodule.tests import DATA_DIR, get_test_descriptor_system, get_test_system
|
||||
from xmodule.tests.xml import XModuleXmlImportTest
|
||||
@@ -72,7 +72,7 @@ class ConditionalFactory:
|
||||
"problem", "SampleProblem", deprecated=True)
|
||||
if source_is_error_module:
|
||||
# Make an error descriptor and module
|
||||
source_descriptor = NonStaffErrorBlock.from_xml(
|
||||
source_descriptor = ErrorBlock.from_xml(
|
||||
'some random xml data',
|
||||
system,
|
||||
id_generator=CourseLocationManager(source_location.course_key),
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
"""
|
||||
Tests for ErrorBlock and NonStaffErrorBlock
|
||||
Tests for ErrorBlock
|
||||
"""
|
||||
|
||||
|
||||
@@ -7,7 +7,7 @@ import unittest
|
||||
|
||||
from opaque_keys.edx.locator import CourseLocator
|
||||
|
||||
from xmodule.error_module import ErrorBlock, NonStaffErrorBlock
|
||||
from xmodule.error_module import ErrorBlock
|
||||
from xmodule.modulestore.xml import CourseLocationManager
|
||||
from xmodule.tests import get_test_system
|
||||
from xmodule.x_module import STUDENT_VIEW
|
||||
@@ -42,28 +42,3 @@ class TestErrorBlock(SetupTestErrorBlock):
|
||||
context_repr = self.system.render(descriptor, STUDENT_VIEW).content
|
||||
assert self.error_msg in context_repr
|
||||
assert repr(self.valid_xml) in context_repr
|
||||
|
||||
|
||||
class TestNonStaffErrorBlock(SetupTestErrorBlock):
|
||||
"""
|
||||
Tests for NonStaffErrorBlock.
|
||||
"""
|
||||
|
||||
def test_non_staff_error_block_create(self):
|
||||
descriptor = NonStaffErrorBlock.from_xml(
|
||||
self.valid_xml,
|
||||
self.system,
|
||||
CourseLocationManager(self.course_id)
|
||||
)
|
||||
assert isinstance(descriptor, NonStaffErrorBlock)
|
||||
|
||||
def test_from_xml_render(self):
|
||||
descriptor = NonStaffErrorBlock.from_xml(
|
||||
self.valid_xml,
|
||||
self.system,
|
||||
CourseLocationManager(self.course_id)
|
||||
)
|
||||
descriptor.xmodule_runtime = self.system
|
||||
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
|
||||
|
||||
@@ -1667,7 +1667,6 @@ class ModuleSystem(MetricsMixin, ConfigurableFragmentWrapper, ModuleSystemShim,
|
||||
descriptor_runtime,
|
||||
publish=None,
|
||||
course_id=None,
|
||||
error_descriptor_class=None,
|
||||
**kwargs,
|
||||
):
|
||||
"""
|
||||
@@ -1689,8 +1688,6 @@ class ModuleSystem(MetricsMixin, ConfigurableFragmentWrapper, ModuleSystemShim,
|
||||
course_id - the course_id containing this module
|
||||
|
||||
publish(event) - A function that allows XModules to publish events (such as grade changes)
|
||||
|
||||
error_descriptor_class - The class to use to render XModules with errors
|
||||
"""
|
||||
|
||||
kwargs.setdefault('id_reader', getattr(descriptor_runtime, 'id_reader', OpaqueKeyReader()))
|
||||
@@ -1704,7 +1701,6 @@ class ModuleSystem(MetricsMixin, ConfigurableFragmentWrapper, ModuleSystemShim,
|
||||
|
||||
if publish:
|
||||
self.publish = publish
|
||||
self.error_descriptor_class = error_descriptor_class
|
||||
self.xmodule_instance = None
|
||||
|
||||
self.descriptor_runtime = descriptor_runtime
|
||||
|
||||
Reference in New Issue
Block a user