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

Fix pagination for 'last' page. #1074

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

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
12 changes: 5 additions & 7 deletions graphene_django/fields.py
Original file line number Diff line number Diff line change
Expand Up @@ -147,21 +147,19 @@ 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 = list_length
if max_limit is not None and "last" not in args:
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Without checking for last here and below, list_slice_length was being set to 4, and after to 0, slicing off reporters 5-8.

Query for last 2 reporters should be getting 7 and 8, but it wasn't getting those correctly.

list_slice_length = min(max_limit, 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:
if max_limit is not None and "first" not in args and "last" not in args:
args["first"] = max_limit

connection = connection_from_list_slice(
Expand Down
3 changes: 3 additions & 0 deletions graphene_django/tests/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,9 @@ def __init__(self, *args, **kwargs):
def some_method(self):
return 123

class Meta:
ordering = ("id",)


class CNNReporterManager(models.Manager):
def get_queryset(self):
Expand Down
52 changes: 52 additions & 0 deletions graphene_django/tests/test_query.py
Original file line number Diff line number Diff line change
Expand Up @@ -727,6 +727,58 @@ class Query(graphene.ObjectType):
assert result.data == expected


def test_should_return_last_page_if_more_than_max_limit(graphene_settings):
graphene_settings.RELAY_CONNECTION_MAX_LIMIT = 4

class ReporterType(DjangoObjectType):
class Meta:
model = Reporter
interfaces = (Node,)

class Query(graphene.ObjectType):
all_reporters = DjangoConnectionField(ReporterType)

assert Query.all_reporters.max_limit == 4

reporters = [Reporter(**kwargs) for kwargs in REPORTERS]
Reporter.objects.bulk_create(reporters)

r = Reporter.objects.create(
first_name="John", last_name="Doe", email="johndoe@example.com", a_choice=1
)

r2 = Reporter.objects.create(
first_name="Jane", last_name="Doe", email="janedoe@example.com", a_choice=2
)

schema = graphene.Schema(query=Query)
query = """
query NodeFilteringQuery {
allReporters(last: 2) {
edges {
node {
id
}
}
}
}
"""

expected = {
"allReporters": {
"edges": [
{"node": {"id": to_global_id("ReporterType", r.id)}},
{"node": {"id": to_global_id("ReporterType", r2.id)}},
]
}
}

result = schema.execute(query)
assert not result.errors
assert len(result.data["allReporters"]["edges"]) == 2
assert result.data == expected


def test_should_query_promise_connectionfields():
from promise import Promise

Expand Down