From 0415e67e754b8c1ac67b037e89db300140ce9f93 Mon Sep 17 00:00:00 2001 From: Irtaza Akram Date: Mon, 26 Feb 2024 15:30:36 +0500 Subject: [PATCH] feat: remove depreacted fragment & runtime methods --- .github/workflows/ci.yml | 1 + CHANGELOG.rst | 11 ++++ docs/fragment.rst | 8 --- docs/index.rst | 3 +- .../settings-and-theme-support.rst | 4 +- setup.py | 2 +- tox.ini | 21 ++++--- xblock/__init__.py | 2 +- xblock/fragment.py | 26 --------- xblock/mixins.py | 8 +-- xblock/runtime.py | 56 ++++++------------- xblock/test/test_fragment.py | 22 -------- xblock/test/test_parsing.py | 6 +- xblock/test/test_runtime.py | 4 +- xblock/test/toy_runtime.py | 3 +- 15 files changed, 51 insertions(+), 126 deletions(-) delete mode 100644 docs/fragment.rst delete mode 100644 xblock/fragment.py delete mode 100644 xblock/test/test_fragment.py diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 0efc7b6b8..1f5eb2ef8 100644 --- a/.github/workflows/ci.yml +++ b/.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] diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 05f21a2c4..162b95886 100644 --- a/CHANGELOG.rst +++ b/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 ------------------- diff --git a/docs/fragment.rst b/docs/fragment.rst deleted file mode 100644 index cb695cfcd..000000000 --- a/docs/fragment.rst +++ /dev/null @@ -1,8 +0,0 @@ -.. _Fragment API: - -############ -Fragment API -############ - -.. automodule:: xblock.fragment - :members: diff --git a/docs/index.rst b/docs/index.rst index d1baa37b5..629ce2428 100644 --- a/docs/index.rst +++ b/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 diff --git a/docs/xblock-utils/settings-and-theme-support.rst b/docs/xblock-utils/settings-and-theme-support.rst index 1b4b9fa83..b35422772 100644 --- a/docs/xblock-utils/settings-and-theme-support.rst +++ b/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 `__ + into fragment. ``fragment`` must be a + `web_fragments.fragment `__ instance. So, having met usage requirements and set up theme configuration diff --git a/setup.py b/setup.py index ec7ca0f77..c8a230682 100755 --- a/setup.py +++ b/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', diff --git a/tox.ini b/tox.ini index 16ea4b3d3..34cb20dfc 100644 --- a/tox.ini +++ b/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 - diff --git a/xblock/__init__.py b/xblock/__init__.py index d10628e18..82fc398f6 100644 --- a/xblock/__init__.py +++ b/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' diff --git a/xblock/fragment.py b/xblock/fragment.py deleted file mode 100644 index 36949ea5b..000000000 --- a/xblock/fragment.py +++ /dev/null @@ -1,26 +0,0 @@ -""" -Makes the Fragment class available through the old namespace location. -""" -import warnings - -import web_fragments.fragment - - -class Fragment(web_fragments.fragment.Fragment): - """ - A wrapper around web_fragments.fragment.Fragment that provides - backwards compatibility for the old location. - - Deprecated. - """ - def __init__(self, *args, **kwargs): - warnings.warn( - 'xblock.fragment is deprecated. Please use web_fragments.fragment instead', - DeprecationWarning, - stacklevel=2 - ) - super().__init__(*args, **kwargs) - - # Provide older names for renamed methods - add_frag_resources = web_fragments.fragment.Fragment.add_fragment_resources - add_frags_resources = web_fragments.fragment.Fragment.add_resources diff --git a/xblock/mixins.py b/xblock/mixins.py index e921caf80..bdb473455 100644 --- a/xblock/mixins.py +++ b/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 diff --git a/xblock/runtime.py b/xblock/runtime.py index 05e8af10a..5ca1cabcc 100644 --- a/xblock/runtime.py +++ b/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 diff --git a/xblock/test/test_fragment.py b/xblock/test/test_fragment.py deleted file mode 100644 index 991343288..000000000 --- a/xblock/test/test_fragment.py +++ /dev/null @@ -1,22 +0,0 @@ -""" -Unit tests for the Fragment class. - -Note: this class has been deprecated in favor of web_fragments.fragment.Fragment -""" -from unittest import TestCase - -from xblock.fragment import Fragment - - -class TestFragment(TestCase): - """ - Unit tests for fragments. - """ - def test_fragment(self): - """ - Test the delegated Fragment class. - """ - TEST_HTML = '

Hello, world!

' # pylint: disable=invalid-name - fragment = Fragment() - fragment.add_content(TEST_HTML) - self.assertEqual(fragment.body_html(), TEST_HTML) diff --git a/xblock/test/test_parsing.py b/xblock/test/test_parsing.py index de5925295..db2447f03 100644 --- a/xblock/test/test_parsing.py +++ b/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) diff --git a/xblock/test/test_runtime.py b/xblock/test/test_runtime.py index 60e6fb170..4e58d813f 100644 --- a/xblock/test/test_runtime.py +++ b/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): diff --git a/xblock/test/toy_runtime.py b/xblock/test/toy_runtime.py index 4de1150c5..342cdb998 100644 --- a/xblock/test/toy_runtime.py +++ b/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):