GraphQL Pagination skips first element, when there is parameter `after` set and it's left as empty or set as zero.

[NikoGrano]

API Platform version(s) affected: 2.5.4

Description
GraphQL Pagination skips first element, when there is parameter after set and it's left as empty or set as zero.

How to reproduce

query listAll {
  products(name: "", after: "") {
    pageInfo {
      hasNextPage,
      hasPreviousPage,
    }
    totalCount,
    edges {
      cursor,
      node {
        id,
        name,
        ean,
        price,
        expiring, 
      }
    }
  }
}

or

query listAll {
  products(name: "", after: "MA==") {
    pageInfo {
      hasNextPage,
      hasPreviousPage,
    }
    totalCount,
    edges {
      cursor,
      node {
        id,
        name,
        ean,
        price,
        expiring, 
      }
    }
  }
}

Possible Solution
If parameter empty, return also first (MA==) entry also. Do not return one "after" it. If it is set to 0 (MA==), return 1 (MQ==) as first entry.
Posting a PR soon.

[NikoGrano]

PR available #3356

[alanpoulain]

I don't understand why it should work like this.
Is this a behavior you have seen in another framework? Is there a specification?

[NikoGrano]

(Replying to @alanpoulain)

There isnt many of these frameworks. However, if there is no cursor set, it should do one of following:

1. Throw a Exception for missing cursor value.
2. Not affect at all to the pagination.

My Proposed PR is doing the 2. option. It makes sure empty cursor value doesnt affect at all the result of query.

Overall I do not think there is even a specification for current behauvior, but for sure it is not the way it should work now. One of those two options should be applied in case of empty cursor value.

Skipping first value in collection when cursor is empty; is a buggy and should be consired as bug, not a feature.

[alanpoulain]

I prefer the first option then. It doesn't make sense to use an empty value IMO.

[NikoGrano]

@alanpoulain Agreed, as it is invalid request. I can do pr for that tomorrow. However, is that BC?

[alanpoulain]

Since the current behavior is somehow buggy, this is not really a BC.

[NikoGrano]

Thank you. I will be preparing PR tomorrow to throw exception

[alanpoulain]

You can reuse the error thrown just after 🙂
Don't forget to do the same for before and to add some unit tests.

[NikoGrano]

@alanpoulain As promised, PR has been submited. Waiting for your comments. Thank you.

😃

[alanpoulain]

Thanks for your PR!

[NikoGrano]

Closed in 255b22f