Opened 11 years ago
Closed 11 years ago
#22034 closed Bug (fixed)
Checks for ModelAdmin ForeignKeys fail with GenericInlineModelAdmin
Reported by: | Julian Wachholz | Owned by: | josven |
---|---|---|---|
Component: | contrib.admin | Version: | dev |
Severity: | Release blocker | Keywords: | checks |
Cc: | Triage Stage: | Accepted | |
Has patch: | no | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
Checks admin.E202
fail when trying to detect a ForeignKey
from a GenericInlineModelAdmin
:
The following setup yields this error:
<class 'app.admin.BarInline'>: (admin.E202) 'app.Bar' has no ForeignKey to 'app.Foo'.
models.py
class Foo(models.Model): """Foo may have some Bars.""" text = models.TextField() class Bar(models.Model): text = models.TextField() content_type = models.ForeignKey(ContentType) object_id = models.PositiveIntegerField() content_object = generic.GenericForeignKey('content_type', 'object_id')
admin.py
class BarInline(GenericStackedInline): model = Bar @admin.register(Foo) class FooAdmin(admin.ModelAdmin): inlines = [BarInline]
I think this will require an exception in _check_fk_name
for generic InlineAdmin classes here:
https://github.com/django/django/blob/06bd181f97e5e5561ae7e80efae4c38ee670773f/django/contrib/admin/checks.py#L866
Change History (8)
comment:1 by , 11 years ago
comment:2 by , 11 years ago
Severity: | Normal → Release blocker |
---|---|
Triage Stage: | Unreviewed → Accepted |
This is a newly added system check, so it's a release blocker.
comment:3 by , 11 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
These is a PR: https://github.com/django/django/pull/2351
I'm introducing dependency to django.contrib.contenttypes.fields.GenericForeignKey.
Don't know if this is acceptable or not.
comment:4 by , 11 years ago
Changed the approach.
- Created a GenericInlineModelAdminChecks that override def _check_fk_name.
- Moved the dependency for django.contrib.contenttypes in django.conrtib.admin.checks, to having a dependency for django.contrib.admin.checks in django.contrib.contenttypes.checks instead.
comment:5 by , 11 years ago
The lastest approach did not completely work. The previous commit https://github.com/josven/django/commit/b697147d0b9e4b762aae7b2c057231894d6098c2 still works.
comment:6 by , 11 years ago
I can confirm that the current pull request doesn't fix it but https://github.com/josven/django/commit/b697147 works for me.
As far as the dependency goes, could we have a Inline._check_fk()
on all inlines that lazy-imports code to handle the check?
Or, have a hook like Inline._checks = 'django.contrib.contenttypes.checks.GenericInlineModelAdminChecks'
As a side note, I like to do hacky things with my inlines (like store data in json on the original model) and sometimes that means it doesn't make sense to have an actual foreign key. And it would be nice to have a way to override the error.
comment:7 by , 11 years ago
The problem here is that GenericInlineModelAdmin is using the base InlineModelAdmin checks. We just need to make GenericInlineModelAdmin run it's own, generic-specific checks of the relation to the parent.
I'm working on a patch, incoming shortly.
comment:8 by , 11 years ago
Resolution: | → fixed |
---|---|
Status: | assigned → closed |
I've 3 errors:
But
object_id
andcontent_type
fields are in my database.Note for @jwa:
contenttypes.generic
has been deprecated (https://github.com/django/django/commit/10e3faf191d8f230dde8534d1c8fad8c8717816e)