summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorAntoine Catton <acatton@fusionbox.com>2015-10-09 10:55:19 -0600
committerSimon Charette <charette.s@gmail.com>2015-10-12 18:00:59 -0400
commit80dac8c33e7f6f22577e4346f44e4c5ee89b648c (patch)
tree113d156bf62b84bad2dd758b033a8af864740a32
parent533c10998ad45854208d7e4b89d3c077faf16b8a (diff)
Fixed #25535 -- Made ForeignObject checks less strict.
Check that the foreign object `from_fields` are a subset of any unique constraints on the foreign model.
-rw-r--r--django/db/models/fields/related.py25
-rw-r--r--tests/foreign_object/tests.py60
-rw-r--r--tests/invalid_models_tests/test_relative_fields.py83
3 files changed, 159 insertions, 9 deletions
diff --git a/django/db/models/fields/related.py b/django/db/models/fields/related.py
index 24bab96df6..74a7a3e595 100644
--- a/django/db/models/fields/related.py
+++ b/django/db/models/fields/related.py
@@ -464,22 +464,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 a unique=True on any of the fields %s of '%s', or add them all or a "
+ "subset to a unique_together constraint."
+ % (field_combination, model_name),
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/tests/foreign_object/tests.py b/tests/foreign_object/tests.py
index 06e2551e4f..559c988edd 100644
--- a/tests/foreign_object/tests.py
+++ b/tests/foreign_object/tests.py
@@ -2,6 +2,8 @@ import datetime
from operator import attrgetter
from django.core.exceptions import FieldError
+from django.db import models
+from django.db.models.fields.related import ForeignObject
from django.test import TestCase, skipUnlessDBFeature
from django.utils import translation
@@ -391,3 +393,61 @@ 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)
+
+ 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',
+ )
+
+ field = Child._meta.get_field('parent')
+ errors = field.check(from_model=Child)
+
+ self.assertEqual(errors, [])
+
+ 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',
+ )
+
+ field = Child._meta.get_field('parent')
+ errors = field.check(from_model=Child)
+
+ self.assertEqual(errors, [])
diff --git a/tests/invalid_models_tests/test_relative_fields.py b/tests/invalid_models_tests/test_relative_fields.py
index ad1c25da52..545aa87548 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,9 @@ 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 a unique=True on any of the fields 'country_id', 'city_id' of 'Person', "
+ "or add them all or a subset to a unique_together constraint.",
obj=field,
id='fields.E310',
)
@@ -1395,3 +1396,79 @@ 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 a unique=True on any of the fields 'a', 'b' of 'Parent', or add them "
+ "all or a subset 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 a unique=True on any of the fields 'a', 'b', 'd' of 'Parent', or add "
+ "them all or a subset to a unique_together constraint.",
+ obj=field,
+ id='fields.E310',
+ ),
+ ]
+ self.assertEqual(expected, errors)