From 45e049937d2564d11035827ca9a9221b86215e45 Mon Sep 17 00:00:00 2001 From: Anubhav Joshi Date: Tue, 3 Jun 2014 09:06:59 +0530 Subject: [PATCH] Fixed #13776 -- Fixed ModelForm.is_valid() exception with non-nullable FK and blank=True. Thanks peterbe for the report. --- django/forms/models.py | 11 +++++++++-- tests/model_forms/models.py | 6 ++++++ tests/model_forms/tests.py | 30 +++++++++++++++++++++++++++++- 3 files changed, 44 insertions(+), 3 deletions(-) diff --git a/django/forms/models.py b/django/forms/models.py index 263d3d4340b..055af3cf777 100644 --- a/django/forms/models.py +++ b/django/forms/models.py @@ -401,10 +401,12 @@ class BaseModelForm(BaseForm): def _post_clean(self): opts = self._meta - # Update the model instance with self.cleaned_data. - self.instance = construct_instance(self, self.instance, opts.fields, opts.exclude) exclude = self._get_validation_exclusions() + # a subset of `exclude` which won't have the InlineForeignKeyField + # if we're adding a new object since that value doesn't exist + # until after the new instance is saved to the database. + construct_instance_exclude = list(exclude) # Foreign Keys being used to represent inline relationships # are excluded from basic field value validation. This is for two @@ -415,8 +417,13 @@ class BaseModelForm(BaseForm): # so this can't be part of _get_validation_exclusions(). for name, field in self.fields.items(): if isinstance(field, InlineForeignKeyField): + if self.cleaned_data.get(name) is not None and self.cleaned_data[name]._state.adding: + construct_instance_exclude.append(name) exclude.append(name) + # Update the model instance with self.cleaned_data. + self.instance = construct_instance(self, self.instance, opts.fields, construct_instance_exclude) + try: self.instance.full_clean(exclude=exclude, validate_unique=False) except ValidationError as e: diff --git a/tests/model_forms/models.py b/tests/model_forms/models.py index e71c482eb5e..d327a5f0743 100644 --- a/tests/model_forms/models.py +++ b/tests/model_forms/models.py @@ -401,3 +401,9 @@ class Character(models.Model): class StumpJoke(models.Model): most_recently_fooled = models.ForeignKey(Character, limit_choices_to=today_callable_dict, related_name="+") has_fooled_today = models.ManyToManyField(Character, limit_choices_to=today_callable_q, related_name="+") + + +# Model for #13776 +class Student(models.Model): + character = models.ForeignKey(Character) + study = models.CharField(max_length=30) diff --git a/tests/model_forms/tests.py b/tests/model_forms/tests.py index de96246c6fd..783a0316f77 100644 --- a/tests/model_forms/tests.py +++ b/tests/model_forms/tests.py @@ -23,7 +23,7 @@ from .models import (Article, ArticleStatus, Author, Author1, BetterWriter, BigI ImprovedArticle, ImprovedArticleWithParentLink, Inventory, Person, Post, Price, Product, Publication, TextFile, Triple, Writer, WriterProfile, Colour, ColourfulItem, DateTimePost, CustomErrorMessage, - test_images, StumpJoke, Character) + test_images, StumpJoke, Character, Student) if test_images: from .models import ImageFile, OptionalImageFile @@ -199,6 +199,34 @@ class ModelFormBaseTest(TestCase): instance = construct_instance(form, Person(), fields=()) self.assertEqual(instance.name, '') + def test_blank_with_null_foreign_key_field(self): + """ + #13776 -- ModelForm's with models having a FK set to null=False and + required=False should be valid. + """ + class FormForTestingIsValid(forms.ModelForm): + class Meta: + model = Student + fields = '__all__' + + def __init__(self, *args, **kwargs): + super(FormForTestingIsValid, self).__init__(*args, **kwargs) + self.fields['character'].required = False + + char = Character.objects.create(username='user', + last_action=datetime.datetime.today()) + data = {'study': 'Engineering'} + data2 = {'study': 'Engineering', 'character': char.pk} + + # form is valid because required=False for field 'character' + f1 = FormForTestingIsValid(data) + self.assertTrue(f1.is_valid()) + + f2 = FormForTestingIsValid(data2) + self.assertTrue(f2.is_valid()) + obj = f2.save() + self.assertEqual(obj.character, char) + def test_missing_fields_attribute(self): message = ( "Creating a ModelForm without either the 'fields' attribute "