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
This commit is contained in:
fdomain 2023-05-02 02:55:59 +02:00 committed by GitHub
parent 54307a4394
commit f1a11d41cb
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 35 additions and 20 deletions

View File

@ -801,6 +801,10 @@ class CursorPagination(BasePagination):
""" """
Return a tuple of strings, that may be used in an `order_by` method. 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 = [ ordering_filters = [
filter_cls for filter_cls in getattr(view, 'filter_backends', []) filter_cls for filter_cls in getattr(view, 'filter_backends', [])
if hasattr(filter_cls, 'get_ordering') if hasattr(filter_cls, 'get_ordering')
@ -811,26 +815,19 @@ class CursorPagination(BasePagination):
# then we defer to that filter to determine the ordering. # then we defer to that filter to determine the ordering.
filter_cls = ordering_filters[0] filter_cls = ordering_filters[0]
filter_instance = filter_cls() filter_instance = filter_cls()
ordering = filter_instance.get_ordering(request, queryset, view) ordering_from_filter = filter_instance.get_ordering(request, queryset, view)
assert ordering is not None, ( if ordering_from_filter:
'Using cursor pagination, but filter class {filter_cls} ' ordering = ordering_from_filter
'returned a `None` ordering.'.format(
filter_cls=filter_cls.__name__ assert ordering is not None, (
) 'Using cursor pagination, but no ordering attribute was declared '
) 'on the pagination class.'
else: )
# The default case is to check for an `ordering` attribute assert '__' not in ordering, (
# on this pagination instance. 'Cursor pagination does not support double underscore lookups '
ordering = self.ordering 'for orderings. Orderings should be an unchanging, unique or '
assert ordering is not None, ( 'nearly-unique field on the model, such as "-created" or "pk".'
'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)), ( assert isinstance(ordering, (str, list, tuple)), (
'Invalid ordering. Expected string or tuple, but got {type}'.format( 'Invalid ordering. Expected string or tuple, but got {type}'.format(

View File

@ -632,6 +632,24 @@ class CursorPaginationTestsMixin:
ordering = self.pagination.get_ordering(request, [], MockView()) ordering = self.pagination.get_ordering(request, [], MockView())
assert ordering == ('created',) 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): def test_cursor_pagination(self):
(previous, current, next, previous_url, next_url) = self.get_pages('/') (previous, current, next, previous_url, next_url) = self.get_pages('/')