From c7aff31397a7228f6ac2e33c10ebdf36c4b7a9b7 Mon Sep 17 00:00:00 2001 From: Tim Graham Date: Tue, 13 Oct 2015 06:08:27 -0700 Subject: [PATCH] Refs #25535 -- Minor edits to ForeignObject check changes. --- django/db/models/fields/related.py | 10 +++--- docs/ref/checks.txt | 5 +-- tests/foreign_object/tests.py | 25 +++++--------- .../test_relative_fields.py | 34 +++++++++---------- 4 files changed, 33 insertions(+), 41 deletions(-) diff --git a/django/db/models/fields/related.py b/django/db/models/fields/related.py index 74a7a3e595..ef3de8f139 100644 --- a/django/db/models/fields/related.py +++ b/django/db/models/fields/related.py @@ -474,7 +474,6 @@ class ForeignObject(RelatedField): 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: @@ -483,11 +482,12 @@ class ForeignObject(RelatedField): model_name = self.remote_field.model.__name__ return [ checks.Error( - "No subset of the fields %s on model '%s' is unique" + "No subset of the fields %s on model '%s' is unique." % (field_combination, model_name), - 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), + 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', ) diff --git a/docs/ref/checks.txt b/docs/ref/checks.txt index cc8400395a..f0cf952ffd 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 559c988edd..5188645325 100644 --- a/tests/foreign_object/tests.py +++ b/tests/foreign_object/tests.py @@ -4,7 +4,7 @@ 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.test import SimpleTestCase, TestCase, skipUnlessDBFeature from django.utils import translation from .models import ( @@ -394,21 +394,21 @@ class MultiColumnFKTests(TestCase): 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'), - ) + 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, @@ -417,10 +417,7 @@ class MultiColumnFKTests(TestCase): related_name='children', ) - field = Child._meta.get_field('parent') - errors = field.check(from_model=Child) - - self.assertEqual(errors, []) + self.assertEqual(Child._meta.get_field('parent').check(from_model=Child), []) def test_check_subset_composite_foreign_object(self): class Parent(models.Model): @@ -429,16 +426,13 @@ class MultiColumnFKTests(TestCase): c = models.PositiveIntegerField() class Meta: - unique_together = ( - ('a', 'b'), - ) + 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, @@ -447,7 +441,4 @@ class MultiColumnFKTests(TestCase): related_name='children', ) - field = Child._meta.get_field('parent') - errors = field.check(from_model=Child) - - self.assertEqual(errors, []) + 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 545aa87548..3ba95fdbe4 100644 --- a/tests/invalid_models_tests/test_relative_fields.py +++ b/tests/invalid_models_tests/test_relative_fields.py @@ -508,9 +508,11 @@ class RelativeFieldTests(IsolatedModelsTestCase): errors = field.check() expected = [ Error( - "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.", + "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', ) @@ -1404,15 +1406,12 @@ class M2mThroughFieldsTests(IsolatedModelsTestCase): c = models.PositiveIntegerField() class Meta: - unique_together = ( - ('a', 'b', 'c'), - ) + 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, @@ -1425,9 +1424,11 @@ class M2mThroughFieldsTests(IsolatedModelsTestCase): 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.", + "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', ), @@ -1442,16 +1443,13 @@ class M2mThroughFieldsTests(IsolatedModelsTestCase): d = models.PositiveIntegerField() class Meta: - unique_together = ( - ('a', 'b', 'c'), - ) + 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, @@ -1464,9 +1462,11 @@ class M2mThroughFieldsTests(IsolatedModelsTestCase): 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.", + "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', ),