Context
In the latest release there is a bugfix (#682) that intends to Ensure correct filter types for DjangoFilterConnectionFields, which is related to issue #678. The problem is that the change in the function get_filtering_args_from_filterset() has two possible issues (one minor and one major):
form_field not defined (minor):
This is a simple one. Following the logic in this new implementation if the name of the filter is not in the filterset_class.declared_filters and field_name is not an attribute of the model, it will never get form_field defined, so when it gets to:
if not form_field:
form_field = filter_field.field
We get a local variable 'form_field' referenced before assignment.
- Filters that are not applied to fields of the model will crash (major):
After fixing the previous issue, we still have the problem that the new implementation will only look for filters either in the declared_filters or for filter names that could match a field from a model. In our case, we have a mixin that overrides the get_filters method and updates the filters with some common ones that are not field filters. So we get a django.core.exceptions.FieldDoesNotExist: xxx has no field named 'xxxxx'
Steps to reproduce it
- Create a filterset based on a mixin:
class FooMixin:
@classmethod
def get_filters(cls):
filters = super().get_filters()
filters.update({
'partner__status': django_filters.ChoiceFilter(
method='filter_status',
field_name='users__status',
choices=users.field.model.STATUS,
),
})
return filters
class FooFilter(FooMixin, django_filters.FilterSet):
. . .
- Try any query that uses this filter.
What can we do?
I do have a proposal on how to fix this issue, but I wanted to hear your opinion first about this issue, as if this is an actual issue or if we are doing something wrong. So please, if you have any suggestions, let me know.
Proposal
My proposal to fix this issue would be changing the current implementation of the function mentioned before get_filtering_args_from_filterset:
def get_filtering_args_from_filterset(filterset_class, type):
""" Inspect a FilterSet and produce the arguments to pass to
a Graphene Field. These arguments will be available to
filter against in the GraphQL
"""
from ..forms.converter import convert_form_field
args = {}
model = filterset_class._meta.model
for name, filter_field in six.iteritems(filterset_class.base_filters):
form_field = None
if name in filterset_class.declared_filters:
form_field = filter_field.field
else:
field_name = name.split("__", 1)[0]
if hasattr(model, field_name):
model_field = model._meta.get_field(field_name)
if hasattr(model_field, "formfield"):
form_field = model_field.formfield(
required=filter_field.extra.get("required", False)
)
# Fallback to field defined on filter if we can't get it from the
# model field
if not form_field:
form_field = filter_field.field
field_type = convert_form_field(form_field).Argument()
field_type.description = filter_field.label
args[name] = field_type
return args
Here we are defining form_field as None, in case it doesn't match any of the following conditions, and checking if the filter is related to a field of the model before calling get_field.
Context
In the latest release there is a bugfix (#682) that intends to Ensure correct filter types for DjangoFilterConnectionFields, which is related to issue #678. The problem is that the change in the function
get_filtering_args_from_filterset()has two possible issues (one minor and one major):form_fieldnot defined (minor):This is a simple one. Following the logic in this new implementation if the
nameof the filter is not in thefilterset_class.declared_filtersandfield_nameis not an attribute of the model, it will never get form_field defined, so when it gets to:We get a
local variable 'form_field' referenced before assignment.After fixing the previous issue, we still have the problem that the new implementation will only look for filters either in the
declared_filtersor for filter names that could match a field from a model. In our case, we have a mixin that overrides theget_filtersmethod and updates the filters with some common ones that are not field filters. So we get adjango.core.exceptions.FieldDoesNotExist: xxx has no field named 'xxxxx'Steps to reproduce it
What can we do?
I do have a proposal on how to fix this issue, but I wanted to hear your opinion first about this issue, as if this is an actual issue or if we are doing something wrong. So please, if you have any suggestions, let me know.
Proposal
My proposal to fix this issue would be changing the current implementation of the function mentioned before
get_filtering_args_from_filterset:Here we are defining
form_fieldasNone, in case it doesn't match any of the following conditions, and checking if the filter is related to a field of the model before callingget_field.