[Django] #37364: Make NullBooleanSelect.format_value() return a list, consistent with ChoiceWidget

[Django]

#37364: Make NullBooleanSelect.format_value() return a list, consistent with
ChoiceWidget
-------------------------------------+-------------------------------------
Reporter: Brian Helba | Type:
| Cleanup/optimization
Status: new | Component: Forms
Version: dev | Severity: Normal
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
`ChoiceWidget.format_value()` is documented (in its docstring) to "Return
selected values as a list", and every `ChoiceWidget` subclass honors that,
except `NullBooleanSelect`, which returns a bare string: `"unknown"`,
`"true"`, or `"false"`.

`NullBooleanSelect` does not override `get_context()` or `optgroups()`. It
relies on the inherited `ChoiceWidget.optgroups()`, which marks an option
as selected with `str(subvalue) in value`. When `value` is a list that is
list membership. When `value` is the bare string it becomes a substring
test, which only produces correct results because none of `"unknown"`,
`"true"`, or `"false"` happens to be a substring of another. The parent's
contract is satisfied by accident rather than by design, and it may be
confusing for anyone who later customizes the choices or the parent
implementation.

----

History: #27866 (1.11) changed `ChoiceWidget.format_value()` from
returning a set to returning a list but did not touch `NullBooleanSelect`.
#17210 (2.2) changed the `NullBooleanSelect` mapping from `1/2/3` to
`unknown/true/false` and kept returning a bare string.

----

Proposed change: wrap the result in a one-element list, i.e.
`["unknown"]`, `["true"]`, or `["false"]`. The rendered HTML should be
identical. The only impact would be for third-party code that calls
`widget.format_value()` directly and uses the result as a value
representation.
--
Ticket URL: <https://code.djangoproject.com/ticket/37364>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

[Django]

#37364: Make NullBooleanSelect.format_value() return a list, consistent with
ChoiceWidget
-------------------------------------+-------------------------------------

Reporter: Brian Helba | Owner: Brian
Type: | Helba
Cleanup/optimization | Status: assigned
Component: Forms | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage:
| Unreviewed
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Brian Helba):

* has_patch: 0 => 1
* owner: (none) => Brian Helba
* status: new => assigned

Comment:

https://github.com/django/django/pull/22009
--
Ticket URL: <https://code.djangoproject.com/ticket/37364#comment:1>

[Django]

#37364: Make NullBooleanSelect.format_value() return a list, consistent with
ChoiceWidget
-------------------------------------+-------------------------------------
Reporter: Brian Helba | Owner: Brian
Type: | Helba

Cleanup/optimization | Status: closed
Component: Forms | Version: dev
Severity: Normal | Resolution: wontfix

Keywords: | Triage Stage:
| Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by David Smith):

* resolution: => wontfix
* status: assigned => closed

Comment:

Looking at django-stubs nicely shows the inconsistency here

https://github.com/typeddjango/django-
stubs/blob/c7b59a09461784a6ec07704fcfc67391f7e8000d/django-
stubs/forms/widgets.pyi#L277-L283

{{{
# NullBooleanSelect.format_value violates the Liskov substitution
principle
@override
def format_value(self, value: Any) -> str: ... # type:
ignore[override]
}}}

However, as it has been like this _forever_ any change now would be a
breaking change. Here is one public example:

https://github.com/International-GNSS-
Service/SLM/blob/71151140698fecacb56eec5891bd85cad03a3b03/src/slm/forms.py#L137-L156

{{{

...
def format_value(self, value):
if value in [None, "", "None"]:
return [""]
return [super().format_value(value)]
}}}

I don't think that making the code consistent now is worth the churn for
users so I'll close as `wontfix`. If you'd like to discuss further, the
next step would be to raise it on the
[https://forum.djangoproject.com/c/internals/5 django forum] to seek wider
consensus with the community.
--
Ticket URL: <https://code.djangoproject.com/ticket/37364#comment:2>