Opened 9 years ago

Closed 9 years ago

#24787 closed New feature (fixed)

Cannot assign a ForeignKey field with blank=True and null=False in Model.clean()

Reported by: Suriya Subramanian Owned by: nobody
Component: Forms Version: 1.8
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

Here's my model code

from django.db import models
from django.contrib.auth.models import User

class MyModel(models.Model):
    user = models.ForeignKey(User, blank=True, null=False)

    def clean(self):
        defaultuser = User.objects.first()
        if not defaultuser:
            raise ValueError('Need at least one user')
        self.user = defaultuser
        return super(MyModel, self).clean()

Here's my code where I create and initialize a ModelForm

from __future__ import print_function

import django
django.setup()

from myapp.models import MyModel
from django.forms.models import modelform_factory

MyForm = modelform_factory(MyModel, fields=('user', ))
myform = MyForm({})
print(myform.is_valid())

Here's the crash on running the above program:

Traceback (most recent call last):
  File "a.py", line 10, in <module>
    print myform.is_valid()
  File "/home/vagrant/bookshelf.git/my-env/local/lib/python2.7/site-packages/django/forms/forms.py", line 184, in is_valid
    return self.is_bound and not self.errors
  File "/home/vagrant/bookshelf.git/my-env/local/lib/python2.7/site-packages/django/forms/forms.py", line 176, in errors
    self.full_clean()
  File "/home/vagrant/bookshelf.git/my-env/local/lib/python2.7/site-packages/django/forms/forms.py", line 394, in full_clean
    self._post_clean()
  File "/home/vagrant/bookshelf.git/my-env/local/lib/python2.7/site-packages/django/forms/models.py", line 427, in _post_clean
    self.instance = construct_instance(self, self.instance, opts.fields, construct_instance_exclude)
  File "/home/vagrant/bookshelf.git/my-env/local/lib/python2.7/site-packages/django/forms/models.py", line 62, in construct_instance
    f.save_form_data(instance, cleaned_data[f.name])
  File "/home/vagrant/bookshelf.git/my-env/local/lib/python2.7/site-packages/django/db/models/fields/__init__.py", line 874, in save_form_data
    setattr(instance, self.name, data)
  File "/home/vagrant/bookshelf.git/my-env/local/lib/python2.7/site-packages/django/db/models/fields/related.py", line 619, in __set__
    (instance._meta.object_name, self.field.name)
ValueError: Cannot assign None: "MyModel.user" does not allow null values.

What's the right way to support a ForeignKey with blank=True and null=False? Form validation does not even raise ValidationError. It simply crashes.

Had a conversation with @apollo13 on IRC who said that this is a bug to be reported.

Change History (9)

comment:1 by Tim Graham, 9 years ago

In the clean() method, I believe you need to add the user to self.cleaned_data['user'] instead of simply assigning it to self.user.

comment:2 by Suriya Subramanian, 9 years ago

The clean() method is for the model, not the form. The model has no cleaned_data.

comment:3 by Tim Graham, 9 years ago

Right, I missed that. In your example code, do you need the field on the form at all? If you always assign defaultuser in clean(), I think you could move that to the model.save() method.

As model validation doesn’t run until after the instance is constructed (where the error is being thrown), I'm not sure there's a simple fix besides moving the logic to the form.clean() method as I hinted at before.

comment:4 by Suriya Subramanian, 9 years ago

In my specific situation, the field is optional in the form. If the user does not specify a value for the field, then MyModel.clean() will set the field. That's what hoped to do, but I encountered the crash before I got there. I will override Form.clean() and see if that works for me. Thank you for your replies.

The code sample I have written is just to illustrate what I think is a bug --- there is no good way to use a ForeignKey with blank=True and null=False in a form. Maybe this should be documented somewhere. You can keep this bug open if you think this is a code/documentation bug.

comment:5 by Abhaya Agarwal, 9 years ago

Component: UncategorizedForms

comment:6 by Tim Graham, 9 years ago

Summary: ModelForm validation crashes for ForeignKey field with blank=True and null=FalseCannot assign a ForeignKey field with blank=True and null=False in Model.clean()
Triage Stage: UnreviewedAccepted
Type: UncategorizedNew feature

Not quite sure what the proper resolution would look like (i.e. if there's a good way to allow the use case), but we can accept the ticket and perhaps document the limitation if needed (although it seems a bit of an edge case so I'm not sure where the best place to do so would be).

comment:7 by monikasulik, 9 years ago

Maybe I'm missing something, but isn't the code in the clean method wrong in the example? Shouldn't you be raising ValidationError instead of ValueError?

comment:8 by Suriya Subramanian, 9 years ago

The code in the clean method is correct. I deliberately raise ValueError because this error is not related to field validation, but related to the fact that there is not valid default value of user (in this example). The key point of this example is that clean() tries to set the value of a blank=True and null=False field. However model form validation causes a crash even before that.

comment:9 by Tim Graham, 9 years ago

Resolution: fixed
Status: newclosed

The null assignment exception in the ticket description was removed in 04e13c89138d48c20e774a2b6bf06796f73ac0fe (#26179) which will be in Django 1.10. I believe that resolves this issue.

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