Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve HTML form rendering of choice fields #4137

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,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 == field.basic_value %}selected{% endif %} {% if select.disabled %}disabled{% endif %}>{{ select.display_text }}</option>
{% endif %}
{% endfor %}
</select>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,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 in field.basic_value %}selected{% endif %} {% if select.disabled %}disabled{% endif %}>{{ select.display_text }}</option>
{% endif %}
{% empty %}
<option>{{ no_items }}</option>
Expand Down
2 changes: 1 addition & 1 deletion rest_framework/templates/rest_framework/inline/select.html
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,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 == field.basic_value %}selected{% endif %} {% if select.disabled %}disabled{% endif %}>{{ select.display_text }}</option>
{% endif %}
{% endfor %}
</select>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,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 in field.basic_value %}selected{% endif %} {% if select.disabled %}disabled{% endif %}>{{ select.display_text }}</option>
{% endif %}
{% empty %}
<option>{{ no_items }}</option>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,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 == field.basic_value %}selected{% endif %} {% if select.disabled %}disabled{% endif %}>{{ select.display_text }}</option>
{% endif %}
{% endfor %}
</select>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,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 in field.basic_value %}selected{% endif %} {% if select.disabled %}disabled{% endif %}>{{ select.display_text }}</option>
{% endif %}
{% empty %}
<option>{{ no_items }}</option>
Expand Down
18 changes: 13 additions & 5 deletions rest_framework/utils/serializer_helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,10 @@
from rest_framework.compat import unicode_to_repr


# A singleton helper value to detect unset keyword arguments
unset = object()


class ReturnDict(OrderedDict):
"""
Return object from `serialier.data` for the `Serializer` class.
Expand Down Expand Up @@ -58,10 +62,11 @@ class BoundField(object):
providing an API similar to Django forms and form fields.
"""

def __init__(self, field, value, errors, prefix=''):
def __init__(self, field, value, errors, prefix='', basic_value=unset):
self._field = field
self._prefix = prefix
self.value = value
self.basic_value = value if basic_value is unset else basic_value
self.errors = errors
self.name = prefix + self.field_name

Expand All @@ -79,7 +84,8 @@ def __repr__(self):

def as_form_field(self):
value = '' if (self.value is None or self.value is False) else force_text(self.value)
return self.__class__(self._field, value, self.errors, self._prefix)
return self.__class__(self._field, value, self.errors, self._prefix,
self.basic_value)


class NestedBoundField(BoundField):
Expand All @@ -89,10 +95,11 @@ class NestedBoundField(BoundField):
`BoundField` that is used for serializer fields.
"""

def __init__(self, field, value, errors, prefix=''):
def __init__(self, field, value, errors, prefix='', basic_value=unset):
if value is None or value is '':
value = {}
super(NestedBoundField, self).__init__(field, value, errors, prefix)
super(NestedBoundField, self).__init__(field, value, errors, prefix,
basic_value)

def __iter__(self):
for field in self.fields.values():
Expand All @@ -113,7 +120,8 @@ def as_form_field(self):
values[key] = value
else:
values[key] = '' if (value is None or value is False) else force_text(value)
return self.__class__(self._field, values, self.errors, self._prefix)
return self.__class__(self._field, values, self.errors, self._prefix,
self.basic_value)


class BindingDict(collections.MutableMapping):
Expand Down
87 changes: 87 additions & 0 deletions tests/test_renderers.py
Original file line number Diff line number Diff line change
Expand Up @@ -485,3 +485,90 @@ def test_render_with_provided_args(self):
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)