From 796080d080c74e742a3b02a1cb98f03133a13510 Mon Sep 17 00:00:00 2001 From: Fabian Braun Date: Fri, 24 Oct 2025 23:56:08 +0200 Subject: [PATCH 1/6] feat: Remove distinction between PlaceholderRelationSerializer and PlaceholderSerializer --- djangocms_rest/serializers/pages.py | 18 ++---- djangocms_rest/serializers/placeholders.py | 72 +++++++++------------- 2 files changed, 33 insertions(+), 57 deletions(-) diff --git a/djangocms_rest/serializers/pages.py b/djangocms_rest/serializers/pages.py index 2b12e48..4f71b6f 100644 --- a/djangocms_rest/serializers/pages.py +++ b/djangocms_rest/serializers/pages.py @@ -4,7 +4,7 @@ from rest_framework import serializers -from djangocms_rest.serializers.placeholders import PlaceholderRelationSerializer +from djangocms_rest.serializers.placeholders import PlaceholderSerializer from djangocms_rest.utils import get_absolute_frontend_url @@ -125,7 +125,7 @@ def to_representation(self, page_content: PageContent) -> dict: class PageContentSerializer(BasePageSerializer, BasePageContentMixin): - placeholders = PlaceholderRelationSerializer(many=True, required=False) + placeholders = PlaceholderSerializer(many=True, required=False) def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) @@ -136,19 +136,9 @@ def to_representation(self, page_content: PageContent) -> dict: placeholders = [ placeholder for placeholder in page_content.placeholders.all() if placeholder.slot in declared_slots ] - - placeholders_data = [ - { - "content_type_id": placeholder.content_type_id, - "object_id": placeholder.object_id, - "slot": placeholder.slot, - } - for placeholder in placeholders - ] - data = self.get_base_representation(page_content) - data["placeholders"] = PlaceholderRelationSerializer( - placeholders_data, + data["placeholders"] = PlaceholderSerializer( + placeholders, language=page_content.language, many=True, context={"request": self.request}, diff --git a/djangocms_rest/serializers/placeholders.py b/djangocms_rest/serializers/placeholders.py index a031973..36a9ae9 100644 --- a/djangocms_rest/serializers/placeholders.py +++ b/djangocms_rest/serializers/placeholders.py @@ -11,68 +11,54 @@ class PlaceholderSerializer(serializers.Serializer): slot = serializers.CharField() label = serializers.CharField() language = serializers.CharField() - content = serializers.ListSerializer( - child=serializers.JSONField(), allow_empty=True, required=False - ) + content = serializers.ListSerializer(child=serializers.JSONField(), allow_empty=True, required=False) + details = serializers.URLField() html = serializers.CharField(default="", required=False) def __init__(self, *args, **kwargs): - request = kwargs.pop("request", None) - placeholder = kwargs.pop("instance", None) - language = kwargs.pop("language", None) - render_plugins = kwargs.pop("render_plugins", True) + self.request = kwargs.pop("request", None) + self.language = kwargs.pop("language", None) + self.render_plugins = kwargs.pop("render_plugins", True) super().__init__(*args, **kwargs) - if request is None: - request = self.context.get("request") + if self.request is None: + self.request = self.context.get("request") - if placeholder and request and language: - if render_plugins: + def to_representation(self, instance): + instance.label = instance.get_label() + instance.language = self.language + instance.details = self.get_details(instance) + if instance and self.request and self.language: + if self.render_plugins: from djangocms_rest.plugin_rendering import RESTRenderer - renderer = RESTRenderer(request) - placeholder.content = renderer.serialize_placeholder( - placeholder, - context=Context({"request": request}), - language=language, + renderer = RESTRenderer(self.request) + instance.content = renderer.serialize_placeholder( + instance, + context=Context({"request": self.request}), + language=self.language, use_cache=True, ) - if request.GET.get("html", False): - html = render_html(request, placeholder, language) + if self.request.GET.get("html", False): + html = render_html(self.request, instance, self.language) for key, value in html.items(): - if not hasattr(placeholder, key): - setattr(placeholder, key, value) - self.fields[key] = serializers.CharField() - placeholder.label = placeholder.get_label() - placeholder.language = language - self.instance = placeholder - - -class PlaceholderRelationSerializer(serializers.Serializer): - content_type_id = serializers.IntegerField() - object_id = serializers.IntegerField() - slot = serializers.CharField() - details = serializers.URLField() + if not hasattr(instance, key): + setattr(instance, key, value) - def __init__(self, *args, **kwargs): - language = kwargs.pop("language", None) - super().__init__(*args, **kwargs) - self.request = self.context.get("request") - self.language = language - - def to_representation(self, instance): - instance["details"] = self.get_details(instance) return super().to_representation(instance) def get_details(self, instance): - return get_absolute_frontend_url( + url = get_absolute_frontend_url( self.request, reverse( "placeholder-detail", args=[ self.language, - instance.get("content_type_id"), - instance.get("object_id"), - instance.get("slot"), + instance.content_type_id, + instance.object_id, + instance.slot, ], ), ) + if self.request.GET: + url += f"?{self.request.GET.urlencode()}" + return url From 1ebc1fec37929334cdd0c22fdd3d49546057b673 Mon Sep 17 00:00:00 2001 From: Fabian Braun Date: Sat, 25 Oct 2025 16:00:59 +0200 Subject: [PATCH 2/6] fix: Preview placeholders not shown --- djangocms_rest/serializers/pages.py | 3 ++- djangocms_rest/views.py | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/djangocms_rest/serializers/pages.py b/djangocms_rest/serializers/pages.py index 4f71b6f..82de5e3 100644 --- a/djangocms_rest/serializers/pages.py +++ b/djangocms_rest/serializers/pages.py @@ -1,6 +1,7 @@ from django.db import models from cms.models import PageContent +from cms.utils.placeholder import get_declared_placeholders_for_obj from rest_framework import serializers @@ -132,7 +133,7 @@ def __init__(self, *args, **kwargs): self.request = self.context.get("request") def to_representation(self, page_content: PageContent) -> dict: - declared_slots = [placeholder.slot for placeholder in page_content.page.get_declared_placeholders()] + declared_slots = [placeholder.slot for placeholder in get_declared_placeholders_for_obj(page_content)] placeholders = [ placeholder for placeholder in page_content.placeholders.all() if placeholder.slot in declared_slots ] diff --git a/djangocms_rest/views.py b/djangocms_rest/views.py index 0a9a550..3e6c291 100644 --- a/djangocms_rest/views.py +++ b/djangocms_rest/views.py @@ -177,7 +177,7 @@ def get(self, request: Request, language: str, path: str = "") -> Response: try: page_content = getattr(page, self.content_getter)(language, fallback=True) - if page_content is None: + if not page_content: raise PageContent.DoesNotExist() serializer = self.serializer_class(page_content, read_only=True, context={"request": request}) return Response(serializer.data) From 148d0777a3bc9f92a08e064fc015d45f01cc7df0 Mon Sep 17 00:00:00 2001 From: Fabian Braun Date: Sun, 26 Oct 2025 16:39:21 +0100 Subject: [PATCH 3/6] fix: Typde definitions --- tests/types.py | 19 +++++++------------ 1 file changed, 7 insertions(+), 12 deletions(-) diff --git a/tests/types.py b/tests/types.py index 2e12779..f0f04f7 100644 --- a/tests/types.py +++ b/tests/types.py @@ -29,15 +29,18 @@ PAGE_TREE_META_FIELD_TYPES = {**PAGE_META_FIELD_TYPES, "children": list} -PLACEHOLDER_RELATION_FIELD_TYPES = { - "content_type_id": int, - "object_id": int, + +PLACEHOLDER_FIELD_TYPES = { "slot": str, + "label": str, + "language": str, + "content": list, + "html": str, } PAGE_CONTENT_FIELD_TYPES = { **PAGE_META_FIELD_TYPES, - "placeholders": [PLACEHOLDER_RELATION_FIELD_TYPES], + "placeholders": [PLACEHOLDER_FIELD_TYPES], } LANGUAGE_FIELD_TYPES = { @@ -49,14 +52,6 @@ "hide_untranslated": bool, } -PLACEHOLDER_FIELD_TYPES = { - "slot": str, - "label": str, - "language": str, - "content": list, - "html": str, -} - PLUGIN_FIELD_TYPES = { "plugin_type": str, "title": str, From b56c310d31d596c45783ad32e5ef45fdcdd4c7c4 Mon Sep 17 00:00:00 2001 From: Fabian Braun Date: Sun, 26 Oct 2025 17:08:57 +0100 Subject: [PATCH 4/6] fix: Placeholder serializer copied admin GET params --- djangocms_rest/serializers/placeholders.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/djangocms_rest/serializers/placeholders.py b/djangocms_rest/serializers/placeholders.py index 36a9ae9..9e54e8f 100644 --- a/djangocms_rest/serializers/placeholders.py +++ b/djangocms_rest/serializers/placeholders.py @@ -1,3 +1,5 @@ +from urllib.parse import urlencode + from django.template import Context from django.urls import reverse @@ -59,6 +61,7 @@ def get_details(self, instance): ], ), ) - if self.request.GET: - url += f"?{self.request.GET.urlencode()}" + get_params = {key: self.request.GET[key] for key in ("html", "preview") if key in self.request.GET} + if get_params: + url += "?" + urlencode(get_params) return url From b1bd800f1f5584d6277485df38ee3636f61ab881 Mon Sep 17 00:00:00 2001 From: Fabian Braun Date: Thu, 6 Nov 2025 09:10:15 +0100 Subject: [PATCH 5/6] fix: Add schema for placeholder content and tests that the open api schema is correct --- djangocms_rest/serializers/placeholders.py | 27 +- tests/settings.py | 1 + tests/test_openapi_schema.py | 286 +++++++++++++++++++++ tests/urls.py | 14 +- 4 files changed, 324 insertions(+), 4 deletions(-) create mode 100644 tests/test_openapi_schema.py diff --git a/djangocms_rest/serializers/placeholders.py b/djangocms_rest/serializers/placeholders.py index 9e54e8f..6de99dc 100644 --- a/djangocms_rest/serializers/placeholders.py +++ b/djangocms_rest/serializers/placeholders.py @@ -8,12 +8,37 @@ from djangocms_rest.serializers.utils.render import render_html from djangocms_rest.utils import get_absolute_frontend_url +try: + from drf_spectacular.utils import extend_schema_field + + HAS_SPECTACULAR = True +except ImportError: + HAS_SPECTACULAR = False + + def extend_schema_field(field_schema): + def decorator(field): + return field + + return decorator + class PlaceholderSerializer(serializers.Serializer): slot = serializers.CharField() label = serializers.CharField() language = serializers.CharField() - content = serializers.ListSerializer(child=serializers.JSONField(), allow_empty=True, required=False) + + # Annotate the content field for OpenAPI schema generation + @extend_schema_field( + { + "type": "array", + "items": {"type": "object"}, + "description": "List of serialized plugin data for this placeholder", + } + ) + class ContentField(serializers.ListSerializer): + child = serializers.JSONField() + + content = ContentField(allow_empty=True, required=False) details = serializers.URLField() html = serializers.CharField(default="", required=False) diff --git a/tests/settings.py b/tests/settings.py index 8458435..2b17ce1 100644 --- a/tests/settings.py +++ b/tests/settings.py @@ -181,6 +181,7 @@ def __getitem__(self, item): REST_FRAMEWORK = { "DEFAULT_PAGINATION_CLASS": "rest_framework.pagination.PageNumberPagination", "PAGE_SIZE": 10, + "DEFAULT_SCHEMA_CLASS": "drf_spectacular.openapi.AutoSchema", } USE_TZ = True diff --git a/tests/test_openapi_schema.py b/tests/test_openapi_schema.py new file mode 100644 index 0000000..d6431fd --- /dev/null +++ b/tests/test_openapi_schema.py @@ -0,0 +1,286 @@ +""" +Tests for OpenAPI schema generation with drf-spectacular. + +Ensures that all serializer fields are properly documented in the OpenAPI schema, +particularly dynamically populated fields like PlaceholderSerializer.content. +""" +from rest_framework.reverse import reverse + +from tests.base import RESTTestCase + + +class OpenAPISchemaTestCase(RESTTestCase): + """Test OpenAPI schema generation for djangocms-rest endpoints.""" + + def test_schema_endpoint_accessible(self): + """ + Test that the OpenAPI schema endpoint is accessible. + """ + url = reverse("schema") + response = self.client.get(url) + self.assertEqual(response.status_code, 200) + self.assertIn("openapi", response.data) + self.assertIn("info", response.data) + self.assertIn("paths", response.data) + self.assertIn("components", response.data) + + def test_all_endpoints_have_valid_schemas(self): + """ + Test that all endpoints have valid schema definitions. + + Ensures that: + - All paths have at least one operation defined + - All operations have response schemas + - No operations have error markers + """ + url = reverse("schema") + response = self.client.get(url) + self.assertEqual(response.status_code, 200) + + paths = response.data.get("paths", {}) + self.assertTrue(len(paths) > 0, "Schema should contain at least one endpoint") + + errors = [] + for path, path_item in paths.items(): + # Check that path has at least one HTTP method + methods = [m for m in path_item.keys() if m in ["get", "post", "put", "patch", "delete"]] + if not methods: + errors.append(f"Path '{path}' has no HTTP methods defined") + continue + + for method, operation in path_item.items(): + if method not in ["get", "post", "put", "patch", "delete"]: + continue + + # Check that operation has responses + if "responses" not in operation: + errors.append(f"{method.upper()} {path} has no 'responses' defined") + continue + + # Check for at least one successful response (2xx) + responses = operation["responses"] + has_success = any(str(code).startswith("2") for code in responses.keys()) + if not has_success: + errors.append(f"{method.upper()} {path} has no successful (2xx) response defined") + + # Check that 200/201 responses have content with schema + for code in ["200", "201"]: + if code in responses: + response_obj = responses[code] + if "content" in response_obj: + content = response_obj["content"] + if "application/json" in content: + json_content = content["application/json"] + if "schema" not in json_content: + errors.append(f"{method.upper()} {path} response {code} has no schema defined") + + if errors: + self.fail("Schema validation errors:\n" + "\n".join(f" - {e}" for e in errors)) + + def test_all_serializers_in_components(self): + """ + Test that all used serializers are properly defined in components/schemas. + """ + url = reverse("schema") + response = self.client.get(url) + self.assertEqual(response.status_code, 200) + + components = response.data.get("components", {}) + schemas = components.get("schemas", {}) + + # Expected serializers based on our API + expected_serializers = [ + "Language", + "PageContent", + "PageMeta", + "PageList", + "Placeholder", + "PluginDefinition", + ] + + missing = [] + for serializer_name in expected_serializers: + if serializer_name not in schemas: + missing.append(serializer_name) + + if missing: + available = list(schemas.keys()) + self.fail(f"Missing serializers in schema: {missing}\n" f"Available serializers: {available}") + + def test_all_serializers_have_required_structure(self): + """ + Test that all serializers in the schema have proper structure. + + Checks: + - Each schema has 'type' defined + - Each schema has 'properties' (for object types) + - Properties have types defined + """ + url = reverse("schema") + response = self.client.get(url) + self.assertEqual(response.status_code, 200) + + schemas = response.data.get("components", {}).get("schemas", {}) + errors = [] + + for schema_name, schema_def in schemas.items(): + # Check type is defined + if "type" not in schema_def and "$ref" not in schema_def and "allOf" not in schema_def: + errors.append(f"Schema '{schema_name}' has no 'type', '$ref', or 'allOf' defined") + continue + + # For object types, check properties + if schema_def.get("type") == "object": + if "properties" not in schema_def: + # Some object types might be empty or use additionalProperties + if "additionalProperties" not in schema_def: + errors.append( + f"Schema '{schema_name}' (type: object) has no 'properties' or 'additionalProperties'" + ) + continue + + # Check each property has a type or $ref + properties = schema_def.get("properties", {}) + for prop_name, prop_def in properties.items(): + if isinstance(prop_def, dict): + if ( + "type" not in prop_def + and "$ref" not in prop_def + and "allOf" not in prop_def + and "anyOf" not in prop_def + ): + errors.append( + f"Schema '{schema_name}' property '{prop_name}' has no type or reference defined" + ) + + if errors: + self.fail("Schema structure errors:\n" + "\n".join(f" - {e}" for e in errors)) + + def test_placeholder_serializer_content_field_in_schema(self): + """ + Test that the PlaceholderSerializer.content field appears in the OpenAPI schema. + + This is a regression test for the issue where dynamically populated fields + were not appearing in the schema documentation. + """ + url = reverse("schema") + response = self.client.get(url) + self.assertEqual(response.status_code, 200) + + # Get the schema data + schema = response.data + components = schema.get("components", {}) + schemas = components.get("schemas", {}) + + # Find the PlaceholderSerializer schema + placeholder_schema = schemas.get("Placeholder") + self.assertIsNotNone( + placeholder_schema, + "PlaceholderSerializer should be present in the schema components", + ) + + # Check that the content field is defined + properties = placeholder_schema.get("properties", {}) + self.assertIn( + "content", + properties, + "The 'content' field should be present in PlaceholderSerializer schema", + ) + + # Verify the content field has the correct type + content_field = properties["content"] + self.assertEqual( + content_field.get("type"), + "array", + "The 'content' field should be of type 'array'", + ) + + # Verify the array items are objects + items = content_field.get("items", {}) + self.assertEqual( + items.get("type"), + "object", + "The 'content' field items should be of type 'object'", + ) + + # Verify description is present + self.assertIn( + "description", + content_field, + "The 'content' field should have a description", + ) + + def test_placeholder_serializer_all_fields_in_schema(self): + """ + Test that all PlaceholderSerializer fields are present in the schema. + """ + url = reverse("schema") + response = self.client.get(url) + self.assertEqual(response.status_code, 200) + + schema = response.data + schemas = schema.get("components", {}).get("schemas", {}) + placeholder_schema = schemas.get("Placeholder") + + self.assertIsNotNone(placeholder_schema) + + properties = placeholder_schema.get("properties", {}) + expected_fields = ["slot", "label", "language", "content", "details", "html"] + + for field in expected_fields: + self.assertIn( + field, + properties, + f"Field '{field}' should be present in PlaceholderSerializer schema", + ) + + def test_placeholder_detail_endpoint_in_schema(self): + """ + Test that the placeholder detail endpoint is documented in the schema. + """ + url = reverse("schema") + response = self.client.get(url) + self.assertEqual(response.status_code, 200) + + schema = response.data + paths = schema.get("paths", {}) + + # Check for placeholder detail endpoint pattern + placeholder_endpoints = [path for path in paths.keys() if "placeholder" in path.lower()] + self.assertTrue( + len(placeholder_endpoints) > 0, + "At least one placeholder endpoint should be documented in the schema", + ) + + def test_preview_parameter_documented(self): + """ + Test that the 'preview' query parameter is documented for relevant endpoints. + """ + url = reverse("schema") + response = self.client.get(url) + self.assertEqual(response.status_code, 200) + + paths = response.data.get("paths", {}) + + # Endpoints that should support preview parameter + preview_endpoints = [path for path in paths.keys() if any(x in path for x in ["/pages/", "/placeholders/"])] + + missing_preview = [] + for path in preview_endpoints: + path_item = paths[path] + if "get" in path_item: + operation = path_item["get"] + parameters = operation.get("parameters", []) + + # Check if preview parameter is documented + has_preview = any( + param.get("name") == "preview" and param.get("in") == "query" for param in parameters + ) + + if not has_preview: + missing_preview.append(path) + + # This is informational - some endpoints might not need preview + # So we just check that at least some have it + if preview_endpoints and len(missing_preview) == len(preview_endpoints): + self.fail(f"No preview parameter found in any of the relevant endpoints: {preview_endpoints}") diff --git a/tests/urls.py b/tests/urls.py index ab83d0a..f01bb06 100644 --- a/tests/urls.py +++ b/tests/urls.py @@ -2,6 +2,13 @@ from django.contrib.staticfiles.urls import staticfiles_urlpatterns from django.urls import include, path +try: + from drf_spectacular.views import SpectacularAPIView + + HAS_SPECTACULAR = True +except ImportError: + HAS_SPECTACULAR = False + admin.autodiscover() urlpatterns = [ @@ -9,11 +16,12 @@ "api/", include("djangocms_rest.urls"), ), - path( - "api/pizza//", lambda request, pk: f"", name="pizza-detail" - ), + path("api/pizza//", lambda request, pk: f"", name="pizza-detail"), path("admin/", admin.site.urls), path("", include("cms.urls")), ] +if HAS_SPECTACULAR: + urlpatterns.insert(0, path("api/schema/", SpectacularAPIView.as_view(), name="schema")) + urlpatterns += staticfiles_urlpatterns() From dcbbac98d04362beafbfa4da97883abd6ee06735 Mon Sep 17 00:00:00 2001 From: Fabian Braun Date: Thu, 6 Nov 2025 09:16:11 +0100 Subject: [PATCH 6/6] Update coverage base for test setup --- djangocms_rest/serializers/placeholders.py | 2 +- tests/urls.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/djangocms_rest/serializers/placeholders.py b/djangocms_rest/serializers/placeholders.py index 6de99dc..99ecd5a 100644 --- a/djangocms_rest/serializers/placeholders.py +++ b/djangocms_rest/serializers/placeholders.py @@ -12,7 +12,7 @@ from drf_spectacular.utils import extend_schema_field HAS_SPECTACULAR = True -except ImportError: +except ImportError: # pragma: no cover HAS_SPECTACULAR = False def extend_schema_field(field_schema): diff --git a/tests/urls.py b/tests/urls.py index f01bb06..819577d 100644 --- a/tests/urls.py +++ b/tests/urls.py @@ -6,7 +6,7 @@ from drf_spectacular.views import SpectacularAPIView HAS_SPECTACULAR = True -except ImportError: +except ImportError: # pragma: no cover HAS_SPECTACULAR = False admin.autodiscover()