From 454b74052e1d3813e1e0c7f3e08c8532e68986e5 Mon Sep 17 00:00:00 2001
From: Thomas Leonard <64223923+tcleonard@users.noreply.github.com>
Date: Wed, 23 Dec 2020 05:04:45 +0100
Subject: [PATCH] Fix backward Relay pagination (#1046)

* Fix backward Relay pagination

* linting

Co-authored-by: Thomas Leonard <thomas@loftorbital.com>
---
 graphene_django/fields.py           | 15 ++---
 graphene_django/tests/test_query.py | 97 +++++++++++++++++++++++++++++
 2 files changed, 105 insertions(+), 7 deletions(-)

diff --git a/graphene_django/fields.py b/graphene_django/fields.py
index 78efceb..f7d9a19 100644
--- a/graphene_django/fields.py
+++ b/graphene_django/fields.py
@@ -147,14 +147,11 @@ class DjangoConnectionField(ConnectionField):
 
         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
@@ -162,7 +159,11 @@ class DjangoConnectionField(ConnectionField):
         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:],
diff --git a/graphene_django/tests/test_query.py b/graphene_django/tests/test_query.py
index 6add0b8..a2d8373 100644
--- a/graphene_django/tests/test_query.py
+++ b/graphene_django/tests/test_query.py
@@ -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):
     class ReporterType(DjangoObjectType):
         class Meta: