Skip to content
This repository

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse code

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.
  • Loading branch information...
commit 8c2fd050f80f528cc1609c1a7f16901194834831 1 parent 0176982
Ramiro Morales authored May 21, 2013
7  django/forms/models.py
@@ -13,7 +13,7 @@
13 13
 from django.forms.formsets import BaseFormSet, formset_factory
14 14
 from django.forms.util import ErrorList
15 15
 from django.forms.widgets import (SelectMultiple, HiddenInput,
16  
-    MultipleHiddenInput, media_property)
  16
+    MultipleHiddenInput, media_property, CheckboxSelectMultiple)
17 17
 from django.utils.encoding import smart_text, force_text
18 18
 from django.utils.datastructures import SortedDict
19 19
 from django.utils import six
@@ -1104,9 +1104,10 @@ def __init__(self, queryset, cache_choices=False, required=True,
1104 1104
         super(ModelMultipleChoiceField, self).__init__(queryset, None,
1105 1105
             cache_choices, required, widget, label, initial, help_text,
1106 1106
             *args, **kwargs)
1107  
-        if isinstance(self.widget, SelectMultiple):
  1107
+        # Remove this in Django 1.8
  1108
+        if isinstance(self.widget, SelectMultiple) and not isinstance(self.widget, CheckboxSelectMultiple):
1108 1109
             msg = _('Hold down "Control", or "Command" on a Mac, to select more than one.')
1109  
-            self.help_text = string_concat(self.help_text, ' ', msg) if self.help_text else msg
  1110
+            self.help_text = string_concat(self.help_text, ' ', msg)
1110 1111
 
1111 1112
     def clean(self, value):
1112 1113
         if self.required and not value:
8  docs/releases/1.6.txt
@@ -522,10 +522,10 @@ facilities together with Django built-in form :doc:`fields </ref/forms/fields>`
522 522
 and :doc:`widgets </ref/forms/widgets>` aren't affected but need to be aware of
523 523
 what's described in :ref:`m2m-help_text-deprecation` below.
524 524
 
525  
-This is because, as an temporary backward-compatible provision, the described
526  
-non-standard behavior has been preserved but moved to the model form field layer
527  
-and occurs only when the associated widget is
528  
-:class:`~django.forms.SelectMultiple` or a subclass.
  525
+This is because, as an ad-hoc temporary backward-compatibility provision, the
  526
+described non-standard behavior has been preserved but moved to the model form
  527
+field layer and occurs only when the associated widget is
  528
+:class:`~django.forms.SelectMultiple` or selected subclasses.
529 529
 
530 530
 QuerySet iteration
531 531
 ~~~~~~~~~~~~~~~~~~
12  tests/admin_widgets/tests.py
@@ -13,6 +13,7 @@
13 13
 from django.db.models import CharField, DateField
14 14
 from django.test import TestCase as DjangoTestCase
15 15
 from django.test.utils import override_settings
  16
+from django.utils import six
16 17
 from django.utils import translation
17 18
 from django.utils.html import conditional_escape
18 19
 from django.utils.unittest import TestCase
@@ -139,6 +140,17 @@ def testChoicesWithRadioFields(self):
139 140
     def testInheritance(self):
140 141
         self.assertFormfield(models.Album, 'backside_art', widgets.AdminFileWidget)
141 142
 
  143
+    def test_m2m_widgets(self):
  144
+        """m2m fields help text as it applies to admin app (#9321)."""
  145
+        class AdvisorAdmin(admin.ModelAdmin):
  146
+            filter_vertical=['companies']
  147
+
  148
+        self.assertFormfield(models.Advisor, 'companies', widgets.FilteredSelectMultiple,
  149
+                             filter_vertical=['companies'])
  150
+        ma = AdvisorAdmin(models.Advisor, admin.site)
  151
+        f = ma.formfield_for_dbfield(models.Advisor._meta.get_field('companies'), request=None)
  152
+        self.assertEqual(six.text_type(f.help_text), ' Hold down "Control", or "Command" on a Mac, to select more than one.')
  153
+
142 154
 
143 155
 @override_settings(PASSWORD_HASHERS=('django.contrib.auth.hashers.SHA1PasswordHasher',))
144 156
 class AdminFormfieldForDBFieldWithRequestTests(DjangoTestCase):
4  tests/model_forms/models.py
@@ -255,3 +255,7 @@ def __str__(self):
255 255
 class ColourfulItem(models.Model):
256 256
     name = models.CharField(max_length=50)
257 257
     colours = models.ManyToManyField(Colour)
  258
+
  259
+class ArticleStatusNote(models.Model):
  260
+    name = models.CharField(max_length=20)
  261
+    status = models.ManyToManyField(ArticleStatus)
35  tests/model_forms/tests.py
@@ -24,7 +24,7 @@
24 24
     DerivedPost, ExplicitPK, FlexibleDatePost, ImprovedArticle,
25 25
     ImprovedArticleWithParentLink, Inventory, Post, Price,
26 26
     Product, TextFile, AuthorProfile, Colour, ColourfulItem,
27  
-    test_images)
  27
+    ArticleStatusNote, test_images)
28 28
 
29 29
 if test_images:
30 30
     from .models import ImageFile, OptionalImageFile
@@ -234,6 +234,20 @@ class Meta:
234 234
         model = ColourfulItem
235 235
         fields = '__all__'
236 236
 
  237
+# model forms for testing work on #9321:
  238
+
  239
+class StatusNoteForm(forms.ModelForm):
  240
+    class Meta:
  241
+        model = ArticleStatusNote
  242
+        fields = '__all__'
  243
+
  244
+
  245
+class StatusNoteCBM2mForm(forms.ModelForm):
  246
+    class Meta:
  247
+        model = ArticleStatusNote
  248
+        fields = '__all__'
  249
+        widgets = {'status': forms.CheckboxSelectMultiple}
  250
+
237 251
 
238 252
 class ModelFormBaseTest(TestCase):
239 253
     def test_base_form(self):
@@ -1677,3 +1691,22 @@ def test_iterable_model_m2m(self) :
1677 1691
         <option value="%(blue_pk)s">Blue</option>
1678 1692
         </select> <span class="helptext"> Hold down "Control", or "Command" on a Mac, to select more than one.</span></p>"""
1679 1693
             % {'blue_pk': colour.pk})
  1694
+
  1695
+
  1696
+class M2mHelpTextTest(TestCase):
  1697
+    """Tests for ticket #9321."""
  1698
+    def test_multiple_widgets(self):
  1699
+        """Help text of different widgets for ManyToManyFields model fields"""
  1700
+        dreaded_help_text = '<span class="helptext"> Hold down "Control", or "Command" on a Mac, to select more than one.</span>'
  1701
+
  1702
+        # Default widget (SelectMultiple):
  1703
+        std_form = StatusNoteForm()
  1704
+        self.assertInHTML(dreaded_help_text, std_form.as_p())
  1705
+
  1706
+        # Overridden widget (CheckboxSelectMultiple, a subclass of
  1707
+        # SelectMultiple but with a UI that doesn't involve Control/Command
  1708
+        # keystrokes to extend selection):
  1709
+        form = StatusNoteCBM2mForm()
  1710
+        html = form.as_p()
  1711
+        self.assertInHTML('<ul id="id_status">', html)
  1712
+        self.assertInHTML(dreaded_help_text, html, count=0)

0 notes on commit 8c2fd05

Please sign in to comment.
Something went wrong with that request. Please try again.