Opened 8 years ago

Closed 3 years ago

Last modified 3 years ago

#27697 closed Bug (duplicate)

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

Reported by: aruseni Owned by: Andrew Nester
Component: Documentation Version: 1.10
Severity: Normal Keywords: postgres
Cc: Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
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 8 years ago.
Form field error

Download all attachments as: .zip

Change History (15)

comment:1 by Tim Graham, 8 years ago

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

comment:2 by Tim Graham, 8 years ago

Triage Stage: UnreviewedAccepted

Accepting as the current behavior doesn't seem ideal.

comment:3 by Andrew Nester, 8 years ago

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 by Andrew Nester, 8 years ago

Has patch: set

comment:5 by Sayid Munawar, 8 years ago

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

comment:6 by Andrew Nester, 8 years ago

Thanks Sayid! Just fixed it in my PR

comment:7 by Tim Graham, 8 years ago

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.

by Andrew Nester, 8 years ago

Form field error

comment:8 by Andrew Nester, 8 years ago

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 8 years ago by Andrew Nester (previous) (diff)

comment:9 by Andrew Nester, 8 years ago

Patch needs improvement: unset

comment:10 by Tim Graham, 8 years ago

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.

comment:11 by Mariusz Felisiak, 4 years ago

Component: contrib.postgresDatabase layer (models, ORM)

comment:12 by Jacob Walls, 3 years ago

I suggest marking as a duplicate of #22224. Any proposal to run model-validation on not-nullable blankable fields would fix both (but I suggest wontfixing #22224 as the status quo represents a supported design pattern, will comment there shortly).

Re: Tim's suggestion:

if nothing is submitted, that empty string is transformed into an empty dict so it can be saved

I think this is dangerous because we should preserve the distinction between None and {}, in case the field is nullable. There could be code depending on this distinction.

comment:13 by Jacob Walls, 3 years ago

Resolution: duplicate
Status: assignedclosed

Per discussion we're going to have a look at clarifying the documentation around the blank=True, null=False idiom, and we can do that in #22224.

comment:14 by Jacob Walls, 3 years ago

Component: Database layer (models, ORM)Documentation
Has patch: unset
Patch needs improvement: unset
Note: See TracTickets for help on using tickets.
Back to Top