#14467 closed (duplicate)
AllValuesFilterSpec does not work with Null
| Reported by: | oyvind | Owned by: | nobody |
|---|---|---|---|
| Component: | contrib.admin | Version: | dev |
| Severity: | Keywords: | Filterspec | |
| Cc: | Triage Stage: | Accepted | |
| Has patch: | yes | Needs documentation: | no |
| Needs tests: | no | Patch needs improvement: | no |
| Easy pickings: | no | UI/UX: | no |
Description
AllValuesFilterSpec does not make the proper __isnull query for nullable / None values.
<a href="?field=None">None</a> Only redirects to ?e=1
Would expect the link to be
<a href="?field__isnull=1">None</a>
Attachments (6)
Change History (17)
by , 14 years ago
| Attachment: | fix_for_None_values.diff added |
|---|
comment:1 by , 14 years ago
| Has patch: | set |
|---|---|
| Needs tests: | set |
by , 14 years ago
| Attachment: | AllValuesFilterSpec_Null_Tests.py added |
|---|
Start of tests for patch, not sure where to put them
comment:2 by , 14 years ago
| Resolution: | → invalid |
|---|---|
| Status: | new → closed |
I am pretty sure that qs.filter(field=None) and qs.filter(field__isnull=1) are the exact same query. So, href="?field=None" should be fine.
comment:3 by , 14 years ago
| Resolution: | invalid |
|---|---|
| Status: | closed → reopened |
| Triage Stage: | Unreviewed → Accepted |
To clarify the problem: href="?field=None" will result in query qs.filter(field='None'). Reopening...
comment:4 by , 14 years ago
| Needs tests: | unset |
|---|
comment:5 by , 14 years ago
| Component: | Uncategorized → django.contrib.admin |
|---|---|
| Keywords: | Filterspec added |
Should the tests try to filter on some field containing None, and see that everything works all the way to database? That is, the mock request actually returns the right filtered list of objects when None filtering is requested. Test behavior, not implementation and all that...
comment:6 by , 14 years ago
How would i actually test that, there is no self.assertNotRaises, try / except in the test?
comment:7 by , 14 years ago
Just call the function, if it raises an exception the test will fail (technically error, but it amounts to the same thing).
by , 14 years ago
| Attachment: | AllValuesFilterSpec_Null.2.diff added |
|---|
Added patch with regressiontests, testing cl.get_query_set
by , 14 years ago
| Attachment: | AllValuesFilterSpec_Null.3.diff added |
|---|
Small refactoring of .2.diff, now without test_sqlite changes!
comment:8 by , 14 years ago
| Triage Stage: | Accepted → Ready for checkin |
|---|
IMHO this is now ready for committer. Please see the comment above lookup_choice_vals in choices().
comment:9 by , 14 years ago
| Triage Stage: | Ready for checkin → Accepted |
|---|
IMHO, the TestCase needs some code style changes:
- Regression test can now use
django.test.client.RequestFactoryto mock a request instead of creating it's own. - You don't need to delete created ORM objects in tearDown() - the DB will get flushed anyway.
- Python has comments, you don't need to put noop string expressions.
Also, looping through the list (using "in") to check if there is a None, when we just looped feels weird.
by , 14 years ago
| Attachment: | AllValuesFilterSpec_Null.4.diff added |
|---|
New patch using RequestFactory, Python comments, no delete() in tearDown, added back include_none
Fix without touching FilterSpec