From aaecf038cae61f114db396f74e06759c95f21e93 Mon Sep 17 00:00:00 2001 From: Josef Rousek Date: Sat, 5 Nov 2016 12:27:44 +0100 Subject: [PATCH] Fixed #27370 -- Prevented Select widget from using 'required' with a non-empty first value. --- AUTHORS | 1 + django/forms/widgets.py | 22 ++++++++++ .../field_tests/test_choicefield.py | 2 +- tests/forms_tests/tests/test_forms.py | 40 +++++++++---------- tests/forms_tests/tests/tests.py | 8 ++-- tests/forms_tests/widget_tests/test_select.py | 20 ++++++++++ tests/model_forms/tests.py | 4 +- 7 files changed, 70 insertions(+), 27 deletions(-) diff --git a/AUTHORS b/AUTHORS index ff17d1a418..82938eea88 100644 --- a/AUTHORS +++ b/AUTHORS @@ -397,6 +397,7 @@ answer newbie questions, and generally made Django that much better: Jorge Bastida Jorge Gajon José Tomás Tocino García + Josef Rousek Joseph Kocherhans Josh Smeaton Joshua Ginsberg diff --git a/django/forms/widgets.py b/django/forms/widgets.py index dd68662d43..4f028d34ad 100644 --- a/django/forms/widgets.py +++ b/django/forms/widgets.py @@ -680,6 +680,28 @@ class Select(ChoiceWidget): context['widget']['attrs']['multiple'] = 'multiple' return context + @staticmethod + def _choice_has_empty_value(choice): + """Return True if the choice's value is empty string or None.""" + value, _ = choice + return ( + (isinstance(value, six.string_types) and not bool(value)) or + value is None + ) + + def use_required_attribute(self, initial): + """ + Don't render 'required' if the first ' + '' ) diff --git a/tests/forms_tests/tests/test_forms.py b/tests/forms_tests/tests/test_forms.py index 4079be1235..2b93575bf2 100644 --- a/tests/forms_tests/tests/test_forms.py +++ b/tests/forms_tests/tests/test_forms.py @@ -502,12 +502,12 @@ class FormsTestCase(SimpleTestCase): language = ChoiceField(choices=[('P', 'Python'), ('J', 'Java')]) f = FrameworkForm(auto_id=False) - self.assertHTMLEqual(str(f['language']), """ """) f = FrameworkForm({'name': 'Django', 'language': 'P'}, auto_id=False) - self.assertHTMLEqual(str(f['language']), """ """) @@ -531,12 +531,12 @@ class FormsTestCase(SimpleTestCase): language = ChoiceField(choices=[('P', 'Python'), ('J', 'Java')], widget=Select(attrs={'class': 'foo'})) f = FrameworkForm(auto_id=False) - self.assertHTMLEqual(str(f['language']), """ """) f = FrameworkForm({'name': 'Django', 'language': 'P'}, auto_id=False) - self.assertHTMLEqual(str(f['language']), """ """) @@ -552,12 +552,12 @@ class FormsTestCase(SimpleTestCase): ) f = FrameworkForm(auto_id=False) - self.assertHTMLEqual(str(f['language']), """ """) f = FrameworkForm({'name': 'Django', 'language': 'P'}, auto_id=False) - self.assertHTMLEqual(str(f['language']), """ """) @@ -568,10 +568,10 @@ class FormsTestCase(SimpleTestCase): language = ChoiceField() f = FrameworkForm(auto_id=False) - self.assertHTMLEqual(str(f['language']), """ """) f.fields['language'].choices = [('P', 'Python'), ('J', 'Java')] - self.assertHTMLEqual(str(f['language']), """ """) @@ -2363,37 +2363,37 @@ Password: is_cool = NullBooleanField() p = Person({'name': 'Joe'}, auto_id=False) - self.assertHTMLEqual(str(p['is_cool']), """ """) p = Person({'name': 'Joe', 'is_cool': '1'}, auto_id=False) - self.assertHTMLEqual(str(p['is_cool']), """ """) p = Person({'name': 'Joe', 'is_cool': '2'}, auto_id=False) - self.assertHTMLEqual(str(p['is_cool']), """ """) p = Person({'name': 'Joe', 'is_cool': '3'}, auto_id=False) - self.assertHTMLEqual(str(p['is_cool']), """ """) p = Person({'name': 'Joe', 'is_cool': True}, auto_id=False) - self.assertHTMLEqual(str(p['is_cool']), """ """) p = Person({'name': 'Joe', 'is_cool': False}, auto_id=False) - self.assertHTMLEqual(str(p['is_cool']), """ @@ -2759,7 +2759,7 @@ Good luck picking a username that doesn't already exist.

"""
    • This field is required.
  • - @@ -2775,7 +2775,7 @@ Good luck picking a username that doesn't already exist.

    - @@ -2793,7 +2793,7 @@ Good luck picking a username that doesn't already exist.

    • This field is required.
    - @@ -3516,7 +3516,7 @@ Good luck picking a username that doesn't already exist.

    '

    ' '

    ' - '

    ' '' '' '

    ', @@ -3528,7 +3528,7 @@ Good luck picking a username that doesn't already exist.

    '
  • ' '
  • ' - '
  • ' '' '' '
  • ', @@ -3542,7 +3542,7 @@ Good luck picking a username that doesn't already exist.

    '' '' - '' '' '' '', diff --git a/tests/forms_tests/tests/tests.py b/tests/forms_tests/tests/tests.py index c19d1a66cd..f0007fbe8f 100644 --- a/tests/forms_tests/tests/tests.py +++ b/tests/forms_tests/tests/tests.py @@ -109,12 +109,12 @@ class ModelFormCallableModelDefault(TestCase): ChoiceOptionModel.objects.create(id=3, name='option 3') self.assertHTMLEqual( ChoiceFieldForm().as_p(), - """

    -

    @@ -145,12 +145,12 @@ class ModelFormCallableModelDefault(TestCase): 'multi_choice': [obj2, obj3], 'multi_choice_int': ChoiceOptionModel.objects.exclude(name="default"), }).as_p(), - """

    -

    diff --git a/tests/forms_tests/widget_tests/test_select.py b/tests/forms_tests/widget_tests/test_select.py index 4b717b6043..d2660ea787 100644 --- a/tests/forms_tests/widget_tests/test_select.py +++ b/tests/forms_tests/widget_tests/test_select.py @@ -294,3 +294,23 @@ class SelectTest(WidgetTest): self.assertIsNot(widget.choices, obj.choices) self.assertEqual(widget.attrs, obj.attrs) self.assertIsNot(widget.attrs, obj.attrs) + + def test_doesnt_render_required_when_impossible_to_select_empty_field(self): + widget = self.widget(choices=[('J', 'John'), ('P', 'Paul')]) + self.assertIs(widget.use_required_attribute(initial=None), False) + + def test_renders_required_when_possible_to_select_empty_field_str(self): + widget = self.widget(choices=[('', 'select please'), ('P', 'Paul')]) + self.assertIs(widget.use_required_attribute(initial=None), True) + + def test_renders_required_when_possible_to_select_empty_field_list(self): + widget = self.widget(choices=[['', 'select please'], ['P', 'Paul']]) + self.assertIs(widget.use_required_attribute(initial=None), True) + + def test_renders_required_when_possible_to_select_empty_field_none(self): + widget = self.widget(choices=[(None, 'select please'), ('P', 'Paul')]) + self.assertIs(widget.use_required_attribute(initial=None), True) + + def test_doesnt_render_required_when_no_choices_are_available(self): + widget = self.widget(choices=[]) + self.assertIs(widget.use_required_attribute(initial=None), False) diff --git a/tests/model_forms/tests.py b/tests/model_forms/tests.py index f03b4433a8..1686629c3a 100644 --- a/tests/model_forms/tests.py +++ b/tests/model_forms/tests.py @@ -2635,11 +2635,11 @@ class OtherModelFormTests(TestCase):

    -

    -