Handle empty list for href and id in filters #4439
Conversation
daviddavis
force-pushed
the
fix4437
branch
2 times, most recently
from
b7ef225
to
8e3c0f3
Compare
daviddavis
force-pushed
the
fix4437
branch
2 times, most recently
from
58d6f09
to
50268ce
Compare
|
Nice fix! Can you add a check for empty lists to the |
daviddavis
force-pushed
the
fix4437
branch
3 times, most recently
from
6fcdf28
to
45e942d
Compare
daviddavis
changed the title
Fix bug where empty list for the pulp_href__in or pulp_id__in filters return all results instead of no results. The problem is that when using FilterMethod, django-filter just simply ignores None and empty set and returns the full list of results: https://github.com/carltongibson/django-filter/blob/e4a70a667a0bf3882fd44b557bc76583d2c65cd1/django_filters/filters.py#L804-L805 There's no way to update the method to prevent this since this logic happens before the method is called. Instead, this change moves the method to the filter and adds a None check. fixes pulp#4437
| pass | ||
| def filter(self, qs, value): | ||
| if value is None: | ||
| return qs | ||
| return qs.filter(pk__in=value) |
There was a problem hiding this comment.
Does this issue also apply when there is not a filter method provided? I'm now a bit confused by the link you provided to django-filter.
There was a problem hiding this comment.
Yes, anytime a filter method is used for an "in" filter, the issue would occur.
There was a problem hiding this comment.
I filed a bug for the problem: carltongibson/django-filter#1609
There was a problem hiding this comment.
Oh, when a filter method is NOT used, there is no such issue. It just calls the filter method on the filter without checking against EMPTY_VALUES.
There was a problem hiding this comment.
Hmm, I guess this is where all filters get this by default:
|
Does this need backports? And how far back? |
|
We requested this feature so I wouldn't be surprised if we're the only ones using these filters. Moreover, it's an edge case that I am not sure many users will hit. That said, it's been available in pulp since 3.24.0. |
|
For us though, we're still on an older version of pulpcore (until pulp/pulp_deb#295 is merged) and we've patched our version. |
Fix bug where empty list for the
pulp_href__inandpulp_id__infilters return all results instead of no results. The problem is that when using FilterMethod, django-filter just simply ignores empty set and returns the full list of results:https://github.com/carltongibson/django-filter/blob/e4a70a667a0bf3882fd44b557bc76583d2c65cd1/django_filters/filters.py#L804-L805
There's no way to update the method to prevent this since this logic happens before the method is called. Instead, this change moves the method to the filter and adds a None check.
fixes #4437