Fix 0 value IntegerField in TemplateHTMLRenderer

[nikhil96sher]

Signed-off-by: Nikhil Sheoran [email protected]

Note: Before submitting this pull request, please review our contributing guidelines.

Description

Fixes #5767
The current implementation of TemplateHTMLRenderer's input.html for various template packs checked {% if field.value %} which gave incorrect validation for IntegerField with 0 as input. This PR fixes it by checking {% if field.value is not None and field.value != "" %}

[nikhil96sher]

The second check is required because utils.serializer_helpers.BoundField.as_field_error() converts None to '' (caught by TestNestedBoundField.test_rendering_nested_fields_with_none_value )

[xordoquy]

I need to ensure this won't have side effets on non integer fields that rely on that template.

[nikhil96sher]

@xordoquy Should I also include tests in the PR for the remaining non-integer fields that uses the input.html?

[kgeorgy]

We can list which falsy expressions led to no "value" rendering before:

• None => In the current solution: no "value" attribute : OK
• False => In the current solution: ? Is there case where we could have a boolean value rendered with an html input? In the current solution there will be a "False" text as value. (OK?)
• zero of any numeric type, for example, 0, 0L, 0.0, 0j: value is rendered as 0: OK
• any empty sequence, for example, '', (), []: empty string => no value : OK. What should we have for [] and ()?
• any empty mapping, for example, {}: Mapping in the value (OK?)

Maybe, to be sure to only have impact when the value is a numeric 0 value should we test :

{% if field.value == 0 or field.value %}...{% endif %} (Absolutely not tested, from scratch coding) ?

[tomchristie]

I'd suggest we just change it to {% if field.value is not None %}
At that point I'd consider this fixed.

We can list which falsy expressions led to no "value" rendering before:

I don't really care about what False, (), {} render to in there, since they shouldn't map on to text inputs anyways, and we can't properly represent them when they do.

[nikhil96sher]

@tomchristie As mentioned in the description as well, the second check is required because utils.serializer_helpers.BoundField.as_field_error() converts None to '' (caught by the test TestNestedBoundField.test_rendering_nested_fields_with_none_value )
@kgeorgy A mapping wouldn't render on the input.html template, but instead uses the fieldset.html template and recursively calls the parser for each key of the map.

[kgeorgy]

@tomchristie It makes sense to me too. I just wanted to list all the possible impacts the change has.
@nikhil96sher I know a mapping/list/ or other object is not intended to be rendered in a simple html input field, this is just to consider all the cases, and decide what to do with. A "we don't care" answer here is completely OK for me :)

[nikhil96sher]

@tomchristie Any updates on this?

[carltongibson]

I'd suggest we just change it to {% if field.value is not None %}
At that point I'd consider this fixed.

If the PR is updated (to the simpler solution) it should be good to go.

[carltongibson]

Closing in favour of #5834. Thanks @nikhil96sher!