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

Select extra node in pagination relations for free hasNextPage query #4908

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
17 changes: 16 additions & 1 deletion lib/graphql/pagination/relation_connection.rb
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,22 @@ def limited_nodes
# returns an array of nodes
def load_nodes
# Return an array so we can consistently use `.index(node)` on it
@nodes ||= limited_nodes.to_a
@nodes ||= begin
original_node_limit = relation_limit(limited_nodes)
if original_node_limit
overshot_nodes = set_limit(limited_nodes, original_node_limit + 1).to_a
if overshot_nodes.size > original_node_limit
@has_next_page = true
overshot_nodes[0...-1]
else
# we didn't overshoot and there are no next pages
@has_next_page = false
overshot_nodes
end
else
limited_nodes.to_a
end
end
end
end
end
Expand Down
27 changes: 24 additions & 3 deletions spec/graphql/pagination/active_record_relation_connection_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,10 @@ def total_count
assert_equal ["Item"] * 10, results["data"]["items"]["nodes"].map { |i| i["__typename"] }
assert_equal 1, log.split("\n").size, "It runs only one query when less than total count is requested"
assert_equal 0, log.scan("COUNT(").size, "It runs no count query"
end

it "runs one query for both `nodes` and `hasNextPage` if nodes happen to be selected first" do
results = nil
log = with_active_record_log do
results = schema.execute("{
items(first: 3) {
Expand All @@ -172,10 +175,28 @@ def total_count
}
}")
end
# This currently runs one query to load the nodes, then another one to count _just beyond_ the nodes.
# A better implementation would load `first + 1` nodes and use that to set `has_next_page`.
assert_equal ["Item", "Item", "Item"], results["data"]["items"]["nodes"].map { |i| i["__typename"] }
assert_equal 2, log.split("\n").size, "It runs two queries -- TODO this could be better"
assert_equal 1, log.split("\n").size, "It runs only one query"
assert_equal 0, log.scan("COUNT(").size, "It runs no count query"
end

it "runs two queries for `nodes` and `hasNextPage` if hasNextPage is selected first" do
results = nil
log = with_active_record_log do
results = schema.execute("{
items(first: 3) {
pageInfo {
hasNextPage
}
nodes {
__typename
}
}
}")
end
assert_equal ["Item", "Item", "Item"], results["data"]["items"]["nodes"].map { |i| i["__typename"] }
assert_equal 2, log.split("\n").size, "It runs two queries"
assert_equal 1, log.scan("COUNT(").size, "It runs one count query"
end

describe "already-loaded ActiveRecord relations" do
Expand Down