Ticket #9321: 9321+14402.ManyToManyField-help-text.diff

django/contrib/admin/options.py

@@ -175 +175 @@
 
         if db_field.name in self.raw_id_fields:
             kwargs['widget'] = widgets.ManyToManyRawIdWidget(db_field.rel, using=db)
-            kwargs['help_text'] = ''
         elif db_field.name in (list(self.filter_vertical) + list(self.filter_horizontal)):
             kwargs['widget'] = widgets.FilteredSelectMultiple(db_field.verbose_name, (db_field.name in self.filter_vertical))
 

django/db/models/fields/related.py

@@ -9 +9 @@
 from django.db.models.query_utils import QueryWrapper
 from django.db.models.deletion import CASCADE
 from django.utils.encoding import smart_unicode
-from django.utils.translation import (ugettext_lazy as _, string_concat,
-    ungettext, ugettext)
+from django.utils.translation import ugettext_lazy as _, ungettext, ugettext
 from django.utils.functional import curry
 from django.core import exceptions
 from django import forms
@@ -1021 +1020 @@
 
         Field.__init__(self, **kwargs)
 
-        msg = _('Hold down "Control", or "Command" on a Mac, to select more than one.')
-        self.help_text = string_concat(self.help_text, ' ', msg)
-
     def get_choices_default(self):
         return Field.get_choices(self, include_blank=False)
 

django/forms/fields.py

@@ -80 +80 @@
             label = smart_unicode(label)
         self.required, self.label, self.initial = required, label, initial
         self.show_hidden_initial = show_hidden_initial
-        if help_text is None:
-            self.help_text = u''
-        else:
-            self.help_text = smart_unicode(help_text)
         widget = widget or self.widget
         if isinstance(widget, type):
             widget = widget()
@@ -103 +99 @@
 
         self.widget = widget
 
+        if help_text is None:
+            self.help_text = u''
+        else:
+            self.help_text = smart_unicode(help_text)
+
+        # Allow the widget to alter the help_text
+        self.help_text = self.widget.alter_help_text(self.help_text)
+
         # Increase the creation counter, and save our local copy.
         self.creation_counter = Field.creation_counter
         Field.creation_counter += 1

django/forms/widgets.py

@@ -12 +12 @@
 from django.conf import settings
 from django.utils.datastructures import MultiValueDict, MergeDict
 from django.utils.html import escape, conditional_escape
-from django.utils.translation import ugettext, ugettext_lazy
+from django.utils.translation import ugettext, ugettext_lazy, string_concat
 from django.utils.encoding import StrAndUnicode, force_unicode
-from django.utils.safestring import mark_safe
+from django.utils.safestring import mark_safe 
 from django.utils import datetime_safe, formats
 
 __all__ = (
@@ -155 +155 @@
         memo[id(self)] = obj
         return obj
 
+    def alter_help_text(self, help_text):
+        """
+        Usually called by the form field to potentially alter the help text.
+        """
+        return help_text
+    
     def render(self, name, value, attrs=None):
         """
         Returns this Widget rendered as HTML, as a Unicode string.
@@ -575 +581 @@
         return initial != data
 
 class SelectMultiple(Select):
+    
+    def alter_help_text(self, help_text):
+        extra_text = ugettext_lazy('Hold down "Control", or "Command" '
+                                   'on a Mac, to select more than one.')
+        return string_concat(help_text, ' ', extra_text)
+    
     def render(self, name, value, attrs=None, choices=()):
         if value is None: value = []
         final_attrs = self.build_attrs(attrs, name=name)
@@ -690 +702 @@
     id_for_label = classmethod(id_for_label)
 
 class CheckboxSelectMultiple(SelectMultiple):
+    
+    def alter_help_text(self, help_text):
+        # Do not show the 'Hold down "Control"' message that appears
+        # for SelectMultiple.
+        return help_text
+    
     def render(self, name, value, attrs=None, choices=()):
         if value is None: value = []
         has_id = attrs and 'id' in attrs

docs/ref/forms/widgets.txt

@@ -258 +258 @@
     <tr><th>Name:</th><td><input type="text" name="name" class="special"/></td></tr>
     <tr><th>Url:</th><td><input type="text" name="url"/></td></tr>
     <tr><th>Comment:</th><td><input type="text" name="comment" size="40"/></td></tr>
+
+Altering the help text
+----------------------
+
+.. versionadded:: 1.4
+
+In some cases you might want to customize a form field's help text depending
+on the widget used. To do so, simply override the widget's
+:meth:`~alter_help_text` method::
+
+    class MyCustomSelectMultiple(SelectMultiple):
+        
+        def alter_help_text(self, help_text):
+            extra_text = 'You can hold down "Control" to select multiple ones.'
+            return '%s %s' % (help_text, extra_text)

docs/releases/1.4.txt

@@ -60 +60 @@
 ``template.Library`` to ease the creation of template tags that store some
 data in a specified context variable.
 
+Widget.alter_help_text
+~~~~~~~~~~~~~~~~~~~~~~
+
+The :meth:`~alter_help_text` method was added to `:class:~forms.widgets.Widget`
+to allow a widget to customize the help text provided by a form field or a
+model field. This introduced a backwards incompatible change if you are
+defining a custom ``SelectMultiple`` widget. See `the notes on backwards
+incompatible changes`_ below to address this change.
+
+.. _the notes on backwards incompatible changes: backwards-incompatible-changes-1.4_
+
 .. _backwards-incompatible-changes-1.4:
 
 Backwards incompatible changes in 1.4
@@ -214 +225 @@
 
 Don't forget to escape characters that have a special meaning in a regular
 expression.
+
+Custom ``SelectMultiple`` widgets and ``alter_help_text``
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+The implementation of the new `meth:~Widget.alter_help_text` method was
+originally prompted by the fixing of a bug where the 'Hold down "Control", or
+"Command" on a Mac, to select more than one.' message would systematically be
+appended to a ``ManyToManyField``'s help text, even if that field were
+configured to use a different widget than ``SelectMultiple``. Thus, the
+action of appending that message was transfered from ``ManyToManyField`` to
+``SelectMultiple`` via the new ``alter_help_text`` method::
+
+    class SelectMultiple(Select):
+    
+        def alter_help_text(self, help_text):
+            extra_text = ugettext_lazy('Hold down "Control", or "Command" '
+                                       'on a Mac, to select more than one.')
+            return string_concat(help_text, ' ', extra_text)
+
+This means that if you are defining a widget inheriting from
+``SelectMultiple``, then that message will now systematically be appended to
+the help text, which may not make sense in the context of use for that widget.
+To cancel this new behaviour, simply override the ``alter_help_text`` method of
+your widget as is done, for example, by the
+`:class:~forms.widgets.CheckboxSelectMultiple` class::
+
+    class CheckboxSelectMultiple(SelectMultiple):
+        
+        def alter_help_text(self, help_text):
+            return help_text

tests/regressiontests/admin_widgets/models.py

@@ -67 +67 @@
     A single car tire. This to test that a user can only select their own cars.
     """
     car = models.ForeignKey(Car)
+
+class Director(models.Model):
+    name = models.CharField(max_length=40)
+
+    def __unicode__(self):
+        return self.name
+
+class Genre(models.Model):
+    name = models.CharField(max_length=40)
+
+    def __unicode__(self):
+        return self.name
+
+class Orchestra(models.Model):
+    name = models.CharField(max_length=100)
+    members = models.ManyToManyField(Member, help_text='This is the orchestra members help_text.')
+    director = models.ForeignKey(Director)
+    genres = models.ManyToManyField(Genre, help_text='This is the orchestra genres help_text.')
+
+    def __unicode__(self):
+        return self.name

tests/regressiontests/admin_widgets/tests.py

@@ -321 +321 @@
         self.assertEqual(w._has_changed([1, 2], [u'1']), True)
         self.assertEqual(w._has_changed([1, 2], [u'1', u'3']), True)
 
+        
+class ManyToManyRawIdWidgetHelpTextTest(DjangoTestCase):
+    """
+    Ensure that the help_text is displayed for M2M raw_id_fields. 
+    Refs #14402.
+    """
+    fixtures = ["admin-widgets-users.xml"]
+    admin_root = '/widget_admin'
+
+    def setUp(self):
+        self.client.login(username="super", password="secret")
+
+    def tearDown(self):
+        self.client.logout()
+
+    def test_m2m_has_helptext(self):
+        """Non raw_id m2m model field help_text shouldn't be affected by the fix for this ticket."""
+        response = self.client.get('%s/admin_widgets/orchestra/add/' % self.admin_root)
+        self.assert_("This is the orchestra genres help_text." in response.content)
+
+    def test_inline_m2m_has_helptext(self):
+        """Non raw_id m2m model field help_text shouldn't be affected by the fix for this ticket (inline case)."""
+        response = self.client.get('%s/admin_widgets/director/add/' % self.admin_root)
+        self.assert_("This is the orchestra genres help_text." in response.content)
+
+    def test_raw_id_m2m_has_helptext(self):
+        """raw_id m2m model field help_text shouldn't be ignored when displaying its admin widget."""
+        response = self.client.get('%s/admin_widgets/orchestra/add/' % self.admin_root)
+        self.assert_("This is the orchestra members help_text." in response.content)
+
+    def test_raw_id_inline_m2m_has_helptext(self):
+        """raw_id m2m model field help_text shouldn't be ignored when displaying the admin widget (inline case)."""
+        response = self.client.get('%s/admin_widgets/director/add/' % self.admin_root)
+        self.assert_("This is the orchestra members help_text." in response.content)
+        
 class RelatedFieldWidgetWrapperTests(DjangoTestCase):
     def test_no_can_add_related(self):
         rel = models.Inventory._meta.get_field('parent').rel

tests/regressiontests/admin_widgets/widgetadmin.py

@@ -22 +22 @@
 class EventAdmin(admin.ModelAdmin):
     raw_id_fields = ['band']
 
+class OrchestraAdmin(admin.ModelAdmin):
+    raw_id_fields = ['members']
+
+class OrchestraInline(admin.StackedInline):
+    model = models.Orchestra
+    raw_id_fields = ['members']
+
+class DirectorAdmin(admin.ModelAdmin):
+    inlines = [OrchestraInline]
+    
 site = WidgetAdmin(name='widget-admin')
 
 site.register(models.User)
 site.register(models.Car, CarAdmin)
 site.register(models.CarTire, CarTireAdmin)
 site.register(models.Event, EventAdmin)
+site.register(models.Orchestra, OrchestraAdmin)
+site.register(models.Director, DirectorAdmin)

tests/regressiontests/forms/tests/forms.py

@@ -1092 +1092 @@
 <option value="f" selected="selected">foo</option>
 <option value="b" selected="selected">bar</option>
 <option value="w">whiz</option>
-</select></li>""")
+</select> <span class="helptext"> Hold down "Control", or "Command" on a Mac, to select more than one.</span></li>""")
 
         # The 'initial' parameter is meaningless if you pass data.
         p = UserRegistration({}, initial={'username': initial_django, 'options': initial_options}, auto_id=False)
@@ -1102 +1102 @@
 <option value="f">foo</option>
 <option value="b">bar</option>
 <option value="w">whiz</option>
-</select></li>""")
+</select> <span class="helptext"> Hold down "Control", or "Command" on a Mac, to select more than one.</span></li>""")
         p = UserRegistration({'username': u''}, initial={'username': initial_django}, auto_id=False)
         self.assertEqual(p.as_ul(), """<li><ul class="errorlist"><li>This field is required.</li></ul>Username: <input type="text" name="username" maxlength="10" /></li>
 <li><ul class="errorlist"><li>This field is required.</li></ul>Password: <input type="password" name="password" /></li>
@@ -1110 +1110 @@
 <option value="f">foo</option>
 <option value="b">bar</option>
 <option value="w">whiz</option>
-</select></li>""")
+</select> <span class="helptext"> Hold down "Control", or "Command" on a Mac, to select more than one.</span></li>""")
         p = UserRegistration({'username': u'foo', 'options':['f','b']}, initial={'username': initial_django}, auto_id=False)
         self.assertEqual(p.as_ul(), """<li>Username: <input type="text" name="username" value="foo" maxlength="10" /></li>
 <li><ul class="errorlist"><li>This field is required.</li></ul>Password: <input type="password" name="password" /></li>
@@ -1118 +1118 @@
 <option value="f" selected="selected">foo</option>
 <option value="b" selected="selected">bar</option>
 <option value="w">whiz</option>
-</select></li>""")
+</select> <span class="helptext"> Hold down "Control", or "Command" on a Mac, to select more than one.</span></li>""")
 
         # A callable 'initial' value is *not* used as a fallback if data is not provided.
         # In this example, we don't provide a value for 'username', and the form raises a
@@ -1141 +1141 @@
 <option value="f">foo</option>
 <option value="b" selected="selected">bar</option>
 <option value="w" selected="selected">whiz</option>
-</select></li>""")
+</select> <span class="helptext"> Hold down "Control", or "Command" on a Mac, to select more than one.</span></li>""")
         p = UserRegistration(initial={'username': initial_stephane, 'options': initial_options}, auto_id=False)
         self.assertEqual(p.as_ul(), """<li>Username: <input type="text" name="username" value="stephane" maxlength="10" /></li>
 <li>Password: <input type="password" name="password" /></li>
@@ -1149 +1149 @@
 <option value="f" selected="selected">foo</option>
 <option value="b" selected="selected">bar</option>
 <option value="w">whiz</option>
-</select></li>""")
+</select> <span class="helptext"> Hold down "Control", or "Command" on a Mac, to select more than one.</span></li>""")
 
     def test_boundfield_values(self):
         # It's possible to get to the value which would be used for rendering

tests/regressiontests/model_forms_regress/tests.py

@@ -3 +3 @@
 from django import forms
 from django.core.exceptions import FieldError, ValidationError
 from django.core.files.uploadedfile import SimpleUploadedFile
+from django.forms.widgets import CheckboxSelectMultiple
 from django.forms.models import (modelform_factory, ModelChoiceField,
     fields_for_model, construct_instance)
 from django.utils import unittest
@@ -140 +141 @@
 </select> <span class="helptext"> Hold down "Control", or "Command" on a Mac, to select more than one.</span></li>"""
             % (book1.pk, book2.pk, book3.pk))
 
+class ManyToManyHelpTextTests(TestCase):
+    
+    def test_dont_display_hold_down_command_help_text(self):
+        """
+        Ensures that the 'Hold down "Control"' message is not systematically displayed for
+        a M2M field if it does not use the default SelectMultiple widget.
+        Refs #9321.
+        """
+        # Override the widget and help_text
+        def formfield_for_dbfield(db_field, **kwargs):
+            if db_field.name == 'publications':
+                kwargs['widget'] = CheckboxSelectMultiple
+            return db_field.formfield(**kwargs)
+
+        # Set up some Publications to use as data
+        book1 = Publication.objects.create(title="First Book", date_published=date(2007,1,1))
+        book2 = Publication.objects.create(title="Second Book", date_published=date(2008,1,1))
+
+        # Create a ModelForm, instantiate it, and check that the output is as expected
+        ModelForm = modelform_factory(Article, formfield_callback=formfield_for_dbfield)
+        form = ModelForm()
+        self.assertEqual(unicode(form['publications'].help_text), u'')
+        
 class CFFForm(forms.ModelForm):
     class Meta:
         model = CustomFF

tests/regressiontests/model_formsets_regress/tests.py

@@ -228 +228 @@
             self.assertTrue(isinstance(form.errors, ErrorDict))
             self.assertTrue(isinstance(form.non_field_errors(), ErrorList))
 
-class CustomWidget(forms.CharField):
+class CustomWidget(forms.TextInput):
     pass