From 2e5dd9aa5505900f0687bedb6c1261e048f954c1 Mon Sep 17 00:00:00 2001 From: Matt Westcott Date: Tue, 1 Aug 2017 12:58:51 +0100 Subject: [PATCH] Refactor #3605 to work alongside 'required' param --- docs/topics/streamfield.rst | 4 +- wagtail/wagtailcore/blocks/stream_block.py | 72 +++++++++------------- wagtail/wagtailcore/tests/test_blocks.py | 40 +++++++++--- 3 files changed, 63 insertions(+), 53 deletions(-) diff --git a/docs/topics/streamfield.rst b/docs/topics/streamfield.rst index 6bab7fc78e..3a14439a3b 100644 --- a/docs/topics/streamfield.rst +++ b/docs/topics/streamfield.rst @@ -441,7 +441,7 @@ Since ``StreamField`` accepts an instance of ``StreamBlock`` as a parameter, in .. code-block:: python class HomePage(Page): - carousel = StreamField(CarouselBlock(max_num=10, min_max_fields={'video': {'max_num': 2}})) + carousel = StreamField(CarouselBlock(max_num=10, block_counts={'video': {'max_num': 2}})) ``StreamBlock`` accepts the following options as either keyword arguments or ``Meta`` properties: @@ -454,7 +454,7 @@ Since ``StreamField`` accepts an instance of ``StreamBlock`` as a parameter, in ``max_num`` Maximum number of sub-blocks that the stream may have. -``min_max_fields`` +``block_counts`` Specifies the minimum and maximum number of each block type, as a dictionary mapping block names to dicts with (optional) ``min_num`` and ``max_num`` fields. diff --git a/wagtail/wagtailcore/blocks/stream_block.py b/wagtail/wagtailcore/blocks/stream_block.py index 067b46f8d5..296fe9afd3 100644 --- a/wagtail/wagtailcore/blocks/stream_block.py +++ b/wagtail/wagtailcore/blocks/stream_block.py @@ -36,16 +36,9 @@ class StreamBlockValidationError(ValidationError): class BaseStreamBlock(Block): - def __init__(self, local_blocks=None, min_num=None, max_num=None, min_max_fields=None, **kwargs): + def __init__(self, local_blocks=None, **kwargs): self._constructor_kwargs = kwargs - # Used to validate the minimum and maximum number of elements in the block - self.min_num = min_num - self.max_num = max_num - if min_max_fields is None: - min_max_fields = {} - self.min_max_fields = min_max_fields - super(BaseStreamBlock, self).__init__(**kwargs) # create a local (shallow) copy of base_blocks so that it can be supplemented by local_blocks @@ -201,46 +194,36 @@ class BaseStreamBlock(Block): except ValidationError as e: errors[i] = ErrorList([e]) - if self.required and len(value) == 0: - non_block_errors.append(ValidationError('This field is required', code='invalid')) - - # Validate that the min_num and max_num has a value - # and if it does meet the conditions of the number of components in the block - if self.min_num and self.min_num > len(value): - non_block_errors.append(ErrorList( - [_('The minimum number of items is %s' % self.min_num)] + if self.meta.min_num is not None and self.meta.min_num > len(value): + non_block_errors.append(ValidationError( + _('The minimum number of items is %d') % self.meta.min_num )) - if self.max_num and self.max_num < len(value): - non_block_errors.append(ErrorList( - [_('The maximum number of items is %s' % self.max_num)] + elif self.required and len(value) == 0: + non_block_errors.append(ValidationError(_('This field is required.'))) + + if self.meta.max_num is not None and self.meta.max_num < len(value): + non_block_errors.append(ValidationError( + _('The maximum number of items is %d') % self.meta.max_num )) - if self.min_max_fields: - fields = {} + if self.meta.block_counts: + block_counts = collections.defaultdict(int) for item in value: - if item.block_type not in self.min_max_fields: - continue - if item.block_type not in fields: - fields[item.block_type] = 0 - fields[item.block_type] += 1 + block_counts[item.block_type] += 1 - for field in self.min_max_fields: - field_title = field.replace('_', ' ').title() - max_num = self.min_max_fields[field].get('max_num', None) - min_num = self.min_max_fields[field].get('min_num', None) - if field in fields: - if min_num and min_num > fields[field]: - non_block_errors.append(ErrorList( - ['{}: {}'.format(field_title, _('The minimum number of items is %s' % min_num))] - )) - if max_num and max_num < fields[field]: - non_block_errors.append(ErrorList( - ['{}: {}'.format(field_title, _('The maximum number of items is %s' % max_num))] - )) - elif min_num: - non_block_errors.append(ErrorList( - ['{}: {}'.format(field_title, _('The minimum number of items is %s' % min_num))] - )) + for block_name, min_max in self.meta.block_counts.items(): + block = self.child_blocks[block_name] + max_num = min_max.get('max_num', None) + min_num = min_max.get('min_num', None) + block_count = block_counts[block_name] + if min_num is not None and min_num > block_count: + non_block_errors.append(ValidationError( + '{}: {}'.format(block.label, _('The minimum number of items is %d') % min_num) + )) + if max_num is not None and max_num < block_count: + non_block_errors.append(ValidationError( + '{}: {}'.format(block.label, _('The maximum number of items is %d') % max_num) + )) if errors or non_block_errors: # The message here is arbitrary - outputting error messages is delegated to the child blocks, @@ -334,6 +317,9 @@ class BaseStreamBlock(Block): icon = "placeholder" default = [] required = True + min_num = None + max_num = None + block_counts = {} class StreamBlock(six.with_metaclass(DeclarativeSubBlocksMetaclass, BaseStreamBlock)): diff --git a/wagtail/wagtailcore/tests/test_blocks.py b/wagtail/wagtailcore/tests/test_blocks.py index 4cb00c8628..af654909ed 100644 --- a/wagtail/wagtailcore/tests/test_blocks.py +++ b/wagtail/wagtailcore/tests/test_blocks.py @@ -2051,9 +2051,13 @@ class TestStreamBlock(WagtailTestUtils, SimpleTestCase): with self.assertRaises(ValidationError) as catcher: block.clean(value) self.assertEqual(catcher.exception.params, { - '__all__': [['The minimum number of items is 1']] + '__all__': ['The minimum number of items is 1'] }) + # a value with >= 1 blocks should pass validation + value = blocks.StreamValue(block, [('char', 'foo')]) + self.assertTrue(block.clean(value)) + def test_max_num_validation_errors(self): class ValidatedBlock(blocks.StreamBlock): char = blocks.CharBlock() @@ -2070,14 +2074,18 @@ class TestStreamBlock(WagtailTestUtils, SimpleTestCase): with self.assertRaises(ValidationError) as catcher: block.clean(value) self.assertEqual(catcher.exception.params, { - '__all__': [['The maximum number of items is 1']] + '__all__': ['The maximum number of items is 1'] }) - def test_min_max_fields_min_validation_errors(self): + # a value with 1 block should pass validation + value = blocks.StreamValue(block, [('char', 'foo')]) + self.assertTrue(block.clean(value)) + + def test_block_counts_min_validation_errors(self): class ValidatedBlock(blocks.StreamBlock): char = blocks.CharBlock() url = blocks.URLBlock() - block = ValidatedBlock(min_max_fields={'char': {'min_num': 1}}) + block = ValidatedBlock(block_counts={'char': {'min_num': 1}}) value = blocks.StreamValue(block, [ ('url', 'http://example.com/'), @@ -2087,14 +2095,22 @@ class TestStreamBlock(WagtailTestUtils, SimpleTestCase): with self.assertRaises(ValidationError) as catcher: block.clean(value) self.assertEqual(catcher.exception.params, { - '__all__': [['Char: The minimum number of items is 1']] + '__all__': ['Char: The minimum number of items is 1'] }) - def test_min_max_fields_max_validation_errors(self): + # a value with 1 char block should pass validation + value = blocks.StreamValue(block, [ + ('url', 'http://example.com/'), + ('char', 'foo'), + ('url', 'http://example.com/'), + ]) + self.assertTrue(block.clean(value)) + + def test_block_counts_max_validation_errors(self): class ValidatedBlock(blocks.StreamBlock): char = blocks.CharBlock() url = blocks.URLBlock() - block = ValidatedBlock(min_max_fields={'char': {'max_num': 1}}) + block = ValidatedBlock(block_counts={'char': {'max_num': 1}}) value = blocks.StreamValue(block, [ ('char', 'foo'), @@ -2106,9 +2122,17 @@ class TestStreamBlock(WagtailTestUtils, SimpleTestCase): with self.assertRaises(ValidationError) as catcher: block.clean(value) self.assertEqual(catcher.exception.params, { - '__all__': [['Char: The maximum number of items is 1']] + '__all__': ['Char: The maximum number of items is 1'] }) + # a value with 1 char block should pass validation + value = blocks.StreamValue(block, [ + ('char', 'foo'), + ('url', 'http://example.com/'), + ('url', 'http://example.com/'), + ]) + self.assertTrue(block.clean(value)) + def test_block_level_validation_renders_errors(self): block = FooStreamBlock()