Opened 13 years ago

Closed 12 years ago

#18441 closed Bug (duplicate)

A ModelForm with a blank=True, null=False ForeignKey doesn't validate

Reported by: James Aylett Owned by: nobody
Component: Forms Version: 1.4
Severity: Normal Keywords:
Cc: Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

For the following:

class Collection(models.Model):
    created_by = models.ForeignKey(User, null=False, blank=True)
    # and some other fields

(with a pre_save hook that populates created_by if it isn't already set), if on creation in the admin the HTML input for created_by is left empty (raw id) or unselected (non-raw id ie dropdown of all users), Django will attempt to set the field to None, which will result in a field validation failure:

File "/home/james/projects/artfinder/ENV/lib/python2.6/site-packages/django/core/handlers/base.py" in get_response
  111.                         response = callback(request, *callback_args, **callback_kwargs)
File "/home/james/projects/artfinder/ENV/lib/python2.6/site-packages/django/contrib/admin/options.py" in wrapper
  366.                 return self.admin_site.admin_view(view)(*args, **kwargs)
File "/home/james/projects/artfinder/ENV/lib/python2.6/site-packages/django/utils/decorators.py" in _wrapped_view
  91.                     response = view_func(request, *args, **kwargs)
File "/home/james/projects/artfinder/ENV/lib/python2.6/site-packages/django/views/decorators/cache.py" in _wrapped_view_func
  89.         response = view_func(request, *args, **kwargs)
File "/home/james/projects/artfinder/ENV/lib/python2.6/site-packages/django/contrib/admin/sites.py" in inner
  196.             return view(request, *args, **kwargs)
File "/home/james/projects/artfinder/ENV/lib/python2.6/site-packages/django/utils/decorators.py" in _wrapper
  25.             return bound_func(*args, **kwargs)
File "/home/james/projects/artfinder/ENV/lib/python2.6/site-packages/django/utils/decorators.py" in _wrapped_view
  91.                     response = view_func(request, *args, **kwargs)
File "/home/james/projects/artfinder/ENV/lib/python2.6/site-packages/django/utils/decorators.py" in bound_func
  21.                 return func(self, *args2, **kwargs2)
File "/home/james/projects/artfinder/ENV/lib/python2.6/site-packages/django/db/transaction.py" in inner
  209.                 return func(*args, **kwargs)
File "/home/james/projects/artfinder/ENV/lib/python2.6/site-packages/django/contrib/admin/options.py" in add_view
  937.             if form.is_valid():
File "/home/james/projects/artfinder/ENV/lib/python2.6/site-packages/django/forms/forms.py" in is_valid
  124.         return self.is_bound and not bool(self.errors)
File "/home/james/projects/artfinder/ENV/lib/python2.6/site-packages/django/forms/forms.py" in _get_errors
  115.             self.full_clean()
File "/home/james/projects/artfinder/ENV/lib/python2.6/site-packages/django/forms/forms.py" in full_clean
  272.         self._post_clean()
File "/home/james/projects/artfinder/ENV/lib/python2.6/site-packages/django/forms/models.py" in _post_clean
  309.         self.instance = construct_instance(self, self.instance, opts.fields, opts.exclude)
File "/home/james/projects/artfinder/ENV/lib/python2.6/site-packages/django/forms/models.py" in construct_instance
  51.             f.save_form_data(instance, cleaned_data[f.name])
File "/home/james/projects/artfinder/ENV/lib/python2.6/site-packages/django/db/models/fields/__init__.py" in save_form_data
  454.         setattr(instance, self.name, data)
File "/home/james/projects/artfinder/ENV/lib/python2.6/site-packages/django/db/models/fields/related.py" in __set__
  362.                                 (instance._meta.object_name, self.field.name))

Exception Type: ValueError at /admin/galleries/collection/add/
Exception Value: Cannot assign None: "Collection.created_by" does not allow null values.

I would expect this not to happen, allowing the presave hook to step in and do the right thing. If there's an obvious solution that I just haven't noticed, or no solution, then the rest of this can be skipped.

I believe that the following note in BaseModelForm._get_validation_exclusions() explains what's going on:

Note: don't exclude the field from
validation if the model field allows blanks. If it does, the blank
value may be included in a unique check, so cannot be excluded
from validation.

However I cannot find anything in the documentation that would suggest this behaviour. The current suggested solution, I think, would be to have the field explicitly listed in the ModelForm subclass and in excludes in that form. That would be something like:

class CollectionAdminForm(forms.ModelForm):
        # need to exclude created_by, but explicitly list it as a field
        created_by = # SOMETHING
        class Meta:
                model = Collection
                exclude = ('created_by',)

however there's nothing in the documentation to suggest what the SOMETHING should be (ie what form field type should be used for created_by). If I try to use a ModelChoiceField, it doesn't populate it from the instance we're editing. Worse, because I'm linking to User I really want this to be a raw id field, and that seems to be utterly magical and doesn't have an actual form field type to represent it.

At this point I think I've convinced myself that Django cannot do what I want, ie there's no simple way of configuring the admin to allow a presave-populated ForeignKey to be settable on creation but to fall back to the presave population.

Change History (3)

comment:1 by Aymeric Augustin, 12 years ago

I think you're supposed to use ModelAdmin.save_model rather than a signal (assuming the presave hook you're referring to is the pre_save signal).

comment:2 by Aymeric Augustin, 12 years ago

Component: DocumentationForms
Summary: blank=True, null=False ForeignKey fields populated by presave hooks cannot be edited cleanly in the adminA ModelForm with a blank=True, null=False ForeignKey doesn't validate
Triage Stage: UnreviewedAccepted
Type: UncategorizedBug

My first comment is wrong: you reach the exception before ModelAdmin.save_model gets called.


Now I think that's a real bug of model forms.

If you have a blank-able but not nullable foreign key, you'd expect validation of a model form to succeed even if no value is provided for the foreign key. This isn't true.

With this definition:

from django import forms
from django.db import models

class Name(models.Model):
    name = models.CharField(max_length=32, unique=True)

class BlankFK(models.Model):
    related = models.ForeignKey(Name, blank=True, null=False)

class BlankFKForm(forms.ModelForm):
    class Meta:
        model = BlankFK

I get this error:

>>> BlankFKForm({}).is_valid()
Traceback (most recent call last):
  File "<console>", line 1, in <module>
  File "/Users/myk/Documents/dev/django-trunk/django/forms/forms.py", line 124, in is_valid
    return self.is_bound and not bool(self.errors)
  File "/Users/myk/Documents/dev/django-trunk/django/forms/forms.py", line 115, in _get_errors
    self.full_clean()
  File "/Users/myk/Documents/dev/django-trunk/django/forms/forms.py", line 272, in full_clean
    self._post_clean()
  File "/Users/myk/Documents/dev/django-trunk/django/forms/models.py", line 309, in _post_clean
    self.instance = construct_instance(self, self.instance, opts.fields, opts.exclude)
  File "/Users/myk/Documents/dev/django-trunk/django/forms/models.py", line 51, in construct_instance
    f.save_form_data(instance, cleaned_data[f.name])
  File "/Users/myk/Documents/dev/django-trunk/django/db/models/fields/__init__.py", line 454, in save_form_data
    setattr(instance, self.name, data)
  File "/Users/myk/Documents/dev/django-trunk/django/db/models/fields/related.py", line 362, in __set__
    (instance._meta.object_name, self.field.name))
ValueError: Cannot assign None: "BlankFK.related" does not allow null values.

I'm changing the summary to reflect this.

comment:3 by Claude Paroz, 12 years ago

Resolution: duplicate
Status: newclosed

This is a duplicate of #13776

Note: See TracTickets for help on using tickets.
Back to Top