Opened 6 years ago
Closed 6 years ago
#30436 closed Cleanup/optimization (fixed)
on_delete attribute must be callable.
Reported by: | Rémy Hubscher | Owned by: | robinh00d |
---|---|---|---|
Component: | Database layer (models, ORM) | Version: | dev |
Severity: | Normal | Keywords: | |
Cc: | Triage Stage: | Accepted | |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
If you set on_delete=None
as a ForeignKey field parameter you might get the following error:
File "django/contrib/admin/options.py", line 1823, in get_deleted_objects return get_deleted_objects(objs, request, self.admin_site) File "django/contrib/admin/utils.py", line 134, in get_deleted_objects collector.collect(objs) File "django/contrib/admin/utils.py", line 197, in collect return super().collect(objs, source_attr=source_attr, **kwargs) File "django/db/models/deletion.py", line 221, in collect field.remote_field.on_delete(self, field, sub_objs, self.using) TypeError: 'NoneType' object is not callable
I believe that we could validate the on_delete value to prevent such behaviour. Or at least tell that None is not a valid on_delete value.
Refs https://docs.djangoproject.com/fr/2.2/ref/models/fields/#django.db.models.ForeignKey.on_delete
Change History (7)
comment:1 by , 6 years ago
comment:2 by , 6 years ago
Component: | Uncategorized → Database layer (models, ORM) |
---|---|
Summary: | ForeignKey on_delete parameter should be validated → on_delete attribute must be callable. |
Triage Stage: | Unreviewed → Accepted |
Type: | Uncategorized → Cleanup/optimization |
Version: | 2.2 → master |
Thanks for the report. I can imagine that someone may provide custom on_delete
behavior, hence I don't think we should validate this attribute with a static list of on_delete
behaviors defined by Django. IMO we can raise
raise ValueError('on_delete must be callable.')
in __init__()
.
comment:3 by , 6 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
follow-up: 5 comment:4 by , 6 years ago
I agree with adding a check in __init__()
and raising ValueError('on_delete must be callable.')
. I'm going to submit a patch soon, do you have any suggestion as to where I should write my tests?
IMO it would be unnecessary to add an extra check for
None
since it covers all options foron_delete
explicitly in documentation:https://docs.djangoproject.com/en/2.2/ref/models/fields/#django.db.models.ForeignKey.on_delete