diff --git a/wagtail/wagtailcore/blocks/base.py b/wagtail/wagtailcore/blocks/base.py index 362fb7931b..ba483c309d 100644 --- a/wagtail/wagtailcore/blocks/base.py +++ b/wagtail/wagtailcore/blocks/base.py @@ -393,7 +393,8 @@ class DeclarativeSubBlocksMetaclass(BaseBlock): (cheerfully stolen from https://github.com/django/django/blob/master/django/forms/forms.py) """ def __new__(mcs, name, bases, attrs): - # Collect sub-blocks from current class. + # Collect sub-blocks declared on the current class. + # These are available on the class as `declared_blocks` current_blocks = [] for key, value in list(attrs.items()): if isinstance(value, Block): @@ -403,23 +404,22 @@ class DeclarativeSubBlocksMetaclass(BaseBlock): current_blocks.sort(key=lambda x: x[1].creation_counter) attrs['declared_blocks'] = collections.OrderedDict(current_blocks) - new_class = (super(DeclarativeSubBlocksMetaclass, mcs) - .__new__(mcs, name, bases, attrs)) + new_class = (super(DeclarativeSubBlocksMetaclass, mcs).__new__( + mcs, name, bases, attrs)) - # Walk through the MRO. - declared_blocks = collections.OrderedDict() + # Walk through the MRO, collecting all inherited sub-blocks, to make + # the combined `base_blocks`. + base_blocks = collections.OrderedDict() for base in reversed(new_class.__mro__): # Collect sub-blocks from base class. if hasattr(base, 'declared_blocks'): - declared_blocks.update(base.declared_blocks) + base_blocks.update(base.declared_blocks) # Field shadowing. for attr, value in base.__dict__.items(): - if value is None and attr in declared_blocks: - declared_blocks.pop(attr) - - new_class.base_blocks = declared_blocks - new_class.declared_blocks = declared_blocks + if value is None and attr in base_blocks: + base_blocks.pop(attr) + new_class.base_blocks = base_blocks return new_class diff --git a/wagtail/wagtailcore/blocks/field_block.py b/wagtail/wagtailcore/blocks/field_block.py index 2453046fb9..8e4519ecdc 100644 --- a/wagtail/wagtailcore/blocks/field_block.py +++ b/wagtail/wagtailcore/blocks/field_block.py @@ -228,6 +228,20 @@ class ChoiceBlock(FieldBlock): """ return ('wagtail.wagtailcore.blocks.ChoiceBlock', [], self._constructor_kwargs) + def get_searchable_content(self, value): + # Return the display value as the searchable value + text_value = force_text(value) + for k, v in self.field.choices: + if isinstance(v, (list, tuple)): + # This is an optgroup, so look inside the group for options + for k2, v2 in v: + if value == k2 or text_value == force_text(k2): + return [k, v2] + else: + if value == k or text_value == force_text(k): + return [v] + return [] # Value was not found in the list of choices + class RichTextBlock(FieldBlock): diff --git a/wagtail/wagtailcore/blocks/struct_block.py b/wagtail/wagtailcore/blocks/struct_block.py index ac1786fa39..c71ce68c1c 100644 --- a/wagtail/wagtailcore/blocks/struct_block.py +++ b/wagtail/wagtailcore/blocks/struct_block.py @@ -160,6 +160,11 @@ class BaseStructBlock(Block): return errors + def render(self, value): + value = collections.OrderedDict( + (key, value.get(key)) for key in self.child_blocks.keys()) + return super(BaseStructBlock, self).render(value) + class StructBlock(six.with_metaclass(DeclarativeSubBlocksMetaclass, BaseStructBlock)): pass diff --git a/wagtail/wagtailcore/tests/test_blocks.py b/wagtail/wagtailcore/tests/test_blocks.py index 64b57801c5..93bcb30d80 100644 --- a/wagtail/wagtailcore/tests/test_blocks.py +++ b/wagtail/wagtailcore/tests/test_blocks.py @@ -6,7 +6,7 @@ import unittest from django import forms from django.forms.utils import ErrorList from django.core.exceptions import ValidationError -from django.test import TestCase +from django.test import TestCase, SimpleTestCase from django.utils.safestring import mark_safe, SafeData from wagtail.wagtailcore import blocks @@ -38,9 +38,9 @@ class TestFieldBlock(unittest.TestCase): def test_charfield_render_form_with_error(self): block = blocks.CharBlock() - html = block.render_form("Hello world!", - errors=ErrorList([ValidationError("This field is required.")]) - ) + html = block.render_form( + "Hello world!", + errors=ErrorList([ValidationError("This field is required.")])) self.assertIn('This field is required.', html) @@ -77,18 +77,16 @@ class TestFieldBlock(unittest.TestCase): self.assertIn('', html) self.assertIn('', html) - @unittest.expectedFailure # Returning "choice-1" instead of "Choice 1" - def test_choicefield_searchable_content(self): - class ChoiceBlock(blocks.FieldBlock): - field = forms.ChoiceField(choices=( - ('choice-1', "Choice 1"), - ('choice-2', "Choice 2"), - )) - - block = ChoiceBlock() - content = block.get_searchable_content("choice-1") - - self.assertEqual(content, ["Choice 1"]) + def test_searchable_content(self): + """ + FieldBlock should not return anything for `get_searchable_content` by + default. Subclasses are free to override it and provide relevant + content. + """ + class CustomBlock(blocks.FieldBlock): + field = forms.CharField(required=True) + block = CustomBlock() + self.assertEqual(block.get_searchable_content("foo bar"), []) def test_form_handling_is_independent_of_serialisation(self): class Base64EncodingCharBlock(blocks.CharBlock): @@ -311,6 +309,35 @@ class TestChoiceBlock(unittest.TestCase): ) ) + def test_searchable_content(self): + block = blocks.ChoiceBlock(choices=[ + ('choice-1', "Choice 1"), + ('choice-2', "Choice 2"), + ]) + self.assertEqual(block.get_searchable_content("choice-1"), + ["Choice 1"]) + + def test_optgroup_searchable_content(self): + block = blocks.ChoiceBlock(choices=[ + ('Section 1', [ + ('1-1', "Block 1"), + ('1-2', "Block 2"), + ]), + ('Section 2', [ + ('2-1', "Block 1"), + ('2-2', "Block 2"), + ]), + ]) + self.assertEqual(block.get_searchable_content("2-2"), + ["Section 2", "Block 2"]) + + def test_invalid_searchable_content(self): + block = blocks.ChoiceBlock(choices=[ + ('one', 'One'), + ('two', 'Two'), + ]) + self.assertEqual(block.get_searchable_content('three'), []) + class TestRawHTMLBlock(unittest.TestCase): def test_get_default_with_fallback_value(self): @@ -420,7 +447,7 @@ class TestMeta(unittest.TestCase): self.assertEqual(block.meta.test, 'Foo') -class TestStructBlock(unittest.TestCase): +class TestStructBlock(SimpleTestCase): def test_initialisation(self): block = blocks.StructBlock([ ('title', blocks.CharBlock()), @@ -461,8 +488,19 @@ class TestStructBlock(unittest.TestCase): self.assertEqual(list(block.child_blocks.keys()), ['title', 'link', 'classname']) - @unittest.expectedFailure # Field order doesn't match inheritance order def test_initialisation_with_mixins(self): + """ + The order of fields of classes with multiple parent classes is slightly + surprising at first. Child fields are inherited in a bottom-up order, + by traversing the MRO in reverse. In the example below, + ``StyledLinkBlock`` will have an MRO of:: + + [StyledLinkBlock, StylingMixin, LinkBlock, StructBlock, ...] + + This will result in ``classname`` appearing *after* ``title`` and + ``link`` in ``StyleLinkBlock`.child_blocks`, even though + ``StylingMixin`` appeared before ``LinkBlock``. + """ class LinkBlock(blocks.StructBlock): title = blocks.CharBlock() link = blocks.URLBlock() @@ -470,12 +508,13 @@ class TestStructBlock(unittest.TestCase): class StylingMixin(blocks.StructBlock): classname = blocks.CharBlock() - class StyledLinkBlock(LinkBlock, StylingMixin): - pass + class StyledLinkBlock(StylingMixin, LinkBlock): + source = blocks.CharBlock() block = StyledLinkBlock() - self.assertEqual(list(block.child_blocks.keys()), ['title', 'link', 'classname']) + self.assertEqual(list(block.child_blocks.keys()), + ['title', 'link', 'classname', 'source']) def test_render(self): class LinkBlock(blocks.StructBlock): @@ -487,13 +526,17 @@ class TestStructBlock(unittest.TestCase): 'title': "Wagtail site", 'link': 'http://www.wagtail.io', }) + expected_html = '\n'.join([ + '
', + '
title
', + '
Wagtail site
', + '
link
', + '
http://www.wagtail.io
', + '
', + ]) - self.assertIn('
title
', html) - self.assertIn('
Wagtail site
', html) - self.assertIn('
link
', html) - self.assertIn('
http://www.wagtail.io
', html) + self.assertHTMLEqual(html, expected_html) - @unittest.expectedFailure def test_render_unknown_field(self): class LinkBlock(blocks.StructBlock): title = blocks.CharBlock() @@ -934,8 +977,19 @@ class TestStreamBlock(unittest.TestCase): self.assertEqual(list(block.child_blocks.keys()), ['heading', 'paragraph', 'intro']) - @unittest.expectedFailure # Field order doesn't match inheritance order def test_initialisation_with_mixins(self): + """ + The order of child blocks of ``StreamBlock``\s with multiple parent + classes is slightly surprising at first. Child blocks are inherited in + a bottom-up order, by traversing the MRO in reverse. In the example + below, ``ArticleWithIntroBlock`` will have an MRO of:: + + [ArticleWithIntroBlock, IntroMixin, ArticleBlock, StreamBlock, ...] + + This will result in ``intro`` appearing *after* ``heading`` and + ``paragraph`` in ``ArticleWithIntroBlock.child_blocks``, even though + ``IntroMixin`` appeared before ``ArticleBlock``. + """ class ArticleBlock(blocks.StreamBlock): heading = blocks.CharBlock() paragraph = blocks.CharBlock() @@ -943,12 +997,13 @@ class TestStreamBlock(unittest.TestCase): class IntroMixin(blocks.StreamBlock): intro = blocks.CharBlock() - class ArticleWithIntroBlock(ArticleBlock, IntroMixin): - pass + class ArticleWithIntroBlock(IntroMixin, ArticleBlock): + by_line = blocks.CharBlock() block = ArticleWithIntroBlock() - self.assertEqual(list(block.child_blocks.keys()), ['heading', 'paragraph', 'intro']) + self.assertEqual(list(block.child_blocks.keys()), + ['heading', 'paragraph', 'intro', 'by_line']) def render_article(self, data): class ArticleBlock(blocks.StreamBlock):