From 011abb7d96c75f6154c15a8a0f997d8c27698679 Mon Sep 17 00:00:00 2001 From: Anubhav Joshi Date: Wed, 9 Jul 2014 02:12:40 +0530 Subject: [PATCH] Fixed #19671 -- Added warnings that null and validators are ignored for ManyToManyField. Thanks Loic Bistuer and Tim Graham for help and review. --- django/contrib/admin/filters.py | 5 +-- django/db/models/fields/related.py | 26 ++++++++++++++++ docs/ref/checks.txt | 2 ++ docs/ref/models/fields.txt | 4 +++ tests/admin_filters/models.py | 2 +- tests/admin_widgets/models.py | 2 +- tests/forms_tests/models.py | 2 +- .../test_relative_fields.py | 31 ++++++++++++++++++- tests/m2m_regress/models.py | 2 +- tests/model_package/models/article.py | 2 +- tests/model_package/tests.py | 4 +-- tests/queries/models.py | 2 +- tests/serializers_regress/models.py | 6 ++-- tests/update_only_fields/models.py | 2 +- 14 files changed, 76 insertions(+), 16 deletions(-) diff --git a/django/contrib/admin/filters.py b/django/contrib/admin/filters.py index 9e539614a3..d5f31ab88c 100644 --- a/django/contrib/admin/filters.py +++ b/django/contrib/admin/filters.py @@ -8,6 +8,7 @@ certain test -- e.g. being a DateField or ForeignKey. import datetime from django.db import models +from django.db.models.fields.related import ManyToManyField from django.core.exceptions import ImproperlyConfigured, ValidationError from django.utils.encoding import smart_text, force_text from django.utils.translation import ugettext_lazy as _ @@ -207,8 +208,8 @@ class RelatedFieldListFilter(FieldListFilter): 'display': val, } if (isinstance(self.field, models.related.RelatedObject) and - self.field.field.null or hasattr(self.field, 'rel') and - self.field.null): + (self.field.field.null or isinstance(self.field.field, ManyToManyField)) or + hasattr(self.field, 'rel') and (self.field.null or isinstance(self.field, ManyToManyField))): yield { 'selected': bool(self.lookup_val_isnull), 'query_string': cl.get_query_string({ diff --git a/django/db/models/fields/related.py b/django/db/models/fields/related.py index b311fbe3f6..1dc5ff9b1d 100644 --- a/django/db/models/fields/related.py +++ b/django/db/models/fields/related.py @@ -1899,6 +1899,7 @@ class ManyToManyField(RelatedField): errors = super(ManyToManyField, self).check(**kwargs) errors.extend(self._check_unique(**kwargs)) errors.extend(self._check_relationship_model(**kwargs)) + errors.extend(self._check_ignored_options(**kwargs)) return errors def _check_unique(self, **kwargs): @@ -1913,6 +1914,31 @@ class ManyToManyField(RelatedField): ] return [] + def _check_ignored_options(self, **kwargs): + warnings = [] + + if self.null: + warnings.append( + checks.Warning( + 'null has no effect on ManyToManyField.', + hint=None, + obj=self, + id='fields.W340', + ) + ) + + if len(self._validators) > 0: + warnings.append( + checks.Warning( + 'ManyToManyField does not support validators.', + hint=None, + obj=self, + id='fields.W341', + ) + ) + + return warnings + def _check_relationship_model(self, from_model=None, **kwargs): if hasattr(self.rel.through, '_meta'): qualified_model_name = "%s.%s" % ( diff --git a/docs/ref/checks.txt b/docs/ref/checks.txt index 85cd616253..f008661319 100644 --- a/docs/ref/checks.txt +++ b/docs/ref/checks.txt @@ -148,6 +148,8 @@ Related Fields * **fields.E338**: The intermediary model ```` has no field ````. * **fields.E339**: ``.`` is not a foreign key to ````. +* **fields.W340**: ``null`` has no effect on ``ManyToManyField``. +* **fields.W341**: ``ManyToManyField`` does not support ``validators``. Signals ~~~~~~~ diff --git a/docs/ref/models/fields.txt b/docs/ref/models/fields.txt index a99522c77c..3a64ad934a 100644 --- a/docs/ref/models/fields.txt +++ b/docs/ref/models/fields.txt @@ -1448,6 +1448,10 @@ that control how the relationship functions. If in doubt, leave it to its default of ``True``. +:class:`ManyToManyField` does not support :attr:`~Field.validators`. + +:attr:`~Field.null` has no effect since there is no way to require a +relationship at the database level. .. _ref-onetoone: diff --git a/tests/admin_filters/models.py b/tests/admin_filters/models.py index 4634ebb535..3025d1b4a4 100644 --- a/tests/admin_filters/models.py +++ b/tests/admin_filters/models.py @@ -10,7 +10,7 @@ class Book(models.Model): title = models.CharField(max_length=50) year = models.PositiveIntegerField(null=True, blank=True) author = models.ForeignKey(User, verbose_name="Verbose Author", related_name='books_authored', blank=True, null=True) - contributors = models.ManyToManyField(User, verbose_name="Verbose Contributors", related_name='books_contributed', blank=True, null=True) + contributors = models.ManyToManyField(User, verbose_name="Verbose Contributors", related_name='books_contributed', blank=True) is_best_seller = models.NullBooleanField(default=0) date_registered = models.DateField(null=True) no = models.IntegerField(verbose_name='number', blank=True, null=True) # This field is intentionally 2 characters long. See #16080. diff --git a/tests/admin_widgets/models.py b/tests/admin_widgets/models.py index 5d295f0c9b..97153caadc 100644 --- a/tests/admin_widgets/models.py +++ b/tests/admin_widgets/models.py @@ -63,7 +63,7 @@ class Inventory(models.Model): class Event(models.Model): main_band = models.ForeignKey(Band, limit_choices_to=models.Q(pk__gt=0), related_name='events_main_band_at') - supporting_bands = models.ManyToManyField(Band, null=True, blank=True, related_name='events_supporting_band_at') + supporting_bands = models.ManyToManyField(Band, blank=True, related_name='events_supporting_band_at') start_date = models.DateField(blank=True, null=True) start_time = models.TimeField(blank=True, null=True) description = models.TextField(blank=True) diff --git a/tests/forms_tests/models.py b/tests/forms_tests/models.py index 8ade923a91..2ec0aa11ef 100644 --- a/tests/forms_tests/models.py +++ b/tests/forms_tests/models.py @@ -92,7 +92,7 @@ class ChoiceFieldModel(models.Model): class OptionalMultiChoiceModel(models.Model): multi_choice = models.ManyToManyField(ChoiceOptionModel, blank=False, related_name='not_relevant', default=lambda: ChoiceOptionModel.objects.filter(name='default')) - multi_choice_optional = models.ManyToManyField(ChoiceOptionModel, blank=True, null=True, + multi_choice_optional = models.ManyToManyField(ChoiceOptionModel, blank=True, related_name='not_relevant2') diff --git a/tests/invalid_models_tests/test_relative_fields.py b/tests/invalid_models_tests/test_relative_fields.py index a1a4c6037f..4b4c5947f3 100644 --- a/tests/invalid_models_tests/test_relative_fields.py +++ b/tests/invalid_models_tests/test_relative_fields.py @@ -1,7 +1,7 @@ # -*- encoding: utf-8 -*- from __future__ import unicode_literals -from django.core.checks import Error +from django.core.checks import Error, Warning as DjangoWarning from django.db import models from django.test.utils import override_settings from django.test.testcases import skipIfDBFeature @@ -60,6 +60,35 @@ class RelativeFieldTests(IsolatedModelsTestCase): ] self.assertEqual(errors, expected) + def test_many_to_many_with_useless_options(self): + class Model(models.Model): + name = models.CharField(max_length=20) + + class ModelM2M(models.Model): + m2m = models.ManyToManyField(Model, null=True, validators=['']) + + errors = ModelM2M.check() + field = ModelM2M._meta.get_field('m2m') + + expected = [ + DjangoWarning( + 'null has no effect on ManyToManyField.', + hint=None, + obj=field, + id='fields.W340', + ) + ] + expected.append( + DjangoWarning( + 'ManyToManyField does not support validators.', + hint=None, + obj=field, + id='fields.W341', + ) + ) + + self.assertEqual(errors, expected) + def test_ambiguous_relationship_model(self): class Person(models.Model): diff --git a/tests/m2m_regress/models.py b/tests/m2m_regress/models.py index 48fdad9304..4c1538dcbf 100644 --- a/tests/m2m_regress/models.py +++ b/tests/m2m_regress/models.py @@ -60,7 +60,7 @@ class Line(models.Model): class Worksheet(models.Model): id = models.CharField(primary_key=True, max_length=100) - lines = models.ManyToManyField(Line, blank=True, null=True) + lines = models.ManyToManyField(Line, blank=True) # Regression for #11226 -- A model with the same name that another one to diff --git a/tests/model_package/models/article.py b/tests/model_package/models/article.py index 4dfcdc1056..98ee3cd740 100644 --- a/tests/model_package/models/article.py +++ b/tests/model_package/models/article.py @@ -5,4 +5,4 @@ from django.db import models class Article(models.Model): sites = models.ManyToManyField(Site) headline = models.CharField(max_length=100) - publications = models.ManyToManyField("model_package.Publication", null=True, blank=True,) + publications = models.ManyToManyField("model_package.Publication", blank=True) diff --git a/tests/model_package/tests.py b/tests/model_package/tests.py index 7dc1f84a35..0105f1bd21 100644 --- a/tests/model_package/tests.py +++ b/tests/model_package/tests.py @@ -10,9 +10,7 @@ from .models.article import Article class Advertisement(models.Model): customer = models.CharField(max_length=100) - publications = models.ManyToManyField( - "model_package.Publication", null=True, blank=True - ) + publications = models.ManyToManyField("model_package.Publication", blank=True) class ModelPackageTests(TestCase): diff --git a/tests/queries/models.py b/tests/queries/models.py index 465b8f2446..bb3d52a635 100644 --- a/tests/queries/models.py +++ b/tests/queries/models.py @@ -101,7 +101,7 @@ class Item(models.Model): name = models.CharField(max_length=10) created = models.DateTimeField() modified = models.DateTimeField(blank=True, null=True) - tags = models.ManyToManyField(Tag, blank=True, null=True) + tags = models.ManyToManyField(Tag, blank=True) creator = models.ForeignKey(Author) note = models.ForeignKey(Note) diff --git a/tests/serializers_regress/models.py b/tests/serializers_regress/models.py index c2d7789259..82a3e6dd23 100644 --- a/tests/serializers_regress/models.py +++ b/tests/serializers_regress/models.py @@ -168,7 +168,7 @@ class FKDataNaturalKey(models.Model): class M2MData(models.Model): - data = models.ManyToManyField(Anchor, null=True) + data = models.ManyToManyField(Anchor) class O2OData(models.Model): @@ -181,7 +181,7 @@ class FKSelfData(models.Model): class M2MSelfData(models.Model): - data = models.ManyToManyField('self', null=True, symmetrical=False) + data = models.ManyToManyField('self', symmetrical=False) class FKDataToField(models.Model): @@ -193,7 +193,7 @@ class FKDataToO2O(models.Model): class M2MIntermediateData(models.Model): - data = models.ManyToManyField(Anchor, null=True, through='Intermediate') + data = models.ManyToManyField(Anchor, through='Intermediate') class Intermediate(models.Model): diff --git a/tests/update_only_fields/models.py b/tests/update_only_fields/models.py index ea319834c0..f2903b5bae 100644 --- a/tests/update_only_fields/models.py +++ b/tests/update_only_fields/models.py @@ -25,7 +25,7 @@ class Person(models.Model): class Employee(Person): employee_num = models.IntegerField(default=0) profile = models.ForeignKey('Profile', related_name='profiles', null=True) - accounts = models.ManyToManyField('Account', related_name='employees', blank=True, null=True) + accounts = models.ManyToManyField('Account', related_name='employees', blank=True) @python_2_unicode_compatible