Add system check to reject invalid block names in StreamField - fixes #1154

wagtail/wagtailcore/blocks/base.py

@@ -6,6 +6,7 @@ from __future__ import absolute_import, unicode_literals
 import collections
 from importlib import import_module
 
+from django.core import checks
 from django.core.exceptions import ImproperlyConfigured
 from django.utils.safestring import mark_safe
 from django.utils.text import capfirst
@@ -223,6 +224,54 @@ class Block(six.with_metaclass(BaseBlock, object)):
         """
         return []
 
+    def check(self, **kwargs):
+        """
+        Hook for the Django system checks framework -
+        returns a list of django.core.checks.Error objects indicating validity errors in the block
+        """
+        return []
+
+    def _check_name(self, **kwargs):
+        """
+        Helper method called by container blocks as part of the system checks framework,
+        to validate that this block's name is a valid identifier.
+        (Not called universally, because not all blocks need names)
+        """
+        errors = []
+        if not self.name:
+            errors.append(checks.Error(
+                "Block name %r is invalid" % self.name,
+                hint="Block name cannot be empty",
+                obj=kwargs.get('field', self),
+                id='wagtailcore.E001',
+            ))
+
+        if ' ' in self.name:
+            errors.append(checks.Error(
+                "Block name %r is invalid" % self.name,
+                hint="Block names cannot contain spaces",
+                obj=kwargs.get('field', self),
+                id='wagtailcore.E001',
+            ))
+
+        if '-' in self.name:
+            errors.append(checks.Error(
+                "Block name %r is invalid" % self.name,
+                "Block names cannot contain dashes",
+                obj=kwargs.get('field', self),
+                id='wagtailcore.E001',
+            ))
+
+        if self.name and self.name[0].isdigit():
+            errors.append(checks.Error(
+                "Block name %r is invalid" % self.name,
+                "Block names cannot begin with a digit",
+                obj=kwargs.get('field', self),
+                id='wagtailcore.E001',
+            ))
+
+        return errors
+
     def deconstruct(self):
         # adapted from django.utils.deconstruct.deconstructible
         module_name = self.__module__

wagtail/wagtailcore/blocks/list_block.py

@@ -153,6 +153,11 @@ class ListBlock(Block):
 
         return content
 
+    def check(self, **kwargs):
+        errors = super(ListBlock, self).check(**kwargs)
+        errors.extend(self.child_block.check(**kwargs))
+        return errors
+
 DECONSTRUCT_ALIASES = {
     ListBlock: 'wagtail.wagtailcore.blocks.ListBlock',
 }

wagtail/wagtailcore/blocks/stream_block.py

@@ -223,6 +223,14 @@ class BaseStreamBlock(Block):
         kwargs = self._constructor_kwargs
         return (path, args, kwargs)
 
+    def check(self, **kwargs):
+        errors = super(BaseStreamBlock, self).check(**kwargs)
+        for name, child_block in self.child_blocks.items():
+            errors.extend(child_block.check(**kwargs))
+            errors.extend(child_block._check_name(**kwargs))
+
+        return errors
+
 
 class StreamBlock(six.with_metaclass(DeclarativeSubBlocksMetaclass, BaseStreamBlock)):
     pass

wagtail/wagtailcore/blocks/struct_block.py

@@ -150,6 +150,14 @@ class BaseStructBlock(Block):
         kwargs = self._constructor_kwargs
         return (path, args, kwargs)
 
+    def check(self, **kwargs):
+        errors = super(BaseStructBlock, self).check(**kwargs)
+        for name, child_block in self.child_blocks.items():
+            errors.extend(child_block.check(**kwargs))
+            errors.extend(child_block._check_name(**kwargs))
+
+        return errors
+
 
 class StructBlock(six.with_metaclass(DeclarativeSubBlocksMetaclass, BaseStructBlock)):
     pass

wagtail/wagtailcore/fields.py

@@ -107,3 +107,8 @@ class StreamField(with_metaclass(models.SubfieldBase, models.Field)):
 
     def get_searchable_content(self, value):
         return self.stream_block.get_searchable_content(value)
+
+    def check(self, **kwargs):
+        errors = super(StreamField, self).check(**kwargs)
+        errors.extend(self.stream_block.check(field=self, **kwargs))
+        return errors

wagtail/wagtailcore/tests/test_blocks.py

@@ -1249,3 +1249,100 @@ class TestPageChooserBlock(TestCase):
 
         self.assertEqual(nonrequired_block.clean(christmas_page), christmas_page)
         self.assertEqual(nonrequired_block.clean(None), None)
+
+
+class TestSystemCheck(TestCase):
+    def test_name_must_be_nonempty(self):
+        block = blocks.StreamBlock([
+            ('heading', blocks.CharBlock()),
+            ('', blocks.RichTextBlock()),
+        ])
+
+        errors = block.check()
+        self.assertEqual(len(errors), 1)
+        self.assertEqual(errors[0].id, 'wagtailcore.E001')
+        self.assertEqual(errors[0].hint, "Block name cannot be empty")
+
+    def test_name_cannot_contain_spaces(self):
+        block = blocks.StreamBlock([
+            ('heading', blocks.CharBlock()),
+            ('rich text', blocks.RichTextBlock()),
+        ])
+
+        errors = block.check()
+        self.assertEqual(len(errors), 1)
+        self.assertEqual(errors[0].id, 'wagtailcore.E001')
+        self.assertEqual(errors[0].hint, "Block names cannot contain spaces")
+
+    def test_name_cannot_contain_dashes(self):
+        block = blocks.StreamBlock([
+            ('heading', blocks.CharBlock()),
+            ('rich-text', blocks.RichTextBlock()),
+        ])
+
+        errors = block.check()
+        self.assertEqual(len(errors), 1)
+        self.assertEqual(errors[0].id, 'wagtailcore.E001')
+        self.assertEqual(errors[0].hint, "Block names cannot contain dashes")
+
+    def test_name_cannot_begin_with_digit(self):
+        block = blocks.StreamBlock([
+            ('heading', blocks.CharBlock()),
+            ('99richtext', blocks.RichTextBlock()),
+        ])
+
+        errors = block.check()
+        self.assertEqual(len(errors), 1)
+        self.assertEqual(errors[0].id, 'wagtailcore.E001')
+        self.assertEqual(errors[0].hint, "Block names cannot begin with a digit")
+
+    def test_system_checks_recurse_into_lists(self):
+        block = blocks.StreamBlock([
+            ('paragraph_list', blocks.ListBlock(
+                blocks.StructBlock([
+                    ('heading', blocks.CharBlock()),
+                    ('rich text', blocks.RichTextBlock()),
+                ])
+            ))
+        ])
+
+        errors = block.check()
+        self.assertEqual(len(errors), 1)
+        self.assertEqual(errors[0].id, 'wagtailcore.E001')
+        self.assertEqual(errors[0].hint, "Block names cannot contain spaces")
+
+    def test_system_checks_recurse_into_streams(self):
+        block = blocks.StreamBlock([
+            ('carousel', blocks.StreamBlock([
+                ('text', blocks.StructBlock([
+                    ('heading', blocks.CharBlock()),
+                    ('rich text', blocks.RichTextBlock()),
+                ]))
+            ]))
+        ])
+
+        errors = block.check()
+        self.assertEqual(len(errors), 1)
+        self.assertEqual(errors[0].id, 'wagtailcore.E001')
+        self.assertEqual(errors[0].hint, "Block names cannot contain spaces")
+
+    def test_system_checks_recurse_into_structs(self):
+        block = blocks.StreamBlock([
+            ('two_column', blocks.StructBlock([
+                ('left', blocks.StructBlock([
+                    ('heading', blocks.CharBlock()),
+                    ('rich text', blocks.RichTextBlock()),
+                ])),
+                ('right', blocks.StructBlock([
+                    ('heading', blocks.CharBlock()),
+                    ('rich text', blocks.RichTextBlock()),
+                ]))
+            ]))
+        ])
+
+        errors = block.check()
+        self.assertEqual(len(errors), 2)
+        self.assertEqual(errors[0].id, 'wagtailcore.E001')
+        self.assertEqual(errors[0].hint, "Block names cannot contain spaces")
+        self.assertEqual(errors[1].id, 'wagtailcore.E001')
+        self.assertEqual(errors[1].hint, "Block names cannot contain spaces")

wagtail/wagtailcore/tests/test_streamfield.py

@@ -1,8 +1,11 @@
 import json
 
 from django.test import TestCase
+from django.db import models
 
 from wagtail.tests.testapp.models import StreamModel
+from wagtail.wagtailcore import blocks
+from wagtail.wagtailcore.fields import StreamField
 from wagtail.wagtailimages.models import Image
 from wagtail.wagtailimages.tests.utils import get_test_image_file
 
@@ -70,3 +73,15 @@ class TestLazyStreamField(TestCase):
 
         with self.assertNumQueries(0):
             instances_lookup[self.no_image.pk].body[0]
+
+    def test_system_check_validates_block(self):
+        class InvalidStreamModel(models.Model):
+            body = StreamField([
+                ('heading', blocks.CharBlock()),
+                ('rich text', blocks.RichTextBlock()),
+            ])
+
+        errors = InvalidStreamModel.check()
+        self.assertEqual(len(errors), 1)
+        self.assertEqual(errors[0].id, 'wagtailcore.E001')
+        self.assertEqual(errors[0].hint, "Block names cannot contain spaces")