From 2140be5e6a2d3a789906f34bf96c9663dfc075be Mon Sep 17 00:00:00 2001 From: Thomas Leonard <64223923+tcleonard@users.noreply.github.com> Date: Mon, 26 Oct 2020 17:09:21 +0100 Subject: [PATCH] Add offset pagination (#1013) * Add offset filtering * Formatting Co-authored-by: Thomas Leonard --- graphene_django/fields.py | 21 ++- graphene_django/filter/tests/test_fields.py | 6 +- graphene_django/tests/test_query.py | 141 ++++++++++++++++++++ graphene_django/tests/test_types.py | 2 +- 4 files changed, 165 insertions(+), 5 deletions(-) diff --git a/graphene_django/fields.py b/graphene_django/fields.py index 67559aa..78efceb 100644 --- a/graphene_django/fields.py +++ b/graphene_django/fields.py @@ -4,11 +4,13 @@ import six from django.db.models.query import QuerySet from graphql_relay.connection.arrayconnection import ( connection_from_list_slice, + cursor_to_offset, get_offset_with_default, + offset_to_cursor, ) from promise import Promise -from graphene import NonNull +from graphene import Int, NonNull from graphene.relay import ConnectionField, PageInfo from graphene.types import Field, List @@ -81,6 +83,7 @@ class DjangoConnectionField(ConnectionField): "enforce_first_or_last", graphene_settings.RELAY_CONNECTION_ENFORCE_FIRST_OR_LAST, ) + kwargs.setdefault("offset", Int()) super(DjangoConnectionField, self).__init__(*args, **kwargs) @property @@ -131,6 +134,15 @@ class DjangoConnectionField(ConnectionField): @classmethod def resolve_connection(cls, connection, args, iterable, max_limit=None): + # Remove the offset parameter and convert it to an after cursor. + offset = args.pop("offset", None) + after = args.get("after") + if offset: + if after: + offset += cursor_to_offset(after) + 1 + # input offset starts at 1 while the graphene offset starts at 0 + args["after"] = offset_to_cursor(offset - 1) + iterable = maybe_queryset(iterable) if isinstance(iterable, QuerySet): @@ -181,6 +193,8 @@ class DjangoConnectionField(ConnectionField): ): first = args.get("first") last = args.get("last") + offset = args.get("offset") + before = args.get("before") if enforce_first_or_last: assert first or last, ( @@ -200,6 +214,11 @@ class DjangoConnectionField(ConnectionField): ).format(last, info.field_name, max_limit) args["last"] = min(last, max_limit) + if offset is not None: + assert before is None, ( + "You can't provide a `before` value at the same time as an `offset` value to properly paginate the `{}` connection." + ).format(info.field_name) + # eventually leads to DjangoObjectType's get_queryset (accepts queryset) # or a resolve_foo (does not accept queryset) iterable = resolver(root, info, **args) diff --git a/graphene_django/filter/tests/test_fields.py b/graphene_django/filter/tests/test_fields.py index b8ae6fe..88749aa 100644 --- a/graphene_django/filter/tests/test_fields.py +++ b/graphene_django/filter/tests/test_fields.py @@ -59,7 +59,7 @@ def get_args(field): def assert_arguments(field, *arguments): - ignore = ("after", "before", "first", "last", "order_by") + ignore = ("offset", "after", "before", "first", "last", "order_by") args = get_args(field) actual = [name for name in args if name not in ignore and not name.startswith("_")] assert set(arguments) == set( @@ -945,7 +945,7 @@ def test_integer_field_filter_type(): } type Query { - pets(before: String, after: String, first: Int, last: Int, age: Int): PetTypeConnection + pets(offset: Int, before: String, after: String, first: Int, last: Int, age: Int): PetTypeConnection } """ ) @@ -997,7 +997,7 @@ def test_other_filter_types(): } type Query { - pets(before: String, after: String, first: Int, last: Int, age: Int, age_Isnull: Boolean, age_Lt: Int): PetTypeConnection + pets(offset: Int, before: String, after: String, first: Int, last: Int, age: Int, age_Isnull: Boolean, age_Lt: Int): PetTypeConnection } """ ) diff --git a/graphene_django/tests/test_query.py b/graphene_django/tests/test_query.py index 3881ed8..6add0b8 100644 --- a/graphene_django/tests/test_query.py +++ b/graphene_django/tests/test_query.py @@ -1314,3 +1314,144 @@ def test_should_preserve_annotations(): } } assert result.data == expected, str(result.data) + + +def test_connection_should_enable_offset_filtering(): + Reporter.objects.create(first_name="John", last_name="Doe") + Reporter.objects.create(first_name="Some", last_name="Guy") + + class ReporterType(DjangoObjectType): + class Meta: + model = Reporter + interfaces = (Node,) + + class Query(graphene.ObjectType): + all_reporters = DjangoConnectionField(ReporterType) + + schema = graphene.Schema(query=Query) + query = """ + query { + allReporters(first: 1, offset: 1) { + edges { + node { + firstName + lastName + } + } + } + } + """ + + result = schema.execute(query) + assert not result.errors + expected = { + "allReporters": {"edges": [{"node": {"firstName": "Some", "lastName": "Guy"}},]} + } + assert result.data == expected + + +def test_connection_should_enable_offset_filtering_higher_than_max_limit( + graphene_settings, +): + graphene_settings.RELAY_CONNECTION_MAX_LIMIT = 2 + Reporter.objects.create(first_name="John", last_name="Doe") + Reporter.objects.create(first_name="Some", last_name="Guy") + Reporter.objects.create(first_name="Jane", last_name="Roe") + Reporter.objects.create(first_name="Some", last_name="Lady") + + class ReporterType(DjangoObjectType): + class Meta: + model = Reporter + interfaces = (Node,) + + class Query(graphene.ObjectType): + all_reporters = DjangoConnectionField(ReporterType) + + schema = graphene.Schema(query=Query) + query = """ + query { + allReporters(first: 1, offset: 3) { + edges { + node { + firstName + lastName + } + } + } + } + """ + + result = schema.execute(query) + assert not result.errors + expected = { + "allReporters": { + "edges": [{"node": {"firstName": "Some", "lastName": "Lady"}},] + } + } + assert result.data == expected + + +def test_connection_should_forbid_offset_filtering_with_before(): + class ReporterType(DjangoObjectType): + class Meta: + model = Reporter + interfaces = (Node,) + + class Query(graphene.ObjectType): + all_reporters = DjangoConnectionField(ReporterType) + + schema = graphene.Schema(query=Query) + query = """ + query ReporterPromiseConnectionQuery ($before: String) { + allReporters(first: 1, before: $before, offset: 1) { + edges { + node { + firstName + lastName + } + } + } + } + """ + before = base64.b64encode(b"arrayconnection:2").decode() + result = schema.execute(query, variable_values=dict(before=before)) + expected_error = "You can't provide a `before` value at the same time as an `offset` value to properly paginate the `allReporters` connection." + assert len(result.errors) == 1 + assert result.errors[0].message == expected_error + + +def test_connection_should_allow_offset_filtering_with_after(): + Reporter.objects.create(first_name="John", last_name="Doe") + Reporter.objects.create(first_name="Some", last_name="Guy") + Reporter.objects.create(first_name="Jane", last_name="Roe") + Reporter.objects.create(first_name="Some", last_name="Lady") + + class ReporterType(DjangoObjectType): + class Meta: + model = Reporter + interfaces = (Node,) + + class Query(graphene.ObjectType): + all_reporters = DjangoConnectionField(ReporterType) + + schema = graphene.Schema(query=Query) + query = """ + query ReporterPromiseConnectionQuery ($after: String) { + allReporters(first: 1, after: $after, offset: 1) { + edges { + node { + firstName + lastName + } + } + } + } + """ + + after = base64.b64encode(b"arrayconnection:0").decode() + result = schema.execute(query, variable_values=dict(after=after)) + assert not result.errors + expected = { + "allReporters": {"edges": [{"node": {"firstName": "Jane", "lastName": "Roe"}},]} + } + assert result.data == expected diff --git a/graphene_django/tests/test_types.py b/graphene_django/tests/test_types.py index fb95820..ce9a29b 100644 --- a/graphene_django/tests/test_types.py +++ b/graphene_django/tests/test_types.py @@ -172,7 +172,7 @@ type Reporter { pets: [Reporter!]! aChoice: ReporterAChoice reporterType: ReporterReporterType - articles(before: String, after: String, first: Int, last: Int): ArticleConnection! + articles(offset: Int, before: String, after: String, first: Int, last: Int): ArticleConnection! } enum ReporterAChoice {