Be consistent about what kind of value the 'default' of a Block is.

Most places assume that it's the block's native value type (e.g. ListBlock.html_declarations calls child_block.render_form with it), but StructBlock erroneously calls to_python on it (which would imply that it's expecting a JSONish value), and some native value types (e.g. StreamValue / StructValue) can't be passed as defaults because they contain a reference to the not-yet-constructed block object.

To get around this, we introduce a block.get_default() method which is guaranteed to return a native value (performing type conversions when the 'default' passed in the constructor / meta class isn't one already) and avoid calling to_python on this return value.
This commit is contained in:
Matt Westcott 2015-05-28 11:41:48 +01:00
parent ca0085c68d
commit deb9df4185
5 changed files with 138 additions and 16 deletions

View file

@ -152,13 +152,23 @@ class Block(six.with_metaclass(BaseBlock, object)):
"""
return BoundBlock(self, value, prefix=prefix, errors=errors)
def get_default(self):
"""
Return this block's default value (conventionally found in self.meta.default),
converted to the value type expected by this block. This caters for the case
where that value type is not something that can be expressed statically at
model definition type (e.g. something like StructValue which incorporates a
pointer back to the block definion object).
"""
return self.meta.default
def prototype_block(self):
"""
Return a BoundBlock that can be used as a basis for new empty block instances to be added on the fly
(new list items, for example). This will have a prefix of '__PREFIX__' (to be dynamically replaced with
a real prefix when it's inserted into the page) and a value equal to the block's default value.
"""
return self.bind(self.meta.default, '__PREFIX__')
return self.bind(self.get_default(), '__PREFIX__')
def clean(self, value):
"""

View file

@ -28,7 +28,7 @@ class ListBlock(Block):
if not hasattr(self.meta, 'default'):
# Default to a list consisting of one empty (i.e. default-valued) child item
self.meta.default = [self.child_block.meta.default]
self.meta.default = [self.child_block.get_default()]
self.dependencies = [self.child_block]
self.child_js_initializer = self.child_block.js_initializer()
@ -54,7 +54,7 @@ class ListBlock(Block):
# this is the output of render_list_member as rendered with the prefix '__PREFIX__'
# (to be replaced dynamically when adding the new item) and the child block's default value
# as its value.
list_member_html = self.render_list_member(self.child_block.meta.default, '__PREFIX__', '')
list_member_html = self.render_list_member(self.child_block.get_default(), '__PREFIX__', '')
return format_html(
'<script type="text/template" id="{0}-newmember">{1}</script>',

View file

@ -22,13 +22,8 @@ __all__ = ['BaseStreamBlock', 'StreamBlock', 'StreamValue']
class BaseStreamBlock(Block):
# TODO: decide what it means to pass a 'default' arg to StreamBlock's constructor. Logically we want it to be
# of type StreamValue, but we can't construct one of those because it needs a reference back to the StreamBlock
# that we haven't constructed yet...
class Meta:
@property
def default(self):
return StreamValue(self, [])
default = []
def __init__(self, local_blocks=None, **kwargs):
self._constructor_kwargs = kwargs
@ -43,6 +38,17 @@ class BaseStreamBlock(Block):
self.dependencies = self.child_blocks.values()
def get_default(self):
"""
Default values set on a StreamBlock should be a list of (type_name, value) tuples -
we can't use StreamValue directly, because that would require a reference back to
the StreamBlock that hasn't been built yet.
For consistency, then, we need to convert it to a StreamValue here for StreamBlock
to work with.
"""
return StreamValue(self, self.meta.default)
def render_list_member(self, block_type_name, value, prefix, index, errors=None):
"""
Render the HTML for a single list item. This consists of an <li> wrapper, hidden fields
@ -65,7 +71,7 @@ class BaseStreamBlock(Block):
(
self.definition_prefix,
name,
mark_safe(escape_script(self.render_list_member(name, child_block.meta.default, '__PREFIX__', '')))
mark_safe(escape_script(self.render_list_member(name, child_block.get_default(), '__PREFIX__', '')))
)
for name, child_block in self.child_blocks.items()
]

View file

@ -42,6 +42,14 @@ class BaseStructBlock(Block):
self.dependencies = self.child_blocks.values()
def get_default(self):
"""
Any default value passed in the constructor or self.meta is going to be a dict
rather than a StructValue; for consistency, we need to convert it to a StructValue
for StructBlock to work with
"""
return StructValue(self, self.meta.default.items())
def js_initializer(self):
# skip JS setup entirely if no children have js_initializers
if not self.child_js_initializers:
@ -64,7 +72,7 @@ class BaseStructBlock(Block):
error_dict = {}
child_renderings = [
block.render_form(value.get(name, block.meta.default), prefix="%s-%s" % (prefix, name),
block.render_form(value.get(name, block.get_default()), prefix="%s-%s" % (prefix, name),
errors=error_dict.get(name))
for name, block in self.child_blocks.items()
]
@ -106,7 +114,9 @@ class BaseStructBlock(Block):
return StructValue(self, [
(
name,
child_block.to_python(value.get(name, child_block.meta.default))
(child_block.to_python(value[name]) if name in value else child_block.get_default())
# NB the result of get_default is NOT passed through to_python, as it's expected
# to be in the block's native type already
)
for name, child_block in self.child_blocks.items()
])
@ -122,7 +132,7 @@ class BaseStructBlock(Block):
content = []
for name, block in self.child_blocks.items():
content.extend(block.get_searchable_content(value.get(name, block.meta.default)))
content.extend(block.get_searchable_content(value.get(name, block.get_default())))
return content

View file

@ -454,6 +454,24 @@ class TestStructBlock(unittest.TestCase):
self.assertTrue(isinstance(struct_val, blocks.StructValue))
self.assertTrue(isinstance(struct_val.bound_blocks['link'].block, blocks.URLBlock))
def test_default_is_returned_as_structvalue(self):
"""When returning the default value of a StructBlock (e.g. because it's
a child of another StructBlock, and the outer value is missing that key)
we should receive it as a StructValue, not just a plain dict"""
class PersonBlock(blocks.StructBlock):
first_name = blocks.CharBlock()
surname = blocks.CharBlock()
class EventBlock(blocks.StructBlock):
title = blocks.CharBlock()
guest_speaker = PersonBlock(default={'first_name': 'Ed', 'surname': 'Balls'})
event_block = EventBlock()
event = event_block.to_python({'title': 'Birthday party'})
self.assertEqual(event['guest_speaker']['first_name'], 'Ed')
self.assertTrue(isinstance(event['guest_speaker'], blocks.StructValue))
class TestListBlock(unittest.TestCase):
@ -639,6 +657,34 @@ class TestListBlock(unittest.TestCase):
block_value = block.value_from_datadict(post_data, {}, 'shoppinglist')
self.assertEqual(block_value[2], "item 2")
def test_can_specify_default(self):
class ShoppingListBlock(blocks.StructBlock):
shop = blocks.CharBlock()
items = blocks.ListBlock(blocks.CharBlock(), default=['peas', 'beans', 'carrots'])
block = ShoppingListBlock()
# the value here does not specify an 'items' field, so this should revert to the ListBlock's default
form_html = block.render_form(block.to_python({'shop': 'Tesco'}), prefix='shoppinglist')
self.assertIn('<input type="hidden" name="shoppinglist-items-count" id="shoppinglist-items-count" value="3">', form_html)
self.assertIn('value="peas"', form_html)
def test_default_default(self):
"""
if no explicit 'default' is set on the ListBlock, it should fall back on
a single instance of the child block in its default state.
"""
class ShoppingListBlock(blocks.StructBlock):
shop = blocks.CharBlock()
items = blocks.ListBlock(blocks.CharBlock(default='chocolate'))
block = ShoppingListBlock()
# the value here does not specify an 'items' field, so this should revert to the ListBlock's default
form_html = block.render_form(block.to_python({'shop': 'Tesco'}), prefix='shoppinglist')
self.assertIn('<input type="hidden" name="shoppinglist-items-count" id="shoppinglist-items-count" value="1">', form_html)
self.assertIn('value="chocolate"', form_html)
class TestStreamBlock(unittest.TestCase):
def test_initialisation(self):
@ -919,7 +965,57 @@ class TestStreamBlock(unittest.TestCase):
content = block.get_searchable_content(value)
self.assertEqual(content, [
"My title",
"My first paragraph",
"My second paragraph",
"My title",
"My first paragraph",
"My second paragraph",
])
def test_meta_default(self):
"""Test that we can specify a default value in the Meta of a StreamBlock"""
class ArticleBlock(blocks.StreamBlock):
heading = blocks.CharBlock()
paragraph = blocks.CharBlock()
class Meta:
default = [('heading', 'A default heading')]
# to access the default value, we retrieve it through a StructBlock
# from a struct value that's missing that key
class ArticleContainerBlock(blocks.StructBlock):
author = blocks.CharBlock()
article = ArticleBlock()
block = ArticleContainerBlock()
struct_value = block.to_python({'author': 'Bob'})
stream_value = struct_value['article']
self.assertTrue(isinstance(stream_value, blocks.StreamValue))
self.assertEqual(len(stream_value), 1)
self.assertEqual(stream_value[0].block_type, 'heading')
self.assertEqual(stream_value[0].value, 'A default heading')
def test_constructor_default(self):
"""Test that we can specify a default value in the constructor of a StreamBlock"""
class ArticleBlock(blocks.StreamBlock):
heading = blocks.CharBlock()
paragraph = blocks.CharBlock()
class Meta:
default = [('heading', 'A default heading')]
# to access the default value, we retrieve it through a StructBlock
# from a struct value that's missing that key
class ArticleContainerBlock(blocks.StructBlock):
author = blocks.CharBlock()
article = ArticleBlock(default=[('heading', 'A different default heading')])
block = ArticleContainerBlock()
struct_value = block.to_python({'author': 'Bob'})
stream_value = struct_value['article']
self.assertTrue(isinstance(stream_value, blocks.StreamValue))
self.assertEqual(len(stream_value), 1)
self.assertEqual(stream_value[0].block_type, 'heading')
self.assertEqual(stream_value[0].value, 'A different default heading')