Resolve form display with ChoiceField, MultipleChoiceField and non-string choices. (#4374)

* Add tests for html-form-rendering choice fields
* Resolve issues with ChoiceField, MultipleChoiceField and non-string options
* Ensure None template comparisons don't match string None

rest_framework/templates/rest_framework/horizontal/checkbox_multiple.html

@@ -1,3 +1,5 @@
+{% load rest_framework %}
+
 <div class="form-group">
   {% if field.label %}
     <label class="col-sm-2 control-label {% if style.hide_label %}sr-only{% endif %}">
@@ -9,7 +11,7 @@
     {% if style.inline %}
       {% for key, text in field.choices.items %}
         <label class="checkbox-inline">
-          <input type="checkbox" name="{{ field.name }}" value="{{ key }}" {% if key in field.value %}checked{% endif %}>
+          <input type="checkbox" name="{{ field.name }}" value="{{ key }}" {% if key|as_string in field.value|as_list_of_strings %}checked{% endif %}>
           {{ text }}
         </label>
       {% endfor %}
@@ -17,7 +19,7 @@
       {% for key, text in field.choices.items %}
         <div class="checkbox">
           <label>
-            <input type="checkbox" name="{{ field.name }}" value="{{ key }}" {% if key in field.value %}checked{% endif %}>
+            <input type="checkbox" name="{{ field.name }}" value="{{ key }}" {% if key|as_string in field.value|as_list_of_strings %}checked{% endif %}>
             {{ text }}
           </label>
         </div>

rest_framework/templates/rest_framework/horizontal/radio.html

@@ -1,4 +1,6 @@
 {% load i18n %}
+{% load rest_framework %}
+
 {% trans "None" as none_choice %}
 
 <div class="form-group">
@@ -19,7 +21,7 @@
 
       {% for key, text in field.choices.items %}
         <label class="radio-inline">
-          <input type="radio" name="{{ field.name }}" value="{{ key }}" {% if key == field.value %}checked{% endif %} />
+          <input type="radio" name="{{ field.name }}" value="{{ key }}" {% if key|as_string == field.value|as_string %}checked{% endif %} />
           {{ text }}
         </label>
       {% endfor %}
@@ -35,7 +37,7 @@
         {% for key, text in field.choices.items %}
           <div class="radio">
             <label>
-              <input type="radio" name="{{ field.name }}" value="{{ key }}" {% if key == field.value %}checked{% endif %} />
+              <input type="radio" name="{{ field.name }}" value="{{ key }}" {% if key|as_string == field.value|as_string %}checked{% endif %} />
               {{ text }}
             </label>
           </div>

rest_framework/templates/rest_framework/horizontal/select.html

@@ -1,3 +1,5 @@
+{% load rest_framework %}
+
 <div class="form-group">
   {% if field.label %}
     <label class="col-sm-2 control-label {% if style.hide_label %}sr-only{% endif %}">
@@ -16,7 +18,7 @@
           {% elif select.end_option_group %}
             </optgroup>
           {% else %}
-            <option value="{{ select.value }}" {% if select.value == field.value %}selected{% endif %} {% if select.disabled %}disabled{% endif %}>{{ select.display_text }}</option>
+            <option value="{{ select.value }}" {% if select.value|as_string == field.value|as_string %}selected{% endif %} {% if select.disabled %}disabled{% endif %}>{{ select.display_text }}</option>
           {% endif %}
       {% endfor %}
     </select>

rest_framework/templates/rest_framework/horizontal/select_multiple.html

@@ -1,4 +1,6 @@
 {% load i18n %}
+{% load rest_framework %}
+
 {% trans "No items to select." as no_items %}
 
 <div class="form-group">
@@ -16,7 +18,7 @@
         {% elif select.end_option_group %}
           </optgroup>
         {% else %}
-          <option value="{{ select.value }}" {% if select.value in field.value %}selected{% endif %} {% if select.disabled %}disabled{% endif %}>{{ select.display_text }}</option>
+          <option value="{{ select.value }}" {% if select.value|as_string in field.value|as_list_of_strings %}selected{% endif %} {% if select.disabled %}disabled{% endif %}>{{ select.display_text }}</option>
         {% endif %}
       {% empty %}
           <option>{{ no_items }}</option>

rest_framework/templates/rest_framework/inline/checkbox_multiple.html

@@ -1,3 +1,5 @@
+{% load rest_framework %}
+
 <div class="form-group {% if field.errors %}has-error{% endif %}">
   {% if field.label %}
     <label class="sr-only">{{ field.label }}</label>
@@ -6,7 +8,7 @@
   {% for key, text in field.choices.items %}
     <div class="checkbox">
       <label>
-        <input type="checkbox" name="{{ field.name }}" value="{{ key }}" {% if key in field.value %}checked{% endif %}>
+        <input type="checkbox" name="{{ field.name }}" value="{{ key }}" {% if key|as_string in field.value|as_list_of_strings %}checked{% endif %}>
         {{ text }}
       </label>
     </div>

rest_framework/templates/rest_framework/inline/radio.html

@@ -1,4 +1,5 @@
 {% load i18n %}
+{% load rest_framework %}
 {% trans "None" as none_choice %}
 
 <div class="form-group {% if field.errors %}has-error{% endif %}">
@@ -20,7 +21,7 @@
   {% for key, text in field.choices.items %}
     <div class="radio">
       <label>
-        <input type="radio" name="{{ field.name }}" value="{{ key }}" {% if key == field.value %}checked{% endif %}>
+        <input type="radio" name="{{ field.name }}" value="{{ key }}" {% if key|as_string == field.value|as_string %}checked{% endif %}>
         {{ text }}
       </label>
     </div>

rest_framework/templates/rest_framework/inline/select.html

@@ -1,3 +1,5 @@
+{% load rest_framework %}
+
 <div class="form-group {% if field.errors %}has-error{% endif %}">
   {% if field.label %}
     <label class="sr-only">
@@ -15,7 +17,7 @@
         {% elif select.end_option_group %}
           </optgroup>
         {% else %}
-          <option value="{{ select.value }}" {% if select.value == field.value %}selected{% endif %} {% if select.disabled %}disabled{% endif %}>{{ select.display_text }}</option>
+          <option value="{{ select.value }}" {% if select.value|as_string == field.value|as_string %}selected{% endif %} {% if select.disabled %}disabled{% endif %}>{{ select.display_text }}</option>
         {% endif %}
     {% endfor %}
   </select>

rest_framework/templates/rest_framework/inline/select_multiple.html

@@ -1,4 +1,5 @@
 {% load i18n %}
+{% load rest_framework %}
 {% trans "No items to select." as no_items %}
 
 <div class="form-group {% if field.errors %}has-error{% endif %}">
@@ -15,7 +16,7 @@
           {% elif select.end_option_group %}
             </optgroup>
           {% else %}
-            <option value="{{ select.value }}" {% if select.value in field.value %}selected{% endif %} {% if select.disabled %}disabled{% endif %}>{{ select.display_text }}</option>
+            <option value="{{ select.value }}" {% if select.value|as_string in field.value|as_list_of_strings %}selected{% endif %} {% if select.disabled %}disabled{% endif %}>{{ select.display_text }}</option>
           {% endif %}
       {% empty %}
       <option>{{ no_items }}</option>

rest_framework/templates/rest_framework/vertical/checkbox_multiple.html

@@ -1,3 +1,5 @@
+{% load rest_framework %}
+
 <div class="form-group {% if field.errors %}has-error{% endif %}">
   {% if field.label %}
     <label {% if style.hide_label %}class="sr-only"{% endif %}>{{ field.label }}</label>
@@ -7,7 +9,7 @@
     <div>
       {% for key, text in field.choices.items %}
         <label class="checkbox-inline">
-          <input type="checkbox" name="{{ field.name }}" value="{{ key }}" {% if key in field.value %}checked{% endif %}>
+          <input type="checkbox" name="{{ field.name }}" value="{{ key }}" {% if key|as_string in field.value|as_list_of_stringsg %}checked{% endif %}>
             {{ text }}
         </label>
       {% endfor %}
@@ -16,7 +18,7 @@
     {% for key, text in field.choices.items %}
       <div class="checkbox">
         <label>
-          <input type="checkbox" name="{{ field.name }}" value="{{ key }}" {% if key in field.value %}checked{% endif %}>
+          <input type="checkbox" name="{{ field.name }}" value="{{ key }}" {% if key|as_string in field.value|as_list_of_stringsg %}checked{% endif %}>
           {{ text }}
         </label>
       </div>

rest_framework/templates/rest_framework/vertical/radio.html

@@ -1,4 +1,5 @@
 {% load i18n %}
+{% load rest_framework %}
 {% trans "None" as none_choice %}
 
 <div class="form-group {% if field.errors %}has-error{% endif %}">
@@ -19,7 +20,7 @@
 
         {% for key, text in field.choices.items %}
           <label class="radio-inline">
-            <input type="radio" name="{{ field.name }}" value="{{ key }}" {% if key == field.value %}checked{% endif %}>
+            <input type="radio" name="{{ field.name }}" value="{{ key }}" {% if key|as_string == field.value|as_string %}checked{% endif %}>
             {{ text }}
           </label>
         {% endfor %}
@@ -37,7 +38,7 @@
       {% for key, text in field.choices.items %}
         <div class="radio">
           <label>
-            <input type="radio" name="{{ field.name }}" value="{{ key }}" {% if key == field.value %}checked{% endif %}>
+            <input type="radio" name="{{ field.name }}" value="{{ key }}" {% if key|as_string == field.value|as_string %}checked{% endif %}>
             {{ text }}
           </label>
         </div>

rest_framework/templates/rest_framework/vertical/select.html

@@ -1,3 +1,5 @@
+{% load rest_framework %}
+
 <div class="form-group {% if field.errors %}has-error{% endif %}">
   {% if field.label %}
     <label {% if style.hide_label %}class="sr-only"{% endif %}>
@@ -15,7 +17,7 @@
         {% elif select.end_option_group %}
           </optgroup>
         {% else %}
-          <option value="{{ select.value }}" {% if select.value == field.value %}selected{% endif %} {% if select.disabled %}disabled{% endif %}>{{ select.display_text }}</option>
+          <option value="{{ select.value }}" {% if select.value|as_string == field.value|as_string %}selected{% endif %} {% if select.disabled %}disabled{% endif %}>{{ select.display_text }}</option>
         {% endif %}
     {% endfor %}
   </select>

rest_framework/templates/rest_framework/vertical/select_multiple.html

@@ -1,4 +1,5 @@
 {% load i18n %}
+{% load rest_framework %}
 {% trans "No items to select." as no_items %}
 
 <div class="form-group {% if field.errors %}has-error{% endif %}">
@@ -15,7 +16,7 @@
         {% elif select.end_option_group %}
           </optgroup>
         {% else %}
-          <option value="{{ select.value }}" {% if select.value in field.value %}selected{% endif %} {% if select.disabled %}disabled{% endif %}>{{ select.display_text }}</option>
+          <option value="{{ select.value }}" {% if select.value|as_string in field.value|as_list_of_strings %}selected{% endif %} {% if select.disabled %}disabled{% endif %}>{{ select.display_text }}</option>
         {% endif %}
     {% empty %}
         <option>{{ no_items }}</option>

rest_framework/templatetags/rest_framework.py

@@ -89,6 +89,21 @@ def add_query_param(request, key, val):
     return escape(replace_query_param(uri, key, val))
 
 
+@register.filter
+def as_string(value):
+    if value is None:
+        return ''
+    return '%s' % value
+
+
+@register.filter
+def as_list_of_strings(value):
+    return [
+        '' if (item is None) else ('%s' % item)
+        for item in value
+    ]
+
+
 @register.filter
 def add_class(value, css_class):
     """

rest_framework/utils/serializer_helpers.py

@@ -78,7 +78,7 @@ class BoundField(object):
         ))
 
     def as_form_field(self):
-        value = '' if (self.value is None or self.value is False) else force_text(self.value)
+        value = '' if (self.value is None or self.value is False) else self.value
         return self.__class__(self._field, value, self.errors, self._prefix)
 
 

tests/test_renderers.py

@@ -481,3 +481,90 @@ class TestHTMLFormRenderer(TestCase):
         result = renderer.render(self.serializer.data, None, {})
 
         self.assertIsInstance(result, SafeText)
+
+
+class TestChoiceFieldHTMLFormRenderer(TestCase):
+    """
+    Test rendering ChoiceField with HTMLFormRenderer.
+    """
+
+    def setUp(self):
+        choices = ((1, 'Option1'), (2, 'Option2'), (12, 'Option12'))
+
+        class TestSerializer(serializers.Serializer):
+            test_field = serializers.ChoiceField(choices=choices,
+                                                 initial=2)
+
+        self.TestSerializer = TestSerializer
+        self.renderer = HTMLFormRenderer()
+
+    def test_render_initial_option(self):
+        serializer = self.TestSerializer()
+        result = self.renderer.render(serializer.data)
+
+        self.assertIsInstance(result, SafeText)
+
+        self.assertInHTML('<option value="2" selected>Option2</option>',
+                          result)
+        self.assertInHTML('<option value="1">Option1</option>', result)
+        self.assertInHTML('<option value="12">Option12</option>', result)
+
+    def test_render_selected_option(self):
+        serializer = self.TestSerializer(data={'test_field': '12'})
+
+        serializer.is_valid()
+        result = self.renderer.render(serializer.data)
+
+        self.assertIsInstance(result, SafeText)
+
+        self.assertInHTML('<option value="12" selected>Option12</option>',
+                          result)
+        self.assertInHTML('<option value="1">Option1</option>', result)
+        self.assertInHTML('<option value="2">Option2</option>', result)
+
+
+class TestMultipleChoiceFieldHTMLFormRenderer(TestCase):
+    """
+    Test rendering MultipleChoiceField with HTMLFormRenderer.
+    """
+
+    def setUp(self):
+        self.renderer = HTMLFormRenderer()
+
+    def test_render_selected_option_with_string_option_ids(self):
+        choices = (('1', 'Option1'), ('2', 'Option2'), ('12', 'Option12'),
+                   ('}', 'OptionBrace'))
+
+        class TestSerializer(serializers.Serializer):
+            test_field = serializers.MultipleChoiceField(choices=choices)
+
+        serializer = TestSerializer(data={'test_field': ['12']})
+        serializer.is_valid()
+
+        result = self.renderer.render(serializer.data)
+
+        self.assertIsInstance(result, SafeText)
+
+        self.assertInHTML('<option value="12" selected>Option12</option>',
+                          result)
+        self.assertInHTML('<option value="1">Option1</option>', result)
+        self.assertInHTML('<option value="2">Option2</option>', result)
+        self.assertInHTML('<option value="}">OptionBrace</option>', result)
+
+    def test_render_selected_option_with_integer_option_ids(self):
+        choices = ((1, 'Option1'), (2, 'Option2'), (12, 'Option12'))
+
+        class TestSerializer(serializers.Serializer):
+            test_field = serializers.MultipleChoiceField(choices=choices)
+
+        serializer = TestSerializer(data={'test_field': ['12']})
+        serializer.is_valid()
+
+        result = self.renderer.render(serializer.data)
+
+        self.assertIsInstance(result, SafeText)
+
+        self.assertInHTML('<option value="12" selected>Option12</option>',
+                          result)
+        self.assertInHTML('<option value="1">Option1</option>', result)
+        self.assertInHTML('<option value="2">Option2</option>', result)