[Django] #29459: data=None in Form results NOT in empty QueryDict
Django
-----------------------------------------+--------------------------------
Reporter: Sven R. Kunze | Owner: nobody
Type: Bug | Status: new
Component: Forms | Version: 2.0
Severity: Normal | Keywords: Form QueryDict
Triage Stage: Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 1 |
-----------------------------------------+--------------------------------
You might have a look here:
https://github.com/django/django/blob/362813d6287925b8f63f/django/forms/forms.py#L78
None is converted to a regular dict but not to a QueryDict.
Methods of the form might rely on the API of a QueryDict such as
'iterlists' or 'getlist' which a regular dict doesn't provide.
--
Ticket URL: <https://code.djangoproject.com/ticket/29459>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: Sven R. Kunze | Owner: nobody
Type: Bug | Status: new
Component: Forms | Version: 2.0
Keywords: Form QueryDict | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 1
Changes (by Herbert Fortes):
* cc: Herbert Fortes (added)
Comment:
Hi,
To be clear, [https://docs.djangoproject.com/en/2.0/ref/request-response
/#querydict-objects QueryDict] is a subclass of dict.
The idea is add functionality?
Regards,
--
Ticket URL: <https://code.djangoproject.com/ticket/29459#comment:1>
Django
Reporter: Sven R. Kunze | Owner: nobody
Type: Bug | Status: new
Component: Forms | Version: 2.0
Keywords: Form QueryDict | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 1
Comment (by Sven R. Kunze):
Replying to [comment:1 Herbert Fortes]:
> Is the idea to add a feature?
Sorry, if that wasn't clear from the issue itself. Instead of:
{{{
self.data = {} if data is None else data
}}}
this would be 100% compatible with what views usually pass into forms:
{{{
self.data = QueryDict() if data is None else data
}}}
> At a first look, the dictionary is there to avoid that the code breaks.
I think so as well and it works in many places until people think they can
rely on the QueryDict API such as 'iterlists' or 'getlist' which is not
always true.
--
Ticket URL: <https://code.djangoproject.com/ticket/29459#comment:2>
Django
Reporter: Sven R. Kunze | Owner: nobody
Type: Bug | Status: new
Component: Forms | Version: 2.0
Keywords: Form QueryDict | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 1
Comment (by Tim Graham):
A similar issue was raised in #27989. The main concern I have is that this
would couple `django.forms` to `django.http`.
--
Ticket URL: <https://code.djangoproject.com/ticket/29459#comment:3>
Django
Reporter: Sven R. Kunze | Owner: nobody
Type: Bug | Status: new
Component: Forms | Version: 2.0
Keywords: Form QueryDict | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 1
Comment (by Sven R. Kunze):
Yes, I thought that this was the reason to change it from "data or {}" to
ternary operator.
What do you think about:
1) either using {{{MultiValueDict}}} instead of {{{QueryDict}}}? That
would couple {{{django.forms}}} to {{{django.utils.datastructures}}}
2) or moving {{{QueryDict}}} to {{{django.utils.datastructures}}} and then
using it in the {{{BaseForm}}} constructor?
At least, {{{MultiValueDict}}} supports the {{{getlist}}}, {{setlist}}}
and {{{lists}}} APIs. So, those would behave the same because the dicts
are empty anyway. Althought, I would rather tend to option 2.
What do you think?
--
Ticket URL: <https://code.djangoproject.com/ticket/29459#comment:4>
Django
Reporter: Sven R. Kunze | Owner: nobody
Type: Bug | Status: new
Component: Forms | Version: 2.0
Keywords: Form QueryDict | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 1
Comment (by Simon Charette):
I like the `MultiValueDict` approach, +1.
--
Ticket URL: <https://code.djangoproject.com/ticket/29459#comment:5>
Django
Reporter: Sven R. Kunze | Owner: nobody
Type: Bug | Status: new
Component: Forms | Version: 2.0
Keywords: Form QueryDict | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 1
Comment (by Herbert Fortes):
MultiValueDict +1
--
Ticket URL: <https://code.djangoproject.com/ticket/29459#comment:6>
Django
Reporter: Sven R. Kunze | Owner: nobody
Type: Bug | Status: new
Component: Forms | Version: 2.0
Keywords: Form QueryDict | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 1
Comment (by Sven R. Kunze):
Okay seems like we have winner. :D
Just exploring the other option a bit more: why is moving QueryDict to
datastructures not a feasible option? It actually sounds like a more
reasonable place for it.
--
Ticket URL: <https://code.djangoproject.com/ticket/29459#comment:7>
Django
dict
--------------------------------------+------------------------------------
Reporter: Sven R. Kunze | Owner: nobody
Component: Forms | Version: 2.0
Severity: Normal | Resolution:
Keywords: Form QueryDict | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 1
Changes (by Tim Graham):
* type: Bug => Cleanup/optimization
* stage: Unreviewed => Accepted
Comment:
I agree that `QueryDict` should remain in `django.http`. I don't feel that
it's something meant for use outside of the request/response cycle.
--
Ticket URL: <https://code.djangoproject.com/ticket/29459#comment:8>
Django
dict
--------------------------------------+------------------------------------
Component: Forms | Version: 2.0
Severity: Normal | Resolution:
Keywords: Form QueryDict | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 1
Comment (by Sven R. Kunze):
{{{
I don't feel that it's something meant for use outside of the
request/response cycle.
}}}
Sorry, for still nagging here, but I still don't understand why it cannot
be moved to datastructures.
Its doc string: "A specialized MultiValueDict which represents a query
string." <<< That sounds like a perfect valid datastructure handling
encoding and query string syntax. Its source code has no reference to
request/responses.
\\
Also APIs {{{MultiValueDict}}} doesn't provide:
* urlencode
* encoding (getter and setter)
* fromkeys
* mutability
* isinstance check
\\
I just want to make sure we don't need to touch those this part in 3 years
AGAIN when somebody else needs the real thing.
--
Ticket URL: <https://code.djangoproject.com/ticket/29459#comment:9>
Django
dict
--------------------------------------+------------------------------------
Component: Forms | Version: 2.0
Severity: Normal | Resolution:
Keywords: Form QueryDict | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 1
Comment (by Jeff):
Do we want to move ahead with the `MultiValueDict`, or delay for more
discussion concerning the `QueryDict`?
--
Ticket URL: <https://code.djangoproject.com/ticket/29459#comment:10>
Django
dict
--------------------------------------+------------------------------------
Component: Forms | Version: 2.0
Severity: Normal | Resolution:
Keywords: Form QueryDict | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 1
Comment (by Herbert Fortes):
IMHO, an empty dict should not make too much noise on the code. For now at
least.
--
Ticket URL: <https://code.djangoproject.com/ticket/29459#comment:11>
Django
dict
Reporter: Sven R. Kunze | Owner: Andra
Type: | Denis Ionescu
Cleanup/optimization | Status: assigned
Component: Forms | Version: 2.0
Severity: Normal | Resolution:
Keywords: Form QueryDict | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 1
Changes (by Andra Denis Ionescu):
* owner: nobody => Andra Denis Ionescu
* status: new => assigned
--
Ticket URL: <https://code.djangoproject.com/ticket/29459#comment:12>
Django
dict
-------------------------------------+-------------------------------------
Reporter: Sven R. Kunze | Owner: Andra
Type: | Denis Ionescu
Cleanup/optimization | Status: assigned
Severity: Normal | Resolution:
Keywords: Form QueryDict | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 1 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 1
Changes (by Tim Graham):
* has_patch: 0 => 1
* needs_tests: 0 => 1
Comment:
[https://github.com/django/django/pull/11044 PR]. Please uncheck "Needs
tests" after updating.
--
Ticket URL: <https://code.djangoproject.com/ticket/29459#comment:13>
Django
dict
-------------------------------------+-------------------------------------
Reporter: Sven R. Kunze | Owner: Andra
Type: | Denis Ionescu
Cleanup/optimization | Status: assigned
Severity: Normal | Resolution:
Keywords: Form QueryDict | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 1
Changes (by Andra Denis Ionescu):
* needs_tests: 1 => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/29459#comment:14>
Django
dict
-------------------------------------+-------------------------------------
Reporter: Sven R. Kunze | Owner: Andra
Type: | Denis Ionescu
Cleanup/optimization | Status: assigned
Severity: Normal | Resolution:
Keywords: Form QueryDict | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 1
Changes (by Simon Charette):
* stage: Accepted => Ready for checkin
--
Ticket URL: <https://code.djangoproject.com/ticket/29459#comment:15>
Django
dict
-------------------------------------+-------------------------------------
Reporter: Sven R. Kunze | Owner: Andra
Type: | Denis Ionescu
Component: Forms | Version: 2.0
Severity: Normal | Resolution: fixed
Keywords: Form QueryDict | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 1
Changes (by Tim Graham <timograham@…>):
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"4c086d7da4c5cf23935a5340dbb9a8d6835cf7cc" 4c086d7d]:
{{{
#!CommitTicketReference repository=""
revision="4c086d7da4c5cf23935a5340dbb9a8d6835cf7cc"
Fixed #29459 -- Initialized form data/files with empty MultiValueDicts.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/29459#comment:16>