Skip to content

Commit

Permalink
Improve field value handling in HTML form output
Browse files Browse the repository at this point in the history
Let each form field's HTML template decide if it needs native or
string-ified field value, instead of forcing all templates to always
work only with unnecessarily restrictive string representation.

Fixes encode#4120, encode#4121
  • Loading branch information
arkadini committed May 15, 2016
1 parent 2cb7157 commit dc0442c
Show file tree
Hide file tree
Showing 12 changed files with 21 additions and 31 deletions.
3 changes: 0 additions & 3 deletions rest_framework/renderers.py
Original file line number Diff line number Diff line change
Expand Up @@ -319,9 +319,6 @@ def render_field(self, field, parent_style):
style['template_pack'] = parent_style.get('template_pack', self.template_pack)
style['renderer'] = self

# Get a clone of the field with text-only value representation.
field = field.as_form_field()

if style.get('input_type') == 'datetime-local' and isinstance(field.value, six.text_type):
field.value = field.value.rstrip('Z')

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
{% endif %}

<div class="col-sm-10">
<input name="{{ field.name }}" {% if style.input_type != "file" %}class="form-control"{% endif %} type="{{ style.input_type }}" {% if style.placeholder %}placeholder="{{ style.placeholder }}"{% endif %} {% if field.value %}value="{{ field.value }}"{% endif %}>
<input name="{{ field.name }}" {% if style.input_type != "file" %}class="form-control"{% endif %} type="{{ style.input_type }}" {% if style.placeholder %}placeholder="{{ style.placeholder }}"{% endif %} {% if field.string_value %}value="{{ field.string_value }}"{% endif %}>

{% if field.errors %}
{% for error in field.errors %}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
<div class="col-sm-10">
<select class="form-control" name="{{ field.name }}">
{% if field.allow_null or field.allow_blank %}
<option value="" {% if not field.value %}selected{% endif %}>--------</option>
<option value="" {% if not field.string_value %}selected{% endif %}>--------</option>
{% endif %}
{% for select in field.iter_options %}
{% if select.start_option_group %}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
{% endif %}

<div class="col-sm-10">
<textarea name="{{ field.name }}" class="form-control" {% if style.placeholder %}placeholder="{{ style.placeholder }}"{% endif %} {% if style.rows %}rows="{{ style.rows }}"{% endif %}>{% if field.value %}{{ field.value }}{% endif %}</textarea>
<textarea name="{{ field.name }}" class="form-control" {% if style.placeholder %}placeholder="{{ style.placeholder }}"{% endif %} {% if style.rows %}rows="{{ style.rows }}"{% endif %}>{% if field.string_value %}{{ field.string_value }}{% endif %}</textarea>

{% if field.errors %}
{% for error in field.errors %}
Expand Down
2 changes: 1 addition & 1 deletion rest_framework/templates/rest_framework/inline/input.html
Original file line number Diff line number Diff line change
Expand Up @@ -5,5 +5,5 @@
</label>
{% endif %}

<input name="{{ field.name }}" {% if style.input_type != "file" %}class="form-control"{% endif %} type="{{ style.input_type }}" {% if style.placeholder %}placeholder="{{ style.placeholder }}"{% endif %} {% if field.value %}value="{{ field.value }}"{% endif %}>
<input name="{{ field.name }}" {% if style.input_type != "file" %}class="form-control"{% endif %} type="{{ style.input_type }}" {% if style.placeholder %}placeholder="{{ style.placeholder }}"{% endif %} {% if field.string_value %}value="{{ field.string_value }}"{% endif %}>
</div>
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 @@ -7,7 +7,7 @@

<select class="form-control" name="{{ field.name }}">
{% if field.allow_null or field.allow_blank %}
<option value="" {% if not field.value %}selected{% endif %}>--------</option>
<option value="" {% if not field.string_value %}selected{% endif %}>--------</option>
{% endif %}
{% for select in field.iter_options %}
{% if select.start_option_group %}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,5 +5,5 @@
</label>
{% endif %}

<input name="{{ field.name }}" type="text" class="form-control" {% if style.placeholder %}placeholder="{{ style.placeholder }}"{% endif %} {% if field.value %}value="{{ field.value }}"{% endif %}>
<input name="{{ field.name }}" type="text" class="form-control" {% if style.placeholder %}placeholder="{{ style.placeholder }}"{% endif %} {% if field.string_value %}value="{{ field.string_value }}"{% endif %}>
</div>
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
<label {% if style.hide_label %}class="sr-only"{% endif %}>{{ field.label }}</label>
{% endif %}

<input name="{{ field.name }}" {% if style.input_type != "file" %}class="form-control"{% endif %} type="{{ style.input_type }}" {% if style.placeholder %}placeholder="{{ style.placeholder }}"{% endif %} {% if field.value %}value="{{ field.value }}"{% endif %}>
<input name="{{ field.name }}" {% if style.input_type != "file" %}class="form-control"{% endif %} type="{{ style.input_type }}" {% if style.placeholder %}placeholder="{{ style.placeholder }}"{% endif %} {% if field.string_value %}value="{{ field.string_value }}"{% endif %}>

{% if field.errors %}
{% for error in field.errors %}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

<select class="form-control" name="{{ field.name }}">
{% if field.allow_null or field.allow_blank %}
<option value="" {% if not field.value %}selected{% endif %}>--------</option>
<option value="" {% if not field.string_value %}selected{% endif %}>--------</option>
{% endif %}
{% for select in field.iter_options %}
{% if select.start_option_group %}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
</label>
{% endif %}

<textarea name="{{ field.name }}" class="form-control" {% if style.placeholder %}placeholder="{{ style.placeholder }}"{% endif %} {% if style.rows %}rows="{{ style.rows }}"{% endif %}>{% if field.value %}{{ field.value }}{% endif %}</textarea>
<textarea name="{{ field.name }}" class="form-control" {% if style.placeholder %}placeholder="{{ style.placeholder }}"{% endif %} {% if style.rows %}rows="{{ style.rows }}"{% endif %}>{% if field.string_value %}{{ field.string_value }}{% endif %}</textarea>

{% if field.errors %}
{% for error in field.errors %}<span class="help-block">{{ error }}</span>{% endfor %}
Expand Down
19 changes: 6 additions & 13 deletions rest_framework/utils/serializer_helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -72,15 +72,17 @@ def __getattr__(self, attr_name):
def _proxy_class(self):
return self._field.__class__

@property
def string_value(self):
if self.value is None or self.value is False:
return ''
return force_text(self.value)

def __repr__(self):
return unicode_to_repr('<%s value=%s errors=%s>' % (
self.__class__.__name__, self.value, self.errors
))

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)


class NestedBoundField(BoundField):
"""
Expand All @@ -106,15 +108,6 @@ def __getitem__(self, key):
return NestedBoundField(field, value, error, prefix=self.name + '.')
return BoundField(field, value, error, prefix=self.name + '.')

def as_form_field(self):
values = {}
for key, value in self.value.items():
if isinstance(value, (list, dict)):
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)


class BindingDict(collections.MutableMapping):
"""
Expand Down
12 changes: 6 additions & 6 deletions tests/test_bound_fields.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,15 +45,15 @@ class ExampleSerializer(serializers.Serializer):
assert serializer['amount'].errors is None
assert serializer['amount'].name == 'amount'

def test_as_form_fields(self):
def test_string_value(self):
class ExampleSerializer(serializers.Serializer):
bool_field = serializers.BooleanField()
null_field = serializers.IntegerField(allow_null=True)

serializer = ExampleSerializer(data={'bool_field': False, 'null_field': None})
assert serializer.is_valid()
assert serializer['bool_field'].as_form_field().value == ''
assert serializer['null_field'].as_form_field().value == ''
assert serializer['bool_field'].string_value == ''
assert serializer['null_field'].string_value == ''


class TestNestedBoundField:
Expand All @@ -78,7 +78,7 @@ class ExampleSerializer(serializers.Serializer):
assert serializer['nested']['amount'].errors is None
assert serializer['nested']['amount'].name == 'nested.amount'

def test_as_form_fields(self):
def test_string_value(self):
class Nested(serializers.Serializer):
bool_field = serializers.BooleanField()
null_field = serializers.IntegerField(allow_null=True)
Expand All @@ -88,8 +88,8 @@ class ExampleSerializer(serializers.Serializer):

serializer = ExampleSerializer(data={'nested': {'bool_field': False, 'null_field': None}})
assert serializer.is_valid()
assert serializer['nested']['bool_field'].as_form_field().value == ''
assert serializer['nested']['null_field'].as_form_field().value == ''
assert serializer['nested']['bool_field'].string_value == ''
assert serializer['nested']['null_field'].string_value == ''

def test_rendering_nested_fields_with_none_value(self):
from rest_framework.renderers import HTMLFormRenderer
Expand Down

0 comments on commit dc0442c

Please sign in to comment.