PB-903 Drop the explicit schema invocations

According to the docs, it isn't necessary to invoke the Schemas by ourselves. The endpoint functions return the data as lists, querysets or dicts and the Framework handles the conversion to the schema. I don't see an immediate drawback to our version, but I'm also not so comfortable to deviate from the standard way proposed by the docs. From the [docs](https://django-ninja.dev/guides/response/), NinjaAPI seems to do quite some tasks with the returned data itself and in my opinion we shouldn't be doing this ourselves unless we have a good reason to. Also, there might be performance optimizations being done by NinjaAPI (consider especially when returning entire querysets)
geoadmin · Oct 7, 2024 · 0ac20f5 · 0ac20f5
1 parent 2078261
commit 0ac20f5
Show file tree

Hide file tree

Showing 3 changed files with 85 additions and 105 deletions.
diff --git a/app/distributions/api.py b/app/distributions/api.py
@@ -1,5 +1,4 @@
 from ninja import Router
-from schemas import TranslationsSchema
 from utils.language import LanguageCode
 from utils.language import get_language
 from utils.language import get_translation
@@ -17,30 +16,30 @@
 router = Router()
 
 
-def attribution_to_response(model: Attribution, lang: LanguageCode) -> AttributionSchema:
+def attribution_to_response(model: Attribution, lang: LanguageCode) -> dict:
     """
-    Transforms the given model using the given language into a response object.
+    Transforms the given model using the given language into the response structure
     """
-    response = AttributionSchema(
-        id=str(model.id),
-        name=get_translation(model, "name", lang),
-        name_translations=TranslationsSchema(
-            de=model.name_de,
-            fr=model.name_fr,
-            en=model.name_en,
-            it=model.name_it,
-            rm=model.name_rm,
-        ),
-        description=get_translation(model, "description", lang),
-        description_translations=TranslationsSchema(
-            de=model.description_de,
-            fr=model.description_fr,
-            en=model.description_en,
-            it=model.description_it,
-            rm=model.description_rm,
-        ),
-        provider_id=str(model.provider.id),
-    )
+    response = {
+        "id": str(model.id),
+        "name": get_translation(model, "name", lang),
+        "name_translations": {
+            "de": model.name_de,
+            "fr": model.name_fr,
+            "en": model.name_en,
+            "it": model.name_it,
+            "rm": model.name_rm,
+        },
+        "description": get_translation(model, "description", lang),
+        "description_translations": {
+            "de": model.description_de,
+            "fr": model.description_fr,
+            "en": model.description_en,
+            "it": model.description_it,
+            "rm": model.description_rm,
+        },
+        "provider_id": str(model.provider.id),
+    }
     return response
 
 
@@ -109,21 +108,7 @@ def attributions(request: HttpRequest, lang: LanguageCode | None = None):
     lang_to_use = get_language(lang, request.headers)
 
     responses = [attribution_to_response(model, lang_to_use) for model in models]
-    return AttributionListSchema(items=responses)
-
-
-def dataset_to_response(model: Dataset) -> DatasetSchema:
-    """
-    Transforms the given model into a response object.
-    """
-    return DatasetSchema(
-        id=str(model.id),
-        slug=model.slug,
-        created=model.created,
-        updated=model.updated,
-        provider_id=str(model.provider.id),
-        attribution_id=str(model.attribution.id),
-    )
+    return {"items": responses}
 
 
 @router.get("datasets/{dataset_id}", response={200: DatasetSchema}, exclude_none=True)
@@ -132,8 +117,7 @@ def dataset(request: HttpRequest, dataset_id: int):
     Get the dataset with the given ID.
     """
     model = get_object_or_404(Dataset, id=dataset_id)
-    response = dataset_to_response(model)
-    return response
+    return model
 
 
 @router.get("datasets", response={200: DatasetListSchema}, exclude_none=True)
@@ -145,5 +129,4 @@ def datasets(request: HttpRequest):
     corresponding endpoint for a specific attribution.
     """
     models = Dataset.objects.order_by("id").all()
-    responses = [dataset_to_response(model) for model in models]
-    return DatasetListSchema(items=responses)
+    return {"items": models}
diff --git a/app/provider/api.py b/app/provider/api.py
@@ -9,34 +9,33 @@
 from .models import Provider
 from .schemas import ProviderListSchema
 from .schemas import ProviderSchema
-from .schemas import TranslationsSchema
 
 router = Router()
 
 
-def provider_to_response(model: Provider, lang: LanguageCode) -> ProviderSchema:
+def provider_to_response(model: Provider, lang: LanguageCode) -> dict:
     """
-    Transforms the given model using the given language into a response object.
+    Transforms the given model using the given language into the response structure.
     """
-    response = ProviderSchema(
-        id=str(model.id),
-        name=get_translation(model, "name", lang),
-        name_translations=TranslationsSchema(
-            de=model.name_de,
-            fr=model.name_fr,
-            en=model.name_en,
-            it=model.name_it,
-            rm=model.name_rm,
-        ),
-        acronym=get_translation(model, "acronym", lang),
-        acronym_translations=TranslationsSchema(
-            de=model.acronym_de,
-            fr=model.acronym_fr,
-            en=model.acronym_en,
-            it=model.acronym_it,
-            rm=model.acronym_rm,
-        )
-    )
+    response = {
+        "id": str(model.id),
+        "name": get_translation(model, "name", lang),
+        "name_translations": {
+            "de": model.name_de,
+            "fr": model.name_fr,
+            "en": model.name_en,
+            "it": model.name_it,
+            "rm": model.name_rm,
+        },
+        "acronym": get_translation(model, "acronym", lang),
+        "acronym_translations": {
+            "de": model.acronym_de,
+            "fr": model.acronym_fr,
+            "en": model.acronym_en,
+            "it": model.acronym_it,
+            "rm": model.acronym_rm,
+        }
+    }
     return response
 
 
@@ -104,4 +103,4 @@ def providers(request: HttpRequest, lang: LanguageCode | None = None):
     lang_to_use = get_language(lang, request.headers)
 
     schemas = [provider_to_response(model, lang_to_use) for model in models]
-    return ProviderListSchema(items=schemas)
+    return {"items": schemas}
diff --git a/app/provider/tests/test_api.py b/app/provider/tests/test_api.py
@@ -2,8 +2,6 @@
 from provider.api import provider_to_response
 from provider.api import router
 from provider.models import Provider
-from provider.schemas import ProviderSchema
-from schemas import TranslationsSchema
 
 from django.test import TestCase
 
@@ -31,25 +29,25 @@ def test_provider_to_response_returns_response_with_language_as_defined(self):
 
         actual = provider_to_response(model, lang="de")
 
-        expected = ProviderSchema(
-            id=str(model.id),
-            name="Bundesamt für Umwelt",
-            name_translations=TranslationsSchema(
-                de="Bundesamt für Umwelt",
-                fr="Office fédéral de l'environnement",
-                en="Federal Office for the Environment",
-                it="Ufficio federale dell'ambiente",
-                rm="Uffizi federal per l'ambient",
-            ),
-            acronym="BAFU",
-            acronym_translations=TranslationsSchema(
-                de="BAFU",
-                fr="OFEV",
-                en="FOEN",
-                it="UFAM",
-                rm="UFAM",
-            )
-        )
+        expected = {
+            "id": str(model.id),
+            "name": "Bundesamt für Umwelt",
+            "name_translations": {
+                "de": "Bundesamt für Umwelt",
+                "fr": "Office fédéral de l'environnement",
+                "en": "Federal Office for the Environment",
+                "it": "Ufficio federale dell'ambiente",
+                "rm": "Uffizi federal per l'ambient",
+            },
+            "acronym": "BAFU",
+            "acronym_translations": {
+                "de": "BAFU",
+                "fr": "OFEV",
+                "en": "FOEN",
+                "it": "UFAM",
+                "rm": "UFAM",
+            }
+        }
 
         assert actual == expected
 
@@ -63,25 +61,25 @@ def test_provider_to_response_returns_response_with_default_language_if_undefine
 
         actual = provider_to_response(provider, lang="it")
 
-        expected = ProviderSchema(
-            id=str(provider.id),
-            name="Federal Office for the Environment",
-            name_translations=TranslationsSchema(
-                de="Bundesamt für Umwelt",
-                fr="Office fédéral de l'environnement",
-                en="Federal Office for the Environment",
-                it=None,
-                rm=None,
-            ),
-            acronym="FOEN",
-            acronym_translations=TranslationsSchema(
-                de="BAFU",
-                fr="OFEV",
-                en="FOEN",
-                it=None,
-                rm=None,
-            )
-        )
+        expected = {
+            "id": str(provider.id),
+            "name": "Federal Office for the Environment",
+            "name_translations": {
+                "de": "Bundesamt für Umwelt",
+                "fr": "Office fédéral de l'environnement",
+                "en": "Federal Office for the Environment",
+                "it": None,
+                "rm": None,
+            },
+            "acronym": "FOEN",
+            "acronym_translations": {
+                "de": "BAFU",
+                "fr": "OFEV",
+                "en": "FOEN",
+                "it": None,
+                "rm": None,
+            }
+        }
 
         assert actual == expected