Merge pull request #90 from saritasa-nest/feature/minor-fixes

Some fixes for issues discovered in new version

HISTORY.rst

@@ -2,6 +2,12 @@
 History
 =======
 
+UNRELEASED
+------------------
+
+* Fix issues with query params parsing
+* Make `get_queryset` consistent for start actions
+
 1.3.0 (2025-01-09)
 ------------------
 

import_export_extensions/api/mixins.py

@@ -3,6 +3,9 @@ class LimitQuerySetToCurrentUserMixin:
 
     def get_queryset(self):
         """Return user's jobs."""
+        if self.action == "start":
+            # To make it consistent and for better support of drf-spectacular
+            return super().get_queryset()  # pragma: no cover
         return (
             super()
             .get_queryset()

import_export_extensions/api/views/export_job.py

@@ -125,6 +125,9 @@ class ExportJobViewSet(
 
     def get_queryset(self):
         """Filter export jobs by resource used in viewset."""
+        if self.action == "start":
+            # To make it consistent and for better support of drf-spectacular
+            return super().get_queryset() # pragma: no cover
         return super().get_queryset().filter(
             resource_path=self.resource_class.class_path,
         )
@@ -157,12 +160,13 @@ def get_export_create_serializer_class(self):
 
     def start(self, request: Request):
         """Validate request data and start ExportJob."""
-        query_params = dict(request.query_params)
-        ordering = query_params.pop("ordering", self.ordering)
+        ordering = request.query_params.get("ordering", "")
+        if ordering:
+            ordering = ordering.split(",")
         serializer = self.get_serializer(
             data=request.data,
             ordering=ordering,
-            filter_kwargs=query_params,
+            filter_kwargs=request.query_params,
         )
         serializer.is_valid(raise_exception=True)
         export_job = serializer.save()

pyproject.toml

@@ -246,6 +246,8 @@ ignore = [
   "C408",
   # https://docs.astral.sh/ruff/rules/mutable-class-default
   "RUF012",
+  # https://docs.astral.sh/ruff/rules/assignment-in-assert/
+  "RUF018",
   # https://docs.astral.sh/ruff/rules/raise-vanilla-args
   "TRY003",
   # https://docs.astral.sh/ruff/rules/try-consider-else

test_project/fake_app/filters.py

@@ -8,7 +8,13 @@ class ArtistFilterSet(filters.FilterSet):
 
     class Meta:
         model = Artist
-        fields = [
-            "id",
-            "name",
-        ]
+        fields = {
+            "id": (
+                "exact",
+                "in",
+            ),
+            "name": (
+                "exact",
+                "in",
+            ),
+        }

test_project/tests/integration_tests/test_api/test_export.py

@@ -1,3 +1,5 @@
+import collections.abc
+
 from django.contrib.auth.models import User
 from django.urls import reverse
 
@@ -9,33 +11,115 @@
 
 
 @pytest.mark.django_db(transaction=True)
+def test_export_api_creates_export_job(
+    admin_api_client: test.APIClient,
+):
+    """Ensure export start API creates new export job."""
+    response = admin_api_client.post(
+        path=reverse("export-artist-start"),
+        data={
+            "file_format": "csv",
+        },
+    )
+    assert response.status_code == status.HTTP_201_CREATED, response.data
+    assert response.data["export_status"] == ExportJob.ExportStatus.CREATED
+    assert ExportJob.objects.filter(id=response.data["id"]).exists()
+
+
 @pytest.mark.parametrize(
-    argnames=["export_url"],
+    argnames=[
+        "filter_query",
+        "filter_name",
+        "filter_value",
+    ],
     argvalues=[
         pytest.param(
-            reverse("export-artist-start"),
-            id="Url without filter_kwargs",
+            "name=Artist",
+            "name",
+            "Artist",
+            id="Simple str filter",
+        ),
+        pytest.param(
+            "id=1",
+            "id",
+            "1",
+            id="Simple int filter",
         ),
         pytest.param(
-            f"{reverse('export-artist-start')}?name=Artist",
-            id="Url with valid filter_kwargs",
+            "name__in=Some,Artist",
+            "name__in",
+            "Some,Artist",
+            id="Simple `in` filter",
         ),
     ],
 )
-def test_export_api_creates_export_job(
+def test_export_api_filtering(
     admin_api_client: test.APIClient,
-    export_url: str,
+    filter_query: str,
+    filter_name: str,
+    filter_value: str,
 ):
-    """Ensure export start API creates new export job."""
+    """Ensure export start API passes filter kwargs correctly."""
     response = admin_api_client.post(
-        path=export_url,
+        path=f"{reverse('export-artist-start')}?{filter_query}",
         data={
             "file_format": "csv",
         },
     )
     assert response.status_code == status.HTTP_201_CREATED, response.data
     assert response.data["export_status"] == ExportJob.ExportStatus.CREATED
-    assert ExportJob.objects.filter(id=response.data["id"]).exists()
+    assert (
+        export_job := ExportJob.objects.filter(id=response.data["id"]).first()
+    )
+    assert (
+        export_job.resource_kwargs["filter_kwargs"][filter_name]
+        == filter_value
+    ), export_job.resource_kwargs["filter_kwargs"]
+
+
+@pytest.mark.parametrize(
+    argnames=[
+        "ordering_query",
+        "ordering_value",
+    ],
+    argvalues=[
+        pytest.param(
+            "ordering=name",
+            [
+                "name",
+            ],
+            id="One field",
+        ),
+        pytest.param(
+            "ordering=name%2C-id",
+            [
+                "name",
+                "-id",
+            ],
+            id="Many fields",
+        ),
+    ],
+)
+def test_export_api_ordering(
+    admin_api_client: test.APIClient,
+    ordering_query: str,
+    ordering_value: collections.abc.Sequence[str],
+):
+    """Ensure export start API passes ordering correctly."""
+    response = admin_api_client.post(
+        path=f"{reverse('export-artist-start')}?{ordering_query}",
+        data={
+            "file_format": "csv",
+        },
+    )
+    assert response.status_code == status.HTTP_201_CREATED, response.data
+    assert response.data["export_status"] == ExportJob.ExportStatus.CREATED
+    assert (
+        export_job := ExportJob.objects.filter(id=response.data["id"]).first()
+    )
+    assert (
+        export_job.resource_kwargs["ordering"] == ordering_value
+    ), export_job.resource_kwargs["ordering"]
 
 
 @pytest.mark.django_db(transaction=True)