From 375e1cfe2b2e1c3c57f882147c34902c6e8189ac Mon Sep 17 00:00:00 2001 From: haxoza Date: Sat, 14 Nov 2015 17:38:27 +0100 Subject: [PATCH] Fixed #25349 -- Allowed a ModelForm to unset a fields with blank=True, required=False. --- django/forms/models.py | 10 +++++----- tests/model_forms/models.py | 6 ++++++ tests/model_forms/tests.py | 36 +++++++++++++++++++++++++++++++++++- 3 files changed, 46 insertions(+), 6 deletions(-) diff --git a/django/forms/models.py b/django/forms/models.py index 8c0f2e97a6..8e8c8cfb55 100644 --- a/django/forms/models.py +++ b/django/forms/models.py @@ -376,11 +376,6 @@ class BaseModelForm(BaseForm): exclude = self._get_validation_exclusions() - try: - self.instance = construct_instance(self, self.instance, opts.fields, exclude) - except ValidationError as e: - self._update_errors(e) - # Foreign Keys being used to represent inline relationships # are excluded from basic field value validation. This is for two # reasons: firstly, the value may not be supplied (#12507; the @@ -392,6 +387,11 @@ class BaseModelForm(BaseForm): if isinstance(field, InlineForeignKeyField): exclude.append(name) + try: + self.instance = construct_instance(self, self.instance, opts.fields, opts.exclude) + except ValidationError as e: + self._update_errors(e) + 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 d9c08ae9c0..4688720ac0 100644 --- a/tests/model_forms/models.py +++ b/tests/model_forms/models.py @@ -474,3 +474,9 @@ class StrictAssignmentAll(models.Model): if self._should_error is True: raise ValidationError(message="Cannot set attribute", code='invalid') super(StrictAssignmentAll, self).__setattr__(key, value) + + +# A model with ForeignKey(blank=False, null=True) +class Award(models.Model): + name = models.CharField(max_length=30) + character = models.ForeignKey(Character, models.SET_NULL, blank=False, null=True) diff --git a/tests/model_forms/tests.py b/tests/model_forms/tests.py index a1adeec445..968407e845 100644 --- a/tests/model_forms/tests.py +++ b/tests/model_forms/tests.py @@ -23,7 +23,7 @@ from django.utils import six from django.utils._os import upath from .models import ( - Article, ArticleStatus, Author, Author1, BetterWriter, BigInt, Book, + Article, ArticleStatus, Author, Author1, Award, BetterWriter, BigInt, Book, Category, Character, Colour, ColourfulItem, CommaSeparatedInteger, CustomErrorMessage, CustomFF, CustomFieldForExclusionModel, DateTimePost, DerivedBook, DerivedPost, Document, ExplicitPK, FilePathModel, @@ -236,6 +236,40 @@ class ModelFormBaseTest(TestCase): obj = f2.save() self.assertEqual(obj.character, char) + def test_blank_false_with_null_true_foreign_key_field(self): + """ + A ModelForm with a model having ForeignKey(blank=False, null=True) + and the form field set to required=False should allow the field to be + unset. + """ + class AwardForm(forms.ModelForm): + class Meta: + model = Award + fields = '__all__' + + def __init__(self, *args, **kwargs): + super(AwardForm, self).__init__(*args, **kwargs) + self.fields['character'].required = False + + character = Character.objects.create(username='user', last_action=datetime.datetime.today()) + award = Award.objects.create(name='Best sprinter', character=character) + data = {'name': 'Best tester', 'character': ''} # remove character + form = AwardForm(data=data, instance=award) + self.assertTrue(form.is_valid()) + award = form.save() + self.assertIsNone(award.character) + + def test_save_blank_false_with_required_false(self): + """ + A ModelForm with a model with a field set to blank=False and the form + field set to required=False should allow the field to be unset. + """ + obj = Writer.objects.create(name='test') + form = CustomWriterForm(data={'name': ''}, instance=obj) + self.assertTrue(form.is_valid()) + obj = form.save() + self.assertEqual(obj.name, '') + def test_missing_fields_attribute(self): message = ( "Creating a ModelForm without either the 'fields' attribute "