Fixed #22034 -- Added a specific set of relation checks for GenericInlineModelAdmin. (70ec4d77) · Commits · Dom Sekotill / django

django/contrib/admin/checks.py

+3 −3

Original line number	Diff line number	Diff line
		@@ -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 @@ class InlineModelAdminChecks(BaseModelAdminChecks):
		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 @@ class InlineModelAdminChecks(BaseModelAdminChecks):
		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

+78 −0

Original line number	Diff line number	Diff line
		@@ -2,19 +2,97 @@ from __future__ import unicode_literals

		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

+13 −1

Original line number	Diff line number	Diff line
		@@ -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

+10 −1

Original line number	Diff line number	Diff line
		@@ -4,7 +4,8 @@ Tests of ModelAdmin system checks logic.

		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

+124 −1

Original line number	Diff line number	Diff line
		@@ -4,11 +4,12 @@ import warnings

		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 SystemChecksTestCase(TestCase):
		]
		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