#35316: Support scalars as query parameters in admin changelist filters (for
backward compatibility).
-------------------------------------+-------------------------------------
Reporter: Manolis | Owner: nobody
Stamatogiannakis |
Type: Uncategorized | Status: new
Component: contrib.admin | Version: 5.0
Severity: Normal | Resolution:
Keywords: | Triage Stage:
| Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Manolis Stamatogiannakis):
* resolution: invalid =>
* status: closed => new
Comment:
Replying to [comment:1 Mariusz Felisiak]:
> `build_q_object_from_lookup_parameters()` is a private API and all
built-in filter use `GET.lists()` so I don't see anything breaking in the
#1873. If you extend built-in filters and really want to use
`build_q_object_from_lookup_parameters()` you should also use
`GET.lists()`.
I believe you may have misunderstood the case described. Of course
`build_q_object_from_lookup_parameters()` is not directly used, since I am
referring to code that pre-dates Django 5, when the function was
introduced. The problem is that this code now needs version conditionals
in order to work with both Django 4.x and Django 5.x.
This isn't anything super fancy. Just a subclass of BooleanFieldListFilter
that needs to have True as the default selection instead of All. It looks
like this ATM:
{{{
class BooleanDefaultTrueFilter(BooleanFieldListFilter):
def __init__(self, *args, **kwargs):
super().__init__(*args, **kwargs)
if self.lookup_kwarg2 not in self.used_parameters and
self.lookup_kwarg not in self.used_parameters:
self.`[self.lookup_kwarg] = prepare_lookup_value(
self.lookup_kwarg,
True if django.VERSION < (5,0) else (True,)
)
self.lookup_val = '1'
}}}
Without the conditional, an exception will be raised in
`build_q_object_from_lookup_parameters()`, which is why the specific
function was updated.
Of course one may still argue that `used_parameters` is internal state.
But overriding `__init__()` like above is still the most elegant way I
could find to achive the desired effect (see [1]). Is there a
better/"sanctioned" way to achieve this effect? The documentation for
BooleanFieldListFilter/FieldListFilter is pretty sparse.
Also, a quick GH search shows a number of repositories modifying
`used_parameters` [2]. So, although not thorougly documented, this is not
something super-exotic for users customizing admin filters, and one would
expect that there will be more codebases that will have to implement
similar workarounds.
Overall, the sparse documentation for
BooleanFieldListFilter/FieldListFilter should not be a reason for
breakage, when that is easily avoidable.
Along this line, I think that updating
`build_q_object_from_lookup_parameters()` as proposed in PR #17990 will be
a useful addition, as long as there are no side-effects that I missed and
would outweigh the benefits.
[1] https://forum.djangoproject.com/t/django-admin-filter-how-to-default-
to-no-or-yes-rather-than-all/6684
[2]
https://github.com/search?q=self.used_parameters%5B+lang%3APython+&type=code
--
Ticket URL: <https://code.djangoproject.com/ticket/35316#comment:2>
Django <https://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 unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
To view this discussion on the web visit
https://groups.google.com/d/msgid/django-updates/0107018e53dd43a7-857311fa-74c5-4f9d-bef4-3097de1c3749-000000%40eu-central-1.amazonses.com.
