#8528: Admin list_filter doesn't respect null=True
------------------------------------------------+---------------------------
Reporter: StevenPotter | Owner: julien
Status: new | Milestone: 1.3
Component: django.contrib.admin | Version: 1.3-alpha
Resolution: | Keywords:
Triage Stage: Accepted | Has patch: 1
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 1 |
------------------------------------------------+---------------------------
Changes (by DrMeers):
* needs_better_patch: 0 => 1
Comment:
Looks good Julien. Only minor comments:
The `if hasattr(self.field, 'rel')` prevents dealing with a
`RelatedObject`, which is actually not a field but a reverse relationship.
I think it makes sense not to allow `isnull` filtering on these?
Any reason you've cast `self.lookup_val_isnull` to `bool` in
`RelatedFilterSpec` but not in `AllValuesFilterSpec`?
You need an `__init__.py` in `tests/regressiontests/admin_filterspecs/`.
It might be a good idea to include a reverse relationship in the tests
(e.g. a `Chapter` model with a `ForeignKey` to `Book`, and then test a
filter in both directions)? As mentioned above, I'm not sure that an
`isnull` lookup on a reverse relationship makes sense, but it would be
nice to explicitly ensure that nothing breaks here.
In the tests, would `choices[-1]` be more appropriate than `choices[3]`
and `choices[4]` for testing the "last choice"?
Lines in `tests.py` and `filterspecs.py` exceed PEP8's 79-character limit.
Thanks for the great work, hopefully we can get this RFC today.
--
Ticket URL: <http://code.djangoproject.com/ticket/8528#comment:21>
Django <http://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
--
You received this message because you are subscribed to the Google Groups
"Django updates" group.
To post to this group, send email to [email protected].
To unsubscribe from this group, send email to
[email protected].
For more options, visit this group at
http://groups.google.com/group/django-updates?hl=en.
