ModelMultipleChoiceFilter's custom method gets called even when field is not filtered on

[carltongibson]

Discussed in https://github.com/carltongibson/django-filter/discussions/1622

Converting to an issue to investigate.

^{Originally posted by **johncmacy** November 10, 2023} Hi, I have a FilterSet with a ModelMultipleChoiceFilter that has a custom method: ``` py class Filters(filters.FilterSet): publisher = filters.ModelMultipleChoiceFilter( queryset=Publisher.objects.all(), method="publisher_filter", ) def publisher_filter(self, queryset, name, value): return queryset.filter(publisher__in=value) # contrived example, we have more complex business logic ``` After getting some unexpected results, I found out that the custom method is getting called even when the field is not passed as a query param. For example, requests to `/api/book` return an empty list `[]`. After digging into it a bit, it seems that _every_ filter gets called regardless if it's passed as a query param. Normally this is fine because they get an empty value and it doesn't apply any filtering. However, when a custom method is supplied, it gets called with `` as the `value`. So in the example above, it filters books by publisher in `[]`, which results in the empty result set. I got around it with: ``` py def publisher_filter(self, queryset, name, value): if not value: return queryset return queryset.filter(publisher__in=value) ``` However, I didn't expect the method to get called in the first place, since the request didn't have a "publisher" query param. Am I missing something in the filter configuration that would prevent it from being called in the first place? Thanks!

[johncmacy]

Continuing from #1622

Thanks Carlton.

Filters are called if the field is present in filterset.form.cleaned_data.

It's not expected though...

Note that the value is validated by the Filter.field, so raw value transformation and empty value checking should be unnecessary. -- https://django-filter.readthedocs.io/en/stable/ref/filters.html#method

In my case, every filter on the filterset is present in cleaned_data. CharFilter, ModelChoiceFilter, etc. have their default empty value ("", None, etc.). However, for the ModelMultipleChoiceFilter that was not submitted, its value in cleaned_data is <QuerySet []>, which is not in EMPTY_VALUES and so it bypasses this if statement:

class Filter:
    def filter(self, qs, value):
        if value in EMPTY_VALUES:
            return qs

After further digging, it appears that filters with and without custom methods are behaving the same way (showing up in cleaned_data with a value of <QuerySet []>), so apparently it has nothing to do with custom methods as my original post indicated.

I'd need to dig in to see exactly why non-submitted fields are showing there.

To me, this is the issue. I ran into a similar problem when implementing an empty values filter. If it can be updated so that non-submitted filters do not show up in cleaned_data, that would be ideal. Or, update filter_queryset to only look at filters that were submitted:

class BaseFilterSet:
    def filter_queryset(self, queryset):
        for name in self.form.cleaned_data.keys() & self.request.query_params.keys():
            value = self.form.cleaned_data[name]
            queryset = self.filters[name].filter(queryset, value)
            ...
        return queryset

[johncmacy]

Update:

changed_data doesn't include submitted fields that have empty values:

for name in self.form.changed_data:

request.query_params and form.data could include query params that are not filters (and therefore not in cleaned_data:

for name in self.request.query_params:

for name in self.form.data:

So it seems we need to use a subset of keys that are in both:

for name in self.form.cleaned_data.keys() & self.request.query_params.keys():

for name in self.form.cleaned_data.keys() & self.form.data.keys():

[carltongibson]

changed_data doesn't include submitted fields that have empty values

So that's what I'm expecting (without cracking open the debugger).

Can you put the failing case in a test case against Django-filter's test suite? That makes it much easier to see exactly what you mean, rather than prose descriptions, where I can't see 100% what's going on. Thanks!

[johncmacy]

Understood - yes, I'll do my best to write something using your test suite.

In the meantime, I have an example repo with tests that demonstrate the issue, if that's helpful.

[craigds]

I noticed something similar in a failing test in our app just now when upgrading from django-filter 23.2 to 23.3 - a callable choices on a ChoiceFilter (which wasn't used in the request) is now getting called when it wasn't before.

I suspect 6119d454ee47aebd0ac09e4c29fd4c2137742040 but the diff looks fine to me. For reference we're (still) on Django 3.2.x so the 5.x specific changes in that commit shouldn't apply to us

[carltongibson]

@craigds A test case against django-filter's test suite would be handy.

[carltongibson]

@johncmacy This may be resolved in the new v24.2. If you'd like to give that a run and confirm that would be great.

[johncmacy]

Hi @carltongibson, I upgraded to v24.2, and am still getting the same behavior as before.

[carltongibson]

@johncmacy Thanks for confirming. It's not the same issue then ✅

[carltongibson]

@johncmacy Just re-reading this, I still need some time to sit down with the debugger, but if I were writing this myself I'd would automatically handle the none value before filtering, so I'm suspecting the answer is likely to be That's just how it works.

[johncmacy]

Ok, thanks for following up on this @carltongibson. It's not a deal-breaker, we can work around it.