Raise an exception if `MultipleChoiceFilter` filter doesn't receive a list data type

[lliendo]

I was writing some unit tests against a filter. My filter consisted in some fields, but one of them (registration_state) was MultipleChoiceFilter.

The unit test I was writing was roughly:

def test_filter_users_by_status(self, _, registration_state, expected_users_count):
    user_filter = UserFilter(
        {
            'registration_state': registration_state,
        },
        queryset=User.objects.all(),
    )
    ...
    # Assert something about `user_filter.qs`

The filter type doesn't run as expected as the type of the value for the "registration_state" needs to be a list. So the correct way to do this is:

def test_filter_users_by_status(self, _, registration_state, expected_users_count):
    user_filter = UserFilter(
        {
            'registration_state': [registration_state],
        },
        queryset=User.objects.all(),
    )
    ...
    # Assert something about `user_filter.qs`

What it would be tremendously helpful is to raise an exception in the first case so developers don't waste time trying to figure out why the filter doesn't bring the correct results just because the data type is incorrect (should be a list containing choices and not a string).

Same suggestion could be applied to other types that don't expect simple data types as strings, ints, etc.

If this suggestion is not adequate there still should be a way to easily notify the user that is doing something wrong in these kind of situations.

[rpkilby]

Hi @lliendo. MultipleChoiceFilter is provided by the underlying django-filter package, not drf-filters. Going ahead and closing since I don't think there's an issue on our end. That said, happy to continue to discussing here.

Either way, I don't think this is a bug. When the .qs is accessed, validation is performed implicitly and errors aren't explicitly raised (the view code is expected to handle errors). Filtering is then performed on the cleaned filter data. Since the filter is invalid, it isn't applied to the queryset.

As to how the view code is supposed to handle errors, it depends on what context you're operating in. For APIs, the DjangoFilterBackend specifically checks filterset.is_valid() and will raise the
filterset.errors accordingly. However, in a Django view, you don't want 400 responses. Instead, the validation errors are rendered to the template. Note that this behavior mirrors Django's forms, which also doesn't raise validation errors.

So, long story short, your test code should check filterset.is_valid() or filterset.errors.

[rpkilby]

Actually, this really is a documentation issue. Although you can technically get away with just:

f = MyFilter({'some': 'data'})
f.qs

Users should know to call .is_valid() and raise/handle .errors if they're directly manipulating filterset instances.

f = MyFilter({'some': 'data'})
if not f.is_valid():
    raise f.errors
f.qs

Added this to a list of my own docs-related enhancements/issues.