feat: add support for xblock 2

.github/workflows/unit-tests.yml

@@ -12,6 +12,7 @@ jobs:
     if: (github.repository == 'edx/edx-platform-private') || (github.repository == 'openedx/edx-platform' && (startsWith(github.base_ref, 'open-release') == false))
     runs-on: [ edx-platform-runner ]
     strategy:
+      fail-fast: false
       matrix:
         python-version:
           - "3.8"

cms/djangoapps/contentstore/helpers.py

@@ -344,7 +344,7 @@ def _import_xml_node_to_parent(
 
     if not xblock_class.has_children:
         # No children to worry about. The XML may contain child nodes, but they're not XBlocks.
-        temp_xblock = xblock_class.parse_xml(node, runtime, keys, id_generator)
+        temp_xblock = xblock_class.parse_xml(node, runtime, keys)
     else:
         # We have to handle the children ourselves, because there are lots of complex interactions between
         #    * the vanilla XBlock parse_xml() method, and its lack of API for "create and save a new XBlock"
@@ -354,7 +354,7 @@ def _import_xml_node_to_parent(
         # serialization of a child block, in order. For blocks that don't support children, their XML content/nodes
         # could be anything (e.g. HTML, capa)
         node_without_children = etree.Element(node.tag, **node.attrib)
-        temp_xblock = xblock_class.parse_xml(node_without_children, runtime, keys, id_generator)
+        temp_xblock = xblock_class.parse_xml(node_without_children, runtime, keys)
         child_nodes = list(node)
     if xblock_class.has_children and temp_xblock.children:
         raise NotImplementedError("We don't yet support pasting XBlocks with children")

lms/djangoapps/courseware/tests/test_video_mongo.py

@@ -2027,7 +2027,8 @@ def test_import_val_data_internal(self):
         xml_object = etree.fromstring(xml_data)
         id_generator = Mock()
         id_generator.target_course_id = "test_course_id"
-        video = self.block.parse_xml(xml_object, module_system, None, id_generator)
+        module_system.id_generator = id_generator
+        video = self.block.parse_xml(xml_object, module_system, None)
 
         assert video.edx_video_id == 'test_edx_video_id'
         video_data = get_video_info(video.edx_video_id)
@@ -2075,12 +2076,11 @@ def test_import_no_video_id(self):
         xml_data = """<video><video_asset></video_asset></video>"""
         xml_object = etree.fromstring(xml_data)
         module_system = DummySystem(load_error_blocks=True)
-        id_generator = Mock()
 
         # Verify edx_video_id is empty before.
         assert self.block.edx_video_id == ''
 
-        video = self.block.parse_xml(xml_object, module_system, None, id_generator)
+        video = self.block.parse_xml(xml_object, module_system, None)
 
         # Verify edx_video_id is populated after the import.
         assert video.edx_video_id != ''
@@ -2112,7 +2112,6 @@ def test_import_val_transcript(self):
         )
         xml_object = etree.fromstring(xml_data)
         module_system = DummySystem(load_error_blocks=True)
-        id_generator = Mock()
 
         # Create static directory in import file system and place transcript files inside it.
         module_system.resources_fs.makedirs(EXPORT_IMPORT_STATIC_DIR, recreate=True)
@@ -2128,7 +2127,7 @@ def test_import_val_transcript(self):
         # Verify edx_video_id is empty before.
         assert self.block.edx_video_id == ''
 
-        video = self.block.parse_xml(xml_object, module_system, None, id_generator)
+        video = self.block.parse_xml(xml_object, module_system, None)
 
         # Verify edx_video_id is populated after the import.
         assert video.edx_video_id != ''
@@ -2218,7 +2217,6 @@ def test_import_val_transcript_priority(self, sub_id, external_transcripts, val_
         language_code = 'en'
 
         module_system = DummySystem(load_error_blocks=True)
-        id_generator = Mock()
 
         # Create static directory in import file system and place transcript files inside it.
         module_system.resources_fs.makedirs(EXPORT_IMPORT_STATIC_DIR, recreate=True)
@@ -2270,7 +2268,7 @@ def test_import_val_transcript_priority(self, sub_id, external_transcripts, val_
         # Verify edx_video_id is empty before import.
         assert self.block.edx_video_id == ''
 
-        video = self.block.parse_xml(xml_object, module_system, None, id_generator)
+        video = self.block.parse_xml(xml_object, module_system, None)
 
         # Verify edx_video_id is not empty after import.
         assert video.edx_video_id != ''
@@ -2298,7 +2296,7 @@ def test_import_val_data_invalid(self):
         """
         xml_object = etree.fromstring(xml_data)
         with pytest.raises(ValCannotCreateError):
-            VideoBlock.parse_xml(xml_object, module_system, None, id_generator=Mock())
+            VideoBlock.parse_xml(xml_object, module_system, None)
         with pytest.raises(ValVideoNotFoundError):
             get_video_info("test_edx_video_id")
 

openedx/core/djangoapps/xblock/runtime/learning_core_runtime.py

@@ -205,7 +205,7 @@ def get_block(self, usage_key, for_parent=None):
             # plus some minor additional lines of code as needed.
             block = block_class.parse_xml_new_runtime(xml_node, runtime=self, keys=keys)
         else:
-            block = block_class.parse_xml(xml_node, runtime=self, keys=keys, id_generator=None)
+            block = block_class.parse_xml(xml_node, runtime=self, keys=keys)
 
         # Update field data with parsed values. We can't call .save() because it will call save_block(), below.
         block.force_save_fields(block._get_fields_to_save())  # pylint: disable=protected-access

openedx/core/djangoapps/xblock/runtime/runtime.py

@@ -217,11 +217,11 @@ def applicable_aside_types(self, block: XBlock):
         """ Disable XBlock asides in this runtime """
         return []
 
-    def parse_xml_file(self, fileobj, id_generator=None):
+    def parse_xml_file(self, fileobj):
         # Deny access to the inherited method
         raise NotImplementedError("XML Serialization is only supported with BlockstoreXBlockRuntime")
 
-    def add_node_as_child(self, block, node, id_generator=None):
+    def add_node_as_child(self, block, node):
         """
         Called by XBlock.parse_xml to treat a child node as a child block.
         """

requirements/edx/base.txt

@@ -6,7 +6,7 @@
 #
 -e git+https://github.com/anupdhabarde/edx-proctoring-proctortrack.git@31c6c9923a51c903ae83760ecbbac191363aa2a2#egg=edx_proctoring_proctortrack
     # via -r requirements/edx/github.in
-acid-xblock==0.2.1
+acid-xblock==0.3.0
     # via -r requirements/edx/kernel.in
 aiohttp==3.9.3
     # via
@@ -528,7 +528,7 @@ edx-rest-api-client==5.6.1
     #   edx-proctoring
 edx-search==3.8.2
     # via -r requirements/edx/kernel.in
-edx-sga==0.23.1
+edx-sga==0.24.1
     # via -r requirements/edx/bundled.in
 edx-submissions==3.6.0
     # via
@@ -804,7 +804,7 @@ optimizely-sdk==4.1.1
     # via
     #   -c requirements/edx/../constraints.txt
     #   -r requirements/edx/bundled.in
-ora2==6.5.1
+ora2==6.6.1
     # via -r requirements/edx/bundled.in
 packaging==23.2
     # via
@@ -1222,7 +1222,7 @@ webob==1.8.7
     #   xblock
 wrapt==1.16.0
     # via -r requirements/edx/paver.txt
-xblock[django]==1.10.0
+xblock[django]==2.0.0
     # via
     #   -r requirements/edx/kernel.in
     #   acid-xblock

requirements/edx/development.txt

@@ -12,7 +12,7 @@ accessible-pygments==0.0.4
     # via
     #   -r requirements/edx/doc.txt
     #   pydata-sphinx-theme
-acid-xblock==0.2.1
+acid-xblock==0.3.0
     # via
     #   -r requirements/edx/doc.txt
     #   -r requirements/edx/testing.txt
@@ -825,7 +825,7 @@ edx-search==3.8.2
     # via
     #   -r requirements/edx/doc.txt
     #   -r requirements/edx/testing.txt
-edx-sga==0.23.1
+edx-sga==0.24.1
     # via
     #   -r requirements/edx/doc.txt
     #   -r requirements/edx/testing.txt
@@ -1333,7 +1333,7 @@ optimizely-sdk==4.1.1
     #   -c requirements/edx/../constraints.txt
     #   -r requirements/edx/doc.txt
     #   -r requirements/edx/testing.txt
-ora2==6.5.1
+ora2==6.6.1
     # via
     #   -r requirements/edx/doc.txt
     #   -r requirements/edx/testing.txt
@@ -2185,7 +2185,7 @@ wrapt==1.16.0
     #   -r requirements/edx/doc.txt
     #   -r requirements/edx/testing.txt
     #   astroid
-xblock[django]==1.10.0
+xblock[django]==2.0.0
     # via
     #   -r requirements/edx/doc.txt
     #   -r requirements/edx/testing.txt

requirements/edx/doc.txt

@@ -8,7 +8,7 @@
     # via -r requirements/edx/base.txt
 accessible-pygments==0.0.4
     # via pydata-sphinx-theme
-acid-xblock==0.2.1
+acid-xblock==0.3.0
     # via -r requirements/edx/base.txt
 aiohttp==3.9.3
     # via
@@ -607,7 +607,7 @@ edx-rest-api-client==5.6.1
     #   edx-proctoring
 edx-search==3.8.2
     # via -r requirements/edx/base.txt
-edx-sga==0.23.1
+edx-sga==0.24.1
     # via -r requirements/edx/base.txt
 edx-submissions==3.6.0
     # via
@@ -942,7 +942,7 @@ optimizely-sdk==4.1.1
     # via
     #   -c requirements/edx/../constraints.txt
     #   -r requirements/edx/base.txt
-ora2==6.5.1
+ora2==6.6.1
     # via -r requirements/edx/base.txt
 packaging==23.2
     # via
@@ -1488,7 +1488,7 @@ webob==1.8.7
     #   xblock
 wrapt==1.16.0
     # via -r requirements/edx/base.txt
-xblock[django]==1.10.0
+xblock[django]==2.0.0
     # via
     #   -r requirements/edx/base.txt
     #   acid-xblock

requirements/edx/testing.txt

@@ -6,7 +6,7 @@
 #
 -e git+https://github.com/anupdhabarde/edx-proctoring-proctortrack.git@31c6c9923a51c903ae83760ecbbac191363aa2a2#egg=edx_proctoring_proctortrack
     # via -r requirements/edx/base.txt
-acid-xblock==0.2.1
+acid-xblock==0.3.0
     # via -r requirements/edx/base.txt
 aiohttp==3.9.3
     # via
@@ -633,7 +633,7 @@ edx-rest-api-client==5.6.1
     #   edx-proctoring
 edx-search==3.8.2
     # via -r requirements/edx/base.txt
-edx-sga==0.23.1
+edx-sga==0.24.1
     # via -r requirements/edx/base.txt
 edx-submissions==3.6.0
     # via
@@ -997,7 +997,7 @@ optimizely-sdk==4.1.1
     # via
     #   -c requirements/edx/../constraints.txt
     #   -r requirements/edx/base.txt
-ora2==6.5.1
+ora2==6.6.1
     # via -r requirements/edx/base.txt
 packaging==23.2
     # via
@@ -1604,7 +1604,7 @@ wrapt==1.16.0
     # via
     #   -r requirements/edx/base.txt
     #   astroid
-xblock[django]==1.10.0
+xblock[django]==2.0.0
     # via
     #   -r requirements/edx/base.txt
     #   acid-xblock

xmodule/course_block.py

@@ -1166,8 +1166,8 @@ def read_grading_policy(cls, paths, system):
         return policy_str
 
     @classmethod
-    def parse_xml(cls, node, runtime, keys, id_generator):
-        instance = super().parse_xml(node, runtime, keys, id_generator)
+    def parse_xml(cls, node, runtime, keys):
+        instance = super().parse_xml(node, runtime, keys)
 
         policy_dir = None
         url_name = node.get('url_name')

xmodule/discussion_block.py

@@ -233,7 +233,7 @@ def student_view_data(self):
         return {'topic_id': self.discussion_id}
 
     @classmethod
-    def parse_xml(cls, node, runtime, keys, id_generator):
+    def parse_xml(cls, node, runtime, keys):
         """
         Parses OLX into XBlock.
 
@@ -246,7 +246,7 @@ def parse_xml(cls, node, runtime, keys, id_generator):
         XBlock.parse_xml. Otherwise this method parses file in "discussion" folder (known as definition_xml), applies
         policy.json and updates fields accordingly.
         """
-        block = super().parse_xml(node, runtime, keys, id_generator)
+        block = super().parse_xml(node, runtime, keys)
 
         cls._apply_metadata_and_policy(block, node, runtime)
 

xmodule/error_block.py

@@ -178,13 +178,13 @@ def from_xml(cls, xml_data, system, id_generator,  # pylint: disable=arguments-d
         return cls._construct(system, xml_data, error_msg, location=id_generator.create_definition('error'))
 
     @classmethod
-    def parse_xml(cls, node, runtime, keys, id_generator):  # lint-amnesty, pylint: disable=unused-argument
+    def parse_xml(cls, node, runtime, keys):  # lint-amnesty, pylint: disable=unused-argument
         """
         Interpret the parsed XML in `node`, creating an XModuleDescriptor.
         """
         # It'd be great to not reserialize and deserialize the xml
         xml = etree.tostring(node).decode('utf-8')
-        block = cls.from_xml(xml, runtime, id_generator)
+        block = cls.from_xml(xml, runtime, runtime.id_generator)
         return block
 
     def export_to_xml(self, resource_fs):

xmodule/modulestore/xml.py

@@ -259,7 +259,7 @@ def construct_xblock_from_class(self, cls, scope_ids, field_data=None, *args, **
 
     # id_generator is ignored, because each ImportSystem is already local to
     # a course, and has it's own id_generator already in place
-    def add_node_as_child(self, block, node, id_generator):  # lint-amnesty, pylint: disable=signature-differs
+    def add_node_as_child(self, block, node):  # lint-amnesty, pylint: disable=signature-differs
         child_block = self.process_xml(etree.tostring(node))
         block.children.append(child_block.scope_ids.usage_id)
 

xmodule/raw_block.py

@@ -78,7 +78,7 @@ def parse_xml_new_runtime(cls, node, runtime, keys):
         try:
             block = super().parse_xml_new_runtime(node, runtime, keys)
         except AttributeError:
-            block = super().parse_xml(node, runtime, keys, id_generator=None)
+            block = super().parse_xml(node, runtime, keys)
         block.data = data_field_value
         return block
 

xmodule/template_block.py

@@ -130,7 +130,7 @@ class TranslateCustomTagBlock(  # pylint: disable=abstract-method
     resources_dir = None
 
     @classmethod
-    def parse_xml(cls, node, runtime, _keys, _id_generator):
+    def parse_xml(cls, node, runtime, _keys):
         """
         Transforms the xml_data from <$custom_tag attr="" attr=""/> to
         <customtag attr="" attr="" impl="$custom_tag"/>

xmodule/tests/test_discussion.py

@@ -69,7 +69,7 @@ def setUp(self):
         self.runtime_mock = mock.Mock(spec=Runtime)
         self.runtime_mock.construct_xblock_from_class = mock.Mock(side_effect=self._construct_xblock_mock)
         self.runtime_mock.get_policy = mock.Mock(return_value={})
-        self.id_gen_mock = mock.Mock()
+        self.id_generator = mock.Mock()
 
     def _construct_xblock_mock(self, cls, keys):  # pylint: disable=unused-argument
         """
@@ -102,7 +102,7 @@ def test_xblock_export_format(self, id_pair, category_pair, target_pair, patched
 
         patched_load_definition_xml.side_effect = Exception("Irrelevant")
 
-        block = DiscussionXBlock.parse_xml(node, self.runtime_mock, self.keys, self.id_gen_mock)
+        block = DiscussionXBlock.parse_xml(node, self.runtime_mock, self.keys)
         try:
             assert block.discussion_id == id_pair.value
             assert block.discussion_category == category_pair.value
@@ -134,7 +134,7 @@ def test_legacy_export_format(self, id_pair, category_pair, target_pair, patched
 
         patched_load_definition_xml.return_value = (definition_node, "irrelevant")
 
-        block = DiscussionXBlock.parse_xml(node, self.runtime_mock, self.keys, self.id_gen_mock)
+        block = DiscussionXBlock.parse_xml(node, self.runtime_mock, self.keys)
         try:
             assert block.discussion_id == id_pair.value
             assert block.discussion_category == category_pair.value

xmodule/tests/test_library_content.py

@@ -195,7 +195,7 @@ def test_xml_export_import_cycle(self):
 
         # Now import it.
         olx_element = etree.fromstring(exported_olx)
-        imported_lc_block = LibraryContentBlock.parse_xml(olx_element, self.runtime, None, self.id_generator)
+        imported_lc_block = LibraryContentBlock.parse_xml(olx_element, self.runtime, None)
 
         self._verify_xblock_properties(imported_lc_block)
 
@@ -219,7 +219,7 @@ def test_xml_import_with_comments(self):
 
         # Import the olx.
         olx_element = etree.fromstring(olx_with_comments)
-        imported_lc_block = LibraryContentBlock.parse_xml(olx_element, self.runtime, None, self.id_generator)
+        imported_lc_block = LibraryContentBlock.parse_xml(olx_element, self.runtime, None)
 
         self._verify_xblock_properties(imported_lc_block)
 

xmodule/tests/test_poll.py

@@ -72,7 +72,8 @@ def test_poll_export_with_unescaped_characters_xml(self):
         '''
         node = etree.fromstring(sample_poll_xml)
 
-        output = PollBlock.parse_xml(node, module_system, self.scope_ids, id_generator)
+        module_system.id_generator = id_generator
+        output = PollBlock.parse_xml(node, module_system, self.scope_ids)
         # Update the answer with invalid character.
         invalid_characters_poll_answer = output.answers[0]
         # Invalid less-than character.

xmodule/tests/test_randomize_block.py

@@ -83,8 +83,7 @@ def test_xml_export_import_cycle(self):
 
         # Now import it.
         olx_element = etree.fromstring(exported_olx)
-        id_generator = Mock()
-        imported_randomize_block = RandomizeBlock.parse_xml(olx_element, runtime, None, id_generator)
+        imported_randomize_block = RandomizeBlock.parse_xml(olx_element, runtime, None)
 
         # Check the new XBlock has the same properties as the old one.
         assert imported_randomize_block.display_name == randomize_block.display_name