[Django] #33130: ErrorDict doesn't pass the dict type check in JsonResponse
Django
--------------------------------------+------------------------
Reporter: Jaap Roes | Owner: nobody
Type: Bug | Status: new
Component: Forms | Version: 4.0
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 |
--------------------------------------+------------------------
Just ran the test suite of a project on Django 4.0a1 (with Python 3.8) and
ran into the following issue:
We have a fairly simple `FormView` that defines the following
`form_invalid` method:
{{{
def form_invalid(self, form):
return JsonResponse(form.errors, status=400)
}}}
This worked fine before, but with Django 4 this raises the following
error:
`TypeError: In order to allow non-dict objects to be serialized set the
safe parameter to False.`
Wrapping `form.errors` with `dict()` fixes this, but I'm not sure this
should be necessary.
--
Ticket URL: <https://code.djangoproject.com/ticket/33130>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
---------------------------------+------------------------------------
Reporter: Jaap Roes | Owner: nobody
Type: Bug | Status: new
Component: Forms | Version: 4.0
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Mariusz Felisiak):
* cc: David Smith (added)
* severity: Normal => Release blocker
* stage: Unreviewed => Accepted
Comment:
Thanks for the report! `Form.errors` should be recognizable as a `dict`
according to
[https://docs.djangoproject.com/en/4.0/ref/forms/api/#django.forms.Form.errors
docs].
Regression in 456466d932830b096d39806e291fe23ec5ed38d5.
--
Ticket URL: <https://code.djangoproject.com/ticket/33130#comment:1>
Django
---------------------------------+------------------------------------
Reporter: Jaap Roes | Owner: nobody
Type: Bug | Status: new
Component: Forms | Version: 4.0
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Mariusz Felisiak):
It should be enough to subclass `dict`:
{{{
class ErrorDict(UserDict, dict, RenderableErrorMixin):
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/33130#comment:2>
Django
---------------------------------+-------------------------------------
Reporter: Jaap Roes | Owner: Jaap Roes
Type: Bug | Status: assigned
Component: Forms | Version: 4.0
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Jaap Roes):
* owner: nobody => Jaap Roes
* status: new => assigned
--
Ticket URL: <https://code.djangoproject.com/ticket/33130#comment:3>
Django
---------------------------------+-------------------------------------
Reporter: Jaap Roes | Owner: Jaap Roes
Type: Bug | Status: assigned
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Jaap Roes):
* has_patch: 0 => 1
Comment:
[https://github.com/django/django/pull/14884 PR] (There are two mariadb
failures, but those seem unrelated)
--
Ticket URL: <https://code.djangoproject.com/ticket/33130#comment:4>
Django
---------------------------------+-------------------------------------
Reporter: Jaap Roes | Owner: Jaap Roes
Type: Bug | Status: assigned
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 1 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
Changes (by Mariusz Felisiak):
* needs_better_patch: 0 => 1
* needs_tests: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/33130#comment:5>
Django
---------------------------------+-------------------------------------
Reporter: Jaap Roes | Owner: Jaap Roes
Type: Bug | Status: assigned
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 1 | Patch needs improvement: 1
Comment (by Jaap Roes):
Sadly the fix isn't as simple as initially thought (comment:2). It solves
the issue with the type check in `JsonResponse` but the actual json
serialization results in an empty JSON object instead of the intended
result of a JSON object with field names and error messages.
I will write additional tests to also check if the `ErrorDict` also
serializes in the desired way, and come up with a better fix (the PR
comments already have a good indication to what that may be).
--
Ticket URL: <https://code.djangoproject.com/ticket/33130#comment:6>
Django
---------------------------------+-------------------------------------
Reporter: Jaap Roes | Owner: Jaap Roes
Type: Bug | Status: assigned
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Jaap Roes):
* needs_better_patch: 1 => 0
* needs_tests: 1 => 0
Comment:
I've updated the [https://github.com/django/django/pull/14884 PR] with a
JSON serialization tests and a modified fix so make the JSON serialization
test pass.
--
Ticket URL: <https://code.djangoproject.com/ticket/33130#comment:7>
Django
-------------------------------------+-------------------------------------
Reporter: Jaap Roes | Owner: Jaap Roes
Type: Bug | Status: assigned
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Mariusz Felisiak):
* stage: Accepted => Ready for checkin
--
Ticket URL: <https://code.djangoproject.com/ticket/33130#comment:8>
Django
-------------------------------------+-------------------------------------
Reporter: Jaap Roes | Owner: Jaap Roes
Component: Forms | Version: 4.0
Severity: Release blocker | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Mariusz Felisiak <felisiak.mariusz@…>):
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"7fe9b6f6df16fa875fe360a1c7d0ac53fcf08a53" 7fe9b6f]:
{{{
#!CommitTicketReference repository=""
revision="7fe9b6f6df16fa875fe360a1c7d0ac53fcf08a53"
Fixed #33130 -- Restored form errors to be a dict.
Regression in 456466d932830b096d39806e291fe23ec5ed38d5.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/33130#comment:9>
Django
-------------------------------------+-------------------------------------
Reporter: Jaap Roes | Owner: Jaap Roes
Type: Bug | Status: closed
Severity: Release blocker | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Mariusz Felisiak <felisiak.mariusz@…>):
In [changeset:"25cfa5db0fe1919c72a437b57fffe7b87cd506ea" 25cfa5db]:
{{{
#!CommitTicketReference repository=""
revision="25cfa5db0fe1919c72a437b57fffe7b87cd506ea"
[4.0.x] Fixed #33130 -- Restored form errors to be a dict.
Regression in 456466d932830b096d39806e291fe23ec5ed38d5.
Backport of 7fe9b6f6df16fa875fe360a1c7d0ac53fcf08a53 from main
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/33130#comment:10>