Opened 13 years ago
Closed 13 years ago
#18181 closed Uncategorized (invalid)
Not filling out a blank=True ForeignKey admin raw_id field raises ValueError because it tries to set the foreign key to None
Reported by: | James Aylett | Owned by: | James Aylett |
---|---|---|---|
Component: | contrib.admin | Version: | 1.3 |
Severity: | Normal | Keywords: | |
Cc: | Triage Stage: | Unreviewed | |
Has patch: | no | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
For a model with:
created_by = models.ForeignKey(User, related_name='created_%(class)s_set', null=False, blank=True)
(via an abstract base model, hence the pattern in related_name
). Submitting an admin with created_by
in raw_id_fields
, but not filling out the field (ie leaving it blank) results in the following backtrace:
File "/usr/local/lib/python2.7/dist-packages/django/core/handlers/base.py", line 111, in get_response response = callback(request, *callback_args, **callback_kwargs) File "/usr/local/lib/python2.7/dist-packages/django/contrib/admin/options.py", line 307, in wrapper return self.admin_site.admin_view(view)(*args, **kwargs) File "/usr/local/lib/python2.7/dist-packages/django/utils/decorators.py", line 93, in _wrapped_view response = view_func(request, *args, **kwargs) File "/usr/local/lib/python2.7/dist-packages/django/views/decorators/cache.py", line 79, in _wrapped_view_func response = view_func(request, *args, **kwargs) File "/usr/local/lib/python2.7/dist-packages/django/contrib/admin/sites.py", line 197, in inner return view(request, *args, **kwargs) File "/usr/local/lib/python2.7/dist-packages/django/utils/decorators.py", line 28, in _wrapper return bound_func(*args, **kwargs) File "/usr/local/lib/python2.7/dist-packages/django/utils/decorators.py", line 93, in _wrapped_view response = view_func(request, *args, **kwargs) File "/usr/local/lib/python2.7/dist-packages/django/utils/decorators.py", line 24, in bound_func return func(self, *args2, **kwargs2) File "/usr/local/lib/python2.7/dist-packages/django/db/transaction.py", line 217, in inner res = func(*args, **kwargs) File "/usr/local/lib/python2.7/dist-packages/django/contrib/admin/options.py", line 864, in add_view if form.is_valid(): File "/usr/local/lib/python2.7/dist-packages/django/forms/forms.py", line 121, in is_valid return self.is_bound and not bool(self.errors) File "/usr/local/lib/python2.7/dist-packages/django/forms/forms.py", line 112, in _get_errors self.full_clean() File "/usr/local/lib/python2.7/dist-packages/django/forms/forms.py", line 269, in full_clean self._post_clean() File "/usr/local/lib/python2.7/dist-packages/django/forms/models.py", line 308, in _post_clean self.instance = construct_instance(self, self.instance, opts.fields, opts.exclude) File "/usr/local/lib/python2.7/dist-packages/django/forms/models.py", line 50, in construct_instance f.save_form_data(instance, cleaned_data[f.name]) File "/usr/local/lib/python2.7/dist-packages/django/db/models/fields/__init__.py", line 416, in save_form_data setattr(instance, self.name, data) File "/usr/local/lib/python2.7/dist-packages/django/db/models/fields/related.py", line 327, in __set__ (instance._meta.object_name, self.field.name)) ValueError: Cannot assign None: “Collection.created_by” does not allow null values.
It should instead display a validation failure for that field in the admin page.
Forcibly setting required=True
on the field as created in django.contrib.admin.options:BaseModelAdmin.formfield_for_foreignkey()
(or in the ModelAdmin
subclass in the app's admin.py
) results in the desired behaviour, but probably isn't actually the correct approach.
This is similar to #9484, which was deduped into #8746, the final patch for which didn't deal with this, although it did deal with ValueError
in another situation. See also a late comment on 8746 which is the same as the issue here, and should have become a new ticket (and may have done, but I can't find it).
It is possible that this has been fixed in 1.4, which I can't migrate to yet; if so, then at least this ticket will capture that for anyone else who has the problem while 1.3 is still common.
Change History (5)
comment:1 by , 13 years ago
Resolution: | → invalid |
---|---|
Status: | new → closed |
comment:2 by , 13 years ago
Resolution: | invalid |
---|---|
Status: | closed → reopened |
Summary: | Not filling out a required ForeignKey admin raw_id field raises ValueError rather than showing a validation failure → Not filling out a blank=True ForeignKey admin raw_id field raises ValueError because it tries to set the foreign key to None |
Sorry, I confused myself and so both missed out some important details and then came to utterly the wrong confusion. Reopened and the missing information is:
The ForeignKey is managed automatically in a presave hook if not already set at save time. However the admin form is trying to se the ForeignKey to None, which fails. Ignore everything after the stack track in the original description. I believe the admin should not try to set ForeignKey fields if they are a raw_id_field and the field in the admin interface was left blank.
comment:3 by , 13 years ago
Currently working this into a new ticket that makes it clear what's going on, why I think it's wrong, and possibly figuring out a test for it at the same time. I'll close this once I've got the new ticket there.
comment:4 by , 13 years ago
Owner: | changed from | to
---|---|
Status: | reopened → new |
comment:5 by , 13 years ago
Resolution: | → invalid |
---|---|
Status: | new → closed |
1.4 turns out to be more of a design or documentation issue. Closing as invalid.
Is there really a problem or bug here?
You specified null=False and blank=True, which means that the database will not accept a null value, but a modelform will allow leaving this empty. This combination is perfectly acceptable if you add the value to your model instance programmatically in case the user leaves it empty. You obviously have not done that, hence the ValueError on saving.
My guess is you really want to be using blank=False here, though of course I may have misunderstood your intentions. Please reopen if that is the case.