Graphene: `order_by` clause issue with django & graphql

[slorg1]

Hi,

When using the order_by clause in graphql,

• the graphql representation is correctly orderBy (camelCase).
• the value however is expected in snake case, which does not match the graphql auto generated code (it should match the API exposed field in all logic).

I tried to fix this but there are 2 places I could find where the order by is injected in a djgango qs:

1. In
   graphene.contrib.django.filter.fields.DjangoFilterConnectionField

    def get_order(self, args):
        return args.get('order_by', None)

Which does not tackle the snake case at all but at least the return is correctly handled by (in case of None (same file):

    def get_queryset(self, qs, args, info):
        filterset_class = self.filterset_class
        filter_kwargs = self.get_filter_kwargs(args)
        order = self.get_order(args)
        if order:
            qs = qs.order_by(order)
        unknown = filterset_class(data=filter_kwargs, queryset=qs)
        return unknown

This in _also_ (not sure why, maybe I did something wrong) pushed to:
django_filters.filterset.BaseFilterSet

    @property
    def qs(self):
        if not hasattr(self, '_qs'):
            valid = self.is_bound and self.form.is_valid()

            if self.strict and self.is_bound and not valid:
                self._qs = self.queryset.none()
                return self._qs

            # start with all the results and filter from there
            qs = self.queryset.all()
            for name, filter_ in six.iteritems(self.filters):
                value = None
                if valid:
                    value = self.form.cleaned_data[name]
                else:
                    raw_value = self.form[name].value()
                    try:
                        value = self.form.fields[name].clean(raw_value)
                    except forms.ValidationError:
                        # for invalid values either:
                        # strictly "apply" filter yielding no results and get outta here
                        if self.strict:
                            self._qs = self.queryset.none()
                            return self._qs
                        else:  # or ignore this filter altogether
                            pass

                if value is not None:  # valid & clean data
                    qs = filter_.filter(qs, value)

            if self._meta.order_by:
                order_field = self.form.fields[self.order_by_field]
                data = self.form[self.order_by_field].data
                ordered_value = None
                try:
                    ordered_value = order_field.clean(data)
                except forms.ValidationError:
                    pass

                if ordered_value in EMPTY_VALUES and self.strict:
                    ordered_value = self.form.fields[self.order_by_field].choices[0][0]

                if ordered_value:
                    qs = qs.order_by(*self.get_order_by(ordered_value))

            self._qs = qs

        return self._qs

Which does not handle at all the None return for get_order_by.

1. When I tested this, I got it not to crash turning the camelCase to snake case in the 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 #2 did not seem to kick in)
2. When I tested sorting by a composite field (field that I had defined on the Node manually and not part of the django model) is when I hit #1 and #2 here. I could not get this working at all.
3. When the system finds an 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 the order_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.

[jkimbo]

@slorg1 I submitted a pull request for this (https://github.com/graphql-python/graphene/pull/238) and it was merged a couple of weeks ago. It's possible it just hasn't been published yet?

[slorg1]

@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 DjangoFilterConnectionField

It does not address the secondary issue issue I had. Maybe I should open 2 git issues. Suggestions?

[slorg1]

@jkimbo I added a couple questions to your PR. Thank you.

[jkimbo]

@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

[syrusakbary]

@slorg1 this should be fixed in 1.0.dev, you can install the latest Django version using pip install graphene-django>=1.0.dev.

Please reopen this issue if you run into the same error :)

[farnoodma]

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:
Unknown argument \"orderBy\" on field ...
or if you use order_by:
Unknown argument \"order_by\" on field ...

There is unused order_by in DjangoFilterConnectionField:

    def __init__(self, type, fields=None, order_by=None,
                 extra_filter_meta=None, filterset_class=None,
                 *args, **kwargs):
        self._fields = fields
        self._provided_filterset_class = filterset_class
        self._filterset_class = None
        self._extra_filter_meta = extra_filter_meta
        self._base_args = None
        super(DjangoFilterConnectionField, self).__init__(type, *args, **kwargs)

And also there is no filter_order_by in DjangoObjectType Meta fields. (which mentioned in other issue)
Please let me know how should I order my objects in latest version?

[Mhs-220]

@farnoodma Hey there!
I had the same problem but finally, I fix it!
Here is my code

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

[giorgi94]

@Mhs-220 Thanks for OrderedDjangoFilterConnectionField. I spent couple of weeks with this problem. But is there any problem with pagination? It uses Base64 encoding, and to simplify working with it, I wrote this functions

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)

[jkimbo]

@giorgi94 if you are having an issue with graphene-django can you please open an issue on that repo

[Mhs-220]

@giorgi94 hey again, no there is no pagination bug for me( i use default graphene pagination )

[androane]

@Mhs-220 Your solution shouldn't work and indeed is not working. When you compute qs you pass it to the super method as the default_manager. The order is not preserved

[Mhs-220]

@androane I copy this method from here and just added three line to it and it's working for me properly

[androane]

Sorry, was a mistake from my side. Digging into the code I saw that default_manager can be a queryset as well. And it wasn't actually working for me because I was trying with a String isntead a list of Strings

[alokRamteke]

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.

DjangoFilterConnectionField methods are changed in the 2.7.0 version.
For more details, you can check the changelogs here

The solution will generate error, connection_resolver() missing 1 required positional argument: 'info’.

I have modified the solution and it worked perfectly.

````python
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

````

[donegjookim]

Why isn't this part of DjangoFilterConnectionField?

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.

DjangoFilterConnectionField methods are changed in the 2.7.0 version.
For more details, you can check the changelogs here

The solution will generate error, connection_resolver() missing 1 required positional argument: 'info’.

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