From b0e18bf83f5af4508b9448fa2b19f9b2e16a2097 Mon Sep 17 00:00:00 2001 From: Oliver Sauder Date: Wed, 7 Oct 2026 15:32:51 +0400 Subject: [PATCH 1/2] Moved ordering filter tests to tests module --- example/tests/test_filters.py | 104 +------------------------ requirements/requirements-testing.txt | 1 + tests/conftest.py | 2 +- tests/models.py | 1 + tests/serializers.py | 2 +- tests/test_filters.py | 106 ++++++++++++++++++++++++++ tests/test_views.py | 64 +++++++++------- tests/urls.py | 31 ++++++++ tests/views.py | 1 + 9 files changed, 179 insertions(+), 133 deletions(-) create mode 100644 tests/test_filters.py create mode 100644 tests/urls.py diff --git a/example/tests/test_filters.py b/example/tests/test_filters.py index ff7bf263..45b28620 100644 --- a/example/tests/test_filters.py +++ b/example/tests/test_filters.py @@ -1,7 +1,7 @@ from rest_framework.reverse import reverse from rest_framework.test import APITestCase -from example.models import Blog, Entry +from example.models import Entry class DJATestFilters(APITestCase): @@ -13,112 +13,10 @@ class DJATestFilters(APITestCase): def setUp(self): self.entries = Entry.objects.all() - self.blogs = Blog.objects.all() self.url = reverse("nopage-entry-list") self.fs_url = reverse("filterset-entry-list") self.no_fs_url = reverse("nofilterset-entry-list") - def test_sort(self): - """ - test sort - """ - response = self.client.get(self.url, data={"sort": "headline"}) - self.assertEqual( - response.status_code, 200, msg=response.content.decode("utf-8") - ) - dja_response = response.json() - headlines = [c["attributes"]["headline"] for c in dja_response["data"]] - sorted_headlines = sorted(headlines) - self.assertEqual(headlines, sorted_headlines) - - def test_sort_reverse(self): - """ - confirm switching the sort order actually works - """ - response = self.client.get(self.url, data={"sort": "-headline"}) - self.assertEqual( - response.status_code, 200, msg=response.content.decode("utf-8") - ) - dja_response = response.json() - headlines = [c["attributes"]["headline"] for c in dja_response["data"]] - sorted_headlines = sorted(headlines) - self.assertNotEqual(headlines, sorted_headlines) - - def test_sort_double_negative(self): - """ - what if they provide multiple `-`'s? It's OK. - """ - response = self.client.get(self.url, data={"sort": "--headline"}) - self.assertEqual( - response.status_code, 200, msg=response.content.decode("utf-8") - ) - dja_response = response.json() - headlines = [c["attributes"]["headline"] for c in dja_response["data"]] - sorted_headlines = sorted(headlines) - self.assertNotEqual(headlines, sorted_headlines) - - def test_sort_invalid(self): - """ - test sort of invalid field - """ - response = self.client.get( - self.url, data={"sort": "nonesuch,headline,-not_a_field"} - ) - self.assertEqual( - response.status_code, 400, msg=response.content.decode("utf-8") - ) - dja_response = response.json() - self.assertEqual( - dja_response["errors"][0]["detail"], - "invalid sort parameters: nonesuch,-not_a_field", - ) - - def test_sort_camelcase(self): - """ - test sort of camelcase field name - """ - response = self.client.get(self.url, data={"sort": "bodyText"}) - self.assertEqual( - response.status_code, 200, msg=response.content.decode("utf-8") - ) - dja_response = response.json() - blog_ids = [(c["attributes"]["bodyText"] or "") for c in dja_response["data"]] - sorted_blog_ids = sorted(blog_ids) - self.assertEqual(blog_ids, sorted_blog_ids) - - def test_sort_underscore(self): - """ - test sort of underscore field name - Do we allow this notation in a search even if camelcase is in effect? - "Be conservative in what you send, be liberal in what you accept" - -- https://en.wikipedia.org/wiki/Robustness_principle - """ - response = self.client.get(self.url, data={"sort": "body_text"}) - self.assertEqual( - response.status_code, 200, msg=response.content.decode("utf-8") - ) - dja_response = response.json() - blog_ids = [(c["attributes"]["bodyText"] or "") for c in dja_response["data"]] - sorted_blog_ids = sorted(blog_ids) - self.assertEqual(blog_ids, sorted_blog_ids) - - def test_sort_related(self): - """ - test sort via related field using JSON:API path `.` and django orm `__` notation. - ORM relations must be predefined in the View's .ordering_fields attr - """ - for datum in ("blog__id", "blog.id"): - response = self.client.get(self.url, data={"sort": datum}) - self.assertEqual( - response.status_code, 200, msg=response.content.decode("utf-8") - ) - dja_response = response.json() - blog_ids = [ - c["relationships"]["blog"]["data"]["id"] for c in dja_response["data"] - ] - sorted_blog_ids = sorted(blog_ids) - self.assertEqual(blog_ids, sorted_blog_ids) - def test_filter_exact(self): """ filter for an exact match diff --git a/requirements/requirements-testing.txt b/requirements/requirements-testing.txt index 3faacad7..612b88a5 100644 --- a/requirements/requirements-testing.txt +++ b/requirements/requirements-testing.txt @@ -1,5 +1,6 @@ factory-boy==3.3.3 Faker==40.37.0 +inline-snapshot==0.36.0 pytest==9.1.1 pytest-cov==7.1.0 pytest-django==4.14.0 diff --git a/tests/conftest.py b/tests/conftest.py index 77b3676b..4621e694 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -34,7 +34,7 @@ def use_rest_framework_json_api_defaults(settings): @pytest.fixture def model(db): - return BasicModel.objects.create(text="Model") + return BasicModel.objects.create(text="Model", long_text="LongText") @pytest.fixture diff --git a/tests/models.py b/tests/models.py index 63cb6434..9735f288 100644 --- a/tests/models.py +++ b/tests/models.py @@ -13,6 +13,7 @@ class Meta: class BasicModel(DJAModel): text = models.CharField(max_length=100) + long_text = models.CharField(max_length=100) class Meta: ordering = ("id",) diff --git a/tests/serializers.py b/tests/serializers.py index ef8a51cf..002ea25a 100644 --- a/tests/serializers.py +++ b/tests/serializers.py @@ -14,7 +14,7 @@ class BasicModelSerializer(serializers.ModelSerializer): class Meta: - fields = ("text",) + fields = ("text", "long_text") model = BasicModel diff --git a/tests/test_filters.py b/tests/test_filters.py new file mode 100644 index 00000000..205cdd8a --- /dev/null +++ b/tests/test_filters.py @@ -0,0 +1,106 @@ +import pytest +from django.urls import reverse +from rest_framework import status + +from tests.models import BasicModel, ForeignKeySource, ForeignKeyTarget + + +@pytest.mark.urls("tests.urls") +class TestOrderingFilter: + @pytest.mark.parametrize( + "sort_param,expected", + [ + # ascending sort + ("text", ["1", "2", "3"]), + # descending sort + ("-text", ["3", "2", "1"]), + # descending sort with two dashes should work + ("--text", ["3", "2", "1"]), + ], + ) + def test_sort(self, db, client, sort_param, expected): + BasicModel.objects.create(text="1") + BasicModel.objects.create(text="3") + BasicModel.objects.create(text="2") + + url = reverse("basicmodel-list") + response = client.get(url, {"sort": sort_param, "page[size]": 3}) + result = response.json() + + assert response.status_code == status.HTTP_200_OK + assert [d["attributes"]["text"] for d in result["data"]] == expected + + @pytest.mark.parametrize( + "sort_param,error", + [ + ( + "nonesuch,text,-not-a-field", + ["invalid sort parameters: nonesuch,-not-a-field"], + ) + ], + ) + def test_sort_invalid(self, db, client, sort_param, error): + url = reverse("basicmodel-list") + response = client.get(url, {"sort": sort_param, "page[size]": 3}) + result = response.json() + + assert response.status_code == status.HTTP_400_BAD_REQUEST + assert [e["detail"] for e in result["errors"]] == error + + @pytest.mark.parametrize( + "format_type,sort_param,expected", + [ + (False, "long_text", ["1", "2", "3"]), + (False, "-long_text", ["3", "2", "1"]), + ("underscore", "long_text", ["1", "2", "3"]), + ("underscore", "-long_text", ["3", "2", "1"]), + ("dasherize", "long-text", ["1", "2", "3"]), + ("dasherize", "-long-text", ["3", "2", "1"]), + ("camelize", "longText", ["1", "2", "3"]), + ("camelize", "-longText", ["3", "2", "1"]), + ("capitalize", "LongText", ["1", "2", "3"]), + ("capitalize", "-LongText", ["3", "2", "1"]), + ], + ) + def test_sort_format(self, db, client, settings, format_type, sort_param, expected): + settings.JSON_API_FORMAT_FIELD_NAMES = format_type + + BasicModel.objects.create(long_text="1") + BasicModel.objects.create(long_text="3") + BasicModel.objects.create(long_text="2") + + url = reverse("basicmodel-list") + response = client.get(url, {"sort": sort_param, "page[size]": 3}) + result = response.json() + + assert response.status_code == status.HTTP_200_OK + values = [d["attributes"][sort_param.lstrip("-")] for d in result["data"]] + assert values == expected + + @pytest.mark.parametrize( + "sort_param,expected", + [ + ("target.name,name", ["1", "2", "3"]), + ("-target.name,-name", ["3", "2", "1"]), + ("target__name,name", ["1", "2", "3"]), + ("-target__name,-name", ["3", "2", "1"]), + ], + ) + def test_sort_related(self, db, client, sort_param, expected): + ForeignKeySource.objects.create( + name="3", target=ForeignKeyTarget.objects.create(name="3") + ) + ForeignKeySource.objects.create( + name="1", target=ForeignKeyTarget.objects.create(name="1") + ) + ForeignKeySource.objects.create( + name="2", target=ForeignKeyTarget.objects.create(name="1") + ) + + url = reverse("foreignkeysource-list") + response = client.get(url, {"sort": sort_param, "page[size]": 3}) + result = response.json() + + assert response.status_code == status.HTTP_200_OK + values = [d["attributes"]["name"] for d in result["data"]] + assert values == expected diff --git a/tests/test_views.py b/tests/test_views.py index 349d37da..3f0de0ba 100644 --- a/tests/test_views.py +++ b/tests/test_views.py @@ -1,5 +1,6 @@ import pytest from django.urls import path, reverse +from inline_snapshot import snapshot from rest_framework import status from rest_framework.decorators import action from rest_framework.response import Response @@ -71,22 +72,24 @@ def test_list(self, client, model): url = reverse("basic-model-list") response = client.get(url) assert response.status_code == status.HTTP_200_OK - assert response.json() == { - "data": [ - { - "type": "BasicModel", - "id": str(model.pk), - "attributes": {"text": "Model"}, - } - ], - "links": { - "first": "http://testserver/basic_models/?page%5Bnumber%5D=1", - "last": "http://testserver/basic_models/?page%5Bnumber%5D=1", - "next": None, - "prev": None, - }, - "meta": {"pagination": {"count": 1, "page": 1, "pages": 1}}, - } + assert response.json() == snapshot( + { + "data": [ + { + "type": "BasicModel", + "id": str(model.pk), + "attributes": {"text": "Model", "long_text": "LongText"}, + } + ], + "links": { + "first": "http://testserver/basic_models/?page%5Bnumber%5D=1", + "last": "http://testserver/basic_models/?page%5Bnumber%5D=1", + "next": None, + "prev": None, + }, + "meta": {"pagination": {"count": 1, "page": 1, "pages": 1}}, + } + ) @pytest.mark.urls(__name__) def test_list_with_include_foreign_key(self, client, foreign_key_source): @@ -237,13 +240,15 @@ def test_retrieve(self, client, model): url = reverse("basic-model-detail", kwargs={"pk": model.pk}) response = client.get(url) assert response.status_code == status.HTTP_200_OK - assert response.json() == { - "data": { - "type": "BasicModel", - "id": str(model.pk), - "attributes": {"text": "Model"}, + assert response.json() == snapshot( + { + "data": { + "type": "BasicModel", + "id": str(model.pk), + "attributes": {"text": "Model", "long_text": "LongText"}, + } } - } + ) @pytest.mark.urls(__name__) def test_retrieve_with_include_foreign_key(self, client, foreign_key_source): @@ -287,13 +292,15 @@ def test_patch(self, client, model): url = reverse("basic-model-detail", kwargs={"pk": model.pk}) response = client.patch(url, data=data) assert response.status_code == status.HTTP_200_OK - assert response.json() == { - "data": { - "type": "BasicModel", - "id": str(model.pk), - "attributes": {"text": "changed"}, + assert response.json() == snapshot( + { + "data": { + "type": "BasicModel", + "id": str(model.pk), + "attributes": {"text": "changed", "long_text": "LongText"}, + } } - } + ) @pytest.mark.urls(__name__) def test_delete(self, client, model): @@ -541,6 +548,7 @@ def patch(self, request, *args, **kwargs): # TODO remove basename and use default (lowercase of model) +# TODO merge with router of tests/urls.py # this makes using HyperlinkedIdentityField easier and reduces # configuration in general router = SimpleRouter() diff --git a/tests/urls.py b/tests/urls.py new file mode 100644 index 00000000..142d2ab6 --- /dev/null +++ b/tests/urls.py @@ -0,0 +1,31 @@ +from rest_framework.routers import SimpleRouter + +from tests.views import ( + BasicModelViewSet, + ForeignKeySourcetHyperlinkedViewSet, + ForeignKeySourceViewSet, + ForeignKeyTargetViewSet, + ManyToManySourceViewSet, + NestedRelatedSourceViewSet, + URLModelViewSet, +) + +router = SimpleRouter() +router.register(r"basic_models", BasicModelViewSet) +router.register(r"url_models", URLModelViewSet) +router.register(r"foreign_key_sources", ForeignKeySourceViewSet) +router.register(r"foreign_key_targets", ForeignKeyTargetViewSet) +router.register( + r"foreign_key_sources_hyperlinked", + ForeignKeySourcetHyperlinkedViewSet, + "foreignkeysourcehyperlinked", +) +router.register( + r"many_to_many_sources", ManyToManySourceViewSet, basename="many-to-many-source" +) +router.register( + r"nested_related_sources", + NestedRelatedSourceViewSet, + basename="nested-related-source", +) +urlpatterns = router.urls diff --git a/tests/views.py b/tests/views.py index 7958c6b9..4f78cd13 100644 --- a/tests/views.py +++ b/tests/views.py @@ -34,6 +34,7 @@ class ForeignKeySourceViewSet(ModelViewSet): serializer_class = ForeignKeySourceSerializer queryset = ForeignKeySource.objects.all() ordering = ["name"] + ordering_fields = ["target__name", "name"] class ForeignKeySourcetHyperlinkedViewSet(ModelViewSet): From 6304a6c16e5eaf050d86c1aca4959d36d11481db Mon Sep 17 00:00:00 2001 From: Oliver Sauder Date: Thu, 8 Oct 2026 10:34:49 +0400 Subject: [PATCH 2/2] Clarified how to set allowed ordering fields in docs --- docs/usage.md | 29 +++++++++++++++++++++-------- 1 file changed, 21 insertions(+), 8 deletions(-) diff --git a/docs/usage.md b/docs/usage.md index bbcae0a5..6f6ba6af 100644 --- a/docs/usage.md +++ b/docs/usage.md @@ -133,12 +133,28 @@ simply don't use this filter backend. #### OrderingFilter -`OrderingFilter` implements the [JSON:API `sort`](https://jsonapi.org/format/#fetching-sorting) and uses -DRF's [ordering filter](https://www.django-rest-framework.org/api-guide/filtering/#orderingfilter). +`OrderingFilter` implements [JSON:API `sorting`](https://jsonapi.org/format/#fetching-sorting) and uses +Django REST framework's [ordering filter](https://www.django-rest-framework.org/api-guide/filtering/#orderingfilter) as a basis. + +Define `ordering_fields` on your view to define what fields are supported for sorting. If not defined per default only direct readable fields +will be allowed. + +Example: + +```python +class BookViewSet(views.ModelViewSet): + queryset = Book.objects.all() + ordering_fields = ['name', 'author__name'] +``` + +For sorting of relationship fields the dot notation can be used. For instance in above example to +sort by name of author, `sort` query parameter needs to be `?sort=author.name`. + +Unlike in DRF, it needs to be noted that in JSON:API when the `sort` query parameter +contains invalid field names, a `400 Bad Request` will be returned. + +A error response will look like the following. -Per the JSON:API specification, "If the server does not support sorting as specified in the query parameter `sort`, -it **MUST** return `400 Bad Request`." For example, for `?sort=abc,foo,def` where `foo` is a valid -field name and the other two are not valid: ```json { "errors": [ @@ -153,9 +169,6 @@ field name and the other two are not valid: } ``` -If you want to silently ignore bad sort fields, just use `rest_framework.filters.OrderingFilter` and set -`ordering_param` to `sort`. - #### DjangoFilterBackend `DjangoFilterBackend` implements a Django ORM-style [JSON:API `filter`](https://jsonapi.org/format/#fetching-filtering)