From 8e4a9d2ba1fb8da768a0bd9ad0efaedab6894325 Mon Sep 17 00:00:00 2001 From: Usman Khalid <2200617@gmail.com> Date: Mon, 17 Aug 2020 00:15:45 +0500 Subject: [PATCH] Convert RandomizeModule to RandomizeBlock. --- common/lib/xmodule/setup.py | 2 +- .../lib/xmodule/xmodule/randomize_module.py | 86 ++++---- common/lib/xmodule/xmodule/seq_module.py | 76 +++---- common/lib/xmodule/xmodule/tests/__init__.py | 8 +- .../xmodule/tests/test_randomize_module.py | 191 +++++++++--------- .../xmodule/tests/test_xblock_wrappers.py | 4 +- 6 files changed, 197 insertions(+), 170 deletions(-) diff --git a/common/lib/xmodule/setup.py b/common/lib/xmodule/setup.py index 2a54bac6f9..530e219c57 100644 --- a/common/lib/xmodule/setup.py +++ b/common/lib/xmodule/setup.py @@ -14,7 +14,6 @@ XMODULES = [ "nonstaff_error = xmodule.error_module:NonStaffErrorDescriptor", "poll_question = xmodule.poll_module:PollDescriptor", "problemset = xmodule.seq_module:SequenceDescriptor", - "randomize = xmodule.randomize_module:RandomizeDescriptor", "split_test = xmodule.split_test_module:SplitTestDescriptor", "section = xmodule.backcompat_module:SemanticSectionDescriptor", "sequential = xmodule.seq_module:SequenceDescriptor", @@ -35,6 +34,7 @@ XBLOCKS = [ "library_content = xmodule.library_content_module:LibraryContentBlock", "library_sourced = xmodule.library_sourced_block:LibrarySourcedBlock", "problem = xmodule.capa_module:ProblemBlock", + "randomize = xmodule.randomize_module:RandomizeBlock", "static_tab = xmodule.html_module:StaticTabBlock", "unit = xmodule.unit_block:UnitBlock", "vertical = xmodule.vertical_block:VerticalBlock", diff --git a/common/lib/xmodule/xmodule/randomize_module.py b/common/lib/xmodule/xmodule/randomize_module.py index f3a9b9f35d..a97996508f 100644 --- a/common/lib/xmodule/xmodule/randomize_module.py +++ b/common/lib/xmodule/xmodule/randomize_module.py @@ -3,22 +3,37 @@ import logging import random +from django.utils.functional import cached_property from lxml import etree from web_fragments.fragment import Fragment from xblock.fields import Integer, Scope -from xmodule.seq_module import SequenceDescriptor -from xmodule.x_module import STUDENT_VIEW, XModule +from xmodule.mako_module import MakoTemplateBlockBase +from xmodule.seq_module import SequenceMixin +from xmodule.xml_module import XmlMixin +from xmodule.x_module import ( + HTMLSnippet, + ResourceTemplates, + STUDENT_VIEW, + XModuleDescriptorToXBlockMixin, + XModuleMixin, + XModuleToXBlockMixin, +) log = logging.getLogger('edx.' + __name__) -class RandomizeFields(object): - choice = Integer(help="Which random child was chosen", scope=Scope.user_state) - - -class RandomizeModule(RandomizeFields, XModule): +class RandomizeBlock( + SequenceMixin, + MakoTemplateBlockBase, + XmlMixin, + XModuleDescriptorToXBlockMixin, + XModuleToXBlockMixin, + HTMLSnippet, + ResourceTemplates, + XModuleMixin, +): """ - Chooses a random child module. Chooses the same one every time for each student. + Chooses a random child xblock. Chooses the same one every time for each student. Example: @@ -38,11 +53,18 @@ class RandomizeModule(RandomizeFields, XModule): grading interaction is a tangle between super and subclasses of descriptors and modules. """ - def __init__(self, *args, **kwargs): - super(RandomizeModule, self).__init__(*args, **kwargs) + choice = Integer(help="Which random child was chosen", scope=Scope.user_state) - # NOTE: calling self.get_children() doesn't work until we've picked a choice - num_choices = len(self.descriptor.get_children()) + resources_dir = None + + filename_extension = "xml" + + show_in_read_only_mode = True + + @cached_property + def child(self): + """ Return XBlock instance of selected choice """ + num_choices = len(self.get_children()) if self.choice is not None and self.choice > num_choices: # Oops. Children changed. Reset. @@ -56,54 +78,40 @@ class RandomizeModule(RandomizeFields, XModule): else: self.choice = random.randrange(0, num_choices) - if self.choice is not None: - # Now get_children() should return a list with one element - log.debug("children of randomize module (should be only 1): %s", self.child) - - @property - def child_descriptor(self): - """ Return descriptor of selected choice """ if self.choice is None: return None - return self.descriptor.get_children()[self.choice] + child = self.get_children()[self.choice] - @property - def child(self): - """ Return module instance of selected choice """ - child_descriptor = self.child_descriptor - if child_descriptor is None: - return None - return self.system.get_module(child_descriptor) + if self.choice is not None: + log.debug("children of randomize module (should be only 1): %s", child) + + return child def get_child_descriptors(self): """ For grading--return just the chosen child. """ - if self.child_descriptor is None: + if self.child is None: return [] - return [self.child_descriptor] + return [self.child] def student_view(self, context): + """ + The student view. + """ if self.child is None: # raise error instead? In fact, could complain on descriptor load... return Fragment(content=u"
Nothing to randomize between
") return self.child.render(STUDENT_VIEW, context) + def get_html(self): + return self.studio_view(None).content + def get_icon_class(self): return self.child.get_icon_class() if self.child else 'other' - -class RandomizeDescriptor(RandomizeFields, SequenceDescriptor): - # the editing interface can be the same as for sequences -- just a container - module_class = RandomizeModule - resources_dir = None - - filename_extension = "xml" - - show_in_read_only_mode = True - def definition_to_xml(self, resource_fs): xml_object = etree.Element('randomize') diff --git a/common/lib/xmodule/xmodule/seq_module.py b/common/lib/xmodule/xmodule/seq_module.py index 1210294b97..44bc588880 100644 --- a/common/lib/xmodule/xmodule/seq_module.py +++ b/common/lib/xmodule/xmodule/seq_module.py @@ -806,7 +806,46 @@ class SequenceModule(SequenceFields, ProctoringFields, XModule): return new_class -class SequenceDescriptor(SequenceFields, ProctoringFields, MakoModuleDescriptor, XmlDescriptor): +class SequenceMixin(SequenceFields): + """ + A mixin of shared code between the SequenceDescriptor and XBlocks + converted from XModules which inherited from SequenceDescriptor. + """ + @classmethod + def definition_from_xml(cls, xml_object, system): + children = [] + for child in xml_object: + try: + child_block = system.process_xml(etree.tostring(child, encoding='unicode')) + children.append(child_block.scope_ids.usage_id) + except Exception as e: + log.exception("Unable to load child when parsing Sequence. Continuing...") + if system.error_tracker is not None: + system.error_tracker(u"ERROR: {0}".format(e)) + continue + return {}, children + + def index_dictionary(self): + """ + Return dictionary prepared with module content and type for indexing. + """ + # return key/value fields in a Python dict object + # values may be numeric / string or dict + # default implementation is an empty dict + xblock_body = super(SequenceMixin, self).index_dictionary() + html_body = { + "display_name": self.display_name, + } + if "content" in xblock_body: + xblock_body["content"].update(html_body) + else: + xblock_body["content"] = html_body + xblock_body["content_type"] = "Sequence" + + return xblock_body + + +class SequenceDescriptor(SequenceMixin, ProctoringFields, MakoModuleDescriptor, XmlDescriptor): """ A Sequence's Descriptor object """ @@ -822,20 +861,6 @@ class SequenceDescriptor(SequenceFields, ProctoringFields, MakoModuleDescriptor, } js_module_name = "SequenceDescriptor" - @classmethod - def definition_from_xml(cls, xml_object, system): - children = [] - for child in xml_object: - try: - child_block = system.process_xml(etree.tostring(child, encoding='unicode')) - children.append(child_block.scope_ids.usage_id) - except Exception as e: - log.exception("Unable to load child when parsing Sequence. Continuing...") - if system.error_tracker is not None: - system.error_tracker(u"ERROR: {0}".format(e)) - continue - return {}, children - def definition_to_xml(self, resource_fs): xml_object = etree.Element('sequential') for child in self.get_children(): @@ -848,28 +873,9 @@ class SequenceDescriptor(SequenceFields, ProctoringFields, MakoModuleDescriptor, `is_entrance_exam` should not be editable in the Studio settings editor. """ non_editable_fields = super(SequenceDescriptor, self).non_editable_metadata_fields - non_editable_fields.append(self.fields['is_entrance_exam']) + non_editable_fields.append(self.fields['is_entrance_exam']) # pylint:disable=unsubscriptable-object return non_editable_fields - def index_dictionary(self): - """ - Return dictionary prepared with module content and type for indexing. - """ - # return key/value fields in a Python dict object - # values may be numeric / string or dict - # default implementation is an empty dict - xblock_body = super(SequenceDescriptor, self).index_dictionary() - html_body = { - "display_name": self.display_name, - } - if "content" in xblock_body: - xblock_body["content"].update(html_body) - else: - xblock_body["content"] = html_body - xblock_body["content_type"] = "Sequence" - - return xblock_body - class HighlightsFields(object): """Only Sections have summaries now, but we may expand that later.""" diff --git a/common/lib/xmodule/xmodule/tests/__init__.py b/common/lib/xmodule/xmodule/tests/__init__.py index 8c2d9fe610..9c954d2051 100644 --- a/common/lib/xmodule/xmodule/tests/__init__.py +++ b/common/lib/xmodule/xmodule/tests/__init__.py @@ -88,7 +88,10 @@ class TestModuleSystem(ModuleSystem): # pylint: disable=abstract-method return rt_repr -def get_test_system(course_id=CourseKey.from_string('/'.join(['org', 'course', 'run']))): +def get_test_system( + course_id=CourseKey.from_string('/'.join(['org', 'course', 'run'])), + user=None, +): """ Construct a test ModuleSystem instance. @@ -101,7 +104,8 @@ def get_test_system(course_id=CourseKey.from_string('/'.join(['org', 'course', ' where `my_render_func` is a function of the form my_render_func(template, context). """ - user = Mock(name='get_test_system.user', is_staff=False) + if not user: + user = Mock(name='get_test_system.user', is_staff=False) descriptor_system = get_test_descriptor_system() diff --git a/common/lib/xmodule/xmodule/tests/test_randomize_module.py b/common/lib/xmodule/xmodule/tests/test_randomize_module.py index 841a70189c..99d9cb7eb3 100644 --- a/common/lib/xmodule/xmodule/tests/test_randomize_module.py +++ b/common/lib/xmodule/xmodule/tests/test_randomize_module.py @@ -2,108 +2,117 @@ Test cases covering workflows and behaviors for the Randomize XModule """ +from fs.memoryfs import MemoryFS +from lxml import etree +from mock import Mock -import unittest -from datetime import datetime, timedelta +from xmodule.modulestore.tests.factories import CourseFactory +from xmodule.modulestore.tests.utils import MixedSplitTestCase +from xmodule.randomize_module import RandomizeBlock +from xmodule.tests import get_test_system -from opaque_keys.edx.locator import BlockUsageLocator -from pytz import UTC -from xblock.fields import ScopeIds - -from xmodule.randomize_module import RandomizeModule - -from .test_course_module import DummySystem as DummyImportSystem - -ORG = 'test_org' -COURSE = 'test_course' - -START = '2013-01-01T01:00:00' -_TODAY = datetime.now(UTC) -_LAST_WEEK = _TODAY - timedelta(days=7) -_NEXT_WEEK = _TODAY + timedelta(days=7) +from .test_course_module import DummySystem as TestImportSystem -class RandomizeModuleTestCase(unittest.TestCase): - """Make sure the randomize module works""" +class RandomizeBlockTest(MixedSplitTestCase): + """ + Base class for tests of LibraryContentModule (library_content_module.py) + """ + maxDiff = None def setUp(self): + super().setUp() + + self.course = CourseFactory.create(modulestore=self.store) + self.chapter = self.make_block("chapter", self.course) + self.sequential = self.make_block("sequential", self.chapter) + self.vertical = self.make_block("vertical", self.sequential) + self.randomize_block = self.make_block( + "randomize", + self.vertical, + display_name="Hello Randomize", + ) + self.child_blocks = [ + self.make_block("html", self.randomize_block, display_name="Hello HTML {}".format(i)) + for i in range(1, 4) + ] + + def _bind_module_system(self, block, user_id): """ - Initialize dummy testing course. + Bind module system to block so we can access student-specific data. """ - super(RandomizeModuleTestCase, self).setUp() - self.system = DummyImportSystem(load_error_modules=True) - self.system.seed = None - self.course = self.get_dummy_course() - self.modulestore = self.system.modulestore + user = Mock(name='get_test_system.user', id=user_id, is_staff=False) + module_system = get_test_system(course_id=block.location.course_key, user=user) + module_system.descriptor_runtime = block.runtime._descriptor_system # pylint: disable=protected-access + block.xmodule_runtime = module_system - def get_dummy_course(self, start=_TODAY): - """Get a dummy course""" - - self.start_xml = ''' - - - - Two houses, ... - Three houses, ... - - - - - - '''.format(org=ORG, course=COURSE, start=start) - - return self.system.process_xml(self.start_xml) - - def test_import(self): + def test_xml_export_import_cycle(self): """ - Just make sure descriptor loads without error + Test the export-import cycle. """ - self.get_dummy_course(START) + randomize_block = self.store.get_item(self.randomize_block.location) - def test_course_has_started(self): - """ - Test CourseDescriptor.has_started. - """ - self.course.start = _LAST_WEEK - self.assertTrue(self.course.has_started()) - self.course.start = _NEXT_WEEK - self.assertFalse(self.course.has_started()) - - def test_children(self): - """ Check course/randomize module works fine """ - - self.assertTrue(self.course.has_children) - self.assertEqual(len(self.course.get_children()), 2) - - def inner_get_module(descriptor): - """ - Override systems.get_module - This method will be called when any call is made to self.system.get_module - """ - if isinstance(descriptor, BlockUsageLocator): - location = descriptor - descriptor = self.modulestore.get_item(location, depth=None) - descriptor.xmodule_runtime = self.get_dummy_course() - descriptor.xmodule_runtime.descriptor_runtime = descriptor._runtime # pylint: disable=protected-access - descriptor.xmodule_runtime.get_module = inner_get_module - return descriptor - - self.system.get_module = inner_get_module - - # Get randomize_descriptor from the course & verify its children - randomize_descriptor = inner_get_module(self.course.id.make_usage_key('randomize', 'my_randomize')) - self.assertTrue(randomize_descriptor.has_children) - self.assertEqual(len(randomize_descriptor.get_children()), 2) - - # Call RandomizeModule which will select an element from the list of available items - randomize_module = RandomizeModule( - randomize_descriptor, - self.system, - scope_ids=ScopeIds(None, None, self.course.id, self.course.id) + expected_olx = ( + '\n' + ' \n' + ' \n' + ' \n' + '\n' + ).format( + block=randomize_block, ) - # Verify the selected child - self.assertEqual(len(randomize_module.get_child_descriptors()), 1, "No child is chosen") - self.assertIn(randomize_module.child.display_name, ['A', 'B'], "Unwanted child selected") + export_fs = MemoryFS() + # Set the virtual FS to export the olx to. + randomize_block.runtime._descriptor_system.export_fs = export_fs # pylint: disable=protected-access + + # Export the olx. + node = etree.Element("unknown_root") + randomize_block.add_xml_to_node(node) + + # Read it back + with export_fs.open('{dir}/{file_name}.xml'.format( + dir=randomize_block.scope_ids.usage_id.block_type, + file_name=randomize_block.scope_ids.usage_id.block_id + )) as f: + exported_olx = f.read() + + # And compare. + self.assertEqual(exported_olx, expected_olx) + + runtime = TestImportSystem(load_error_modules=True, course_id=randomize_block.location.course_key) + runtime.resources_fs = export_fs + + # Now import it. + olx_element = etree.fromstring(exported_olx) + id_generator = Mock() + imported_randomize_block = RandomizeBlock.parse_xml(olx_element, runtime, None, id_generator) + + # Check the new XBlock has the same properties as the old one. + self.assertEqual(imported_randomize_block.display_name, randomize_block.display_name) + self.assertEqual(len(imported_randomize_block.children), 3) + self.assertEqual(imported_randomize_block.children, randomize_block.children) + + def test_children_seen_by_a_user(self): + """ + Test that each student sees only one block as a child of the LibraryContent block. + """ + randomize_block = self.store.get_item(self.randomize_block.location) + self._bind_module_system(randomize_block, 3) + + # Make sure the runtime knows that the block's children vary per-user: + self.assertTrue(randomize_block.has_dynamic_children()) + + self.assertEqual(len(randomize_block.children), 3) + + # Check how many children each user will see: + self.assertEqual(len(randomize_block.get_child_descriptors()), 1) + self.assertEqual(randomize_block.get_child_descriptors()[0].display_name, 'Hello HTML 1') + # Check that get_content_titles() doesn't return titles for hidden/unused children + # get_content_titles() is not overridden in RandomizeBlock so titles of the 3 children are returned. + self.assertEqual(len(randomize_block.get_content_titles()), 3) + + # Bind to another user and check a different child block is displayed to user. + randomize_block = self.store.get_item(self.randomize_block.location) + self._bind_module_system(randomize_block, 1) + self.assertEqual(randomize_block.get_child_descriptors()[0].display_name, 'Hello HTML 2') diff --git a/common/lib/xmodule/xmodule/tests/test_xblock_wrappers.py b/common/lib/xmodule/xmodule/tests/test_xblock_wrappers.py index 12cdaecb75..77a3559f17 100644 --- a/common/lib/xmodule/xmodule/tests/test_xblock_wrappers.py +++ b/common/lib/xmodule/xmodule/tests/test_xblock_wrappers.py @@ -35,7 +35,7 @@ from xmodule.conditional_module import ConditionalDescriptor from xmodule.course_module import CourseDescriptor from xmodule.html_module import HtmlBlock from xmodule.poll_module import PollDescriptor -from xmodule.randomize_module import RandomizeDescriptor +from xmodule.randomize_module import RandomizeBlock from xmodule.seq_module import SequenceDescriptor from xmodule.tests import get_test_descriptor_system, get_test_system from xmodule.vertical_block import VerticalBlock @@ -68,7 +68,7 @@ LEAF_XMODULES = { CONTAINER_XMODULES = { ConditionalDescriptor: [{}], CourseDescriptor: [{}], - RandomizeDescriptor: [{'display_name': 'Test String Display'}], + RandomizeBlock: [{'display_name': 'Test String Display'}], SequenceDescriptor: [{'display_name': u'Test Unicode हिंदी Display'}], VerticalBlock: [{}], WrapperBlock: [{}],