Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

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

…lineModelAdmin.

Thanks to jwa for the report.
  • Loading branch information...
commit 70ec4d776ef0e68960ccee21476b8654e9399f53 1 parent 219d928
@freakboy3742 freakboy3742 authored
View
6 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:
View
78 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')
View
14 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
----
View
11 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')
View
125 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
Please sign in to comment.
Something went wrong with that request. Please try again.