-
Notifications
You must be signed in to change notification settings - Fork 819
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
order_by
clause issue with django & graphql
#251
Comments
@jkimbo it does not seem to be in the newest version (0.10.2) that I got from pip. Also, thank you for your fix. I see you saw the same issue I did. In your changes I see the change to the It does not address the secondary issue issue I had. Maybe I should open 2 git issues. Suggestions? |
@jkimbo I added a couple questions to your PR. Thank you. |
@slorg1 I don't know much about your second issue I'm afraid but if you could create a failing test case for it that would help immensely |
@slorg1 this should be fixed in Please reopen this issue if you run into the same error :) |
Ok whats going on here? There is no orderBy (or order_by) documentation and also there is no order_by function or variable in DjangoFilterConnectionField! Latest version will throw this error: There is unused order_by in DjangoFilterConnectionField:
And also there is no |
@farnoodma Hey there! class OrderedDjangoFilterConnectionField(DjangoFilterConnectionField):
@classmethod
def connection_resolver(cls, resolver, connection, default_manager, max_limit,
enforce_first_or_last, filterset_class, filtering_args,
root, info, **args):
filter_kwargs = {k: v for k, v in args.items() if k in filtering_args}
qs = filterset_class(
data=filter_kwargs,
queryset=default_manager.get_queryset(),
request=info.context
).qs
order = args.get('orderBy', None)
if order:
qs = qs.order_by(*order)
return super(DjangoFilterConnectionField, cls).connection_resolver(
resolver,
connection,
qs,
max_limit,
enforce_first_or_last,
root,
info,
**args
) and then, just use it instead of DjangoFilterConnectionField, like this: person = OrderedDjangoFilterConnectionField( PersonNode, orderBy=graphene.List(of_type=graphene.String) ) and then, the query is something like this: query {
person (orderBy: ["field1","-field2"...]){
edges{
node{
name
}
}
}
} P.S: fix pagination bug |
@Mhs-220 Thanks for import base64
def encode(e):
# BlogNode:2 -> QmxvZ05vZGU6Mg==
try:
return base64.urlsafe_b64encode(e.encode()).decode()
except:
return None
def decode(e):
# QmxvZ05vZGU6Mg== -> BlogNode:2
try:
return base64.urlsafe_b64decode(e).decode()
except:
return None
def cursor_page(page, per_page):
first = per_page
endCursor = encode('arrayconnection:' + str(page * first - 1))
return (first, endCursor) |
@giorgi94 hey again, no there is no pagination bug for me( i use default graphene pagination ) |
@Mhs-220 Your solution shouldn't work and indeed is not working. When you compute |
Sorry, was a mistake from my side. Digging into the code I saw that |
Update: Mhs-220 solution won't work with the current graphene-django version(2.9.1) or greater than graphene-django 2.6.0 version.
The solution will generate error, I have modified the solution and it worked perfectly. from graphene_django.filter import DjangoFilterConnectionField
from graphene.utils.str_converters import to_snake_case
class OrderedDjangoFilterConnectionField(DjangoFilterConnectionField):
@classmethod
def resolve_queryset(
cls, connection, iterable, info, args, filtering_args, filterset_class
):
qs = super(DjangoFilterConnectionField, cls).resolve_queryset(
connection, iterable, info, args
)
filter_kwargs = {k: v for k, v in args.items() if k in filtering_args}
qs = filterset_class(data=filter_kwargs, queryset=qs, request=info.context).qs
order = args.get('orderBy', None)
if order:
if type(order) is str:
snake_order = to_snake_case(order)
else:
snake_order = [to_snake_case(o) for o in order]
qs = qs.order_by(*snake_order)
return qs |
Why isn't this part of
|
Hi,
When using the
order_by
clause in graphql,orderBy
(camelCase).I tried to fix this but there are 2 places I could find where the order by is injected in a djgango qs:
graphene.contrib.django.filter.fields.DjangoFilterConnectionField
Which does not tackle the snake case at all but at least the return is correctly handled by (in case of
None
(same file):This in also (not sure why, maybe I did something wrong) pushed to:
django_filters.filterset.BaseFilterSet
Which does not handle at all the
None
return forget_order_by
.get_order
method, however after that, while the qs was correct (I printed it out) the return from the API was systematically empty... (in that case Django Models integration #2 did not seem to kick in)Node
manually and not part of the django model) is when I hit Relay #1 and Django Models integration #2 here. I could not get this working at all.order_by
it does not know: it prints the django error which exposes all the fields of the raw django model to the API: not super secure... It seems like the system does not validate theorder_by
"clause" / arguments before sending it to django.This is what I found after a couple days of debugging and trying to go around the issue. Please let me know if I can help in any way. The doc was very light on the subject and I understand from slack that @syrusakbary has code changes.
I did not know the best way to help you/us resolve this. Thank you.
The text was updated successfully, but these errors were encountered: