Unexpected behavior of ModelForm.save for fields with blank=False and overridden required attribute

[bboogaard]

Since upgrading to Django 1.8.1, I'm getting unexpected behavior when setting the required attribute for ModelForm fields to False for fields with blank=False in their field declaration on the model. Contrary to previous Django versions, the field is now ignored when calling the save() method of the ModelForm.

The model:

from django.db import models


class CodeName(models.Model):

    code = models.CharField('Code', max_length=20)

    name = models.CharField('Name', max_length=100)

The form:

from django import forms

from .models import CodeName


class CodeNameForm(forms.ModelForm):

    class Meta:
        model = CodeName
        fields = ['code', 'name']

    def __init__(self, *args, **kwargs):
        super(CodeNameForm, self).__init__(*args, **kwargs)
        self.fields['code'].required = False

Failing test:

from django.test import TestCase

from .forms import CodeNameForm
from .models import CodeName


class TestCodeNameForm(TestCase):

    def test_save(self):
        instance = CodeName(code='foo', name='Foobar')
        data = {
            'code': '',
            'name': 'Foobar'
        }
        form = CodeNameForm(data, instance=instance)
        form.full_clean()
        instance = form.save()
        self.assertEqual(instance.code, '')

[bboogaard]

Proposed patch (django/forms/models.py)

[Tim Graham]

I am not sure if we should support this use case because with code='', the submitted model is invalid from a model validation standpoint. Could you elaborate on the use case to omit blank=True from the model field definition but accept blank values on the form?

The relevant code in ​forms/models.py.

Your proposed patch doesn't pass the test suite:

======================================================================
ERROR: test_blank_with_null_foreign_key_field (model_forms.tests.ModelFormBaseTest)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/tim/code/django/tests/model_forms/tests.py", line 231, in test_blank_with_null_foreign_key_field
    self.assertTrue(f1.is_valid())
  File "/home/tim/code/django/django/forms/forms.py", line 168, in is_valid
    return self.is_bound and not self.errors
  File "/home/tim/code/django/django/forms/forms.py", line 160, in errors
    self.full_clean()
  File "/home/tim/code/django/django/forms/forms.py", line 378, in full_clean
    self._post_clean()
  File "/home/tim/code/django/django/forms/models.py", line 433, in _post_clean
    self.instance = construct_instance(self, self.instance, opts.fields, construct_instance_exclude)
  File "/home/tim/code/django/django/forms/models.py", line 60, in construct_instance
    f.save_form_data(instance, cleaned_data[f.name])
  File "/home/tim/code/django/django/db/models/fields/__init__.py", line 856, in save_form_data
    setattr(instance, self.name, data)
  File "/home/tim/code/django/django/db/models/fields/related.py", line 634, in __set__
    (instance._meta.object_name, self.field.name)
ValueError: Cannot assign None: "Student.character" does not allow null values.

[bboogaard]

I am not sure if we should support this use case because with code=, the submitted model is invalid from a model validation standpoint.

Do you mean that the pre-1.8 behavior was unintended?

[Tim Graham]

In my mind, yes. There weren't any tests in Django's test suite to claim it was officially supported, at least.

[bboogaard]

All right, if it was an unintended effect of previous versions, I'll simply change my model field declarations and code logic. I thought the override was supposed to work the way I described (i.e. in principle required, but for some form classes not required).