From 57e08aa23247141db58c244f01afda52110b86b7 Mon Sep 17 00:00:00 2001 From: Russell Keith-Magee Date: Mon, 3 Mar 2014 14:19:06 +0800 Subject: [PATCH] Cleanup of contrib.contenttypes check messages. --- django/contrib/contenttypes/fields.py | 155 +++++++++----------------- tests/contenttypes_tests/tests.py | 74 +++--------- 2 files changed, 70 insertions(+), 159 deletions(-) diff --git a/django/contrib/contenttypes/fields.py b/django/contrib/contenttypes/fields.py index ee1e6a90609..45fbf8b730f 100644 --- a/django/contrib/contenttypes/fields.py +++ b/django/contrib/contenttypes/fields.py @@ -55,24 +55,16 @@ class GenericForeignKey(six.with_metaclass(RenameGenericForeignKeyMethods)): def check(self, **kwargs): errors = [] - errors.extend(self._check_content_type_field()) - errors.extend(self._check_object_id_field()) errors.extend(self._check_field_name()) + errors.extend(self._check_object_id_field()) + errors.extend(self._check_content_type_field()) return errors - def _check_content_type_field(self): - return _check_content_type_field( - model=self.model, - field_name=self.ct_field, - checked_object=self) - - def _check_object_id_field(self): - try: - self.model._meta.get_field(self.fk_field) - except FieldDoesNotExist: + def _check_field_name(self): + if self.name.endswith("_"): return [ checks.Error( - 'The field refers to "%s" field which is missing.' % self.fk_field, + 'Field names must not end with an underscore.', hint=None, obj=self, id='contenttypes.E001', @@ -81,11 +73,13 @@ class GenericForeignKey(six.with_metaclass(RenameGenericForeignKeyMethods)): else: return [] - def _check_field_name(self): - if self.name.endswith("_"): + def _check_object_id_field(self): + try: + self.model._meta.get_field(self.fk_field) + except FieldDoesNotExist: return [ checks.Error( - 'Field names must not end with underscores.', + "The GenericForeignKey object ID references the non-existent field '%s'." % self.fk_field, hint=None, obj=self, id='contenttypes.E002', @@ -94,6 +88,49 @@ class GenericForeignKey(six.with_metaclass(RenameGenericForeignKeyMethods)): else: return [] + def _check_content_type_field(self): + """ Check if field named `field_name` in model `model` exists and is + valid content_type field (is a ForeignKey to ContentType). """ + + try: + field = self.model._meta.get_field(self.ct_field) + except FieldDoesNotExist: + return [ + checks.Error( + "The GenericForeignKey content type references the non-existent field '%s.%s'." % ( + self.model._meta.object_name, self.ct_field + ), + hint=None, + obj=self, + id='contenttypes.E003', + ) + ] + else: + if not isinstance(field, models.ForeignKey): + return [ + checks.Error( + "'%s.%s' is not a ForeignKey." % ( + self.model._meta.object_name, self.ct_field + ), + hint="GenericForeignKeys must use a ForeignKey to 'contenttypes.ContentType' as the 'content_type' field.", + obj=self, + id='contenttypes.E004', + ) + ] + elif field.rel.to != ContentType: + return [ + checks.Error( + "'%s.%s' is not a ForeignKey to 'contenttypes.ContentType'." % ( + self.model._meta.object_name, self.ct_field + ), + hint="GenericForeignKeys must use a ForeignKey to 'contenttypes.ContentType' as the 'content_type' field.", + obj=self, + id='contenttypes.E005', + ) + ] + else: + return [] + def instance_pre_init(self, signal, sender, args, kwargs, **_kwargs): """ Handles initializing an object with the generic FK instead of @@ -228,43 +265,9 @@ class GenericRelation(ForeignObject): def check(self, **kwargs): errors = super(GenericRelation, self).check(**kwargs) - errors.extend(self._check_content_type_field()) - errors.extend(self._check_object_id_field()) errors.extend(self._check_generic_foreign_key_existence()) return errors - def _check_content_type_field(self): - target = self.rel.to - if isinstance(target, ModelBase): - return _check_content_type_field( - model=target, - field_name=self.content_type_field_name, - checked_object=self) - else: - return [] - - def _check_object_id_field(self): - target = self.rel.to - if isinstance(target, ModelBase): - opts = target._meta - try: - opts.get_field(self.object_id_field_name) - except FieldDoesNotExist: - return [ - checks.Error( - 'The field refers to %s.%s field which is missing.' % ( - opts.object_name, self.object_id_field_name - ), - hint=None, - obj=self, - id='contenttypes.E003', - ) - ] - else: - return [] - else: - return [] - def _check_generic_foreign_key_existence(self): target = self.rel.to if isinstance(target, ModelBase): @@ -280,8 +283,8 @@ class GenericRelation(ForeignObject): else: return [ checks.Warning( - ('The field defines a generic relation with the model ' - '%s.%s, but the model lacks GenericForeignKey.') % ( + ("The GenericRelation defines a relation with the model " + "'%s.%s', but that model does not have a GenericForeignKey.") % ( target._meta.app_label, target._meta.object_name ), hint=None, @@ -359,54 +362,6 @@ class GenericRelation(ForeignObject): }) -def _check_content_type_field(model, field_name, checked_object): - """ Check if field named `field_name` in model `model` exists and is - valid content_type field (is a ForeignKey to ContentType). """ - - try: - field = model._meta.get_field(field_name) - except FieldDoesNotExist: - return [ - checks.Error( - 'The field refers to %s.%s field which is missing.' % ( - model._meta.object_name, field_name - ), - hint=None, - obj=checked_object, - id='contenttypes.E005', - ) - ] - else: - if not isinstance(field, models.ForeignKey): - return [ - checks.Error( - ('"%s" field is used by a %s ' - 'as content type field and therefore it must be ' - 'a ForeignKey.') % ( - field_name, checked_object.__class__.__name__ - ), - hint=None, - obj=checked_object, - id='contenttypes.E006', - ) - ] - elif field.rel.to != ContentType: - return [ - checks.Error( - ('"%s" field is used by a %s ' - 'as content type field and therefore it must be ' - 'a ForeignKey to ContentType.') % ( - field_name, checked_object.__class__.__name__ - ), - hint=None, - obj=checked_object, - id='contenttypes.E007', - ) - ] - else: - return [] - - class ReverseGenericRelatedObjectsDescriptor(object): """ This class provides the functionality that makes the related-object diff --git a/tests/contenttypes_tests/tests.py b/tests/contenttypes_tests/tests.py index 5ded3268a1f..fdc2917b34f 100644 --- a/tests/contenttypes_tests/tests.py +++ b/tests/contenttypes_tests/tests.py @@ -106,10 +106,10 @@ class GenericForeignKeyTests(IsolatedModelsTestCase): errors = TaggedItem.content_object.check() expected = [ checks.Error( - 'The field refers to TaggedItem.content_type field which is missing.', + "The GenericForeignKey content type references the non-existent field 'TaggedItem.content_type'.", hint=None, obj=TaggedItem.content_object, - id='contenttypes.E005', + id='contenttypes.E003', ) ] self.assertEqual(errors, expected) @@ -124,12 +124,10 @@ class GenericForeignKeyTests(IsolatedModelsTestCase): errors = Model.content_object.check() expected = [ checks.Error( - ('"content_type" field is used by a GenericForeignKey ' - 'as content type field and therefore it must be ' - 'a ForeignKey.'), - hint=None, + "'Model.content_type' is not a ForeignKey.", + hint="GenericForeignKeys must use a ForeignKey to 'contenttypes.ContentType' as the 'content_type' field.", obj=Model.content_object, - id='contenttypes.E006', + id='contenttypes.E004', ) ] self.assertEqual(errors, expected) @@ -144,12 +142,10 @@ class GenericForeignKeyTests(IsolatedModelsTestCase): errors = Model.content_object.check() expected = [ checks.Error( - ('"content_type" field is used by a GenericForeignKey ' - 'as content type field and therefore it must be ' - 'a ForeignKey to ContentType.'), - hint=None, + "'Model.content_type' is not a ForeignKey to 'contenttypes.ContentType'.", + hint="GenericForeignKeys must use a ForeignKey to 'contenttypes.ContentType' as the 'content_type' field.", obj=Model.content_object, - id='contenttypes.E007', + id='contenttypes.E005', ) ] self.assertEqual(errors, expected) @@ -163,10 +159,10 @@ class GenericForeignKeyTests(IsolatedModelsTestCase): errors = TaggedItem.content_object.check() expected = [ checks.Error( - 'The field refers to "object_id" field which is missing.', + "The GenericForeignKey object ID references the non-existent field 'object_id'.", hint=None, obj=TaggedItem.content_object, - id='contenttypes.E001', + id='contenttypes.E002', ) ] self.assertEqual(errors, expected) @@ -181,10 +177,10 @@ class GenericForeignKeyTests(IsolatedModelsTestCase): errors = Model.content_object_.check() expected = [ checks.Error( - 'Field names must not end with underscores.', + 'Field names must not end with an underscore.', hint=None, obj=Model.content_object_, - id='contenttypes.E002', + id='contenttypes.E001', ) ] self.assertEqual(errors, expected) @@ -259,46 +255,6 @@ class GenericRelationshipTests(IsolatedModelsTestCase): errors = Model.rel.field.check() self.assertEqual(errors, []) - def test_missing_content_type_field(self): - class TaggedItem(models.Model): - # no content_type field - object_id = models.PositiveIntegerField() - content_object = GenericForeignKey() - - class Bookmark(models.Model): - tags = GenericRelation('TaggedItem') - - errors = Bookmark.tags.field.check() - expected = [ - checks.Error( - 'The field refers to TaggedItem.content_type field which is missing.', - hint=None, - obj=Bookmark.tags.field, - id='contenttypes.E005', - ) - ] - self.assertEqual(errors, expected) - - def test_missing_object_id_field(self): - class TaggedItem(models.Model): - content_type = models.ForeignKey(ContentType) - # missing object_id field - content_object = GenericForeignKey() - - class Bookmark(models.Model): - tags = GenericRelation('TaggedItem') - - errors = Bookmark.tags.field.check() - expected = [ - checks.Error( - 'The field refers to TaggedItem.object_id field which is missing.', - hint=None, - obj=Bookmark.tags.field, - id='contenttypes.E003', - ) - ] - self.assertEqual(errors, expected) - def test_missing_generic_foreign_key(self): class TaggedItem(models.Model): content_type = models.ForeignKey(ContentType) @@ -310,9 +266,9 @@ class GenericRelationshipTests(IsolatedModelsTestCase): errors = Bookmark.tags.field.check() expected = [ checks.Warning( - ('The field defines a generic relation with the model ' - 'contenttypes_tests.TaggedItem, but the model lacks ' - 'GenericForeignKey.'), + ("The GenericRelation defines a relation with the model " + "'contenttypes_tests.TaggedItem', but that model does not have a " + "GenericForeignKey."), hint=None, obj=Bookmark.tags.field, id='contenttypes.E004',