Skip to content

Commit

Permalink
Fix backward Relay pagination (#1046)
Browse files Browse the repository at this point in the history
* Fix backward Relay pagination

* linting

Co-authored-by: Thomas Leonard <thomas@loftorbital.com>
  • Loading branch information
tcleonard and Thomas Leonard authored Dec 23, 2020
1 parent 4c0c821 commit 454b740
Show file tree
Hide file tree
Showing 2 changed files with 105 additions and 7 deletions.
15 changes: 8 additions & 7 deletions graphene_django/fields.py
Original file line number Diff line number Diff line change
Expand Up @@ -147,22 +147,23 @@ def resolve_connection(cls, connection, args, iterable, max_limit=None):

if isinstance(iterable, QuerySet):
list_length = iterable.count()
list_slice_length = (
min(max_limit, list_length) if max_limit is not None else list_length
)
else:
list_length = len(iterable)
list_slice_length = (
min(max_limit, list_length) if max_limit is not None else list_length
)
list_slice_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
# would try to do a negative slicing which makes django throw an
# AssertionError
after = min(get_offset_with_default(args.get("after"), -1) + 1, list_length)

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(
iterable[after:],
Expand Down
97 changes: 97 additions & 0 deletions graphene_django/tests/test_query.py
Original file line number Diff line number Diff line change
Expand Up @@ -1213,6 +1213,103 @@ class Query(graphene.ObjectType):
}


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):
class ReporterType(DjangoObjectType):
class Meta:
Expand Down

0 comments on commit 454b740

Please sign in to comment.