Fix backward Relay pagination (#1046)

* Fix backward Relay pagination

* linting

Co-authored-by: Thomas Leonard <thomas@loftorbital.com>
This commit is contained in:
Thomas Leonard 2020-12-23 05:04:45 +01:00 committed by GitHub
parent 4c0c821b74
commit 454b74052e
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 105 additions and 7 deletions

View File

@ -147,14 +147,11 @@ class DjangoConnectionField(ConnectionField):
if isinstance(iterable, QuerySet): if isinstance(iterable, QuerySet):
list_length = iterable.count() list_length = iterable.count()
list_slice_length = (
min(max_limit, list_length) if max_limit is not None else list_length
)
else: else:
list_length = len(iterable) list_length = len(iterable)
list_slice_length = ( list_slice_length = (
min(max_limit, list_length) if max_limit is not None else list_length min(max_limit, list_length) if max_limit is not None else list_length
) )
# If after is higher than list_length, connection_from_list_slice # If after is higher than list_length, connection_from_list_slice
# would try to do a negative slicing which makes django throw an # would try to do a negative slicing which makes django throw an
@ -162,7 +159,11 @@ class DjangoConnectionField(ConnectionField):
after = min(get_offset_with_default(args.get("after"), -1) + 1, list_length) after = min(get_offset_with_default(args.get("after"), -1) + 1, list_length)
if max_limit is not None and "first" not in args: if max_limit is not None and "first" not in args:
args["first"] = max_limit if "last" in args:
args["first"] = list_length
list_slice_length = list_length
else:
args["first"] = max_limit
connection = connection_from_list_slice( connection = connection_from_list_slice(
iterable[after:], iterable[after:],

View File

@ -1213,6 +1213,103 @@ def test_should_have_next_page(graphene_settings):
} }
class TestBackwardPagination:
def setup_schema(self, graphene_settings, max_limit):
graphene_settings.RELAY_CONNECTION_MAX_LIMIT = max_limit
reporters = [Reporter(**kwargs) for kwargs in REPORTERS]
Reporter.objects.bulk_create(reporters)
class ReporterType(DjangoObjectType):
class Meta:
model = Reporter
interfaces = (Node,)
class Query(graphene.ObjectType):
all_reporters = DjangoConnectionField(ReporterType)
schema = graphene.Schema(query=Query)
return schema
def do_queries(self, schema):
# Simply last 3
query_last = """
query {
allReporters(last: 3) {
edges {
node {
firstName
}
}
}
}
"""
result = schema.execute(query_last)
assert not result.errors
assert len(result.data["allReporters"]["edges"]) == 3
assert [
e["node"]["firstName"] for e in result.data["allReporters"]["edges"]
] == ["First 3", "First 4", "First 5"]
# Use a combination of first and last
query_first_and_last = """
query {
allReporters(first: 4, last: 3) {
edges {
node {
firstName
}
}
}
}
"""
result = schema.execute(query_first_and_last)
assert not result.errors
assert len(result.data["allReporters"]["edges"]) == 3
assert [
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
query_first_last_and_after = """
query queryAfter($after: String) {
allReporters(first: 4, last: 3, after: $after) {
edges {
node {
firstName
}
}
}
}
"""
after = base64.b64encode(b"arrayconnection:0").decode()
result = schema.execute(
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
"""
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)
def test_should_preserve_prefetch_related(django_assert_num_queries): def test_should_preserve_prefetch_related(django_assert_num_queries):
class ReporterType(DjangoObjectType): class ReporterType(DjangoObjectType):
class Meta: class Meta: