search

encode / django-rest-framework

Web APIs for Django. 🎸
https://www.django-rest-framework.org
Other
27.83k stars 6.76k forks source link

Last ordering filter has the last say #9371

Closed tuky closed 2 months ago

tuky commented 2 months ago

Description

In https://github.com/encode/django-rest-framework/blob/63063da0820e23ef0edbf92a3031103a6c2ce254/rest_framework/generics.py#L153 every filter backend is called in the order as listed in the view's filter_backends. And since every order_by qs call clears the previous order (https://docs.djangoproject.com/en/5.0/ref/models/querysets/#django.db.models.query.QuerySet.order_by), the last ordering filter should have the final say for cursor pagination.

Although a bug fix, this poses the risk of being a major change for dependent projects.

PS: IMO, the docs should have a warning somewhere about this. It seems uncommon or unexpected from DRF's side to have multiple ordering backends and even more unexpected with cursor pagination where you have to be extra careful anyways. And there should be a warning that CursorPagination only uses ordering[0] for the cursor although it would be possible to implement a cursor on multiple fields a la Q(ordering0__gt=cursor0) | Q(ordering0__gte=cursor0, ordering1__gt=cursor1) | ... (unfolded result QS).

tomchristie commented 2 months ago

this poses the risk of being a major change for dependent projects.

So... our default has to be that we wouldn't accept changes like this.

Furthermore we really need to push towards a high cutoff bar for accepting changes, and should be ensuring we've got really clear contribution guidelines that enforce a process of...