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

Ordering order in the variables not respected #615

Open
j30ng opened this issue Aug 23, 2024 · 2 comments
Open

Ordering order in the variables not respected #615

j30ng opened this issue Aug 23, 2024 · 2 comments
Labels
bug Something isn't working

Comments

@j30ng
Copy link

j30ng commented Aug 23, 2024

Describe the Bug

Hello. I just found out that ordering by multiple fields does not work properly when the order is in the variables, rather than in the query itself. Here's a simple example that reproduces the bug.

class MyObject(models.Model):
    name = models.CharField(max_length=10)
    date = models.DateField()

@strawberry.django.ordering.order(MyObject)
class MyObjectOrder:
    name: Ordering
    date: Ordering

@strawberry.django.type(MyObject, order=MyObjectOrder)
class MyObject:
    name: str
    date: datetime.date

@strawberry.django.connection(ListConnectionWithTotalCount[MyObject])
def objects() -> list[MyObject]:
    return MyObject.objects.all()

If you include the order (date DESC, name DESC) in the query itself, you get the correctly ordered response:

Request:

{
  "operationName": "myQuery",
  "variables": {},
  "query": "query myQuery {\n  objects(order: {date: DESC, name: DESC}) {\n    totalCount\n    edges {\n      node {\n        name\n        date\n      }\n    }\n  }\n}"
}

Response:

{
  "data": {
    "objects": {
      "totalCount": 6,
      "edges": [
        {
          "node": {
            "name": "lemon",
            "date": "2024-08-23"
          }
        },
        {
          "node": {
            "name": "banana",
            "date": "2024-08-23"
          }
        },
        {
          "node": {
            "name": "apple",
            "date": "2024-08-23"
          }
        },
        {
          "node": {
            "name": "lemon",
            "date": "2024-08-22"
          }
        },
        {
          "node": {
            "name": "banana",
            "date": "2024-08-22"
          }
        },
        {
          "node": {
            "name": "apple",
            "date": "2024-08-22"
          }
        }
      ]
    }
  }
}

whereas the response from the following exchange is not ordered as expected.

Request:

{
  "operationName": "myQuery",
  "variables": {
    "order": {
      "date": "DESC",
      "name": "DESC"
    }
  },
  "query": "query myQuery($order: MyObjectOrder) {\n  objects(order: $order) {\n    totalCount\n    edges {\n      node {\n        name\n        date\n      }\n    }\n  }\n}"
}

Response:

{
  "data": {
    "objects": {
      "totalCount": 6,
      "edges": [
        {
          "node": {
            "name": "lemon",
            "date": "2024-08-23"
          }
        },
        {
          "node": {
            "name": "lemon",
            "date": "2024-08-22"
          }
        },
        {
          "node": {
            "name": "banana",
            "date": "2024-08-23"
          }
        },
        {
          "node": {
            "name": "banana",
            "date": "2024-08-22"
          }
        },
        {
          "node": {
            "name": "apple",
            "date": "2024-08-23"
          }
        },
        {
          "node": {
            "name": "apple",
            "date": "2024-08-22"
          }
        }
      ]
    }
  }
}

System Information

  • Strawberry version (if applicable):
    strawberry-graphql==0.237.3
    strawberry-graphql-django==0.47.1

Upvote & Fund

  • We're using Polar.sh so you can upvote and help fund this issue.
  • We receive the funding once the issue is completed & confirmed by you.
  • Thank you in advance for helping prioritize & fund our backlog.
Fund with Polar
@bellini666
Copy link
Member

Hey @j30ng ,

While investigating this I just noticed that variable values ordering are currently not kept by graphql-core. I opened an issue there to discuss that to check how to proper fix this: graphql-python/graphql-core#225

@bellini666
Copy link
Member

The fix here will be to create a "v2" of the ordering stuff and use arrays instead.

I wanted to do this in the past, but then I saw that the ordering was respected and decided against it. Now seeing that the issue still exists for variables, and graphql-core will not be able to workaround that (at least not until graphql js does) shows me that this is the correct move.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants