Reintroduce the ability to easily use the render() return value in a string context

matthiask · matthiask · commit b669cb727a93 · 2022-05-17T08:33:56.000+02:00
diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml
@@ -12,12 +12,12 @@ repos:
       - id: file-contents-sorter
         files: .gitignore
   - repo: https://github.com/asottile/pyupgrade
-    rev: v2.32.0
+    rev: v2.32.1
     hooks:
       - id: pyupgrade
         args: [--py38-plus]
   - repo: https://github.com/adamchainz/django-upgrade
-    rev: 1.5.0
+    rev: 1.7.0
     hooks:
       - id: django-upgrade
         args: [--target-version, "3.2"]
@@ -36,13 +36,13 @@ repos:
       - id: flake8
         args: ["--ignore=E203,E501,W503"]
   - repo: https://github.com/pre-commit/mirrors-eslint
-    rev: v8.13.0
+    rev: v8.15.0
     hooks:
       - id: eslint
         args: [--fix]
         verbose: true
         additional_dependencies:
-          - eslint@8.13.0
+          - eslint@8.15.0
           - eslint-config-prettier@8.5.0
           - "@babel/core"
           - "@babel/eslint-parser"
diff --git a/CHANGELOG.rst b/CHANGELOG.rst
@@ -8,6 +8,12 @@ Change log
 
 .. _Next version: https://github.com/feincms/feincms/compare/v22.2.0...main
 
+- The ``render()`` methods of bundled content types have been changed to return
+  a tuple instead of a HTML fragment in FeinCMS v22.0.0. This was backwards
+  incompatible in some scenarios. Those methods have been changed to return a
+  tuple subclass which automatically renders a HTML fragment if evaluated in a
+  string context.
+
 
 `v22.2.0`_ (2022-05-06)
 ~~~~~~~~~~~~~~~~~~~~~~~
@@ -38,8 +44,9 @@ Change log
 
 .. _v22.0.0: https://github.com/feincms/feincms/compare/v1.20.0...v22.0.0
 
-- Changed all bundled content types' ``render()`` methods to return the
-  ``(template_name, context)`` tuple instead of rendering content themselves.
+- **Possibly backwards incompatible** Changed all bundled content types'
+  ``render()`` methods to return the ``(template_name, context)`` tuple instead
+  of rendering content themselves.
 - Dropped compatibility guarantees with Python < 3.6, Django < 3.2.
 - Added pre-commit.
 - The default view was changed to accept the path as a ``path`` keyword
diff --git a/feincms/content/file/models.py b/feincms/content/file/models.py
@@ -10,6 +10,7 @@
 from django.utils.translation import gettext_lazy as _
 
 from feincms import settings
+from feincms.utils.tuple import AutoRenderTuple
 
 
 class FileContent(models.Model):
@@ -29,7 +30,9 @@ class Meta:
         verbose_name_plural = _("files")
 
     def render(self, **kwargs):
-        return (
-            ["content/file/%s.html" % self.region, "content/file/default.html"],
-            {"content": self},
+        return AutoRenderTuple(
+            (
+                ["content/file/%s.html" % self.region, "content/file/default.html"],
+                {"content": self},
+            )
         )
diff --git a/feincms/content/filer/models.py b/feincms/content/filer/models.py
@@ -4,6 +4,7 @@
 from django.utils.translation import gettext_lazy as _
 
 from feincms.admin.item_editor import FeinCMSInline
+from feincms.utils.tuple import AutoRenderTuple
 
 
 try:
@@ -35,14 +36,16 @@ class Meta:
             abstract = True
 
         def render(self, **kwargs):
-            return (
-                [
-                    f"content/filer/{self.file_type}_{self.type}.html",
-                    "content/filer/%s.html" % self.type,
-                    "content/filer/%s.html" % self.file_type,
-                    "content/filer/default.html",
-                ],
-                {"content": self},
+            return AutoRenderTuple(
+                (
+                    [
+                        f"content/filer/{self.file_type}_{self.type}.html",
+                        "content/filer/%s.html" % self.type,
+                        "content/filer/%s.html" % self.file_type,
+                        "content/filer/default.html",
+                    ],
+                    {"content": self},
+                )
             )
 
     class FilerFileContent(ContentWithFilerFile):
diff --git a/feincms/content/image/models.py b/feincms/content/image/models.py
@@ -11,6 +11,7 @@
 
 from feincms import settings
 from feincms.templatetags import feincms_thumbnail
+from feincms.utils.tuple import AutoRenderTuple
 
 
 class ImageContent(models.Model):
@@ -63,10 +64,7 @@ def render(self, **kwargs):
         if hasattr(self, "position"):
             templates.insert(0, "content/image/%s.html" % self.position)
 
-        return (
-            templates,
-            {"content": self},
-        )
+        return AutoRenderTuple((templates, {"content": self}))
 
     def get_image(self):
         type, separator, size = getattr(self, "format", "").partition(":")
diff --git a/feincms/content/richtext/models.py b/feincms/content/richtext/models.py
@@ -3,6 +3,7 @@
 
 from feincms import settings
 from feincms.contrib.richtext import RichTextField
+from feincms.utils.tuple import AutoRenderTuple
 
 
 class RichTextContent(models.Model):
@@ -29,10 +30,7 @@ class Meta:
         verbose_name_plural = _("rich texts")
 
     def render(self, **kwargs):
-        return (
-            "content/richtext/default.html",
-            {"content": self},
-        )
+        return AutoRenderTuple(("content/richtext/default.html", {"content": self}))
 
     @classmethod
     def initialize_type(cls, cleanse=None):
diff --git a/feincms/content/section/models.py b/feincms/content/section/models.py
@@ -9,6 +9,7 @@
 from feincms.contrib.richtext import RichTextField
 from feincms.module.medialibrary.fields import MediaFileForeignKey
 from feincms.module.medialibrary.models import MediaFile
+from feincms.utils.tuple import AutoRenderTuple
 
 
 class SectionContentInline(FeinCMSInline):
@@ -80,14 +81,16 @@ def render(self, **kwargs):
         else:
             mediafile_type = "nomedia"
 
-        return (
-            [
-                f"content/section/{mediafile_type}_{self.type}.html",
-                "content/section/%s.html" % mediafile_type,
-                "content/section/%s.html" % self.type,
-                "content/section/default.html",
-            ],
-            {"content": self},
+        return AutoRenderTuple(
+            (
+                [
+                    f"content/section/{mediafile_type}_{self.type}.html",
+                    "content/section/%s.html" % mediafile_type,
+                    "content/section/%s.html" % self.type,
+                    "content/section/default.html",
+                ],
+                {"content": self},
+            )
         )
 
     def save(self, *args, **kwargs):
diff --git a/feincms/content/template/models.py b/feincms/content/template/models.py
@@ -3,6 +3,7 @@
 
 from feincms.content.raw.models import RawContent  # noqa
 from feincms.content.richtext.models import RichTextContent  # noqa
+from feincms.utils.tuple import AutoRenderTuple
 
 
 class TemplateContent(models.Model):
@@ -30,7 +31,4 @@ def initialize_type(cls, TEMPLATES):
         )
 
     def render(self, **kwargs):
-        return (
-            self.template,
-            {"content": self},
-        )
+        return AutoRenderTuple((self.template, {"content": self}))
diff --git a/feincms/content/video/models.py b/feincms/content/video/models.py
@@ -3,6 +3,8 @@
 from django.db import models
 from django.utils.translation import gettext_lazy as _
 
+from feincms.utils.tuple import AutoRenderTuple
+
 
 class VideoContent(models.Model):
     """
@@ -69,7 +71,4 @@ def ctx_for_video(self, vurl):
 
     def render(self, **kwargs):
         ctx = self.ctx_for_video(self.video)
-        return (
-            self.get_templates(ctx["portal"]),
-            ctx,
-        )
+        return AutoRenderTuple((self.get_templates(ctx["portal"]), ctx))
diff --git a/feincms/extensions/translations.py b/feincms/extensions/translations.py
@@ -21,7 +21,7 @@
 
 from django.conf import settings as django_settings
 from django.db import models
-from django.http import HttpResponseRedirect, HttpRequest
+from django.http import HttpRequest, HttpResponseRedirect
 from django.utils import translation
 from django.utils.safestring import mark_safe
 from django.utils.translation import gettext_lazy as _
@@ -43,6 +43,7 @@
 
 PRIMARY_LANGUAGE: str = django_settings.LANGUAGES[0][0]
 
+
 # ------------------------------------------------------------------------
 def user_has_language_set(request: HttpRequest) -> bool:
     """
@@ -72,7 +73,9 @@ def translation_allowed_language(select_language: str) -> str:
 
 
 # ------------------------------------------------------------------------
-def translation_set_language(request: HttpRequest, select_language: str) -> Optional[HttpResponseRedirect]:
+def translation_set_language(
+    request: HttpRequest, select_language: str
+) -> Optional[HttpResponseRedirect]:
     """
     Set and activate a language, if that language is available.
     """
@@ -95,7 +98,9 @@ def translation_set_language(request: HttpRequest, select_language: str) -> Opti
 
     if hasattr(request, "session"):
         # User has a session, then set this language there
-        current_session_language = request.session.get(LANGUAGE_SESSION_KEY, PRIMARY_LANGUAGE)
+        current_session_language = request.session.get(
+            LANGUAGE_SESSION_KEY, PRIMARY_LANGUAGE
+        )
 
         if select_language != current_session_language:
             request.session[LANGUAGE_SESSION_KEY] = select_language
diff --git a/feincms/module/medialibrary/contents.py b/feincms/module/medialibrary/contents.py
@@ -5,6 +5,7 @@
 
 from feincms.admin.item_editor import FeinCMSInline
 from feincms.module.medialibrary.fields import ContentWithMediaFile
+from feincms.utils.tuple import AutoRenderTuple
 
 
 class MediaFileContentInline(FeinCMSInline):
@@ -62,12 +63,14 @@ def initialize_type(cls, TYPE_CHOICES=None):
         )
 
     def render(self, **kwargs):
-        return (
-            [
-                f"content/mediafile/{self.mediafile.type}_{self.type}.html",
-                "content/mediafile/%s.html" % self.mediafile.type,
-                "content/mediafile/%s.html" % self.type,
-                "content/mediafile/default.html",
-            ],
-            {"content": self},
+        return AutoRenderTuple(
+            (
+                [
+                    f"content/mediafile/{self.mediafile.type}_{self.type}.html",
+                    "content/mediafile/%s.html" % self.mediafile.type,
+                    "content/mediafile/%s.html" % self.type,
+                    "content/mediafile/default.html",
+                ],
+                {"content": self},
+            )
         )
diff --git a/feincms/static/feincms/tree_editor.js b/feincms/static/feincms/tree_editor.js
@@ -301,13 +301,16 @@ feincms.jQuery(($) => {
        the current state of the tree so we can restore it on a reload.
        Note: We might use html5's session storage? */
   function storeCollapsedNodes(nodes) {
-    const v = JSON.stringify(nodes);
-    Cookies.set("feincms_collapsed_nodes", v, { expires: 7, sameSite: 'strict' })
+    const v = JSON.stringify(nodes)
+    Cookies.set("feincms_collapsed_nodes", v, {
+      expires: 7,
+      sameSite: "strict",
+    })
   }
 
   function retrieveCollapsedNodes() {
-    const collapse_cookie = Cookies.get('feincms_collapsed_nodes');
-    return collapse_cookie ? JSON.parse(collapse_cookie) : null;
+    const collapse_cookie = Cookies.get("feincms_collapsed_nodes")
+    return collapse_cookie ? JSON.parse(collapse_cookie) : null
   }
 
   function expandOrCollapseNode(item) {
diff --git a/feincms/utils/tuple.py b/feincms/utils/tuple.py
@@ -0,0 +1,20 @@
+"""
+Content types in FeinCMS 1.x returned a HTML fragment from ``render()``
+methods.
+
+FeinCMS 22.x changed them to return a ``(template_name, context)`` tuple. This
+allows rendering content type templates while also having access to all
+template variables set in views and context processors without having to
+explicitly pass them on or something.
+
+Some old projects still rely on ``render()`` returning a HTML string. This
+tuple subclass combines the advantages of both approaches: Newer projects see a
+tuple, old projects still get a string.
+"""
+
+from django.template.loader import render_to_string
+
+
+class AutoRenderTuple(tuple):
+    def __str__(self):
+        return render_to_string(self[0], self[1])
diff --git a/tests/testapp/tests/test_extensions.py b/tests/testapp/tests/test_extensions.py
@@ -1,7 +1,3 @@
-
-from unittest import skipIf
-
-from django import VERSION
 from django.conf import settings as django_settings
 from django.contrib.sites.models import Site
 from django.template.defaultfilters import slugify
@@ -105,7 +101,9 @@ def test_translation_set_language_to_session_primary(self):
 
         self.assertEqual(request.LANGUAGE_CODE, "en")
         # We avoid setting the translation language to the primary language, so should not be set
-        self.assertEqual(request.session.get(self.language_session_key, 'unset'), "unset")
+        self.assertEqual(
+            request.session.get(self.language_session_key, "unset"), "unset"
+        )
 
     def test_translation_set_language_to_cookie(self):
         factory = RequestFactory()
diff --git a/tests/testapp/tests/test_stuff.py b/tests/testapp/tests/test_stuff.py
@@ -12,6 +12,7 @@
 from feincms.extensions.datepublisher import granular_now
 from feincms.models import Region, Template
 from feincms.utils import get_object, shorten_string
+from feincms.utils.tuple import AutoRenderTuple
 
 
 # ------------------------------------------------------------------------
@@ -69,6 +70,14 @@ def test_shorten_string(self):
         self.assertEqual(string, "Badger-ger")
         self.assertEqual(len(string), 10)
 
+    def test_autotuple(self):
+        t = AutoRenderTuple(
+            ("content/richtext/default.html", {"content": {"text": "hello"}})
+        )
+
+        self.assertTrue(isinstance(t, tuple))
+        self.assertEqual(str(t).strip(), "hello")
+
 
 # ------------------------------------------------------------------------
 class TimezoneTest(TestCase):
