mirror of
https://github.com/django/django.git
synced 2025-06-06 20:19:13 +00:00
Fixed #23338 -- Added warning when unique=True on ForeigKey
Thanks Jonathan Lindén for the initial patch, and Tim Graham and Gabe Jackson for the suggestions.
This commit is contained in:
parent
abf87333a1
commit
f39b0421b4
@ -153,6 +153,7 @@ class ChangepasswordManagementCommandTestCase(TestCase):
|
|||||||
|
|
||||||
|
|
||||||
@skipIfCustomUser
|
@skipIfCustomUser
|
||||||
|
@override_settings(SILENCED_SYSTEM_CHECKS=['fields.W342']) # ForeignKey(unique=True)
|
||||||
class CreatesuperuserManagementCommandTestCase(TestCase):
|
class CreatesuperuserManagementCommandTestCase(TestCase):
|
||||||
|
|
||||||
def test_basic_usage(self):
|
def test_basic_usage(self):
|
||||||
|
@ -1710,6 +1710,7 @@ class ForeignKey(ForeignObject):
|
|||||||
def check(self, **kwargs):
|
def check(self, **kwargs):
|
||||||
errors = super(ForeignKey, self).check(**kwargs)
|
errors = super(ForeignKey, self).check(**kwargs)
|
||||||
errors.extend(self._check_on_delete())
|
errors.extend(self._check_on_delete())
|
||||||
|
errors.extend(self._check_unique())
|
||||||
return errors
|
return errors
|
||||||
|
|
||||||
def _check_on_delete(self):
|
def _check_on_delete(self):
|
||||||
@ -1735,6 +1736,16 @@ class ForeignKey(ForeignObject):
|
|||||||
else:
|
else:
|
||||||
return []
|
return []
|
||||||
|
|
||||||
|
def _check_unique(self, **kwargs):
|
||||||
|
return [
|
||||||
|
checks.Warning(
|
||||||
|
'Setting unique=True on a ForeignKey has the same effect as using a OneToOneField.',
|
||||||
|
hint='ForeignKey(unique=True) is usually better served by a OneToOneField.',
|
||||||
|
obj=self,
|
||||||
|
id='fields.W342',
|
||||||
|
)
|
||||||
|
] if self.unique else []
|
||||||
|
|
||||||
def deconstruct(self):
|
def deconstruct(self):
|
||||||
name, path, args, kwargs = super(ForeignKey, self).deconstruct()
|
name, path, args, kwargs = super(ForeignKey, self).deconstruct()
|
||||||
del kwargs['to_fields']
|
del kwargs['to_fields']
|
||||||
@ -1891,6 +1902,10 @@ class OneToOneField(ForeignKey):
|
|||||||
else:
|
else:
|
||||||
setattr(instance, self.attname, data)
|
setattr(instance, self.attname, data)
|
||||||
|
|
||||||
|
def _check_unique(self, **kwargs):
|
||||||
|
# override ForeignKey since check isn't applicable here
|
||||||
|
return []
|
||||||
|
|
||||||
|
|
||||||
def create_many_to_many_intermediary_model(field, klass):
|
def create_many_to_many_intermediary_model(field, klass):
|
||||||
from django.db import models
|
from django.db import models
|
||||||
|
@ -154,6 +154,8 @@ Related Fields
|
|||||||
* **fields.E339**: ``<model>.<field name>`` is not a foreign key to ``<model>``.
|
* **fields.E339**: ``<model>.<field name>`` is not a foreign key to ``<model>``.
|
||||||
* **fields.W340**: ``null`` has no effect on ``ManyToManyField``.
|
* **fields.W340**: ``null`` has no effect on ``ManyToManyField``.
|
||||||
* **fields.W341**: ``ManyToManyField`` does not support ``validators``.
|
* **fields.W341**: ``ManyToManyField`` does not support ``validators``.
|
||||||
|
* **fields.W342**: Setting ``unique=True`` on a ``ForeignKey`` has the same
|
||||||
|
effect as using a ``OneToOneField``.
|
||||||
|
|
||||||
Signals
|
Signals
|
||||||
~~~~~~~
|
~~~~~~~
|
||||||
|
@ -8,6 +8,7 @@ from django.contrib.contenttypes.admin import GenericStackedInline
|
|||||||
from django.core import checks
|
from django.core import checks
|
||||||
from django.core.exceptions import ImproperlyConfigured
|
from django.core.exceptions import ImproperlyConfigured
|
||||||
from django.test import TestCase
|
from django.test import TestCase
|
||||||
|
from django.test.utils import override_settings
|
||||||
|
|
||||||
from .models import Song, Book, Album, TwoAlbumFKAndAnE, City, State, Influence
|
from .models import Song, Book, Album, TwoAlbumFKAndAnE, City, State, Influence
|
||||||
|
|
||||||
@ -34,6 +35,10 @@ class ValidFormFieldsets(admin.ModelAdmin):
|
|||||||
)
|
)
|
||||||
|
|
||||||
|
|
||||||
|
@override_settings(
|
||||||
|
SILENCED_SYSTEM_CHECKS=['fields.W342'], # ForeignKey(unique=True)
|
||||||
|
INSTALLED_APPS=['django.contrib.auth', 'django.contrib.contenttypes', 'admin_checks']
|
||||||
|
)
|
||||||
class SystemChecksTestCase(TestCase):
|
class SystemChecksTestCase(TestCase):
|
||||||
|
|
||||||
def test_checks_are_performed(self):
|
def test_checks_are_performed(self):
|
||||||
|
@ -8,7 +8,6 @@ from django.apps import apps
|
|||||||
from django.conf import settings
|
from django.conf import settings
|
||||||
from django.core import checks
|
from django.core import checks
|
||||||
from django.core.checks import Error, Warning
|
from django.core.checks import Error, Warning
|
||||||
from django.core.checks.model_checks import check_all_models
|
|
||||||
from django.core.checks.registry import CheckRegistry
|
from django.core.checks.registry import CheckRegistry
|
||||||
from django.core.checks.compatibility.django_1_7_0 import check_1_7_compatibility
|
from django.core.checks.compatibility.django_1_7_0 import check_1_7_compatibility
|
||||||
from django.core.management.base import CommandError
|
from django.core.management.base import CommandError
|
||||||
@ -303,7 +302,10 @@ class CheckFrameworkReservedNamesTests(TestCase):
|
|||||||
del self.current_models[model]
|
del self.current_models[model]
|
||||||
apps.clear_cache()
|
apps.clear_cache()
|
||||||
|
|
||||||
@override_settings(SILENCED_SYSTEM_CHECKS=['models.E020'])
|
@override_settings(
|
||||||
|
SILENCED_SYSTEM_CHECKS=['models.E20', 'fields.W342'], # ForeignKey(unique=True)
|
||||||
|
INSTALLED_APPS=['django.contrib.auth', 'django.contrib.contenttypes', 'check_framework']
|
||||||
|
)
|
||||||
def test_model_check_method_not_shadowed(self):
|
def test_model_check_method_not_shadowed(self):
|
||||||
class ModelWithAttributeCalledCheck(models.Model):
|
class ModelWithAttributeCalledCheck(models.Model):
|
||||||
check = 42
|
check = 42
|
||||||
@ -318,6 +320,7 @@ class CheckFrameworkReservedNamesTests(TestCase):
|
|||||||
check = models.ForeignKey(ModelWithRelatedManagerCalledCheck)
|
check = models.ForeignKey(ModelWithRelatedManagerCalledCheck)
|
||||||
article = models.ForeignKey(ModelWithRelatedManagerCalledCheck, related_name='check')
|
article = models.ForeignKey(ModelWithRelatedManagerCalledCheck, related_name='check')
|
||||||
|
|
||||||
|
errors = checks.run_checks()
|
||||||
expected = [
|
expected = [
|
||||||
Error(
|
Error(
|
||||||
"The 'ModelWithAttributeCalledCheck.check()' class method is "
|
"The 'ModelWithAttributeCalledCheck.check()' class method is "
|
||||||
@ -341,5 +344,4 @@ class CheckFrameworkReservedNamesTests(TestCase):
|
|||||||
id='models.E020'
|
id='models.E020'
|
||||||
),
|
),
|
||||||
]
|
]
|
||||||
|
self.assertEqual(errors, expected)
|
||||||
self.assertEqual(check_all_models(), expected)
|
|
||||||
|
@ -105,6 +105,7 @@ class IsolatedModelsTestCase(TestCase):
|
|||||||
apps.clear_cache()
|
apps.clear_cache()
|
||||||
|
|
||||||
|
|
||||||
|
@override_settings(SILENCED_SYSTEM_CHECKS=['fields.W342']) # ForeignKey(unique=True)
|
||||||
class GenericForeignKeyTests(IsolatedModelsTestCase):
|
class GenericForeignKeyTests(IsolatedModelsTestCase):
|
||||||
|
|
||||||
def test_str(self):
|
def test_str(self):
|
||||||
@ -202,6 +203,7 @@ class GenericForeignKeyTests(IsolatedModelsTestCase):
|
|||||||
]
|
]
|
||||||
self.assertEqual(errors, expected)
|
self.assertEqual(errors, expected)
|
||||||
|
|
||||||
|
@override_settings(INSTALLED_APPS=['django.contrib.auth', 'django.contrib.contenttypes', 'contenttypes_tests'])
|
||||||
def test_generic_foreign_key_checks_are_performed(self):
|
def test_generic_foreign_key_checks_are_performed(self):
|
||||||
class MyGenericForeignKey(GenericForeignKey):
|
class MyGenericForeignKey(GenericForeignKey):
|
||||||
def check(self, **kwargs):
|
def check(self, **kwargs):
|
||||||
|
@ -8,6 +8,7 @@ import warnings
|
|||||||
from django import test
|
from django import test
|
||||||
from django import forms
|
from django import forms
|
||||||
from django.core import validators
|
from django.core import validators
|
||||||
|
from django.core import checks
|
||||||
from django.core.exceptions import ValidationError
|
from django.core.exceptions import ValidationError
|
||||||
from django.db import connection, transaction, models, IntegrityError
|
from django.db import connection, transaction, models, IntegrityError
|
||||||
from django.db.models.fields import (
|
from django.db.models.fields import (
|
||||||
@ -20,6 +21,7 @@ from django.db.models.fields import (
|
|||||||
from django.db.models.fields.files import FileField, ImageField
|
from django.db.models.fields.files import FileField, ImageField
|
||||||
from django.utils import six
|
from django.utils import six
|
||||||
from django.utils.functional import lazy
|
from django.utils.functional import lazy
|
||||||
|
from django.test.utils import override_settings
|
||||||
|
|
||||||
from .models import (
|
from .models import (
|
||||||
Foo, Bar, Whiz, BigD, BigS, BigIntegerModel, Post, NullBooleanModel,
|
Foo, Bar, Whiz, BigD, BigS, BigIntegerModel, Post, NullBooleanModel,
|
||||||
@ -181,6 +183,22 @@ class ForeignKeyTests(test.TestCase):
|
|||||||
fk_model_empty = FkToChar.objects.select_related('out').get(id=fk_model_empty.pk)
|
fk_model_empty = FkToChar.objects.select_related('out').get(id=fk_model_empty.pk)
|
||||||
self.assertEqual(fk_model_empty.out, char_model_empty)
|
self.assertEqual(fk_model_empty.out, char_model_empty)
|
||||||
|
|
||||||
|
@override_settings(INSTALLED_APPS=['django.contrib.auth', 'django.contrib.contenttypes', 'model_fields'])
|
||||||
|
def test_warning_when_unique_true_on_fk(self):
|
||||||
|
class FKUniqueTrue(models.Model):
|
||||||
|
fk_field = models.ForeignKey(Foo, unique=True)
|
||||||
|
|
||||||
|
expected_warnings = [
|
||||||
|
checks.Warning(
|
||||||
|
'Setting unique=True on a ForeignKey has the same effect as using a OneToOneField.',
|
||||||
|
hint='ForeignKey(unique=True) is usually better served by a OneToOneField.',
|
||||||
|
obj=FKUniqueTrue.fk_field.field,
|
||||||
|
id='fields.W342',
|
||||||
|
)
|
||||||
|
]
|
||||||
|
warnings = checks.run_checks()
|
||||||
|
self.assertEqual(warnings, expected_warnings)
|
||||||
|
|
||||||
|
|
||||||
class DateTimeFieldTests(unittest.TestCase):
|
class DateTimeFieldTests(unittest.TestCase):
|
||||||
def test_datetimefield_to_python_usecs(self):
|
def test_datetimefield_to_python_usecs(self):
|
||||||
|
@ -3,6 +3,7 @@ from django.core.checks import run_checks, Error
|
|||||||
from django.db.models.signals import post_init
|
from django.db.models.signals import post_init
|
||||||
from django.test import TestCase
|
from django.test import TestCase
|
||||||
from django.utils import six
|
from django.utils import six
|
||||||
|
from django.test.utils import override_settings
|
||||||
|
|
||||||
|
|
||||||
class OnPostInit(object):
|
class OnPostInit(object):
|
||||||
@ -14,6 +15,10 @@ def on_post_init(**kwargs):
|
|||||||
pass
|
pass
|
||||||
|
|
||||||
|
|
||||||
|
@override_settings(
|
||||||
|
INSTALLED_APPS=['django.contrib.auth', 'django.contrib.contenttypes'],
|
||||||
|
SILENCED_SYSTEM_CHECKS=['fields.W342'], # ForeignKey(unique=True)
|
||||||
|
)
|
||||||
class ModelValidationTest(TestCase):
|
class ModelValidationTest(TestCase):
|
||||||
def test_models_validate(self):
|
def test_models_validate(self):
|
||||||
# All our models should validate properly
|
# All our models should validate properly
|
||||||
|
Loading…
x
Reference in New Issue
Block a user