From 1123f4551158b7fc65d3bd88c375a4517dcd0720 Mon Sep 17 00:00:00 2001 From: Alex Couper Date: Sat, 20 Jul 2013 21:49:33 +0000 Subject: [PATCH] Fixed #20649 -- Allowed blank field display to be defined in the initial list of choices. --- AUTHORS | 1 + django/db/models/fields/__init__.py | 9 ++- django/forms/widgets.py | 2 + docs/ref/models/fields.txt | 19 ++++-- docs/releases/1.7.txt | 5 ++ tests/forms_tests/models.py | 23 +++++++ tests/forms_tests/tests/tests.py | 97 ++++++++++++++++++++++++++++- tests/model_fields/tests.py | 4 ++ 8 files changed, 152 insertions(+), 8 deletions(-) diff --git a/AUTHORS b/AUTHORS index a7f12d6e48..ade95df06a 100644 --- a/AUTHORS +++ b/AUTHORS @@ -161,6 +161,7 @@ answer newbie questions, and generally made Django that much better: Paul Collier Paul Collins Robert Coup + Alex Couper Deric Crago Brian Fabian Crain David Cramer diff --git a/django/db/models/fields/__init__.py b/django/db/models/fields/__init__.py index d0a2defc48..fb4cbbb11a 100644 --- a/django/db/models/fields/__init__.py +++ b/django/db/models/fields/__init__.py @@ -544,7 +544,14 @@ class Field(object): def get_choices(self, include_blank=True, blank_choice=BLANK_CHOICE_DASH): """Returns choices with a default blank choices included, for use as SelectField choices for this field.""" - first_choice = blank_choice if include_blank else [] + blank_defined = False + for choice, _ in self.choices: + if choice in ('', None): + blank_defined = True + break + + first_choice = (blank_choice if include_blank and + not blank_defined else []) if self.choices: return first_choice + list(self.choices) rel_model = self.rel.to diff --git a/django/forms/widgets.py b/django/forms/widgets.py index 8a351bb637..784ccda159 100644 --- a/django/forms/widgets.py +++ b/django/forms/widgets.py @@ -511,6 +511,8 @@ class Select(Widget): return mark_safe('\n'.join(output)) def render_option(self, selected_choices, option_value, option_label): + if option_value == None: + option_value = '' option_value = force_text(option_value) if option_value in selected_choices: selected_html = mark_safe(' selected="selected"') diff --git a/docs/ref/models/fields.txt b/docs/ref/models/fields.txt index 869996643f..736a836924 100644 --- a/docs/ref/models/fields.txt +++ b/docs/ref/models/fields.txt @@ -152,11 +152,20 @@ method to retrieve the human-readable name for the field's current value. See :meth:`~django.db.models.Model.get_FOO_display` in the database API documentation. -Finally, note that choices can be any iterable object -- not necessarily a list -or tuple. This lets you construct choices dynamically. But if you find yourself -hacking :attr:`~Field.choices` to be dynamic, you're probably better off using a -proper database table with a :class:`ForeignKey`. :attr:`~Field.choices` is -meant for static data that doesn't change much, if ever. +Note that choices can be any iterable object -- not necessarily a list or tuple. +This lets you construct choices dynamically. But if you find yourself hacking +:attr:`~Field.choices` to be dynamic, you're probably better off using a proper +database table with a :class:`ForeignKey`. :attr:`~Field.choices` is meant for +static data that doesn't change much, if ever. + +.. versionadded:: 1.7 + +Unless :attr:`blank=False` is set on the field along with a +:attr:`~Field.default` then a label containing ``"---------"`` will be rendered +with the select box. To override this behavior, add a tuple to ``choices`` +containing ``None``; e.g. ``(None, 'Your String For Display')``. +Alternatively, you can use an empty string instead of ``None`` where this makes +sense - such as on a :class:`~django.db.models.CharField`. ``db_column`` ------------- diff --git a/docs/releases/1.7.txt b/docs/releases/1.7.txt index b75fd89f4c..c47a392dff 100644 --- a/docs/releases/1.7.txt +++ b/docs/releases/1.7.txt @@ -105,6 +105,11 @@ Minor features ` method to more easily customize the login policy. +* :attr:`Field.choices` now allows you to + customize the "empty choice" label by including a tuple with an empty string + or ``None`` for the key and the custom label as the value. The default blank + option ``"----------"`` will be omitted in this case. + Backwards incompatible changes in 1.7 ===================================== diff --git a/tests/forms_tests/models.py b/tests/forms_tests/models.py index bec31d12d7..33898ffbb8 100644 --- a/tests/forms_tests/models.py +++ b/tests/forms_tests/models.py @@ -34,7 +34,30 @@ class Defaults(models.Model): class ChoiceModel(models.Model): """For ModelChoiceField and ModelMultipleChoiceField tests.""" + CHOICES = [ + ('', 'No Preference'), + ('f', 'Foo'), + ('b', 'Bar'), + ] + + INTEGER_CHOICES = [ + (None, 'No Preference'), + (1, 'Foo'), + (2, 'Bar'), + ] + + STRING_CHOICES_WITH_NONE = [ + (None, 'No Preference'), + ('f', 'Foo'), + ('b', 'Bar'), + ] + name = models.CharField(max_length=10) + choice = models.CharField(max_length=2, blank=True, choices=CHOICES) + choice_string_w_none = models.CharField( + max_length=2, blank=True, null=True, choices=STRING_CHOICES_WITH_NONE) + choice_integer = models.IntegerField(choices=INTEGER_CHOICES, blank=True, + null=True) @python_2_unicode_compatible diff --git a/tests/forms_tests/tests/tests.py b/tests/forms_tests/tests/tests.py index 4c391646e7..618ab8e07c 100644 --- a/tests/forms_tests/tests/tests.py +++ b/tests/forms_tests/tests/tests.py @@ -10,8 +10,8 @@ from django.forms.models import ModelFormMetaclass from django.test import TestCase from django.utils import six -from ..models import (ChoiceOptionModel, ChoiceFieldModel, FileModel, Group, - BoundaryModel, Defaults, OptionalMultiChoiceModel) +from ..models import (ChoiceModel, ChoiceOptionModel, ChoiceFieldModel, + FileModel, Group, BoundaryModel, Defaults, OptionalMultiChoiceModel) class ChoiceFieldForm(ModelForm): @@ -34,6 +34,24 @@ class ChoiceFieldExclusionForm(ModelForm): model = ChoiceFieldModel +class EmptyCharLabelChoiceForm(ModelForm): + class Meta: + model = ChoiceModel + fields = ['name', 'choice'] + + +class EmptyIntegerLabelChoiceForm(ModelForm): + class Meta: + model = ChoiceModel + fields = ['name', 'choice_integer'] + + +class EmptyCharLabelNoneChoiceForm(ModelForm): + class Meta: + model = ChoiceModel + fields = ['name', 'choice_string_w_none'] + + class FileForm(Form): file1 = FileField() @@ -259,3 +277,78 @@ class ManyToManyExclusionTestCase(TestCase): self.assertEqual(form.instance.choice_int.pk, data['choice_int']) self.assertEqual(list(form.instance.multi_choice.all()), [opt2, opt3]) self.assertEqual([obj.pk for obj in form.instance.multi_choice_int.all()], data['multi_choice_int']) + + +class EmptyLabelTestCase(TestCase): + def test_empty_field_char(self): + f = EmptyCharLabelChoiceForm() + self.assertHTMLEqual(f.as_p(), + """

+

""") + + def test_empty_field_char_none(self): + f = EmptyCharLabelNoneChoiceForm() + self.assertHTMLEqual(f.as_p(), + """

+

""") + + def test_save_empty_label_forms(self): + # Test that saving a form with a blank choice results in the expected + # value being stored in the database. + tests = [ + (EmptyCharLabelNoneChoiceForm, 'choice_string_w_none', None), + (EmptyIntegerLabelChoiceForm, 'choice_integer', None), + (EmptyCharLabelChoiceForm, 'choice', ''), + ] + + for form, key, expected in tests: + f = form({'name': 'some-key', key: ''}) + self.assertTrue(f.is_valid()) + m = f.save() + self.assertEqual(expected, getattr(m, key)) + self.assertEqual('No Preference', + getattr(m, 'get_{0}_display'.format(key))()) + + def test_empty_field_integer(self): + f = EmptyIntegerLabelChoiceForm() + self.assertHTMLEqual(f.as_p(), + """

+

""") + + def test_get_display_value_on_none(self): + m = ChoiceModel.objects.create(name='test', choice='', choice_integer=None) + self.assertEqual(None, m.choice_integer) + self.assertEqual('No Preference', m.get_choice_integer_display()) + + def test_html_rendering_of_prepopulated_models(self): + none_model = ChoiceModel(name='none-test', choice_integer=None) + f = EmptyIntegerLabelChoiceForm(instance=none_model) + self.assertHTMLEqual(f.as_p(), + """

+

""") + + foo_model = ChoiceModel(name='foo-test', choice_integer=1) + f = EmptyIntegerLabelChoiceForm(instance=foo_model) + self.assertHTMLEqual(f.as_p(), + """

+

""") diff --git a/tests/model_fields/tests.py b/tests/model_fields/tests.py index 04a17df947..378e6280cc 100644 --- a/tests/model_fields/tests.py +++ b/tests/model_fields/tests.py @@ -331,6 +331,10 @@ class ValidationTest(test.TestCase): f = models.CharField(choices=[('a', 'A'), ('b', 'B')]) self.assertRaises(ValidationError, f.clean, "not a", None) + def test_charfield_get_choices_with_blank_defined(self): + f = models.CharField(choices=[('', '<><>'), ('a', 'A')]) + self.assertEqual(f.get_choices(True), [('', '<><>'), ('a', 'A')]) + def test_choices_validation_supports_named_groups(self): f = models.IntegerField( choices=(('group', ((10, 'A'), (20, 'B'))), (30, 'C')))