From 4bc6b939944183533ae74791d21282e613f63a96 Mon Sep 17 00:00:00 2001 From: Tim Graham Date: Thu, 11 Aug 2016 15:18:48 -0400 Subject: [PATCH] Fixed #27039 -- Fixed empty data fallback to model field default in model forms. --- django/forms/models.py | 5 ++++ django/forms/widgets.py | 4 +++ docs/releases/1.10.1.txt | 3 +++ docs/topics/forms/modelforms.txt | 15 +++++++++++ tests/model_forms/models.py | 1 + tests/model_forms/tests.py | 43 ++++++++++++++++++++++++++++++-- 6 files changed, 69 insertions(+), 2 deletions(-) diff --git a/django/forms/models.py b/django/forms/models.py index 7cb2159b1d..628d96b237 100644 --- a/django/forms/models.py +++ b/django/forms/models.py @@ -52,6 +52,11 @@ def construct_instance(form, instance, fields=None, exclude=None): continue if exclude and f.name in exclude: continue + # Leave defaults for fields that aren't in POST data, except for + # checkbox inputs because they don't appear in POST data if not checked. + if (f.has_default() and f.name not in form.data and + not getattr(form[f.name].field.widget, 'dont_use_model_field_default_for_empty_data', False)): + continue # Defer saving file-type fields until after the other fields, so a # callable upload_to can use the values from other fields. if isinstance(f, models.FileField): diff --git a/django/forms/widgets.py b/django/forms/widgets.py index d6b84c10ab..678b2eac7e 100644 --- a/django/forms/widgets.py +++ b/django/forms/widgets.py @@ -481,6 +481,10 @@ def boolean_check(v): class CheckboxInput(Widget): + # Don't use model field defaults for fields that aren't in POST data, + # because checkboxes don't appear in POST data if not checked. + dont_use_model_field_default_for_empty_data = True + def __init__(self, attrs=None, check_test=None): super(CheckboxInput, self).__init__(attrs) # check_test is a callable that takes a value and returns True diff --git a/docs/releases/1.10.1.txt b/docs/releases/1.10.1.txt index 2fd860527a..719f536b29 100644 --- a/docs/releases/1.10.1.txt +++ b/docs/releases/1.10.1.txt @@ -79,3 +79,6 @@ Bugfixes * Reallowed subclassing ``UserCreationForm`` without ``USERNAME_FIELD`` in ``Meta.fields`` (:ticket:`27111`). + +* Fixed a regression in model forms where model fields with a ``default`` that + didn't appear in POST data no longer used the ``default`` (:ticket:`27039`). diff --git a/docs/topics/forms/modelforms.txt b/docs/topics/forms/modelforms.txt index 9dcb2e3e53..4b5d7f27ce 100644 --- a/docs/topics/forms/modelforms.txt +++ b/docs/topics/forms/modelforms.txt @@ -332,6 +332,21 @@ Note that if the form :ref:`hasn't been validated ``form.errors``. A ``ValueError`` will be raised if the data in the form doesn't validate -- i.e., if ``form.errors`` evaluates to ``True``. +If an optional field doesn't appear in the form's data, the resulting model +instance uses the model field :attr:`~django.db.models.Field.default`, if +there is one, for that field. This behavior doesn't apply to fields that use +:class:`~django.forms.CheckboxInput` (or any custom widget with +``dont_use_model_field_default_for_empty_data=True``) since an unchecked +checkbox doesn't appear in the data of an HTML form submission. Use a custom +form field or widget if you're designing an API and want the default fallback +for a ``BooleanField``. + +.. versionchanged:: 1.10.1 + + Older versions don't have the exception for + :class:`~django.forms.CheckboxInput` which means that unchecked checkboxes + receive a value of ``True`` if that's the model field default. + This ``save()`` method accepts an optional ``commit`` keyword argument, which accepts either ``True`` or ``False``. If you call ``save()`` with ``commit=False``, then it will return an object that hasn't yet been saved to diff --git a/tests/model_forms/models.py b/tests/model_forms/models.py index ec39e5ad1f..b00e6a2658 100644 --- a/tests/model_forms/models.py +++ b/tests/model_forms/models.py @@ -122,6 +122,7 @@ class PublicationDefaults(models.Model): date_published = models.DateField(default=datetime.date.today) mode = models.CharField(max_length=2, choices=MODE_CHOICES, default=default_mode) category = models.IntegerField(choices=CATEGORY_CHOICES, default=default_category) + active = models.BooleanField(default=True) class Author(models.Model): diff --git a/tests/model_forms/tests.py b/tests/model_forms/tests.py index 085a95733e..5d8fe4bd91 100644 --- a/tests/model_forms/tests.py +++ b/tests/model_forms/tests.py @@ -565,6 +565,42 @@ class ModelFormBaseTest(TestCase): self.assertEqual(list(OrderFields2.base_fields), ['slug', 'name']) + def test_default_populated_on_optional_field(self): + class PubForm(forms.ModelForm): + mode = forms.CharField(max_length=255, required=False) + + class Meta: + model = PublicationDefaults + fields = ('mode',) + + # Empty data uses the model field default. + mf1 = PubForm({}) + self.assertEqual(mf1.errors, {}) + m1 = mf1.save(commit=False) + self.assertEqual(m1.mode, 'di') + self.assertEqual(m1._meta.get_field('mode').get_default(), 'di') + + # Blank data doesn't use the model field default. + mf2 = PubForm({'mode': ''}) + self.assertEqual(mf2.errors, {}) + m2 = mf2.save(commit=False) + self.assertEqual(m2.mode, '') + + def test_default_not_populated_on_optional_checkbox_input(self): + class PubForm(forms.ModelForm): + class Meta: + model = PublicationDefaults + fields = ('active',) + + # Empty data doesn't use the model default because CheckboxInput + # doesn't have a value in HTML form submission. + mf1 = PubForm({}) + self.assertEqual(mf1.errors, {}) + m1 = mf1.save(commit=False) + self.assertIs(m1.active, False) + self.assertIsInstance(mf1.fields['active'].widget, forms.CheckboxInput) + self.assertIs(m1._meta.get_field('active').get_default(), True) + class FieldOverridesByFormMetaForm(forms.ModelForm): class Meta: @@ -774,7 +810,10 @@ class UniqueTest(TestCase): title = 'Boss' isbn = '12345' DerivedBook.objects.create(title=title, author=self.writer, isbn=isbn) - form = DerivedBookForm({'title': 'Other', 'author': self.writer.pk, 'isbn': isbn}) + form = DerivedBookForm({ + 'title': 'Other', 'author': self.writer.pk, 'isbn': isbn, + 'suffix1': '1', 'suffix2': '2', + }) self.assertFalse(form.is_valid()) self.assertEqual(len(form.errors), 1) self.assertEqual(form.errors['isbn'], ['Derived book with this Isbn already exists.']) @@ -2491,7 +2530,7 @@ class OtherModelFormTests(TestCase): class PublicationDefaultsForm(forms.ModelForm): class Meta: model = PublicationDefaults - fields = '__all__' + fields = ('title', 'date_published', 'mode', 'category') self.maxDiff = 2000 form = PublicationDefaultsForm()