[Django] #30024: The test client request methods should raise an error when passed None as a data value
Django
data value
---------------------------------------------+------------------------
Reporter: Jon Dufresne | Owner: nobody
Type: New feature | Status: new
Component: Testing framework | Version: master
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 |
---------------------------------------------+------------------------
Both GET and POST encoded data do not have a concept of `None` or `NULL`.
The closest approximation is an empty string value or omitting the key.
For example, in a GET request this could be either `/my-url/?my_field=` or
simply `/my-url/` but not `/my-url/?my_field=None`)
When onboarding new developers to projects, this can cause confusion to
those less familiar with these details. For example, a new developer may
try the following:
{{{
def test_setting_value_to_none(self):
self.client.post('/my-url/', {'my_field': None})
self.assertIsNone(...)
}}}
In current versions of Django, behind the scenes, this `None` gets coerced
to the string `'None'` by the test client. The Django form field classes
don't recognize the string `'None'` as an empty value (good) and so this
test doesn't pass. Where the new developer thought a field would be
assigned `None` they instead get a form error. Depending on the
developers' knowledge of these details, this could take much debugging or
consulting a colleague.
I think we can recognize this pattern as a programming mistake and raise
an informative error to guide the developer. I propose something like the
following, but am open to suggestions:
{{{
TypeError: Cannot encode None as POST data. Did you mean to pass an empty
string or omit the value?
}}}
For GET requests, the query string data is processed by
`django.utils.http.urlencode()`. So perhaps this same check can be done
there as encoding `None` in a URL query string as `'None'` is rarely the
intended behavior. For those that really want the string `'None'` in the
query string, they can pass the string `'None'`.
--
Ticket URL: <https://code.djangoproject.com/ticket/30024>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
data value
Reporter: Jon Dufresne | Owner: nobody
Type: New feature | Status: new
Component: Testing framework | Version: master
Keywords: | Triage Stage: Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Jon Dufresne):
* has_patch: 0 => 1
Comment:
[https://github.com/django/django/pull/10738 PR]
--
Ticket URL: <https://code.djangoproject.com/ticket/30024#comment:1>
Django
data value
Reporter: Jon Dufresne | Owner: nobody
Type: New feature | Status: new
Component: Testing framework | Version: master
Keywords: | Triage Stage: Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Carlton Gibson):
I think this is fair. There's no case where `None` is the right thing to
use. (As Jon says, **maybe** `'None'` if you really mean that.)
--
Ticket URL: <https://code.djangoproject.com/ticket/30024#comment:2>
Django
data value
Reporter: Jon Dufresne | Owner: nobody
Type: New feature | Status: new
Component: Testing framework | Version: master
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Carlton Gibson):
* stage: Unreviewed => Accepted
--
Ticket URL: <https://code.djangoproject.com/ticket/30024#comment:3>
Django
data value
Reporter: Jon Dufresne | Owner:
| candypoplatte
Type: New feature | Status: assigned
Component: Testing framework | Version: master
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by candypoplatte):
* owner: nobody => candypoplatte
* status: new => assigned
--
Ticket URL: <https://code.djangoproject.com/ticket/30024#comment:4>
Django
data value
Reporter: Jon Dufresne | Owner:
| candypoplatte
Type: New feature | Status: assigned
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Simon Charette):
candypoplatte, Jon already submitted a PR so there isn't much left to do
here https://github.com/django/django/pull/10738
Jon you might want to assign the ticket to yourself to avoid any
confusion.
--
Ticket URL: <https://code.djangoproject.com/ticket/30024#comment:5>
Django
data value
Reporter: Jon Dufresne | Owner: Jon
| Dufresne
Type: New feature | Status: assigned
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Jon Dufresne):
* owner: candypoplatte => Jon Dufresne
Comment:
Sorry about that. I'll do that for future tickets.
--
Ticket URL: <https://code.djangoproject.com/ticket/30024#comment:6>
Django
data value
Reporter: Jon Dufresne | Owner: Jon
| Dufresne
Type: New feature | Status: assigned
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 Carlton Gibson):
* stage: Accepted => Ready for checkin
--
Ticket URL: <https://code.djangoproject.com/ticket/30024#comment:7>
Django
data value
Reporter: Jon Dufresne | Owner: nobody
Component: Testing framework | Version: master
Keywords: | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham <timograham@…>):
* status: new => closed
* resolution: => fixed
Comment:
In [changeset:"6fe9c45b725cd21eacbb50263bd3449e1a3edf17" 6fe9c45b]:
{{{
#!CommitTicketReference repository=""
revision="6fe9c45b725cd21eacbb50263bd3449e1a3edf17"
Fixed #30024 -- Made urlencode() and Client raise TypeError when None is
passed as data.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/30024#comment:8>