Fixed #22034 -- Added a specific set of relation checks for GenericIn…

…lineModelAdmin.

Thanks to jwa for the report.

django/contrib/admin/checks.py

@@ -846,7 +846,7 @@ class InlineModelAdminChecks(BaseModelAdminChecks):
 
     def check(self, cls, parent_model, **kwargs):
         errors = super(InlineModelAdminChecks, self).check(cls, model=cls.model, **kwargs)
-        errors.extend(self._check_fk_name(cls, parent_model))
+        errors.extend(self._check_relation(cls, parent_model))
         errors.extend(self._check_exclude_of_parent_model(cls, parent_model))
         errors.extend(self._check_extra(cls))
         errors.extend(self._check_max_num(cls))
@@ -861,7 +861,7 @@ def _check_exclude_of_parent_model(self, cls, parent_model):
             return []
 
         # Skip if `fk_name` is invalid.
-        if self._check_fk_name(cls, parent_model):
+        if self._check_relation(cls, parent_model):
             return []
 
         if cls.exclude is None:
@@ -883,7 +883,7 @@ def _check_exclude_of_parent_model(self, cls, parent_model):
         else:
             return []
 
-    def _check_fk_name(self, cls, parent_model):
+    def _check_relation(self, cls, parent_model):
         try:
             _get_foreign_key(parent_model, cls.model, fk_name=cls.fk_name)
         except ValueError as e:

django/contrib/contenttypes/admin.py

@@ -2,19 +2,97 @@
 
 from functools import partial
 
+from django.contrib.admin.checks import InlineModelAdminChecks
 from django.contrib.admin.options import InlineModelAdmin, flatten_fieldsets
+from django.contrib.contenttypes.fields import GenericForeignKey
 from django.contrib.contenttypes.forms import (
     BaseGenericInlineFormSet, generic_inlineformset_factory
 )
+from django.core import checks
+from django.db.models.fields import FieldDoesNotExist
 from django.forms import ALL_FIELDS
 from django.forms.models import modelform_defines_fields
 
 
+class GenericInlineModelAdminChecks(InlineModelAdminChecks):
+    def _check_exclude_of_parent_model(self, cls, parent_model):
+        # There's no FK to exclude, so no exclusion checks are required.
+        return []
+
+    def _check_relation(self, cls, parent_model):
+        # There's no FK, but we do need to confirm that the ct_field and ct_fk_field are valid,
+        # and that they are part of a GenericForeignKey.
+
+        gfks = [
+            f for f in cls.model._meta.virtual_fields
+            if isinstance(f, GenericForeignKey)
+        ]
+        if len(gfks) == 0:
+            return [
+                checks.Error(
+                    "'%s.%s' has no GenericForeignKey." % (
+                        cls.model._meta.app_label, cls.model._meta.object_name
+                    ),
+                    hint=None,
+                    obj=cls,
+                    id='admin.E301'
+                )
+            ]
+        else:
+            # Check that the ct_field and ct_fk_fields exist
+            try:
+                cls.model._meta.get_field(cls.ct_field)
+            except FieldDoesNotExist:
+                return [
+                    checks.Error(
+                        "'ct_field' references '%s', which is not a field on '%s.%s'." % (
+                            cls.ct_field, cls.model._meta.app_label, cls.model._meta.object_name
+                        ),
+                        hint=None,
+                        obj=cls,
+                        id='admin.E302'
+                    )
+                ]
+
+            try:
+                cls.model._meta.get_field(cls.ct_fk_field)
+            except FieldDoesNotExist:
+                return [
+                    checks.Error(
+                        "'ct_fk_field' references '%s', which is not a field on '%s.%s'." % (
+                            cls.ct_fk_field, cls.model._meta.app_label, cls.model._meta.object_name
+                        ),
+                        hint=None,
+                        obj=cls,
+                        id='admin.E303'
+                    )
+                ]
+
+            # There's one or more GenericForeignKeys; make sure that one of them
+            # uses the right ct_field and ct_fk_field.
+            for gfk in gfks:
+                if gfk.ct_field == cls.ct_field and gfk.fk_field == cls.ct_fk_field:
+                    return []
+
+            return [
+                checks.Error(
+                    "'%s.%s' has no GenericForeignKey using content type field '%s' and object ID field '%s'." % (
+                        cls.model._meta.app_label, cls.model._meta.object_name, cls.ct_field, cls.ct_fk_field
+                    ),
+                    hint=None,
+                    obj=cls,
+                    id='admin.E304'
+                )
+            ]
+
+
 class GenericInlineModelAdmin(InlineModelAdmin):
     ct_field = "content_type"
     ct_fk_field = "object_id"
     formset = BaseGenericInlineFormSet
 
+    checks_class = GenericInlineModelAdminChecks
+
     def get_formset(self, request, obj=None, **kwargs):
         if 'fields' in kwargs:
             fields = kwargs.pop('fields')

docs/ref/checks.txt

@@ -199,11 +199,23 @@ The following checks are performed on any
 inline on a :class:`~django.contrib.admin.ModelAdmin`.
 
 * **admin.E201**: Cannot exclude the field ``<field name>``, because it is the foreign key to the parent model ``%s.%s``.
-* **admin.E202**: ``<model>`` has more than one ForeignKey to ``<parent model>``.
+* **admin.E202**: ``<model>`` has no ForeignKey to ``<parent model>``./``<model>`` has more than one ForeignKey to ``<parent model>``.
 * **admin.E203**: The value of ``extra`` must be an integer.
 * **admin.E204**: The value of ``max_num`` must be an integer.
 * **admin.E205**: The value of ``formset`` must inherit from ``BaseModelFormSet``.
 
+GenericInlineModelAdmin
+~~~~~~~~~~~~~~~~~~~~~~~
+
+The following checks are performed on any
+:class:`~django.contrib.contenttypes.admin.GenericInlineModelAdmin` that is
+registered as an inline on a :class:`~django.contrib.admin.ModelAdmin`.
+
+* **admin.E301**: 'ct_field' references ``<label>``, which is not a field on ``<model>``.
+* **admin.E302**: 'ct_fk_field' references ``<label>``, which is not a field on ``<model>``.
+* **admin.E303**: ``<model>`` has no GenericForeignKey.
+* **admin.E304**: ``<model>`` has no GenericForeignKey using content type field ``<field name>`` and object ID field ``<field name>``.
+
 
 Auth
 ----

tests/admin_checks/models.py

@@ -4,7 +4,8 @@
 
 from django.db import models
 from django.utils.encoding import python_2_unicode_compatible
-
+from django.contrib.contenttypes.models import ContentType
+from django.contrib.contenttypes.fields import GenericForeignKey
 
 class Album(models.Model):
     title = models.CharField(max_length=150)
@@ -55,3 +56,11 @@ class State(models.Model):
 
 class City(models.Model):
     state = models.ForeignKey(State)
+
+
+class Influence(models.Model):
+    name = models.TextField()
+
+    content_type = models.ForeignKey(ContentType)
+    object_id = models.PositiveIntegerField()
+    content_object = GenericForeignKey('content_type', 'object_id')

tests/admin_checks/tests.py

@@ -4,11 +4,12 @@
 
 from django import forms
 from django.contrib import admin
+from django.contrib.contenttypes.admin import GenericStackedInline
 from django.core import checks
 from django.core.exceptions import ImproperlyConfigured
 from django.test import TestCase
 
-from .models import Song, Book, Album, TwoAlbumFKAndAnE, City, State
+from .models import Song, Book, Album, TwoAlbumFKAndAnE, City, State, Influence
 
 
 class SongForm(forms.ModelForm):
@@ -183,6 +184,128 @@ class AlbumAdmin(admin.ModelAdmin):
         ]
         self.assertEqual(errors, expected)
 
+    def test_valid_generic_inline_model_admin(self):
+        """
+        Regression test for #22034 - check that generic inlines don't look for
+        normal ForeignKey relations.
+        """
+
+        class InfluenceInline(GenericStackedInline):
+            model = Influence
+
+        class SongAdmin(admin.ModelAdmin):
+            inlines = [InfluenceInline]
+
+        errors = SongAdmin.check(model=Song)
+        self.assertEqual(errors, [])
+
+    def test_generic_inline_model_admin_non_generic_model(self):
+        """
+        Ensure that a model without a GenericForeignKey raises problems if it's included
+        in an GenericInlineModelAdmin definition.
+        """
+
+        class BookInline(GenericStackedInline):
+            model = Book
+
+        class SongAdmin(admin.ModelAdmin):
+            inlines = [BookInline]
+
+        errors = SongAdmin.check(model=Song)
+        expected = [
+            checks.Error(
+                "'admin_checks.Book' has no GenericForeignKey.",
+                hint=None,
+                obj=BookInline,
+                id='admin.E301',
+            )
+        ]
+        self.assertEqual(errors, expected)
+
+    def test_generic_inline_model_admin_bad_ct_field(self):
+        "A GenericInlineModelAdmin raises problems if the ct_field points to a non-existent field."
+
+        class InfluenceInline(GenericStackedInline):
+            model = Influence
+            ct_field = 'nonexistent'
+
+        class SongAdmin(admin.ModelAdmin):
+            inlines = [InfluenceInline]
+
+        errors = SongAdmin.check(model=Song)
+        expected = [
+            checks.Error(
+                "'ct_field' references 'nonexistent', which is not a field on 'admin_checks.Influence'.",
+                hint=None,
+                obj=InfluenceInline,
+                id='admin.E302',
+            )
+        ]
+        self.assertEqual(errors, expected)
+
+    def test_generic_inline_model_admin_bad_fk_field(self):
+        "A GenericInlineModelAdmin raises problems if the ct_fk_field points to a non-existent field."
+
+        class InfluenceInline(GenericStackedInline):
+            model = Influence
+            ct_fk_field = 'nonexistent'
+
+        class SongAdmin(admin.ModelAdmin):
+            inlines = [InfluenceInline]
+
+        errors = SongAdmin.check(model=Song)
+        expected = [
+            checks.Error(
+                "'ct_fk_field' references 'nonexistent', which is not a field on 'admin_checks.Influence'.",
+                hint=None,
+                obj=InfluenceInline,
+                id='admin.E303',
+            )
+        ]
+        self.assertEqual(errors, expected)
+
+    def test_generic_inline_model_admin_non_gfk_ct_field(self):
+        "A GenericInlineModelAdmin raises problems if the ct_field points to a field that isn't part of a GenericForeignKey"
+
+        class InfluenceInline(GenericStackedInline):
+            model = Influence
+            ct_field = 'name'
+
+        class SongAdmin(admin.ModelAdmin):
+            inlines = [InfluenceInline]
+
+        errors = SongAdmin.check(model=Song)
+        expected = [
+            checks.Error(
+                "'admin_checks.Influence' has no GenericForeignKey using content type field 'name' and object ID field 'object_id'.",
+                hint=None,
+                obj=InfluenceInline,
+                id='admin.E304',
+            )
+        ]
+        self.assertEqual(errors, expected)
+
+    def test_generic_inline_model_admin_non_gfk_fk_field(self):
+        "A GenericInlineModelAdmin raises problems if the ct_fk_field points to a field that isn't part of a GenericForeignKey"
+
+        class InfluenceInline(GenericStackedInline):
+            model = Influence
+            ct_fk_field = 'name'
+
+        class SongAdmin(admin.ModelAdmin):
+            inlines = [InfluenceInline]
+
+        errors = SongAdmin.check(model=Song)
+        expected = [
+            checks.Error(
+                "'admin_checks.Influence' has no GenericForeignKey using content type field 'content_type' and object ID field 'name'.",
+                hint=None,
+                obj=InfluenceInline,
+                id='admin.E304',
+            )
+        ]
+        self.assertEqual(errors, expected)
+
     def test_app_label_in_admin_checks(self):
         """
         Regression test for #15669 - Include app label in admin system check messages