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 , 9 years ago
comment:2 by , 9 years ago
The clean()
method is for the model, not the form. The model has no cleaned_data
.
comment:3 by , 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 , 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 , 9 years ago
Component: | Uncategorized → Forms |
---|
comment:6 by , 9 years ago
Summary: | ModelForm validation crashes for ForeignKey field with blank=True and null=False → Cannot assign a ForeignKey field with blank=True and null=False in Model.clean() |
---|---|
Triage Stage: | Unreviewed → Accepted |
Type: | Uncategorized → New 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 , 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 , 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 , 9 years ago
Resolution: | → fixed |
---|---|
Status: | new → closed |
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.
In the
clean()
method, I believe you need to add the user toself.cleaned_data['user']
instead of simply assigning it toself.user
.