Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TypeError in schema generation when using a django_filters.ChoiceFilter with a callable "choices" #688

Closed
glennmatthews opened this issue Mar 23, 2022 · 7 comments · Fixed by #689

Comments

@glennmatthews
Copy link
Contributor

Describe the bug

A django-filters ChoiceFilter takes a parameter choices which can either be a list of valid values, or a function/callable which when called yields such a listing. The latter case is useful when the choices need to be determined at execution time rather than at declaration time. However, when using a filterset that contains filters with callable choices, drf-spectacular schema generation throws a TypeError:

  File "lib/python3.9/site-packages/drf_spectacular/management/commands/spectacular.py", line 50, in handle
    schema = generator.get_schema(request=None, public=True)
  File "lib/python3.9/site-packages/drf_spectacular/generators.py", line 262, in get_schema
    paths=self.parse(request, public),
  File "lib/python3.9/site-packages/drf_spectacular/generators.py", line 233, in parse
    operation = view.schema.get_operation(
  File "lib/python3.9/site-packages/drf_spectacular/openapi.py", line 68, in get_operation
    parameters = self._get_parameters()
  File "lib/python3.9/site-packages/drf_spectacular/openapi.py", line 215, in _get_parameters
    **dict_helper(self._get_filter_parameters()),
  File "lib/python3.9/site-packages/drf_spectacular/openapi.py", line 425, in _get_filter_parameters
    parameters += filter_extension.get_schema_operation_parameters(self)
  File "lib/python3.9/site-packages/drf_spectacular/contrib/django_filters.py", line 58, in get_schema_operation_parameters
    result += self.resolve_filter_field(
  File "lib/python3.9/site-packages/drf_spectacular/contrib/django_filters.py", line 136, in resolve_filter_field
    enum = [c for c, _ in filter_field.extra['choices']]
TypeError: 'method' object is not iterable

Possibly fixing this would be as simple as adding an if not callable(filter_field.extra['choices']) check before attempting to evaluate it as a list, and perhaps providing some fallback logic if that check fails?

To Reproduce
I don't have a minimized example at hand, my apologies, but hopefully the above description provides sufficient context.

Expected behavior
Schema generation should succeed; since the choices are dynamically generated at evaluation time, the generated schema presumably may not be able to provide a defined list of choices.

@tfranzel
Copy link
Owner

Hi @glennmatthews,

interesting! This has never come up before. I created a small addition to our tests

def choice_callable():
    return [('A', 'a'), ('b', 'b')]

class ProductFilter(FilterSet):
    ...
    choice_func = ChoiceFilter(field_name='category', choices=choice_callable) # won't work without field_name

The choice_callable is indeed called by django-filter, but it turns out that the filter still needs to be attached to a model field. Weirdly enough, the docs do not mention this feature at all.

Since we must have a model field, we could still get a choice list from the model field (if applicable). The result of the callable probably must be a subset of the choices in the model field

The question is what makes the most sense here? We could attempt to get a list but it could be misleading since we can only show the full list while the callable allows only a subset. We could either show the whole list (if we have one) or clear the enum altogether. What do you think?

FYI: you can override the detection like so. Theextend_schema_field decorator allows a range of arguments (including fleshed out serializer fields). Though you might need to upgrade to the newest version since there was a bugfix regarding this.

        class SomeFilter(FilterSet):
            some_field = extend_schema_field(OpenApiTypes.NUMBER)(
                RangeFilter(field_name='some_manually_annotated_field_in_qs')
            )

@glennmatthews
Copy link
Contributor Author

Thanks for the speedy response! I'd come to the same conclusion regarding the test addition - glad to see I'm on the right track with trying to reproduce and fix this!

In my opinion, the most correct behavior would be to clear the enum in this case, since we don't have an easy way to know exactly which values are in fact permitted. I'll get a PR open shortly with my attempt at implementing such logic.

Thanks for the tip on the workaround as well - I'll give that a try with our project in the meantime.

@tfranzel
Copy link
Owner

In my opinion, the most correct behavior would be to clear the enum in this case

I agree, better to be defensive but correct.

hehe thanks... I had the same changes already staged. just a minor comment over there.

@tfranzel
Copy link
Owner

awesome stuff, thanks again. just to give you a perspective. since we just released 2 days ago it will be roughly 2-3 weeks depending on the amount of issues coming up in the meantime.

@bryanculver
Copy link

@tfranzel Would you mind releasing a new version soon? We just got tripped up not being able to publish out project to PyPI by referencing a non-packaged dependency:

https://github.com/nautobot/nautobot/runs/6067093468?check_suite_focus=true

@tfranzel
Copy link
Owner

@bryanculver, yeah that won't work 😄 As a matter of fact this got further improved on master.

I have a backlog of 2-3 issues I have to take a look at. Release is coming in the following days. Won't be too long.

@tfranzel
Copy link
Owner

0.22.1 released

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants