diff --git a/integreat_cms/api/v3/events.py b/integreat_cms/api/v3/events.py index aa22ef0e79..f9ad042b8c 100644 --- a/integreat_cms/api/v3/events.py +++ b/integreat_cms/api/v3/events.py @@ -82,6 +82,7 @@ def transform_event_translation( else f"{event_translation.slug}${recurrence_date}" ) absolute_url = event_translation.url_prefix + slug + "/" + content = event_translation.content_for_delivery() return { "id": event_translation.id, "url": settings.BASE_URL + absolute_url, @@ -92,8 +93,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..88f51d93a3 100644 --- a/integreat_cms/api/v3/imprint.py +++ b/integreat_cms/api/v3/imprint.py @@ -32,6 +32,7 @@ def transform_imprint(imprint_translation: ImprintPageTranslation) -> dict[str, :return: data necessary for API """ absolute_url = imprint_translation.get_absolute_url() + content = imprint_translation.content_for_delivery() return { "id": imprint_translation.id, "url": settings.BASE_URL + absolute_url, @@ -39,8 +40,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..9cad662302 100644 --- a/integreat_cms/api/v3/locations.py +++ b/integreat_cms/api/v3/locations.py @@ -116,6 +116,7 @@ def transform_poi_translation( :return: Data for the APIv3 locations endpoint. """ poi = poi_translation.poi + content = poi_translation.content_for_delivery() contacts = Contact.objects.filter(location=poi).all() @@ -164,8 +165,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..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,6 +88,7 @@ def transform_page( organization = page.organization absolute_url = page_translation.get_absolute_url() + content = page_translation.content_for_delivery(request=request) return { "id": page_translation.id, "url": settings.BASE_URL + absolute_url, @@ -96,10 +99,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, @@ -168,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, []), ) ) @@ -269,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( @@ -355,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() @@ -428,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/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/api/v3/social_media_headers.py b/integreat_cms/api/v3/social_media_headers.py index 7f211db73e..18df956b76 100644 --- a/integreat_cms/api/v3/social_media_headers.py +++ b/integreat_cms/api/v3/social_media_headers.py @@ -228,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(page_translation.content), + excerpt=get_excerpt(page_translation.content_for_delivery()), url=page_translation.full_url, ) @@ -270,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(event_translation.content), + excerpt=get_excerpt(event_translation.content_for_delivery()), url=event_translation.full_url, ) @@ -342,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(location_translation.content), + 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 8771be7301..17970221db 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_for_cms + 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..3987d944af 100644 --- a/integreat_cms/cms/models/abstract_content_translation.py +++ b/integreat_cms/cms/models/abstract_content_translation.py @@ -539,6 +539,67 @@ 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_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. + + Everything which is saved back through + :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 + from ..utils.shortcodes import expand_shortcodes_for_cms + + return expand_shortcodes_for_cms(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_for_delivery + + return expand_shortcodes_for_delivery( + 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 b7840ea917..614c4b74b4 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_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 9770cd4d98..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|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 fac1afc8e1..3d9ae37eea 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_for_cms|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..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|safe }} + {{ poi_translation.content_for_cms|safe }}
{% endblock raw_content %} diff --git a/integreat_cms/cms/templatetags/content_filters.py b/integreat_cms/cms/templatetags/content_filters.py index b298153e2d..99161db04f 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.shortcodes import expand_shortcodes_for_cms 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_shortcodes_for_cms(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..65064038ab 100644 --- a/integreat_cms/cms/utils/content_utils.py +++ b/integreat_cms/cms/utils/content_utils.py @@ -15,6 +15,7 @@ from ..models import Contact, MediaFile from ..utils import internal_link_utils from ..utils.link_utils import fix_content_link_encoding +from ..utils.shortcodes import collapse_into_shortcodes logger = logging.getLogger(__name__) @@ -53,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) @@ -127,10 +129,14 @@ 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.conversion.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 content.iter("a"): + for link in list(content.iter("a")): mark_external_links(link) remove_target_attribute(link) update_internal_links(link, language_slug) @@ -331,37 +337,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/shortcodes/__init__.py b/integreat_cms/cms/utils/shortcodes/__init__.py index 2f91747633..d2a905e454 100644 --- a/integreat_cms/cms/utils/shortcodes/__init__.py +++ b/integreat_cms/cms/utils/shortcodes/__init__.py @@ -1,38 +1,32 @@ """ -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.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.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 +:class:`~integreat_cms.cms.utils.shortcodes.base.EditableShortcode`. """ -import logging -from typing import Any +from __future__ import annotations -import shortcodes -from django import template -from django.template.defaultfilters import stringfilter - -from .contact import contact -from .page import page - -logger = logging.getLogger(__name__) - -register = template.Library() - -# Needed context: -# - region -# - language -# - accessed path? -# - login status? -parser = shortcodes.Parser(start="[", end="]", esc="\\", ignore_unknown=True) - - -@register.filter -@stringfilter -def expand_shortcodes(content: str, context: dict[str, Any] | None = None) -> str: - try: - return parser.parse(content, context) - except shortcodes.ShortcodeError as e: - logger.warning( - "Failed expanding shortcodes: %s\ncontext: %r", e, context, exc_info=True - ) - # We failed expanding the shortcodes, - # the best way we can fail gracefully is to just return the original content - return content +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 new file mode 100644 index 0000000000..364eb2b044 --- /dev/null +++ b/integreat_cms/cms/utils/shortcodes/base.py @@ -0,0 +1,211 @@ +""" +This module defines what a shortcode is. +""" + +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.registry.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 + """ + + +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..bbf6d78944 100644 --- a/integreat_cms/cms/utils/shortcodes/contact.py +++ b/integreat_cms/cms/utils/shortcodes/contact.py @@ -1,18 +1,32 @@ -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 Shortcode +from .registry import register + +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 +42,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/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/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 49ad729da9..3e19a8d20f 100644 --- a/integreat_cms/cms/utils/shortcodes/page.py +++ b/integreat_cms/cms/utils/shortcodes/page.py @@ -1,65 +1,67 @@ -from typing import Any +""" +This module contains the shortcode which references a :class:`~integreat_cms.cms.models.pages.page.Page` +""" -from django.utils.translation import gettext_lazy as _ -from lxml.html import Element, fromstring, tostring +from __future__ import annotations -from ...models import Page, PageTranslation -from .utils import shortcode +from typing import TYPE_CHECKING +from ...models import Page +from .internal_link import InternalLinkShortcode +from .registry import register -@shortcode -def page( - pargs: list[str], - kwargs: dict[str, str], # noqa: ARG001 - context: dict[str, Any] | None, - content: str = "", # noqa: ARG001 -) -> str: +if TYPE_CHECKING: + from typing import Final + +#: 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"}, +) + + +@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. + + 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`` + - ``Willkommen`` * - ``[page 1 "this page"]`` - - ``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: - 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 - 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}]" - 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() - return tostring(element).decode("utf-8") + + keyword = "page" + block_keyword = "page_link" + model = Page + short_url_infix = "p" + + 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 + + :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/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/integreat_cms/cms/utils/shortcodes/utils.py b/integreat_cms/cms/utils/shortcodes/utils.py deleted file mode 100644 index 569cae2905..0000000000 --- a/integreat_cms/cms/utils/shortcodes/utils.py +++ /dev/null @@ -1,57 +0,0 @@ -from collections.abc import Callable - -import shortcodes - - -def shortcode[**P, R]( - tag: Callable[P, R] | str | 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/cms/views/pages/page_actions.py b/integreat_cms/cms/views/pages/page_actions.py index 65a89fae09..f7b67577fd 100644 --- a/integreat_cms/cms/views/pages/page_actions.py +++ b/integreat_cms/cms/views/pages/page_actions.py @@ -185,9 +185,9 @@ def preview_page_ajax( return JsonResponse( data={ "title": page_translation.title, - "page_translation": page_translation.content, + "page_translation": page_translation.content_for_cms, "mirrored_translation": ( - mirrored_translation.content if mirrored_translation else "" + mirrored_translation.content_for_cms if mirrored_translation else "" ), "mirrored_page_first": page.mirrored_page_first, "right_to_left": ( @@ -222,7 +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}) + 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 dab00fbf5e..cb6a15434b 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_for_cms): 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..1ab817a6d3 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.shortcodes import expand_shortcodes_for_cms + + ctx.translatable_attributes = [ + ( + attr, + expand_shortcodes_for_cms(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/integreat_cms/locale/de/LC_MESSAGES/django.po b/integreat_cms/locale/de/LC_MESSAGES/django.po index f809630c1a..455208c636 100644 --- a/integreat_cms/locale/de/LC_MESSAGES/django.po +++ b/integreat_cms/locale/de/LC_MESSAGES/django.po @@ -9021,9 +9021,9 @@ msgid "" "right\">" 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" 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_link_shortcodes.py b/tests/cms/utils/test_link_shortcodes.py new file mode 100644 index 0000000000..d5e89d2554 --- /dev/null +++ b/tests/cms/utils/test_link_shortcodes.py @@ -0,0 +1,404 @@ +""" +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.shortcodes import ( + collapse_into_shortcodes, + expand_shortcodes_for_cms, +) + +#: 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.shortcodes.conversion.collapse_into_shortcodes` + on a html string and return the result as a html string again + """ + element = fromstring(content) + collapse_into_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_shortcodes_for_cms("

[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_shortcodes_for_cms('

[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_shortcodes_for_cms("

[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_shortcodes_for_cms( + '

[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_shortcodes_for_cms("

[page 999999]

", "de") + == "

[page 999999]

" + ) + assert ( + expand_shortcodes_for_cms('

[page 999999 "hier"]

', "de") + == '

[page 999999 "hier"]

' + ) + assert ( + expand_shortcodes_for_cms("

[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_shortcodes_for_cms("

[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_shortcodes_for_cms("

[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_shortcodes_for_cms(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_shortcodes_for_cms(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"] + + +@pytest.mark.django_db +def test_expanded_content_is_not_cached(load_test_data: None) -> None: + """ + ``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 + ``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_for_cms == f'

hier

' + + translation.content = "

Neuer Inhalt

" + assert translation.content_for_cms == "

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_for_cms == "

Neuer Inhalt

" + + +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_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 new file mode 100644 index 0000000000..ae02490cae --- /dev/null +++ b/tests/cms/utils/test_shortcodes.py @@ -0,0 +1,181 @@ +""" +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_for_delivery + +#: 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_for_delivery("

[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_for_delivery('

[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_for_delivery("

[page 999999]

", DE) + == "

[MISSING LINK]

" + ) + assert ( + expand_shortcodes_for_delivery('

[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_for_delivery( + "

[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_for_delivery( + "

[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_for_delivery( + '

[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_for_delivery( + '

[page_link 1]Willkommen[/page_link]

', + DE, + ) + 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

' + )