Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions docs/plugins.rst
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,24 @@ Field plugins
value flows through the built-in for the field ``type`` (if any) and then each field plugin
attached to that ``type`` via ``config.field``, innermost-first by ``config.order``.

.. _plugins-shortcode-rendered-fields:

**A platzky plugin needs no goodmap frontend at all.** When a field name matches a shortcode
contributed by a loaded platzky plugin, ``prepare_pin`` also calls that shortcode's
``render_value`` and carries the result as ``html`` on the field value. If no first-party
renderer claims the ``type``, ``FieldRenderer`` seeds the fold with that HTML — so a plugin
displays correctly by shipping a Python shortcode alone: no Module Federation build, no
bundle to serve, no ``config.field`` to keep in sync. Field plugins below remain the way to
add behaviour goodmap's own React tree must participate in, and to wrap what a shortcode
rendered.

That HTML is rendered, not sanitized. It comes from an installed plugin package, which
already executes in the server process — the same trust platzky extends to shortcode output
in post content, and filtering it would block nothing such a package could not do more
directly. The plugin's side of that bargain is to escape the *data* it interpolates. A
first-party renderer always wins over ``html``, so a plugin cannot take over ``hyperlink``
or ``CTA``.

A field plugin is a ``MarkerFieldPluginBase`` whose component is a stage
``({ input, config }) => element`` — it receives the previous stage's output as ``input``.
There's one kind of field plugin; what it does with ``input`` is what makes it read as a
Expand Down
12 changes: 9 additions & 3 deletions frontend/src/components/MarkerPopup/FieldRenderer.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import React, { useReducer, useEffect } from 'react';
import PropTypes from 'prop-types';
import { getFieldPlugins, subscribe } from '../../plugins/pluginRegistry';
import getContentAsString from './fieldContent';
import { builtinFieldRenderers } from './builtinFieldRenderers';
import { builtinFieldRenderers, PluginHtmlField } from './builtinFieldRenderers';

/**
* Renders a marker field value as a pipe.
Expand All @@ -27,11 +27,17 @@ const FieldRenderer = ({ value }) => {
useEffect(() => subscribe(forceRender), []);

const type = value?.type;
const Builtin = type ? builtinFieldRenderers[type] : undefined;
const plugins = type ? getFieldPlugins(type) : [];

// The innermost stage renders the raw value. A first-party renderer for the type wins,
// so a plugin cannot shadow one; failing that, a shortcode that rendered the field
// itself seeds the fold with its own HTML, which is what lets a platzky plugin display
// without shipping any frontend code. Wrappers still wrap either one.
const Builtin = type ? builtinFieldRenderers[type] : undefined;
const Seed = Builtin ?? (value?.html ? PluginHtmlField : undefined);
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated

const stages = [
...(Builtin ? [{ Stage: Builtin, config: undefined }] : []),
...(Seed ? [{ Stage: Seed, config: undefined }] : []),
...plugins.map(({ Plugin, config }) => ({ Stage: Plugin, config })),
];

Expand Down
20 changes: 20 additions & 0 deletions frontend/src/components/MarkerPopup/builtinFieldRenderers.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,26 @@

CTAButtonField.propTypes = { input: fieldInputShape.isRequired };

/**
* Renders the HTML a platzky shortcode produced for its own field value.
*
* The markup is not sanitized, and deliberately so: it comes from an installed plugin
* package, which already runs arbitrary code in the server process — the same trust
* platzky extends to shortcode output in post content. Sanitizing would filter nothing a
* plugin could not do more directly, while breaking legitimate markup. The plugin's
* obligation in return is to escape the *data* it interpolates, which is untrusted.
*
* Never reached for a `type` that has a first-party renderer, so a plugin cannot use it
* to take over `hyperlink` or `CTA`.
*/
export const PluginHtmlField = ({ input }) => (
<span dangerouslySetInnerHTML={{ __html: input.html }} />

Check warning on line 86 in frontend/src/components/MarkerPopup/builtinFieldRenderers.jsx

View workflow job for this annotation

GitHub Actions / lint

Dangerous property 'dangerouslySetInnerHTML' found
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated
);

PluginHtmlField.propTypes = {
input: PropTypes.shape({ html: PropTypes.string.isRequired }).isRequired,
};

// Built-in field renderers, keyed by field `type`. Resolved before plugins so a
// plugin cannot shadow a first-party renderer (e.g. the URL-sanitizing link/button).
export const builtinFieldRenderers = {
Expand Down
74 changes: 74 additions & 0 deletions frontend/tests/Map/Map.test.jsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
import React from 'react';
import '@testing-library/jest-dom';
import { act } from '@testing-library/react';
import MapContainer from '../../src/components/Map/Map';

jest.mock('../../src/components/Map/MapComponent', () => () => <div data-testid="map-component" />);
jest.mock('../../src/components/FiltersForm/FiltersForm', () => () => (
<div data-testid="filters-form" />
));
jest.mock('../../src/components/common/AppToaster', () => () => null);
jest.mock('../../src/services/http/httpService', () => ({
__esModule: true,
default: {
getCategoriesData: jest.fn().mockResolvedValue({ categories: [], defaultChecked: {} }),
getLocationSchema: jest.fn().mockResolvedValue({}),
},
}));

// Map.jsx creates its own React root instead of being rendered by a test renderer,
// so act() has to be told this is an act environment.
globalThis.IS_REACT_ACT_ENVIRONMENT = true;

const renderApp = async () => {
await act(async () => {
MapContainer();
});
};

describe('MapWrap placeholders', () => {
let error;

// PropTypes' `node` validator does not recognise portals, so every render here
// warns about FiltersProvider's children (a pre-existing dev-only warning, not
// something these tests are about). console.error is silenced and asserted on by
// message instead of by call count.
beforeEach(() => {
error = jest.spyOn(console, 'error').mockImplementation(() => {});
});

afterEach(() => {
document.body.innerHTML = '';
jest.restoreAllMocks();
});

it('renders both portals when the left panel is present', async () => {
document.body.innerHTML = '<div id="map"></div><div id="filter-form"></div>';

await renderApp();

expect(document.querySelector('[data-testid="map-component"]')).not.toBeNull();
expect(document.querySelector('[data-testid="filters-form"]')).not.toBeNull();
});

// A deployment with no categories renders no left panel at all, so #filter-form is
// legitimately missing - the map must still come up rather than the whole app
// bailing out.
it('still renders the map when the filters placeholder is missing', async () => {
document.body.innerHTML = '<div id="map"></div>';
await renderApp();

expect(document.querySelector('[data-testid="map-component"]')).not.toBeNull();
Comment thread
coderabbitai[bot] marked this conversation as resolved.
expect(document.querySelector('[data-testid="filters-form"]')).toBeNull();
expect(error).not.toHaveBeenCalledWith(expect.stringContaining('render the map'));
});

it('renders nothing when the map placeholder is missing', async () => {
document.body.innerHTML = '<div id="filter-form"></div>';
await renderApp();

expect(document.querySelector('[data-testid="map-component"]')).toBeNull();
expect(document.querySelector('[data-testid="filters-form"]')).toBeNull();
expect(error).toHaveBeenCalledWith(expect.stringContaining('render the map'));
});
});
45 changes: 45 additions & 0 deletions frontend/tests/Map/map.config.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
import mapConfig from '../../src/components/Map/map.config';

// The view the frontend opened on before it became configurable, and still the fallback
// when a deployment ships no window.INITIAL_VIEW. Spelled out rather than imported from
// the module under test, so a change to it has to be made deliberately here too.
const DEFAULT_COORDINATES = [51.917, 19.013];
const DEFAULT_ZOOM = 7;
const DEFAULT_MAX_ZOOM = 19;

describe('mapConfig', () => {
afterEach(() => {
delete globalThis.INITIAL_VIEW;
});

it('takes the whole view from the deployment when one is provided', () => {
globalThis.INITIAL_VIEW = { center: [53.37, 22.89], zoom: 8, max_zoom: 17 };

expect(mapConfig.initialMapCoordinates).toEqual([53.37, 22.89]);
expect(mapConfig.initialMapZoom).toBe(8);
expect(mapConfig.maxMapZoom).toBe(17);
});

it('falls back to the whole of Poland when the global never arrives', () => {
expect(mapConfig.initialMapCoordinates).toEqual(DEFAULT_COORDINATES);
expect(mapConfig.initialMapZoom).toBe(DEFAULT_ZOOM);
expect(mapConfig.maxMapZoom).toBe(DEFAULT_MAX_ZOOM);
});

it('reads the global when the map mounts, not when the module is imported', () => {
// The module was imported at the top of this file, before any INITIAL_VIEW
// existed. A deployment whose template sets the global after the bundle loads
// must still get its own view rather than the fallback.
globalThis.INITIAL_VIEW = { center: [10, 20], zoom: 3, max_zoom: 12 };

expect(mapConfig.initialMapCoordinates).toEqual([10, 20]);
});

it('keeps a zoom of 0 rather than treating it as absent', () => {
// Goodmap always sends a complete view, so a zero here is a deliberate
// whole-world zoom - `||` would quietly replace it with the default.
globalThis.INITIAL_VIEW = { center: [0, 0], zoom: 0, max_zoom: 19 };

expect(mapConfig.initialMapZoom).toBe(0);
});
});
34 changes: 34 additions & 0 deletions frontend/tests/MarkerPopup/FieldRenderer.test.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,40 @@
expect(screen.getByText('SAVE20')).toBeInTheDocument();
});

it('renders shortcode-provided html when no first-party renderer exists', () => {
render(
<FieldRenderer
value={{ type: 'promocode', html: '<details><summary>Reveal</summary>SAVE20</details>' }}

Check failure on line 33 in frontend/tests/MarkerPopup/FieldRenderer.test.jsx

View workflow job for this annotation

GitHub Actions / lint

Replace `·type:·'promocode',·html:·'<details><summary>Reveal</summary>SAVE20</details>'` with `⏎····················type:·'promocode',⏎····················html:·'<details><summary>Reveal</summary>SAVE20</details>',⏎···············`
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated
/>,
);
expect(screen.getByText('Reveal')).toBeInTheDocument();
expect(screen.getByText('SAVE20')).toBeInTheDocument();
});

it('prefers a first-party renderer over shortcode-provided html', () => {
render(
<FieldRenderer
value={{
type: 'hyperlink',
value: 'https://example.com',
displayValue: 'Example',
html: '<b>hijacked</b>',
}}
/>,
);
expect(screen.getByRole('link', { name: 'Example' })).toBeInTheDocument();
expect(screen.queryByText('hijacked')).not.toBeInTheDocument();
});

it('lets a wrapper plugin wrap shortcode-provided html', () => {
const Wrapper = ({ input }) => <div data-testid="wrapper">{input}</div>;
Wrapper.propTypes = { input: PropTypes.node.isRequired };
act(() => registerPlugin('wrap', Wrapper, { field: 'wrapped', order: 1 }, 'MarkerField'));

render(<FieldRenderer value={{ type: 'wrapped', html: '<i>inner</i>' }} />);
expect(within(screen.getByTestId('wrapper')).getByText('inner')).toBeInTheDocument();
});

it('falls back to the field value when nothing renders the type', () => {
render(<FieldRenderer value={{ type: 'unknown', value: 'plain text' }} />);
expect(screen.getByText('plain text')).toBeInTheDocument();
Expand Down
29 changes: 26 additions & 3 deletions goodmap/formatter.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,28 @@ def safe_gettext(text):
return gettext(text)


def _shortcode_field(shortcode, value):
"""Build the marker-popup payload for a field backed by a platzky shortcode.

The shortcode renders the value itself, so a plugin needs no frontend code here to
be displayable — ``FieldRenderer`` seeds the field's fold with ``html`` when no
first-party renderer claims the ``type``. The entry's own keys travel alongside for
a React field plugin rendering from the data instead; a bare value is placed under
the shortcode's ``content_key`` so such a plugin finds it under the name the
shortcode uses. ``type`` is stamped last, so an entry cannot redirect its own field
at another renderer.

Args:
shortcode: The platzky Shortcode registered for this field name.
value: The field's value from the location data.

Returns:
dict: The field payload, carrying at least ``type`` and ``html``.
"""
entry = value if isinstance(value, dict) else {shortcode.content_key: value}
return {**entry, "type": shortcode.name, "html": shortcode.render_value(value)}


def prepare_pin(place, visible_fields, meta_data, shortcodes=None):
"""Prepare location data for map pin display with translations.

Expand All @@ -32,8 +54,9 @@ def prepare_pin(place, visible_fields, meta_data, shortcodes=None):
visible_fields: List of field names to display in pin
meta_data: List of metadata field names
shortcodes: Optional mapping of field name → Shortcode instance.
When a field name matches a shortcode, its value is transformed via
``shortcode.transform_field_value()`` before display.
When a field name matches a shortcode, the value is replaced by this
popup's field payload: the entry's own keys, the ``type`` the frontend
routes on, and ``html`` — the shortcode's own rendering of the value.

Returns:
dict: Formatted pin data with title, subtitle, position, metadata, and translated fields
Expand All @@ -45,7 +68,7 @@ def prepare_pin(place, visible_fields, meta_data, shortcodes=None):
continue
value = safe_gettext(place[field])
if field in plugins:
value = plugins[field].transform_field_value(value)
value = _shortcode_field(plugins[field], value)
data.append([gettext(field), value])
pin_data = {
"title": place["name"],
Expand Down
29 changes: 15 additions & 14 deletions goodmap/goodmap.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@
from flask_wtf.csrf import CSRFError
from platzky import platzky
from platzky.config import languages_dict
from platzky.content_types import ContentType
from platzky.models import CmsModule
from platzky.plugin.content_transformer import ContentTransformerPluginBase
from platzky.shortcodes import Shortcode

from goodmap.api.admin_api import admin_pages
Expand All @@ -33,6 +33,14 @@

_PLUGIN_ENTRY_POINT_GROUP = "goodmap.plugins"


# A value stored against a map marker and shown in its popup: a kind of content platzky
# does not have, so goodmap names it and registers it. A plugin opts in through
# ``accepted_content_types`` and an operator grants it through ``allowed_content_types``,
# exactly as for a post — and by name, so neither has to import goodmap to handle one.
MARKER_FIELD_CONTENT_TYPE: ContentType = "marker_field"


# Room above the attachment limit for a suggestion's text fields and multipart framing.
MULTIPART_OVERHEAD_ALLOWANCE = 100 * 1024

Expand Down Expand Up @@ -150,6 +158,7 @@ def create_app_from_config(config: GoodmapConfig) -> platzky.Engine:
config,
extra_plugin_bases=list(CAPABILITY_BASES),
extra_plugins_entrypoints=[_PLUGIN_ENTRY_POINT_GROUP],
extra_content_types=[MARKER_FIELD_CONTENT_TYPE],
)

frontend_static_dir = os.path.join(directory, "static", "frontend")
Expand Down Expand Up @@ -245,19 +254,11 @@ def handle_csrf_error(error):

photo_attachment_config = config.attachment

shortcodes: dict[str, Shortcode] = {}
for plugin in app.loaded_plugins:
if isinstance(plugin, ContentTransformerPluginBase):
for name, sc in plugin.shortcodes.items():
if name in shortcodes:
logger.warning(
"Shortcode '%s' from plugin '%s' conflicts with "
"an already-registered shortcode; skipping",
name,
type(plugin).__name__,
)
else:
shortcodes[name] = sc
# Only shortcodes whose plugin is both willing to handle marker fields and granted
# them by the operator. prepare_pin renders these through render_value, which does
# not pass through Engine.transform_content — collecting them off loaded_plugins
# instead would leave allowed_content_types governing posts but not popups.
shortcodes: dict[str, Shortcode] = app.shortcodes_for(MARKER_FIELD_CONTENT_TYPE)

cp = core_pages(
app.db,
Expand Down
Loading
Loading