From 09f865276554f35060ff939722ec4cefd578edf6 Mon Sep 17 00:00:00 2001 From: Marc Tamlyn Date: Tue, 21 May 2013 10:42:15 +0100 Subject: Use assertIsInstance in tests. Gives much nicer errors when it fails. --- tests/model_forms/tests.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) (limited to 'tests/model_forms') diff --git a/tests/model_forms/tests.py b/tests/model_forms/tests.py index c5db011404..db8b6cf0e7 100644 --- a/tests/model_forms/tests.py +++ b/tests/model_forms/tests.py @@ -274,8 +274,8 @@ class ModelFormBaseTest(TestCase): model = Category fields = '__all__' - self.assertTrue(isinstance(ReplaceField.base_fields['url'], - forms.fields.BooleanField)) + self.assertIsInstance(ReplaceField.base_fields['url'], + forms.fields.BooleanField) def test_replace_field_variant_2(self): # Should have the same result as before, @@ -287,8 +287,8 @@ class ModelFormBaseTest(TestCase): model = Category fields = ['url'] - self.assertTrue(isinstance(ReplaceField.base_fields['url'], - forms.fields.BooleanField)) + self.assertIsInstance(ReplaceField.base_fields['url'], + forms.fields.BooleanField) def test_replace_field_variant_3(self): # Should have the same result as before, @@ -300,8 +300,8 @@ class ModelFormBaseTest(TestCase): model = Category fields = [] # url will still appear, since it is explicit above - self.assertTrue(isinstance(ReplaceField.base_fields['url'], - forms.fields.BooleanField)) + self.assertIsInstance(ReplaceField.base_fields['url'], + forms.fields.BooleanField) def test_override_field(self): class AuthorForm(forms.ModelForm): -- cgit v1.3 From 8c2fd050f80f528cc1609c1a7f16901194834831 Mon Sep 17 00:00:00 2001 From: Ramiro Morales Date: Tue, 21 May 2013 18:32:39 -0300 Subject: Made fix for #9321 less buggy and more effective. Don't try to be smart about building a good-looking help string because it evaluates translations too early, simply use the same old strategy as before. Thanks Donald Stufft for the report. Also, actually fix the case reported by the OP by special-casing CheckboxSelectMultiple. Added tests. Refs #9321. --- django/forms/models.py | 7 ++++--- docs/releases/1.6.txt | 8 ++++---- tests/admin_widgets/tests.py | 12 ++++++++++++ tests/model_forms/models.py | 4 ++++ tests/model_forms/tests.py | 35 ++++++++++++++++++++++++++++++++++- 5 files changed, 58 insertions(+), 8 deletions(-) (limited to 'tests/model_forms') diff --git a/django/forms/models.py b/django/forms/models.py index 68c1341cf7..9d5aa5d4d8 100644 --- a/django/forms/models.py +++ b/django/forms/models.py @@ -13,7 +13,7 @@ from django.forms.forms import BaseForm, get_declared_fields from django.forms.formsets import BaseFormSet, formset_factory from django.forms.util import ErrorList from django.forms.widgets import (SelectMultiple, HiddenInput, - MultipleHiddenInput, media_property) + MultipleHiddenInput, media_property, CheckboxSelectMultiple) from django.utils.encoding import smart_text, force_text from django.utils.datastructures import SortedDict from django.utils import six @@ -1104,9 +1104,10 @@ class ModelMultipleChoiceField(ModelChoiceField): super(ModelMultipleChoiceField, self).__init__(queryset, None, cache_choices, required, widget, label, initial, help_text, *args, **kwargs) - if isinstance(self.widget, SelectMultiple): + # Remove this in Django 1.8 + if isinstance(self.widget, SelectMultiple) and not isinstance(self.widget, CheckboxSelectMultiple): msg = _('Hold down "Control", or "Command" on a Mac, to select more than one.') - self.help_text = string_concat(self.help_text, ' ', msg) if self.help_text else msg + self.help_text = string_concat(self.help_text, ' ', msg) def clean(self, value): if self.required and not value: diff --git a/docs/releases/1.6.txt b/docs/releases/1.6.txt index 3bc5a0996c..5ee454d4f8 100644 --- a/docs/releases/1.6.txt +++ b/docs/releases/1.6.txt @@ -522,10 +522,10 @@ facilities together with Django built-in form :doc:`fields ` and :doc:`widgets ` aren't affected but need to be aware of what's described in :ref:`m2m-help_text-deprecation` below. -This is because, as an temporary backward-compatible provision, the described -non-standard behavior has been preserved but moved to the model form field layer -and occurs only when the associated widget is -:class:`~django.forms.SelectMultiple` or a subclass. +This is because, as an ad-hoc temporary backward-compatibility provision, the +described non-standard behavior has been preserved but moved to the model form +field layer and occurs only when the associated widget is +:class:`~django.forms.SelectMultiple` or selected subclasses. QuerySet iteration ~~~~~~~~~~~~~~~~~~ diff --git a/tests/admin_widgets/tests.py b/tests/admin_widgets/tests.py index 4e09922893..98f41c1490 100644 --- a/tests/admin_widgets/tests.py +++ b/tests/admin_widgets/tests.py @@ -13,6 +13,7 @@ from django.core.files.uploadedfile import SimpleUploadedFile from django.db.models import CharField, DateField from django.test import TestCase as DjangoTestCase from django.test.utils import override_settings +from django.utils import six from django.utils import translation from django.utils.html import conditional_escape from django.utils.unittest import TestCase @@ -139,6 +140,17 @@ class AdminFormfieldForDBFieldTests(TestCase): def testInheritance(self): self.assertFormfield(models.Album, 'backside_art', widgets.AdminFileWidget) + def test_m2m_widgets(self): + """m2m fields help text as it applies to admin app (#9321).""" + class AdvisorAdmin(admin.ModelAdmin): + filter_vertical=['companies'] + + self.assertFormfield(models.Advisor, 'companies', widgets.FilteredSelectMultiple, + filter_vertical=['companies']) + ma = AdvisorAdmin(models.Advisor, admin.site) + f = ma.formfield_for_dbfield(models.Advisor._meta.get_field('companies'), request=None) + self.assertEqual(six.text_type(f.help_text), ' Hold down "Control", or "Command" on a Mac, to select more than one.') + @override_settings(PASSWORD_HASHERS=('django.contrib.auth.hashers.SHA1PasswordHasher',)) class AdminFormfieldForDBFieldWithRequestTests(DjangoTestCase): diff --git a/tests/model_forms/models.py b/tests/model_forms/models.py index a79d9b8c5b..9c5e097106 100644 --- a/tests/model_forms/models.py +++ b/tests/model_forms/models.py @@ -255,3 +255,7 @@ class Colour(models.Model): class ColourfulItem(models.Model): name = models.CharField(max_length=50) colours = models.ManyToManyField(Colour) + +class ArticleStatusNote(models.Model): + name = models.CharField(max_length=20) + status = models.ManyToManyField(ArticleStatus) diff --git a/tests/model_forms/tests.py b/tests/model_forms/tests.py index db8b6cf0e7..5219804e0c 100644 --- a/tests/model_forms/tests.py +++ b/tests/model_forms/tests.py @@ -24,7 +24,7 @@ from .models import (Article, ArticleStatus, BetterAuthor, BigInt, DerivedPost, ExplicitPK, FlexibleDatePost, ImprovedArticle, ImprovedArticleWithParentLink, Inventory, Post, Price, Product, TextFile, AuthorProfile, Colour, ColourfulItem, - test_images) + ArticleStatusNote, test_images) if test_images: from .models import ImageFile, OptionalImageFile @@ -234,6 +234,20 @@ class ColourfulItemForm(forms.ModelForm): model = ColourfulItem fields = '__all__' +# model forms for testing work on #9321: + +class StatusNoteForm(forms.ModelForm): + class Meta: + model = ArticleStatusNote + fields = '__all__' + + +class StatusNoteCBM2mForm(forms.ModelForm): + class Meta: + model = ArticleStatusNote + fields = '__all__' + widgets = {'status': forms.CheckboxSelectMultiple} + class ModelFormBaseTest(TestCase): def test_base_form(self): @@ -1677,3 +1691,22 @@ class OldFormForXTests(TestCase): Hold down "Control", or "Command" on a Mac, to select more than one.

""" % {'blue_pk': colour.pk}) + + +class M2mHelpTextTest(TestCase): + """Tests for ticket #9321.""" + def test_multiple_widgets(self): + """Help text of different widgets for ManyToManyFields model fields""" + dreaded_help_text = ' Hold down "Control", or "Command" on a Mac, to select more than one.' + + # Default widget (SelectMultiple): + std_form = StatusNoteForm() + self.assertInHTML(dreaded_help_text, std_form.as_p()) + + # Overridden widget (CheckboxSelectMultiple, a subclass of + # SelectMultiple but with a UI that doesn't involve Control/Command + # keystrokes to extend selection): + form = StatusNoteCBM2mForm() + html = form.as_p() + self.assertInHTML('