Skip to content
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 4 additions & 14 deletions djangocms_rest/serializers/pages.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,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


Expand Down Expand Up @@ -131,7 +131,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)
Expand All @@ -142,19 +142,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},
Expand Down
77 changes: 31 additions & 46 deletions djangocms_rest/serializers/placeholders.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
from urllib.parse import urlencode

from django.template import Context
from django.urls import reverse

Expand All @@ -12,71 +14,54 @@ class PlaceholderSerializer(serializers.Serializer):
label = serializers.CharField()
language = serializers.CharField()
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)
Comment on lines +54 to +56
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion (bug_risk): Directly mutating the instance in 'to_representation' can have unintended side effects.

Using a local dictionary to build the representation avoids unintended changes to the input object and prevents side effects if the instance is reused elsewhere.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@metaforx What do you think about this? Maybe we change the pattern.

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
if not hasattr(instance, key):
setattr(instance, key, value)


class PlaceholderRelationSerializer(serializers.Serializer):
content_type_id = serializers.IntegerField()
object_id = serializers.IntegerField()
slot = serializers.CharField()
details = serializers.URLField()

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):
api_endpoint = 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._preview_mode:
if "?" in api_endpoint:
api_endpoint += "&preview=1"
else:
api_endpoint += "?preview=1"
return api_endpoint
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
19 changes: 7 additions & 12 deletions tests/types.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
Expand All @@ -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,
Expand Down