From a1866b329ee3cf9b34b28b74aec72b98a8056c13 Mon Sep 17 00:00:00 2001
From: ddelange <14880945+ddelange@users.noreply.github.com>
Date: Thu, 23 Mar 2023 11:58:52 +0100
Subject: [PATCH] Add test_ascending with nulls

---
 rest_framework/pagination.py | 11 +++---
 tests/test_pagination.py     | 72 ++++++++++++++++++++++++++++++++++++
 2 files changed, 78 insertions(+), 5 deletions(-)

diff --git a/rest_framework/pagination.py b/rest_framework/pagination.py
index ff2f3684f..2555d0840 100644
--- a/rest_framework/pagination.py
+++ b/rest_framework/pagination.py
@@ -633,8 +633,9 @@ class CursorPagination(BasePagination):
                 kwargs = {order_attr + '__gt': current_position}
 
             # If some records contain a null for the ordering field, don't lose them.
-            filter_query = Q(**kwargs) | Q(**{order_attr + '__isnull': True})
-
+            filter_query = Q(**kwargs)
+            if reverse:
+                filter_query |= Q(**{order_attr + '__isnull': True})
             queryset = queryset.filter(filter_query)
 
         # If we have an offset cursor then offset the entire page by that amount.
@@ -708,7 +709,7 @@ class CursorPagination(BasePagination):
                 # The item in this position and the item following it
                 # have different positions. We can use this position as
                 # our marker.
-                has_item_with_unique_position = True
+                has_item_with_unique_position = position is not None
                 break
 
             # The item in this position has the same position as the item
@@ -761,7 +762,7 @@ class CursorPagination(BasePagination):
                 # The item in this position and the item following it
                 # have different positions. We can use this position as
                 # our marker.
-                has_item_with_unique_position = True
+                has_item_with_unique_position = position is not None
                 break
 
             # The item in this position has the same position as the item
@@ -887,7 +888,7 @@ class CursorPagination(BasePagination):
             attr = instance[field_name]
         else:
             attr = getattr(instance, field_name)
-        return str(attr)
+        return None if attr is None else str(attr)
 
     def get_paginated_response(self, data):
         return Response(OrderedDict([
diff --git a/tests/test_pagination.py b/tests/test_pagination.py
index 82c6f5a3a..1a4667f26 100644
--- a/tests/test_pagination.py
+++ b/tests/test_pagination.py
@@ -1084,6 +1084,78 @@ class TestCursorPaginationWithValueQueryset(CursorPaginationTestsMixin, TestCase
         return (previous, current, next, previous_url, next_url)
 
 
+class NullableCursorPaginationModel(models.Model):
+    created = models.IntegerField(null=True)
+
+
+class TestCursorPaginationWithNulls(TestCase):
+    """
+    Unit tests for `pagination.CursorPagination` with ordering on a nullable field.
+    """
+
+    def setUp(self):
+        class ExamplePagination(pagination.CursorPagination):
+            page_size = 1
+            ordering = 'created'
+
+        self.pagination = ExamplePagination()
+        data = [
+            None, None, 3, 4
+        ]
+        for idx in data:
+            NullableCursorPaginationModel.objects.create(created=idx)
+
+        self.queryset = NullableCursorPaginationModel.objects.all()
+
+    get_pages = TestCursorPagination.get_pages
+
+    def test_ascending(self):
+        (previous, current, next, previous_url, next_url) = self.get_pages('/')
+
+        assert previous is None
+        assert current == [None]
+        assert next == [None]
+        assert previous_url is None
+
+        (previous, current, next, previous_url, next_url) = self.get_pages(next_url)
+
+        assert previous == [None]
+        assert current == [None]
+        assert next == [3]
+
+        (previous, current, next, previous_url, next_url) = self.get_pages(next_url)
+
+        assert previous == [3]  # [None] paging artifact documented at https://github.com/ddelange/django-rest-framework/blob/3.14.0/rest_framework/pagination.py#L789
+        assert current == [3]
+        assert next == [4]
+
+        (previous, current, next, previous_url, next_url) = self.get_pages(next_url)
+
+        assert previous == [3]
+        assert current == [4]
+        assert next is None
+        assert next_url is None
+
+        (previous, current, next, previous_url, next_url) = self.get_pages(previous_url)
+
+        assert previous == [None]
+        assert current == [3]
+        assert next == [4]
+
+        (previous, current, next, previous_url, next_url) = self.get_pages(previous_url)
+
+        assert previous == [None]
+        assert current == [None]
+        assert next == [None]  # [3] paging artifact documented at https://github.com/ddelange/django-rest-framework/blob/3.14.0/rest_framework/pagination.py#L731
+
+        (previous, current, next, previous_url, next_url) = self.get_pages(previous_url)
+
+        assert previous is None
+        assert current == [None]
+        assert next == [None]
+        assert previous_url is None
+
+
 def test_get_displayed_page_numbers():
     """
     Test our contextual page display function.