Skip to content

fix: backward pagination #1346

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

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 13 additions & 11 deletions graphene_django/fields.py
Original file line number Diff line number Diff line change
Expand Up @@ -152,22 +152,24 @@ def resolve_connection(cls, connection, args, iterable, max_limit=None):
array_length = iterable.count()
else:
array_length = len(iterable)
array_slice_length = (
min(max_limit, array_length) if max_limit is not None else array_length
)

# If after is higher than list_length, connection_from_list_slice
# If after is higher than array_length, connection_from_array_slice
# would try to do a negative slicing which makes django throw an
# AssertionError
slice_start = min(
get_offset_with_default(args.get("after"), -1) + 1, array_length
get_offset_with_default(args.get("after"), -1) + 1,
array_length,
)

if max_limit is not None and args.get("first", None) is None:
if args.get("last", None) is not None:
slice_start = max(array_length - args["last"], 0)
else:
args["first"] = max_limit
array_slice_length = array_length - slice_start

# Impose the maximum limit via the `first` field if neither first or last are already provided
# (note that if any of them is provided they must be under max_limit otherwise an error is raised).
if (
max_limit is not None
and args.get("first", None) is None
and args.get("last", None) is None
):
args["first"] = max_limit

connection = connection_from_array_slice(
iterable[slice_start:],
Expand Down
53 changes: 36 additions & 17 deletions graphene_django/tests/test_query.py
Original file line number Diff line number Diff line change
Expand Up @@ -1243,6 +1243,7 @@ class Query(graphene.ObjectType):
}


@pytest.mark.parametrize("max_limit", [100, 4])
class TestBackwardPagination:
def setup_schema(self, graphene_settings, max_limit):
graphene_settings.RELAY_CONNECTION_MAX_LIMIT = max_limit
Expand All @@ -1261,8 +1262,8 @@ class Query(graphene.ObjectType):
schema = graphene.Schema(query=Query)
return schema

def do_queries(self, schema):
# Simply last 3
def test_query_last(self, graphene_settings, max_limit):
schema = self.setup_schema(graphene_settings, max_limit=max_limit)
query_last = """
query {
allReporters(last: 3) {
Expand All @@ -1282,7 +1283,8 @@ def do_queries(self, schema):
e["node"]["firstName"] for e in result.data["allReporters"]["edges"]
] == ["First 3", "First 4", "First 5"]

# Use a combination of first and last
def test_query_first_and_last(self, graphene_settings, max_limit):
schema = self.setup_schema(graphene_settings, max_limit=max_limit)
query_first_and_last = """
query {
allReporters(first: 4, last: 3) {
Expand All @@ -1302,7 +1304,8 @@ def do_queries(self, schema):
e["node"]["firstName"] for e in result.data["allReporters"]["edges"]
] == ["First 1", "First 2", "First 3"]

# Use a combination of first and last and after
def test_query_first_last_and_after(self, graphene_settings, max_limit):
schema = self.setup_schema(graphene_settings, max_limit=max_limit)
query_first_last_and_after = """
query queryAfter($after: String) {
allReporters(first: 4, last: 3, after: $after) {
Expand All @@ -1317,28 +1320,44 @@ def do_queries(self, schema):

after = base64.b64encode(b"arrayconnection:0").decode()
result = schema.execute(
query_first_last_and_after, variable_values=dict(after=after)
query_first_last_and_after,
variable_values=dict(after=after),
)
assert not result.errors
assert len(result.data["allReporters"]["edges"]) == 3
assert [
e["node"]["firstName"] for e in result.data["allReporters"]["edges"]
] == ["First 2", "First 3", "First 4"]

def test_should_query(self, graphene_settings):
"""
Backward pagination should work as expected
def test_query_last_and_before(self, graphene_settings, max_limit):
schema = self.setup_schema(graphene_settings, max_limit=max_limit)
query_first_last_and_after = """
query queryAfter($before: String) {
allReporters(last: 1, before: $before) {
edges {
node {
firstName
}
}
}
}
"""
schema = self.setup_schema(graphene_settings, max_limit=100)
self.do_queries(schema)

def test_should_query_with_low_max_limit(self, graphene_settings):
"""
When doing backward pagination (using last) in combination with a max limit higher than the number of objects
we should really retrieve the last ones.
"""
schema = self.setup_schema(graphene_settings, max_limit=4)
self.do_queries(schema)
result = schema.execute(
query_first_last_and_after,
)
assert not result.errors
assert len(result.data["allReporters"]["edges"]) == 1
assert result.data["allReporters"]["edges"][0]["node"]["firstName"] == "First 5"

before = base64.b64encode(b"arrayconnection:5").decode()
result = schema.execute(
query_first_last_and_after,
variable_values=dict(before=before),
)
assert not result.errors
assert len(result.data["allReporters"]["edges"]) == 1
assert result.data["allReporters"]["edges"][0]["node"]["firstName"] == "First 4"


def test_should_preserve_prefetch_related(django_assert_num_queries):
Expand Down