Graphql-ruby: when setting max_page_size on a connection pageInfo hasNextPage, hasPriorPage is wrong if no first: or last: are specified

[geneeblack]

If I have a max_page_size:10 (and a default_max_page_size:5) set on a connection and do not pass a first: or last: parameter to the connection the pageInfo incorrectly reports that hasNextPage:false hasPriorPage:false.

Example issue
I set the max_page_size: 10 on a connection with a schema max_default_page_size: 5, there are a total of 39 items available on the connection. Without specifying a first: or last: the result is hasNextPage:false hasPriorPage:false on pageInfo. The result is limited to 10 items.

Works correctly
If I set the first: parameter to 50 (or any number>=10) it limits the results to 10 and reports pageInfo correctly as hasNextPage:true hasPriorPage:false.

If I set the last: parameter to 50 (or any number >= 10) it limits the results to 10 and reports pageInfo correctly as hasNextPage:false hasPriorPage:true.

If I set the first: parameter to 2 (or any number<10) it limits the results correctly and reports pageInfo correctly as hasNextPage:true hasPriorPage:false.

If I set the last: parameter to 2 (or any number < 10) it limits the results correctly and reports pageInfo correctly as hasNextPage:false hasPriorPage:true.

Assumption
There is no default ordering assumed on the connection (first or last), therefor if neither a first or last are sent it is unordered and no way to tell if there is a next or prior page.

Possible fix
set first if max_page_size or default_max_page_size are limiting the output and !first && !last here:
https://github.com/rmosolgo/graphql-ruby/blob/c1c1d1b57a5c1d76c6089106554047b09b162a1c/lib/graphql/relay/relation_connection.rb#L94

Issue code
here is where it is setting the pageInfo, if !last and !first it doesn't appear to set it correctly
https://github.com/rmosolgo/graphql-ruby/blob/c1c1d1b57a5c1d76c6089106554047b09b162a1c/lib/graphql/relay/relation_connection.rb#L28

https://github.com/rmosolgo/graphql-ruby/blob/c1c1d1b57a5c1d76c6089106554047b09b162a1c/lib/graphql/relay/relation_connection.rb#L38

[rmosolgo]

Hi, thanks for opening such a detailed issue! Sorry I haven't taken a close look yet, but I will soon, and I'll follow up here :)

[seangransee]

Confirming that I also have this issue with both 1.6.6 and 1.7.9.

[jpserra]

I'm also experiencing this issue, if first or last are not specified, hasNextPage and hasPreviousPage will be false regardless if the connection is being paginated by the max_page_size.

[jpserra]

@rmosolgo @geneeblack @seangransee Anyone managed to address/solve this issue?

meanwhile, created a gist with an example of the problem if anyone's interested:
https://gist.github.com/jpserra/9b2b0bca985c713c1eb914e35dca2298

[jpserra]

@rmosolgo could share your thoughts regarding this issue and the comments so we understand if this is actually the expected behaviour of if we can work on a fix for it? Thank you very much

[modosc]

so i think i fixed this locally via this change but i haven't opened up a pr yet since there are tests that specifically check to make sure this behavior is the way it is currently (ie, hasNextPage / hasPreviousPage don't function without first or last):

https://github.com/rmosolgo/graphql-ruby/blob/master/spec/graphql/relay/relation_connection_spec.rb#L334

https://github.com/rmosolgo/graphql-ruby/blob/master/spec/graphql/relay/relation_connection_spec.rb#L348

[rzane]

Just wanted to chime in and say that this is still an issue on 1.8.10.

I thought I had done something wrong, and since there are several issues about hasNextPage being incorrect, I had a really hard time finding this specific issue.

[modosc]

there was some conversation about it in #1623 and it seems that setting explicit defaults is the preferred solution?

however - it still doesn't make a lot of sense to me because we can't just make (say) first: 10 a default value because it'll break for anyone trying to reverse paginate. the best i've come up with is to handle this in a query analyzer and to figure out which of first/last should be set. this doesn't use graphql's schema so it's unclear to the end users that this is happening but it's basically a different way of doing what i submitted in my original pr.

definitely open to other ideas here, has anyone tested this out on other implementations to see how they handle it?

[cabello]

Hi, just reporting as also affected by this bug.

My DB had 250 records, as a proof of concept I changed the default_max_page_size to 3 and I got hasNextPage as false when providing no pagination parameters, but once I did first: 3 it returned the correct value hasNextPage as true.

[dteoh]

I'm on 1.9.10 and also having this issue. Unlike others, setting first or last has zero effect, it always returns false.

[dteoh]

I wrote my own GraphQL::Relay::BaseConnection class to replace the default ActiveRecord::Relation's connection implementation, it seems to work for our codebase.

Custom class here: https://gist.github.com/dteoh/bf063050ee3298910c70f2af1c61c879

Activate with:

class ApplicationSchema < GraphQL::Schema
  # ... other code here

  GraphQL::Relay::BaseConnection.register_connection_implementation(ActiveRecord::Relation, CustomRelayArRelationConnection)

  # ... other code here
end

[Linell]

Chiming in to say I'm also affected by this issue.

[rmosolgo]

Hi, in 1.10 I'll be releasing a new implementation of connections, and among other things, it will fix this problem. Sorry for the long wait, and thanks @dteoh for sharing your work-around!