diff --git a/django/db/models/fields/related.py b/django/db/models/fields/related.py index d3f5874409..5fa80e59ca 100644 --- a/django/db/models/fields/related.py +++ b/django/db/models/fields/related.py @@ -473,22 +473,35 @@ class ForeignObject(RelatedField): if not self.foreign_related_fields: return [] - has_unique_field = any(rel_field.unique - for rel_field in self.foreign_related_fields) - if not has_unique_field and len(self.foreign_related_fields) > 1: + unique_foreign_fields = { + frozenset([f.name]) + for f in self.remote_field.model._meta.get_fields() + if getattr(f, 'unique', False) + } + unique_foreign_fields.update({ + frozenset(ut) + for ut in self.remote_field.model._meta.unique_together + }) + foreign_fields = {f.name for f in self.foreign_related_fields} + has_unique_constraint = any(u <= foreign_fields for u in unique_foreign_fields) + + if not has_unique_constraint and len(self.foreign_related_fields) > 1: field_combination = ', '.join("'%s'" % rel_field.name for rel_field in self.foreign_related_fields) model_name = self.remote_field.model.__name__ return [ checks.Error( - "None of the fields %s on model '%s' have a unique=True constraint." + "No subset of the fields %s on model '%s' is unique." % (field_combination, model_name), - hint=None, + hint=( + "Add unique=True on any of those fields or add at " + "least a subset of them to a unique_together constraint." + ), obj=self, id='fields.E310', ) ] - elif not has_unique_field: + elif not has_unique_constraint: field_name = self.foreign_related_fields[0].name model_name = self.remote_field.model.__name__ return [ diff --git a/docs/ref/checks.txt b/docs/ref/checks.txt index 208e3fb81f..3cd68a43c8 100644 --- a/docs/ref/checks.txt +++ b/docs/ref/checks.txt @@ -189,8 +189,9 @@ Related Fields for ````. * **fields.E306**: Related name must be a valid Python identifier or end with a ``'+'``. -* **fields.E310**: None of the fields ````, ````, ... on model - ```` have a ``unique=True`` constraint. +* **fields.E310**: No subset of the fields ````, ````, ... on + model ```` is unique. Add ``unique=True`` on any of those fields or + add at least a subset of them to a unique_together constraint. * **fields.E311**: ```` must set ``unique=True`` because it is referenced by a ``ForeignKey``. * **fields.E320**: Field specifies ``on_delete=SET_NULL``, but cannot be null. diff --git a/tests/foreign_object/tests.py b/tests/foreign_object/tests.py index 06e2551e4f..5188645325 100644 --- a/tests/foreign_object/tests.py +++ b/tests/foreign_object/tests.py @@ -2,7 +2,9 @@ import datetime from operator import attrgetter from django.core.exceptions import FieldError -from django.test import TestCase, skipUnlessDBFeature +from django.db import models +from django.db.models.fields.related import ForeignObject +from django.test import SimpleTestCase, TestCase, skipUnlessDBFeature from django.utils import translation from .models import ( @@ -391,3 +393,52 @@ class MultiColumnFKTests(TestCase): """ See: https://code.djangoproject.com/ticket/21566 """ objs = [Person(name="abcd_%s" % i, person_country=self.usa) for i in range(0, 5)] Person.objects.bulk_create(objs, 10) + + +class TestModelCheckTests(SimpleTestCase): + + def test_check_composite_foreign_object(self): + class Parent(models.Model): + a = models.PositiveIntegerField() + b = models.PositiveIntegerField() + + class Meta: + unique_together = (('a', 'b'),) + + class Child(models.Model): + a = models.PositiveIntegerField() + b = models.PositiveIntegerField() + value = models.CharField(max_length=255) + parent = ForeignObject( + Parent, + on_delete=models.SET_NULL, + from_fields=('a', 'b'), + to_fields=('a', 'b'), + related_name='children', + ) + + self.assertEqual(Child._meta.get_field('parent').check(from_model=Child), []) + + def test_check_subset_composite_foreign_object(self): + class Parent(models.Model): + a = models.PositiveIntegerField() + b = models.PositiveIntegerField() + c = models.PositiveIntegerField() + + class Meta: + unique_together = (('a', 'b'),) + + class Child(models.Model): + a = models.PositiveIntegerField() + b = models.PositiveIntegerField() + c = models.PositiveIntegerField() + d = models.CharField(max_length=255) + parent = ForeignObject( + Parent, + on_delete=models.SET_NULL, + from_fields=('a', 'b', 'c'), + to_fields=('a', 'b', 'c'), + related_name='children', + ) + + self.assertEqual(Child._meta.get_field('parent').check(from_model=Child), []) diff --git a/tests/invalid_models_tests/test_relative_fields.py b/tests/invalid_models_tests/test_relative_fields.py index ad1c25da52..3ba95fdbe4 100644 --- a/tests/invalid_models_tests/test_relative_fields.py +++ b/tests/invalid_models_tests/test_relative_fields.py @@ -5,6 +5,7 @@ import warnings from django.core.checks import Error, Warning as DjangoWarning from django.db import models +from django.db.models.fields.related import ForeignObject from django.test import ignore_warnings from django.test.testcases import skipIfDBFeature from django.test.utils import override_settings @@ -507,9 +508,11 @@ class RelativeFieldTests(IsolatedModelsTestCase): errors = field.check() expected = [ Error( - "None of the fields 'country_id', 'city_id' on model 'Person' " - "have a unique=True constraint.", - hint=None, + "No subset of the fields 'country_id', 'city_id' on model 'Person' is unique.", + hint=( + "Add unique=True on any of those fields or add at least " + "a subset of them to a unique_together constraint." + ), obj=field, id='fields.E310', ) @@ -1395,3 +1398,77 @@ class M2mThroughFieldsTests(IsolatedModelsTestCase): obj=field, id='fields.E337')] self.assertEqual(expected, errors) + + def test_superset_foreign_object(self): + class Parent(models.Model): + a = models.PositiveIntegerField() + b = models.PositiveIntegerField() + c = models.PositiveIntegerField() + + class Meta: + unique_together = (('a', 'b', 'c'),) + + class Child(models.Model): + a = models.PositiveIntegerField() + b = models.PositiveIntegerField() + value = models.CharField(max_length=255) + parent = ForeignObject( + Parent, + on_delete=models.SET_NULL, + from_fields=('a', 'b'), + to_fields=('a', 'b'), + related_name='children', + ) + + field = Child._meta.get_field('parent') + errors = field.check(from_model=Child) + expected = [ + Error( + "No subset of the fields 'a', 'b' on model 'Parent' is unique.", + hint=( + "Add unique=True on any of those fields or add at least " + "a subset of them to a unique_together constraint." + ), + obj=field, + id='fields.E310', + ), + ] + self.assertEqual(expected, errors) + + def test_insersection_foreign_object(self): + class Parent(models.Model): + a = models.PositiveIntegerField() + b = models.PositiveIntegerField() + c = models.PositiveIntegerField() + d = models.PositiveIntegerField() + + class Meta: + unique_together = (('a', 'b', 'c'),) + + class Child(models.Model): + a = models.PositiveIntegerField() + b = models.PositiveIntegerField() + d = models.PositiveIntegerField() + value = models.CharField(max_length=255) + parent = ForeignObject( + Parent, + on_delete=models.SET_NULL, + from_fields=('a', 'b', 'd'), + to_fields=('a', 'b', 'd'), + related_name='children', + ) + + field = Child._meta.get_field('parent') + errors = field.check(from_model=Child) + expected = [ + Error( + "No subset of the fields 'a', 'b', 'd' on model 'Parent' is unique.", + hint=( + "Add unique=True on any of those fields or add at least " + "a subset of them to a unique_together constraint." + ), + obj=field, + id='fields.E310', + ), + ] + self.assertEqual(expected, errors)