Skip to content

Commit

Permalink
Fixed #13776 -- Fixed ModelForm.is_valid() exception with non-nullabl…
Browse files Browse the repository at this point in the history
…e FK and blank=True.

Thanks peterbe for the report.
  • Loading branch information
coder9042 authored and timgraham committed Jun 4, 2014
1 parent 7e2c878 commit 45e0499
Show file tree
Hide file tree
Showing 3 changed files with 44 additions and 3 deletions.
11 changes: 9 additions & 2 deletions django/forms/models.py
Expand Up @@ -401,10 +401,12 @@ def _update_errors(self, errors):

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
Expand All @@ -415,8 +417,13 @@ def _post_clean(self):
# 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:
Expand Down
6 changes: 6 additions & 0 deletions tests/model_forms/models.py
Expand Up @@ -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)
30 changes: 29 additions & 1 deletion tests/model_forms/tests.py
Expand Up @@ -23,7 +23,7 @@
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
Expand Down Expand Up @@ -199,6 +199,34 @@ def test_empty_fields_to_construct_instance(self):
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 "
Expand Down

0 comments on commit 45e0499

Please sign in to comment.