From f1a11d41cbd1d16871e8d8426cbf290da57dfb6d Mon Sep 17 00:00:00 2001
From: fdomain <domain.florian@gmail.com>
Date: Tue, 2 May 2023 02:55:59 +0200
Subject: [PATCH] fix: fallback on CursorPagination ordering if unset on the
 view (#8954)

* this commit fixes the usage of a CursorPagination combined with a view
  implementing an ordering filter, without a default ordering value.
* former behavior was to fetch the ordering value from the filter, and
  raises an error if the value was None, preventing the fallback on the
  ordering set on the CursorPagination class itself.
* we reversed the logic by getting first the value set on the class,
  and override it by the ordering filter if the parameter is present
---
 rest_framework/pagination.py | 37 +++++++++++++++++-------------------
 tests/test_pagination.py     | 18 ++++++++++++++++++
 2 files changed, 35 insertions(+), 20 deletions(-)

diff --git a/rest_framework/pagination.py b/rest_framework/pagination.py
index 68ab9c786..af508bef6 100644
--- a/rest_framework/pagination.py
+++ b/rest_framework/pagination.py
@@ -801,6 +801,10 @@ class CursorPagination(BasePagination):
         """
         Return a tuple of strings, that may be used in an `order_by` method.
         """
+        # The default case is to check for an `ordering` attribute
+        # on this pagination instance.
+        ordering = self.ordering
+
         ordering_filters = [
             filter_cls for filter_cls in getattr(view, 'filter_backends', [])
             if hasattr(filter_cls, 'get_ordering')
@@ -811,26 +815,19 @@ class CursorPagination(BasePagination):
             # then we defer to that filter to determine the ordering.
             filter_cls = ordering_filters[0]
             filter_instance = filter_cls()
-            ordering = filter_instance.get_ordering(request, queryset, view)
-            assert ordering is not None, (
-                'Using cursor pagination, but filter class {filter_cls} '
-                'returned a `None` ordering.'.format(
-                    filter_cls=filter_cls.__name__
-                )
-            )
-        else:
-            # The default case is to check for an `ordering` attribute
-            # on this pagination instance.
-            ordering = self.ordering
-            assert ordering is not None, (
-                'Using cursor pagination, but no ordering attribute was declared '
-                'on the pagination class.'
-            )
-            assert '__' not in ordering, (
-                'Cursor pagination does not support double underscore lookups '
-                'for orderings. Orderings should be an unchanging, unique or '
-                'nearly-unique field on the model, such as "-created" or "pk".'
-            )
+            ordering_from_filter = filter_instance.get_ordering(request, queryset, view)
+            if ordering_from_filter:
+                ordering = ordering_from_filter
+
+        assert ordering is not None, (
+            'Using cursor pagination, but no ordering attribute was declared '
+            'on the pagination class.'
+        )
+        assert '__' not in ordering, (
+            'Cursor pagination does not support double underscore lookups '
+            'for orderings. Orderings should be an unchanging, unique or '
+            'nearly-unique field on the model, such as "-created" or "pk".'
+        )
 
         assert isinstance(ordering, (str, list, tuple)), (
             'Invalid ordering. Expected string or tuple, but got {type}'.format(
diff --git a/tests/test_pagination.py b/tests/test_pagination.py
index 8f9b20a0d..d606986ab 100644
--- a/tests/test_pagination.py
+++ b/tests/test_pagination.py
@@ -632,6 +632,24 @@ class CursorPaginationTestsMixin:
         ordering = self.pagination.get_ordering(request, [], MockView())
         assert ordering == ('created',)
 
+    def test_use_with_ordering_filter_without_ordering_default_value(self):
+        class MockView:
+            filter_backends = (filters.OrderingFilter,)
+            ordering_fields = ['username', 'created']
+
+        request = Request(factory.get('/'))
+        ordering = self.pagination.get_ordering(request, [], MockView())
+        # it gets the value of `ordering` provided by CursorPagination
+        assert ordering == ('created',)
+
+        request = Request(factory.get('/', {'ordering': 'username'}))
+        ordering = self.pagination.get_ordering(request, [], MockView())
+        assert ordering == ('username',)
+
+        request = Request(factory.get('/', {'ordering': 'invalid'}))
+        ordering = self.pagination.get_ordering(request, [], MockView())
+        assert ordering == ('created',)
+
     def test_cursor_pagination(self):
         (previous, current, next, previous_url, next_url) = self.get_pages('/')