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.
This commit is contained in:
Ramiro Morales 2013-05-21 18:32:39 -03:00
parent 01769823f1
commit 8c2fd050f8
5 changed files with 58 additions and 8 deletions

View File

@ -13,7 +13,7 @@ from django.forms.forms import BaseForm, get_declared_fields
from django.forms.formsets import BaseFormSet, formset_factory from django.forms.formsets import BaseFormSet, formset_factory
from django.forms.util import ErrorList from django.forms.util import ErrorList
from django.forms.widgets import (SelectMultiple, HiddenInput, 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.encoding import smart_text, force_text
from django.utils.datastructures import SortedDict from django.utils.datastructures import SortedDict
from django.utils import six from django.utils import six
@ -1104,9 +1104,10 @@ class ModelMultipleChoiceField(ModelChoiceField):
super(ModelMultipleChoiceField, self).__init__(queryset, None, super(ModelMultipleChoiceField, self).__init__(queryset, None,
cache_choices, required, widget, label, initial, help_text, cache_choices, required, widget, label, initial, help_text,
*args, **kwargs) *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.') 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): def clean(self, value):
if self.required and not value: if self.required and not value:

View File

@ -522,10 +522,10 @@ facilities together with Django built-in form :doc:`fields </ref/forms/fields>`
and :doc:`widgets </ref/forms/widgets>` aren't affected but need to be aware of and :doc:`widgets </ref/forms/widgets>` aren't affected but need to be aware of
what's described in :ref:`m2m-help_text-deprecation` below. what's described in :ref:`m2m-help_text-deprecation` below.
This is because, as an temporary backward-compatible provision, the described This is because, as an ad-hoc temporary backward-compatibility provision, the
non-standard behavior has been preserved but moved to the model form field layer described non-standard behavior has been preserved but moved to the model form
and occurs only when the associated widget is field layer and occurs only when the associated widget is
:class:`~django.forms.SelectMultiple` or a subclass. :class:`~django.forms.SelectMultiple` or selected subclasses.
QuerySet iteration QuerySet iteration
~~~~~~~~~~~~~~~~~~ ~~~~~~~~~~~~~~~~~~

View File

@ -13,6 +13,7 @@ from django.core.files.uploadedfile import SimpleUploadedFile
from django.db.models import CharField, DateField from django.db.models import CharField, DateField
from django.test import TestCase as DjangoTestCase from django.test import TestCase as DjangoTestCase
from django.test.utils import override_settings from django.test.utils import override_settings
from django.utils import six
from django.utils import translation from django.utils import translation
from django.utils.html import conditional_escape from django.utils.html import conditional_escape
from django.utils.unittest import TestCase from django.utils.unittest import TestCase
@ -139,6 +140,17 @@ class AdminFormfieldForDBFieldTests(TestCase):
def testInheritance(self): def testInheritance(self):
self.assertFormfield(models.Album, 'backside_art', widgets.AdminFileWidget) 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',)) @override_settings(PASSWORD_HASHERS=('django.contrib.auth.hashers.SHA1PasswordHasher',))
class AdminFormfieldForDBFieldWithRequestTests(DjangoTestCase): class AdminFormfieldForDBFieldWithRequestTests(DjangoTestCase):

View File

@ -255,3 +255,7 @@ class Colour(models.Model):
class ColourfulItem(models.Model): class ColourfulItem(models.Model):
name = models.CharField(max_length=50) name = models.CharField(max_length=50)
colours = models.ManyToManyField(Colour) colours = models.ManyToManyField(Colour)
class ArticleStatusNote(models.Model):
name = models.CharField(max_length=20)
status = models.ManyToManyField(ArticleStatus)

View File

@ -24,7 +24,7 @@ from .models import (Article, ArticleStatus, BetterAuthor, BigInt,
DerivedPost, ExplicitPK, FlexibleDatePost, ImprovedArticle, DerivedPost, ExplicitPK, FlexibleDatePost, ImprovedArticle,
ImprovedArticleWithParentLink, Inventory, Post, Price, ImprovedArticleWithParentLink, Inventory, Post, Price,
Product, TextFile, AuthorProfile, Colour, ColourfulItem, Product, TextFile, AuthorProfile, Colour, ColourfulItem,
test_images) ArticleStatusNote, test_images)
if test_images: if test_images:
from .models import ImageFile, OptionalImageFile from .models import ImageFile, OptionalImageFile
@ -234,6 +234,20 @@ class ColourfulItemForm(forms.ModelForm):
model = ColourfulItem model = ColourfulItem
fields = '__all__' 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): class ModelFormBaseTest(TestCase):
def test_base_form(self): def test_base_form(self):
@ -1677,3 +1691,22 @@ class OldFormForXTests(TestCase):
<option value="%(blue_pk)s">Blue</option> <option value="%(blue_pk)s">Blue</option>
</select> <span class="helptext"> Hold down "Control", or "Command" on a Mac, to select more than one.</span></p>""" </select> <span class="helptext"> Hold down "Control", or "Command" on a Mac, to select more than one.</span></p>"""
% {'blue_pk': colour.pk}) % {'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 = '<span class="helptext"> Hold down "Control", or "Command" on a Mac, to select more than one.</span>'
# 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('<ul id="id_status">', html)
self.assertInHTML(dreaded_help_text, html, count=0)