From 4bed63dbb737d8ea2ab9811be4b0ac4a615ac9b9 Mon Sep 17 00:00:00 2001
From: Matt Westcott <matt@west.co.tt>
Date: Wed, 24 Jul 2024 19:17:14 +0100
Subject: [PATCH] Prevent deconstruct_with_lookup from breaking on ListBlock
subclasses with custom constructors
Fixes #12164
---
wagtail/blocks/list_block.py | 24 +++++++---
wagtail/tests/test_streamfield.py | 77 +++++++++++++++++++++++++++++++
2 files changed, 95 insertions(+), 6 deletions(-)
diff --git a/wagtail/blocks/list_block.py b/wagtail/blocks/list_block.py
index cb73f79ddb..152dda0f51 100644
--- a/wagtail/blocks/list_block.py
+++ b/wagtail/blocks/list_block.py
@@ -151,11 +151,20 @@ class ListBlock(Block):
# Default to a list consisting of one empty (i.e. default-valued) child item
self.meta.default = [self.child_block.get_default()]
+ # If a subclass of ListBlock overrides __init__, we cannot assume that the first argument is
+ # the child block, and thus we cannot rely on the conversion applied in construct_from_lookup /
+ # deconstruct_with_lookup to be valid. We set a flag attribute on the __init__ method so that
+ # we can spot this case.
+ __init__.has_child_block_arg = True
+
@classmethod
- def construct_from_lookup(cls, lookup, child_block, **kwargs):
- if isinstance(child_block, int):
- child_block = lookup.get_block(child_block)
- return cls(child_block, **kwargs)
+ def construct_from_lookup(cls, lookup, *args, **kwargs):
+ if getattr(cls.__init__, "has_child_block_arg", False) and isinstance(
+ args[0], int
+ ):
+ child_block = lookup.get_block(args[0])
+ args = (child_block, *args[1:])
+ return cls(*args, **kwargs)
def value_from_datadict(self, data, files, prefix):
count = int(data["%s-count" % prefix])
@@ -404,8 +413,11 @@ class ListBlock(Block):
def deconstruct_with_lookup(self, lookup):
path, args, kwargs = super().deconstruct_with_lookup(lookup)
- if isinstance(args[0], Block):
- args = (lookup.add_block(args[0]),)
+ if getattr(self.__init__, "has_child_block_arg", False) and isinstance(
+ args[0], Block
+ ):
+ block_id = lookup.add_block(args[0])
+ args = (block_id, *args[1:])
return path, args, kwargs
class Meta:
diff --git a/wagtail/tests/test_streamfield.py b/wagtail/tests/test_streamfield.py
index 6ad72b7e85..727847a678 100644
--- a/wagtail/tests/test_streamfield.py
+++ b/wagtail/tests/test_streamfield.py
@@ -824,6 +824,13 @@ class TestConstructStreamFieldFromLookup(TestCase):
self.assertEqual(link_text_block.name, "link_text")
+# Used by TestDeconstructStreamFieldWithLookup.test_deconstruct_with_listblock_subclass -
+# needs to be a module-level definition so that the path returned from deconstruct is valid
+class BulletListBlock(blocks.ListBlock):
+ def __init__(self, **kwargs):
+ super().__init__(blocks.CharBlock(required=True), **kwargs)
+
+
class TestDeconstructStreamFieldWithLookup(TestCase):
def test_deconstruct(self):
class ButtonBlock(blocks.StructBlock):
@@ -874,3 +881,73 @@ class TestDeconstructStreamFieldWithLookup(TestCase):
},
},
)
+
+ def test_deconstruct_with_listblock(self):
+ field = StreamField(
+ [
+ ("heading", blocks.CharBlock(required=True)),
+ ("bullets", blocks.ListBlock(blocks.CharBlock(required=True))),
+ ],
+ blank=True,
+ )
+ field.set_attributes_from_name("body")
+ name, path, args, kwargs = field.deconstruct()
+ self.assertEqual(name, "body")
+ self.assertEqual(path, "wagtail.fields.StreamField")
+ self.assertEqual(
+ args,
+ [
+ [
+ ("heading", 0),
+ ("bullets", 1),
+ ]
+ ],
+ )
+ self.assertEqual(
+ kwargs,
+ {
+ "blank": True,
+ "block_lookup": {
+ 0: ("wagtail.blocks.CharBlock", (), {"required": True}),
+ 1: ("wagtail.blocks.ListBlock", (0,), {}),
+ },
+ },
+ )
+
+ def test_deconstruct_with_listblock_subclass(self):
+ # See https://github.com/wagtail/wagtail/issues/12164 - unlike StructBlock and StreamBlock,
+ # ListBlock's deconstruct method doesn't reduce subclasses to the base ListBlock class.
+ # Therefore, if a ListBlock subclass defines its own __init__ method with an incompatible
+ # signature to the base ListBlock, this custom signature will be preserved in the result of
+ # deconstruct(), and we cannot rely on the first argument being the child block.
+
+ field = StreamField(
+ [
+ ("heading", blocks.CharBlock(required=True)),
+ ("bullets", BulletListBlock()),
+ ],
+ blank=True,
+ )
+ field.set_attributes_from_name("body")
+ name, path, args, kwargs = field.deconstruct()
+ self.assertEqual(name, "body")
+ self.assertEqual(path, "wagtail.fields.StreamField")
+ self.assertEqual(
+ args,
+ [
+ [
+ ("heading", 0),
+ ("bullets", 1),
+ ]
+ ],
+ )
+ self.assertEqual(
+ kwargs,
+ {
+ "blank": True,
+ "block_lookup": {
+ 0: ("wagtail.blocks.CharBlock", (), {"required": True}),
+ 1: ("wagtail.tests.test_streamfield.BulletListBlock", (), {}),
+ },
+ },
+ )