Skip to content

Bugfix: Correct filter types for DjangoFilterConnectionFields #682

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

Merged
merged 4 commits into from
Jul 7, 2019

Conversation

jkimbo
Copy link
Member

@jkimbo jkimbo commented Jun 19, 2019

Fixes #678

For Django IntegerFields, Django-filter converts it to a NumberFilter and NumberFilter defines it's Django form field as a DecimalField. Graphene-Django then uses that form field to determine the GraphQL type to use for the filter, so in this case an IntegerField defined on the model gets converted into a Float argument when used as a filter.

This PR changes the logic so that (if it can) it uses the Django model field to determine the corresponding form field rather than Django-filter.

In this PR I've also cleaned up the test_fields file a bit and added a black compatible config for isort.

Copy link
Collaborator

@zbyte64 zbyte64 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, just have the one question: should we be converting based off the model field instead?

@jkimbo jkimbo force-pushed the bugfix/django-filters-number-types branch from e3fe285 to 53d81ef Compare June 25, 2019 09:36
@jkimbo jkimbo requested a review from zbyte64 June 25, 2019 09:37
Copy link
Member

@mvanlonden mvanlonden left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work!

@mvanlonden mvanlonden merged commit aa30750 into master Jul 7, 2019
@mvanlonden mvanlonden deleted the bugfix/django-filters-number-types branch July 7, 2019 19:11
@jkimbo jkimbo mentioned this pull request Jul 9, 2019
sliverc added a commit to sliverc/caluma that referenced this pull request Jul 18, 2019
This is an regression of
graphql-python/graphene-django#682
but because upstream doesn't support enum for filters it's
not an regression for them.

Before filter field was converted now the form field is.
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 this pull request may close these issues.

Django integer fields gets transformed to Float?
3 participants