From 218abcc9e550d266a9979e10f562fc21b8f34c6a Mon Sep 17 00:00:00 2001 From: Stephen Burrows Date: Wed, 3 Oct 2012 19:50:12 +0300 Subject: [PATCH] Fixed #14567 -- Made ModelMultipleChoiceField return EmptyQuerySet as empty value --- django/forms/models.py | 2 +- docs/ref/forms/fields.txt | 8 +++++-- docs/releases/1.5.txt | 3 +++ tests/modeltests/model_forms/tests.py | 5 +++-- tests/regressiontests/forms/models.py | 6 ++++++ tests/regressiontests/forms/tests/models.py | 23 +++++++++++++++++++-- 6 files changed, 40 insertions(+), 7 deletions(-) diff --git a/django/forms/models.py b/django/forms/models.py index 1aa49eaaec..11fe0c09ea 100644 --- a/django/forms/models.py +++ b/django/forms/models.py @@ -1013,7 +1013,7 @@ class ModelMultipleChoiceField(ModelChoiceField): if self.required and not value: raise ValidationError(self.error_messages['required']) elif not self.required and not value: - return [] + return self.queryset.none() if not isinstance(value, (list, tuple)): raise ValidationError(self.error_messages['list']) key = self.to_field_name or 'pk' diff --git a/docs/ref/forms/fields.txt b/docs/ref/forms/fields.txt index 82a3ea9ab3..27ca002312 100644 --- a/docs/ref/forms/fields.txt +++ b/docs/ref/forms/fields.txt @@ -997,13 +997,17 @@ objects (in the case of ``ModelMultipleChoiceField``) into the .. class:: ModelMultipleChoiceField(**kwargs) * Default widget: ``SelectMultiple`` - * Empty value: ``[]`` (an empty list) - * Normalizes to: A list of model instances. + * Empty value: An empty ``QuerySet`` (self.queryset.none()) + * Normalizes to: A ``QuerySet`` of model instances. * Validates that every id in the given list of values exists in the queryset. * Error message keys: ``required``, ``list``, ``invalid_choice``, ``invalid_pk_value`` + .. versionchanged:: 1.5 + The empty and normalized values were changed to be consistently + ``QuerySets`` instead of ``[]`` and ``QuerySet`` respectively. + Allows the selection of one or more model objects, suitable for representing a many-to-many relation. As with :class:`ModelChoiceField`, you can use ``label_from_instance`` to customize the object diff --git a/docs/releases/1.5.txt b/docs/releases/1.5.txt index 11b3488c11..d87ec36204 100644 --- a/docs/releases/1.5.txt +++ b/docs/releases/1.5.txt @@ -422,6 +422,9 @@ on the form. Miscellaneous ~~~~~~~~~~~~~ +* :class:`django.forms.ModelMultipleChoiceField` now returns an empty + ``QuerySet`` as the empty value instead of an empty list. + * :func:`~django.utils.http.int_to_base36` properly raises a :exc:`TypeError` instead of :exc:`ValueError` for non-integer inputs. diff --git a/tests/modeltests/model_forms/tests.py b/tests/modeltests/model_forms/tests.py index 038ce32287..947d0cf3c3 100644 --- a/tests/modeltests/model_forms/tests.py +++ b/tests/modeltests/model_forms/tests.py @@ -8,6 +8,7 @@ from django import forms from django.core.files.uploadedfile import SimpleUploadedFile from django.core.validators import ValidationError from django.db import connection +from django.db.models.query import EmptyQuerySet from django.forms.models import model_to_dict from django.utils.unittest import skipUnless from django.test import TestCase @@ -1035,8 +1036,8 @@ class OldFormForXTests(TestCase): f.clean([c6.id]) f = forms.ModelMultipleChoiceField(Category.objects.all(), required=False) - self.assertEqual(f.clean([]), []) - self.assertEqual(f.clean(()), []) + self.assertIsInstance(f.clean([]), EmptyQuerySet) + self.assertIsInstance(f.clean(()), EmptyQuerySet) with self.assertRaises(ValidationError): f.clean(['10']) with self.assertRaises(ValidationError): diff --git a/tests/regressiontests/forms/models.py b/tests/regressiontests/forms/models.py index 2f3ee9fa31..6e9c269356 100644 --- a/tests/regressiontests/forms/models.py +++ b/tests/regressiontests/forms/models.py @@ -63,6 +63,12 @@ class ChoiceFieldModel(models.Model): multi_choice_int = models.ManyToManyField(ChoiceOptionModel, blank=False, related_name='multi_choice_int', default=lambda: [1]) +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, + related_name='not_relevant2') + class FileModel(models.Model): file = models.FileField(storage=temp_storage, upload_to='tests') diff --git a/tests/regressiontests/forms/tests/models.py b/tests/regressiontests/forms/tests/models.py index c351509cee..be75643b28 100644 --- a/tests/regressiontests/forms/tests/models.py +++ b/tests/regressiontests/forms/tests/models.py @@ -11,7 +11,7 @@ from django.test import TestCase from django.utils import six from ..models import (ChoiceOptionModel, ChoiceFieldModel, FileModel, Group, - BoundaryModel, Defaults) + BoundaryModel, Defaults, OptionalMultiChoiceModel) class ChoiceFieldForm(ModelForm): @@ -19,6 +19,11 @@ class ChoiceFieldForm(ModelForm): model = ChoiceFieldModel +class OptionalMultiChoiceModelForm(ModelForm): + class Meta: + model = OptionalMultiChoiceModel + + class FileForm(Form): file1 = FileField() @@ -34,6 +39,21 @@ class TestTicket12510(TestCase): field = ModelChoiceField(Group.objects.order_by('-name')) self.assertEqual('a', field.clean(self.groups[0].pk).name) + +class TestTicket14567(TestCase): + """ + Check that the return values of ModelMultipleChoiceFields are QuerySets + """ + def test_empty_queryset_return(self): + "If a model's ManyToManyField has blank=True and is saved with no data, a queryset is returned." + form = OptionalMultiChoiceModelForm({'multi_choice_optional': '', 'multi_choice': ['1']}) + self.assertTrue(form.is_valid()) + # Check that the empty value is a QuerySet + self.assertTrue(isinstance(form.cleaned_data['multi_choice_optional'], models.query.QuerySet)) + # While we're at it, test whether a QuerySet is returned if there *is* a value. + self.assertTrue(isinstance(form.cleaned_data['multi_choice'], models.query.QuerySet)) + + class ModelFormCallableModelDefault(TestCase): def test_no_empty_option(self): "If a model's ForeignKey has blank=False and a default, no empty option is created (Refs #10792)." @@ -103,7 +123,6 @@ class ModelFormCallableModelDefault(TestCase): Hold down "Control", or "Command" on a Mac, to select more than one.

""") - class FormsModelTestCase(TestCase): def test_unicode_filename(self): # FileModel with unicode filename and data #########################