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
Fix filtering with GlobalIDFilter #977
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks like a good change but the tests are failing @hubertsiuzdak
graphene_django/filter/fields.py
Outdated
else: | ||
raise ValidationError(json.dumps(filterset.errors.get_json_data())) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
else: | |
raise ValidationError(json.dumps(filterset.errors.get_json_data())) | |
raise ValidationError(json.dumps(filterset.errors.get_json_data())) |
No need for the else
statement.
51d2020
to
434a776
Compare
if filterset.form.is_valid(): | ||
return filterset.qs | ||
raise ValidationError(filterset.form.errors.as_json()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To support Python 2.7 and django-filter 1.1 that form
property is needed. In Graphene-Django v3 we use django-filter 2.x which allows to call is_valid
and errors
directly on filterset instances. We may want to refactor that for v3 as it slightly improves readability.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work.
@jkimbo Can you take a look again? |
Thanks @hubertsiuzdak |
Released in v2.11.0 |
resolves #808, resolves #542
Filtering with not valid global ID doesn't raise any exception, and the query set without filter applied is returned instead. That happens due to the fact that django-filter no longer performs error handling itself (see carltongibson/django-filter#1078). This PR solves the issue by validating the filterset in DjangoFilterConnectionField.