Opened 11 months ago

Last modified 9 months ago

#27697 assigned Bug

JSONField with blank=True is rendered as non-required field in a ModelForm

Reported by: aruseni Owned by: Andrew Nester
Component: contrib.postgres Version: 1.10
Severity: Normal Keywords: postgres
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: yes
Easy pickings: no UI/UX: no

Description

Try adding a JSONField with blank=True and without null=True to your model, and then use this model in the admin, the field will be rendered as a non-required field, and if you specify {} as the value, everything saves just fine.

If you don’t specify any value though, an error occurs:

null value in column "json" violates not-null constraint

If you just use a zero-configuration JSONField without blank=True, then the form will not validate when an empty dictionary is specified.

Attachments (1)

Screen Shot 2017-02-10 at 2.19.21 PM.png (49.1 KB) - added by Andrew Nester 9 months ago.
Form field error

Download all attachments as: .zip

Change History (11)

comment:1 Changed 10 months ago by Tim Graham

What do you think the expected behavior is in this case?

comment:2 Changed 10 months ago by Tim Graham

Triage Stage: UnreviewedAccepted

Accepting as the current behavior doesn't seem ideal.

comment:3 Changed 10 months ago by Andrew Nester

Owner: set to Andrew Nester
Status: newassigned

I've added pull request #PR

I've decided to make logic so: if we have blank=True and null=False for JSONField, we make this field required with empty value set to {}


comment:4 Changed 10 months ago by Andrew Nester

Has patch: set

comment:5 Changed 10 months ago by Sayid Munawar

CMIIW, in the Django Docs it is not advised to use {} as an empty value. it should be callable like dict

comment:6 Changed 10 months ago by Andrew Nester

Thanks Sayid! Just fixed it in my PR

comment:7 Changed 9 months ago by Tim Graham

Patch needs improvement: set

The behavior with a field like data = JSONField(blank=True) still seems confusing. It renders "null" in an initial form but then creates a "This field is required." error if you try to save that. Before this PR, a value of {} could be saved with such a field, but now it gives "This field is required." for that input. Is this what you expected? If so, perhaps some documentation clarification is in order as this doesn't seem intuitive.

Changed 9 months ago by Andrew Nester

Form field error

comment:8 Changed 9 months ago by Andrew Nester

Thanks Tim!
I've just updated my PR to make it work more clear.

Now it's working like this.
If we have blank=True set , form field will be render as NOT required and valid empty JSON string such as {}, [] will be successfully validated and saved.
But if you try to submit just empty form field, you will get form validation error with message 'None' value must be valid JSON.

See screenshots for details https://code.djangoproject.com/attachment/ticket/27697/Screen%20Shot%202017-02-10%20at%202.19.21%20PM.png

Last edited 9 months ago by Andrew Nester (previous) (diff)

comment:9 Changed 9 months ago by Andrew Nester

Patch needs improvement: unset

comment:10 Changed 9 months ago by Tim Graham

Patch needs improvement: set

I think what I would expect (and I'm certain open to other opinions, including your own if you disagree) with blank=True is that nothing is rendered in the widget and if nothing is submitted, that empty string is transformed into an empty dict so it can be saved. In particular, I'm thinking of an API context where the initial={} that your proposed isn't going to have an effect since the API is only submitted values and doesn't receive an initial form.

I wish we could solve the null case without adding a new form option ("invalidate_null" as you proposed). If "null" is submitted though, an error message might indeed be more appropriate than transforming that into an empty dict. If we need to add some new form option, perhaps allow_null=True (default) would be more consistent with other existing options such as allow_unicode.

Note: See TracTickets for help on using tickets.
Back to Top