Opened 13 years ago

Closed 12 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 Koen Biermans, 13 years ago

Resolution: invalid
Status: newclosed

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.

comment:2 by James Aylett, 13 years ago

Resolution: invalid
Status: closedreopened
Summary: Not filling out a required ForeignKey admin raw_id field raises ValueError rather than showing a validation failureNot 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 James Aylett, 12 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 James Aylett, 12 years ago

Owner: changed from nobody to James Aylett
Status: reopenednew

comment:5 by James Aylett, 12 years ago

Resolution: invalid
Status: newclosed

1.4 turns out to be more of a design or documentation issue. Closing as invalid.

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