diff --git a/CHANGELOG.txt b/CHANGELOG.txt index 03a41fffb..119f92f7f 100644 --- a/CHANGELOG.txt +++ b/CHANGELOG.txt @@ -18,6 +18,7 @@ Changelog * Updated Cloudflare cache module to use the v4 API (Albert O'Connor) * Added `exclude_from_explorer` attribute to the `ModelAdmin` class to allow hiding instances of a page type from Wagtail's explorer views (Andy Babic) * Added `above_login`, `below_login`, `fields` and `login_form` customisation blocks to the login page template (Tim Heap) + * `ChoiceBlock` now accepts a callable as the choices list (Mikalai Radchuk) * Fix: `AbstractForm` now respects custom `get_template` methods on the page model (Gagaro) * Fix: Use specific page model for the parent page in the explore index (Gagaro) * Fix: Remove responsive styles in embed when there is no ratio available (Gagaro) diff --git a/docs/topics/streamfield.rst b/docs/topics/streamfield.rst index db8266005..a95abf361 100644 --- a/docs/topics/streamfield.rst +++ b/docs/topics/streamfield.rst @@ -198,7 +198,7 @@ ChoiceBlock A dropdown select box for choosing from a list of choices. The following keyword arguments are accepted: ``choices`` - A list of choices, in any format accepted by Django's ``choices`` parameter for model fields: https://docs.djangoproject.com/en/stable/ref/models/fields/#field-choices + A list of choices, in any format accepted by Django's ``choices`` parameter for model fields (https://docs.djangoproject.com/en/stable/ref/models/fields/#field-choices), or a callable returning such a list. ``required`` (default: True) If true, the field cannot be left blank. @@ -230,7 +230,7 @@ could be rewritten as a subclass of ChoiceBlock: icon = 'cup' -``StreamField`` definitions can then refer to ``DrinksChoiceBlock()`` in place of the full ``ChoiceBlock`` definition. +``StreamField`` definitions can then refer to ``DrinksChoiceBlock()`` in place of the full ``ChoiceBlock`` definition. Note that this only works when ``choices`` is a fixed list, not a callable. PageChooserBlock ~~~~~~~~~~~~~~~~ diff --git a/wagtail/wagtailcore/blocks/field_block.py b/wagtail/wagtailcore/blocks/field_block.py index 91d48b9a9..82e288a1b 100644 --- a/wagtail/wagtailcore/blocks/field_block.py +++ b/wagtail/wagtailcore/blocks/field_block.py @@ -4,6 +4,7 @@ import datetime from django import forms from django.db.models.fields import BLANK_CHOICE_DASH +from django.forms.fields import CallableChoiceIterator from django.template.loader import render_to_string from django.utils import six from django.utils.dateparse import parse_date, parse_datetime, parse_time @@ -322,41 +323,70 @@ class ChoiceBlock(FieldBlock): def __init__(self, choices=None, required=True, help_text=None, **kwargs): if choices is None: - # no choices specified, so pick up the choice list defined at the class level - choices = list(self.choices) + # no choices specified, so pick up the choice defined at the class level + choices = self.choices + + if callable(choices): + # Support of callable choices. Wrap the callable in an iterator so that we can + # handle this consistently with ordinary choice lists; + # however, the `choices` constructor kwarg as reported by deconstruct() should + # remain as the callable + choices_for_constructor = choices + choices = CallableChoiceIterator(choices) else: - choices = list(choices) + # Cast as a list + choices_for_constructor = choices = list(choices) # keep a copy of all kwargs (including our normalised choices list) for deconstruct() self._constructor_kwargs = kwargs.copy() - self._constructor_kwargs['choices'] = choices + self._constructor_kwargs['choices'] = choices_for_constructor if required is not True: self._constructor_kwargs['required'] = required if help_text is not None: self._constructor_kwargs['help_text'] = help_text - # If choices does not already contain a blank option, insert one - # (to match Django's own behaviour for modelfields: - # https://github.com/django/django/blob/1.7.5/django/db/models/fields/__init__.py#L732-744) - has_blank_choice = False - for v1, v2 in choices: - if isinstance(v2, (list, tuple)): - # this is a named group, and v2 is the value list - has_blank_choice = any([value in ('', None) for value, label in v2]) - if has_blank_choice: - break - else: - # this is an individual choice; v1 is the value - if v1 in ('', None): - has_blank_choice = True - break - - if not has_blank_choice: - choices = BLANK_CHOICE_DASH + choices - - self.field = forms.ChoiceField(choices=choices, required=required, help_text=help_text) + # We will need to modify the choices list to insert a blank option, if there isn't + # one already. We have to do this at render time in the case of callable choices - so rather + # than having separate code paths for static vs dynamic lists, we'll _always_ pass a callable + # to ChoiceField to perform this step at render time. + callable_choices = self.get_callable_choices(choices) + self.field = forms.ChoiceField(choices=callable_choices, required=required, help_text=help_text) super(ChoiceBlock, self).__init__(**kwargs) + def get_callable_choices(self, choices): + """ + Return a callable that we can pass into `forms.ChoiceField`, which will provide the + choices list with the addition of a blank choice (if one does not already exist). + """ + def choices_callable(): + # Variable choices could be an instance of CallableChoiceIterator, which may be wrapping + # something we don't want to evaluate multiple times (e.g. a database query). Cast as a + # list now to prevent it getting evaluated twice (once while searching for a blank choice, + # once while rendering the final ChoiceField). + local_choices = list(choices) + + # If choices does not already contain a blank option, insert one + # (to match Django's own behaviour for modelfields: + # https://github.com/django/django/blob/1.7.5/django/db/models/fields/__init__.py#L732-744) + has_blank_choice = False + for v1, v2 in local_choices: + if isinstance(v2, (list, tuple)): + # this is a named group, and v2 is the value list + has_blank_choice = any([value in ('', None) for value, label in v2]) + if has_blank_choice: + break + else: + # this is an individual choice; v1 is the value + if v1 in ('', None): + has_blank_choice = True + break + + if not has_blank_choice: + return BLANK_CHOICE_DASH + local_choices + + return local_choices + return choices_callable + def deconstruct(self): """ Always deconstruct ChoiceBlock instances as if they were plain ChoiceBlocks with their diff --git a/wagtail/wagtailcore/tests/test_blocks.py b/wagtail/wagtailcore/tests/test_blocks.py index 1e5095fac..de2a20ebe 100644 --- a/wagtail/wagtailcore/tests/test_blocks.py +++ b/wagtail/wagtailcore/tests/test_blocks.py @@ -418,6 +418,19 @@ class TestChoiceBlock(unittest.TestCase): self.assertIn('', html) self.assertIn('', html) + def test_render_required_choice_block_with_callable_choices(self): + def callable_choices(): + return [('tea', 'Tea'), ('coffee', 'Coffee')] + + block = blocks.ChoiceBlock(choices=callable_choices) + html = block.render_form('coffee', prefix='beverage') + self.assertIn('', html) + self.assertIn('' % self.blank_choice_dash_label, html) + self.assertIn('', html) + self.assertIn('', html) + def test_validate_non_required_choice_block(self): block = blocks.ChoiceBlock(choices=[('tea', 'Tea'), ('coffee', 'Coffee')], required=False) self.assertEqual(block.clean('coffee'), 'coffee') @@ -460,6 +484,20 @@ class TestChoiceBlock(unittest.TestCase): self.assertIn('', html) self.assertIn('', html) + def test_render_choice_block_with_existing_blank_choice_and_with_callable_choices(self): + def callable_choices(): + return [('tea', 'Tea'), ('coffee', 'Coffee'), ('', 'No thanks')] + + block = blocks.ChoiceBlock( + choices=callable_choices, + required=False) + html = block.render_form(None, prefix='beverage') + self.assertIn('', html) + self.assertIn('', html) + + self.assertEqual( + block.deconstruct(), + ( + 'wagtail.wagtailcore.blocks.ChoiceBlock', + [], + { + 'choices': callable_choices, + 'required': False, + }, + ) + ) + class TestRawHTMLBlock(unittest.TestCase): def test_get_default_with_fallback_value(self):