graphql-relay-py icon indicating copy to clipboard operation
graphql-relay-py copied to clipboard

has_previous_page and has_next_page always False while navigating

Open robmoorman opened this issue 7 years ago • 4 comments

While using the pageInfo hasPreviousPage and hasNextPage cursor based navigation (before and after), it seems these results are always set to False.

The corresponding code (https://github.com/graphql-python/graphql-relay-py/blob/master/graphql_relay/connection/arrayconnection.py#L104-L105):

return connection_type(
    edges=edges,
    page_info=pageinfo_type(
        start_cursor=first_edge_cursor,
        end_cursor=last_edge_cursor,
        has_previous_page=isinstance(last, int) and start_offset > lower_bound,
        has_next_page=isinstance(first, int) and end_offset < upper_bound
    )
)

Querying with first: 5, after: "xxx" in this case always results in hasPreviousPage: false. The same for last: 5, before: "xxx", which results always in hasNextPage: false.

robmoorman avatar Sep 05 '17 13:09 robmoorman

Fixed by #14 - @robmoorman could you test to see if that fix works for you?

sciyoshi avatar Nov 22 '17 20:11 sciyoshi

Hi @sciyoshi regardless the fix you made, it's probably not in the relay spec at all (till today I have no clue why relay put this in the spec).

See https://facebook.github.io/relay/graphql/connections.htm#sec-undefined.PageInfo.Fields

robmoorman avatar Nov 23 '17 07:11 robmoorman

@robmoorman I'm looking at that spec as well. For example, for hasPreviousPage:

hasPreviousPage is used to indicate whether more edges exist prior to the set defined by the clients arguments. If the client is paginating with last/before, then the server must return true if prior edges exist, otherwise false. If the client is paginating with first/after, then the client may return true if edges prior to after exist, if it can do so efficiently, otherwise may return false.

(emphasis mine)

sciyoshi avatar Nov 23 '17 16:11 sciyoshi

I just got bitten by this too. As previously quoted, the spec says (emphasis mine):

If the client is paginating with first/after, then the client may return true if edges prior to after exist, if it can do so efficiently, otherwise may return false.

By my reading, the current behavior is technically within spec because the behavior is prescribed as "may" and not "must". That said, I suspect that like me, it's very counter-intuitive to consumers, and it cost quite a bit of time and debugging to figure out.

My sense is that this specific PR would dramatically improve the greenfield developer experience, however, I can see how it would likely break existing usages, especially considering that the default value for an unspecified slice_start param is 0. This behavior makes sense for the case where the slice itself begins in the correct place, and where it is not possible to efficiently determine what came before it.

Is there some way this could be reworked to accommodate both cases? For instance what about an optional boolean param that indicates whether the slice_start, list_length, and list_slice_length are known to be correct with respect to the entire dataset? (i.e. start_lengths_valid or something) That seems like it could avoid breaking existing consumers while dramatically reducing confusion for situations where this info is available and known to be valid (which I suspect would include many, many situations backed by traditional relational databases).

ipmcc avatar Apr 24 '18 14:04 ipmcc