Commit 0b05212f authored by Alex Kalderimis's avatar Alex Kalderimis

Fix pageInfo for preloaded associations

This corrects the behaviour of the Keyset::Connnection to take
preloaded associations into account.

Adds specs for the keyset connection, and a regression test for
the motivating example. Also ensures that we can paginate.
parent 95471a4d
---
title: Generate page-info for connections of preloaded associations
merge_request: 51642
author:
type: fixed
...@@ -67,9 +67,14 @@ module Gitlab ...@@ -67,9 +67,14 @@ module Gitlab
# next page # next page
true true
elsif first elsif first
case sliced_nodes
when Array
sliced_nodes.size > limit_value
else
# If we count the number of requested items plus one (`limit_value + 1`), # If we count the number of requested items plus one (`limit_value + 1`),
# then if we get `limit_value + 1` then we know there is a next page # then if we get `limit_value + 1` then we know there is a next page
relation_count(set_limit(sliced_nodes, limit_value + 1)) == limit_value + 1 relation_count(set_limit(sliced_nodes, limit_value + 1)) == limit_value + 1
end
else else
false false
end end
...@@ -157,8 +162,8 @@ module Gitlab ...@@ -157,8 +162,8 @@ module Gitlab
list = OrderInfo.build_order_list(items) list = OrderInfo.build_order_list(items)
if loaded?(items) if loaded?(items) && !before.present? && !after.present?
@order_list = list.presence || [items.primary_key] @order_list = list.presence || [OrderInfo.new(items.primary_key)]
# already sorted, or trivially sorted # already sorted, or trivially sorted
next items if list.present? || items.size <= 1 next items if list.present? || items.size <= 1
...@@ -194,7 +199,7 @@ module Gitlab ...@@ -194,7 +199,7 @@ module Gitlab
ordering = { 'id' => node[:id].to_s } ordering = { 'id' => node[:id].to_s }
order_list.each do |field| order_list.each do |field|
field_name = field.attribute_name field_name = field.try(:attribute_name) || field
field_value = node[field_name] field_value = node[field_name]
ordering[field_name] = if field_value.is_a?(Time) ordering[field_name] = if field_value.is_a?(Time)
field_value.strftime('%Y-%m-%d %H:%M:%S.%N %Z') field_value.strftime('%Y-%m-%d %H:%M:%S.%N %Z')
......
...@@ -40,7 +40,10 @@ module Gitlab ...@@ -40,7 +40,10 @@ module Gitlab
# "issues"."id" > 500 # "issues"."id" > 500
# #
def conditions def conditions
attr_values = order_list.map { |field| decoded_cursor[field.attribute_name] } attr_values = order_list.map do |field|
name = field.try(:attribute_name) || field
decoded_cursor[name]
end
if order_list.count == 1 && attr_values.first.nil? if order_list.count == 1 && attr_values.first.nil?
raise Gitlab::Graphql::Errors::ArgumentError.new('Before/after cursor invalid: `nil` was provided as only sortable value') raise Gitlab::Graphql::Errors::ArgumentError.new('Before/after cursor invalid: `nil` was provided as only sortable value')
......
...@@ -21,6 +21,46 @@ RSpec.describe Gitlab::Graphql::Pagination::Keyset::Connection do ...@@ -21,6 +21,46 @@ RSpec.describe Gitlab::Graphql::Pagination::Keyset::Connection do
Gitlab::Json.parse(Base64Bp.urlsafe_decode64(cursor)) Gitlab::Json.parse(Base64Bp.urlsafe_decode64(cursor))
end end
context 'regression test for #297358' do
let(:projects) { Project.all.preload(:issues) }
let(:nodes) { projects.first.issues }
before do
project = create(:project)
create_list(:issue, 3, project: project)
end
it 'is loaded' do
expect(nodes).to be_loaded
end
it 'does not error when accessing pagination information' do
connection.first = 2
expect(connection).to have_attributes(
has_previous_page: false,
has_next_page: true
)
end
it 'can generate cursors' do
connection.send(:ordered_items) # necessary to generate the order-list
expect(connection.cursor_for(nodes.first)).to be_a(String)
end
it 'can read the next page' do
connection.send(:ordered_items) # necessary to generate the order-list
ordered = nodes.reorder(id: :desc)
next_page = described_class.new(nodes,
context: context,
max_page_size: 3,
after: connection.cursor_for(ordered.second))
expect(next_page.sliced_nodes).to contain_exactly(ordered.third)
end
end
it_behaves_like 'a connection with collection methods' it_behaves_like 'a connection with collection methods'
it_behaves_like 'a redactable connection' do it_behaves_like 'a redactable connection' do
......
...@@ -252,4 +252,40 @@ RSpec.describe 'getting merge request information nested in a project' do ...@@ -252,4 +252,40 @@ RSpec.describe 'getting merge request information nested in a project' do
expect(merge_request_graphql_data['mergeStatus']).to eq('checking') expect(merge_request_graphql_data['mergeStatus']).to eq('checking')
end end
end end
context 'regression test for #297358' do
let(:query) do
<<~GQL
query($path: ID!) {
project(fullPath: $path) {
mrs: mergeRequests(first: 1) {
nodes {
participants { nodes { id } }
notes(first: 1) {
pageInfo { endCursor hasPreviousPage hasNextPage }
nodes { id }
}
}
}
}
}
GQL
end
before do
create_list(:note_on_merge_request, 3, project: project, noteable: merge_request)
end
it 'does not error' do
post_graphql(query,
current_user: current_user,
variables: { path: project.full_path })
expect(graphql_data_at(:project, :mrs, :nodes, :notes, :pageInfo)).to contain_exactly a_hash_including(
'endCursor' => String,
'hasNextPage' => true,
'hasPreviousPage' => false
)
end
end
end end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment