From 812f3e27f2510a157723e69504cb3c89aea0e7d6 Mon Sep 17 00:00:00 2001 From: Jonas Buchholz Date: Tue, 28 Jul 2026 10:13:35 +0000 Subject: [PATCH 1/8] infer shortcodes from html --- integreat_cms/api/v3/events.py | 6 +- integreat_cms/api/v3/imprint.py | 6 +- integreat_cms/api/v3/locations.py | 6 +- integreat_cms/api/v3/pages.py | 7 +- integreat_cms/api/v3/social_media_headers.py | 7 +- .../cms/forms/custom_content_model_form.py | 6 + .../models/abstract_content_translation.py | 17 + .../cms/templates/content_versions.html | 2 +- .../pages/_page_xliff_import_diff.html | 4 +- .../cms/templates/pages/page_pdf.html | 6 +- .../cms/templates/pois/poi_view.html | 2 +- .../cms/templatetags/content_filters.py | 16 + integreat_cms/cms/utils/content_utils.py | 77 ++-- .../cms/utils/internal_link_utils.py | 86 +++++ .../cms/utils/link_shortcode_utils.py | 358 +++++++++++++++++ .../cms/utils/shortcodes/__init__.py | 26 +- integreat_cms/cms/utils/shortcodes/page.py | 145 +++++-- integreat_cms/cms/utils/shortcodes/utils.py | 18 +- integreat_cms/cms/views/pages/page_actions.py | 10 +- integreat_cms/core/signals/hix_signals.py | 3 +- .../utils/machine_translation_api_client.py | 16 + tests/cms/utils/test_link_shortcode_utils.py | 361 ++++++++++++++++++ tests/cms/utils/test_shortcodes.py | 103 +++++ 23 files changed, 1209 insertions(+), 79 deletions(-) create mode 100644 integreat_cms/cms/utils/link_shortcode_utils.py create mode 100644 tests/cms/utils/test_link_shortcode_utils.py create mode 100644 tests/cms/utils/test_shortcodes.py diff --git a/integreat_cms/api/v3/events.py b/integreat_cms/api/v3/events.py index aa22ef0e79..34fa4914a9 100644 --- a/integreat_cms/api/v3/events.py +++ b/integreat_cms/api/v3/events.py @@ -12,6 +12,7 @@ from django.utils import timezone from django.utils.html import strip_tags +from ...cms.utils.shortcodes import expand_shortcodes_of from ..decorators import json_response from .locations import transform_poi @@ -82,6 +83,7 @@ def transform_event_translation( else f"{event_translation.slug}${recurrence_date}" ) absolute_url = event_translation.url_prefix + slug + "/" + content = expand_shortcodes_of(event_translation) return { "id": event_translation.id, "url": settings.BASE_URL + absolute_url, @@ -92,8 +94,8 @@ def transform_event_translation( "published_at": timezone.localtime( event_translation.published_at or event_translation.last_updated, ), - "excerpt": strip_tags(event_translation.content), - "content": event_translation.content, + "excerpt": strip_tags(content), + "content": content, "available_languages": ( transform_available_languages(event_translation, recurrence_date) if recurrence_date diff --git a/integreat_cms/api/v3/imprint.py b/integreat_cms/api/v3/imprint.py index 7e67d08bd0..3455f4914d 100644 --- a/integreat_cms/api/v3/imprint.py +++ b/integreat_cms/api/v3/imprint.py @@ -19,6 +19,7 @@ from ...cms.models.pages.imprint_page_translation import ImprintPageTranslation +from ...cms.utils.shortcodes import expand_shortcodes_of from ..decorators import json_response logger = logging.getLogger(__name__) @@ -32,6 +33,7 @@ def transform_imprint(imprint_translation: ImprintPageTranslation) -> dict[str, :return: data necessary for API """ absolute_url = imprint_translation.get_absolute_url() + content = expand_shortcodes_of(imprint_translation) return { "id": imprint_translation.id, "url": settings.BASE_URL + absolute_url, @@ -39,8 +41,8 @@ def transform_imprint(imprint_translation: ImprintPageTranslation) -> dict[str, "title": imprint_translation.title, "modified_gmt": imprint_translation.last_updated, # deprecated field in the future "last_updated": timezone.localtime(imprint_translation.last_updated), - "excerpt": strip_tags(imprint_translation.content), - "content": imprint_translation.content, + "excerpt": strip_tags(content), + "content": content, "parent": None, "available_languages": imprint_translation.available_languages_dict, "thumbnail": None, diff --git a/integreat_cms/api/v3/locations.py b/integreat_cms/api/v3/locations.py index ca6db6d8fb..75d517c4c1 100644 --- a/integreat_cms/api/v3/locations.py +++ b/integreat_cms/api/v3/locations.py @@ -15,6 +15,7 @@ from ...cms.constants import status from ...cms.models import Contact, POICategoryTranslation from ...cms.models.pois.poi import get_default_opening_hours +from ...cms.utils.shortcodes import expand_shortcodes_of from ...core.utils.strtobool import strtobool from ..decorators import json_response from .location_categories import transform_location_category @@ -116,6 +117,7 @@ def transform_poi_translation( :return: Data for the APIv3 locations endpoint. """ poi = poi_translation.poi + content = expand_shortcodes_of(poi_translation) contacts = Contact.objects.filter(location=poi).all() @@ -164,8 +166,8 @@ def transform_poi_translation( poi_translation.published_at or poi_translation.last_updated, ), "meta_description": poi_translation.meta_description, - "excerpt": strip_tags(poi_translation.content), - "content": poi_translation.content, + "excerpt": strip_tags(content), + "content": content, "available_languages": poi_translation.available_languages_dict, "icon": poi.icon.url if poi.icon else None, "thumbnail": poi.icon.thumbnail_url if poi.icon else None, diff --git a/integreat_cms/api/v3/pages.py b/integreat_cms/api/v3/pages.py index 5fd7935055..578f39f93b 100644 --- a/integreat_cms/api/v3/pages.py +++ b/integreat_cms/api/v3/pages.py @@ -86,6 +86,7 @@ def transform_page( organization = page.organization absolute_url = page_translation.get_absolute_url() + content = expand_shortcodes(page_translation.combined_text, context=context) return { "id": page_translation.id, "url": settings.BASE_URL + absolute_url, @@ -96,10 +97,8 @@ def transform_page( "published_at": timezone.localtime( page_translation.published_at or page_translation.last_updated, ), - "excerpt": strip_tags( - expand_shortcodes(page_translation.combined_text, context=context) - ), - "content": expand_shortcodes(page_translation.combined_text, context=context), + "excerpt": strip_tags(content), + "content": content, "parent": parent, "order": order, "available_languages": page_translation.available_languages_dict, diff --git a/integreat_cms/api/v3/social_media_headers.py b/integreat_cms/api/v3/social_media_headers.py index 7f211db73e..feff50d412 100644 --- a/integreat_cms/api/v3/social_media_headers.py +++ b/integreat_cms/api/v3/social_media_headers.py @@ -18,6 +18,7 @@ from ...cms.utils.internal_link_utils import ( get_public_translation_for_webapp_link_parts, ) +from ...cms.utils.shortcodes import expand_shortcodes_of from ...cms.utils.social_media_utils import ( get_excerpt, get_region_title, @@ -228,7 +229,7 @@ def page_social_media_headers( request=request, title=get_region_title(region, page_translation.title), language_code=language.bcp47_tag, - excerpt=get_excerpt(page_translation.content), + excerpt=get_excerpt(expand_shortcodes_of(page_translation)), url=page_translation.full_url, ) @@ -270,7 +271,7 @@ def event_social_media_headers( request=request, title=get_region_title(region, event_translation.title), language_code=language.bcp47_tag, - excerpt=get_excerpt(event_translation.content), + excerpt=get_excerpt(expand_shortcodes_of(event_translation)), url=event_translation.full_url, ) @@ -342,6 +343,6 @@ def location_social_media_headers( request=request, title=get_region_title(region, location_translation.title), language_code=language.bcp47_tag, - excerpt=get_excerpt(location_translation.content), + excerpt=get_excerpt(expand_shortcodes_of(location_translation)), url=location_translation.full_url, ) diff --git a/integreat_cms/cms/forms/custom_content_model_form.py b/integreat_cms/cms/forms/custom_content_model_form.py index 8771be7301..026f719fc7 100644 --- a/integreat_cms/cms/forms/custom_content_model_form.py +++ b/integreat_cms/cms/forms/custom_content_model_form.py @@ -55,6 +55,12 @@ def __init__(self, **kwargs: Any) -> None: with suppress(ObjectDoesNotExist): self.locked_by_user = self.instance.foreign_object.get_locking_user() + # References to internal content are stored as shortcodes, but editors should keep + # working with ordinary links, so expand them before they are put into the editor + if "content" in self.fields and self.instance.content: + with suppress(ObjectDoesNotExist): + self.initial["content"] = self.instance.content_with_expanded_links + def clean(self) -> dict[str, Any]: """ This method extends the ``clean()``-method to verify that a user can modify this content model diff --git a/integreat_cms/cms/models/abstract_content_translation.py b/integreat_cms/cms/models/abstract_content_translation.py index 01743e0d14..15aa43457d 100644 --- a/integreat_cms/cms/models/abstract_content_translation.py +++ b/integreat_cms/cms/models/abstract_content_translation.py @@ -539,6 +539,23 @@ def path(self) -> str: """ return str(self) + @cached_property + def content_with_expanded_links(self) -> str: + """ + The content as it should be presented to users of the CMS, which means with all + shortcodes referencing internal content expanded into ordinary links. + + Everything which is saved back through + :class:`~integreat_cms.cms.forms.custom_content_model_form.CustomContentModelForm` + is collapsed into shortcodes again. + + :return: The content with expanded links + """ + # Imported here because the utils import the models + from ..utils.link_shortcode_utils import expand_link_shortcodes + + return expand_link_shortcodes(self.content, self.language.slug) + @cached_property def hix_enabled(self) -> bool: """ diff --git a/integreat_cms/cms/templates/content_versions.html b/integreat_cms/cms/templates/content_versions.html index b7840ea917..2f508c7925 100644 --- a/integreat_cms/cms/templates/content_versions.html +++ b/integreat_cms/cms/templates/content_versions.html @@ -109,7 +109,7 @@

- {{ translation.content|safe }} + {{ translation.content_with_expanded_links|safe }}
diff --git a/integreat_cms/cms/templates/pages/_page_xliff_import_diff.html b/integreat_cms/cms/templates/pages/_page_xliff_import_diff.html index 9770cd4d98..aaf90a03e5 100644 --- a/integreat_cms/cms/templates/pages/_page_xliff_import_diff.html +++ b/integreat_cms/cms/templates/pages/_page_xliff_import_diff.html @@ -78,13 +78,13 @@

{{ diff.existing.title }}

- {{ diff.existing.content|safe }} + {{ diff.existing.content_with_expanded_links|safe }}
diff --git a/integreat_cms/cms/templates/pages/page_pdf.html b/integreat_cms/cms/templates/pages/page_pdf.html index fac1afc8e1..7f84398877 100644 --- a/integreat_cms/cms/templates/pages/page_pdf.html +++ b/integreat_cms/cms/templates/pages/page_pdf.html @@ -113,11 +113,11 @@

{{ page_translation.title }}

{% if page.mirrored_page_first %} - {{ page_translation.mirrored_translation_text|pdf_strip_fontstyles|pdf_truncate_links:80|safe }} + {{ page_translation.mirrored_translation_text|expand_links:language.slug|pdf_strip_fontstyles|pdf_truncate_links:80|safe }} {% endif %} - {{ page_translation.content|pdf_strip_fontstyles|pdf_truncate_links:80|safe }} + {{ page_translation.content_with_expanded_links|pdf_strip_fontstyles|pdf_truncate_links:80|safe }} {% if not page.mirrored_page_first %} - {{ page_translation.mirrored_translation_text|pdf_strip_fontstyles|pdf_truncate_links:80|safe }} + {{ page_translation.mirrored_translation_text|expand_links:language.slug|pdf_strip_fontstyles|pdf_truncate_links:80|safe }} {% endif %}
{% for close in info.close %} diff --git a/integreat_cms/cms/templates/pois/poi_view.html b/integreat_cms/cms/templates/pois/poi_view.html index 73dceba5ae..688d3f660c 100644 --- a/integreat_cms/cms/templates/pois/poi_view.html +++ b/integreat_cms/cms/templates/pois/poi_view.html @@ -5,6 +5,6 @@

{{ poi_translation.title }}

- {{ poi_translation.content|safe }} + {{ poi_translation.content_with_expanded_links|safe }}
{% endblock raw_content %} diff --git a/integreat_cms/cms/templatetags/content_filters.py b/integreat_cms/cms/templatetags/content_filters.py index b298153e2d..005c0dd4de 100644 --- a/integreat_cms/cms/templatetags/content_filters.py +++ b/integreat_cms/cms/templatetags/content_filters.py @@ -22,6 +22,7 @@ PageTranslation, POITranslation, ) +from ..utils.link_shortcode_utils import expand_link_shortcodes if TYPE_CHECKING: from collections.abc import Iterable @@ -164,6 +165,21 @@ def build_url( return reverse(target, kwargs=kwargs) +@register.filter +def expand_links(content: str, language_slug: str) -> str: + """ + Expand the shortcodes which reference internal content into ordinary links. + + This is needed wherever content is presented to users of the CMS instead of being + delivered through the API, for example in the PDF export. + + :param content: The content as it is stored in the database + :param language_slug: The slug of the language the content should be presented in + :return: The content with expanded links + """ + return expand_link_shortcodes(content, language_slug) + + @register.filter def remove(elements: list[Any], element: Any) -> list[Any]: """ diff --git a/integreat_cms/cms/utils/content_utils.py b/integreat_cms/cms/utils/content_utils.py index 48b799985c..de95257a5b 100644 --- a/integreat_cms/cms/utils/content_utils.py +++ b/integreat_cms/cms/utils/content_utils.py @@ -14,6 +14,7 @@ from ..models import Contact, MediaFile from ..utils import internal_link_utils +from ..utils.link_shortcode_utils import collapse_link_to_shortcode from ..utils.link_utils import fix_content_link_encoding logger = logging.getLogger(__name__) @@ -127,10 +128,16 @@ def update_links(content: HtmlElement, language_slug: str) -> None: """ Super method that gathers all methods related to updating links + Links to internal pages are replaced by the shortcode representing them, so that they are + only resolved when the content is delivered and never reach the link index of ``linkcheck``. + :param content: The content whose links should be updated :param language_slug: Slug of the current language """ - for link in content.iter("a"): + for link in list(content.iter("a")): + if collapse_link_to_shortcode(link): + # The link does not exist anymore, so there is nothing left to update + continue mark_external_links(link) remove_target_attribute(link) update_internal_links(link, language_slug) @@ -331,37 +338,49 @@ def hide_anchor_tag_around_image(content: HtmlElement) -> None: """ for anchor in content.iter("a"): - children = list(anchor.iterchildren()) - - # Check if the anchor tag has only img children and no other text content - if ( - len(children) == 1 - and (img := children[0]).tag == "img" - and not anchor.text_content().strip() - ): - if img.attrib.get("alt", ""): - if "aria-hidden" in anchor.attrib: - del anchor.attrib["aria-hidden"] - logger.debug( - "Removed 'aria-hidden' from anchor: %r", - tostring(anchor, encoding="unicode"), - ) - if "tabindex" in anchor.attrib: - del anchor.attrib["tabindex"] - logger.debug( - "Removed 'tabindex' from anchor: %r", - tostring(anchor, encoding="unicode"), - ) - else: - # Hide the anchor tag by setting aria-hidden attribute if the image alt text is empty - anchor.set("aria-hidden", "true") + hide_anchor_tag_around_single_image(anchor) + + +def hide_anchor_tag_around_single_image(anchor: HtmlElement) -> None: + """ + Apply :func:`~integreat_cms.cms.utils.content_utils.hide_anchor_tag_around_image` to a single anchor. + + This is also needed when a link is rendered from a shortcode, because those links do not + exist yet when the content is cleaned. + + :param anchor: the anchor tag which might be wrapped around an img tag + """ + children = list(anchor.iterchildren()) + + # Check if the anchor tag has only img children and no other text content + if ( + len(children) == 1 + and (img := children[0]).tag == "img" + and not anchor.text_content().strip() + ): + if img.attrib.get("alt", ""): + if "aria-hidden" in anchor.attrib: + del anchor.attrib["aria-hidden"] logger.debug( - "Set 'aria-hidden' to true for anchor: %r", + "Removed 'aria-hidden' from anchor: %r", tostring(anchor, encoding="unicode"), ) - # Unfocus the anchor tag from tab key - anchor.set("tabindex", "-1") + if "tabindex" in anchor.attrib: + del anchor.attrib["tabindex"] logger.debug( - "Set 'tabindex' to -1 for anchor: %r", + "Removed 'tabindex' from anchor: %r", tostring(anchor, encoding="unicode"), ) + else: + # Hide the anchor tag by setting aria-hidden attribute if the image alt text is empty + anchor.set("aria-hidden", "true") + logger.debug( + "Set 'aria-hidden' to true for anchor: %r", + tostring(anchor, encoding="unicode"), + ) + # Unfocus the anchor tag from tab key + anchor.set("tabindex", "-1") + logger.debug( + "Set 'tabindex' to -1 for anchor: %r", + tostring(anchor, encoding="unicode"), + ) diff --git a/integreat_cms/cms/utils/internal_link_utils.py b/integreat_cms/cms/utils/internal_link_utils.py index bbae8c92d5..5c2bb1bd9a 100644 --- a/integreat_cms/cms/utils/internal_link_utils.py +++ b/integreat_cms/cms/utils/internal_link_utils.py @@ -14,11 +14,14 @@ from ..models import ( EventTranslation, ImprintPageTranslation, + Page, PageTranslation, POITranslation, ) if TYPE_CHECKING: + from typing import Final + from lxml.html import Element from ..models.abstract_content_translation import AbstractContentTranslation @@ -190,3 +193,86 @@ def get_public_translation_for_short_link( return None return instance.public_version + + +#: The first path segment of urls which point to something else than a page +NON_PAGE_URL_INFIXES: Final[frozenset[str]] = frozenset( + {"events", "locations", "disclaimer", "news", "offers", "search"}, +) + + +def get_page_for_link(url: str) -> Page | None: + """ + Get the page an internal url points to. + + In contrast to :func:`~integreat_cms.cms.utils.internal_link_utils.get_public_translation_for_link`, + this does not care about the publication status of the target, because links to pages which + are not public (yet) have to be recognized as internal references as well. + + :param url: The url + :returns: The referenced page, or ``None`` if the url does not point to one + """ + if not url: + return None + parsed_url = urlparse(url) + if parsed_url.netloc == WEBAPP_NETLOC: + return get_page_for_webapp_link(parsed_url.path) + if parsed_url.netloc == SHORT_LINKS_NETLOC: + return get_page_for_short_link(parsed_url.path) + return None + + +def get_page_for_webapp_link(path: str) -> Page | None: + """ + Get the page a webapp url path points to + + :param path: The url path, for example ``/augsburg/de/willkommen/`` + :returns: The referenced page, or ``None`` if the path does not point to one + """ + parts: list[str] = unquote(path).strip("/").split("/") + if len(parts) < 3: + # Not a link to a specific piece of content + return None + + region_slug, language_slug, *path_parts = parts + if path_parts[0] in NON_PAGE_URL_INFIXES: + return None + + pages = Page.objects.filter( + region__slug=region_slug, + translations__language__slug=language_slug, + translations__slug=path_parts[-1], + ).distinct() + + if len(pages) < 2: + return pages.first() + + # The slug of a page is only unique among its siblings, so if the last path part is + # ambiguous, prefer the page whose current url matches the whole path. Outdated urls + # are still tolerated, because their slug is kept in the version history. + for page in pages: + if ( + translation := page.get_translation(language_slug) + ) and translation.get_absolute_url().strip("/") == "/".join(parts): + return page + return pages.first() + + +def get_page_for_short_link(path: str) -> Page | None: + """ + Get the page a short url path points to + + :param path: The url path, for example ``/s/p/124/`` + :returns: The referenced page, or ``None`` if the path does not point to one + """ + parts: list[str] = unquote(path).strip("/").split("/") + if len(parts) != 3 or parts[0] != "s" or parts[1] != "p": + # Short links to other content types do not have a page shortcode (yet) + return None + + try: + translation_id = int(parts[2]) + except ValueError: + return None + + return Page.objects.filter(translations__id=translation_id).first() diff --git a/integreat_cms/cms/utils/link_shortcode_utils.py b/integreat_cms/cms/utils/link_shortcode_utils.py new file mode 100644 index 0000000000..56d0667e80 --- /dev/null +++ b/integreat_cms/cms/utils/link_shortcode_utils.py @@ -0,0 +1,358 @@ +""" +This file contains utility functions to convert between links to internal content and the +shortcodes which represent them. + +Internal references are stored as shortcodes so that they are only resolved when the content +is delivered (see :doc:`ADR 0001 <../../../ADR/0001-compose-referenced-objects-into-content-dynamically-shortcodes>`). +This means the link index kept by our ``linkcheck`` dependency never has to know about them. + +Editors should not have to care about that, so the shortcodes are expanded into ordinary +links whenever content is loaded into an editor (see +:func:`~integreat_cms.cms.utils.link_shortcode_utils.expand_link_shortcodes`) and collapsed +back into shortcodes whenever content is saved (see +:func:`~integreat_cms.cms.utils.link_shortcode_utils.collapse_links_to_shortcodes`). +""" + +from __future__ import annotations + +import logging +from copy import deepcopy +from typing import TYPE_CHECKING + +import shortcodes +from lxml.etree import LxmlError +from lxml.html import Element, fromstring, tostring + +from ..models import Page +from .internal_link_utils import get_page_for_link + +if TYPE_CHECKING: + from typing import Any, Final + + from lxml.html import HtmlElement + + from ..models.pages.page_translation import PageTranslation + +logger = logging.getLogger(__name__) + +#: The keyword of the atomic shortcode which links to a page +PAGE_KEYWORD: Final[str] = "page" + +#: The keyword of the block scoped shortcode which wraps its content in a link to a page +PAGE_LINK_KEYWORD: Final[str] = "page_link" + +#: The keyword which closes :data:`PAGE_LINK_KEYWORD` +PAGE_LINK_END_KEYWORD: Final[str] = "/page_link" + +#: The attribute which marks a link whose text should follow the title of its target +AUTO_UPDATE_ATTRIBUTE: Final[str] = "data-integreat-auto-update" + +#: Characters which must not appear in a quoted shortcode argument. ``"``, ``\``, ``[`` and +#: ``]`` confuse the shortcode parser, which stops at the first closing delimiter and does not +#: unescape anything inside quotes. ``&``, ``<`` and ``>`` are html escaped when the content is +#: serialized, which would pile up another layer of escaping on every save. +#: Link texts containing any of them use the block scoped shortcode instead, whose content is +#: html and therefore not affected. +UNQUOTABLE_CHARACTERS: Final[frozenset[str]] = frozenset('"\\[]&<>') + + +def format_page_shortcode(page_id: int, text: str | None = None) -> str: + """ + Build the atomic shortcode which links to a page + + :param page_id: The id of the :class:`~integreat_cms.cms.models.pages.page.Page` to link to + :param text: The link text, or ``None`` to let the link follow the title of its target + :return: The shortcode + """ + if text is None: + return f"[{PAGE_KEYWORD} {page_id}]" + return f'[{PAGE_KEYWORD} {page_id} "{text}"]' + + +def format_page_link_shortcode(page_id: int, content: str) -> str: + """ + Build the block scoped shortcode which wraps ``content`` in a link to a page + + :param page_id: The id of the :class:`~integreat_cms.cms.models.pages.page.Page` to link to + :param content: The inner html of the link + :return: The shortcode + """ + return f"[{PAGE_LINK_KEYWORD} {page_id}]{content}[{PAGE_LINK_END_KEYWORD}]" + + +def get_editor_page_translation( + page_id: str | int | None, + language_slug: str | None, +) -> PageTranslation | None: + """ + Get the page translation a link shortcode should point to while the content is edited. + + In contrast to the delivered content, the editor also has to be able to show links to + pages which are not public (yet), because it is possible to insert such links. + + :param page_id: The id of the referenced :class:`~integreat_cms.cms.models.pages.page.Page` + :param language_slug: The slug of the language the content is edited in + :return: The referenced translation, or ``None`` if it cannot be resolved + """ + if not page_id or not language_slug: + return None + try: + page = Page.objects.get(id=page_id) + except (Page.DoesNotExist, TypeError, ValueError): + logger.debug( + "Page with id=%r referenced by a shortcode does not exist", page_id + ) + return None + return ( + page.get_translation(language_slug) + or page.get_public_translation(language_slug) + or page.best_translation + ) + + +def expand_link_shortcodes(content: str, language_slug: str) -> str: + """ + Replace all link shortcodes in ``content`` by the link they represent. + + Shortcodes which cannot be resolved are kept verbatim, so that editing content with a + broken reference does not silently drop that reference. + + :param content: The content as it is stored in the database + :param language_slug: The slug of the language the content should be presented in + :return: The content with expanded links + """ + try: + return _link_parser.parse(content, {"language_slug": language_slug}) + except shortcodes.ShortcodeError: + logger.warning( + "Failed expanding link shortcodes in %r", + content, + exc_info=True, + ) + # The best way to fail gracefully is to keep the content as it is + return content + + +def collapse_links_to_shortcodes(content: HtmlElement) -> None: + """ + Replace all links to internal pages in ``content`` by the shortcode representing them + + :param content: The content whose links should be collapsed + """ + for link in list(content.iter("a")): + collapse_link_to_shortcode(link) + + +def collapse_link_to_shortcode(link: HtmlElement) -> bool: + """ + Replace ``link`` by the shortcode representing it, if it points to an internal page. + + Which shortcode is used depends on the content of the link: + + .. list-table:: + :widths: 55 45 + :header-rows: 1 + + * - Link + - Shortcode + * - ``Willkommen`` + - ``[page 1]`` + * - ``hier`` + - ``[page 1 "hier"]`` + * - ```` + - ``[page_link 1][/page_link]`` + + :param link: The link which should be collapsed + :return: Whether the link was replaced + """ + if not (page := get_page_for_link(link.get("href", ""))): + return False + if (parent := link.getparent()) is None: + logger.debug("Cannot collapse link %r without a parent element", link) + return False + + index = parent.index(link) + tail = link.tail or "" + text = link.text or "" + children = list(link) + + if link.get(AUTO_UPDATE_ATTRIBUTE) == "true": + # The link follows the title of its target, so its current content is irrelevant + opening, closing, children = format_page_shortcode(page.id), "", [] + elif not children and not UNQUOTABLE_CHARACTERS.intersection(text): + opening, closing = format_page_shortcode(page.id, text), "" + elif not children: + opening, closing = format_page_link_shortcode(page.id, text), "" + else: + # The children have to stay elements of the content, so the block scoped shortcode + # is split into the text around them + opening = f"[{PAGE_LINK_KEYWORD} {page.id}]{text}" + closing = f"[{PAGE_LINK_END_KEYWORD}]" + + parent.remove(link) + for offset, child in enumerate(children): + parent.insert(index + offset, child) + if children: + children[-1].tail = (children[-1].tail or "") + closing + tail + _append_text_before(parent, index, opening) + else: + _append_text_before(parent, index, opening + closing + tail) + + logger.debug("Collapsed link to %r into a shortcode", page) + return True + + +def _append_text_before(parent: HtmlElement, index: int, text: str) -> None: + """ + Append ``text`` to the character data which precedes the child of ``parent`` at ``index`` + + :param parent: The element whose character data should be extended + :param index: The index of the child element the text should precede + :param text: The text to append + """ + if index == 0: + parent.text = (parent.text or "") + text + else: + previous = parent[index - 1] + previous.tail = (previous.tail or "") + text + + +def _set_inner_html(element: HtmlElement, inner_html: str) -> None: + """ + Set the content of ``element`` to the given html string + + :param element: The element whose content should be set + :param inner_html: The html to insert into the element + """ + try: + parsed = fromstring(f"
{inner_html}
") + except LxmlError: + logger.debug("Failed to parse inner html of a link: %r", inner_html) + element.text = inner_html + return + element.text = parsed.text + for child in parsed: + element.append(child) + + +def _set_link_title(link: HtmlElement, link_title: HtmlElement | str) -> None: + """ + Set the content of ``link`` to the link title of its target + + :param link: The link whose content should be set + :param link_title: The :attr:`~integreat_cms.cms.models.abstract_content_translation.AbstractContentTranslation.link_title` + of the target, which is either an escaped string or an element with a tail + """ + if isinstance(link_title, str): + _set_inner_html(link, link_title) + else: + # The link title is cached on the translation, so it must not be re-parented + link.append(deepcopy(link_title)) + + +def _render_link( + page_id: str | int | None, + context: dict[str, Any] | None, + text: str | None = None, + inner_html: str | None = None, +) -> HtmlElement | None: + """ + Render the link a shortcode represents while the content is edited + + :param page_id: The id of the referenced :class:`~integreat_cms.cms.models.pages.page.Page` + :param context: The context the shortcode is expanded in + :param text: The link text of the atomic shortcode, if it has one + :param inner_html: The content of the block scoped shortcode, if it is used + :return: The link, or ``None`` if the reference cannot be resolved + """ + language_slug = (context or {}).get("language_slug") + if not (translation := get_editor_page_translation(page_id, language_slug)): + return None + + link = Element("a") + link.set("href", translation.full_url) + if inner_html is not None: + _set_inner_html(link, inner_html) + elif text is None: + # Without an explicit link text, the link follows the title of its target + link.set(AUTO_UPDATE_ATTRIBUTE, "true") + _set_link_title(link, translation.link_title) + else: + link.text = text + return link + + +def _unparse(keyword: str, pargs: list[str], kwargs: dict[str, str]) -> str: + """ + Rebuild the source representation of a shortcode tag + + :param keyword: The keyword of the shortcode + :param pargs: The positional arguments of the shortcode + :param kwargs: The keyword arguments of the shortcode + :return: The shortcode tag + """ + # Everything but the id of the target is quoted, so that no quotes get lost while a + # shortcode which cannot be resolved is kept verbatim + arguments = [ + parg if index == 0 and parg and not any(map(str.isspace, parg)) else f'"{parg}"' + for index, parg in enumerate(pargs) + ] + arguments += [f'{key}="{value}"' for key, value in kwargs.items()] + return f"[{' '.join([keyword, *arguments])}]" + + +def _expand_page( + pargs: list[str], + kwargs: dict[str, str], + context: dict[str, Any] | None, +) -> str: + """ + Expand the atomic ``page`` shortcode into a link + + :param pargs: The positional arguments of the shortcode + :param kwargs: The keyword arguments of the shortcode + :param context: The context the shortcode is expanded in + :return: The link, or the shortcode itself if it cannot be resolved + """ + text = pargs[1] if len(pargs) > 1 else None + if (link := _render_link(pargs[0] if pargs else None, context, text=text)) is None: + return _unparse(PAGE_KEYWORD, pargs, kwargs) + return tostring(link, encoding="unicode", with_tail=False) + + +def _expand_page_link( + pargs: list[str], + kwargs: dict[str, str], + context: dict[str, Any] | None, + content: str = "", +) -> str: + """ + Expand the block scoped ``page_link`` shortcode into a link around its content + + :param pargs: The positional arguments of the shortcode + :param kwargs: The keyword arguments of the shortcode + :param context: The context the shortcode is expanded in + :param content: The content enclosed by the shortcode + :return: The link, or the shortcode itself if it cannot be resolved + """ + if ( + link := _render_link(pargs[0] if pargs else None, context, inner_html=content) + ) is None: + return ( + _unparse(PAGE_LINK_KEYWORD, pargs, kwargs) + + content + + f"[{PAGE_LINK_END_KEYWORD}]" + ) + return tostring(link, encoding="unicode", with_tail=False) + + +#: Parser which only knows the link shortcodes and passes everything else through unchanged +_link_parser = shortcodes.Parser( + start="[", + end="]", + esc="\\", + inherit_globals=False, + ignore_unknown=True, +) +_link_parser.register(_expand_page, PAGE_KEYWORD) +_link_parser.register(_expand_page_link, PAGE_LINK_KEYWORD, PAGE_LINK_END_KEYWORD) diff --git a/integreat_cms/cms/utils/shortcodes/__init__.py b/integreat_cms/cms/utils/shortcodes/__init__.py index 2f91747633..0cb47f0f34 100644 --- a/integreat_cms/cms/utils/shortcodes/__init__.py +++ b/integreat_cms/cms/utils/shortcodes/__init__.py @@ -2,15 +2,20 @@ This module contains implementations for the shortcodes content filters """ +from __future__ import annotations + import logging -from typing import Any +from typing import Any, TYPE_CHECKING import shortcodes from django import template from django.template.defaultfilters import stringfilter from .contact import contact -from .page import page +from .page import page, page_link + +if TYPE_CHECKING: + from ...models.abstract_content_translation import AbstractContentTranslation logger = logging.getLogger(__name__) @@ -36,3 +41,20 @@ def expand_shortcodes(content: str, context: dict[str, Any] | None = None) -> st # We failed expanding the shortcodes, # the best way we can fail gracefully is to just return the original content return content + + +def expand_shortcodes_of(translation: AbstractContentTranslation) -> str: + """ + Expand all shortcodes in the content of a content translation + + :param translation: The translation whose content should be expanded + :return: The expanded content + """ + return expand_shortcodes( + translation.content, + context={ + "region_slug": translation.foreign_object.region.slug, + "language_slug": translation.language.slug, + "content_object": translation, + }, + ) diff --git a/integreat_cms/cms/utils/shortcodes/page.py b/integreat_cms/cms/utils/shortcodes/page.py index 49ad729da9..a286f2a925 100644 --- a/integreat_cms/cms/utils/shortcodes/page.py +++ b/integreat_cms/cms/utils/shortcodes/page.py @@ -1,13 +1,93 @@ +from copy import deepcopy from typing import Any from django.utils.translation import gettext_lazy as _ +from lxml.etree import LxmlError from lxml.html import Element, fromstring, tostring from ...models import Page, PageTranslation +from ..content_utils import hide_anchor_tag_around_single_image +from ..link_shortcode_utils import ( + PAGE_KEYWORD, + PAGE_LINK_END_KEYWORD, + PAGE_LINK_KEYWORD, +) from .utils import shortcode -@shortcode +def _get_public_translation( + page_id: str | None, + context: dict[str, Any] | None, +) -> PageTranslation: + """ + Get the public translation a page shortcode refers to + + :param page_id: The id of the :class:`~integreat_cms.cms.models.pages.page.Page` to which should be linked + :param context: The context the shortcode is expanded in + :raises ~integreat_cms.cms.models.pages.page.Page.DoesNotExist: If the page does not exist + :raises ~integreat_cms.cms.models.pages.page_translation.PageTranslation.DoesNotExist: If the + page has no public translation in the requested language + :return: The public translation which is linked to + """ + page = Page.objects.get(id=page_id) + translation = page.get_public_translation( + (context or {}).get("language_slug", page.region.default_language.slug), + ) + if translation is None: + raise PageTranslation.DoesNotExist + return translation + + +def _missing_link(inner_html: str) -> Element: + """ + Build the replacement for a page shortcode whose target cannot be resolved + + :param inner_html: The content of the link, if it has any + :return: The element to insert instead of the link + """ + TEXT_MISSING = _( + "MISSING LINK" + ) # Separate variable because gettext apparently does not find _() if it is in an f-string + try: + return fromstring(f"[{inner_html or TEXT_MISSING}]") + except LxmlError: + element = Element("i") + element.text = f"[{TEXT_MISSING}]" + return element + + +def _render_link(translation: PageTranslation, inner_html: str | None) -> Element: + """ + Build the link to the given translation + + :param translation: The translation which is linked to + :param inner_html: The content of the link, or ``None`` to use the link title of the target + :return: The link element + """ + if inner_html is None: + element = Element("a") + link_title = translation.link_title + if isinstance(link_title, str): + # LXML needs a single root element, so we're doing this in a roundabout way + root = fromstring(f"{link_title}") + element.text = root.text + for child in root: + element.append(child) + else: + # The link title is cached on the translation, so it must not be re-parented + element.append(deepcopy(link_title)) + else: + try: + element = fromstring(f"{inner_html}") + except LxmlError: + element = Element("a") + element.text = inner_html + element.attrib["href"] = translation.get_absolute_url() + hide_anchor_tag_around_single_image(element) + return element + + +@shortcode(PAGE_KEYWORD) def page( pargs: list[str], kwargs: dict[str, str], # noqa: ARG001 @@ -25,6 +105,9 @@ def page( If the target page has an icon set and the shortcode has no ``link_text``, the icon will be included as an ``<ìmg>`` before the page title. + If the link should wrap html instead of plain text, use + :func:`~integreat_cms.cms.utils.shortcodes.page.page_link` instead. + .. list-table:: Examples :widths: 30 70 :header-rows: 0 @@ -39,27 +122,45 @@ def page( page_id = pargs[0] if pargs else None text = pargs[1] if len(pargs) > 1 else None try: - page = Page.objects.get(id=page_id) - translation = page.get_public_translation( - (context or {}).get("language_slug", page.region.default_language.slug) - ) - if translation is None: - raise PageTranslation.DoesNotExist # noqa: TRY301 # But… I want the two lines handling this to not be duplicated + translation = _get_public_translation(page_id, context) except (Page.DoesNotExist, PageTranslation.DoesNotExist): - element = Element("i") - TEXT_MISSING = _( - "MISSING LINK" - ) # Separate variable because gettext apparently does not find _() if it is in an f-string - element.text = f"[{text or TEXT_MISSING}]" + element = _missing_link(text or "") else: - element = Element("a") - if text is None: - # LXML needs a single root element, so we're doing this in a roundabout way - root = fromstring(f"{translation.link_title}") - element.text = root.text - for child in root: - element.append(child) - else: - element.text = text or "" - element.attrib["href"] = translation.get_absolute_url() + element = _render_link(translation, text) + return tostring(element).decode("utf-8") + + +@shortcode(PAGE_LINK_KEYWORD, PAGE_LINK_END_KEYWORD) +def page_link( + pargs: list[str], + kwargs: dict[str, str], # noqa: ARG001 + context: dict[str, Any] | None, + content: str = "", +) -> str: + """ + Shortcode to wrap its content in an internal link to a :class:`~integreat_cms.cms.models.pages.page.Page`. + + This is the block scoped counterpart of :func:`~integreat_cms.cms.utils.shortcodes.page.page`, + which is used whenever the content of the link is not plain text, for example a linked image. + + Positional arguments: + + * ``page_id`` – The id of the :class:`~integreat_cms.cms.models.pages.page.Page` to which should be linked + + .. list-table:: Examples + :widths: 45 55 + :header-rows: 0 + + * - ``[page_link 1]hier[/page_link]`` + - ``hier`` + * - ``[page_link 999999]hier[/page_link]`` + - ``[hier]`` + """ + page_id = pargs[0] if pargs else None + try: + translation = _get_public_translation(page_id, context) + except (Page.DoesNotExist, PageTranslation.DoesNotExist): + element = _missing_link(content) + else: + element = _render_link(translation, content) return tostring(element).decode("utf-8") diff --git a/integreat_cms/cms/utils/shortcodes/utils.py b/integreat_cms/cms/utils/shortcodes/utils.py index 569cae2905..91d14f7a61 100644 --- a/integreat_cms/cms/utils/shortcodes/utils.py +++ b/integreat_cms/cms/utils/shortcodes/utils.py @@ -1,10 +1,24 @@ from collections.abc import Callable +from typing import overload, ParamSpec, TypeVar import shortcodes +R = TypeVar("R") +P = ParamSpec("P") -def shortcode[**P, R]( - tag: Callable[P, R] | str | None, endtag: str | None = None + +@overload +def shortcode(tag: Callable[P, R]) -> Callable[P, R]: ... + + +@overload +def shortcode( + tag: str | None = None, endtag: str | None = None +) -> Callable[[Callable[P, R]], Callable[P, R]]: ... + + +def shortcode( + tag: Callable[P, R] | str | None = None, endtag: str | None = None ) -> Callable[[Callable[P, R]], Callable[P, R]] | Callable[P, R]: """ Decorator to register a function as a shortcode diff --git a/integreat_cms/cms/views/pages/page_actions.py b/integreat_cms/cms/views/pages/page_actions.py index 65a89fae09..29b7d3f4cf 100644 --- a/integreat_cms/cms/views/pages/page_actions.py +++ b/integreat_cms/cms/views/pages/page_actions.py @@ -185,9 +185,11 @@ def preview_page_ajax( return JsonResponse( data={ "title": page_translation.title, - "page_translation": page_translation.content, + "page_translation": page_translation.content_with_expanded_links, "mirrored_translation": ( - mirrored_translation.content if mirrored_translation else "" + mirrored_translation.content_with_expanded_links + if mirrored_translation + else "" ), "mirrored_page_first": page.mirrored_page_first, "right_to_left": ( @@ -222,7 +224,9 @@ def get_page_content_ajax( region = Region.objects.filter(slug=region_slug).first() page = get_object_or_404(region.pages, id=page_id) if page_translation := page.get_translation(language_slug): - return JsonResponse(data={"content": page_translation.content}) + return JsonResponse( + data={"content": page_translation.content_with_expanded_links} + ) raise Http404("Translation of the given page could not be found") diff --git a/integreat_cms/core/signals/hix_signals.py b/integreat_cms/core/signals/hix_signals.py index dab00fbf5e..57eb70c508 100644 --- a/integreat_cms/core/signals/hix_signals.py +++ b/integreat_cms/core/signals/hix_signals.py @@ -58,7 +58,8 @@ def page_translation_save_handler(instance: PageTranslation, **kwargs: Any) -> N instance.hix_feedback = latest_version.hix_feedback return - if data := lookup_hix_score(instance.content): + # Shortcodes would be scored as unreadable gibberish, so use the expanded content + if data := lookup_hix_score(instance.content_with_expanded_links): logger.debug("Storing hix score %s for %r", data["score"], instance) instance.hix_score = data["score"] diff --git a/integreat_cms/core/utils/machine_translation_api_client.py b/integreat_cms/core/utils/machine_translation_api_client.py index d4b5c7033b..bfb7fc9a32 100644 --- a/integreat_cms/core/utils/machine_translation_api_client.py +++ b/integreat_cms/core/utils/machine_translation_api_client.py @@ -358,6 +358,22 @@ def prepare_content_objects(self) -> list[TranslationContext]: and not (attr == "title" and skip_title) ] + # Machine translation providers must never see shortcodes, because they + # would happily translate them into something we cannot resolve anymore. + # Whatever comes back is collapsed into shortcodes again on save. + # Imported here because this module is loaded before the models are ready + from ...cms.utils.link_shortcode_utils import expand_link_shortcodes + + ctx.translatable_attributes = [ + ( + attr, + expand_link_shortcodes(value, self.source_language.slug) + if attr == "content" + else value, + ) + for attr, value in ctx.translatable_attributes + ] + ctx.word_count = word_count( ctx.translatable_attributes, ) diff --git a/tests/cms/utils/test_link_shortcode_utils.py b/tests/cms/utils/test_link_shortcode_utils.py new file mode 100644 index 0000000000..c6f61fe285 --- /dev/null +++ b/tests/cms/utils/test_link_shortcode_utils.py @@ -0,0 +1,361 @@ +""" +Tests for the conversion between internal links and their shortcode representation +""" + +from __future__ import annotations + +import pytest +from lxml.html import fromstring, tostring + +from integreat_cms.cms.constants import status +from integreat_cms.cms.models import PageTranslation +from integreat_cms.cms.utils.content_utils import clean_content +from integreat_cms.cms.utils.link_shortcode_utils import ( + collapse_links_to_shortcodes, + expand_link_shortcodes, +) + +#: The full url of the German translation of page 1 in the Augsburg region +WILLKOMMEN_URL = "https://integreat.app/augsburg/de/willkommen/" + +#: The full url of the German translation of page 3, a child of page 1 +UBER_DIE_APP_URL = ( + "https://integreat.app/augsburg/de/willkommen/uber-die-app-integreat-augsburg/" +) + + +def unpublish_page_3() -> None: + """ + Turn all German translations of page 3 into drafts, + so that the page has no public translation in German anymore + """ + PageTranslation.objects.filter(page_id=3, language__slug="de").update( + status=status.DRAFT, + ) + + +def collapse(content: str) -> str: + """ + Run :func:`~integreat_cms.cms.utils.link_shortcode_utils.collapse_links_to_shortcodes` + on a html string and return the result as a html string again + """ + element = fromstring(content) + collapse_links_to_shortcodes(element) + return tostring(element, encoding="unicode", with_tail=False) + + +@pytest.mark.django_db +def test_expand_page_shortcode_without_text(load_test_data: None) -> None: + """ + A ``[page]`` shortcode without link text becomes an auto updating link + """ + assert expand_link_shortcodes("

[page 1]

", "de") == ( + f'

Willkommen

' + ) + + +@pytest.mark.django_db +def test_expand_page_shortcode_with_text(load_test_data: None) -> None: + """ + A ``[page]`` shortcode with link text becomes a plain link + """ + assert expand_link_shortcodes('

[page 1 "hier"]

', "de") == ( + f'

hier

' + ) + + +@pytest.mark.django_db +def test_expand_page_shortcode_uses_requested_language(load_test_data: None) -> None: + """ + The shortcode is expanded to the url of the translation in the requested language + """ + assert expand_link_shortcodes("

[page 1]

", "en") == ( + '

Welcome

' + ) + + +@pytest.mark.django_db +def test_expand_page_link_shortcode(load_test_data: None) -> None: + """ + A ``[page_link]`` block shortcode wraps its content in a link + """ + assert ( + expand_link_shortcodes( + '

[page_link 1][/page_link]

', "de" + ) + == f'

' + ) + + +@pytest.mark.django_db +def test_expand_unresolvable_shortcode_is_kept_verbatim(load_test_data: None) -> None: + """ + Shortcodes which cannot be resolved must survive the round trip untouched + instead of silently vanishing from the content + """ + assert ( + expand_link_shortcodes("

[page 999999]

", "de") == "

[page 999999]

" + ) + assert ( + expand_link_shortcodes('

[page 999999 "hier"]

', "de") + == '

[page 999999 "hier"]

' + ) + assert ( + expand_link_shortcodes("

[page_link 999999]x[/page_link]

", "de") + == "

[page_link 999999]x[/page_link]

" + ) + + +@pytest.mark.django_db +def test_expand_leaves_other_shortcodes_alone(load_test_data: None) -> None: + """ + Only link shortcodes are expanded, everything else is passed through + """ + assert ( + expand_link_shortcodes("

[contact 1 email]

", "de") + == "

[contact 1 email]

" + ) + + +@pytest.mark.django_db +def test_expand_page_shortcode_to_draft_page(load_test_data: None) -> None: + """ + Editors may link to pages which have no public translation yet, + so those shortcodes must be expanded as well + """ + unpublish_page_3() + assert expand_link_shortcodes("

[page 3]

", "de") == ( + f'

' + "Über die App Integreat Augsburg

" + ) + + +@pytest.mark.django_db +def test_collapse_auto_updating_link(load_test_data: None) -> None: + """ + An auto updating link collapses to a ``[page]`` shortcode without link text + """ + assert ( + collapse( + f'

Willkommen

' + ) + == "

[page 1]

" + ) + + +@pytest.mark.django_db +def test_collapse_link_with_custom_text(load_test_data: None) -> None: + """ + A link with custom text collapses to a ``[page]`` shortcode with link text + """ + assert ( + collapse(f'

vor hier nach

') + == '

vor [page 1 "hier"] nach

' + ) + + +@pytest.mark.django_db +def test_collapse_link_with_markup(load_test_data: None) -> None: + """ + A link containing markup collapses to the ``[page_link]`` block shortcode, + which preserves the inner html + """ + assert ( + collapse(f'

fette Schrift

') + == "

[page_link 1]fette Schrift[/page_link]

" + ) + + +@pytest.mark.django_db +def test_collapse_link_with_quote_in_text(load_test_data: None) -> None: + """ + Link texts which cannot be expressed as a shortcode argument + fall back to the ``[page_link]`` block shortcode + """ + assert ( + collapse(f'

"hier"

') + == '

[page_link 1]"hier"[/page_link]

' + ) + + +@pytest.mark.django_db +def test_collapse_short_url(load_test_data: None) -> None: + """ + Short urls to pages are collapsed as well + """ + assert ( + collapse('

hier

') + == '

[page 1 "hier"]

' + ) + + +@pytest.mark.django_db +def test_collapse_leaves_external_links_alone(load_test_data: None) -> None: + """ + Only links to pages are collapsed + """ + content = '

extern

' + assert collapse(content) == content + + +@pytest.mark.django_db +def test_collapse_leaves_other_internal_links_alone(load_test_data: None) -> None: + """ + Links to events, locations and the imprint are not collapsed yet + """ + content = ( + '

' + "Veranstaltung

" + ) + assert collapse(content) == content + + +@pytest.mark.django_db +def test_collapse_link_to_draft_page(load_test_data: None) -> None: + """ + Links to pages without a public translation are collapsed too, + so that they do not end up in the link index either + """ + unpublish_page_3() + assert ( + collapse(f'

hier

') + == '

[page 3 "hier"]

' + ) + + +@pytest.mark.django_db +@pytest.mark.parametrize( + "shortcodes", + [ + "

[page 1]

", + '

[page 1 "hier"]

', + "

[page_link 1]fett[/page_link]

", + '

vor [page 1] mitte [page 1 "da"] nach

', + "", + ], +) +def test_round_trip(load_test_data: None, shortcodes: str) -> None: + """ + Expanding and collapsing again must not change the stored content + """ + assert collapse(expand_link_shortcodes(shortcodes, "de")) == shortcodes + + +@pytest.mark.django_db +def test_clean_content_collapses_internal_links(load_test_data: None) -> None: + """ + Content saved through :func:`~integreat_cms.cms.utils.content_utils.clean_content` + must never contain urls to internal pages + """ + cleaned = clean_content( + f'

hier und ' + 'extern

', + "de", + 1, + ) + assert WILLKOMMEN_URL not in cleaned + assert '[page 1 "hier"]' in cleaned + assert 'href="https://example.com/"' in cleaned + + +@pytest.mark.django_db +def test_collapse_link_with_ampersand_in_text(load_test_data: None) -> None: + """ + Link texts containing characters which are html escaped when the content is serialized + also fall back to the block scoped shortcode, so that no second layer of escaping + piles up on every save + """ + assert ( + collapse(f'

Recht & Ordnung

') + == "

[page_link 1]Recht & Ordnung[/page_link]

" + ) + + +@pytest.mark.django_db +def test_collapse_linked_image(load_test_data: None) -> None: + """ + A linked image keeps its image element and is wrapped in the block scoped shortcode + """ + assert ( + collapse( + f'

' + ) + == '

[page_link 1][/page_link]

' + ) + + +@pytest.mark.django_db +@pytest.mark.parametrize( + "shortcodes", + [ + "

[page_link 1]Recht & Ordnung[/page_link]

", + '

[page_link 1][/page_link]

', + ], +) +def test_round_trip_of_escaped_content( + load_test_data: None, + shortcodes: str, +) -> None: + """ + Content which needs html escaping must survive an arbitrary number of save cycles + """ + content = shortcodes + for _iteration in range(3): + content = collapse(expand_link_shortcodes(content, "de")) + assert content == shortcodes + + +@pytest.mark.django_db +def test_content_form_expands_shortcodes_for_the_editor(load_test_data: None) -> None: + """ + The content which is put into the editor contains links instead of shortcodes + """ + from integreat_cms.cms.forms import PageTranslationForm + + translation = get_latest_german_translation(page_id=2) + PageTranslation.objects.filter(pk=translation.pk).update( + content='

[page 1 "hier"]

', + ) + translation.refresh_from_db() + + form = PageTranslationForm(instance=translation) + assert form.initial["content"] == f'

hier

' + + +@pytest.mark.django_db +def test_content_form_collapses_links_on_save(load_test_data: None) -> None: + """ + What the editor submits is stored as shortcodes, so that no url to internal content + is ever handed to ``linkcheck`` + """ + from integreat_cms.cms.forms import PageTranslationForm + + translation = get_latest_german_translation(page_id=2) + form = PageTranslationForm( + data={ + "title": translation.title, + "slug": translation.slug, + "status": translation.status, + "content": f'

hier

', + }, + instance=translation, + ) + assert form.is_valid(), form.errors + assert form.cleaned_data["content"] == '

[page 1 "hier"]

' + assert "integreat.app" not in form.cleaned_data["content"] + + +def get_latest_german_translation(page_id: int) -> PageTranslation: + """ + Get the latest German translation of the given page + + :param page_id: The id of the page + :return: The translation + """ + return ( + PageTranslation.objects.filter(page_id=page_id, language__slug="de") + .order_by("-version") + .first() + ) diff --git a/tests/cms/utils/test_shortcodes.py b/tests/cms/utils/test_shortcodes.py new file mode 100644 index 0000000000..fc2eaf3983 --- /dev/null +++ b/tests/cms/utils/test_shortcodes.py @@ -0,0 +1,103 @@ +""" +Tests for the shortcodes which are expanded when content is delivered +""" + +from __future__ import annotations + +import pytest +from django.utils import translation + +from integreat_cms.cms.utils.shortcodes import expand_shortcodes + +#: The context the shortcodes are expanded in +DE = {"language_slug": "de"} + + +@pytest.mark.django_db +def test_page_shortcode_without_text(load_test_data: None) -> None: + """ + A ``[page]`` shortcode without link text uses the title of its target + """ + assert ( + expand_shortcodes("

[page 1]

", DE) + == '

Willkommen

' + ) + + +@pytest.mark.django_db +def test_page_shortcode_with_text(load_test_data: None) -> None: + """ + A ``[page]`` shortcode with link text uses that text + """ + assert ( + expand_shortcodes('

[page 1 "hier"]

', DE) + == '

hier

' + ) + + +@pytest.mark.django_db +def test_page_shortcode_missing_target(load_test_data: None) -> None: + """ + A ``[page]`` shortcode whose target does not exist is marked as a missing link + """ + with translation.override("en"): + assert ( + expand_shortcodes("

[page 999999]

", DE) + == "

[MISSING LINK]

" + ) + assert ( + expand_shortcodes('

[page 999999 "hier"]

', DE) == "

[hier]

" + ) + + +@pytest.mark.django_db +def test_page_link_shortcode(load_test_data: None) -> None: + """ + A ``[page_link]`` shortcode wraps its content in a link to its target + """ + assert ( + expand_shortcodes("

[page_link 1]hier lang[/page_link]

", DE) + == '

hier lang

' + ) + + +@pytest.mark.django_db +def test_page_link_shortcode_missing_target(load_test_data: None) -> None: + """ + A ``[page_link]`` shortcode whose target does not exist keeps its content + """ + assert ( + expand_shortcodes("

[page_link 999999]hier[/page_link]

", DE) + == "

[hier]

" + ) + + +@pytest.mark.django_db +def test_page_link_shortcode_hides_linked_image_without_alt_text( + load_test_data: None, +) -> None: + """ + A link which only contains an image without alt text has to be hidden from screen + readers and the tab key, just like the same link would be if it was part of the content + """ + assert expand_shortcodes( + '

[page_link 1][/page_link]

', DE + ) == ( + '

' + ) + + +@pytest.mark.django_db +def test_page_link_shortcode_keeps_linked_image_with_alt_text( + load_test_data: None, +) -> None: + """ + A link around an image which has an alt text stays reachable + """ + result = expand_shortcodes( + '

[page_link 1]Willkommen[/page_link]

', + DE, + ) + assert "aria-hidden" not in result + assert "tabindex" not in result From c6d879462a345e00a9eda09990d92d1f33ed7b7b Mon Sep 17 00:00:00 2001 From: Jonas Buchholz Date: Mon, 10 Aug 2026 16:12:19 +0000 Subject: [PATCH 2/8] make content_ith_expanded_links no cached property to avoid one-off for form read --- .../models/abstract_content_translation.py | 8 +++- integreat_cms/cms/utils/shortcodes/utils.py | 11 ++--- tests/cms/utils/test_link_shortcode_utils.py | 45 +++++++++++++++++++ 3 files changed, 56 insertions(+), 8 deletions(-) diff --git a/integreat_cms/cms/models/abstract_content_translation.py b/integreat_cms/cms/models/abstract_content_translation.py index 15aa43457d..c17a968936 100644 --- a/integreat_cms/cms/models/abstract_content_translation.py +++ b/integreat_cms/cms/models/abstract_content_translation.py @@ -539,7 +539,7 @@ def path(self) -> str: """ return str(self) - @cached_property + @property def content_with_expanded_links(self) -> str: """ The content as it should be presented to users of the CMS, which means with all @@ -549,6 +549,12 @@ def content_with_expanded_links(self) -> str: :class:`~integreat_cms.cms.forms.custom_content_model_form.CustomContentModelForm` is collapsed into shortcodes again. + This deliberately is not a :class:`~django.utils.functional.cached_property`: + :class:`~integreat_cms.cms.forms.custom_content_model_form.CustomContentModelForm` + reads it while initializing the form and then assigns the submitted content to the + very same instance, so a cached value would be the *previous* content by the time + the instance is saved. + :return: The content with expanded links """ # Imported here because the utils import the models diff --git a/integreat_cms/cms/utils/shortcodes/utils.py b/integreat_cms/cms/utils/shortcodes/utils.py index 91d14f7a61..7cf06efa41 100644 --- a/integreat_cms/cms/utils/shortcodes/utils.py +++ b/integreat_cms/cms/utils/shortcodes/utils.py @@ -1,23 +1,20 @@ from collections.abc import Callable -from typing import overload, ParamSpec, TypeVar +from typing import overload import shortcodes -R = TypeVar("R") -P = ParamSpec("P") - @overload -def shortcode(tag: Callable[P, R]) -> Callable[P, R]: ... +def shortcode[**P, R](tag: Callable[P, R]) -> Callable[P, R]: ... @overload -def shortcode( +def shortcode[**P, R]( tag: str | None = None, endtag: str | None = None ) -> Callable[[Callable[P, R]], Callable[P, R]]: ... -def shortcode( +def shortcode[**P, R]( tag: Callable[P, R] | str | None = None, endtag: str | None = None ) -> Callable[[Callable[P, R]], Callable[P, R]] | Callable[P, R]: """ diff --git a/tests/cms/utils/test_link_shortcode_utils.py b/tests/cms/utils/test_link_shortcode_utils.py index c6f61fe285..c3719b9133 100644 --- a/tests/cms/utils/test_link_shortcode_utils.py +++ b/tests/cms/utils/test_link_shortcode_utils.py @@ -347,6 +347,51 @@ def test_content_form_collapses_links_on_save(load_test_data: None) -> None: assert "integreat.app" not in form.cleaned_data["content"] +@pytest.mark.django_db +def test_expanded_content_is_not_cached(load_test_data: None) -> None: + """ + ``content_with_expanded_links`` must follow later changes of the content. + + It must not be a cached property: the content form reads it while initializing and then + assigns the submitted content to the very same instance, so anything reading it during + ``pre_save`` (the HIX score calculation does) would otherwise see the previous content. + """ + translation = get_latest_german_translation(page_id=2) + translation.content = '

[page 1 "hier"]

' + assert ( + translation.content_with_expanded_links + == f'

hier

' + ) + + translation.content = "

Neuer Inhalt

" + assert translation.content_with_expanded_links == "

Neuer Inhalt

" + + +@pytest.mark.django_db +def test_content_form_does_not_freeze_expanded_content(load_test_data: None) -> None: + """ + After the content form has been validated, the expanded content of its instance must + reflect what was submitted, not what was in the database when the form was built + """ + from integreat_cms.cms.forms import PageTranslationForm + + translation = get_latest_german_translation(page_id=2) + form = PageTranslationForm( + data={ + "title": translation.title, + "slug": translation.slug, + "status": translation.status, + "content": "

Neuer Inhalt

", + }, + instance=translation, + ) + # Building the form reads the expanded content to populate the editor + assert form.initial["content"] + assert form.is_valid(), form.errors + + assert form.instance.content_with_expanded_links == "

Neuer Inhalt

" + + def get_latest_german_translation(page_id: int) -> PageTranslation: """ Get the latest German translation of the given page From 12a75873c724f83d0a5d9cc16756f6de769b68ca Mon Sep 17 00:00:00 2001 From: Jonas Buchholz Date: Mon, 10 Aug 2026 16:28:39 +0000 Subject: [PATCH 3/8] fix build-documentation --- integreat_cms/cms/utils/link_shortcode_utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/integreat_cms/cms/utils/link_shortcode_utils.py b/integreat_cms/cms/utils/link_shortcode_utils.py index 56d0667e80..a3d7ecdc38 100644 --- a/integreat_cms/cms/utils/link_shortcode_utils.py +++ b/integreat_cms/cms/utils/link_shortcode_utils.py @@ -3,7 +3,7 @@ shortcodes which represent them. Internal references are stored as shortcodes so that they are only resolved when the content -is delivered (see :doc:`ADR 0001 <../../../ADR/0001-compose-referenced-objects-into-content-dynamically-shortcodes>`). +is delivered (see ``ADR/0001-compose-referenced-objects-into-content-dynamically-shortcodes.md``). This means the link index kept by our ``linkcheck`` dependency never has to know about them. Editors should not have to care about that, so the shortcodes are expanded into ordinary From 2a8583fbe598897ac1ffad604e1615d7fc30bbb2 Mon Sep 17 00:00:00 2001 From: Jonas Buchholz Date: Wed, 12 Aug 2026 06:27:00 +0000 Subject: [PATCH 4/8] refactor to content_for_cms and content_for_delivery --- integreat_cms/api/v3/events.py | 3 +- integreat_cms/api/v3/imprint.py | 3 +- integreat_cms/api/v3/locations.py | 3 +- integreat_cms/api/v3/pages.py | 33 ++------- integreat_cms/api/v3/social_media_headers.py | 7 +- .../cms/forms/custom_content_model_form.py | 2 +- .../models/abstract_content_translation.py | 40 +++++++++- .../cms/templates/content_versions.html | 2 +- .../pages/_page_xliff_import_diff.html | 4 +- .../cms/templates/pages/page_pdf.html | 2 +- .../cms/templates/pois/poi_view.html | 2 +- .../cms/utils/shortcodes/__init__.py | 22 +----- integreat_cms/cms/views/pages/page_actions.py | 10 +-- integreat_cms/core/signals/hix_signals.py | 2 +- tests/cms/utils/test_link_shortcode_utils.py | 11 +-- tests/cms/utils/test_shortcodes.py | 73 +++++++++++++++++++ 16 files changed, 141 insertions(+), 78 deletions(-) diff --git a/integreat_cms/api/v3/events.py b/integreat_cms/api/v3/events.py index 34fa4914a9..f9ad042b8c 100644 --- a/integreat_cms/api/v3/events.py +++ b/integreat_cms/api/v3/events.py @@ -12,7 +12,6 @@ from django.utils import timezone from django.utils.html import strip_tags -from ...cms.utils.shortcodes import expand_shortcodes_of from ..decorators import json_response from .locations import transform_poi @@ -83,7 +82,7 @@ def transform_event_translation( else f"{event_translation.slug}${recurrence_date}" ) absolute_url = event_translation.url_prefix + slug + "/" - content = expand_shortcodes_of(event_translation) + content = event_translation.content_for_delivery() return { "id": event_translation.id, "url": settings.BASE_URL + absolute_url, diff --git a/integreat_cms/api/v3/imprint.py b/integreat_cms/api/v3/imprint.py index 3455f4914d..88f51d93a3 100644 --- a/integreat_cms/api/v3/imprint.py +++ b/integreat_cms/api/v3/imprint.py @@ -19,7 +19,6 @@ from ...cms.models.pages.imprint_page_translation import ImprintPageTranslation -from ...cms.utils.shortcodes import expand_shortcodes_of from ..decorators import json_response logger = logging.getLogger(__name__) @@ -33,7 +32,7 @@ def transform_imprint(imprint_translation: ImprintPageTranslation) -> dict[str, :return: data necessary for API """ absolute_url = imprint_translation.get_absolute_url() - content = expand_shortcodes_of(imprint_translation) + content = imprint_translation.content_for_delivery() return { "id": imprint_translation.id, "url": settings.BASE_URL + absolute_url, diff --git a/integreat_cms/api/v3/locations.py b/integreat_cms/api/v3/locations.py index 75d517c4c1..9cad662302 100644 --- a/integreat_cms/api/v3/locations.py +++ b/integreat_cms/api/v3/locations.py @@ -15,7 +15,6 @@ from ...cms.constants import status from ...cms.models import Contact, POICategoryTranslation from ...cms.models.pois.poi import get_default_opening_hours -from ...cms.utils.shortcodes import expand_shortcodes_of from ...core.utils.strtobool import strtobool from ..decorators import json_response from .location_categories import transform_location_category @@ -117,7 +116,7 @@ def transform_poi_translation( :return: Data for the APIv3 locations endpoint. """ poi = poi_translation.poi - content = expand_shortcodes_of(poi_translation) + content = poi_translation.content_for_delivery() contacts = Contact.objects.filter(location=poi).all() diff --git a/integreat_cms/api/v3/pages.py b/integreat_cms/api/v3/pages.py index 578f39f93b..2dbce6cdce 100644 --- a/integreat_cms/api/v3/pages.py +++ b/integreat_cms/api/v3/pages.py @@ -21,7 +21,6 @@ from ...cms.forms import PageTranslationForm from ...cms.models import Page, PageTranslation -from ...cms.utils.shortcodes import expand_shortcodes from ..decorators import json_response, matomo_tracking from .offers import transform_offer @@ -38,13 +37,16 @@ def transform_page( page_translation: PageTranslation, page: Page | None = None, - context: dict[str, Any] | None = None, + request: HttpRequest | None = None, slug_history: list[str] | None = None, ) -> dict[str, Any]: """ Function to create a dict from a single page_translation Object. :param page_translation: single page translation object + :param page: the page the translation belongs to + :param request: the current request, passed to the shortcodes as context + :param slug_history: all slugs this translation has used :raises ~django.http.Http404: HTTP status 404 if a parent is archived :return: data necessary for API @@ -86,7 +88,7 @@ def transform_page( organization = page.organization absolute_url = page_translation.get_absolute_url() - content = expand_shortcodes(page_translation.combined_text, context=context) + content = page_translation.content_for_delivery(request=request) return { "id": page_translation.id, "url": settings.BASE_URL + absolute_url, @@ -167,12 +169,7 @@ def pages( transform_page( page_translation, page, - context={ - "region_slug": region_slug, - "language_slug": language_slug, - "content_object": page_translation, - "request": request, - }, + request=request, slug_history=slug_history.get(page.id, []), ) ) @@ -268,12 +265,7 @@ def single_page( transform_page( page_translation, page, - context={ - "region_slug": region_slug, - "language_slug": language_slug, - "content_object": page_translation, - "request": request, - }, + request=request, slug_history=list( dict.fromkeys( page_translation.all_versions.order_by("version").values_list( @@ -354,12 +346,7 @@ def children( transform_page( page_translation, page, - context={ - "region_slug": region_slug, - "language_slug": language_slug, - "content_object": page_translation, - "request": request, - }, + request=request, slug_history=slug_history.get(page.id, []), ) for page in pages.values() @@ -427,10 +414,6 @@ def get_public_ancestor_translations( transform_page( public_translation, ancestor, - context={ - "language_slug": language_slug, - "content_object": public_translation, - }, slug_history=slug_history.get(ancestor.id, []), ) ) diff --git a/integreat_cms/api/v3/social_media_headers.py b/integreat_cms/api/v3/social_media_headers.py index feff50d412..18df956b76 100644 --- a/integreat_cms/api/v3/social_media_headers.py +++ b/integreat_cms/api/v3/social_media_headers.py @@ -18,7 +18,6 @@ from ...cms.utils.internal_link_utils import ( get_public_translation_for_webapp_link_parts, ) -from ...cms.utils.shortcodes import expand_shortcodes_of from ...cms.utils.social_media_utils import ( get_excerpt, get_region_title, @@ -229,7 +228,7 @@ def page_social_media_headers( request=request, title=get_region_title(region, page_translation.title), language_code=language.bcp47_tag, - excerpt=get_excerpt(expand_shortcodes_of(page_translation)), + excerpt=get_excerpt(page_translation.content_for_delivery()), url=page_translation.full_url, ) @@ -271,7 +270,7 @@ def event_social_media_headers( request=request, title=get_region_title(region, event_translation.title), language_code=language.bcp47_tag, - excerpt=get_excerpt(expand_shortcodes_of(event_translation)), + excerpt=get_excerpt(event_translation.content_for_delivery()), url=event_translation.full_url, ) @@ -343,6 +342,6 @@ def location_social_media_headers( request=request, title=get_region_title(region, location_translation.title), language_code=language.bcp47_tag, - excerpt=get_excerpt(expand_shortcodes_of(location_translation)), + excerpt=get_excerpt(location_translation.content_for_delivery()), url=location_translation.full_url, ) diff --git a/integreat_cms/cms/forms/custom_content_model_form.py b/integreat_cms/cms/forms/custom_content_model_form.py index 026f719fc7..17970221db 100644 --- a/integreat_cms/cms/forms/custom_content_model_form.py +++ b/integreat_cms/cms/forms/custom_content_model_form.py @@ -59,7 +59,7 @@ def __init__(self, **kwargs: Any) -> None: # working with ordinary links, so expand them before they are put into the editor if "content" in self.fields and self.instance.content: with suppress(ObjectDoesNotExist): - self.initial["content"] = self.instance.content_with_expanded_links + self.initial["content"] = self.instance.content_for_cms def clean(self) -> dict[str, Any]: """ diff --git a/integreat_cms/cms/models/abstract_content_translation.py b/integreat_cms/cms/models/abstract_content_translation.py index c17a968936..599264353a 100644 --- a/integreat_cms/cms/models/abstract_content_translation.py +++ b/integreat_cms/cms/models/abstract_content_translation.py @@ -539,8 +539,21 @@ def path(self) -> str: """ return str(self) + @cached_property + def combined_text(self) -> str: + """ + The content this translation delivers, including any content embedded from elsewhere. + + Only :class:`~integreat_cms.cms.models.pages.page_translation.PageTranslation` embeds + anything (the translation of its mirrored page), so for every other content type this + is just the content itself. + + :return: The content to deliver, still containing shortcodes + """ + return self.content + @property - def content_with_expanded_links(self) -> str: + def content_for_cms(self) -> str: """ The content as it should be presented to users of the CMS, which means with all shortcodes referencing internal content expanded into ordinary links. @@ -562,6 +575,31 @@ def content_with_expanded_links(self) -> str: return expand_link_shortcodes(self.content, self.language.slug) + def content_for_delivery(self, **extra_context: Any) -> str: + r""" + The content as it should be delivered by the API, which means with every shortcode + expanded into the representation of the object it references. + + This is a method rather than a property because expanding a shortcode may need + context which cannot be derived from the translation, such as the current request. + + :param \**extra_context: Additional context for the shortcodes, which takes + precedence over the context derived from this translation + :return: The expanded content + """ + # Imported here because the utils import the models + from ..utils.shortcodes import expand_shortcodes + + return expand_shortcodes( + self.combined_text, + context={ + "region_slug": self.foreign_object.region.slug, + "language_slug": self.language.slug, + "content_object": self, + **extra_context, + }, + ) + @cached_property def hix_enabled(self) -> bool: """ diff --git a/integreat_cms/cms/templates/content_versions.html b/integreat_cms/cms/templates/content_versions.html index 2f508c7925..614c4b74b4 100644 --- a/integreat_cms/cms/templates/content_versions.html +++ b/integreat_cms/cms/templates/content_versions.html @@ -109,7 +109,7 @@

- {{ translation.content_with_expanded_links|safe }} + {{ translation.content_for_cms|safe }}
diff --git a/integreat_cms/cms/templates/pages/_page_xliff_import_diff.html b/integreat_cms/cms/templates/pages/_page_xliff_import_diff.html index aaf90a03e5..f506f59a95 100644 --- a/integreat_cms/cms/templates/pages/_page_xliff_import_diff.html +++ b/integreat_cms/cms/templates/pages/_page_xliff_import_diff.html @@ -78,13 +78,13 @@

{{ diff.existing.title }}

- {{ diff.existing.content_with_expanded_links|safe }} + {{ diff.existing.content_for_cms|safe }}
diff --git a/integreat_cms/cms/templates/pages/page_pdf.html b/integreat_cms/cms/templates/pages/page_pdf.html index 7f84398877..3d9ae37eea 100644 --- a/integreat_cms/cms/templates/pages/page_pdf.html +++ b/integreat_cms/cms/templates/pages/page_pdf.html @@ -115,7 +115,7 @@

{{ page_translation.title }}

{% if page.mirrored_page_first %} {{ page_translation.mirrored_translation_text|expand_links:language.slug|pdf_strip_fontstyles|pdf_truncate_links:80|safe }} {% endif %} - {{ page_translation.content_with_expanded_links|pdf_strip_fontstyles|pdf_truncate_links:80|safe }} + {{ page_translation.content_for_cms|pdf_strip_fontstyles|pdf_truncate_links:80|safe }} {% if not page.mirrored_page_first %} {{ page_translation.mirrored_translation_text|expand_links:language.slug|pdf_strip_fontstyles|pdf_truncate_links:80|safe }} {% endif %} diff --git a/integreat_cms/cms/templates/pois/poi_view.html b/integreat_cms/cms/templates/pois/poi_view.html index 688d3f660c..9fc0452566 100644 --- a/integreat_cms/cms/templates/pois/poi_view.html +++ b/integreat_cms/cms/templates/pois/poi_view.html @@ -5,6 +5,6 @@

{{ poi_translation.title }}

- {{ poi_translation.content_with_expanded_links|safe }} + {{ poi_translation.content_for_cms|safe }}
{% endblock raw_content %} diff --git a/integreat_cms/cms/utils/shortcodes/__init__.py b/integreat_cms/cms/utils/shortcodes/__init__.py index 0cb47f0f34..124984687d 100644 --- a/integreat_cms/cms/utils/shortcodes/__init__.py +++ b/integreat_cms/cms/utils/shortcodes/__init__.py @@ -5,7 +5,7 @@ from __future__ import annotations import logging -from typing import Any, TYPE_CHECKING +from typing import Any import shortcodes from django import template @@ -14,9 +14,6 @@ from .contact import contact from .page import page, page_link -if TYPE_CHECKING: - from ...models.abstract_content_translation import AbstractContentTranslation - logger = logging.getLogger(__name__) register = template.Library() @@ -41,20 +38,3 @@ def expand_shortcodes(content: str, context: dict[str, Any] | None = None) -> st # We failed expanding the shortcodes, # the best way we can fail gracefully is to just return the original content return content - - -def expand_shortcodes_of(translation: AbstractContentTranslation) -> str: - """ - Expand all shortcodes in the content of a content translation - - :param translation: The translation whose content should be expanded - :return: The expanded content - """ - return expand_shortcodes( - translation.content, - context={ - "region_slug": translation.foreign_object.region.slug, - "language_slug": translation.language.slug, - "content_object": translation, - }, - ) diff --git a/integreat_cms/cms/views/pages/page_actions.py b/integreat_cms/cms/views/pages/page_actions.py index 29b7d3f4cf..f7b67577fd 100644 --- a/integreat_cms/cms/views/pages/page_actions.py +++ b/integreat_cms/cms/views/pages/page_actions.py @@ -185,11 +185,9 @@ def preview_page_ajax( return JsonResponse( data={ "title": page_translation.title, - "page_translation": page_translation.content_with_expanded_links, + "page_translation": page_translation.content_for_cms, "mirrored_translation": ( - mirrored_translation.content_with_expanded_links - if mirrored_translation - else "" + mirrored_translation.content_for_cms if mirrored_translation else "" ), "mirrored_page_first": page.mirrored_page_first, "right_to_left": ( @@ -224,9 +222,7 @@ def get_page_content_ajax( region = Region.objects.filter(slug=region_slug).first() page = get_object_or_404(region.pages, id=page_id) if page_translation := page.get_translation(language_slug): - return JsonResponse( - data={"content": page_translation.content_with_expanded_links} - ) + return JsonResponse(data={"content": page_translation.content_for_cms}) raise Http404("Translation of the given page could not be found") diff --git a/integreat_cms/core/signals/hix_signals.py b/integreat_cms/core/signals/hix_signals.py index 57eb70c508..cb6a15434b 100644 --- a/integreat_cms/core/signals/hix_signals.py +++ b/integreat_cms/core/signals/hix_signals.py @@ -59,7 +59,7 @@ def page_translation_save_handler(instance: PageTranslation, **kwargs: Any) -> N return # Shortcodes would be scored as unreadable gibberish, so use the expanded content - if data := lookup_hix_score(instance.content_with_expanded_links): + if data := lookup_hix_score(instance.content_for_cms): logger.debug("Storing hix score %s for %r", data["score"], instance) instance.hix_score = data["score"] diff --git a/tests/cms/utils/test_link_shortcode_utils.py b/tests/cms/utils/test_link_shortcode_utils.py index c3719b9133..51c6a57331 100644 --- a/tests/cms/utils/test_link_shortcode_utils.py +++ b/tests/cms/utils/test_link_shortcode_utils.py @@ -350,7 +350,7 @@ def test_content_form_collapses_links_on_save(load_test_data: None) -> None: @pytest.mark.django_db def test_expanded_content_is_not_cached(load_test_data: None) -> None: """ - ``content_with_expanded_links`` must follow later changes of the content. + ``content_for_cms`` must follow later changes of the content. It must not be a cached property: the content form reads it while initializing and then assigns the submitted content to the very same instance, so anything reading it during @@ -358,13 +358,10 @@ def test_expanded_content_is_not_cached(load_test_data: None) -> None: """ translation = get_latest_german_translation(page_id=2) translation.content = '

[page 1 "hier"]

' - assert ( - translation.content_with_expanded_links - == f'

hier

' - ) + assert translation.content_for_cms == f'

hier

' translation.content = "

Neuer Inhalt

" - assert translation.content_with_expanded_links == "

Neuer Inhalt

" + assert translation.content_for_cms == "

Neuer Inhalt

" @pytest.mark.django_db @@ -389,7 +386,7 @@ def test_content_form_does_not_freeze_expanded_content(load_test_data: None) -> assert form.initial["content"] assert form.is_valid(), form.errors - assert form.instance.content_with_expanded_links == "

Neuer Inhalt

" + assert form.instance.content_for_cms == "

Neuer Inhalt

" def get_latest_german_translation(page_id: int) -> PageTranslation: diff --git a/tests/cms/utils/test_shortcodes.py b/tests/cms/utils/test_shortcodes.py index fc2eaf3983..f9aef1733d 100644 --- a/tests/cms/utils/test_shortcodes.py +++ b/tests/cms/utils/test_shortcodes.py @@ -101,3 +101,76 @@ def test_page_link_shortcode_keeps_linked_image_with_alt_text( ) assert "aria-hidden" not in result assert "tabindex" not in result + + +@pytest.mark.django_db +def test_content_for_delivery_expands_shortcodes(load_test_data: None) -> None: + """ + A translation expands its own shortcodes with the context derived from itself + """ + from integreat_cms.cms.models import PageTranslation + + translation = ( + PageTranslation.objects.filter(page_id=2, language__slug="de") + .order_by("-version") + .first() + ) + PageTranslation.objects.filter(pk=translation.pk).update( + content='

[page 1 "hier"]

', + ) + translation.refresh_from_db() + + assert translation.content_for_delivery() == ( + '

hier

' + ) + + +@pytest.mark.django_db +def test_content_for_delivery_includes_mirrored_content(load_test_data: None) -> None: + """ + Pages deliver the content of their mirrored page as well, so the shortcodes in there + have to be expanded too + """ + from integreat_cms.cms.models import Page, PageTranslation + + mirrored = Page.objects.get(id=1) + PageTranslation.objects.filter(page_id=1, language__slug="de").update( + content='

[page 1 "gespiegelt"]

', + ) + + page = Page.objects.get(id=2) + page.mirrored_page = mirrored + page.mirrored_page_first = False + page.save() + + translation = ( + PageTranslation.objects.filter(page_id=2, language__slug="de") + .order_by("-version") + .first() + ) + assert 'gespiegelt' in ( + translation.content_for_delivery() + ) + + +@pytest.mark.django_db +def test_content_for_delivery_accepts_extra_context(load_test_data: None) -> None: + """ + Context which cannot be derived from the translation can be passed in and wins over + the derived context + """ + from integreat_cms.cms.models import PageTranslation + + translation = ( + PageTranslation.objects.filter(page_id=2, language__slug="de") + .order_by("-version") + .first() + ) + PageTranslation.objects.filter(pk=translation.pk).update( + content="

[page 1]

", + ) + translation.refresh_from_db() + + assert translation.content_for_delivery(language_slug="en") == ( + '

Welcome

' + ) From f5efbb6a2a4a5087835f7f4d515056ac6e2759f7 Mon Sep 17 00:00:00 2001 From: Jonas Buchholz Date: Wed, 12 Aug 2026 07:47:30 +0000 Subject: [PATCH 5/8] serve full urls instead of relative paths in api --- integreat_cms/cms/utils/shortcodes/page.py | 10 ++- tests/api/test_api_shortcodes.py | 97 ++++++++++++++++++++++ tests/cms/utils/test_shortcodes.py | 14 ++-- 3 files changed, 110 insertions(+), 11 deletions(-) create mode 100644 tests/api/test_api_shortcodes.py diff --git a/integreat_cms/cms/utils/shortcodes/page.py b/integreat_cms/cms/utils/shortcodes/page.py index a286f2a925..86681845b8 100644 --- a/integreat_cms/cms/utils/shortcodes/page.py +++ b/integreat_cms/cms/utils/shortcodes/page.py @@ -82,7 +82,9 @@ def _render_link(translation: PageTranslation, inner_html: str | None) -> Elemen except LxmlError: element = Element("a") element.text = inner_html - element.attrib["href"] = translation.get_absolute_url() + # Absolute, because that is what internal links looked like before they were stored as + # shortcodes: the content delivered to clients has always contained full webapp urls + element.attrib["href"] = translation.full_url hide_anchor_tag_around_single_image(element) return element @@ -113,9 +115,9 @@ def page( :header-rows: 0 * - ``[page 1]`` - - ``Willkommen`` + - ``Willkommen`` * - ``[page 1 "this page"]`` - - ``this page`` + - ``this page`` * - ``[page 999999]`` - ``[MISSING LINK]`` """ @@ -152,7 +154,7 @@ def page_link( :header-rows: 0 * - ``[page_link 1]hier[/page_link]`` - - ``hier`` + - ``hier`` * - ``[page_link 999999]hier[/page_link]`` - ``[hier]`` """ diff --git a/tests/api/test_api_shortcodes.py b/tests/api/test_api_shortcodes.py new file mode 100644 index 0000000000..8da3faf1a4 --- /dev/null +++ b/tests/api/test_api_shortcodes.py @@ -0,0 +1,97 @@ +""" +This module tests that shortcodes are expanded in the content delivered by the API +""" + +from __future__ import annotations + +import pytest +from django.test.client import Client + +from integreat_cms.cms.models import PageTranslation + +#: The absolute url of the German translation of page 1 in the Augsburg region +WILLKOMMEN_URL = "https://integreat.app/augsburg/de/willkommen/" + + +def store_content(content: str, page_id: int = 2) -> None: + """ + Put the given content into every German translation of a page, bypassing the form so + that it is stored exactly as given + + :param content: The content to store + :param page_id: The id of the page whose content should be replaced + """ + PageTranslation.objects.filter(page_id=page_id, language__slug="de").update( + content=content, + ) + + +def get_page(page_id: int = 2) -> dict: + """ + Request a single page from the API + + :param page_id: The id of the page to request + :return: The delivered page + """ + response = Client().get(f"/api/v3/augsburg/de/page/?id={page_id}") + assert response.status_code == 200, response.content + return response.json() + + +@pytest.mark.django_db +def test_page_shortcode_is_delivered_as_absolute_link(load_test_data: None) -> None: + """ + A page shortcode is delivered as a link with an absolute url. + + Before internal links were stored as shortcodes they were stored as absolute urls, so + delivering a relative path here would change what clients receive. + """ + store_content('

[page 1 "hier"]

') + + assert get_page()["content"] == f'

hier

' + + +@pytest.mark.django_db +def test_page_shortcode_without_text_is_delivered_with_the_target_title( + load_test_data: None, +) -> None: + """ + A page shortcode without link text follows the title of its target + """ + store_content("

[page 1]

") + + assert get_page()["content"] == f'

Willkommen

' + + +@pytest.mark.django_db +def test_page_link_shortcode_is_delivered_as_absolute_link( + load_test_data: None, +) -> None: + """ + The block scoped shortcode wraps its content in a link with an absolute url too + """ + store_content("

[page_link 1]hier[/page_link]

") + + assert get_page()["content"] == f'

hier

' + + +@pytest.mark.django_db +def test_shortcode_is_stripped_from_the_excerpt(load_test_data: None) -> None: + """ + The excerpt is derived from the expanded content, so it contains the link text + instead of the shortcode + """ + store_content('

[page 1 "hier"]

') + + assert get_page()["excerpt"] == "hier" + + +@pytest.mark.django_db +def test_delivered_content_never_contains_a_shortcode(load_test_data: None) -> None: + """ + Whatever happens, no shortcode may leak into the delivered content + """ + store_content('

[page 1] and [page 1 "hier"]

') + + content = get_page()["content"] + assert "[page" not in content diff --git a/tests/cms/utils/test_shortcodes.py b/tests/cms/utils/test_shortcodes.py index f9aef1733d..76b1cc4763 100644 --- a/tests/cms/utils/test_shortcodes.py +++ b/tests/cms/utils/test_shortcodes.py @@ -20,7 +20,7 @@ def test_page_shortcode_without_text(load_test_data: None) -> None: """ assert ( expand_shortcodes("

[page 1]

", DE) - == '

Willkommen

' + == '

Willkommen

' ) @@ -31,7 +31,7 @@ def test_page_shortcode_with_text(load_test_data: None) -> None: """ assert ( expand_shortcodes('

[page 1 "hier"]

', DE) - == '

hier

' + == '

hier

' ) @@ -57,7 +57,7 @@ def test_page_link_shortcode(load_test_data: None) -> None: """ assert ( expand_shortcodes("

[page_link 1]hier lang[/page_link]

", DE) - == '

hier lang

' + == '

hier lang

' ) @@ -83,7 +83,7 @@ def test_page_link_shortcode_hides_linked_image_without_alt_text( assert expand_shortcodes( '

[page_link 1][/page_link]

', DE ) == ( - '

' ) @@ -121,7 +121,7 @@ def test_content_for_delivery_expands_shortcodes(load_test_data: None) -> None: translation.refresh_from_db() assert translation.content_for_delivery() == ( - '

hier

' + '

hier

' ) @@ -148,7 +148,7 @@ def test_content_for_delivery_includes_mirrored_content(load_test_data: None) -> .order_by("-version") .first() ) - assert 'gespiegelt' in ( + assert 'gespiegelt' in ( translation.content_for_delivery() ) @@ -172,5 +172,5 @@ def test_content_for_delivery_accepts_extra_context(load_test_data: None) -> Non translation.refresh_from_db() assert translation.content_for_delivery(language_slug="en") == ( - '

Welcome

' + '

Welcome

' ) From b47b0fded4dd9d980e9a7cb3f57f771af2afc116 Mon Sep 17 00:00:00 2001 From: Jonas Buchholz Date: Tue, 25 Aug 2026 10:44:38 +0200 Subject: [PATCH 6/8] refactor shortcodes and link_shortcode_utils into EditableShortcode registry --- integreat_cms/api/v3/raw_content.py | 23 +- .../models/abstract_content_translation.py | 8 +- .../cms/templatetags/content_filters.py | 4 +- integreat_cms/cms/utils/content_utils.py | 11 +- .../cms/utils/internal_link_utils.py | 86 --- .../cms/utils/link_shortcode_utils.py | 358 ----------- .../cms/utils/shortcodes/__init__.py | 193 +++++- integreat_cms/cms/utils/shortcodes/base.py | 242 ++++++++ integreat_cms/cms/utils/shortcodes/contact.py | 64 +- .../cms/utils/shortcodes/internal_link.py | 563 ++++++++++++++++++ integreat_cms/cms/utils/shortcodes/page.py | 177 ++---- integreat_cms/cms/utils/shortcodes/utils.py | 68 --- .../utils/machine_translation_api_client.py | 4 +- ...tcode_utils.py => test_link_shortcodes.py} | 33 +- tests/cms/utils/test_shortcode_registry.py | 113 ++++ tests/cms/utils/test_shortcodes.py | 23 +- 16 files changed, 1220 insertions(+), 750 deletions(-) delete mode 100644 integreat_cms/cms/utils/link_shortcode_utils.py create mode 100644 integreat_cms/cms/utils/shortcodes/base.py create mode 100644 integreat_cms/cms/utils/shortcodes/internal_link.py delete mode 100644 integreat_cms/cms/utils/shortcodes/utils.py rename tests/cms/utils/{test_link_shortcode_utils.py => test_link_shortcodes.py} (92%) create mode 100644 tests/cms/utils/test_shortcode_registry.py diff --git a/integreat_cms/api/v3/raw_content.py b/integreat_cms/api/v3/raw_content.py index 964810e564..02dae543b2 100644 --- a/integreat_cms/api/v3/raw_content.py +++ b/integreat_cms/api/v3/raw_content.py @@ -17,7 +17,6 @@ from ...cms.utils.internal_link_utils import ( get_public_translation_for_webapp_link_parts, ) -from ...cms.utils.shortcodes import expand_shortcodes from ...cms.utils.social_media_utils import ( get_region_title, ) @@ -28,15 +27,12 @@ ) if TYPE_CHECKING: - from typing import Any - from django.http import ( HttpRequest, HttpResponse, ) from ...cms.models.abstract_content_translation import AbstractContentTranslation - from ...cms.models.regions.region import Region logger = logging.getLogger(__name__) @@ -73,28 +69,17 @@ def render_error_content(request: HttpRequest, error: str) -> HttpResponse: def get_content( request: HttpRequest, - region: Region, - language: Language, translation: AbstractContentTranslation, ) -> str: """ Returns the content of a translation with all shortcodes expanded :param request: The current request - :param region: The region the translation belongs to - :param language: The language of the translation :param translation: The translation whose content should be rendered :return: The content of the translation """ - content = getattr(translation, "combined_text", translation.content) - context: dict[str, Any] = { - "region_slug": region.slug, - "language_slug": language.slug, - "content_object": translation, - "request": request, - } - return expand_shortcodes(content, context=context) + return translation.content_for_delivery(request=request) @partial_content_response @@ -198,7 +183,7 @@ def page_content( "raw_content.html", { "title": get_region_title(region, page_translation.title), - "content": get_content(request, region, language, page_translation), + "content": get_content(request, page_translation), "language_code": language.bcp47_tag, }, ) @@ -240,7 +225,7 @@ def event_content( "raw_content.html", { "title": get_region_title(region, event_translation.title), - "content": get_content(request, region, language, event_translation), + "content": get_content(request, event_translation), "language_code": language.bcp47_tag, }, ) @@ -314,7 +299,7 @@ def location_content( "raw_content.html", { "title": get_region_title(region, location_translation.title), - "content": get_content(request, region, language, location_translation), + "content": get_content(request, location_translation), "language_code": language.bcp47_tag, }, ) diff --git a/integreat_cms/cms/models/abstract_content_translation.py b/integreat_cms/cms/models/abstract_content_translation.py index 599264353a..3987d944af 100644 --- a/integreat_cms/cms/models/abstract_content_translation.py +++ b/integreat_cms/cms/models/abstract_content_translation.py @@ -571,9 +571,9 @@ def content_for_cms(self) -> str: :return: The content with expanded links """ # Imported here because the utils import the models - from ..utils.link_shortcode_utils import expand_link_shortcodes + from ..utils.shortcodes import expand_shortcodes_for_cms - return expand_link_shortcodes(self.content, self.language.slug) + return expand_shortcodes_for_cms(self.content, self.language.slug) def content_for_delivery(self, **extra_context: Any) -> str: r""" @@ -588,9 +588,9 @@ def content_for_delivery(self, **extra_context: Any) -> str: :return: The expanded content """ # Imported here because the utils import the models - from ..utils.shortcodes import expand_shortcodes + from ..utils.shortcodes import expand_shortcodes_for_delivery - return expand_shortcodes( + return expand_shortcodes_for_delivery( self.combined_text, context={ "region_slug": self.foreign_object.region.slug, diff --git a/integreat_cms/cms/templatetags/content_filters.py b/integreat_cms/cms/templatetags/content_filters.py index 005c0dd4de..99161db04f 100644 --- a/integreat_cms/cms/templatetags/content_filters.py +++ b/integreat_cms/cms/templatetags/content_filters.py @@ -22,7 +22,7 @@ PageTranslation, POITranslation, ) -from ..utils.link_shortcode_utils import expand_link_shortcodes +from ..utils.shortcodes import expand_shortcodes_for_cms if TYPE_CHECKING: from collections.abc import Iterable @@ -177,7 +177,7 @@ def expand_links(content: str, language_slug: str) -> str: :param language_slug: The slug of the language the content should be presented in :return: The content with expanded links """ - return expand_link_shortcodes(content, language_slug) + return expand_shortcodes_for_cms(content, language_slug) @register.filter diff --git a/integreat_cms/cms/utils/content_utils.py b/integreat_cms/cms/utils/content_utils.py index de95257a5b..c1835547d9 100644 --- a/integreat_cms/cms/utils/content_utils.py +++ b/integreat_cms/cms/utils/content_utils.py @@ -14,8 +14,8 @@ from ..models import Contact, MediaFile from ..utils import internal_link_utils -from ..utils.link_shortcode_utils import collapse_link_to_shortcode from ..utils.link_utils import fix_content_link_encoding +from ..utils.shortcodes import collapse_into_shortcodes logger = logging.getLogger(__name__) @@ -54,6 +54,7 @@ def clean_content( content = _xss_cleaner.clean_html(content) convert_heading(content) convert_monospaced_tags(content) + collapse_into_shortcodes(content) update_links(content, language_slug) fix_alt_texts(content) fix_notranslate(content) @@ -128,16 +129,14 @@ def update_links(content: HtmlElement, language_slug: str) -> None: """ Super method that gathers all methods related to updating links - Links to internal pages are replaced by the shortcode representing them, so that they are - only resolved when the content is delivered and never reach the link index of ``linkcheck``. + Links which reference internal content are already gone at this point, because + :func:`~integreat_cms.cms.utils.shortcodes.collapse_into_shortcodes` replaced them by the + shortcode representing them. :param content: The content whose links should be updated :param language_slug: Slug of the current language """ for link in list(content.iter("a")): - if collapse_link_to_shortcode(link): - # The link does not exist anymore, so there is nothing left to update - continue mark_external_links(link) remove_target_attribute(link) update_internal_links(link, language_slug) diff --git a/integreat_cms/cms/utils/internal_link_utils.py b/integreat_cms/cms/utils/internal_link_utils.py index 5c2bb1bd9a..bbae8c92d5 100644 --- a/integreat_cms/cms/utils/internal_link_utils.py +++ b/integreat_cms/cms/utils/internal_link_utils.py @@ -14,14 +14,11 @@ from ..models import ( EventTranslation, ImprintPageTranslation, - Page, PageTranslation, POITranslation, ) if TYPE_CHECKING: - from typing import Final - from lxml.html import Element from ..models.abstract_content_translation import AbstractContentTranslation @@ -193,86 +190,3 @@ def get_public_translation_for_short_link( return None return instance.public_version - - -#: The first path segment of urls which point to something else than a page -NON_PAGE_URL_INFIXES: Final[frozenset[str]] = frozenset( - {"events", "locations", "disclaimer", "news", "offers", "search"}, -) - - -def get_page_for_link(url: str) -> Page | None: - """ - Get the page an internal url points to. - - In contrast to :func:`~integreat_cms.cms.utils.internal_link_utils.get_public_translation_for_link`, - this does not care about the publication status of the target, because links to pages which - are not public (yet) have to be recognized as internal references as well. - - :param url: The url - :returns: The referenced page, or ``None`` if the url does not point to one - """ - if not url: - return None - parsed_url = urlparse(url) - if parsed_url.netloc == WEBAPP_NETLOC: - return get_page_for_webapp_link(parsed_url.path) - if parsed_url.netloc == SHORT_LINKS_NETLOC: - return get_page_for_short_link(parsed_url.path) - return None - - -def get_page_for_webapp_link(path: str) -> Page | None: - """ - Get the page a webapp url path points to - - :param path: The url path, for example ``/augsburg/de/willkommen/`` - :returns: The referenced page, or ``None`` if the path does not point to one - """ - parts: list[str] = unquote(path).strip("/").split("/") - if len(parts) < 3: - # Not a link to a specific piece of content - return None - - region_slug, language_slug, *path_parts = parts - if path_parts[0] in NON_PAGE_URL_INFIXES: - return None - - pages = Page.objects.filter( - region__slug=region_slug, - translations__language__slug=language_slug, - translations__slug=path_parts[-1], - ).distinct() - - if len(pages) < 2: - return pages.first() - - # The slug of a page is only unique among its siblings, so if the last path part is - # ambiguous, prefer the page whose current url matches the whole path. Outdated urls - # are still tolerated, because their slug is kept in the version history. - for page in pages: - if ( - translation := page.get_translation(language_slug) - ) and translation.get_absolute_url().strip("/") == "/".join(parts): - return page - return pages.first() - - -def get_page_for_short_link(path: str) -> Page | None: - """ - Get the page a short url path points to - - :param path: The url path, for example ``/s/p/124/`` - :returns: The referenced page, or ``None`` if the path does not point to one - """ - parts: list[str] = unquote(path).strip("/").split("/") - if len(parts) != 3 or parts[0] != "s" or parts[1] != "p": - # Short links to other content types do not have a page shortcode (yet) - return None - - try: - translation_id = int(parts[2]) - except ValueError: - return None - - return Page.objects.filter(translations__id=translation_id).first() diff --git a/integreat_cms/cms/utils/link_shortcode_utils.py b/integreat_cms/cms/utils/link_shortcode_utils.py deleted file mode 100644 index a3d7ecdc38..0000000000 --- a/integreat_cms/cms/utils/link_shortcode_utils.py +++ /dev/null @@ -1,358 +0,0 @@ -""" -This file contains utility functions to convert between links to internal content and the -shortcodes which represent them. - -Internal references are stored as shortcodes so that they are only resolved when the content -is delivered (see ``ADR/0001-compose-referenced-objects-into-content-dynamically-shortcodes.md``). -This means the link index kept by our ``linkcheck`` dependency never has to know about them. - -Editors should not have to care about that, so the shortcodes are expanded into ordinary -links whenever content is loaded into an editor (see -:func:`~integreat_cms.cms.utils.link_shortcode_utils.expand_link_shortcodes`) and collapsed -back into shortcodes whenever content is saved (see -:func:`~integreat_cms.cms.utils.link_shortcode_utils.collapse_links_to_shortcodes`). -""" - -from __future__ import annotations - -import logging -from copy import deepcopy -from typing import TYPE_CHECKING - -import shortcodes -from lxml.etree import LxmlError -from lxml.html import Element, fromstring, tostring - -from ..models import Page -from .internal_link_utils import get_page_for_link - -if TYPE_CHECKING: - from typing import Any, Final - - from lxml.html import HtmlElement - - from ..models.pages.page_translation import PageTranslation - -logger = logging.getLogger(__name__) - -#: The keyword of the atomic shortcode which links to a page -PAGE_KEYWORD: Final[str] = "page" - -#: The keyword of the block scoped shortcode which wraps its content in a link to a page -PAGE_LINK_KEYWORD: Final[str] = "page_link" - -#: The keyword which closes :data:`PAGE_LINK_KEYWORD` -PAGE_LINK_END_KEYWORD: Final[str] = "/page_link" - -#: The attribute which marks a link whose text should follow the title of its target -AUTO_UPDATE_ATTRIBUTE: Final[str] = "data-integreat-auto-update" - -#: Characters which must not appear in a quoted shortcode argument. ``"``, ``\``, ``[`` and -#: ``]`` confuse the shortcode parser, which stops at the first closing delimiter and does not -#: unescape anything inside quotes. ``&``, ``<`` and ``>`` are html escaped when the content is -#: serialized, which would pile up another layer of escaping on every save. -#: Link texts containing any of them use the block scoped shortcode instead, whose content is -#: html and therefore not affected. -UNQUOTABLE_CHARACTERS: Final[frozenset[str]] = frozenset('"\\[]&<>') - - -def format_page_shortcode(page_id: int, text: str | None = None) -> str: - """ - Build the atomic shortcode which links to a page - - :param page_id: The id of the :class:`~integreat_cms.cms.models.pages.page.Page` to link to - :param text: The link text, or ``None`` to let the link follow the title of its target - :return: The shortcode - """ - if text is None: - return f"[{PAGE_KEYWORD} {page_id}]" - return f'[{PAGE_KEYWORD} {page_id} "{text}"]' - - -def format_page_link_shortcode(page_id: int, content: str) -> str: - """ - Build the block scoped shortcode which wraps ``content`` in a link to a page - - :param page_id: The id of the :class:`~integreat_cms.cms.models.pages.page.Page` to link to - :param content: The inner html of the link - :return: The shortcode - """ - return f"[{PAGE_LINK_KEYWORD} {page_id}]{content}[{PAGE_LINK_END_KEYWORD}]" - - -def get_editor_page_translation( - page_id: str | int | None, - language_slug: str | None, -) -> PageTranslation | None: - """ - Get the page translation a link shortcode should point to while the content is edited. - - In contrast to the delivered content, the editor also has to be able to show links to - pages which are not public (yet), because it is possible to insert such links. - - :param page_id: The id of the referenced :class:`~integreat_cms.cms.models.pages.page.Page` - :param language_slug: The slug of the language the content is edited in - :return: The referenced translation, or ``None`` if it cannot be resolved - """ - if not page_id or not language_slug: - return None - try: - page = Page.objects.get(id=page_id) - except (Page.DoesNotExist, TypeError, ValueError): - logger.debug( - "Page with id=%r referenced by a shortcode does not exist", page_id - ) - return None - return ( - page.get_translation(language_slug) - or page.get_public_translation(language_slug) - or page.best_translation - ) - - -def expand_link_shortcodes(content: str, language_slug: str) -> str: - """ - Replace all link shortcodes in ``content`` by the link they represent. - - Shortcodes which cannot be resolved are kept verbatim, so that editing content with a - broken reference does not silently drop that reference. - - :param content: The content as it is stored in the database - :param language_slug: The slug of the language the content should be presented in - :return: The content with expanded links - """ - try: - return _link_parser.parse(content, {"language_slug": language_slug}) - except shortcodes.ShortcodeError: - logger.warning( - "Failed expanding link shortcodes in %r", - content, - exc_info=True, - ) - # The best way to fail gracefully is to keep the content as it is - return content - - -def collapse_links_to_shortcodes(content: HtmlElement) -> None: - """ - Replace all links to internal pages in ``content`` by the shortcode representing them - - :param content: The content whose links should be collapsed - """ - for link in list(content.iter("a")): - collapse_link_to_shortcode(link) - - -def collapse_link_to_shortcode(link: HtmlElement) -> bool: - """ - Replace ``link`` by the shortcode representing it, if it points to an internal page. - - Which shortcode is used depends on the content of the link: - - .. list-table:: - :widths: 55 45 - :header-rows: 1 - - * - Link - - Shortcode - * - ``Willkommen`` - - ``[page 1]`` - * - ``hier`` - - ``[page 1 "hier"]`` - * - ```` - - ``[page_link 1][/page_link]`` - - :param link: The link which should be collapsed - :return: Whether the link was replaced - """ - if not (page := get_page_for_link(link.get("href", ""))): - return False - if (parent := link.getparent()) is None: - logger.debug("Cannot collapse link %r without a parent element", link) - return False - - index = parent.index(link) - tail = link.tail or "" - text = link.text or "" - children = list(link) - - if link.get(AUTO_UPDATE_ATTRIBUTE) == "true": - # The link follows the title of its target, so its current content is irrelevant - opening, closing, children = format_page_shortcode(page.id), "", [] - elif not children and not UNQUOTABLE_CHARACTERS.intersection(text): - opening, closing = format_page_shortcode(page.id, text), "" - elif not children: - opening, closing = format_page_link_shortcode(page.id, text), "" - else: - # The children have to stay elements of the content, so the block scoped shortcode - # is split into the text around them - opening = f"[{PAGE_LINK_KEYWORD} {page.id}]{text}" - closing = f"[{PAGE_LINK_END_KEYWORD}]" - - parent.remove(link) - for offset, child in enumerate(children): - parent.insert(index + offset, child) - if children: - children[-1].tail = (children[-1].tail or "") + closing + tail - _append_text_before(parent, index, opening) - else: - _append_text_before(parent, index, opening + closing + tail) - - logger.debug("Collapsed link to %r into a shortcode", page) - return True - - -def _append_text_before(parent: HtmlElement, index: int, text: str) -> None: - """ - Append ``text`` to the character data which precedes the child of ``parent`` at ``index`` - - :param parent: The element whose character data should be extended - :param index: The index of the child element the text should precede - :param text: The text to append - """ - if index == 0: - parent.text = (parent.text or "") + text - else: - previous = parent[index - 1] - previous.tail = (previous.tail or "") + text - - -def _set_inner_html(element: HtmlElement, inner_html: str) -> None: - """ - Set the content of ``element`` to the given html string - - :param element: The element whose content should be set - :param inner_html: The html to insert into the element - """ - try: - parsed = fromstring(f"
{inner_html}
") - except LxmlError: - logger.debug("Failed to parse inner html of a link: %r", inner_html) - element.text = inner_html - return - element.text = parsed.text - for child in parsed: - element.append(child) - - -def _set_link_title(link: HtmlElement, link_title: HtmlElement | str) -> None: - """ - Set the content of ``link`` to the link title of its target - - :param link: The link whose content should be set - :param link_title: The :attr:`~integreat_cms.cms.models.abstract_content_translation.AbstractContentTranslation.link_title` - of the target, which is either an escaped string or an element with a tail - """ - if isinstance(link_title, str): - _set_inner_html(link, link_title) - else: - # The link title is cached on the translation, so it must not be re-parented - link.append(deepcopy(link_title)) - - -def _render_link( - page_id: str | int | None, - context: dict[str, Any] | None, - text: str | None = None, - inner_html: str | None = None, -) -> HtmlElement | None: - """ - Render the link a shortcode represents while the content is edited - - :param page_id: The id of the referenced :class:`~integreat_cms.cms.models.pages.page.Page` - :param context: The context the shortcode is expanded in - :param text: The link text of the atomic shortcode, if it has one - :param inner_html: The content of the block scoped shortcode, if it is used - :return: The link, or ``None`` if the reference cannot be resolved - """ - language_slug = (context or {}).get("language_slug") - if not (translation := get_editor_page_translation(page_id, language_slug)): - return None - - link = Element("a") - link.set("href", translation.full_url) - if inner_html is not None: - _set_inner_html(link, inner_html) - elif text is None: - # Without an explicit link text, the link follows the title of its target - link.set(AUTO_UPDATE_ATTRIBUTE, "true") - _set_link_title(link, translation.link_title) - else: - link.text = text - return link - - -def _unparse(keyword: str, pargs: list[str], kwargs: dict[str, str]) -> str: - """ - Rebuild the source representation of a shortcode tag - - :param keyword: The keyword of the shortcode - :param pargs: The positional arguments of the shortcode - :param kwargs: The keyword arguments of the shortcode - :return: The shortcode tag - """ - # Everything but the id of the target is quoted, so that no quotes get lost while a - # shortcode which cannot be resolved is kept verbatim - arguments = [ - parg if index == 0 and parg and not any(map(str.isspace, parg)) else f'"{parg}"' - for index, parg in enumerate(pargs) - ] - arguments += [f'{key}="{value}"' for key, value in kwargs.items()] - return f"[{' '.join([keyword, *arguments])}]" - - -def _expand_page( - pargs: list[str], - kwargs: dict[str, str], - context: dict[str, Any] | None, -) -> str: - """ - Expand the atomic ``page`` shortcode into a link - - :param pargs: The positional arguments of the shortcode - :param kwargs: The keyword arguments of the shortcode - :param context: The context the shortcode is expanded in - :return: The link, or the shortcode itself if it cannot be resolved - """ - text = pargs[1] if len(pargs) > 1 else None - if (link := _render_link(pargs[0] if pargs else None, context, text=text)) is None: - return _unparse(PAGE_KEYWORD, pargs, kwargs) - return tostring(link, encoding="unicode", with_tail=False) - - -def _expand_page_link( - pargs: list[str], - kwargs: dict[str, str], - context: dict[str, Any] | None, - content: str = "", -) -> str: - """ - Expand the block scoped ``page_link`` shortcode into a link around its content - - :param pargs: The positional arguments of the shortcode - :param kwargs: The keyword arguments of the shortcode - :param context: The context the shortcode is expanded in - :param content: The content enclosed by the shortcode - :return: The link, or the shortcode itself if it cannot be resolved - """ - if ( - link := _render_link(pargs[0] if pargs else None, context, inner_html=content) - ) is None: - return ( - _unparse(PAGE_LINK_KEYWORD, pargs, kwargs) - + content - + f"[{PAGE_LINK_END_KEYWORD}]" - ) - return tostring(link, encoding="unicode", with_tail=False) - - -#: Parser which only knows the link shortcodes and passes everything else through unchanged -_link_parser = shortcodes.Parser( - start="[", - end="]", - esc="\\", - inherit_globals=False, - ignore_unknown=True, -) -_link_parser.register(_expand_page, PAGE_KEYWORD) -_link_parser.register(_expand_page_link, PAGE_LINK_KEYWORD, PAGE_LINK_END_KEYWORD) diff --git a/integreat_cms/cms/utils/shortcodes/__init__.py b/integreat_cms/cms/utils/shortcodes/__init__.py index 124984687d..687c8aba62 100644 --- a/integreat_cms/cms/utils/shortcodes/__init__.py +++ b/integreat_cms/cms/utils/shortcodes/__init__.py @@ -1,40 +1,191 @@ """ -This module contains implementations for the shortcodes content filters +This package contains the shortcodes which reference other objects from the content of a +translation, and the conversions between those shortcodes and the html they represent. + +References are stored as shortcodes so that they are only resolved when the content is +requested (see ``ADR/0001-compose-referenced-objects-into-content-dynamically-shortcodes.md``). +That happens in two flavours: + +* :func:`~integreat_cms.cms.utils.shortcodes.expand_shortcodes_for_delivery` builds the + representation which is delivered to end users +* :func:`~integreat_cms.cms.utils.shortcodes.expand_shortcodes_for_cms` builds the + representation which is presented to users of the CMS, because editors should not have to + care about shortcodes at all + +Whatever the CMS gets back is turned into shortcodes again by +:func:`~integreat_cms.cms.utils.shortcodes.collapse_into_shortcodes`, so that references to +internal content never reach the link index kept by our ``linkcheck`` dependency. + +All three are implemented by the shortcodes themselves, see +:class:`~integreat_cms.cms.utils.shortcodes.base.Shortcode` and +:class:`~integreat_cms.cms.utils.shortcodes.base.EditableShortcode`. """ from __future__ import annotations import logging -from typing import Any +from functools import cache +from typing import TYPE_CHECKING import shortcodes -from django import template -from django.template.defaultfilters import stringfilter -from .contact import contact -from .page import page, page_link +from .base import EditableShortcode, registered_shortcodes + +if TYPE_CHECKING: + from typing import Any + + from lxml.html import HtmlElement + + from .base import Shortcode logger = logging.getLogger(__name__) -register = template.Library() -# Needed context: -# - region -# - language -# - accessed path? -# - login status? -parser = shortcodes.Parser(start="[", end="]", esc="\\", ignore_unknown=True) +@cache +def get_shortcodes() -> tuple[Shortcode, ...]: + """ + Get all registered shortcodes. + + The modules which define them are imported here instead of at the top of this module, + because a shortcode may need anything from the models to the content utils, which in turn + need this module to collapse content into shortcodes. + + :return: The registered shortcodes + """ + from . import contact, page + + return registered_shortcodes() + + +@cache +def _delivery_parser() -> shortcodes.Parser: + """ + Get the parser which expands shortcodes into the content delivered to end users + + :return: The parser + """ + parser = _build_parser() + for shortcode in get_shortcodes(): + parser.register(shortcode.expand, shortcode.keyword) + if shortcode.block_keyword: + parser.register( + shortcode.expand_block, + shortcode.block_keyword, + shortcode.block_end_keyword, + ) + return parser + + +@cache +def _editable_shortcodes() -> tuple[EditableShortcode, ...]: + """ + Get the shortcodes which are hidden from the users of the CMS + + :return: The editable shortcodes + """ + return tuple( + shortcode + for shortcode in get_shortcodes() + if isinstance(shortcode, EditableShortcode) + ) + + +@cache +def _cms_parser() -> shortcodes.Parser: + """ + Get the parser which expands shortcodes into the content presented in the CMS. + Shortcodes which are not editable are not registered at all, so that they are kept + verbatim instead of being expanded into something the CMS could not collapse again. -@register.filter -@stringfilter -def expand_shortcodes(content: str, context: dict[str, Any] | None = None) -> str: + :return: The parser + """ + parser = _build_parser() + for shortcode in _editable_shortcodes(): + parser.register(shortcode.expand_for_cms, shortcode.keyword) + if shortcode.block_keyword: + parser.register( + shortcode.expand_block_for_cms, + shortcode.block_keyword, + shortcode.block_end_keyword, + ) + return parser + + +def _build_parser() -> shortcodes.Parser: + """ + Build an empty parser which uses our shortcode syntax + + :return: The parser + """ + return shortcodes.Parser( + start="[", + end="]", + esc="\\", + inherit_globals=False, + ignore_unknown=True, + ) + + +def expand_shortcodes_for_delivery( + content: str, + context: dict[str, Any] | None = None, +) -> str: + """ + Replace all shortcodes in ``content`` by the representation delivered to end users + + :param content: The content as it is stored in the database + :param context: The context the shortcodes are expanded in + :return: The expanded content + """ + try: + return _delivery_parser().parse(content, context) + except shortcodes.ShortcodeError: + logger.warning( + "Failed expanding shortcodes in %r\ncontext: %r", + content, + context, + exc_info=True, + ) + # The best way to fail gracefully is to keep the content as it is + return content + + +def expand_shortcodes_for_cms(content: str, language_slug: str) -> str: + """ + Replace all editable shortcodes in ``content`` by the html they are edited as. + + Shortcodes which cannot be resolved are kept verbatim, so that editing content with a + broken reference does not silently drop that reference. + + :param content: The content as it is stored in the database + :param language_slug: The slug of the language the content should be presented in + :return: The content with expanded references + """ try: - return parser.parse(content, context) - except shortcodes.ShortcodeError as e: + return _cms_parser().parse(content, {"language_slug": language_slug}) + except shortcodes.ShortcodeError: logger.warning( - "Failed expanding shortcodes: %s\ncontext: %r", e, context, exc_info=True + "Failed expanding shortcodes for the CMS in %r", + content, + exc_info=True, ) - # We failed expanding the shortcodes, - # the best way we can fail gracefully is to just return the original content + # The best way to fail gracefully is to keep the content as it is return content + + +def collapse_into_shortcodes(content: HtmlElement) -> None: + """ + Replace everything in ``content`` which references another object by its shortcode. + + The tree is walked once and every element is passed through the cheap predicate of every + editable shortcode, so that only elements which really might be a reference cause the + database lookups needed to resolve them. + + :param content: The content which should be collapsed + """ + editable = _editable_shortcodes() + for element in list(content.iter()): + for shortcode in editable: + if shortcode.matches(element) and shortcode.collapse(element): + break diff --git a/integreat_cms/cms/utils/shortcodes/base.py b/integreat_cms/cms/utils/shortcodes/base.py new file mode 100644 index 0000000000..7df468f9e8 --- /dev/null +++ b/integreat_cms/cms/utils/shortcodes/base.py @@ -0,0 +1,242 @@ +""" +This module defines what a shortcode is and keeps the registry of all known shortcodes. +""" + +from __future__ import annotations + +from abc import ABC, abstractmethod +from typing import TYPE_CHECKING + +if TYPE_CHECKING: + from typing import Any, ClassVar + + from lxml.html import HtmlElement + + +class Shortcode(ABC): + """ + A marker which references an object from the content of a translation. + + Shortcodes are only resolved when the content is requested, so that it always reflects the + current state of the referenced object (see + ``ADR/0001-compose-referenced-objects-into-content-dynamically-shortcodes.md``). + + Every shortcode has an atomic form (``[keyword …]``) and may additionally have a block + scoped form (``[block_keyword …]…[/block_keyword]``) which encloses content. + + Subclasses become known to the application by being decorated with + :func:`~integreat_cms.cms.utils.shortcodes.base.register`. + """ + + #: The keyword of the atomic form, for example ``page`` in ``[page 1]`` + keyword: ClassVar[str] + + #: The keyword of the block scoped form, for example ``page_link`` in + #: ``[page_link 1]…[/page_link]``, or ``None`` if this shortcode has no block scoped form + block_keyword: ClassVar[str | None] = None + + @property + def block_end_keyword(self) -> str | None: + """ + The keyword which closes the block scoped form of this shortcode + + :return: The end keyword, or ``None`` if this shortcode has no block scoped form + """ + return f"/{self.block_keyword}" if self.block_keyword else None + + @abstractmethod + def expand( + self, + pargs: list[str], + kwargs: dict[str, str], + context: dict[str, Any] | None, + ) -> str: + """ + Expand the atomic form of this shortcode into the html which is delivered to end users + + :param pargs: The positional arguments of the shortcode + :param kwargs: The keyword arguments of the shortcode + :param context: The context the shortcode is expanded in + :return: The html which represents the referenced object + """ + + def expand_block( + self, + pargs: list[str], + kwargs: dict[str, str], + context: dict[str, Any] | None, + content: str = "", + ) -> str: + """ + Expand the block scoped form of this shortcode into the html which is delivered to end users + + :param pargs: The positional arguments of the shortcode + :param kwargs: The keyword arguments of the shortcode + :param context: The context the shortcode is expanded in + :param content: The content enclosed by the shortcode + :raises NotImplementedError: If this shortcode has no block scoped form + :return: The html which represents the referenced object + """ + raise NotImplementedError( + f"The shortcode {self.keyword!r} has no block scoped form", + ) + + def unparse(self, pargs: list[str], kwargs: dict[str, str]) -> str: + """ + Rebuild the source representation of the atomic form of this shortcode. + + This is what an expansion falls back to when the referenced object cannot be resolved + and the shortcode should be kept verbatim instead. + + :param pargs: The positional arguments of the shortcode + :param kwargs: The keyword arguments of the shortcode + :return: The shortcode tag + """ + return _unparse(self.keyword, pargs, kwargs) + + def unparse_block( + self, + pargs: list[str], + kwargs: dict[str, str], + content: str, + ) -> str: + """ + Rebuild the source representation of the block scoped form of this shortcode + + :param pargs: The positional arguments of the shortcode + :param kwargs: The keyword arguments of the shortcode + :param content: The content enclosed by the shortcode + :return: The shortcode with its content + """ + return ( + _unparse(self.block_keyword or self.keyword, pargs, kwargs) + + content + + f"[{self.block_end_keyword}]" + ) + + +class EditableShortcode(Shortcode, ABC): + """ + A shortcode which is presented as ordinary html while the content is edited in the CMS. + + Editors should not have to care about shortcodes, so such a shortcode bundles three pieces + which together let the CMS hide it: + + 1. :meth:`expand_for_cms` (and :meth:`expand_block_for_cms`) turn the shortcode into the + html which is loaded into the editor + 2. :meth:`matches` cheaply decides whether an element might be that html again + 3. :meth:`collapse` resolves what :meth:`matches` found and replaces it by the shortcode + + The split between 2. and 3. exists because every element of every saved content is passed + through :meth:`matches`, which therefore must not query the database. Only the elements it + accepts are handed to :meth:`collapse`, which may. + """ + + @abstractmethod + def expand_for_cms( + self, + pargs: list[str], + kwargs: dict[str, str], + context: dict[str, Any] | None, + ) -> str: + """ + Expand the atomic form of this shortcode into the html which is edited in the CMS + + :param pargs: The positional arguments of the shortcode + :param kwargs: The keyword arguments of the shortcode + :param context: The context the shortcode is expanded in + :return: The html which represents the referenced object + """ + + def expand_block_for_cms( + self, + pargs: list[str], + kwargs: dict[str, str], + context: dict[str, Any] | None, + content: str = "", + ) -> str: + """ + Expand the block scoped form of this shortcode into the html which is edited in the CMS + + :param pargs: The positional arguments of the shortcode + :param kwargs: The keyword arguments of the shortcode + :param context: The context the shortcode is expanded in + :param content: The content enclosed by the shortcode + :raises NotImplementedError: If this shortcode has no block scoped form + :return: The html which represents the referenced object + """ + raise NotImplementedError( + f"The shortcode {self.keyword!r} has no block scoped form", + ) + + @abstractmethod + def matches(self, element: HtmlElement) -> bool: + """ + Cheaply decide whether ``element`` might be an expansion of this shortcode. + + This is called for every element of every saved content, so it must be a matter of + string comparisons and must never query the database. False positives are fine, + :meth:`collapse` sorts them out. + + :param element: The element to check + :return: Whether the element is a candidate for being collapsed + """ + + @abstractmethod + def collapse(self, element: HtmlElement) -> bool: + """ + Replace ``element`` by the shortcode representing it, if it really references an object + + :param element: The element which should be collapsed + :return: Whether the element was replaced + """ + + +#: All registered shortcodes, in the order they were registered +_registry: list[Shortcode] = [] + + +def register[ShortcodeT: Shortcode](shortcode: type[ShortcodeT]) -> type[ShortcodeT]: + """ + Class decorator which makes a shortcode known to the application:: + + @register + class CatShortcode(Shortcode): + keyword = "cat" + + def expand(self, pargs, kwargs, context): + return "(=^･ω･^=)" + + :param shortcode: The shortcode to register + :return: The shortcode itself, so that this can be used as a decorator + """ + _registry.append(shortcode()) + return shortcode + + +def registered_shortcodes() -> tuple[Shortcode, ...]: + """ + Get the shortcodes which have been registered so far + + :return: The registered shortcodes + """ + return tuple(_registry) + + +def _unparse(keyword: str, pargs: list[str], kwargs: dict[str, str]) -> str: + """ + Rebuild the source representation of a shortcode tag + + :param keyword: The keyword of the shortcode + :param pargs: The positional arguments of the shortcode + :param kwargs: The keyword arguments of the shortcode + :return: The shortcode tag + """ + # Everything but the id of the target is quoted, so that no quotes get lost while a + # shortcode which cannot be resolved is kept verbatim + arguments = [ + parg if index == 0 and parg and not any(map(str.isspace, parg)) else f'"{parg}"' + for index, parg in enumerate(pargs) + ] + arguments += [f'{key}="{value}"' for key, value in kwargs.items()] + return f"[{' '.join([keyword, *arguments])}]" diff --git a/integreat_cms/cms/utils/shortcodes/contact.py b/integreat_cms/cms/utils/shortcodes/contact.py index d1d75bc9df..89ba7c4e77 100644 --- a/integreat_cms/cms/utils/shortcodes/contact.py +++ b/integreat_cms/cms/utils/shortcodes/contact.py @@ -1,18 +1,31 @@ -from typing import Any +""" +This module contains the shortcode which references a :class:`~integreat_cms.cms.models.contact.contact.Contact` +""" + +from __future__ import annotations + +from typing import TYPE_CHECKING from lxml.html import tostring from ..content_utils import render_contact_card -from .utils import shortcode +from .base import register, Shortcode + +if TYPE_CHECKING: + from typing import Any, Final + +#: The details of a contact which can be requested individually +CONTACT_DETAILS: Final[tuple[str, ...]] = ( + "address", + "email", + "phone_number", + "mobile_phone_number", + "website", +) -@shortcode -def contact( - pargs: list[str], - kwargs: dict[str, str], # noqa: ARG001 - context: dict[str, Any] | None, # noqa: ARG001 - content: str = "", # noqa: ARG001 -) -> str: +@register +class ContactShortcode(Shortcode): """ Shortcode to insert a contact card with details from a :class:`~integreat_cms.cms.models.contact.contact.Contact`. @@ -28,14 +41,25 @@ def contact( * ``mobile_phone_number`` (optional) – Whether the mobile phone number should be shown and other, not explicitly wanted details should be hidden * ``website`` (optional) – Whether the website should be shown and other, not explicitly wanted details should be hidden """ - contact_id = pargs[0] if pargs else None - options = ( - "address", - "email", - "phone_number", - "mobile_phone_number", - "website", - ) - wanted = tuple(arg for arg in pargs[1:] if arg in options) or options - element = render_contact_card(contact_id, wanted) - return tostring(element).decode("utf-8") + + keyword = "contact" + + def expand( + self, + pargs: list[str], + kwargs: dict[str, str], # noqa: ARG002 + context: dict[str, Any] | None, # noqa: ARG002 + ) -> str: + """ + Expand the shortcode into the rendered contact card + + :param pargs: The positional arguments of the shortcode + :param kwargs: The keyword arguments of the shortcode + :param context: The context the shortcode is expanded in + :return: The rendered contact card + """ + contact_id = pargs[0] if pargs else None + wanted = ( + tuple(arg for arg in pargs[1:] if arg in CONTACT_DETAILS) or CONTACT_DETAILS + ) + return tostring(render_contact_card(contact_id, wanted)).decode("utf-8") diff --git a/integreat_cms/cms/utils/shortcodes/internal_link.py b/integreat_cms/cms/utils/shortcodes/internal_link.py new file mode 100644 index 0000000000..dceae8abf6 --- /dev/null +++ b/integreat_cms/cms/utils/shortcodes/internal_link.py @@ -0,0 +1,563 @@ +""" +This module contains everything shortcodes which reference internal content by a link have +in common. Adding such a shortcode for another kind of content is a matter of declaring the +model it references and how its urls look, see +:class:`~integreat_cms.cms.utils.shortcodes.internal_link.InternalLinkShortcode`. +""" + +from __future__ import annotations + +import logging +from copy import deepcopy +from typing import TYPE_CHECKING +from urllib.parse import unquote, urlparse + +from django.utils.translation import gettext_lazy as _ +from lxml.etree import LxmlError +from lxml.html import Element, fromstring, tostring + +from ..content_utils import hide_anchor_tag_around_single_image +from ..internal_link_utils import SHORT_LINKS_NETLOC, WEBAPP_NETLOC +from .base import EditableShortcode + +if TYPE_CHECKING: + from typing import Any, ClassVar, Final + + from lxml.html import HtmlElement + + from ...models.abstract_content_model import AbstractContentModel + from ...models.abstract_content_translation import AbstractContentTranslation + +logger = logging.getLogger(__name__) + +#: The attribute which marks a link whose text should follow the title of its target +AUTO_UPDATE_ATTRIBUTE: Final[str] = "data-integreat-auto-update" + +#: Characters which must not appear in a quoted shortcode argument. ``"``, ``\``, ``[`` and +#: ``]`` confuse the shortcode parser, which stops at the first closing delimiter and does not +#: unescape anything inside quotes. ``&``, ``<`` and ``>`` are html escaped when the content is +#: serialized, which would pile up another layer of escaping on every save. +#: Link texts containing any of them use the block scoped shortcode instead, whose content is +#: html and therefore not affected. +UNQUOTABLE_CHARACTERS: Final[frozenset[str]] = frozenset('"\\[]&<>') + + +class InternalLinkShortcode(EditableShortcode): + """ + A shortcode which references internal content by a link. + + Both forms of the shortcode are used, depending on what the link contains: + + .. list-table:: + :widths: 55 45 + :header-rows: 1 + + * - Link + - Shortcode + * - ``Willkommen`` + - ``[page 1]`` + * - ``hier`` + - ``[page 1 "hier"]`` + * - ```` + - ``[page_link 1][/page_link]`` + + Subclasses only declare which content they reference and how its urls look:: + + @register + class EventShortcode(InternalLinkShortcode): + keyword = "event" + block_keyword = "event_link" + model = Event + url_infix = "events" + """ + + #: The model of the content this shortcode references + model: ClassVar[type[AbstractContentModel]] + + #: The path segment which distinguishes webapp urls to this content from urls to other + #: content, for example ``events`` in ``/augsburg/de/events/test-veranstaltung/`` + url_infix: ClassVar[str | None] = None + + #: The path segment which identifies this content in short urls, for example ``p`` in + #: ``/s/p/42/``, or ``None`` if this content has no short urls + short_url_infix: ClassVar[str | None] = None + + def matches_url_infix(self, infix: str) -> bool: + """ + Whether the first path segment after region and language belongs to this content + + :param infix: The path segment + :return: Whether a url with this segment points to this kind of content + """ + return infix == self.url_infix + + def format_shortcode(self, object_id: int, text: str | None = None) -> str: + """ + Build the atomic shortcode which links to the given object + + :param object_id: The id of the object to link to + :param text: The link text, or ``None`` to let the link follow the title of its target + :return: The shortcode + """ + if text is None: + return f"[{self.keyword} {object_id}]" + return f'[{self.keyword} {object_id} "{text}"]' + + def format_block_shortcode(self, object_id: int, content: str) -> str: + """ + Build the block scoped shortcode which wraps ``content`` in a link to the given object + + :param object_id: The id of the object to link to + :param content: The inner html of the link + :return: The shortcode + """ + return f"[{self.block_keyword} {object_id}]{content}[{self.block_end_keyword}]" + + # Expansion into the content which is delivered to end users + + def expand( + self, + pargs: list[str], + kwargs: dict[str, str], # noqa: ARG002 + context: dict[str, Any] | None, + ) -> str: + """ + Expand the atomic form into a link to the public translation of its target + + :param pargs: The positional arguments of the shortcode + :param kwargs: The keyword arguments of the shortcode + :param context: The context the shortcode is expanded in + :return: The link, or a marker that the reference is broken + """ + text = pargs[1] if len(pargs) > 1 else None + if (translation := self._get_public_translation(pargs, context)) is None: + element = _missing_link(text or "") + else: + element = _render_link(translation, text) + return tostring(element).decode("utf-8") + + def expand_block( + self, + pargs: list[str], + kwargs: dict[str, str], # noqa: ARG002 + context: dict[str, Any] | None, + content: str = "", + ) -> str: + """ + Expand the block scoped form into a link around its content + + :param pargs: The positional arguments of the shortcode + :param kwargs: The keyword arguments of the shortcode + :param context: The context the shortcode is expanded in + :param content: The content enclosed by the shortcode + :return: The link, or a marker that the reference is broken + """ + if (translation := self._get_public_translation(pargs, context)) is None: + element = _missing_link(content) + else: + element = _render_link(translation, content) + return tostring(element).decode("utf-8") + + def _get_public_translation( + self, + pargs: list[str], + context: dict[str, Any] | None, + ) -> AbstractContentTranslation | None: + """ + Get the public translation this shortcode refers to + + :param pargs: The positional arguments of the shortcode, the first of which is the id + of the referenced object + :param context: The context the shortcode is expanded in + :return: The public translation which is linked to, or ``None`` if it cannot be resolved + """ + if (referenced := self._get_object_by_id(pargs[0] if pargs else None)) is None: + return None + language_slug = (context or {}).get( + "language_slug", + referenced.region.default_language.slug, + ) + return referenced.get_public_translation(language_slug) + + # Expansion into the content which is edited in the CMS + + def expand_for_cms( + self, + pargs: list[str], + kwargs: dict[str, str], + context: dict[str, Any] | None, + ) -> str: + """ + Expand the atomic form into the link which is loaded into the editor + + :param pargs: The positional arguments of the shortcode + :param kwargs: The keyword arguments of the shortcode + :param context: The context the shortcode is expanded in + :return: The link, or the shortcode itself if it cannot be resolved + """ + text = pargs[1] if len(pargs) > 1 else None + if (link := self._render_editor_link(pargs, context, text=text)) is None: + return self.unparse(pargs, kwargs) + return tostring(link, encoding="unicode", with_tail=False) + + def expand_block_for_cms( + self, + pargs: list[str], + kwargs: dict[str, str], + context: dict[str, Any] | None, + content: str = "", + ) -> str: + """ + Expand the block scoped form into the link which is loaded into the editor + + :param pargs: The positional arguments of the shortcode + :param kwargs: The keyword arguments of the shortcode + :param context: The context the shortcode is expanded in + :param content: The content enclosed by the shortcode + :return: The link, or the shortcode itself if it cannot be resolved + """ + link = self._render_editor_link(pargs, context, inner_html=content) + if link is None: + return self.unparse_block(pargs, kwargs, content) + return tostring(link, encoding="unicode", with_tail=False) + + def _render_editor_link( + self, + pargs: list[str], + context: dict[str, Any] | None, + text: str | None = None, + inner_html: str | None = None, + ) -> HtmlElement | None: + """ + Render the link this shortcode represents while the content is edited + + :param pargs: The positional arguments of the shortcode, the first of which is the id + of the referenced object + :param context: The context the shortcode is expanded in + :param text: The link text of the atomic form, if it has one + :param inner_html: The content of the block scoped form, if it is used + :return: The link, or ``None`` if the reference cannot be resolved + """ + language_slug = (context or {}).get("language_slug") + translation = self._get_editor_translation( + pargs[0] if pargs else None, + language_slug, + ) + if translation is None: + return None + + link = Element("a") + link.set("href", translation.full_url) + if inner_html is not None: + _set_inner_html(link, inner_html) + elif text is None: + # Without an explicit link text, the link follows the title of its target + link.set(AUTO_UPDATE_ATTRIBUTE, "true") + _set_link_title(link, translation.link_title) + else: + link.text = text + return link + + def _get_editor_translation( + self, + object_id: str | int | None, + language_slug: str | None, + ) -> AbstractContentTranslation | None: + """ + Get the translation this shortcode should point to while the content is edited. + + In contrast to the delivered content, the editor also has to be able to show links to + content which is not public (yet), because it is possible to insert such links. + + :param object_id: The id of the referenced object + :param language_slug: The slug of the language the content is edited in + :return: The referenced translation, or ``None`` if it cannot be resolved + """ + if not language_slug: + return None + if (referenced := self._get_object_by_id(object_id)) is None: + return None + return ( + referenced.get_translation(language_slug) + or referenced.get_public_translation(language_slug) + or referenced.best_translation + ) + + def _get_object_by_id( + self, + object_id: str | int | None, + ) -> AbstractContentModel | None: + """ + Get the object a shortcode references by its id + + :param object_id: The id of the referenced object + :return: The referenced object, or ``None`` if it does not exist + """ + if not object_id: + return None + try: + return self.model.objects.get(id=object_id) + except (self.model.DoesNotExist, TypeError, ValueError): + logger.debug( + "%s with id=%r referenced by a shortcode does not exist", + self.model.__name__, + object_id, + ) + return None + + # Collapsing the content which was edited in the CMS + + def matches(self, element: HtmlElement) -> bool: + """ + Whether ``element`` is a link whose url looks like it points to this kind of content + + :param element: The element to check + :return: Whether the element is a candidate for being collapsed + """ + return element.tag == "a" and self._references(element.get("href", "")) + + def _references(self, url: str) -> bool: + """ + Whether ``url`` points to this kind of content, judged by its shape alone + + :param url: The url to check + :return: Whether the url might point to this kind of content + """ + if not url: + return False + parsed_url = urlparse(url) + if parsed_url.netloc == WEBAPP_NETLOC: + return self._webapp_path_parts(parsed_url.path) is not None + if parsed_url.netloc == SHORT_LINKS_NETLOC: + return self._short_link_translation_id(parsed_url.path) is not None + return False + + def collapse(self, link: HtmlElement) -> bool: + """ + Replace ``link`` by the shortcode representing it, if it points to this kind of content. + + The children of the link have to stay elements of the content, so a link which contains + markup is not replaced by a single text node but by the opening and closing tag of the + block scoped shortcode around its children. + + :param link: The link which should be collapsed + :return: Whether the link was replaced + """ + if not (referenced := self.get_object_for_url(link.get("href", ""))): + return False + if (parent := link.getparent()) is None: + logger.debug("Cannot collapse link %r without a parent element", link) + return False + + index = parent.index(link) + tail = link.tail or "" + text = link.text or "" + children = list(link) + + if link.get(AUTO_UPDATE_ATTRIBUTE) == "true": + # The link follows the title of its target, so its current content is irrelevant + opening, closing, children = self.format_shortcode(referenced.id), "", [] + elif not children and not UNQUOTABLE_CHARACTERS.intersection(text): + opening, closing = self.format_shortcode(referenced.id, text), "" + elif not children: + opening, closing = self.format_block_shortcode(referenced.id, text), "" + else: + opening = f"[{self.block_keyword} {referenced.id}]{text}" + closing = f"[{self.block_end_keyword}]" + + parent.remove(link) + for offset, child in enumerate(children): + parent.insert(index + offset, child) + if children: + children[-1].tail = (children[-1].tail or "") + closing + tail + _append_text_before(parent, index, opening) + else: + _append_text_before(parent, index, opening + closing + tail) + + logger.debug("Collapsed link to %r into a shortcode", referenced) + return True + + def get_object_for_url(self, url: str) -> AbstractContentModel | None: + """ + Get the object an internal url points to. + + In contrast to :func:`~integreat_cms.cms.utils.internal_link_utils.get_public_translation_for_link`, + this does not care about the publication status of the target, because links to content + which is not public (yet) have to be recognized as internal references as well. + + :param url: The url + :return: The referenced object, or ``None`` if the url does not point to one + """ + if not url: + return None + parsed_url = urlparse(url) + if parsed_url.netloc == WEBAPP_NETLOC: + if (parts := self._webapp_path_parts(parsed_url.path)) is None: + return None + region_slug, language_slug, *path_parts = parts + return self._get_object_for_webapp_link( + region_slug, + language_slug, + path_parts, + ) + if parsed_url.netloc == SHORT_LINKS_NETLOC: + if ( + translation_id := self._short_link_translation_id(parsed_url.path) + ) is None: + return None + return self.model.objects.filter(translations__id=translation_id).first() + return None + + def _webapp_path_parts(self, path: str) -> list[str] | None: + """ + Split the path of a webapp url into its parts, if it points to this kind of content + + :param path: The url path, for example ``/augsburg/de/willkommen/`` + :return: The path parts, or ``None`` if the path does not point to this content + """ + parts: list[str] = unquote(path).strip("/").split("/") + if len(parts) < 3 or not self.matches_url_infix(parts[2]): + # Not a link to a specific piece of this kind of content + return None + return parts + + def _short_link_translation_id(self, path: str) -> int | None: + """ + Get the id of the translation a short url path points to + + :param path: The url path, for example ``/s/p/124/`` + :return: The id of the referenced translation, or ``None`` if the path does not + point to this kind of content + """ + parts: list[str] = unquote(path).strip("/").split("/") + if len(parts) != 3 or parts[0] != "s" or parts[1] != self.short_url_infix: + return None + try: + return int(parts[2]) + except ValueError: + return None + + def _get_object_for_webapp_link( + self, + region_slug: str, + language_slug: str, + path_parts: list[str], + ) -> AbstractContentModel | None: + """ + Get the object a webapp url points to + + :param region_slug: The slug of the region of the referenced content + :param language_slug: The slug of the language of the url + :param path_parts: The path parts after region and language, + for example ``["willkommen"]`` + :return: The referenced object, or ``None`` if it does not exist + """ + referenced = self.model.objects.filter( + region__slug=region_slug, + translations__language__slug=language_slug, + translations__slug=path_parts[-1], + ).distinct() + + if len(referenced) < 2: + return referenced.first() + + # The slug of a page is only unique among its siblings, so if the last path part is + # ambiguous, prefer the object whose current url matches the whole path. Outdated urls + # are still tolerated, because their slug is kept in the version history. + path = "/".join([region_slug, language_slug, *path_parts]) + for candidate in referenced: + if ( + translation := candidate.get_translation(language_slug) + ) and translation.get_absolute_url().strip("/") == path: + return candidate + return referenced.first() + + +def _missing_link(inner_html: str) -> HtmlElement: + """ + Build the replacement for a shortcode whose target cannot be resolved + + :param inner_html: The content of the link, if it has any + :return: The element to insert instead of the link + """ + TEXT_MISSING = _( + "MISSING LINK" + ) # Separate variable because gettext apparently does not find _() if it is in an f-string + try: + return fromstring(f"[{inner_html or TEXT_MISSING}]") + except LxmlError: + element = Element("i") + element.text = f"[{TEXT_MISSING}]" + return element + + +def _render_link( + translation: AbstractContentTranslation, + inner_html: str | None, +) -> HtmlElement: + """ + Build the link to the given translation which is delivered to end users + + :param translation: The translation which is linked to + :param inner_html: The content of the link, or ``None`` to use the link title of the target + :return: The link element + """ + element = Element("a") + if inner_html is None: + _set_link_title(element, translation.link_title) + else: + _set_inner_html(element, inner_html) + # Absolute, because that is what internal links looked like before they were stored as + # shortcodes: the content delivered to clients has always contained full webapp urls + element.attrib["href"] = translation.full_url + hide_anchor_tag_around_single_image(element) + return element + + +def _set_inner_html(element: HtmlElement, inner_html: str) -> None: + """ + Set the content of ``element`` to the given html string + + :param element: The element whose content should be set + :param inner_html: The html to insert into the element + """ + try: + # LXML needs a single root element, so we're doing this in a roundabout way + parsed = fromstring(f"
{inner_html}
") + except LxmlError: + logger.debug("Failed to parse inner html of a link: %r", inner_html) + element.text = inner_html + return + element.text = parsed.text + for child in parsed: + element.append(child) + + +def _set_link_title(link: HtmlElement, link_title: HtmlElement | str) -> None: + """ + Set the content of ``link`` to the link title of its target + + :param link: The link whose content should be set + :param link_title: The :attr:`~integreat_cms.cms.models.abstract_content_translation.AbstractContentTranslation.link_title` + of the target, which is either an escaped string or an element with a tail + """ + if isinstance(link_title, str): + _set_inner_html(link, link_title) + else: + # The link title is cached on the translation, so it must not be re-parented + link.append(deepcopy(link_title)) + + +def _append_text_before(parent: HtmlElement, index: int, text: str) -> None: + """ + Append ``text`` to the character data which precedes the child of ``parent`` at ``index`` + + :param parent: The element whose character data should be extended + :param index: The index of the child element the text should precede + :param text: The text to append + """ + if index == 0: + parent.text = (parent.text or "") + text + else: + previous = parent[index - 1] + previous.tail = (previous.tail or "") + text diff --git a/integreat_cms/cms/utils/shortcodes/page.py b/integreat_cms/cms/utils/shortcodes/page.py index 86681845b8..f3a64c7d92 100644 --- a/integreat_cms/cms/utils/shortcodes/page.py +++ b/integreat_cms/cms/utils/shortcodes/page.py @@ -1,168 +1,67 @@ -from copy import deepcopy -from typing import Any - -from django.utils.translation import gettext_lazy as _ -from lxml.etree import LxmlError -from lxml.html import Element, fromstring, tostring - -from ...models import Page, PageTranslation -from ..content_utils import hide_anchor_tag_around_single_image -from ..link_shortcode_utils import ( - PAGE_KEYWORD, - PAGE_LINK_END_KEYWORD, - PAGE_LINK_KEYWORD, -) -from .utils import shortcode +""" +This module contains the shortcode which references a :class:`~integreat_cms.cms.models.pages.page.Page` +""" +from __future__ import annotations -def _get_public_translation( - page_id: str | None, - context: dict[str, Any] | None, -) -> PageTranslation: - """ - Get the public translation a page shortcode refers to - - :param page_id: The id of the :class:`~integreat_cms.cms.models.pages.page.Page` to which should be linked - :param context: The context the shortcode is expanded in - :raises ~integreat_cms.cms.models.pages.page.Page.DoesNotExist: If the page does not exist - :raises ~integreat_cms.cms.models.pages.page_translation.PageTranslation.DoesNotExist: If the - page has no public translation in the requested language - :return: The public translation which is linked to - """ - page = Page.objects.get(id=page_id) - translation = page.get_public_translation( - (context or {}).get("language_slug", page.region.default_language.slug), - ) - if translation is None: - raise PageTranslation.DoesNotExist - return translation +from typing import TYPE_CHECKING +from ...models import Page +from .base import register +from .internal_link import InternalLinkShortcode -def _missing_link(inner_html: str) -> Element: - """ - Build the replacement for a page shortcode whose target cannot be resolved +if TYPE_CHECKING: + from typing import Final - :param inner_html: The content of the link, if it has any - :return: The element to insert instead of the link - """ - TEXT_MISSING = _( - "MISSING LINK" - ) # Separate variable because gettext apparently does not find _() if it is in an f-string - try: - return fromstring(f"[{inner_html or TEXT_MISSING}]") - except LxmlError: - element = Element("i") - element.text = f"[{TEXT_MISSING}]" - return element - - -def _render_link(translation: PageTranslation, inner_html: str | None) -> Element: - """ - Build the link to the given translation +#: The first path segment of webapp urls which point to something else than a page +NON_PAGE_URL_INFIXES: Final[frozenset[str]] = frozenset( + {"events", "locations", "disclaimer", "news", "offers", "search"}, +) - :param translation: The translation which is linked to - :param inner_html: The content of the link, or ``None`` to use the link title of the target - :return: The link element - """ - if inner_html is None: - element = Element("a") - link_title = translation.link_title - if isinstance(link_title, str): - # LXML needs a single root element, so we're doing this in a roundabout way - root = fromstring(f"{link_title}") - element.text = root.text - for child in root: - element.append(child) - else: - # The link title is cached on the translation, so it must not be re-parented - element.append(deepcopy(link_title)) - else: - try: - element = fromstring(f"{inner_html}") - except LxmlError: - element = Element("a") - element.text = inner_html - # Absolute, because that is what internal links looked like before they were stored as - # shortcodes: the content delivered to clients has always contained full webapp urls - element.attrib["href"] = translation.full_url - hide_anchor_tag_around_single_image(element) - return element - - -@shortcode(PAGE_KEYWORD) -def page( - pargs: list[str], - kwargs: dict[str, str], # noqa: ARG001 - context: dict[str, Any] | None, - content: str = "", # noqa: ARG001 -) -> str: + +@register +class PageShortcode(InternalLinkShortcode): """ Shortcode to insert an internal link to a :class:`~integreat_cms.cms.models.pages.page.Page`. - Positional arguments: + Positional arguments of the atomic form ``[page …]``: * ``page_id`` – The id of the :class:`~integreat_cms.cms.models.pages.page.Page` to which should be linked * ``link_text`` (optional) – If not given, the title of the public :class:`~integreat_cms.cms.models.pages.page_translation.PageTranslation` is used If the target page has an icon set and the shortcode has no ``link_text``, - the icon will be included as an ``<ìmg>`` before the page title. + the icon will be included as an ```` before the page title. - If the link should wrap html instead of plain text, use - :func:`~integreat_cms.cms.utils.shortcodes.page.page_link` instead. + Whenever the link should wrap html instead of plain text, the block scoped form + ``[page_link …]…[/page_link]`` is used, which takes only the ``page_id``. .. list-table:: Examples - :widths: 30 70 + :widths: 45 55 :header-rows: 0 * - ``[page 1]`` - ``Willkommen`` * - ``[page 1 "this page"]`` - ``this page`` + * - ``[page_link 1]hier[/page_link]`` + - ``hier`` * - ``[page 999999]`` - ``[MISSING LINK]`` + * - ``[page_link 999999]hier[/page_link]`` + - ``[hier]`` """ - page_id = pargs[0] if pargs else None - text = pargs[1] if len(pargs) > 1 else None - try: - translation = _get_public_translation(page_id, context) - except (Page.DoesNotExist, PageTranslation.DoesNotExist): - element = _missing_link(text or "") - else: - element = _render_link(translation, text) - return tostring(element).decode("utf-8") - - -@shortcode(PAGE_LINK_KEYWORD, PAGE_LINK_END_KEYWORD) -def page_link( - pargs: list[str], - kwargs: dict[str, str], # noqa: ARG001 - context: dict[str, Any] | None, - content: str = "", -) -> str: - """ - Shortcode to wrap its content in an internal link to a :class:`~integreat_cms.cms.models.pages.page.Page`. - - This is the block scoped counterpart of :func:`~integreat_cms.cms.utils.shortcodes.page.page`, - which is used whenever the content of the link is not plain text, for example a linked image. - Positional arguments: + keyword = "page" + block_keyword = "page_link" + model = Page + short_url_infix = "p" - * ``page_id`` – The id of the :class:`~integreat_cms.cms.models.pages.page.Page` to which should be linked - - .. list-table:: Examples - :widths: 45 55 - :header-rows: 0 + def matches_url_infix(self, infix: str) -> bool: + """ + Pages are the only content whose urls have no distinguishing path segment, so every + url which does not belong to another kind of content points to a page - * - ``[page_link 1]hier[/page_link]`` - - ``hier`` - * - ``[page_link 999999]hier[/page_link]`` - - ``[hier]`` - """ - page_id = pargs[0] if pargs else None - try: - translation = _get_public_translation(page_id, context) - except (Page.DoesNotExist, PageTranslation.DoesNotExist): - element = _missing_link(content) - else: - element = _render_link(translation, content) - return tostring(element).decode("utf-8") + :param infix: The first path segment after region and language + :return: Whether a url with this segment points to a page + """ + return infix not in NON_PAGE_URL_INFIXES diff --git a/integreat_cms/cms/utils/shortcodes/utils.py b/integreat_cms/cms/utils/shortcodes/utils.py deleted file mode 100644 index 7cf06efa41..0000000000 --- a/integreat_cms/cms/utils/shortcodes/utils.py +++ /dev/null @@ -1,68 +0,0 @@ -from collections.abc import Callable -from typing import overload - -import shortcodes - - -@overload -def shortcode[**P, R](tag: Callable[P, R]) -> Callable[P, R]: ... - - -@overload -def shortcode[**P, R]( - tag: str | None = None, endtag: str | None = None -) -> Callable[[Callable[P, R]], Callable[P, R]]: ... - - -def shortcode[**P, R]( - tag: Callable[P, R] | str | None = None, endtag: str | None = None -) -> Callable[[Callable[P, R]], Callable[P, R]] | Callable[P, R]: - """ - Decorator to register a function as a shortcode - - For example, this would declare a shortcode ``cat`` (taken from the function name), - where the first parameter can be ``0`` or ``1`` to select between two ASCII cats to insert:: - - @shortcode - def cat(pargs, kwargs, context, content=""): - cats = ''' - (=^･ω･^=) - ฅ/ᐠ- ˕ -マ - '''.split() - return cats[pargs[0]] - - If the shortcode should use a different keyword than just the name of the function, - it can be provided as an argument to the decorator:: - - @shortcode("ASCII_cat") - def cat(pargs, kwargs, context, content=""): - […] - - When ``endtag`` is given, the shortcode will not be atomic but can enclose content - between its opening tag and end tag:: - - @shortcode("cat", "tac") - def cat(pargs, kwargs, context, content=""): - return f"⚞({content})⚟" - - This can then be used as ``[cat]some feline content[tac]``. - """ - - def inner(func: Callable[P, R]) -> Callable[P, R]: - """Just register the shortcode once and return the original function""" - nonlocal tag - if not tag: - # Default tag is the function name - tag = func.__name__ - shortcodes.register(tag, endtag)(func) - return func - - if callable(tag): - # We are being used without parantheses (``@shortcode``) - # and the first argument already is the function being decorated. - func = tag - tag = None - return inner(func) - # We are being used with parantheses (``@shortcode("keyword")``). - # Return the inner function itself so it can be called with the function being defined next - return inner diff --git a/integreat_cms/core/utils/machine_translation_api_client.py b/integreat_cms/core/utils/machine_translation_api_client.py index bfb7fc9a32..1ab817a6d3 100644 --- a/integreat_cms/core/utils/machine_translation_api_client.py +++ b/integreat_cms/core/utils/machine_translation_api_client.py @@ -362,12 +362,12 @@ def prepare_content_objects(self) -> list[TranslationContext]: # would happily translate them into something we cannot resolve anymore. # Whatever comes back is collapsed into shortcodes again on save. # Imported here because this module is loaded before the models are ready - from ...cms.utils.link_shortcode_utils import expand_link_shortcodes + from ...cms.utils.shortcodes import expand_shortcodes_for_cms ctx.translatable_attributes = [ ( attr, - expand_link_shortcodes(value, self.source_language.slug) + expand_shortcodes_for_cms(value, self.source_language.slug) if attr == "content" else value, ) diff --git a/tests/cms/utils/test_link_shortcode_utils.py b/tests/cms/utils/test_link_shortcodes.py similarity index 92% rename from tests/cms/utils/test_link_shortcode_utils.py rename to tests/cms/utils/test_link_shortcodes.py index 51c6a57331..4972efffa9 100644 --- a/tests/cms/utils/test_link_shortcode_utils.py +++ b/tests/cms/utils/test_link_shortcodes.py @@ -10,9 +10,9 @@ from integreat_cms.cms.constants import status from integreat_cms.cms.models import PageTranslation from integreat_cms.cms.utils.content_utils import clean_content -from integreat_cms.cms.utils.link_shortcode_utils import ( - collapse_links_to_shortcodes, - expand_link_shortcodes, +from integreat_cms.cms.utils.shortcodes import ( + collapse_into_shortcodes, + expand_shortcodes_for_cms, ) #: The full url of the German translation of page 1 in the Augsburg region @@ -36,11 +36,11 @@ def unpublish_page_3() -> None: def collapse(content: str) -> str: """ - Run :func:`~integreat_cms.cms.utils.link_shortcode_utils.collapse_links_to_shortcodes` + Run :func:`~integreat_cms.cms.utils.shortcodes.collapse_into_shortcodes` on a html string and return the result as a html string again """ element = fromstring(content) - collapse_links_to_shortcodes(element) + collapse_into_shortcodes(element) return tostring(element, encoding="unicode", with_tail=False) @@ -49,7 +49,7 @@ def test_expand_page_shortcode_without_text(load_test_data: None) -> None: """ A ``[page]`` shortcode without link text becomes an auto updating link """ - assert expand_link_shortcodes("

[page 1]

", "de") == ( + assert expand_shortcodes_for_cms("

[page 1]

", "de") == ( f'

Willkommen

' ) @@ -59,7 +59,7 @@ def test_expand_page_shortcode_with_text(load_test_data: None) -> None: """ A ``[page]`` shortcode with link text becomes a plain link """ - assert expand_link_shortcodes('

[page 1 "hier"]

', "de") == ( + assert expand_shortcodes_for_cms('

[page 1 "hier"]

', "de") == ( f'

hier

' ) @@ -69,7 +69,7 @@ def test_expand_page_shortcode_uses_requested_language(load_test_data: None) -> """ The shortcode is expanded to the url of the translation in the requested language """ - assert expand_link_shortcodes("

[page 1]

", "en") == ( + assert expand_shortcodes_for_cms("

[page 1]

", "en") == ( '

Welcome

' ) @@ -81,7 +81,7 @@ def test_expand_page_link_shortcode(load_test_data: None) -> None: A ``[page_link]`` block shortcode wraps its content in a link """ assert ( - expand_link_shortcodes( + expand_shortcodes_for_cms( '

[page_link 1][/page_link]

', "de" ) == f'

' @@ -95,14 +95,15 @@ def test_expand_unresolvable_shortcode_is_kept_verbatim(load_test_data: None) -> instead of silently vanishing from the content """ assert ( - expand_link_shortcodes("

[page 999999]

", "de") == "

[page 999999]

" + expand_shortcodes_for_cms("

[page 999999]

", "de") + == "

[page 999999]

" ) assert ( - expand_link_shortcodes('

[page 999999 "hier"]

', "de") + expand_shortcodes_for_cms('

[page 999999 "hier"]

', "de") == '

[page 999999 "hier"]

' ) assert ( - expand_link_shortcodes("

[page_link 999999]x[/page_link]

", "de") + expand_shortcodes_for_cms("

[page_link 999999]x[/page_link]

", "de") == "

[page_link 999999]x[/page_link]

" ) @@ -113,7 +114,7 @@ def test_expand_leaves_other_shortcodes_alone(load_test_data: None) -> None: Only link shortcodes are expanded, everything else is passed through """ assert ( - expand_link_shortcodes("

[contact 1 email]

", "de") + expand_shortcodes_for_cms("

[contact 1 email]

", "de") == "

[contact 1 email]

" ) @@ -125,7 +126,7 @@ def test_expand_page_shortcode_to_draft_page(load_test_data: None) -> None: so those shortcodes must be expanded as well """ unpublish_page_3() - assert expand_link_shortcodes("

[page 3]

", "de") == ( + assert expand_shortcodes_for_cms("

[page 3]

", "de") == ( f'

' "Über die App Integreat Augsburg

" ) @@ -239,7 +240,7 @@ def test_round_trip(load_test_data: None, shortcodes: str) -> None: """ Expanding and collapsing again must not change the stored content """ - assert collapse(expand_link_shortcodes(shortcodes, "de")) == shortcodes + assert collapse(expand_shortcodes_for_cms(shortcodes, "de")) == shortcodes @pytest.mark.django_db @@ -303,7 +304,7 @@ def test_round_trip_of_escaped_content( """ content = shortcodes for _iteration in range(3): - content = collapse(expand_link_shortcodes(content, "de")) + content = collapse(expand_shortcodes_for_cms(content, "de")) assert content == shortcodes diff --git a/tests/cms/utils/test_shortcode_registry.py b/tests/cms/utils/test_shortcode_registry.py new file mode 100644 index 0000000000..b3c5a23f1a --- /dev/null +++ b/tests/cms/utils/test_shortcode_registry.py @@ -0,0 +1,113 @@ +""" +Tests for the registry which bundles the three pieces every shortcode consists of +""" + +from __future__ import annotations + +from typing import TYPE_CHECKING + +import pytest +from lxml.html import fromstring + +from integreat_cms.cms.utils.shortcodes import ( + EditableShortcode, + expand_shortcodes_for_cms, + get_shortcodes, +) +from integreat_cms.cms.utils.shortcodes.page import PageShortcode + +if TYPE_CHECKING: + from collections.abc import Callable + + from lxml.html import HtmlElement + +#: The full url of the German translation of page 1 in the Augsburg region +WILLKOMMEN_URL = "https://integreat.app/augsburg/de/willkommen/" + + +def parse(html: str) -> HtmlElement: + """ + Parse a single html element, no matter whether it is a block level element or not + + :param html: The element to parse + :return: The parsed element + """ + return fromstring(f"
{html}
")[0] + + +def test_registry_contains_all_shortcodes() -> None: + """ + Every module which defines a shortcode is loaded when the registry is used + """ + assert {shortcode.keyword for shortcode in get_shortcodes()} == {"page", "contact"} + + +def test_only_editable_shortcodes_have_a_cms_representation() -> None: + """ + A shortcode which references a page is hidden from the editor, a contact card is not + """ + editable = { + shortcode.keyword + for shortcode in get_shortcodes() + if isinstance(shortcode, EditableShortcode) + } + assert editable == {"page"} + + +@pytest.mark.django_db +def test_shortcodes_without_a_cms_representation_are_kept_verbatim( + load_test_data: None, +) -> None: + """ + Only editable shortcodes are expanded for the CMS, everything else is left untouched + """ + assert expand_shortcodes_for_cms("

[contact 1 email]

", "de") == ( + "

[contact 1 email]

" + ) + + +@pytest.mark.parametrize( + ("html", "expected"), + [ + (f'hier', True), + ('hier', True), + ('extern', False), + ( + 'Fest', + False, + ), + ('Augsburg', False), + ('Impressum', False), + ("ohne Ziel", False), + (f'kein Link', False), + ], +) +def test_predicate_recognizes_links_to_pages(html: str, expected: bool) -> None: + """ + The predicate accepts exactly those elements which might be a link to a page + + :param html: The element to check + :param expected: Whether the predicate should accept it + """ + assert PageShortcode().matches(parse(html)) is expected + + +@pytest.mark.django_db +def test_predicate_does_not_query_the_database( + django_assert_num_queries: Callable, +) -> None: + """ + The predicate is run for every element of every saved content, so it has to decide + without touching the database. Only what it accepts is looked up by ``collapse``. + + :param django_assert_num_queries: The fixture providing the query assertion + """ + elements = [ + parse(f'hier'), + parse('extern'), + parse("

kein Link

"), + ] + shortcode = PageShortcode() + with django_assert_num_queries(0): + for element in elements: + shortcode.matches(element) diff --git a/tests/cms/utils/test_shortcodes.py b/tests/cms/utils/test_shortcodes.py index 76b1cc4763..ae02490cae 100644 --- a/tests/cms/utils/test_shortcodes.py +++ b/tests/cms/utils/test_shortcodes.py @@ -7,7 +7,7 @@ import pytest from django.utils import translation -from integreat_cms.cms.utils.shortcodes import expand_shortcodes +from integreat_cms.cms.utils.shortcodes import expand_shortcodes_for_delivery #: The context the shortcodes are expanded in DE = {"language_slug": "de"} @@ -19,7 +19,7 @@ def test_page_shortcode_without_text(load_test_data: None) -> None: A ``[page]`` shortcode without link text uses the title of its target """ assert ( - expand_shortcodes("

[page 1]

", DE) + expand_shortcodes_for_delivery("

[page 1]

", DE) == '

Willkommen

' ) @@ -30,7 +30,7 @@ def test_page_shortcode_with_text(load_test_data: None) -> None: A ``[page]`` shortcode with link text uses that text """ assert ( - expand_shortcodes('

[page 1 "hier"]

', DE) + expand_shortcodes_for_delivery('

[page 1 "hier"]

', DE) == '

hier

' ) @@ -42,11 +42,12 @@ def test_page_shortcode_missing_target(load_test_data: None) -> None: """ with translation.override("en"): assert ( - expand_shortcodes("

[page 999999]

", DE) + expand_shortcodes_for_delivery("

[page 999999]

", DE) == "

[MISSING LINK]

" ) assert ( - expand_shortcodes('

[page 999999 "hier"]

', DE) == "

[hier]

" + expand_shortcodes_for_delivery('

[page 999999 "hier"]

', DE) + == "

[hier]

" ) @@ -56,7 +57,9 @@ def test_page_link_shortcode(load_test_data: None) -> None: A ``[page_link]`` shortcode wraps its content in a link to its target """ assert ( - expand_shortcodes("

[page_link 1]hier lang[/page_link]

", DE) + expand_shortcodes_for_delivery( + "

[page_link 1]hier lang[/page_link]

", DE + ) == '

hier lang

' ) @@ -67,7 +70,9 @@ def test_page_link_shortcode_missing_target(load_test_data: None) -> None: A ``[page_link]`` shortcode whose target does not exist keeps its content """ assert ( - expand_shortcodes("

[page_link 999999]hier[/page_link]

", DE) + expand_shortcodes_for_delivery( + "

[page_link 999999]hier[/page_link]

", DE + ) == "

[hier]

" ) @@ -80,7 +85,7 @@ def test_page_link_shortcode_hides_linked_image_without_alt_text( A link which only contains an image without alt text has to be hidden from screen readers and the tab key, just like the same link would be if it was part of the content """ - assert expand_shortcodes( + assert expand_shortcodes_for_delivery( '

[page_link 1][/page_link]

', DE ) == ( '

" msgstr "" "deshalb sind Unterschiede gegeben und zu erwarten. Mehr " -"Details dazu in der Dokumentation " +"Details dazu in der Dokumentation " #: cms/templates/statistics/statistics_sidebar.html msgid "Adjust shown data" @@ -9564,7 +9564,7 @@ msgstr "" msgid "The PDF could not be successfully generated." msgstr "PDF-Datei konnte nicht erfolgreich erzeugt werden." -#: cms/utils/shortcodes/page.py +#: cms/utils/shortcodes/internal_link.py msgid "MISSING LINK" msgstr "FEHLENDER LINK" From 39ad7e1632d5b910a8213237bb53af4809010fe5 Mon Sep 17 00:00:00 2001 From: Jonas Buchholz Date: Wed, 2 Sep 2026 09:10:51 +0200 Subject: [PATCH 8/8] refactoring: move shortcode helper functions to conversion and registry modules --- integreat_cms/cms/utils/content_utils.py | 2 +- .../cms/utils/shortcodes/__init__.py | 183 ++---------------- integreat_cms/cms/utils/shortcodes/base.py | 35 +--- integreat_cms/cms/utils/shortcodes/contact.py | 3 +- .../cms/utils/shortcodes/conversion.py | 144 ++++++++++++++ integreat_cms/cms/utils/shortcodes/page.py | 2 +- .../cms/utils/shortcodes/registry.py | 65 +++++++ tests/cms/utils/test_link_shortcodes.py | 2 +- 8 files changed, 228 insertions(+), 208 deletions(-) create mode 100644 integreat_cms/cms/utils/shortcodes/conversion.py create mode 100644 integreat_cms/cms/utils/shortcodes/registry.py diff --git a/integreat_cms/cms/utils/content_utils.py b/integreat_cms/cms/utils/content_utils.py index c1835547d9..65064038ab 100644 --- a/integreat_cms/cms/utils/content_utils.py +++ b/integreat_cms/cms/utils/content_utils.py @@ -130,7 +130,7 @@ def update_links(content: HtmlElement, language_slug: str) -> None: Super method that gathers all methods related to updating links Links which reference internal content are already gone at this point, because - :func:`~integreat_cms.cms.utils.shortcodes.collapse_into_shortcodes` replaced them by the + :func:`~integreat_cms.cms.utils.shortcodes.conversion.collapse_into_shortcodes` replaced them by the shortcode representing them. :param content: The content whose links should be updated diff --git a/integreat_cms/cms/utils/shortcodes/__init__.py b/integreat_cms/cms/utils/shortcodes/__init__.py index 687c8aba62..d2a905e454 100644 --- a/integreat_cms/cms/utils/shortcodes/__init__.py +++ b/integreat_cms/cms/utils/shortcodes/__init__.py @@ -6,15 +6,15 @@ requested (see ``ADR/0001-compose-referenced-objects-into-content-dynamically-shortcodes.md``). That happens in two flavours: -* :func:`~integreat_cms.cms.utils.shortcodes.expand_shortcodes_for_delivery` builds the - representation which is delivered to end users -* :func:`~integreat_cms.cms.utils.shortcodes.expand_shortcodes_for_cms` builds the +* :func:`~integreat_cms.cms.utils.shortcodes.conversion.expand_shortcodes_for_delivery` builds + the representation which is delivered to end users +* :func:`~integreat_cms.cms.utils.shortcodes.conversion.expand_shortcodes_for_cms` builds the representation which is presented to users of the CMS, because editors should not have to care about shortcodes at all Whatever the CMS gets back is turned into shortcodes again by -:func:`~integreat_cms.cms.utils.shortcodes.collapse_into_shortcodes`, so that references to -internal content never reach the link index kept by our ``linkcheck`` dependency. +:func:`~integreat_cms.cms.utils.shortcodes.conversion.collapse_into_shortcodes`, so that +references to internal content never reach the link index kept by our ``linkcheck`` dependency. All three are implemented by the shortcodes themselves, see :class:`~integreat_cms.cms.utils.shortcodes.base.Shortcode` and @@ -23,169 +23,10 @@ from __future__ import annotations -import logging -from functools import cache -from typing import TYPE_CHECKING - -import shortcodes - -from .base import EditableShortcode, registered_shortcodes - -if TYPE_CHECKING: - from typing import Any - - from lxml.html import HtmlElement - - from .base import Shortcode - -logger = logging.getLogger(__name__) - - -@cache -def get_shortcodes() -> tuple[Shortcode, ...]: - """ - Get all registered shortcodes. - - The modules which define them are imported here instead of at the top of this module, - because a shortcode may need anything from the models to the content utils, which in turn - need this module to collapse content into shortcodes. - - :return: The registered shortcodes - """ - from . import contact, page - - return registered_shortcodes() - - -@cache -def _delivery_parser() -> shortcodes.Parser: - """ - Get the parser which expands shortcodes into the content delivered to end users - - :return: The parser - """ - parser = _build_parser() - for shortcode in get_shortcodes(): - parser.register(shortcode.expand, shortcode.keyword) - if shortcode.block_keyword: - parser.register( - shortcode.expand_block, - shortcode.block_keyword, - shortcode.block_end_keyword, - ) - return parser - - -@cache -def _editable_shortcodes() -> tuple[EditableShortcode, ...]: - """ - Get the shortcodes which are hidden from the users of the CMS - - :return: The editable shortcodes - """ - return tuple( - shortcode - for shortcode in get_shortcodes() - if isinstance(shortcode, EditableShortcode) - ) - - -@cache -def _cms_parser() -> shortcodes.Parser: - """ - Get the parser which expands shortcodes into the content presented in the CMS. - - Shortcodes which are not editable are not registered at all, so that they are kept - verbatim instead of being expanded into something the CMS could not collapse again. - - :return: The parser - """ - parser = _build_parser() - for shortcode in _editable_shortcodes(): - parser.register(shortcode.expand_for_cms, shortcode.keyword) - if shortcode.block_keyword: - parser.register( - shortcode.expand_block_for_cms, - shortcode.block_keyword, - shortcode.block_end_keyword, - ) - return parser - - -def _build_parser() -> shortcodes.Parser: - """ - Build an empty parser which uses our shortcode syntax - - :return: The parser - """ - return shortcodes.Parser( - start="[", - end="]", - esc="\\", - inherit_globals=False, - ignore_unknown=True, - ) - - -def expand_shortcodes_for_delivery( - content: str, - context: dict[str, Any] | None = None, -) -> str: - """ - Replace all shortcodes in ``content`` by the representation delivered to end users - - :param content: The content as it is stored in the database - :param context: The context the shortcodes are expanded in - :return: The expanded content - """ - try: - return _delivery_parser().parse(content, context) - except shortcodes.ShortcodeError: - logger.warning( - "Failed expanding shortcodes in %r\ncontext: %r", - content, - context, - exc_info=True, - ) - # The best way to fail gracefully is to keep the content as it is - return content - - -def expand_shortcodes_for_cms(content: str, language_slug: str) -> str: - """ - Replace all editable shortcodes in ``content`` by the html they are edited as. - - Shortcodes which cannot be resolved are kept verbatim, so that editing content with a - broken reference does not silently drop that reference. - - :param content: The content as it is stored in the database - :param language_slug: The slug of the language the content should be presented in - :return: The content with expanded references - """ - try: - return _cms_parser().parse(content, {"language_slug": language_slug}) - except shortcodes.ShortcodeError: - logger.warning( - "Failed expanding shortcodes for the CMS in %r", - content, - exc_info=True, - ) - # The best way to fail gracefully is to keep the content as it is - return content - - -def collapse_into_shortcodes(content: HtmlElement) -> None: - """ - Replace everything in ``content`` which references another object by its shortcode. - - The tree is walked once and every element is passed through the cheap predicate of every - editable shortcode, so that only elements which really might be a reference cause the - database lookups needed to resolve them. - - :param content: The content which should be collapsed - """ - editable = _editable_shortcodes() - for element in list(content.iter()): - for shortcode in editable: - if shortcode.matches(element) and shortcode.collapse(element): - break +from .base import EditableShortcode, Shortcode +from .conversion import ( + collapse_into_shortcodes, + expand_shortcodes_for_cms, + expand_shortcodes_for_delivery, +) +from .registry import editable_shortcodes, get_shortcodes, register diff --git a/integreat_cms/cms/utils/shortcodes/base.py b/integreat_cms/cms/utils/shortcodes/base.py index 7df468f9e8..364eb2b044 100644 --- a/integreat_cms/cms/utils/shortcodes/base.py +++ b/integreat_cms/cms/utils/shortcodes/base.py @@ -1,5 +1,5 @@ """ -This module defines what a shortcode is and keeps the registry of all known shortcodes. +This module defines what a shortcode is. """ from __future__ import annotations @@ -25,7 +25,7 @@ class Shortcode(ABC): scoped form (``[block_keyword …]…[/block_keyword]``) which encloses content. Subclasses become known to the application by being decorated with - :func:`~integreat_cms.cms.utils.shortcodes.base.register`. + :func:`~integreat_cms.cms.utils.shortcodes.registry.register`. """ #: The keyword of the atomic form, for example ``page`` in ``[page 1]`` @@ -192,37 +192,6 @@ def collapse(self, element: HtmlElement) -> bool: """ -#: All registered shortcodes, in the order they were registered -_registry: list[Shortcode] = [] - - -def register[ShortcodeT: Shortcode](shortcode: type[ShortcodeT]) -> type[ShortcodeT]: - """ - Class decorator which makes a shortcode known to the application:: - - @register - class CatShortcode(Shortcode): - keyword = "cat" - - def expand(self, pargs, kwargs, context): - return "(=^･ω･^=)" - - :param shortcode: The shortcode to register - :return: The shortcode itself, so that this can be used as a decorator - """ - _registry.append(shortcode()) - return shortcode - - -def registered_shortcodes() -> tuple[Shortcode, ...]: - """ - Get the shortcodes which have been registered so far - - :return: The registered shortcodes - """ - return tuple(_registry) - - def _unparse(keyword: str, pargs: list[str], kwargs: dict[str, str]) -> str: """ Rebuild the source representation of a shortcode tag diff --git a/integreat_cms/cms/utils/shortcodes/contact.py b/integreat_cms/cms/utils/shortcodes/contact.py index 89ba7c4e77..bbf6d78944 100644 --- a/integreat_cms/cms/utils/shortcodes/contact.py +++ b/integreat_cms/cms/utils/shortcodes/contact.py @@ -9,7 +9,8 @@ from lxml.html import tostring from ..content_utils import render_contact_card -from .base import register, Shortcode +from .base import Shortcode +from .registry import register if TYPE_CHECKING: from typing import Any, Final diff --git a/integreat_cms/cms/utils/shortcodes/conversion.py b/integreat_cms/cms/utils/shortcodes/conversion.py new file mode 100644 index 0000000000..3d03a929e4 --- /dev/null +++ b/integreat_cms/cms/utils/shortcodes/conversion.py @@ -0,0 +1,144 @@ +""" +This module contains the conversions between shortcodes and the html they represent, in all +three directions: expansion for delivery, expansion for the CMS and collapsing back into +shortcodes. All three are implemented by the shortcodes themselves, see +:class:`~integreat_cms.cms.utils.shortcodes.base.Shortcode` and +:class:`~integreat_cms.cms.utils.shortcodes.base.EditableShortcode`. +""" + +from __future__ import annotations + +import logging +from functools import cache +from typing import TYPE_CHECKING + +import shortcodes + +from .registry import editable_shortcodes, get_shortcodes + +if TYPE_CHECKING: + from typing import Any + + from lxml.html import HtmlElement + +logger = logging.getLogger(__name__) + + +@cache +def _delivery_parser() -> shortcodes.Parser: + """ + Get the parser which expands shortcodes into the content delivered to end users + + :return: The parser + """ + parser = _build_parser() + for shortcode in get_shortcodes(): + parser.register(shortcode.expand, shortcode.keyword) + if shortcode.block_keyword: + parser.register( + shortcode.expand_block, + shortcode.block_keyword, + shortcode.block_end_keyword, + ) + return parser + + +@cache +def _cms_parser() -> shortcodes.Parser: + """ + Get the parser which expands shortcodes into the content presented in the CMS. + + Shortcodes which are not editable are not registered at all, so that they are kept + verbatim instead of being expanded into something the CMS could not collapse again. + + :return: The parser + """ + parser = _build_parser() + for shortcode in editable_shortcodes(): + parser.register(shortcode.expand_for_cms, shortcode.keyword) + if shortcode.block_keyword: + parser.register( + shortcode.expand_block_for_cms, + shortcode.block_keyword, + shortcode.block_end_keyword, + ) + return parser + + +def _build_parser() -> shortcodes.Parser: + """ + Build an empty parser which uses our shortcode syntax + + :return: The parser + """ + return shortcodes.Parser( + start="[", + end="]", + esc="\\", + inherit_globals=False, + ignore_unknown=True, + ) + + +def expand_shortcodes_for_delivery( + content: str, + context: dict[str, Any] | None = None, +) -> str: + """ + Replace all shortcodes in ``content`` by the representation delivered to end users + + :param content: The content as it is stored in the database + :param context: The context the shortcodes are expanded in + :return: The expanded content + """ + try: + return _delivery_parser().parse(content, context) + except shortcodes.ShortcodeError: + logger.warning( + "Failed expanding shortcodes in %r\ncontext: %r", + content, + context, + exc_info=True, + ) + # The best way to fail gracefully is to keep the content as it is + return content + + +def expand_shortcodes_for_cms(content: str, language_slug: str) -> str: + """ + Replace all editable shortcodes in ``content`` by the html they are edited as. + + Shortcodes which cannot be resolved are kept verbatim, so that editing content with a + broken reference does not silently drop that reference. + + :param content: The content as it is stored in the database + :param language_slug: The slug of the language the content should be presented in + :return: The content with expanded references + """ + try: + return _cms_parser().parse(content, {"language_slug": language_slug}) + except shortcodes.ShortcodeError: + logger.warning( + "Failed expanding shortcodes for the CMS in %r", + content, + exc_info=True, + ) + # The best way to fail gracefully is to keep the content as it is + return content + + +def collapse_into_shortcodes(content: HtmlElement) -> None: + """ + Replace everything in ``content`` which references another object by its shortcode. + + The tree is walked once and every element is passed through the cheap predicate of every + editable shortcode, so that only elements which really might be a reference cause the + database lookups needed to resolve them. + + :param content: The content which should be collapsed + """ + editable = editable_shortcodes() + for element in list(content.iter()): + for shortcode in editable: + if shortcode.matches(element) and shortcode.collapse(element): + break diff --git a/integreat_cms/cms/utils/shortcodes/page.py b/integreat_cms/cms/utils/shortcodes/page.py index f3a64c7d92..3e19a8d20f 100644 --- a/integreat_cms/cms/utils/shortcodes/page.py +++ b/integreat_cms/cms/utils/shortcodes/page.py @@ -7,8 +7,8 @@ from typing import TYPE_CHECKING from ...models import Page -from .base import register from .internal_link import InternalLinkShortcode +from .registry import register if TYPE_CHECKING: from typing import Final diff --git a/integreat_cms/cms/utils/shortcodes/registry.py b/integreat_cms/cms/utils/shortcodes/registry.py new file mode 100644 index 0000000000..c2be105d27 --- /dev/null +++ b/integreat_cms/cms/utils/shortcodes/registry.py @@ -0,0 +1,65 @@ +""" +This module keeps the registry of all known shortcodes. +""" + +from __future__ import annotations + +from functools import cache +from typing import TYPE_CHECKING + +from .base import EditableShortcode + +if TYPE_CHECKING: + from .base import Shortcode + + +#: All registered shortcodes, in the order they were registered +_registry: list[Shortcode] = [] + + +def register[ShortcodeT: Shortcode](shortcode: type[ShortcodeT]) -> type[ShortcodeT]: + """ + Class decorator which makes a shortcode known to the application:: + + @register + class CatShortcode(Shortcode): + keyword = "cat" + + def expand(self, pargs, kwargs, context): + return "(=^･ω･^=)" + + :param shortcode: The shortcode to register + :return: The shortcode itself, so that this can be used as a decorator + """ + _registry.append(shortcode()) + return shortcode + + +@cache +def get_shortcodes() -> tuple[Shortcode, ...]: + """ + Get all registered shortcodes. + + The modules which define them are imported here instead of at the top of this module, + because a shortcode may need anything from the models to the content utils, which in turn + need this package to collapse content into shortcodes. + + :return: The registered shortcodes + """ + from . import contact, page # noqa: F401 + + return tuple(_registry) + + +@cache +def editable_shortcodes() -> tuple[EditableShortcode, ...]: + """ + Get the shortcodes which are hidden from the users of the CMS + + :return: The editable shortcodes + """ + return tuple( + shortcode + for shortcode in get_shortcodes() + if isinstance(shortcode, EditableShortcode) + ) diff --git a/tests/cms/utils/test_link_shortcodes.py b/tests/cms/utils/test_link_shortcodes.py index 4972efffa9..d5e89d2554 100644 --- a/tests/cms/utils/test_link_shortcodes.py +++ b/tests/cms/utils/test_link_shortcodes.py @@ -36,7 +36,7 @@ def unpublish_page_3() -> None: def collapse(content: str) -> str: """ - Run :func:`~integreat_cms.cms.utils.shortcodes.collapse_into_shortcodes` + Run :func:`~integreat_cms.cms.utils.shortcodes.conversion.collapse_into_shortcodes` on a html string and return the result as a html string again """ element = fromstring(content)