feat: remove depreacted fragment & runtime methods (#680)

.github/workflows/ci.yml

@@ -12,6 +12,7 @@ jobs:
     name: tests
     runs-on: ubuntu-20.04
     strategy:
+      fail-fast: false
       matrix:
         python-version: ['3.8']
         toxenv: [quality, django32, django42]

CHANGELOG.rst

@@ -7,6 +7,17 @@ These are notable changes in XBlock.
 Unreleased
 ----------
 
+2.0.0 - 2024-02-26
+------------------
+
+* Removed deprecations
+* xblock.fragment (removed completely)
+* xblock.runtime.Runtime._aside_from_xml (just the id_generator argument)
+* xblock.runtime.Runtime._usage_id_from_node (just the id_generator argument)
+* xblock.runtime.Runtime.add_node_as_child (just the id_generator argument)
+* xblock.runtime.Runtime.parse_xml_string (just the id_generator argument)
+* xblock.runtime.Runtime.parse_xml_file (just the id_generator argument)
+
 1.10.0 - 2024-01-12
 -------------------
 

docs/index.rst

@@ -13,13 +13,12 @@ in depth and guides developers through the process of creating an XBlock.
 
 .. toctree::
     :titlesonly:
-    
+
     changelog
     introduction
     xblock
     fields
     runtime
-    fragment
     plugins
     exceptions
     xblock-tutorial/index

docs/xblock-utils/settings-and-theme-support.rst

@@ -145,8 +145,8 @@ specified via settings.
 -  ``include_theme_files(fragment)`` - this method is an entry point to
    ``ThemableXBlockMixin`` functionality. It calls ``get_theme`` to
    obtain theme configuration, fetches theme files and includes them
-   into fragment. ``fragment`` must be an
-   `XBlock.Fragment <https://github.com/edx/XBlock/blob/master/xblock/fragment.py>`__
+   into fragment. ``fragment`` must be a
+   `web_fragments.fragment <https://github.com/openedx/web-fragments/blob/master/web_fragments/fragment.py>`__
    instance.
 
 So, having met usage requirements and set up theme configuration

setup.py

@@ -69,7 +69,7 @@ def get_version(*file_paths):
         'Development Status :: 5 - Production/Stable',
         'Framework :: Django',
         'Framework :: Django :: 3.2',
-        'Framework :: Django :: 4.0',
+        'Framework :: Django :: 4.2',
         'Intended Audience :: Developers',
         'License :: OSI Approved :: Apache Software License',
         'Natural Language :: English',

tox.ini

@@ -8,34 +8,33 @@ filterwarnings = always
 norecursedirs = .* docs requirements
 
 [testenv]
-deps = 
+deps =
     django32: Django>=3.2,<4.0
-    django42: Django>=4.2,<4.3
+    django42: Django>=4.2,<5.0
     -r requirements/test.txt
 changedir = {envsitepackagesdir}
-commands = 
+commands =
     python -Wd -m pytest {posargs:xblock}
     python -m coverage xml
     mv coverage.xml {toxinidir}
-allowlist_externals = 
+allowlist_externals =
     make
     mv
 
 [testenv:docs]
-basepython = 
+basepython =
     python3.8
-changedir = 
+changedir =
     {toxinidir}/docs
-deps = 
+deps =
     -r requirements/doc.txt
-commands = 
+commands =
     make html
 
 [testenv:quality]
-deps = 
+deps =
     django32: Django>=3.2,<4.0
     -r requirements/test.txt
 changedir = {toxinidir}
-commands = 
+commands =
     make quality
-

xblock/__init__.py

@@ -27,4 +27,4 @@ def __init__(self, *args, **kwargs):
 # without causing a circular import
 xblock.fields.XBlockMixin = XBlockMixin
 
-__version__ = '1.10.0'
+__version__ = '2.0.0'

xblock/mixins.py

@@ -425,7 +425,7 @@ class XmlSerializationMixin(ScopedStorageMixin):
     """
 
     @classmethod
-    def parse_xml(cls, node, runtime, keys, id_generator):
+    def parse_xml(cls, node, runtime, keys):
         """
         Use `node` to construct a new block.
 
@@ -437,10 +437,6 @@ def parse_xml(cls, node, runtime, keys, id_generator):
             keys (:class:`.ScopeIds`): The keys identifying where this block
                 will store its data.
 
-            id_generator (:class:`.IdGenerator`): An object that will allow the
-                runtime to generate correct definition and usage ids for
-                children of this block.
-
         """
         block = runtime.construct_xblock_from_class(cls, keys)
 
@@ -456,7 +452,7 @@ def parse_xml(cls, node, runtime, keys, id_generator):
             if namespace == XML_NAMESPACES["option"]:
                 cls._set_field_if_present(block, tag, child.text, child.attrib)
             else:
-                block.runtime.add_node_as_child(block, child, id_generator)
+                block.runtime.add_node_as_child(block, child)
 
         # Attributes become fields.
         for name, value in list(node.items()):  # lxml has no iteritems

xblock/runtime.py

@@ -517,8 +517,8 @@ def publish(self, block, event_type, event_data):
 
     # Construction
     def __init__(
-            self, id_reader, field_data=None, mixins=(), services=None,
-            default_class=None, select=None, id_generator=None
+            self, id_reader, id_generator, field_data=None, mixins=(),
+            services=None, default_class=None, select=None
     ):
         """
         Arguments:
@@ -569,8 +569,6 @@ def __init__(
         self._view_name = None
 
         self.id_generator = id_generator
-        if id_generator is None:
-            warnings.warn("IdGenerator will be required in the future in order to support XBlockAsides", FutureWarning)
 
     # Block operations
 
@@ -704,54 +702,33 @@ def save_block(self, block):
 
     # Parsing XML
 
-    def parse_xml_string(self, xml, id_generator=None):
+    def parse_xml_string(self, xml):
         """Parse a string of XML, returning a usage id."""
-        if id_generator is not None:
-            warnings.warn(
-                "Passing an id_generator directly is deprecated "
-                "in favor of constructing the Runtime with the id_generator",
-                DeprecationWarning,
-                stacklevel=2,
-            )
-
-        id_generator = id_generator or self.id_generator
         if isinstance(xml, bytes):
             io_type = BytesIO
         else:
             io_type = StringIO
-        return self.parse_xml_file(io_type(xml), id_generator)
+        return self.parse_xml_file(io_type(xml))
 
-    def parse_xml_file(self, fileobj, id_generator=None):
+    def parse_xml_file(self, fileobj):
         """Parse an open XML file, returning a usage id."""
         root = etree.parse(fileobj).getroot()
-        usage_id = self._usage_id_from_node(root, None, id_generator)
+        usage_id = self._usage_id_from_node(root, None)
         return usage_id
 
-    def _usage_id_from_node(self, node, parent_id, id_generator=None):
+    def _usage_id_from_node(self, node, parent_id):
         """Create a new usage id from an XML dom node.
 
         Args:
             node (lxml.etree.Element): The DOM node to interpret.
             parent_id: The usage ID of the parent block
-            id_generator (IdGenerator): The :class:`.IdGenerator` to use
-                for creating ids
         """
-        if id_generator is not None:
-            warnings.warn(
-                "Passing an id_generator directly is deprecated "
-                "in favor of constructing the Runtime with the id_generator",
-                DeprecationWarning,
-                stacklevel=3,
-            )
-
-        id_generator = id_generator or self.id_generator
-
         block_type = node.tag
         # remove xblock-family from elements
         node.attrib.pop('xblock-family', None)
         # TODO: a way for this node to be a usage to an existing definition?
-        def_id = id_generator.create_definition(block_type)
-        usage_id = id_generator.create_usage(def_id)
+        def_id = self.id_generator.create_definition(block_type)
+        usage_id = self.id_generator.create_usage(def_id)
         keys = ScopeIds(None, block_type, def_id, usage_id)
         block_class = self.mixologist.mix(self.load_block_type(block_type))
         # pull the asides out of the xml payload
@@ -765,31 +742,30 @@ def _usage_id_from_node(self, node, parent_id, id_generator=None):
                     aside_children.append(child)
         # now process them & remove them from the xml payload
         for child in aside_children:
-            self._aside_from_xml(child, def_id, usage_id, id_generator)
+            self._aside_from_xml(child, def_id, usage_id)
             node.remove(child)
-        block = block_class.parse_xml(node, self, keys, id_generator)
+        block = block_class.parse_xml(node, self, keys)
         block.parent = parent_id
         block.save()
         return usage_id
 
-    def _aside_from_xml(self, node, block_def_id, block_usage_id, id_generator):
+    def _aside_from_xml(self, node, block_def_id, block_usage_id):
         """
         Create an aside from the xml and attach it to the given block
         """
-        id_generator = id_generator or self.id_generator
 
         aside_type = node.tag
         aside_class = self.load_aside_type(aside_type)
-        aside_def_id, aside_usage_id = id_generator.create_aside(block_def_id, block_usage_id, aside_type)
+        aside_def_id, aside_usage_id = self.id_generator.create_aside(block_def_id, block_usage_id, aside_type)
         keys = ScopeIds(None, aside_type, aside_def_id, aside_usage_id)
-        aside = aside_class.parse_xml(node, self, keys, id_generator)
+        aside = aside_class.parse_xml(node, self, keys)
         aside.save()
 
-    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.
         """
-        usage_id = self._usage_id_from_node(node, block.scope_ids.usage_id, id_generator)
+        usage_id = self._usage_id_from_node(node, block.scope_ids.usage_id)
         block.children.append(usage_id)
 
     # Exporting XML

xblock/test/test_parsing.py

@@ -56,7 +56,7 @@ class Specialized(XBlock):
     num_children = Integer(default=0, scope=Scope.user_state)
 
     @classmethod
-    def parse_xml(cls, node, runtime, keys, id_generator):
+    def parse_xml(cls, node, runtime, keys):
         """We'll just set num_children to the number of child nodes."""
         block = runtime.construct_xblock_from_class(cls, keys)
         block.num_children = len(node)
@@ -69,12 +69,12 @@ class CustomXml(XBlock):
     has_children = True
 
     @classmethod
-    def parse_xml(cls, node, runtime, keys, id_generator):
+    def parse_xml(cls, node, runtime, keys):
         """Parse the XML node and save it as a string"""
         block = runtime.construct_xblock_from_class(cls, keys)
         for child in node:
             if child.tag is not etree.Comment:
-                block.runtime.add_node_as_child(block, child, id_generator)
+                block.runtime.add_node_as_child(block, child)
         # Now build self.inner_xml from the XML of node's children
         # We can't just call tounicode() on each child because it adds xmlns: attributes
         xml_str = etree.tounicode(node)

xblock/test/test_runtime.py

@@ -662,13 +662,13 @@ class TestRuntimeDeprecation(WarningTestMixin, TestCase):
     def test_passed_field_data(self):
         field_data = Mock(spec=FieldData)
         with self.assertWarns(FieldDataDeprecationWarning):
-            runtime = TestRuntime(Mock(spec=IdReader), field_data)
+            runtime = TestRuntime(Mock(spec=IdReader), field_data=field_data)
         with self.assertWarns(FieldDataDeprecationWarning):
             self.assertEqual(runtime.field_data, field_data)
 
     def test_set_field_data(self):
         field_data = Mock(spec=FieldData)
-        runtime = TestRuntime(Mock(spec=IdReader), None)
+        runtime = TestRuntime(Mock(spec=IdReader), field_data=None)
         with self.assertWarns(FieldDataDeprecationWarning):
             runtime.field_data = field_data
         with self.assertWarns(FieldDataDeprecationWarning):

xblock/test/toy_runtime.py

@@ -97,8 +97,7 @@ class ToyRuntime(Runtime):
     # pylint: disable=abstract-method
 
     def __init__(self, user_id=None):
-        super().__init__(ID_MANAGER, services={'field-data': KvsFieldData(TOYRUNTIME_KVS)})
-        self.id_generator = ID_MANAGER
+        super().__init__(ID_MANAGER, ID_MANAGER, services={'field-data': KvsFieldData(TOYRUNTIME_KVS)})
         self.user_id = user_id
 
     def render_template(self, template_name, **kwargs):