From facf19df95a1bb157f6d8cd3d5c1e61e9fd93fe5 Mon Sep 17 00:00:00 2001 From: Matt Westcott Date: Wed, 21 Sep 2016 15:03:35 +0100 Subject: [PATCH] Implement value_omitted_from_data on Block This allows Django >=1.10.2's ModelForm logic to determine whether or not the field has been omitted from the form submission (and should thus revert to the initial or default value), since the default rule of "look for an item in the postdata matching the field's name" doesn't work for Block-based fields such as StreamField. Fixes #2994 for Django 1.10.2 and above (assuming https://github.com/django/django/pull/7217 gets merged without major changes...) --- wagtail/wagtailcore/blocks/base.py | 11 ++++++ wagtail/wagtailcore/blocks/field_block.py | 3 ++ wagtail/wagtailcore/blocks/list_block.py | 3 ++ wagtail/wagtailcore/blocks/stream_block.py | 3 ++ wagtail/wagtailcore/blocks/struct_block.py | 6 +++ wagtail/wagtailcore/tests/test_blocks.py | 46 ++++++++++++++++++++++ 6 files changed, 72 insertions(+) diff --git a/wagtail/wagtailcore/blocks/base.py b/wagtail/wagtailcore/blocks/base.py index 24b9f4624..e8e1e6007 100644 --- a/wagtail/wagtailcore/blocks/base.py +++ b/wagtail/wagtailcore/blocks/base.py @@ -170,6 +170,14 @@ class Block(six.with_metaclass(BaseBlock, object)): def value_from_datadict(self, data, files, prefix): raise NotImplementedError('%s.value_from_datadict' % self.__class__) + def value_omitted_from_data(self, data, files, name): + """ + Used only for top-level blocks wrapped by BlockWidget (i.e.: typically only StreamBlock) + to inform ModelForm logic on Django >=1.10.2 whether the field is absent from the form + submission (and should therefore revert to the field default). + """ + return name not in data + def bind(self, value, prefix=None, errors=None): """ Return a BoundBlock which represents the association of this block definition with a value @@ -595,6 +603,9 @@ class BlockWidget(forms.Widget): def value_from_datadict(self, data, files, name): return self.block_def.value_from_datadict(data, files, name) + def value_omitted_from_data(self, data, files, name): + return self.block_def.value_omitted_from_data(data, files, name) + class BlockField(forms.Field): """Wraps a block object as a form field so that it can be incorporated into a Django form""" diff --git a/wagtail/wagtailcore/blocks/field_block.py b/wagtail/wagtailcore/blocks/field_block.py index 71af4b90b..0e42ddf90 100644 --- a/wagtail/wagtailcore/blocks/field_block.py +++ b/wagtail/wagtailcore/blocks/field_block.py @@ -68,6 +68,9 @@ class FieldBlock(Block): def value_from_datadict(self, data, files, prefix): return self.value_from_form(self.field.widget.value_from_datadict(data, files, prefix)) + def value_omitted_from_data(self, data, files, prefix): + return self.field.widget.value_omitted_from_data(data, files, prefix) + def clean(self, value): # We need an annoying value_for_form -> value_from_form round trip here to account for # the possibility that the form field is set up to validate a different value type to diff --git a/wagtail/wagtailcore/blocks/list_block.py b/wagtail/wagtailcore/blocks/list_block.py index dde7d753f..495a5d1fc 100644 --- a/wagtail/wagtailcore/blocks/list_block.py +++ b/wagtail/wagtailcore/blocks/list_block.py @@ -108,6 +108,9 @@ class ListBlock(Block): values_with_indexes.sort() return [v for (i, v) in values_with_indexes] + def value_omitted_from_data(self, data, files, prefix): + return ('%s-count' % prefix) not in data + def clean(self, value): result = [] errors = [] diff --git a/wagtail/wagtailcore/blocks/stream_block.py b/wagtail/wagtailcore/blocks/stream_block.py index 240b15d76..b515c40a9 100644 --- a/wagtail/wagtailcore/blocks/stream_block.py +++ b/wagtail/wagtailcore/blocks/stream_block.py @@ -171,6 +171,9 @@ class BaseStreamBlock(Block): for (index, child_block_type_name, value) in values_with_indexes ]) + def value_omitted_from_data(self, data, files, prefix): + return ('%s-count' % prefix) not in data + def clean(self, value): cleaned_data = [] errors = {} diff --git a/wagtail/wagtailcore/blocks/struct_block.py b/wagtail/wagtailcore/blocks/struct_block.py index 67e73d3d7..dc60e1b16 100644 --- a/wagtail/wagtailcore/blocks/struct_block.py +++ b/wagtail/wagtailcore/blocks/struct_block.py @@ -97,6 +97,12 @@ class BaseStructBlock(Block): for name, block in self.child_blocks.items() ]) + def value_omitted_from_data(self, data, files, prefix): + return all( + block.value_omitted_from_data(data, files, '%s-%s' % (prefix, name)) + for name, block in self.child_blocks.items() + ) + def clean(self, value): result = [] # build up a list of (name, value) tuples to be passed to the StructValue constructor errors = {} diff --git a/wagtail/wagtailcore/tests/test_blocks.py b/wagtail/wagtailcore/tests/test_blocks.py index e9029bdd2..57013afb8 100644 --- a/wagtail/wagtailcore/tests/test_blocks.py +++ b/wagtail/wagtailcore/tests/test_blocks.py @@ -9,6 +9,7 @@ import warnings from decimal import Decimal # non-standard import name for ugettext_lazy, to prevent strings from being picked up for translation +import django from django import forms from django.core.exceptions import ValidationError from django.forms.utils import ErrorList @@ -692,6 +693,13 @@ class TestRawHTMLBlock(unittest.TestCase): self.assertEqual(result, 'BÖÖM') self.assertIsInstance(result, SafeData) + @unittest.skipIf(django.VERSION < (1, 10, 2), "value_omitted_from_data is not available") + def test_value_omitted_from_data(self): + block = blocks.RawHTMLBlock() + self.assertFalse(block.value_omitted_from_data({'rawhtml': 'ohai'}, {}, 'rawhtml')) + self.assertFalse(block.value_omitted_from_data({'rawhtml': ''}, {}, 'rawhtml')) + self.assertTrue(block.value_omitted_from_data({'nothing-here': 'nope'}, {}, 'rawhtml')) + def test_clean_required_field(self): block = blocks.RawHTMLBlock() result = block.clean(mark_safe('BÖÖM')) @@ -1096,6 +1104,17 @@ class TestStructBlock(SimpleTestCase): self.assertTrue(isinstance(struct_val, blocks.StructValue)) self.assertTrue(isinstance(struct_val.bound_blocks['link'].block, blocks.URLBlock)) + @unittest.skipIf(django.VERSION < (1, 10, 2), "value_omitted_from_data is not available") + def test_value_omitted_from_data(self): + block = blocks.StructBlock([ + ('title', blocks.CharBlock()), + ('link', blocks.URLBlock()), + ]) + + # overall value is considered present in the form if any sub-field is present + self.assertFalse(block.value_omitted_from_data({'mylink-title': 'Torchbox'}, {}, 'mylink')) + self.assertTrue(block.value_omitted_from_data({'nothing-here': 'nope'}, {}, 'mylink')) + 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) @@ -1408,6 +1427,18 @@ class TestListBlock(unittest.TestCase): self.assertEqual(content, ["Wagtail", "Django"]) + @unittest.skipIf(django.VERSION < (1, 10, 2), "value_omitted_from_data is not available") + def test_value_omitted_from_data(self): + block = blocks.ListBlock(blocks.CharBlock()) + + # overall value is considered present in the form if the 'count' field is present + self.assertFalse(block.value_omitted_from_data({'mylist-count': '0'}, {}, 'mylist')) + self.assertFalse(block.value_omitted_from_data({ + 'mylist-count': '1', + 'mylist-0-value': 'hello', 'mylist-0-deleted': '', 'mylist-0-order': '0' + }, {}, 'mylist')) + self.assertTrue(block.value_omitted_from_data({'nothing-here': 'nope'}, {}, 'mylist')) + def test_ordering_in_form_submission_uses_order_field(self): block = blocks.ListBlock(blocks.CharBlock()) @@ -1784,6 +1815,21 @@ class TestStreamBlock(SimpleTestCase): html ) + @unittest.skipIf(django.VERSION < (1, 10, 2), "value_omitted_from_data is not available") + def test_value_omitted_from_data(self): + block = blocks.StreamBlock([ + ('heading', blocks.CharBlock()), + ]) + + # overall value is considered present in the form if the 'count' field is present + self.assertFalse(block.value_omitted_from_data({'mystream-count': '0'}, {}, 'mystream')) + self.assertFalse(block.value_omitted_from_data({ + 'mystream-count': '1', + 'mystream-0-type': 'heading', 'mystream-0-value': 'hello', + 'mystream-0-deleted': '', 'mystream-0-order': '0' + }, {}, 'mystream')) + self.assertTrue(block.value_omitted_from_data({'nothing-here': 'nope'}, {}, 'mystream')) + def test_validation_errors(self): class ValidatedBlock(blocks.StreamBlock): char = blocks.CharBlock()