Render JSON fields with proper indentation in browsable API forms.

[aleehedl]

Description

Fixes bugs related to nested JSONField as_form_field rendering.

Closes #6211

[s4ke]

Can we get this merged?

[s4ke]

Something I had to add to have the proper behaviour in the browsable API was special checking for nulls and errors:

    def as_form_field(self):
        values = {}
        for key, value in self.value.items():
            if isinstance(value, (list, dict)):
                values[key] = value
            else:
                field = self.fields[key]
                error = self.errors.get(key) if isinstance(self.errors, dict) else None
                if getattr(field, '_is_jsonfield', False) and error is not None or value is None:
                    if value is None:
                        values[key] = None
                    else:
                        class JSONString(str):
                            def __new__(cls, value):
                                ret = str.__new__(cls, value)
                                ret.is_json_string = True
                                return ret
                        values[key] = JSONString(value)
                else:
                    values[key] = '' if (value is None or value is False) else force_str(value)
        return self.__class__(self._field, values, self.errors, self._prefix)

[tomchristie]

Yeah seems like a decent improvement actually. Thanks!

[s4ke]

@tomchristie My comment was intended as a hint of what we needed to add for proper rendering of nulls and errors for json fields in the browsable api. Do you think that this would help?

[tomchristie]

Not sure, it's not clear enough to me from the comment what the actual effect/change is that you're suggesting. (From a quick scan)

[s4ke]

Right now, the browsable API has the following behaviour for json fields for nested bound fields:

1. You put in a broken/non-json payload into the JSON field in the browsable api, e.g. {'broken': json}. It quotes the input again to produce "{'broken': json}" in the browsable api on the error page.
2. You put in a null json value that makes validation fail (json field is not nullable). It quotes the input to "null" on the error page.

While not a big issue, this is a nuissance when testing out the browsable API and you mistyped before sending a request as you have to manually fix the error in your payload as well as the extra quotes around it that were introduced by drf.

EDIT: I will produce screenshots.

[s4ke]

Without my snippet:

With my snippet:

[tomchristie]

Not sure. We might consider it, tho I don't know if the complexity is worth it or not. We're a big ol' clunky framework as things are, and sometimes it might be worth us just letting a few of these niggles pass, vs. adding extra code workarounds. 🤷‍♂️

Prob needs a fresh discussion if you're keen on pushing for it, and see if others have any feedback. (?)

[s4ke]

I don't see it as a big showstopper for us, we have found the workaround and use it in the codepath we need it for. In the end it's your call.

EDIT: And as this is "only" about the browsable API I understand your stance.

If I come around to it I will maybe create a PR. For now, the workaround can be found here.

class PayloadNestedField(NestedBoundField):
    # this is already fixed, but we are at 3.12.1 still
    def __init__(self, field, value, errors, prefix=''):  # type: ignore
        if value is not None and not isinstance(value, Mapping):
            value = {}
        super().__init__(field, value, errors, prefix)

    def __getitem__(self, key: Any) -> Any:
        field = self.fields[key]
        value = self.value.get(key) if self.value else None
        error = self.errors.get(key) if isinstance(self.errors, dict) else None  # type: ignore
        if hasattr(field, 'fields'):
            return NestedBoundField(field, value, error, prefix=self.name + '.') # type: ignore
        if getattr(field, '_is_jsonfield', False):
            return JSONBoundField(field, value, error, prefix=self.name + '.') # type: ignore
        return BoundField(field, value, error, prefix=self.name + '.') # type: ignore

    def as_form_field(self) -> Any:
        values = {}
        for key, value in self.value.items():
            if isinstance(value, (list, dict)):
                values[key] = value
            else:
                field = self.fields[key]
                error = self.errors.get(key) if isinstance(self.errors, dict) else None  # type: ignore
                if getattr(field, '_is_jsonfield', False) and error is not None or value is None:
                    if value is None:
                        values[key] = None
                    else:
                        class JSONString(str):
                            def __new__(cls, value):
                                ret = str.__new__(cls, value)
                                ret.is_json_string = True
                                return ret
                        values[key] = JSONString(value)
                else:
                    values[key] = '' if (value is None or value is False) else force_str(value)  # type: ignore
        return self.__class__(self._field, values, self.errors, self._prefix)  # type: ignore