#22034 closed Bug (fixed)

Checks for ModelAdmin ForeignKeys fail with GenericInlineModelAdmin

Reported by: jwa Owned by: josven
Component: contrib.admin Version: master
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 Changed 18 months ago by babu

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

I've 3 errors:

<class 'app.admin.BarInline'>: (admin.E202) 'app.Bar' has no ForeignKey to 'app.Foo'.
Bar.content_object: (contenttypes.E001) The field refers to "Bar.object_id" field which is missing.
Bar.content_object: (contenttypes.E005) The field refers to Bar.content_type field which is missing.

But object_id and content_type fields are in my database.

Note for @jwa: contenttypes.generic has been deprecated (https://github.com/django/django/commit/10e3faf191d8f230dde8534d1c8fad8c8717816e)

comment:2 Changed 18 months ago by russellm

  • Severity changed from Normal to Release blocker
  • Triage Stage changed from Unreviewed to Accepted

This is a newly added system check, so it's a release blocker.

comment:3 Changed 18 months ago by josven

  • Owner changed from nobody to josven
  • Status changed from new to 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 Changed 18 months ago by josven

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.

https://github.com/django/django/pull/2351

comment:5 Changed 18 months ago by josven

The lastest approach did not completely work. The previous commit https://github.com/josven/django/commit/b697147d0b9e4b762aae7b2c057231894d6098c2 still works.

comment:6 Changed 17 months ago by CollinAnderson

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.

Version 0, edited 17 months ago by CollinAnderson (next)

comment:7 Changed 17 months ago by russellm

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 Changed 17 months ago by Russell Keith-Magee <russell@…>

  • Resolution set to fixed
  • Status changed from assigned to closed

In 70ec4d776ef0e68960ccee21476b8654e9399f53:

Fixed #22034 -- Added a specific set of relation checks for GenericInlineModelAdmin.

Thanks to jwa for the report.

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