From 95bf91593c7276c6cbba72a0da3e16fbc2d04365 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Krzysztof=20Ko=C5=82odzi=C5=84ski?= Date: Mon, 17 Aug 2026 22:22:53 +0200 Subject: [PATCH 01/28] feat: added markers styling --- e2e-tests/e2e_test_data_initial.json | 14 ++ e2e-tests/tests/basic/test_marker_styles.py | 76 ++++++++++ .../components/MarkerPopup/MarkerPopup.jsx | 14 +- .../MarkerPopup/getTypedMarkerIcon.jsx | 109 ++++++++++++++ .../MarkerPopup/getTypedMarkerIcon.test.jsx | 137 ++++++++++++++++++ goodmap/data_models/location.py | 22 ++- goodmap/db.py | 65 +++++++++ goodmap/goodmap.py | 6 + goodmap/templates/map.html | 2 + tests/unit_tests/data_models/test_location.py | 26 ++++ tests/unit_tests/test_core_api.py | 33 +++++ tests/unit_tests/test_db.py | 114 +++++++++++++++ tests/unit_tests/test_goodmap.py | 57 ++++++++ 13 files changed, 669 insertions(+), 6 deletions(-) create mode 100644 e2e-tests/tests/basic/test_marker_styles.py create mode 100644 frontend/src/components/MarkerPopup/getTypedMarkerIcon.jsx create mode 100644 frontend/tests/MarkerPopup/getTypedMarkerIcon.test.jsx diff --git a/e2e-tests/e2e_test_data_initial.json b/e2e-tests/e2e_test_data_initial.json index 17e8b58d..d894c6bb 100644 --- a/e2e-tests/e2e_test_data_initial.json +++ b/e2e-tests/e2e_test_data_initial.json @@ -272,6 +272,20 @@ "cars" ] }, + "marker_styles": { + "icon_field": "type_of_place", + "color_field": "speed_limit", + "icons": { + "big bridge": "M1 11h14v2H1zM2 7h1v4H2zM13 7h1v4h-1zM4 5h1v6H4zM11 5h1v6h-1zM7 4h2v7H7z", + "small bridge": "M2 9c2-3 10-3 12 0" + }, + "colors": { + "10": "#2e7d32", + "30": "#ef6c00", + "50": "#c62828" + }, + "default_color": "#2a81cb" + }, "visible_data": [ "remark", "accessible_by", diff --git a/e2e-tests/tests/basic/test_marker_styles.py b/e2e-tests/tests/basic/test_marker_styles.py new file mode 100644 index 00000000..d5f35f92 --- /dev/null +++ b/e2e-tests/tests/basic/test_marker_styles.py @@ -0,0 +1,76 @@ +""" +Marker Styles Tests + +Tests that the map picks pin icon/color per marker_styles (icon_field: +type_of_place, color_field: speed_limit - see e2e_test_data_initial.json), and +that a location with both a remark and a marker_styles match keeps its +type/color styling with an asterisk badge overlay, rather than losing it to +the plain asterisk icon (see getTypedMarkerIcon.jsx/MarkerPopup.jsx). +""" + +from playwright.sync_api import Page, expect + +from tests.conftest import BASE_URL, MARKER_LOAD_TIMEOUT, open_test_popup + +BIG_BRIDGE_GLYPH = "M1 11h14v2H1zM2 7h1v4H2zM13 7h1v4h-1zM4 5h1v6H4zM11 5h1v6h-1zM7 4h2v7H7z" +SMALL_BRIDGE_GLYPH = "M2 9c2-3 10-3 12 0" + + +class TestMarkerStyles: + """Test suite for marker_styles-driven pin icons/colors""" + + def test_fast_bridge_marker_uses_type_glyph_and_red_speed_color(self, page: Page): + """Pokoju (big bridge, speed_limit=50, no remark) is the only seeded bridge + with all three of lighting+benches+toilets (amenities is an "and" category - + see test_and_filter_within_category_narrows_results in test_map.py), so + checking all three isolates its marker without relying on clustering + distance/zoom assumptions.""" + page.goto(BASE_URL, wait_until="domcontentloaded") + + # "cars" is checked by default (Pokoju is cars-accessible); narrow further. + for amenity in ("lighting", "benches", "toilets"): + page.get_by_role("checkbox", name=amenity, exact=False).click() + + marker = page.locator(".custom-typed-marker-icon") + expect(marker).to_have_count(1, timeout=MARKER_LOAD_TIMEOUT) + + paths = marker.locator("path") + # First path is the pin shape itself, filled with speed_limit=50's color. + expect(paths.first).to_have_attribute("fill", "#c62828") + # Second path is the type_of_place glyph, configured for "big bridge". + expect(paths.nth(1)).to_have_attribute("d", BIG_BRIDGE_GLYPH) + # No remark on Pokoju, so no asterisk badge. + expect(marker.locator("text")).to_have_count(0) + + # Note: a second real-browser color case (e.g. speed_limit=10 -> green) isn't + # covered here. The only speed=10 bridge without a remark (Piaskowy) can't be + # isolated to a standalone marker via the left panel's filters - its amenities + # ([benches]) are a subset of a remarked neighbor's (Tumski, [lighting, + # benches]) barely 230m away, so any filter combo that includes Piaskowy also + # includes Tumski, and Leaflet.markercluster groups them into one cluster + # bubble at the map's default zoom, hiding both individual markers. The + # color-lookup logic itself (arbitrary field values, including a "10" -> + # green case) is covered generically at the unit level in + # frontend/tests/MarkerPopup/getTypedMarkerIcon.test.jsx. + + def test_remarked_bridge_keeps_type_and_color_styling_with_asterisk_badge(self, page: Page): + """Zwierzyniecka has both a remark and marker_styles-matching fields + (small bridge, speed_limit=10) - it should render its normal typed/colored + pin plus an asterisk badge, not fall back to the plain asterisk icon + (every type_of_place/speed_limit value happens to be covered by + marker_styles in this seeded dataset, so that plain-icon fallback path + isn't exercised here - it's covered at the unit level instead, see + getTypedMarkerIcon.test.jsx's "falls back to the plain asterisk icon" + case).""" + page.goto(BASE_URL, wait_until="domcontentloaded") + open_test_popup(page) + + expect(page.locator('img[alt="Marker-Asterisk"]')).to_have_count(0) + + marker = page.locator(".custom-typed-marker-icon") + expect(marker).to_have_count(1, timeout=MARKER_LOAD_TIMEOUT) + + paths = marker.locator("path") + expect(paths.first).to_have_attribute("fill", "#2e7d32") # speed_limit=10 + expect(paths.nth(1)).to_have_attribute("d", SMALL_BRIDGE_GLYPH) + expect(marker.locator("text")).to_have_text("*") diff --git a/frontend/src/components/MarkerPopup/MarkerPopup.jsx b/frontend/src/components/MarkerPopup/MarkerPopup.jsx index 6bec59fb..a48dead3 100644 --- a/frontend/src/components/MarkerPopup/MarkerPopup.jsx +++ b/frontend/src/components/MarkerPopup/MarkerPopup.jsx @@ -10,6 +10,7 @@ import useMapStore from '../Map/store/map.store'; import LocationDetailsBox from './LocationDetails'; import MobilePopup from './MobilePopup'; import DesktopPopup from './DesktopPopup'; +import { getTypedMarkerIcon } from './getTypedMarkerIcon'; import iconAsterisk from '../../res/img/marker-icon-asterisk.png'; /** @@ -122,9 +123,16 @@ const MarkerPopup = ({ place }) => { alt: place.has_remark ? 'Marker-Asterisk' : 'Marker', }; - // Only add icon prop if we have a custom icon (for remarks) - // This prevents passing undefined which can cause issues with MarkerClusterGroup - if (place.has_remark) { + // Prefer a marker_styles match (getTypedMarkerIcon adds an asterisk badge to + // it when place.has_remark is set, so a remarked location keeps its type/color + // styling) and only fall back to the plain asterisk icon when there's no + // match to style - e.g. a legacy/unconfigured deployment. Only add an icon + // prop when we actually have a custom icon: passing icon={undefined} causes + // errors in MarkerClusterGroup during cluster zoom animations. + const typedIcon = getTypedMarkerIcon(place); + if (typedIcon) { + markerProps.icon = typedIcon; + } else if (place.has_remark) { markerProps.icon = asteriskIcon; } diff --git a/frontend/src/components/MarkerPopup/getTypedMarkerIcon.jsx b/frontend/src/components/MarkerPopup/getTypedMarkerIcon.jsx new file mode 100644 index 00000000..92fb0482 --- /dev/null +++ b/frontend/src/components/MarkerPopup/getTypedMarkerIcon.jsx @@ -0,0 +1,109 @@ +import React from 'react'; +import PropTypes from 'prop-types'; +import { DivIcon } from 'leaflet'; +import ReactDOMServer from 'react-dom/server'; + +const PIN_WIDTH = 30; +const PIN_HEIGHT = 42; +const FALLBACK_COLOR = '#2a81cb'; // leaflet default marker blue + +/** + * Teardrop pin shape (matches Leaflet's default marker silhouette) filled with + * `color`, optionally holding a 16x16 glyph (`glyphPath`) centered near its top, + * and an asterisk badge in the pin's own color scheme when `hasRemark` is set - + * so a remarked location keeps its type/color styling instead of being replaced + * by a plain, uncolored asterisk marker. + */ +const PinSvg = ({ color, glyphPath, hasRemark }) => ( + + + {glyphPath !== '' && } + {hasRemark && ( + + * + + )} + +); + +PinSvg.propTypes = { + color: PropTypes.string.isRequired, + glyphPath: PropTypes.string.isRequired, + hasRemark: PropTypes.bool.isRequired, +}; + +/** + * Builds a Leaflet icon for `place` based on the deployment's marker styling + * lookup table (window.MARKER_STYLES, set server-side from the map's + * `marker_styles` config - see goodmap's db.get_marker_styles), or returns + * `null` when neither `icon_field` nor `color_field` produced a configured + * lookup match - callers should omit the `icon` prop in that case and fall + * back to Leaflet's default marker (or the plain asterisk icon for a remarked + * location with no marker_styles match) so unconfigured/legacy deployments + * are unchanged. When `place.has_remark` is set and a match *was* found, the + * returned icon carries an asterisk badge instead of losing its type/color + * styling to the plain asterisk marker. + * + * Expected shape of window.MARKER_STYLES: + * { + * icon_field: 'type_of_place', // which location field selects the glyph + * color_field: 'status', // which location field selects the fill color + * icons: { parcel_locker: 'M2 4h12v9H2z...' }, // field value -> SVG path (16x16 box) + * colors: { open: '#2e7d32' }, // field value -> fill color + * default_color: '#2a81cb', // fallback fill color + * } + * + * @param {Object} place - Location data, as returned by GET /api/locations + * @returns {import('leaflet').DivIcon|null} + */ +export const getTypedMarkerIcon = place => { + const markerStyles = globalThis.MARKER_STYLES || {}; + const { + icon_field: iconField, + color_field: colorField, + icons, + colors, + default_color: defaultColor, + } = markerStyles; + + const glyphPath = (iconField && icons && icons[place[iconField]]) || ''; + const matchedColor = (colorField && colors && colors[place[colorField]]) || ''; + + if (!glyphPath && !matchedColor) { + return null; + } + + return new DivIcon({ + html: ReactDOMServer.renderToString( + , + ), + className: 'custom-typed-marker-icon', + iconSize: [PIN_WIDTH, PIN_HEIGHT], + iconAnchor: [PIN_WIDTH / 2, PIN_HEIGHT], + popupAnchor: [0, -PIN_HEIGHT], + }); +}; diff --git a/frontend/tests/MarkerPopup/getTypedMarkerIcon.test.jsx b/frontend/tests/MarkerPopup/getTypedMarkerIcon.test.jsx new file mode 100644 index 00000000..e04dc73e --- /dev/null +++ b/frontend/tests/MarkerPopup/getTypedMarkerIcon.test.jsx @@ -0,0 +1,137 @@ +import { getTypedMarkerIcon } from '../../src/components/MarkerPopup/getTypedMarkerIcon'; + +// window.MARKER_STYLES is server-rendered JSON (see goodmap's map.html/db.get_marker_styles), +// so fixtures are parsed from JSON strings here too - keeps the snake_case backend field +// names (icon_field, color_field, default_color) faithful to what actually arrives. +const setMarkerStyles = json => { + globalThis.MARKER_STYLES = JSON.parse(json); +}; + +describe('getTypedMarkerIcon', () => { + afterEach(() => { + delete globalThis.MARKER_STYLES; + }); + + it('returns null when window.MARKER_STYLES is not set (legacy/unconfigured backend)', () => { + expect(getTypedMarkerIcon({ uuid: '1', position: [50, 50] })).toBeNull(); + }); + + it('returns null when window.MARKER_STYLES is set but empty (default db config)', () => { + setMarkerStyles('{}'); + expect(getTypedMarkerIcon({ uuid: '1', position: [50, 50] })).toBeNull(); + }); + + it('returns null when the place value has no matching icon or color entry', () => { + setMarkerStyles(`{ + "icon_field": "pointType", + "color_field": "pointStatus", + "icons": { "parcelLocker": "M0 0h16v16H0z" }, + "colors": { "open": "#2e7d32" } + }`); + + expect( + getTypedMarkerIcon({ uuid: '1', position: [50, 50], pointType: 'unknownType' }), + ).toBeNull(); + }); + + it('builds a DivIcon when the icon field matches a configured glyph', () => { + setMarkerStyles(`{ + "icon_field": "pointType", + "icons": { "parcelLocker": "M0 0h16v16H0z" } + }`); + + const icon = getTypedMarkerIcon({ + uuid: '1', + position: [50, 50], + pointType: 'parcelLocker', + }); + + expect(icon).not.toBeNull(); + expect(icon.options.html).toContain('M0 0h16v16H0z'); + expect(icon.options.html).toContain('#2a81cb'); // fallback color, no color_field set + expect(icon.options.iconSize).toEqual([30, 42]); + }); + + it('builds a DivIcon when the color field matches a configured color, with no glyph', () => { + setMarkerStyles(`{ + "color_field": "pointStatus", + "colors": { "open": "#2e7d32" } + }`); + + const icon = getTypedMarkerIcon({ uuid: '1', position: [50, 50], pointStatus: 'open' }); + + expect(icon).not.toBeNull(); + expect(icon.options.html).toContain('#2e7d32'); + }); + + it('picks the color matching each value on a multi-tier color_field (e.g. speed-based coloring)', () => { + setMarkerStyles(`{ + "color_field": "speedLimit", + "colors": { "10": "#2e7d32", "30": "#ef6c00", "50": "#c62828" } + }`); + + const iconFor = speedLimit => + getTypedMarkerIcon({ uuid: '1', position: [50, 50], speedLimit }); + + expect(iconFor('10').options.html).toContain('#2e7d32'); + expect(iconFor('30').options.html).toContain('#ef6c00'); + expect(iconFor('50').options.html).toContain('#c62828'); + }); + + it('adds an asterisk badge when place.has_remark is set and a match was found', () => { + setMarkerStyles(`{ + "icon_field": "pointType", + "icons": { "parcelLocker": "M0 0h16v16H0z" } + }`); + + const icon = getTypedMarkerIcon({ + uuid: '1', + position: [50, 50], + pointType: 'parcelLocker', + has_remark: true, + }); + + expect(icon).not.toBeNull(); + expect(icon.options.html).toContain('M0 0h16v16H0z'); // keeps the type glyph + expect(icon.options.html).toContain('>*'); // asterisk badge overlay + }); + + it('omits the asterisk badge when place.has_remark is not set', () => { + setMarkerStyles(`{ + "icon_field": "pointType", + "icons": { "parcelLocker": "M0 0h16v16H0z" } + }`); + + const icon = getTypedMarkerIcon({ + uuid: '1', + position: [50, 50], + pointType: 'parcelLocker', + }); + + expect(icon.options.html).not.toContain(' { + setMarkerStyles('{}'); + + expect( + getTypedMarkerIcon({ uuid: '1', position: [50, 50], has_remark: true }), + ).toBeNull(); + }); + + it('uses default_color from MARKER_STYLES when the matched value has no color entry', () => { + setMarkerStyles(`{ + "icon_field": "pointType", + "icons": { "parcelLocker": "M0 0h16v16H0z" }, + "default_color": "#123456" + }`); + + const icon = getTypedMarkerIcon({ + uuid: '1', + position: [50, 50], + pointType: 'parcelLocker', + }); + + expect(icon.options.html).toContain('#123456'); + }); +}); diff --git a/goodmap/data_models/location.py b/goodmap/data_models/location.py index 183ef652..ea8c4024 100644 --- a/goodmap/data_models/location.py +++ b/goodmap/data_models/location.py @@ -6,7 +6,7 @@ """ import warnings -from typing import Annotated, Any, Type, cast +from typing import Annotated, Any, ClassVar, Type, cast from annotated_types import Ge, Le from pydantic import ( @@ -37,6 +37,11 @@ class LocationBase(BaseModel, extra="allow"): uuid: str = Field(..., max_length=100) # TODO make this UUID and deprecate string remark: str | None = None + # Names of category fields whose values should ride along on basic_info(), + # e.g. so the frontend can pick a pin icon/color without a full detail fetch. + # Populated by create_location_model(); empty for the base class. + pin_marker_fields: ClassVar[frozenset[str]] = frozenset() + @model_validator(mode="before") @classmethod def validate_uuid_exists(cls, data: Any) -> Any: @@ -85,9 +90,18 @@ def model_dump(self, **kwargs) -> dict[str, Any]: return super().model_dump(**kwargs) def basic_info(self) -> dict[str, Any]: - """Get basic location information summary.""" + """Get basic location information summary. + + Includes the uuid/position/remark flag always shown on the map, plus the + value of any category field named in ``pin_marker_fields`` - enough for the + frontend to choose a pin icon/color without fetching full location detail. + """ data = self.model_dump(include={"uuid", "position"}) data["has_remark"] = bool(self.remark) + for field in sorted(self.pin_marker_fields): + value = getattr(self, field, None) + if value is not None: + data[field] = value return data @@ -255,9 +269,11 @@ def create_location_model( allowed = frozenset() fields[field_name] = _build_field_definition(field_type_str, allowed) - return create_model( + location_model = create_model( "Location", __base__=LocationBase, __module__="goodmap.data_models.location", **fields, ) + location_model.pin_marker_fields = frozenset(categories.keys()) & fields.keys() + return location_model diff --git a/goodmap/db.py b/goodmap/db.py index 7683861d..cdd3ec8b 100644 --- a/goodmap/db.py +++ b/goodmap/db.py @@ -562,6 +562,70 @@ def get_meta_data(db): return globals()[f"{db.module_name}_get_meta_data"] +# ------------------------------------------------ +# get_marker_styles + + +def google_json_db_get_marker_styles(self) -> dict[str, Any]: + """ + Retrieve marker icon/color styling configuration from Google Cloud Storage JSON blob. + + Returns: + dict: Pin styling lookup table (icon_field, color_field, icons, colors). + Returns empty dict if not found. + """ + return self.data.get("map", {}).get("marker_styles", {}) + + +def json_file_db_get_marker_styles(self) -> dict[str, Any]: + """ + Retrieve marker icon/color styling configuration from JSON file database. + + Returns: + dict: Pin styling lookup table (icon_field, color_field, icons, colors). + Returns empty dict if not found. + """ + return self.data.get("map", {}).get("marker_styles", {}) + + +def json_db_get_marker_styles(self) -> dict[str, Any]: + """ + Retrieve marker icon/color styling configuration from in-memory JSON database. + + Returns: + dict: Pin styling lookup table (icon_field, color_field, icons, colors). + Returns empty dict if not found. + """ + return self.data.get("marker_styles", {}) + + +def mongodb_db_get_marker_styles(self) -> dict[str, Any]: + """ + Retrieve marker icon/color styling configuration from MongoDB. + + Returns: + dict: Pin styling lookup table (icon_field, color_field, icons, colors). + Returns empty dict if config document not found or field missing. + """ + config_doc = self.db.config.find_one({"_id": "map_config"}) + if config_doc: + return config_doc.get("marker_styles", {}) + return {} + + +def get_marker_styles(db): + """ + Get the appropriate get_marker_styles function for the given database backend. + + Args: + db: Database instance (must have module_name attribute). + + Returns: + callable: Backend-specific get_marker_styles function. + """ + return globals()[f"{db.module_name}_get_marker_styles"] + + # ------------------------------------------------ # get_categories @@ -1777,6 +1841,7 @@ def extend_db_with_goodmap_queries(db, location_model): db.extend("get_data", get_data(db)) db.extend("get_visible_data", get_visible_data(db)) db.extend("get_meta_data", get_meta_data(db)) + db.extend("get_marker_styles", get_marker_styles(db)) db.extend("get_locations", get_locations(db, location_model)) db.extend("get_locations_paginated", get_locations_paginated(db, location_model)) db.extend("get_location", get_location(db, location_model)) diff --git a/goodmap/goodmap.py b/goodmap/goodmap.py index 64139404..56ee2173 100644 --- a/goodmap/goodmap.py +++ b/goodmap/goodmap.py @@ -285,11 +285,17 @@ def index(): Returns: Rendered map.html template with feature flags and the plugin manifest """ + try: + marker_styles = app.db.get_marker_styles() # type: ignore[attr-defined] + except (KeyError, AttributeError): + marker_styles = {} + return render_template( "map.html", feature_flags=config.feature_flags, goodmap_frontend_lib_url=config.goodmap_frontend_lib_url, plugin_manifest=plugin_manifest, + marker_styles=marker_styles, ) @goodmap.route("/goodmap-admin") diff --git a/goodmap/templates/map.html b/goodmap/templates/map.html index d1b1eacb..7ada444f 100644 --- a/goodmap/templates/map.html +++ b/goodmap/templates/map.html @@ -121,6 +121,8 @@ window.SHOW_ACCESSIBILITY_TABLE = {{ feature_flags.SHOW_ACCESSIBILITY_TABLE | default(false) | tojson }}; window.FEATURE_FLAGS = {{ feature_flags | tojson }}; window.PLUGIN_MANIFEST = {{ plugin_manifest | tojson }}; +// Deployment-specific pin icon/color lookup table, see goodmap/db.py's get_marker_styles. +window.MARKER_STYLES = {{ marker_styles | tojson }}; {% endblock %} diff --git a/tests/unit_tests/data_models/test_location.py b/tests/unit_tests/data_models/test_location.py index 3be93ab8..98e10d20 100644 --- a/tests/unit_tests/data_models/test_location.py +++ b/tests/unit_tests/data_models/test_location.py @@ -129,6 +129,32 @@ def test_category_validation_rejects_invalid_list_item(): location_model(uuid="2", tags=["red", "yellow"], position=(50, 50)) +def test_basic_info_includes_category_field_values(): + """basic_info() should surface category field values (for pin icon/color + selection) alongside the existing uuid/position/remark.""" + location_model = create_location_model( + obligatory_fields=[("type_of_place", "str"), ("name", "str")], + categories={"type_of_place": ["parcel_locker", "container"]}, + ) + location = location_model( + uuid="1", name="test", type_of_place="parcel_locker", position=(50, 50) + ) + assert location.basic_info() == { + "uuid": "1", + "position": (50, 50), + "remark": False, + "type_of_place": "parcel_locker", + } + + +def test_basic_info_omits_category_fields_when_none_configured(): + """Backward compatibility: deployments without categories get the original + uuid/position/remark shape, unchanged.""" + location_model = create_location_model(obligatory_fields=[("name", "str")], categories={}) + location = location_model(uuid="1", name="test", position=(50, 50)) + assert location.basic_info() == {"uuid": "1", "position": (50, 50), "remark": False} + + def test_create_location_model_with_int_field(): """Test that non-str simple fields (like int) are created without max_length.""" location_model = create_location_model(obligatory_fields=[("capacity", "int")], categories={}) diff --git a/tests/unit_tests/test_core_api.py b/tests/unit_tests/test_core_api.py index 6dbb03e5..2a7ec185 100644 --- a/tests/unit_tests/test_core_api.py +++ b/tests/unit_tests/test_core_api.py @@ -315,6 +315,39 @@ def test_get_locations_accepts_valid_and_undeclared_parameters(test_app, query): assert response.status_code == 200 +def test_get_locations_includes_category_field_for_pin_styling(): + """/api/locations should surface category field values (e.g. a point-type + category), so the frontend can pick a marker icon/color without a full + per-location detail fetch.""" + client = create_test_app( + db_overrides={ + "categories": {"point_type": ["parcel_locker", "container"]}, + "location_obligatory_fields": [("point_type", "str"), ("name", "str")], + "data": [ + { + "name": "locker-1", + "position": [50, 50], + "point_type": "parcel_locker", + "uuid": "11111111-1111-1111-1111-111111111111", + }, + ], + "visible_data": ["name", "point_type"], + } + ) + + response = client.get("/api/locations") + + assert response.status_code == 200 + assert response.json == [ + { + "uuid": "11111111-1111-1111-1111-111111111111", + "position": [50, 50], + "has_remark": False, + "point_type": "parcel_locker", + }, + ] + + def test_get_locations_multi_value_same_category_uses_or_semantics(): """Selecting several checkboxes within one category should return the union of matches, not only entries that have every selected value.""" diff --git a/tests/unit_tests/test_db.py b/tests/unit_tests/test_db.py index d915ce2a..29616092 100644 --- a/tests/unit_tests/test_db.py +++ b/tests/unit_tests/test_db.py @@ -26,6 +26,7 @@ google_json_db_get_data, google_json_db_get_location_obligatory_fields, google_json_db_get_locations_paginated, + google_json_db_get_marker_styles, google_json_db_get_meta_data, google_json_db_get_visible_data, json_db_add_location, @@ -57,6 +58,7 @@ json_file_db_get_data, json_file_db_get_location_obligatory_fields, json_file_db_get_locations_paginated, + json_file_db_get_marker_styles, json_file_db_get_meta_data, json_file_db_get_report, json_file_db_get_reports, @@ -81,6 +83,7 @@ mongodb_db_get_location_obligatory_fields, mongodb_db_get_locations, mongodb_db_get_locations_paginated, + mongodb_db_get_marker_styles, mongodb_db_get_meta_data, mongodb_db_get_report, mongodb_db_get_reports, @@ -296,6 +299,41 @@ def test_json_file_db_get_meta_data_empty(): assert result == {} +@mock.patch( + "builtins.open", + mock.mock_open( + read_data=json.dumps( + { + "map": { + "marker_styles": { + "icon_field": "type_of_place", + "color_field": "status", + "icons": {"parcel_locker": "M0 0h16v16H0z"}, + "colors": {"open": "#2e7d32"}, + } + } + } + ) + ), +) +def test_json_file_db_get_marker_styles(): + db = JsonFile("/fake/path/data.json") + result = json_file_db_get_marker_styles(db) + assert result == { + "icon_field": "type_of_place", + "color_field": "status", + "icons": {"parcel_locker": "M0 0h16v16H0z"}, + "colors": {"open": "#2e7d32"}, + } + + +@mock.patch("builtins.open", mock.mock_open(read_data=json.dumps({"map": {}}))) +def test_json_file_db_get_marker_styles_empty(): + db = JsonFile("/fake/path/data.json") + result = json_file_db_get_marker_styles(db) + assert result == {} + + # Test get_visible_data and get_meta_data for google_json_db @mock.patch("platzky.db.google_json_db.Client") def test_google_json_db_get_visible_data(mock_cli): @@ -337,6 +375,38 @@ def test_google_json_db_get_meta_data_empty(mock_cli): assert result == {} +@mock.patch("platzky.db.google_json_db.Client") +def test_google_json_db_get_marker_styles(mock_cli): + mock_cli.return_value.bucket.return_value.blob.return_value.download_as_text.return_value = ( + json.dumps( + { + "map": { + "marker_styles": { + "icon_field": "type_of_place", + "icons": {"parcel_locker": "M0 0h16v16H0z"}, + } + } + } + ) + ) + db = GoogleJsonDb("bucket", "blob") + result = google_json_db_get_marker_styles(db) + assert result == { + "icon_field": "type_of_place", + "icons": {"parcel_locker": "M0 0h16v16H0z"}, + } + + +@mock.patch("platzky.db.google_json_db.Client") +def test_google_json_db_get_marker_styles_empty(mock_cli): + mock_cli.return_value.bucket.return_value.blob.return_value.download_as_text.return_value = ( + json.dumps({"map": {}}) + ) + db = GoogleJsonDb("bucket", "blob") + result = google_json_db_get_marker_styles(db) + assert result == {} + + def test_get_location_from_raw_data_found(): raw = {"data": [{"uuid": "X", "position": [0, 0]}]} Location = create_location_model([], {}) @@ -1092,6 +1162,50 @@ def test_mongodb_db_get_meta_data_no_config(mock_client): assert result == {} +@mock.patch("platzky.db.mongodb_db.MongoClient") +def test_mongodb_db_get_marker_styles(mock_client): + mock_db = mock.Mock() + mock_client.return_value.__getitem__.return_value = mock_db + mock_db.config.find_one.return_value = { + "_id": "map_config", + "marker_styles": { + "icon_field": "type_of_place", + "icons": {"parcel_locker": "M0 0h16v16H0z"}, + }, + } + + db = MongoDB("mongodb://localhost:27017", "test_db") + result = mongodb_db_get_marker_styles(db) + assert result == { + "icon_field": "type_of_place", + "icons": {"parcel_locker": "M0 0h16v16H0z"}, + } + + +@mock.patch("platzky.db.mongodb_db.MongoClient") +def test_mongodb_db_get_marker_styles_empty(mock_client): + mock_db = mock.Mock() + mock_client.return_value.__getitem__.return_value = mock_db + mock_db.config.find_one.return_value = { + "_id": "map_config", + } + + db = MongoDB("mongodb://localhost:27017", "test_db") + result = mongodb_db_get_marker_styles(db) + assert result == {} + + +@mock.patch("platzky.db.mongodb_db.MongoClient") +def test_mongodb_db_get_marker_styles_no_config(mock_client): + mock_db = mock.Mock() + mock_client.return_value.__getitem__.return_value = mock_db + mock_db.config.find_one.return_value = None + + db = MongoDB("mongodb://localhost:27017", "test_db") + result = mongodb_db_get_marker_styles(db) + assert result == {} + + @mock.patch("platzky.db.mongodb_db.MongoClient") def test_mongodb_db_get_location(mock_client): mock_db = mock.Mock() diff --git a/tests/unit_tests/test_goodmap.py b/tests/unit_tests/test_goodmap.py index 5fab3be2..f8a4766d 100644 --- a/tests/unit_tests/test_goodmap.py +++ b/tests/unit_tests/test_goodmap.py @@ -107,6 +107,63 @@ def test_frontend_lib_url_uses_bundled_static_when_present(): assert 'src="/static/frontend/index.min.js"' in response.data.decode("utf-8") +def test_map_route_includes_marker_styles(): + """The frontend picks pin icon/color per marker_styles.config's iconField/colorField + at runtime from window.MARKER_STYLES - a deployment-specific lookup table that lives + in the database (like categories/visible_data), not hardcoded in the frontend build.""" + config = GoodmapConfig( + APP_NAME="test_app", + SECRET_KEY="test_secret", + USE_WWW=False, + BLOG_PREFIX="/blog", + DB=JsonDbConfig( + DATA={ + "site_content": {"pages": []}, + "categories": {"type_of_place": ["parcel_locker", "container"]}, + "marker_styles": { + "icon_field": "type_of_place", + "icons": {"parcel_locker": "M0 0h16v16H0z"}, + "colors": {}, + }, + }, + TYPE="json", + ), + ) + app = goodmap.create_app_from_config(config) + app.config["WTF_CSRF_ENABLED"] = False # NOSONAR + client = app.test_client() + + response = client.get("/map") + assert response.status_code == 200 + + response_text = response.data.decode("utf-8") + assert "MARKER_STYLES" in response_text + assert "icon_field" in response_text + assert "parcel_locker" in response_text + + +def test_map_route_marker_styles_defaults_to_empty(): + """Deployments that don't configure marker_styles get an empty object, so the + frontend falls back to Leaflet's default marker - no behavior change.""" + config = GoodmapConfig( + APP_NAME="test_app", + SECRET_KEY="test_secret", + USE_WWW=False, + BLOG_PREFIX="/blog", + DB=JsonDbConfig( + DATA={"site_content": {"pages": []}, "categories": {}}, + TYPE="json", + ), + ) + app = goodmap.create_app_from_config(config) + app.config["WTF_CSRF_ENABLED"] = False # NOSONAR + client = app.test_client() + + response = client.get("/map") + assert response.status_code == 200 + assert "window.MARKER_STYLES={};" in response.data.decode("utf-8") + + def test_map_route_includes_photo_constraints(): """The frontend sources photo upload limits (max size, allowed types) live from the backend's AttachmentConfig rather than hardcoding its own copy - this test From c50fa7dca70a0f61f6186e01626fdd675bb30e91 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Krzysztof=20Ko=C5=82odzi=C5=84ski?= Date: Thu, 20 Aug 2026 00:56:24 +0200 Subject: [PATCH 02/28] fixes --- frontend/src/components/MarkerPopup/ReportProblemForm.jsx | 2 +- tests/unit_tests/data_models/test_location.py | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/frontend/src/components/MarkerPopup/ReportProblemForm.jsx b/frontend/src/components/MarkerPopup/ReportProblemForm.jsx index 6c46004a..ec66f7b6 100644 --- a/frontend/src/components/MarkerPopup/ReportProblemForm.jsx +++ b/frontend/src/components/MarkerPopup/ReportProblemForm.jsx @@ -212,7 +212,7 @@ const ReportProblemForm = ({ placeId }) => { if (schemaError) { return ( - {t('loadReportFormError')} + {t('loadReportFormError')}
{t('retry')} diff --git a/tests/unit_tests/data_models/test_location.py b/tests/unit_tests/data_models/test_location.py index 98e10d20..0330733a 100644 --- a/tests/unit_tests/data_models/test_location.py +++ b/tests/unit_tests/data_models/test_location.py @@ -131,7 +131,7 @@ def test_category_validation_rejects_invalid_list_item(): def test_basic_info_includes_category_field_values(): """basic_info() should surface category field values (for pin icon/color - selection) alongside the existing uuid/position/remark.""" + selection) alongside the existing uuid/position/has_remark.""" location_model = create_location_model( obligatory_fields=[("type_of_place", "str"), ("name", "str")], categories={"type_of_place": ["parcel_locker", "container"]}, @@ -142,17 +142,17 @@ def test_basic_info_includes_category_field_values(): assert location.basic_info() == { "uuid": "1", "position": (50, 50), - "remark": False, + "has_remark": False, "type_of_place": "parcel_locker", } def test_basic_info_omits_category_fields_when_none_configured(): """Backward compatibility: deployments without categories get the original - uuid/position/remark shape, unchanged.""" + uuid/position/has_remark shape, unchanged.""" location_model = create_location_model(obligatory_fields=[("name", "str")], categories={}) location = location_model(uuid="1", name="test", position=(50, 50)) - assert location.basic_info() == {"uuid": "1", "position": (50, 50), "remark": False} + assert location.basic_info() == {"uuid": "1", "position": (50, 50), "has_remark": False} def test_create_location_model_with_int_field(): From 242cb5f5943e2dba76d6d712fedac1d320bebe77 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Krzysztof=20Ko=C5=82odzi=C5=84ski?= Date: Thu, 20 Aug 2026 01:22:17 +0200 Subject: [PATCH 03/28] fix popup --- e2e-tests/e2e_test_data_initial.json | 4 +- e2e-tests/tests/basic/test_marker_styles.py | 31 +++-- .../components/MarkerPopup/MarkerPopup.jsx | 2 +- .../MarkerPopup/getTypedMarkerIcon.jsx | 106 +++++++++++------- .../MarkerPopup/getTypedMarkerIcon.test.jsx | 45 ++++++-- 5 files changed, 125 insertions(+), 63 deletions(-) diff --git a/e2e-tests/e2e_test_data_initial.json b/e2e-tests/e2e_test_data_initial.json index d894c6bb..8af116ba 100644 --- a/e2e-tests/e2e_test_data_initial.json +++ b/e2e-tests/e2e_test_data_initial.json @@ -276,8 +276,8 @@ "icon_field": "type_of_place", "color_field": "speed_limit", "icons": { - "big bridge": "M1 11h14v2H1zM2 7h1v4H2zM13 7h1v4h-1zM4 5h1v6H4zM11 5h1v6h-1zM7 4h2v7H7z", - "small bridge": "M2 9c2-3 10-3 12 0" + "big bridge": "https://cdn.jsdelivr.net/npm/@phosphor-icons/core@2/assets/fill/bridge-fill.svg", + "small bridge": "https://cdn.jsdelivr.net/npm/@phosphor-icons/core@2/assets/fill/footprints-fill.svg" }, "colors": { "10": "#2e7d32", diff --git a/e2e-tests/tests/basic/test_marker_styles.py b/e2e-tests/tests/basic/test_marker_styles.py index d5f35f92..4027bea8 100644 --- a/e2e-tests/tests/basic/test_marker_styles.py +++ b/e2e-tests/tests/basic/test_marker_styles.py @@ -12,8 +12,16 @@ from tests.conftest import BASE_URL, MARKER_LOAD_TIMEOUT, open_test_popup -BIG_BRIDGE_GLYPH = "M1 11h14v2H1zM2 7h1v4H2zM13 7h1v4h-1zM4 5h1v6H4zM11 5h1v6h-1zM7 4h2v7H7z" -SMALL_BRIDGE_GLYPH = "M2 9c2-3 10-3 12 0" +# "big bridge" and "small bridge" each get their own Phosphor Icons (MIT) glyph - +# see e2e_test_data_initial.json's marker_styles.icons and getTypedMarkerIcon.jsx +# (icon URLs are CSS mask-image'd onto the pin, tinted by the matched color, +# rather than embedded as inline SVG path data). +BIG_BRIDGE_GLYPH_URL = ( + "https://cdn.jsdelivr.net/npm/@phosphor-icons/core@2/assets/fill/bridge-fill.svg" +) +SMALL_BRIDGE_GLYPH_URL = ( + "https://cdn.jsdelivr.net/npm/@phosphor-icons/core@2/assets/fill/footprints-fill.svg" +) class TestMarkerStyles: @@ -34,11 +42,13 @@ def test_fast_bridge_marker_uses_type_glyph_and_red_speed_color(self, page: Page marker = page.locator(".custom-typed-marker-icon") expect(marker).to_have_count(1, timeout=MARKER_LOAD_TIMEOUT) - paths = marker.locator("path") - # First path is the pin shape itself, filled with speed_limit=50's color. - expect(paths.first).to_have_attribute("fill", "#c62828") - # Second path is the type_of_place glyph, configured for "big bridge". - expect(paths.nth(1)).to_have_attribute("d", BIG_BRIDGE_GLYPH) + # The pin shape itself, filled with speed_limit=50's color. + expect(marker.locator("path")).to_have_attribute("fill", "#c62828") + # The type_of_place glyph, configured for "big bridge" - masked onto a div + # via CSS rather than embedded as an inline . + glyph = marker.locator(".custom-typed-marker-glyph") + expect(glyph).to_have_count(1) + expect(glyph).to_have_css("mask-image", f'url("{BIG_BRIDGE_GLYPH_URL}")') # No remark on Pokoju, so no asterisk badge. expect(marker.locator("text")).to_have_count(0) @@ -70,7 +80,8 @@ def test_remarked_bridge_keeps_type_and_color_styling_with_asterisk_badge(self, marker = page.locator(".custom-typed-marker-icon") expect(marker).to_have_count(1, timeout=MARKER_LOAD_TIMEOUT) - paths = marker.locator("path") - expect(paths.first).to_have_attribute("fill", "#2e7d32") # speed_limit=10 - expect(paths.nth(1)).to_have_attribute("d", SMALL_BRIDGE_GLYPH) + expect(marker.locator("path")).to_have_attribute("fill", "#2e7d32") # speed_limit=10 + glyph = marker.locator(".custom-typed-marker-glyph") + expect(glyph).to_have_count(1) + expect(glyph).to_have_css("mask-image", f'url("{SMALL_BRIDGE_GLYPH_URL}")') expect(marker.locator("text")).to_have_text("*") diff --git a/frontend/src/components/MarkerPopup/MarkerPopup.jsx b/frontend/src/components/MarkerPopup/MarkerPopup.jsx index a48dead3..b18e5710 100644 --- a/frontend/src/components/MarkerPopup/MarkerPopup.jsx +++ b/frontend/src/components/MarkerPopup/MarkerPopup.jsx @@ -10,7 +10,7 @@ import useMapStore from '../Map/store/map.store'; import LocationDetailsBox from './LocationDetails'; import MobilePopup from './MobilePopup'; import DesktopPopup from './DesktopPopup'; -import { getTypedMarkerIcon } from './getTypedMarkerIcon'; +import getTypedMarkerIcon from './getTypedMarkerIcon'; import iconAsterisk from '../../res/img/marker-icon-asterisk.png'; /** diff --git a/frontend/src/components/MarkerPopup/getTypedMarkerIcon.jsx b/frontend/src/components/MarkerPopup/getTypedMarkerIcon.jsx index 92fb0482..1d4a2588 100644 --- a/frontend/src/components/MarkerPopup/getTypedMarkerIcon.jsx +++ b/frontend/src/components/MarkerPopup/getTypedMarkerIcon.jsx @@ -7,48 +7,74 @@ const PIN_WIDTH = 30; const PIN_HEIGHT = 42; const FALLBACK_COLOR = '#2a81cb'; // leaflet default marker blue +const GLYPH_SIZE = 16; +const GLYPH_OFFSET_TOP = 6; +const GLYPH_OFFSET_LEFT = 7; + /** * Teardrop pin shape (matches Leaflet's default marker silhouette) filled with - * `color`, optionally holding a 16x16 glyph (`glyphPath`) centered near its top, - * and an asterisk badge in the pin's own color scheme when `hasRemark` is set - - * so a remarked location keeps its type/color styling instead of being replaced - * by a plain, uncolored asterisk marker. + * `color`, optionally holding a glyph (`glyphUrl`, an icon image masked to the + * pin's own color via CSS mask-image so it doesn't need to be fetched or + * recolored server-side) centered near its top, and an asterisk badge in the + * pin's own color scheme when `hasRemark` is set - so a remarked location + * keeps its type/color styling instead of being replaced by a plain, + * uncolored asterisk marker. */ -const PinSvg = ({ color, glyphPath, hasRemark }) => ( - - - {glyphPath !== '' && } - {hasRemark && ( - - * - +const PinSvg = ({ color, glyphUrl, hasRemark }) => ( +
+ + + {hasRemark && ( + + * + + )} + + {glyphUrl !== '' && ( +
)} - +
); PinSvg.propTypes = { color: PropTypes.string.isRequired, - glyphPath: PropTypes.string.isRequired, + glyphUrl: PropTypes.string.isRequired, hasRemark: PropTypes.bool.isRequired, }; @@ -68,7 +94,7 @@ PinSvg.propTypes = { * { * icon_field: 'type_of_place', // which location field selects the glyph * color_field: 'status', // which location field selects the fill color - * icons: { parcel_locker: 'M2 4h12v9H2z...' }, // field value -> SVG path (16x16 box) + * icons: { parcel_locker: 'https://cdn.example.com/parcel-locker.svg' }, // field value -> icon URL, masked+tinted via CSS (see PinSvg) * colors: { open: '#2e7d32' }, // field value -> fill color * default_color: '#2a81cb', // fallback fill color * } @@ -76,7 +102,7 @@ PinSvg.propTypes = { * @param {Object} place - Location data, as returned by GET /api/locations * @returns {import('leaflet').DivIcon|null} */ -export const getTypedMarkerIcon = place => { +const getTypedMarkerIcon = place => { const markerStyles = globalThis.MARKER_STYLES || {}; const { icon_field: iconField, @@ -86,10 +112,10 @@ export const getTypedMarkerIcon = place => { default_color: defaultColor, } = markerStyles; - const glyphPath = (iconField && icons && icons[place[iconField]]) || ''; + const glyphUrl = (iconField && icons && icons[place[iconField]]) || ''; const matchedColor = (colorField && colors && colors[place[colorField]]) || ''; - if (!glyphPath && !matchedColor) { + if (!glyphUrl && !matchedColor) { return null; } @@ -97,7 +123,7 @@ export const getTypedMarkerIcon = place => { html: ReactDOMServer.renderToString( , ), @@ -107,3 +133,5 @@ export const getTypedMarkerIcon = place => { popupAnchor: [0, -PIN_HEIGHT], }); }; + +export default getTypedMarkerIcon; diff --git a/frontend/tests/MarkerPopup/getTypedMarkerIcon.test.jsx b/frontend/tests/MarkerPopup/getTypedMarkerIcon.test.jsx index e04dc73e..9c73c81a 100644 --- a/frontend/tests/MarkerPopup/getTypedMarkerIcon.test.jsx +++ b/frontend/tests/MarkerPopup/getTypedMarkerIcon.test.jsx @@ -1,4 +1,4 @@ -import { getTypedMarkerIcon } from '../../src/components/MarkerPopup/getTypedMarkerIcon'; +import getTypedMarkerIcon from '../../src/components/MarkerPopup/getTypedMarkerIcon'; // window.MARKER_STYLES is server-rendered JSON (see goodmap's map.html/db.get_marker_styles), // so fixtures are parsed from JSON strings here too - keeps the snake_case backend field @@ -25,7 +25,7 @@ describe('getTypedMarkerIcon', () => { setMarkerStyles(`{ "icon_field": "pointType", "color_field": "pointStatus", - "icons": { "parcelLocker": "M0 0h16v16H0z" }, + "icons": { "parcelLocker": "https://cdn.example.com/parcel-locker.svg" }, "colors": { "open": "#2e7d32" } }`); @@ -37,7 +37,7 @@ describe('getTypedMarkerIcon', () => { it('builds a DivIcon when the icon field matches a configured glyph', () => { setMarkerStyles(`{ "icon_field": "pointType", - "icons": { "parcelLocker": "M0 0h16v16H0z" } + "icons": { "parcelLocker": "https://cdn.example.com/parcel-locker.svg" } }`); const icon = getTypedMarkerIcon({ @@ -47,11 +47,36 @@ describe('getTypedMarkerIcon', () => { }); expect(icon).not.toBeNull(); - expect(icon.options.html).toContain('M0 0h16v16H0z'); + expect(icon.options.html).toContain('https://cdn.example.com/parcel-locker.svg'); expect(icon.options.html).toContain('#2a81cb'); // fallback color, no color_field set expect(icon.options.iconSize).toEqual([30, 42]); }); + it('masks the icon URL through CSS so it picks up the matched color, instead of embedding SVG path data', () => { + setMarkerStyles(`{ + "icon_field": "pointType", + "color_field": "pointStatus", + "icons": { "parcelLocker": "https://cdn.example.com/parcel-locker.svg" }, + "colors": { "open": "#2e7d32" } + }`); + + const icon = getTypedMarkerIcon({ + uuid: '1', + position: [50, 50], + pointType: 'parcelLocker', + pointStatus: 'open', + }); + + // the glyph URL drives a CSS mask (mask-image / -webkit-mask-image) on a + //
, not an inline , so any icon set (not just + // single-path ones) works and no extra is added beyond the pin + // body's own teardrop shape. + expect(icon.options.html).toContain( + 'mask-image:url(https://cdn.example.com/parcel-locker.svg)', + ); + expect(icon.options.html.match(/ { setMarkerStyles(`{ "color_field": "pointStatus", @@ -81,7 +106,7 @@ describe('getTypedMarkerIcon', () => { it('adds an asterisk badge when place.has_remark is set and a match was found', () => { setMarkerStyles(`{ "icon_field": "pointType", - "icons": { "parcelLocker": "M0 0h16v16H0z" } + "icons": { "parcelLocker": "https://cdn.example.com/parcel-locker.svg" } }`); const icon = getTypedMarkerIcon({ @@ -92,14 +117,14 @@ describe('getTypedMarkerIcon', () => { }); expect(icon).not.toBeNull(); - expect(icon.options.html).toContain('M0 0h16v16H0z'); // keeps the type glyph + expect(icon.options.html).toContain('https://cdn.example.com/parcel-locker.svg'); // keeps the type glyph expect(icon.options.html).toContain('>*'); // asterisk badge overlay }); it('omits the asterisk badge when place.has_remark is not set', () => { setMarkerStyles(`{ "icon_field": "pointType", - "icons": { "parcelLocker": "M0 0h16v16H0z" } + "icons": { "parcelLocker": "https://cdn.example.com/parcel-locker.svg" } }`); const icon = getTypedMarkerIcon({ @@ -114,15 +139,13 @@ describe('getTypedMarkerIcon', () => { it('returns null (falls back to the plain asterisk icon) when has_remark is set but nothing matches', () => { setMarkerStyles('{}'); - expect( - getTypedMarkerIcon({ uuid: '1', position: [50, 50], has_remark: true }), - ).toBeNull(); + expect(getTypedMarkerIcon({ uuid: '1', position: [50, 50], has_remark: true })).toBeNull(); }); it('uses default_color from MARKER_STYLES when the matched value has no color entry', () => { setMarkerStyles(`{ "icon_field": "pointType", - "icons": { "parcelLocker": "M0 0h16v16H0z" }, + "icons": { "parcelLocker": "https://cdn.example.com/parcel-locker.svg" }, "default_color": "#123456" }`); From c76d8efee6b500cd25d8c190fe54cce6de5ceffe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Krzysztof=20Ko=C5=82odzi=C5=84ski?= Date: Thu, 20 Aug 2026 02:52:42 +0200 Subject: [PATCH 04/28] some icon fixes --- e2e-tests/tests/basic/test_marker_styles.py | 13 +- .../MarkerPopup/getTypedMarkerIcon.jsx | 131 +++++++++++------- .../MarkerPopup/getTypedMarkerIcon.test.jsx | 16 +-- 3 files changed, 94 insertions(+), 66 deletions(-) diff --git a/e2e-tests/tests/basic/test_marker_styles.py b/e2e-tests/tests/basic/test_marker_styles.py index 4027bea8..1eb2d47e 100644 --- a/e2e-tests/tests/basic/test_marker_styles.py +++ b/e2e-tests/tests/basic/test_marker_styles.py @@ -42,15 +42,17 @@ def test_fast_bridge_marker_uses_type_glyph_and_red_speed_color(self, page: Page marker = page.locator(".custom-typed-marker-icon") expect(marker).to_have_count(1, timeout=MARKER_LOAD_TIMEOUT) - # The pin shape itself, filled with speed_limit=50's color. - expect(marker.locator("path")).to_have_attribute("fill", "#c62828") + # The pin shape itself (a masked div, not an inline ), filled with + # speed_limit=50's color. + pin = marker.locator(".custom-typed-marker-pin") + expect(pin).to_have_css("background-color", "rgb(198, 40, 40)") # #c62828 # The type_of_place glyph, configured for "big bridge" - masked onto a div # via CSS rather than embedded as an inline . glyph = marker.locator(".custom-typed-marker-glyph") expect(glyph).to_have_count(1) expect(glyph).to_have_css("mask-image", f'url("{BIG_BRIDGE_GLYPH_URL}")') # No remark on Pokoju, so no asterisk badge. - expect(marker.locator("text")).to_have_count(0) + expect(marker.locator("span")).to_have_count(0) # Note: a second real-browser color case (e.g. speed_limit=10 -> green) isn't # covered here. The only speed=10 bridge without a remark (Piaskowy) can't be @@ -80,8 +82,9 @@ def test_remarked_bridge_keeps_type_and_color_styling_with_asterisk_badge(self, marker = page.locator(".custom-typed-marker-icon") expect(marker).to_have_count(1, timeout=MARKER_LOAD_TIMEOUT) - expect(marker.locator("path")).to_have_attribute("fill", "#2e7d32") # speed_limit=10 + pin = marker.locator(".custom-typed-marker-pin") + expect(pin).to_have_css("background-color", "rgb(46, 125, 50)") # #2e7d32 (speed_limit=10) glyph = marker.locator(".custom-typed-marker-glyph") expect(glyph).to_have_count(1) expect(glyph).to_have_css("mask-image", f'url("{SMALL_BRIDGE_GLYPH_URL}")') - expect(marker.locator("text")).to_have_text("*") + expect(marker.locator("span")).to_have_text("*") diff --git a/frontend/src/components/MarkerPopup/getTypedMarkerIcon.jsx b/frontend/src/components/MarkerPopup/getTypedMarkerIcon.jsx index 1d4a2588..d9a77a35 100644 --- a/frontend/src/components/MarkerPopup/getTypedMarkerIcon.jsx +++ b/frontend/src/components/MarkerPopup/getTypedMarkerIcon.jsx @@ -3,53 +3,66 @@ import PropTypes from 'prop-types'; import { DivIcon } from 'leaflet'; import ReactDOMServer from 'react-dom/server'; -const PIN_WIDTH = 30; -const PIN_HEIGHT = 42; +const PIN_WIDTH = 72; +const PIN_HEIGHT = 80; const FALLBACK_COLOR = '#2a81cb'; // leaflet default marker blue -const GLYPH_SIZE = 16; -const GLYPH_OFFSET_TOP = 6; -const GLYPH_OFFSET_LEFT = 7; +// Phosphor Icons (MIT, https://phosphoricons.com/) "map-pin-simple" glyph - +// a solid ball on a thin stem, not a balloon-style teardrop - reused as the +// pin body itself via CSS mask-image so we don't hand-draw/maintain our own +// pin shape - see PinIcon below. Its head is a solid circle (no cutout) +// centered at (50%, ~28%) of the box, so the glyph below sits inside that +// circle rather than fighting a hole like the balloon-pin design did. PIN_WIDTH +// is deliberately wider than the icon's native aspect ratio (mask-size 100% +// 100% stretches non-uniformly to fit) so the ball has real room for the +// glyph - the icon's own head is quite narrow relative to its height. +const PIN_SHAPE_URL = + 'https://cdn.jsdelivr.net/npm/@phosphor-icons/core@2/assets/fill/map-pin-simple-fill.svg'; + +// The icon's own artwork doesn't reach the bottom of its 256x256 viewBox - +// there's blank margin below the stem's rounded tip (part of Phosphor's +// standard icon padding). Since the mask is stretched to fill the box +// exactly, that margin becomes real empty space at the bottom of our div - so +// the anchor Leaflet pins to the map coordinate has to target the actual +// rendered tip position, not the box's bottom edge, or the marker floats +// above its true location. Measured empirically (screenshot pixel-row of the +// last visible fill pixel) rather than computed from the path's raw +// coordinates, since drop-shadow/antialiasing shift the rendered edge a +// little from the raw path's numbers. +const STEM_TIP_FRACTION = 0.8875; + +const GLYPH_SIZE = 24; +const GLYPH_OFFSET_TOP = 10; +const GLYPH_OFFSET_LEFT = 24; + +const maskStyle = (url, color) => ({ + backgroundColor: color, + WebkitMaskImage: `url(${url})`, + maskImage: `url(${url})`, + WebkitMaskSize: '100% 100%', + maskSize: '100% 100%', + WebkitMaskRepeat: 'no-repeat', + maskRepeat: 'no-repeat', +}); /** - * Teardrop pin shape (matches Leaflet's default marker silhouette) filled with - * `color`, optionally holding a glyph (`glyphUrl`, an icon image masked to the - * pin's own color via CSS mask-image so it doesn't need to be fetched or - * recolored server-side) centered near its top, and an asterisk badge in the - * pin's own color scheme when `hasRemark` is set - so a remarked location - * keeps its type/color styling instead of being replaced by a plain, - * uncolored asterisk marker. + * Pin shape (Phosphor's map-pin-simple glyph, masked to `color`), optionally + * holding a glyph (`glyphUrl`, masked to white) inside its head, and an + * asterisk badge in the pin's own color scheme when `hasRemark` is set - so a + * remarked location keeps its type/color styling instead of being replaced by + * a plain, uncolored asterisk marker. */ -const PinSvg = ({ color, glyphUrl, hasRemark }) => ( +const PinIcon = ({ color, glyphUrl, hasRemark }) => (
- - - {hasRemark && ( - - * - - )} - +
{glyphUrl !== '' && (
( left: GLYPH_OFFSET_LEFT, width: GLYPH_SIZE, height: GLYPH_SIZE, - backgroundColor: '#ffffff', - WebkitMaskImage: `url(${glyphUrl})`, - maskImage: `url(${glyphUrl})`, - WebkitMaskSize: 'contain', - maskSize: 'contain', - WebkitMaskRepeat: 'no-repeat', - maskRepeat: 'no-repeat', + ...maskStyle(glyphUrl, '#ffffff'), }} /> )} + {hasRemark && ( + [-1, 1].map(y => `${x}px ${y}px 0 ${color}`)) + .join(', '), + }} + > + * + + )}
); -PinSvg.propTypes = { +PinIcon.propTypes = { color: PropTypes.string.isRequired, glyphUrl: PropTypes.string.isRequired, hasRemark: PropTypes.bool.isRequired, @@ -94,7 +119,7 @@ PinSvg.propTypes = { * { * icon_field: 'type_of_place', // which location field selects the glyph * color_field: 'status', // which location field selects the fill color - * icons: { parcel_locker: 'https://cdn.example.com/parcel-locker.svg' }, // field value -> icon URL, masked+tinted via CSS (see PinSvg) + * icons: { parcel_locker: 'https://cdn.example.com/parcel-locker.svg' }, // field value -> icon URL, masked+tinted via CSS (see PinIcon) * colors: { open: '#2e7d32' }, // field value -> fill color * default_color: '#2a81cb', // fallback fill color * } @@ -121,7 +146,7 @@ const getTypedMarkerIcon = place => { return new DivIcon({ html: ReactDOMServer.renderToString( - { ), className: 'custom-typed-marker-icon', iconSize: [PIN_WIDTH, PIN_HEIGHT], - iconAnchor: [PIN_WIDTH / 2, PIN_HEIGHT], - popupAnchor: [0, -PIN_HEIGHT], + iconAnchor: [PIN_WIDTH / 2, PIN_HEIGHT * STEM_TIP_FRACTION], + popupAnchor: [0, -PIN_HEIGHT * STEM_TIP_FRACTION], }); }; diff --git a/frontend/tests/MarkerPopup/getTypedMarkerIcon.test.jsx b/frontend/tests/MarkerPopup/getTypedMarkerIcon.test.jsx index 9c73c81a..6074f9dc 100644 --- a/frontend/tests/MarkerPopup/getTypedMarkerIcon.test.jsx +++ b/frontend/tests/MarkerPopup/getTypedMarkerIcon.test.jsx @@ -49,7 +49,7 @@ describe('getTypedMarkerIcon', () => { expect(icon).not.toBeNull(); expect(icon.options.html).toContain('https://cdn.example.com/parcel-locker.svg'); expect(icon.options.html).toContain('#2a81cb'); // fallback color, no color_field set - expect(icon.options.iconSize).toEqual([30, 42]); + expect(icon.options.iconSize).toEqual([72, 80]); }); it('masks the icon URL through CSS so it picks up the matched color, instead of embedding SVG path data', () => { @@ -67,14 +67,14 @@ describe('getTypedMarkerIcon', () => { pointStatus: 'open', }); - // the glyph URL drives a CSS mask (mask-image / -webkit-mask-image) on a - //
, not an inline , so any icon set (not just - // single-path ones) works and no extra is added beyond the pin - // body's own teardrop shape. + // both the pin body (map-pin-fill.svg) and the glyph are CSS-masked + //
s tinted via background-color, not inline SVG , so + // any icon set (not just single-path ones) works for either. expect(icon.options.html).toContain( 'mask-image:url(https://cdn.example.com/parcel-locker.svg)', ); - expect(icon.options.html.match(/ { @@ -118,7 +118,7 @@ describe('getTypedMarkerIcon', () => { expect(icon).not.toBeNull(); expect(icon.options.html).toContain('https://cdn.example.com/parcel-locker.svg'); // keeps the type glyph - expect(icon.options.html).toContain('>*'); // asterisk badge overlay + expect(icon.options.html).toContain('>*'); // asterisk badge overlay }); it('omits the asterisk badge when place.has_remark is not set', () => { @@ -133,7 +133,7 @@ describe('getTypedMarkerIcon', () => { pointType: 'parcelLocker', }); - expect(icon.options.html).not.toContain('*'); }); it('returns null (falls back to the plain asterisk icon) when has_remark is set but nothing matches', () => { From b99e8e50fdcd000165715d75ec87f64539c32668 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Krzysztof=20Ko=C5=82odzi=C5=84ski?= Date: Thu, 20 Aug 2026 02:56:56 +0200 Subject: [PATCH 05/28] fix lint --- frontend/src/components/MarkerPopup/getTypedMarkerIcon.jsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/frontend/src/components/MarkerPopup/getTypedMarkerIcon.jsx b/frontend/src/components/MarkerPopup/getTypedMarkerIcon.jsx index d9a77a35..e9e8e619 100644 --- a/frontend/src/components/MarkerPopup/getTypedMarkerIcon.jsx +++ b/frontend/src/components/MarkerPopup/getTypedMarkerIcon.jsx @@ -137,8 +137,8 @@ const getTypedMarkerIcon = place => { default_color: defaultColor, } = markerStyles; - const glyphUrl = (iconField && icons && icons[place[iconField]]) || ''; - const matchedColor = (colorField && colors && colors[place[colorField]]) || ''; + const glyphUrl = icons?.[place[iconField]] || ''; + const matchedColor = colors?.[place[colorField]] || ''; if (!glyphUrl && !matchedColor) { return null; From e15aa031565074a55f4c3a21f62405134cdadf7e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Krzysztof=20Ko=C5=82odzi=C5=84ski?= Date: Thu, 20 Aug 2026 03:01:16 +0200 Subject: [PATCH 06/28] fix linting --- tests/unit_tests/data_models/test_location.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/unit_tests/data_models/test_location.py b/tests/unit_tests/data_models/test_location.py index 0330733a..a9cb1d5c 100644 --- a/tests/unit_tests/data_models/test_location.py +++ b/tests/unit_tests/data_models/test_location.py @@ -1,8 +1,9 @@ import warnings +from typing import cast import pytest -from goodmap.data_models.location import create_location_model +from goodmap.data_models.location import LocationBase, create_location_model from goodmap.exceptions import LocationValidationError @@ -139,6 +140,7 @@ def test_basic_info_includes_category_field_values(): location = location_model( uuid="1", name="test", type_of_place="parcel_locker", position=(50, 50) ) + location = cast(LocationBase, location) assert location.basic_info() == { "uuid": "1", "position": (50, 50), @@ -152,6 +154,7 @@ def test_basic_info_omits_category_fields_when_none_configured(): uuid/position/has_remark shape, unchanged.""" location_model = create_location_model(obligatory_fields=[("name", "str")], categories={}) location = location_model(uuid="1", name="test", position=(50, 50)) + location = cast(LocationBase, location) assert location.basic_info() == {"uuid": "1", "position": (50, 50), "has_remark": False} From 30088a2322e4280acc091aa3e50009de761425c0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Krzysztof=20Ko=C5=82odzi=C5=84ski?= Date: Thu, 20 Aug 2026 10:22:46 +0200 Subject: [PATCH 07/28] some fixes --- .../MarkerPopup/getTypedMarkerIcon.jsx | 65 +++++++------------ 1 file changed, 25 insertions(+), 40 deletions(-) diff --git a/frontend/src/components/MarkerPopup/getTypedMarkerIcon.jsx b/frontend/src/components/MarkerPopup/getTypedMarkerIcon.jsx index e9e8e619..ecc77c98 100644 --- a/frontend/src/components/MarkerPopup/getTypedMarkerIcon.jsx +++ b/frontend/src/components/MarkerPopup/getTypedMarkerIcon.jsx @@ -5,30 +5,21 @@ import ReactDOMServer from 'react-dom/server'; const PIN_WIDTH = 72; const PIN_HEIGHT = 80; -const FALLBACK_COLOR = '#2a81cb'; // leaflet default marker blue +// Matches the accent color used elsewhere on the page (buttons, left panel). +const FALLBACK_COLOR = globalThis.SECONDARY_COLOR || '#2a81cb'; -// Phosphor Icons (MIT, https://phosphoricons.com/) "map-pin-simple" glyph - -// a solid ball on a thin stem, not a balloon-style teardrop - reused as the -// pin body itself via CSS mask-image so we don't hand-draw/maintain our own -// pin shape - see PinIcon below. Its head is a solid circle (no cutout) -// centered at (50%, ~28%) of the box, so the glyph below sits inside that -// circle rather than fighting a hole like the balloon-pin design did. PIN_WIDTH -// is deliberately wider than the icon's native aspect ratio (mask-size 100% -// 100% stretches non-uniformly to fit) so the ball has real room for the -// glyph - the icon's own head is quite narrow relative to its height. +// Phosphor Icons "map-pin-simple" glyph (MIT, phosphoricons.com), masked as +// the pin body - see PinIcon below. PIN_WIDTH is wider than the icon's own +// aspect ratio so its ball has room for the glyph. const PIN_SHAPE_URL = 'https://cdn.jsdelivr.net/npm/@phosphor-icons/core@2/assets/fill/map-pin-simple-fill.svg'; -// The icon's own artwork doesn't reach the bottom of its 256x256 viewBox - -// there's blank margin below the stem's rounded tip (part of Phosphor's -// standard icon padding). Since the mask is stretched to fill the box -// exactly, that margin becomes real empty space at the bottom of our div - so -// the anchor Leaflet pins to the map coordinate has to target the actual -// rendered tip position, not the box's bottom edge, or the marker floats -// above its true location. Measured empirically (screenshot pixel-row of the -// last visible fill pixel) rather than computed from the path's raw -// coordinates, since drop-shadow/antialiasing shift the rendered edge a -// little from the raw path's numbers. +// The icon's artwork leaves blank margin below the stem tip, which becomes +// real empty space once stretched to fill the box - so the anchor has to +// target the actual rendered tip, not the box edge, or the marker floats +// above its true location. Measured empirically from a screenshot rather +// than the raw path coordinates, since drop-shadow/antialiasing shift the +// rendered edge slightly. const STEM_TIP_FRACTION = 0.8875; const GLYPH_SIZE = 24; @@ -46,11 +37,10 @@ const maskStyle = (url, color) => ({ }); /** - * Pin shape (Phosphor's map-pin-simple glyph, masked to `color`), optionally - * holding a glyph (`glyphUrl`, masked to white) inside its head, and an - * asterisk badge in the pin's own color scheme when `hasRemark` is set - so a - * remarked location keeps its type/color styling instead of being replaced by - * a plain, uncolored asterisk marker. + * Pin shape masked to `color`, optionally holding a glyph (`glyphUrl`) inside + * its head, and an asterisk badge when `hasRemark` is set - so a remarked + * location keeps its type/color styling instead of losing it to a plain + * asterisk marker. */ const PinIcon = ({ color, glyphUrl, hasRemark }) => (
@@ -104,24 +94,19 @@ PinIcon.propTypes = { }; /** - * Builds a Leaflet icon for `place` based on the deployment's marker styling - * lookup table (window.MARKER_STYLES, set server-side from the map's - * `marker_styles` config - see goodmap's db.get_marker_styles), or returns - * `null` when neither `icon_field` nor `color_field` produced a configured - * lookup match - callers should omit the `icon` prop in that case and fall - * back to Leaflet's default marker (or the plain asterisk icon for a remarked - * location with no marker_styles match) so unconfigured/legacy deployments - * are unchanged. When `place.has_remark` is set and a match *was* found, the - * returned icon carries an asterisk badge instead of losing its type/color - * styling to the plain asterisk marker. + * Builds a Leaflet icon for `place` from the deployment's marker styling + * lookup table (window.MARKER_STYLES, see goodmap's db.get_marker_styles), or + * `null` when neither `icon_field` nor `color_field` matched - callers should + * omit the `icon` prop then and fall back to Leaflet's default marker (or the + * plain asterisk icon for a remarked location). * * Expected shape of window.MARKER_STYLES: * { - * icon_field: 'type_of_place', // which location field selects the glyph - * color_field: 'status', // which location field selects the fill color - * icons: { parcel_locker: 'https://cdn.example.com/parcel-locker.svg' }, // field value -> icon URL, masked+tinted via CSS (see PinIcon) - * colors: { open: '#2e7d32' }, // field value -> fill color - * default_color: '#2a81cb', // fallback fill color + * icon_field: 'type_of_place', + * color_field: 'status', + * icons: { parcel_locker: 'https://cdn.example.com/parcel-locker.svg' }, + * colors: { open: '#2e7d32' }, + * default_color: '#2a81cb', * } * * @param {Object} place - Location data, as returned by GET /api/locations From 6b2bc2c08f4b8607ddb67bf5cb1b347d8108d8e0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Krzysztof=20Ko=C5=82odzi=C5=84ski?= Date: Thu, 20 Aug 2026 10:53:21 +0200 Subject: [PATCH 08/28] fixes --- e2e-tests/e2e_test_data_initial.json | 2 +- e2e-tests/tests/basic/test_marker_styles.py | 62 ++++++++++++------- .../MarkerPopup/getTypedMarkerIcon.jsx | 36 +++++------ .../MarkerPopup/getTypedMarkerIcon.test.jsx | 8 +-- 4 files changed, 61 insertions(+), 47 deletions(-) diff --git a/e2e-tests/e2e_test_data_initial.json b/e2e-tests/e2e_test_data_initial.json index 8af116ba..fc66933f 100644 --- a/e2e-tests/e2e_test_data_initial.json +++ b/e2e-tests/e2e_test_data_initial.json @@ -112,7 +112,7 @@ "is_free": "true", "speed_limit": "10", "amenities": [ - "benches" + "toilets" ], "uuid": "5986e755-1eaa-4121-a01c-4fef1d5d1da1" }, diff --git a/e2e-tests/tests/basic/test_marker_styles.py b/e2e-tests/tests/basic/test_marker_styles.py index 1eb2d47e..4c540ffe 100644 --- a/e2e-tests/tests/basic/test_marker_styles.py +++ b/e2e-tests/tests/basic/test_marker_styles.py @@ -12,14 +12,14 @@ from tests.conftest import BASE_URL, MARKER_LOAD_TIMEOUT, open_test_popup -# "big bridge" and "small bridge" each get their own Phosphor Icons (MIT) glyph - -# see e2e_test_data_initial.json's marker_styles.icons and getTypedMarkerIcon.jsx -# (icon URLs are CSS mask-image'd onto the pin, tinted by the matched color, -# rather than embedded as inline SVG path data). -BIG_BRIDGE_GLYPH_URL = ( +# "big bridge" and "small bridge" each get their own Phosphor Icons (MIT) type +# icon - see e2e_test_data_initial.json's marker_styles.icons and +# getTypedMarkerIcon.jsx (icon URLs are CSS mask-image'd onto the pin, tinted +# by the matched color, rather than embedded as inline SVG path data). +BIG_BRIDGE_TYPE_ICON_URL = ( "https://cdn.jsdelivr.net/npm/@phosphor-icons/core@2/assets/fill/bridge-fill.svg" ) -SMALL_BRIDGE_GLYPH_URL = ( +SMALL_BRIDGE_TYPE_ICON_URL = ( "https://cdn.jsdelivr.net/npm/@phosphor-icons/core@2/assets/fill/footprints-fill.svg" ) @@ -27,7 +27,7 @@ class TestMarkerStyles: """Test suite for marker_styles-driven pin icons/colors""" - def test_fast_bridge_marker_uses_type_glyph_and_red_speed_color(self, page: Page): + def test_fast_bridge_marker_uses_type_icon_and_red_speed_color(self, page: Page): """Pokoju (big bridge, speed_limit=50, no remark) is the only seeded bridge with all three of lighting+benches+toilets (amenities is an "and" category - see test_and_filter_within_category_narrows_results in test_map.py), so @@ -46,24 +46,38 @@ def test_fast_bridge_marker_uses_type_glyph_and_red_speed_color(self, page: Page # speed_limit=50's color. pin = marker.locator(".custom-typed-marker-pin") expect(pin).to_have_css("background-color", "rgb(198, 40, 40)") # #c62828 - # The type_of_place glyph, configured for "big bridge" - masked onto a div + # The type_of_place icon, configured for "big bridge" - masked onto a div # via CSS rather than embedded as an inline . - glyph = marker.locator(".custom-typed-marker-glyph") - expect(glyph).to_have_count(1) - expect(glyph).to_have_css("mask-image", f'url("{BIG_BRIDGE_GLYPH_URL}")') + type_icon = marker.locator(".custom-typed-marker-type-icon") + expect(type_icon).to_have_count(1) + expect(type_icon).to_have_css("mask-image", f'url("{BIG_BRIDGE_TYPE_ICON_URL}")') # No remark on Pokoju, so no asterisk badge. expect(marker.locator("span")).to_have_count(0) - # Note: a second real-browser color case (e.g. speed_limit=10 -> green) isn't - # covered here. The only speed=10 bridge without a remark (Piaskowy) can't be - # isolated to a standalone marker via the left panel's filters - its amenities - # ([benches]) are a subset of a remarked neighbor's (Tumski, [lighting, - # benches]) barely 230m away, so any filter combo that includes Piaskowy also - # includes Tumski, and Leaflet.markercluster groups them into one cluster - # bubble at the map's default zoom, hiding both individual markers. The - # color-lookup logic itself (arbitrary field values, including a "10" -> - # green case) is covered generically at the unit level in - # frontend/tests/MarkerPopup/getTypedMarkerIcon.test.jsx. + def test_slow_bridge_marker_uses_type_icon_and_green_speed_color(self, page: Page): + """Piaskowy (small bridge, speed_limit=10, no remark, toilets) is the + only seeded speed<=10 bridge with toilets - the other two speed=10 + bridges (Zwierzyniecka, Tumski) have lighting/benches but neither has + toilets, so combining the speed_limit=10 radio with the toilets + checkbox isolates it without relying on clustering distance/zoom + assumptions. "cars" is unchecked first since Piaskowy is + pedestrians-only.""" + page.goto(BASE_URL, wait_until="domcontentloaded") + + page.get_by_role("checkbox", name="cars", exact=False).click() + page.get_by_role("radio", name="10 km/h", exact=False).click() + page.get_by_role("checkbox", name="toilets", exact=False).click() + + marker = page.locator(".custom-typed-marker-icon") + expect(marker).to_have_count(1, timeout=MARKER_LOAD_TIMEOUT) + + pin = marker.locator(".custom-typed-marker-pin") + expect(pin).to_have_css("background-color", "rgb(46, 125, 50)") # #2e7d32 (speed_limit=10) + type_icon = marker.locator(".custom-typed-marker-type-icon") + expect(type_icon).to_have_count(1) + expect(type_icon).to_have_css("mask-image", f'url("{SMALL_BRIDGE_TYPE_ICON_URL}")') + # No remark on Piaskowy, so no asterisk badge. + expect(marker.locator("span")).to_have_count(0) def test_remarked_bridge_keeps_type_and_color_styling_with_asterisk_badge(self, page: Page): """Zwierzyniecka has both a remark and marker_styles-matching fields @@ -84,7 +98,7 @@ def test_remarked_bridge_keeps_type_and_color_styling_with_asterisk_badge(self, pin = marker.locator(".custom-typed-marker-pin") expect(pin).to_have_css("background-color", "rgb(46, 125, 50)") # #2e7d32 (speed_limit=10) - glyph = marker.locator(".custom-typed-marker-glyph") - expect(glyph).to_have_count(1) - expect(glyph).to_have_css("mask-image", f'url("{SMALL_BRIDGE_GLYPH_URL}")') + type_icon = marker.locator(".custom-typed-marker-type-icon") + expect(type_icon).to_have_count(1) + expect(type_icon).to_have_css("mask-image", f'url("{SMALL_BRIDGE_TYPE_ICON_URL}")') expect(marker.locator("span")).to_have_text("*") diff --git a/frontend/src/components/MarkerPopup/getTypedMarkerIcon.jsx b/frontend/src/components/MarkerPopup/getTypedMarkerIcon.jsx index ecc77c98..301480fc 100644 --- a/frontend/src/components/MarkerPopup/getTypedMarkerIcon.jsx +++ b/frontend/src/components/MarkerPopup/getTypedMarkerIcon.jsx @@ -8,9 +8,9 @@ const PIN_HEIGHT = 80; // Matches the accent color used elsewhere on the page (buttons, left panel). const FALLBACK_COLOR = globalThis.SECONDARY_COLOR || '#2a81cb'; -// Phosphor Icons "map-pin-simple" glyph (MIT, phosphoricons.com), masked as +// Phosphor Icons "map-pin-simple" shape (MIT, phosphoricons.com), masked as // the pin body - see PinIcon below. PIN_WIDTH is wider than the icon's own -// aspect ratio so its ball has room for the glyph. +// aspect ratio so its ball has room for the type icon. const PIN_SHAPE_URL = 'https://cdn.jsdelivr.net/npm/@phosphor-icons/core@2/assets/fill/map-pin-simple-fill.svg'; @@ -22,9 +22,9 @@ const PIN_SHAPE_URL = // rendered edge slightly. const STEM_TIP_FRACTION = 0.8875; -const GLYPH_SIZE = 24; -const GLYPH_OFFSET_TOP = 10; -const GLYPH_OFFSET_LEFT = 24; +const TYPE_ICON_SIZE = 24; +const TYPE_ICON_OFFSET_TOP = 10; +const TYPE_ICON_OFFSET_LEFT = 24; const maskStyle = (url, color) => ({ backgroundColor: color, @@ -37,12 +37,12 @@ const maskStyle = (url, color) => ({ }); /** - * Pin shape masked to `color`, optionally holding a glyph (`glyphUrl`) inside + * Pin shape masked to `color`, optionally holding a type icon (`typeIconUrl`) inside * its head, and an asterisk badge when `hasRemark` is set - so a remarked * location keeps its type/color styling instead of losing it to a plain * asterisk marker. */ -const PinIcon = ({ color, glyphUrl, hasRemark }) => ( +const PinIcon = ({ color, typeIconUrl, hasRemark }) => (
( ...maskStyle(PIN_SHAPE_URL, color), }} /> - {glyphUrl !== '' && ( + {typeIconUrl !== '' && (
)} @@ -89,7 +89,7 @@ const PinIcon = ({ color, glyphUrl, hasRemark }) => ( PinIcon.propTypes = { color: PropTypes.string.isRequired, - glyphUrl: PropTypes.string.isRequired, + typeIconUrl: PropTypes.string.isRequired, hasRemark: PropTypes.bool.isRequired, }; @@ -122,10 +122,10 @@ const getTypedMarkerIcon = place => { default_color: defaultColor, } = markerStyles; - const glyphUrl = icons?.[place[iconField]] || ''; + const typeIconUrl = icons?.[place[iconField]] || ''; const matchedColor = colors?.[place[colorField]] || ''; - if (!glyphUrl && !matchedColor) { + if (!typeIconUrl && !matchedColor) { return null; } @@ -133,7 +133,7 @@ const getTypedMarkerIcon = place => { html: ReactDOMServer.renderToString( , ), diff --git a/frontend/tests/MarkerPopup/getTypedMarkerIcon.test.jsx b/frontend/tests/MarkerPopup/getTypedMarkerIcon.test.jsx index 6074f9dc..2ac9de40 100644 --- a/frontend/tests/MarkerPopup/getTypedMarkerIcon.test.jsx +++ b/frontend/tests/MarkerPopup/getTypedMarkerIcon.test.jsx @@ -34,7 +34,7 @@ describe('getTypedMarkerIcon', () => { ).toBeNull(); }); - it('builds a DivIcon when the icon field matches a configured glyph', () => { + it('builds a DivIcon when the icon field matches a configured type icon', () => { setMarkerStyles(`{ "icon_field": "pointType", "icons": { "parcelLocker": "https://cdn.example.com/parcel-locker.svg" } @@ -67,7 +67,7 @@ describe('getTypedMarkerIcon', () => { pointStatus: 'open', }); - // both the pin body (map-pin-fill.svg) and the glyph are CSS-masked + // both the pin body (map-pin-fill.svg) and the type icon are CSS-masked //
s tinted via background-color, not inline SVG , so // any icon set (not just single-path ones) works for either. expect(icon.options.html).toContain( @@ -77,7 +77,7 @@ describe('getTypedMarkerIcon', () => { expect(icon.options.html).not.toContain(' { + it('builds a DivIcon when the color field matches a configured color, with no type icon', () => { setMarkerStyles(`{ "color_field": "pointStatus", "colors": { "open": "#2e7d32" } @@ -117,7 +117,7 @@ describe('getTypedMarkerIcon', () => { }); expect(icon).not.toBeNull(); - expect(icon.options.html).toContain('https://cdn.example.com/parcel-locker.svg'); // keeps the type glyph + expect(icon.options.html).toContain('https://cdn.example.com/parcel-locker.svg'); // keeps the type icon expect(icon.options.html).toContain('>*'); // asterisk badge overlay }); From 049554d0795ff337fdd25b3d278a3d507786a6f5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Krzysztof=20Ko=C5=82odzi=C5=84ski?= Date: Thu, 20 Aug 2026 11:08:14 +0200 Subject: [PATCH 09/28] some trims --- .../MarkerPopup/getTypedMarkerIcon.jsx | 27 ++++--------------- 1 file changed, 5 insertions(+), 22 deletions(-) diff --git a/frontend/src/components/MarkerPopup/getTypedMarkerIcon.jsx b/frontend/src/components/MarkerPopup/getTypedMarkerIcon.jsx index 301480fc..d515a2c6 100644 --- a/frontend/src/components/MarkerPopup/getTypedMarkerIcon.jsx +++ b/frontend/src/components/MarkerPopup/getTypedMarkerIcon.jsx @@ -5,24 +5,16 @@ import ReactDOMServer from 'react-dom/server'; const PIN_WIDTH = 72; const PIN_HEIGHT = 80; -// Matches the accent color used elsewhere on the page (buttons, left panel). -const FALLBACK_COLOR = globalThis.SECONDARY_COLOR || '#2a81cb'; +const FALLBACK_COLOR = globalThis.SECONDARY_COLOR || 'black'; -// Phosphor Icons "map-pin-simple" shape (MIT, phosphoricons.com), masked as -// the pin body - see PinIcon below. PIN_WIDTH is wider than the icon's own -// aspect ratio so its ball has room for the type icon. +// TODO make pin shape configurable const PIN_SHAPE_URL = 'https://cdn.jsdelivr.net/npm/@phosphor-icons/core@2/assets/fill/map-pin-simple-fill.svg'; -// The icon's artwork leaves blank margin below the stem tip, which becomes -// real empty space once stretched to fill the box - so the anchor has to -// target the actual rendered tip, not the box edge, or the marker floats -// above its true location. Measured empirically from a screenshot rather -// than the raw path coordinates, since drop-shadow/antialiasing shift the -// rendered edge slightly. -const STEM_TIP_FRACTION = 0.8875; - const TYPE_ICON_SIZE = 24; + +// Because the pin shape is not a perfectly aligned, anchor point is not at the bottom of the pin +// we need to adjust the anchor and popup positions accordingly const TYPE_ICON_OFFSET_TOP = 10; const TYPE_ICON_OFFSET_LEFT = 24; @@ -100,15 +92,6 @@ PinIcon.propTypes = { * omit the `icon` prop then and fall back to Leaflet's default marker (or the * plain asterisk icon for a remarked location). * - * Expected shape of window.MARKER_STYLES: - * { - * icon_field: 'type_of_place', - * color_field: 'status', - * icons: { parcel_locker: 'https://cdn.example.com/parcel-locker.svg' }, - * colors: { open: '#2e7d32' }, - * default_color: '#2a81cb', - * } - * * @param {Object} place - Location data, as returned by GET /api/locations * @returns {import('leaflet').DivIcon|null} */ From d0634390ffabdfd95599853949168ddcf254a48a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Krzysztof=20Ko=C5=82odzi=C5=84ski?= Date: Thu, 20 Aug 2026 11:26:18 +0200 Subject: [PATCH 10/28] some fixes --- .../MarkerPopup/getTypedMarkerIcon.jsx | 17 +++++++++++++---- .../MarkerPopup/getTypedMarkerIcon.test.jsx | 2 +- 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/frontend/src/components/MarkerPopup/getTypedMarkerIcon.jsx b/frontend/src/components/MarkerPopup/getTypedMarkerIcon.jsx index d515a2c6..dcac3dfc 100644 --- a/frontend/src/components/MarkerPopup/getTypedMarkerIcon.jsx +++ b/frontend/src/components/MarkerPopup/getTypedMarkerIcon.jsx @@ -2,15 +2,17 @@ import React from 'react'; import PropTypes from 'prop-types'; import { DivIcon } from 'leaflet'; import ReactDOMServer from 'react-dom/server'; +// Phosphor Icons "map-pin-simple" (fill style), MIT license, phosphoricons.com - +// vendored locally (see the .svg file) instead of fetched from a CDN, since +// it's a fixed asset we chose, not deployment config, and every styled marker +// on every deployment depends on it. +// TODO make pin shape configurable +import PIN_SHAPE_URL from '../../res/svg/marker-pin.svg'; const PIN_WIDTH = 72; const PIN_HEIGHT = 80; const FALLBACK_COLOR = globalThis.SECONDARY_COLOR || 'black'; -// TODO make pin shape configurable -const PIN_SHAPE_URL = - 'https://cdn.jsdelivr.net/npm/@phosphor-icons/core@2/assets/fill/map-pin-simple-fill.svg'; - const TYPE_ICON_SIZE = 24; // Because the pin shape is not a perfectly aligned, anchor point is not at the bottom of the pin @@ -18,6 +20,13 @@ const TYPE_ICON_SIZE = 24; const TYPE_ICON_OFFSET_TOP = 10; const TYPE_ICON_OFFSET_LEFT = 24; +// The pin's own artwork doesn't reach the bottom of its viewBox, so the +// anchor Leaflet pins to the map coordinate has to target the actual +// rendered tip, not the box edge, or the marker floats above its true +// location. Measured empirically from a screenshot rather than the raw path +// coordinates, since drop-shadow/antialiasing shift the rendered edge a bit. +const STEM_TIP_FRACTION = 0.8875; + const maskStyle = (url, color) => ({ backgroundColor: color, WebkitMaskImage: `url(${url})`, diff --git a/frontend/tests/MarkerPopup/getTypedMarkerIcon.test.jsx b/frontend/tests/MarkerPopup/getTypedMarkerIcon.test.jsx index 2ac9de40..03dec5b6 100644 --- a/frontend/tests/MarkerPopup/getTypedMarkerIcon.test.jsx +++ b/frontend/tests/MarkerPopup/getTypedMarkerIcon.test.jsx @@ -48,7 +48,7 @@ describe('getTypedMarkerIcon', () => { expect(icon).not.toBeNull(); expect(icon.options.html).toContain('https://cdn.example.com/parcel-locker.svg'); - expect(icon.options.html).toContain('#2a81cb'); // fallback color, no color_field set + expect(icon.options.html).toContain('background-color:black'); // fallback color, no color_field set expect(icon.options.iconSize).toEqual([72, 80]); }); From 3e198199691a88e4d8e58b7ca2cbc5b1ac6b505e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Krzysztof=20Ko=C5=82odzi=C5=84ski?= Date: Thu, 20 Aug 2026 12:03:08 +0200 Subject: [PATCH 11/28] added missing marker --- frontend/src/res/svg/marker-pin.svg | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 frontend/src/res/svg/marker-pin.svg diff --git a/frontend/src/res/svg/marker-pin.svg b/frontend/src/res/svg/marker-pin.svg new file mode 100644 index 00000000..59e03460 --- /dev/null +++ b/frontend/src/res/svg/marker-pin.svg @@ -0,0 +1,2 @@ + + From e2896450cb7b400848f53e07b394372461438339 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Krzysztof=20Ko=C5=82odzi=C5=84ski?= Date: Thu, 20 Aug 2026 12:41:29 +0200 Subject: [PATCH 12/28] cleanup --- e2e-tests/tests/basic/test_marker_styles.py | 18 ++--- .../components/MarkerPopup/MarkerPopup.jsx | 31 +++------ .../MarkerPopup/getTypedMarkerIcon.jsx | 63 ++++++++---------- frontend/src/res/img/marker-icon-asterisk.png | Bin 6212 -> 0 bytes frontend/src/res/svg/marker-pin.svg | 3 +- .../tests/MarkerPopup/MarkerPopup.test.jsx | 21 +++--- .../MarkerPopup/getTypedMarkerIcon.test.jsx | 17 ++++- 7 files changed, 72 insertions(+), 81 deletions(-) delete mode 100644 frontend/src/res/img/marker-icon-asterisk.png diff --git a/e2e-tests/tests/basic/test_marker_styles.py b/e2e-tests/tests/basic/test_marker_styles.py index 4c540ffe..62e3f200 100644 --- a/e2e-tests/tests/basic/test_marker_styles.py +++ b/e2e-tests/tests/basic/test_marker_styles.py @@ -4,8 +4,8 @@ Tests that the map picks pin icon/color per marker_styles (icon_field: type_of_place, color_field: speed_limit - see e2e_test_data_initial.json), and that a location with both a remark and a marker_styles match keeps its -type/color styling with an asterisk badge overlay, rather than losing it to -the plain asterisk icon (see getTypedMarkerIcon.jsx/MarkerPopup.jsx). +type/color styling with an asterisk badge overlay, rather than losing it to a +plain, unstyled asterisk badge (see getTypedMarkerIcon.jsx/MarkerPopup.jsx). """ from playwright.sync_api import Page, expect @@ -82,12 +82,14 @@ def test_slow_bridge_marker_uses_type_icon_and_green_speed_color(self, page: Pag def test_remarked_bridge_keeps_type_and_color_styling_with_asterisk_badge(self, page: Page): """Zwierzyniecka has both a remark and marker_styles-matching fields (small bridge, speed_limit=10) - it should render its normal typed/colored - pin plus an asterisk badge, not fall back to the plain asterisk icon - (every type_of_place/speed_limit value happens to be covered by - marker_styles in this seeded dataset, so that plain-icon fallback path - isn't exercised here - it's covered at the unit level instead, see - getTypedMarkerIcon.test.jsx's "falls back to the plain asterisk icon" - case).""" + pin plus an asterisk badge, not fall back to our own pin in the plain + fallback color with no type icon (every type_of_place/speed_limit value + happens to be covered by marker_styles in this seeded dataset, so that + fallback-color path isn't exercised here - it's covered at the unit + level instead, see getTypedMarkerIcon.test.jsx's "returns our own pin in + the fallback color with just the badge" case). Also guards against ever + reintroducing the old PNG-based asterisk icon this replaced. + """ page.goto(BASE_URL, wait_until="domcontentloaded") open_test_popup(page) diff --git a/frontend/src/components/MarkerPopup/MarkerPopup.jsx b/frontend/src/components/MarkerPopup/MarkerPopup.jsx index b18e5710..558a13c8 100644 --- a/frontend/src/components/MarkerPopup/MarkerPopup.jsx +++ b/frontend/src/components/MarkerPopup/MarkerPopup.jsx @@ -2,7 +2,6 @@ import React, { useState, useEffect } from 'react'; import PropTypes from 'prop-types'; import { Marker } from 'react-leaflet'; import { isMobile } from 'react-device-detect'; -import { Icon } from 'leaflet'; import { useTranslation } from 'react-i18next'; import httpService from '../../services/http/httpService'; import useMapStore from '../Map/store/map.store'; @@ -11,7 +10,6 @@ import LocationDetailsBox from './LocationDetails'; import MobilePopup from './MobilePopup'; import DesktopPopup from './DesktopPopup'; import getTypedMarkerIcon from './getTypedMarkerIcon'; -import iconAsterisk from '../../res/img/marker-icon-asterisk.png'; /** * Wrapper component that fetches full location details and renders them in a popup. @@ -70,17 +68,6 @@ LocationDetailsBoxWrapper.propTypes = { }).isRequired, }; -/** - * Custom Leaflet icon for markers with remarks/special annotations. - * Displays an asterisk icon to visually distinguish remarked locations from standard markers. - */ -const asteriskIcon = new Icon({ - iconUrl: iconAsterisk, - iconSize: [40, 48], // size of the icon - iconAnchor: [19, 46], // point of the icon which will correspond to marker's location - popupAnchor: [0, -40], // point from which the popup should open relative to the iconAnchor -}); - /** * Interactive map marker component that displays location details in a popup when clicked. * Supports special visual indication for locations with remarks using an asterisk icon. @@ -88,7 +75,7 @@ const asteriskIcon = new Icon({ * @param {Object} props - Component props * @param {Object} props.place - Location data object * @param {number[]} props.place.position - Coordinates [latitude, longitude] - * @param {boolean} [props.place.has_remark] - Whether this location has a remark (uses asterisk icon if true) + * @param {boolean} [props.place.has_remark] - Whether this location has a remark (adds an asterisk badge if true) * @returns {React.ReactElement} Leaflet Marker component with click-to-show-details functionality */ const MarkerPopup = ({ place }) => { @@ -120,20 +107,20 @@ const MarkerPopup = ({ place }) => { eventHandlers: { click: handleMarkerClick, }, - alt: place.has_remark ? 'Marker-Asterisk' : 'Marker', + // getTypedMarkerIcon renders as a
, not an , so 'alt' has no + // visible effect once it returns an icon - kept as plain text for the + // one case it still applies to: Leaflet's own default marker below. + alt: 'Marker', }; - // Prefer a marker_styles match (getTypedMarkerIcon adds an asterisk badge to - // it when place.has_remark is set, so a remarked location keeps its type/color - // styling) and only fall back to the plain asterisk icon when there's no - // match to style - e.g. a legacy/unconfigured deployment. Only add an icon - // prop when we actually have a custom icon: passing icon={undefined} causes + // getTypedMarkerIcon returns our own pin whenever there's a marker_styles + // match or a remark to badge, null only for a plain, unremarked location - + // which then falls back to Leaflet's default marker. Only add an icon prop + // when we actually have a custom icon: passing icon={undefined} causes // errors in MarkerClusterGroup during cluster zoom animations. const typedIcon = getTypedMarkerIcon(place); if (typedIcon) { markerProps.icon = typedIcon; - } else if (place.has_remark) { - markerProps.icon = asteriskIcon; } return ( diff --git a/frontend/src/components/MarkerPopup/getTypedMarkerIcon.jsx b/frontend/src/components/MarkerPopup/getTypedMarkerIcon.jsx index dcac3dfc..0cf301ed 100644 --- a/frontend/src/components/MarkerPopup/getTypedMarkerIcon.jsx +++ b/frontend/src/components/MarkerPopup/getTypedMarkerIcon.jsx @@ -2,30 +2,19 @@ import React from 'react'; import PropTypes from 'prop-types'; import { DivIcon } from 'leaflet'; import ReactDOMServer from 'react-dom/server'; -// Phosphor Icons "map-pin-simple" (fill style), MIT license, phosphoricons.com - -// vendored locally (see the .svg file) instead of fetched from a CDN, since -// it's a fixed asset we chose, not deployment config, and every styled marker -// on every deployment depends on it. -// TODO make pin shape configurable +// Custom balloon pin (sharp point, solid head, no third-party asset/CDN) - see +// the .svg file. Its point sits exactly on the viewBox's bottom edge, so the +// anchor below doesn't need any empirical correction the way a borrowed icon +// with its own padding would. import PIN_SHAPE_URL from '../../res/svg/marker-pin.svg'; -const PIN_WIDTH = 72; -const PIN_HEIGHT = 80; +const PIN_WIDTH = 36; +const PIN_HEIGHT = 40; const FALLBACK_COLOR = globalThis.SECONDARY_COLOR || 'black'; -const TYPE_ICON_SIZE = 24; - -// Because the pin shape is not a perfectly aligned, anchor point is not at the bottom of the pin -// we need to adjust the anchor and popup positions accordingly -const TYPE_ICON_OFFSET_TOP = 10; -const TYPE_ICON_OFFSET_LEFT = 24; - -// The pin's own artwork doesn't reach the bottom of its viewBox, so the -// anchor Leaflet pins to the map coordinate has to target the actual -// rendered tip, not the box edge, or the marker floats above its true -// location. Measured empirically from a screenshot rather than the raw path -// coordinates, since drop-shadow/antialiasing shift the rendered edge a bit. -const STEM_TIP_FRACTION = 0.8875; +const TYPE_ICON_SIZE = 16; +const TYPE_ICON_OFFSET_TOP = 6.5; +const TYPE_ICON_OFFSET_LEFT = 10; const maskStyle = (url, color) => ({ backgroundColor: color, @@ -38,10 +27,10 @@ const maskStyle = (url, color) => ({ }); /** - * Pin shape masked to `color`, optionally holding a type icon (`typeIconUrl`) inside - * its head, and an asterisk badge when `hasRemark` is set - so a remarked - * location keeps its type/color styling instead of losing it to a plain - * asterisk marker. + * Pin shape masked to `color`, optionally holding a type icon (`typeIconUrl`) + * inside its head, and an asterisk badge when `hasRemark` is set - so a + * remarked location keeps its type/color styling (or just its fallback color, + * if nothing else matched) instead of losing it to an unrelated asterisk icon. */ const PinIcon = ({ color, typeIconUrl, hasRemark }) => (
@@ -71,9 +60,9 @@ const PinIcon = ({ color, typeIconUrl, hasRemark }) => ( { const typeIconUrl = icons?.[place[iconField]] || ''; const matchedColor = colors?.[place[colorField]] || ''; + const hasRemark = Boolean(place.has_remark); - if (!typeIconUrl && !matchedColor) { + if (!typeIconUrl && !matchedColor && !hasRemark) { return null; } @@ -126,13 +117,13 @@ const getTypedMarkerIcon = place => { , ), className: 'custom-typed-marker-icon', iconSize: [PIN_WIDTH, PIN_HEIGHT], - iconAnchor: [PIN_WIDTH / 2, PIN_HEIGHT * STEM_TIP_FRACTION], - popupAnchor: [0, -PIN_HEIGHT * STEM_TIP_FRACTION], + iconAnchor: [PIN_WIDTH / 2, PIN_HEIGHT], + popupAnchor: [0, -PIN_HEIGHT], }); }; diff --git a/frontend/src/res/img/marker-icon-asterisk.png b/frontend/src/res/img/marker-icon-asterisk.png deleted file mode 100644 index 9e5cf850e9f60b0d4b7ec9cda1d9d907aecc7f1f..0000000000000000000000000000000000000000 GIT binary patch literal 0 HcmV?d00001 literal 6212 zcmeHKdpOiv_a8%a+(I1@)ifuU)ZAy7nP!MlOc{wOx5Rg5zQfeq%#2$q5+a1?%1IY> zQ0YP!L?o9E5_Kpll1_Ksqa3{78M^&`@AJG*&-1?jHS=tHeb-*=v)B6Uwb%G2d$_yk zY0uDx!C-n!S7%RXwuT0FvL^JG8%@rErt6>&lh0(@z$QZvkfoXiObwEhc4`wc7Lws= z>M(U^PK1ULBr9!{V}{x!)tm~+W)pHYBwJ6|z#$o@28ZcEb1pRSkUR~Vi=ZLcI=)vG zBr89YRbN*~QX+qzU6?En1RjmUq6uWkAB!hZu>>jshrna0SP~V7gRIpKe2qwnBw)vn zK`8M82E&8NU?8M2311)tVX(;DjhS@U+C}Ddf&NF3zZxz~n#?b7H+Ec1?Xa14(5c{8 zzc6gsJDq*Rx%1H57xsddw64W0B28!A)YEO;r8*=}zk!B<-u zH0%}oTrs;+lC$etR35o`=?p%O0P~3rHcd6xDR4^YAF3JajX2K-*8j=Uwul{=-aTrf9#^rbMBLreaPBTm z4c=P4v*Uxe*`nuJ@7DXLn5&I+SJ#F4XAQHPT)d-vI~x?wxZ)%5i@zea*5qc2wcQF1<}we-`l8sd@cxgY1xey9c`1#BOM56g|p zm59&)Pr?Dwks_(`z+g0oNGZTw2PzO8Fjyd_BVSfrLLvk_I?|iO!m^|cFht*z=ysE!yC8HgaF ziD)cpaiky|kF?iD&}2M5)zf*&1O)U%M}{aAQYr=$5fOooAfP3(U<{5zpy!hiRHg3A;vMBK{;0@kSYWcF+zz6a3o<0IuZ%3BREBWL$7@I4D;9_ALaD`^8@xCKIaC z;c+pb5EMbCa%flFcUvlN+!qfe1;GN5ROJPc{hg&k!2e3tcd;o~ROx&h2;}|+_dDy) zxvPvJD;A6DEa8SJ!(%$rk;?t4JPB98qpCy_hYwK5wqz8UfVD*tDOfxTz~O9B1UoDN zAll)vBqIJBD5hAh0K{BS2?c?p1rQEs$LI10I39|GExi z<5HJMWFi1MPJsvr1~F1`uxdd`IMvaENk`(**smH7A)w$x4s@iOKpYnNb%8AqfnEwg z$tKQ@NFWl41RRb;w#8Gv@m>YW%;>TfHR-~AoS**2>F+ph5P~ zk-G}y(Ed>>)%cC_0z=22#vg?O)m1_uRF{GZaK}@S1L5E}!w}Z^5;p`82ZK=em?+oJ zdcnUb1rpDeYzx@gqCg6lh$8as2q-e%j*Q{~WIH~VjK@-Vpo)br=yC~P5dp|R$6$y@ zh%2Z(RliUOYt=!```$Vt1XP{^L>LN7KoM~NmoUsk!Wd=G_?$5f^Zz)ZsSLhpGLYZ6 z4r*RdFT{Llh7+7Y@%}GA6KCqpqL;nRuKrc&!ZPp>si-E72G7TY~~zUl;SBAScGVZa%_`Ed$$g4wqT`(xXv@$mCZ?x z7anMYR|MPwg2VX(4P}pVJ7<>!nAnF0N7TPzEPrE}72O*Bi1Kpy;_Si7>aotDb)%mc z>COIc{~nYm(tDgdnjRJ38}x+*t!{=tDsd@}Xby~d+&NIuUP?p0lF z`rv9o_gOsb?Dkvk9bk=x*&sUTR8{SwrsRU(b!PeHnt{4@T|=p_)eOQ-56#?~_pz_S zuC(7v%Q&>@wpMRyTQYz30xW*$Q(?>CC7fV7X7v92+_TMoEhZxcl&phn>cgWOW}mxY zB?y29ncWs1BLsK2 zHH76_MaqXyE7LFuBeWsgncl4>O`8au`P8!?<dw+>b|H7<%GBhg};ib>DmW zkF%_^>deXcnrVl3hiiE1GthSVG6rKt{f5KC4S^pYkSL)u8;9tvX#eOF_Qyz*gZiKB zC3O4K91@)J59t0Sj7lMtMj9U?JNJCl0S|tRW8};#_ec?o&>ga1xmW#L+U&9&&WCgx zJ$6alYjwP`x1wSMN@IiFew6YE2}@7;Ul zfwiR9$InijdD&1pLPQ%s4C{YHy__Jg9J)*|dG+ed;tO2Ash<-692&WQV~8;*-~~53 zb-ngTeP6I<4tvkN>_h`yOWlRAizLBx!qJ5RJ&fbJx@`qg&3s|v4s+n9dW^MP=k5%-hE3bF{aylSAOy|z2)9z#Too-`dsH_0hk15!_=ArpDTt9 zhExYzz1+LG^?A7t>Nff{rpMZsPpf(F32SQ)^}4x-p0lEG>61gp{7W-$W+7grv49@K z*cGsYO9Rc|8bPkZ?~m?{=(CRPJ#GWKimHWET$WEsE!^;~QZ3_EMo3Zn^xQey;jxc> z0$_Si)I&X%F1S`io@S}Jrf3p5GcrWnQ2ym~VX1D8bV-e&3udQNz>UhffdBw#p14$gD31-iw;jv+?7}MxJDAd;+%k z8a$40jr{{Gvx?aM%BDi`+C@P<> zpLN9pANMe__go6r)@1w5u)FRllA3I-Lh0-0nK7gPWLkuEa`0D|r<7G)FCm+F)LAg9 zrTN$McAIR|K<=vXn2YtuXxyruSTsn0?XSbpBJJat`%e~5ogvwB<~ZH;5LsKhYTX|z zs)~dGmB=HZtimdwD>)8cx-F%uc958#LpdFm5RbdG;E|S7kMYVU9ON6v5%fcJeDj{^ zC&gpW2DFls1FzIMiu+*m{9|=37woq{Y%$_xJThAEQv1mfMy?V)9LAQO$X;l6)O^iO z{941<3>o>)vXVKvB1&21{7#Qp7~)`FmgD|o1@Wcam6u9=Q z%qHtvG$kKtIhS0ub@}9=hwn<~U4+(-JUE!64U05p>{&P!&oHxETggo8VTX*kH6h&N zX!r-x(^r3Wjia%R5^T<#U(UaR2V$+(_#49iHuZh1w{h!cTsUI@SKoWvJakmpnAG9@ z&R!#S=Tpw5{2c{*G%_R;hnaNaVD2hZ@99itzWxQrtjC}*lBpFIQRou z5PR2u)r<_^6FQZd61j9u6s|9~ri83GXvg7o-1L*1=^Mak%b(+yyf5rU?pwWLulWUd z5e>#==RHaec<(NG2EPS0b0Bp;u}&vbqE=kn>mNJs_Y}(EmGAvdIx-ZTh=|HJm}5~N z8ZyiE*qibcukT!a&^@c-nBm)(nj5TuD?zLFwwLY6*4Wv)OtYkDMWDuv52k$s#kwv!%!IZ-p#^=ZmjAFQQw - + diff --git a/frontend/tests/MarkerPopup/MarkerPopup.test.jsx b/frontend/tests/MarkerPopup/MarkerPopup.test.jsx index 9689d6ca..c43dfaac 100644 --- a/frontend/tests/MarkerPopup/MarkerPopup.test.jsx +++ b/frontend/tests/MarkerPopup/MarkerPopup.test.jsx @@ -105,7 +105,7 @@ describe('MarkerPopup with remark', () => { globalThis.fetch.mockRestore(); }); - it('should render marker popup with asterisks when remark is true', () => { + it('should render our own pin with an asterisk badge when remark is true', () => { // eslint-disable-next-line camelcase -- matches backend API schema property name const locationWhenRemarkIsTrue = { ...location, has_remark: true }; act(() => { @@ -122,7 +122,9 @@ describe('MarkerPopup with remark', () => { , ); }); - expect(screen.getByAltText(/Marker-Asterisk/i)).toBeInTheDocument(); + const marker = document.querySelector('.custom-typed-marker-icon'); + expect(marker).toBeInTheDocument(); + expect(marker.querySelector('span')).toHaveTextContent('*'); }); it('should pass custom icon prop when remark is true', () => { @@ -140,15 +142,14 @@ describe('MarkerPopup with remark', () => { ); }); - const marker = screen.getByAltText(/Marker-Asterisk/i); - const leafletMarker = marker.closest('.leaflet-marker-icon'); + const marker = document.querySelector('.custom-typed-marker-icon'); - // When remark is true, marker should have custom asterisk icon - expect(leafletMarker).toBeInTheDocument(); + // When remark is true, marker should have our own pin, not Leaflet's default icon + expect(marker).toBeInTheDocument(); - // Verify custom asterisk icon dimensions (40x48) are applied - const style = window.getComputedStyle(leafletMarker); - expect(style.width).toBe('40px'); // asteriskIcon width - expect(style.height).toBe('48px'); // asteriskIcon height + // Verify our pin's dimensions (36x40) are applied, not Leaflet's default (25x41) + const style = window.getComputedStyle(marker); + expect(style.width).toBe('36px'); + expect(style.height).toBe('40px'); }); }); diff --git a/frontend/tests/MarkerPopup/getTypedMarkerIcon.test.jsx b/frontend/tests/MarkerPopup/getTypedMarkerIcon.test.jsx index 03dec5b6..cd1cc7e8 100644 --- a/frontend/tests/MarkerPopup/getTypedMarkerIcon.test.jsx +++ b/frontend/tests/MarkerPopup/getTypedMarkerIcon.test.jsx @@ -49,7 +49,7 @@ describe('getTypedMarkerIcon', () => { expect(icon).not.toBeNull(); expect(icon.options.html).toContain('https://cdn.example.com/parcel-locker.svg'); expect(icon.options.html).toContain('background-color:black'); // fallback color, no color_field set - expect(icon.options.iconSize).toEqual([72, 80]); + expect(icon.options.iconSize).toEqual([36, 40]); }); it('masks the icon URL through CSS so it picks up the matched color, instead of embedding SVG path data', () => { @@ -136,10 +136,21 @@ describe('getTypedMarkerIcon', () => { expect(icon.options.html).not.toContain('>*'); }); - it('returns null (falls back to the plain asterisk icon) when has_remark is set but nothing matches', () => { + it('returns our own pin in the fallback color with just the badge when has_remark is set but nothing matches', () => { setMarkerStyles('{}'); - expect(getTypedMarkerIcon({ uuid: '1', position: [50, 50], has_remark: true })).toBeNull(); + const icon = getTypedMarkerIcon({ uuid: '1', position: [50, 50], has_remark: true }); + + expect(icon).not.toBeNull(); + expect(icon.options.html).toContain('background-color:black'); // fallback color + expect(icon.options.html).toContain('>*'); + expect(icon.options.html).not.toContain('custom-typed-marker-type-icon'); + }); + + it('still returns null when there is neither a match nor a remark to show', () => { + setMarkerStyles('{}'); + + expect(getTypedMarkerIcon({ uuid: '1', position: [50, 50] })).toBeNull(); }); it('uses default_color from MARKER_STYLES when the matched value has no color entry', () => { From 4ff6c8ac92efb3448ba5b0e63c3ec4b66233e784 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Krzysztof=20Ko=C5=82odzi=C5=84ski?= Date: Thu, 20 Aug 2026 12:48:26 +0200 Subject: [PATCH 13/28] cleanup --- e2e-tests/e2e_test_data_initial.json | 3 +-- .../MarkerPopup/getTypedMarkerIcon.jsx | 26 ++++++++----------- .../tests/MarkerPopup/MarkerPopup.test.jsx | 6 ++--- .../MarkerPopup/getTypedMarkerIcon.test.jsx | 7 ++--- 4 files changed, 19 insertions(+), 23 deletions(-) diff --git a/e2e-tests/e2e_test_data_initial.json b/e2e-tests/e2e_test_data_initial.json index fc66933f..a0c95225 100644 --- a/e2e-tests/e2e_test_data_initial.json +++ b/e2e-tests/e2e_test_data_initial.json @@ -283,8 +283,7 @@ "10": "#2e7d32", "30": "#ef6c00", "50": "#c62828" - }, - "default_color": "#2a81cb" + } }, "visible_data": [ "remark", diff --git a/frontend/src/components/MarkerPopup/getTypedMarkerIcon.jsx b/frontend/src/components/MarkerPopup/getTypedMarkerIcon.jsx index 0cf301ed..a703841a 100644 --- a/frontend/src/components/MarkerPopup/getTypedMarkerIcon.jsx +++ b/frontend/src/components/MarkerPopup/getTypedMarkerIcon.jsx @@ -8,13 +8,15 @@ import ReactDOMServer from 'react-dom/server'; // with its own padding would. import PIN_SHAPE_URL from '../../res/svg/marker-pin.svg'; -const PIN_WIDTH = 36; -const PIN_HEIGHT = 40; +const PIN_WIDTH = 45; +const PIN_HEIGHT = 50; +// The marker's default color (used whenever color_field doesn't match) is +// always the page's own secondary color, not a separately configurable value. const FALLBACK_COLOR = globalThis.SECONDARY_COLOR || 'black'; -const TYPE_ICON_SIZE = 16; -const TYPE_ICON_OFFSET_TOP = 6.5; -const TYPE_ICON_OFFSET_LEFT = 10; +const TYPE_ICON_SIZE = 20; +const TYPE_ICON_OFFSET_TOP = 8; +const TYPE_ICON_OFFSET_LEFT = 12; const maskStyle = (url, color) => ({ backgroundColor: color, @@ -61,8 +63,8 @@ const PinIcon = ({ color, typeIconUrl, hasRemark }) => ( style={{ position: 'absolute', top: 1, - left: 19, - fontSize: 17, + left: 24, + fontSize: 21, fontWeight: 'bold', lineHeight: 1, color: '#ffffff', @@ -96,13 +98,7 @@ PinIcon.propTypes = { */ const getTypedMarkerIcon = place => { const markerStyles = globalThis.MARKER_STYLES || {}; - const { - icon_field: iconField, - color_field: colorField, - icons, - colors, - default_color: defaultColor, - } = markerStyles; + const { icon_field: iconField, color_field: colorField, icons, colors } = markerStyles; const typeIconUrl = icons?.[place[iconField]] || ''; const matchedColor = colors?.[place[colorField]] || ''; @@ -115,7 +111,7 @@ const getTypedMarkerIcon = place => { return new DivIcon({ html: ReactDOMServer.renderToString( , diff --git a/frontend/tests/MarkerPopup/MarkerPopup.test.jsx b/frontend/tests/MarkerPopup/MarkerPopup.test.jsx index c43dfaac..5beab11b 100644 --- a/frontend/tests/MarkerPopup/MarkerPopup.test.jsx +++ b/frontend/tests/MarkerPopup/MarkerPopup.test.jsx @@ -147,9 +147,9 @@ describe('MarkerPopup with remark', () => { // When remark is true, marker should have our own pin, not Leaflet's default icon expect(marker).toBeInTheDocument(); - // Verify our pin's dimensions (36x40) are applied, not Leaflet's default (25x41) + // Verify our pin's dimensions (45x50) are applied, not Leaflet's default (25x41) const style = window.getComputedStyle(marker); - expect(style.width).toBe('36px'); - expect(style.height).toBe('40px'); + expect(style.width).toBe('45px'); + expect(style.height).toBe('50px'); }); }); diff --git a/frontend/tests/MarkerPopup/getTypedMarkerIcon.test.jsx b/frontend/tests/MarkerPopup/getTypedMarkerIcon.test.jsx index cd1cc7e8..9715e681 100644 --- a/frontend/tests/MarkerPopup/getTypedMarkerIcon.test.jsx +++ b/frontend/tests/MarkerPopup/getTypedMarkerIcon.test.jsx @@ -49,7 +49,7 @@ describe('getTypedMarkerIcon', () => { expect(icon).not.toBeNull(); expect(icon.options.html).toContain('https://cdn.example.com/parcel-locker.svg'); expect(icon.options.html).toContain('background-color:black'); // fallback color, no color_field set - expect(icon.options.iconSize).toEqual([36, 40]); + expect(icon.options.iconSize).toEqual([45, 50]); }); it('masks the icon URL through CSS so it picks up the matched color, instead of embedding SVG path data', () => { @@ -153,7 +153,7 @@ describe('getTypedMarkerIcon', () => { expect(getTypedMarkerIcon({ uuid: '1', position: [50, 50] })).toBeNull(); }); - it('uses default_color from MARKER_STYLES when the matched value has no color entry', () => { + it('ignores a configured default_color and uses the page fallback color instead', () => { setMarkerStyles(`{ "icon_field": "pointType", "icons": { "parcelLocker": "https://cdn.example.com/parcel-locker.svg" }, @@ -166,6 +166,7 @@ describe('getTypedMarkerIcon', () => { pointType: 'parcelLocker', }); - expect(icon.options.html).toContain('#123456'); + expect(icon.options.html).not.toContain('#123456'); + expect(icon.options.html).toContain('background-color:black'); }); }); From 0c3ee6a4e74d18c13bfbdac606c5301c7cd85e8e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Krzysztof=20Ko=C5=82odzi=C5=84ski?= Date: Thu, 20 Aug 2026 12:52:11 +0200 Subject: [PATCH 14/28] cleanup comment --- frontend/tests/MarkerPopup/getTypedMarkerIcon.test.jsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frontend/tests/MarkerPopup/getTypedMarkerIcon.test.jsx b/frontend/tests/MarkerPopup/getTypedMarkerIcon.test.jsx index 9715e681..abf79e98 100644 --- a/frontend/tests/MarkerPopup/getTypedMarkerIcon.test.jsx +++ b/frontend/tests/MarkerPopup/getTypedMarkerIcon.test.jsx @@ -67,7 +67,7 @@ describe('getTypedMarkerIcon', () => { pointStatus: 'open', }); - // both the pin body (map-pin-fill.svg) and the type icon are CSS-masked + // both the pin body (our own marker-pin.svg) and the type icon are CSS-masked //
s tinted via background-color, not inline SVG , so // any icon set (not just single-path ones) works for either. expect(icon.options.html).toContain( From 3597a6636527be8027c691302da920cc9d95c3d8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Krzysztof=20Ko=C5=82odzi=C5=84ski?= Date: Thu, 20 Aug 2026 12:58:16 +0200 Subject: [PATCH 15/28] more human readable icons --- tests/unit_tests/test_db.py | 39 ++++++++++++++++++++------------ tests/unit_tests/test_goodmap.py | 4 +++- 2 files changed, 28 insertions(+), 15 deletions(-) diff --git a/tests/unit_tests/test_db.py b/tests/unit_tests/test_db.py index 29616092..9759c97c 100644 --- a/tests/unit_tests/test_db.py +++ b/tests/unit_tests/test_db.py @@ -308,7 +308,9 @@ def test_json_file_db_get_meta_data_empty(): "marker_styles": { "icon_field": "type_of_place", "color_field": "status", - "icons": {"parcel_locker": "M0 0h16v16H0z"}, + "icons": { + "parcel_locker": "https://cdn.jsdelivr.net/npm/@phosphor-icons/core@2/assets/fill/package-fill.svg" + }, "colors": {"open": "#2e7d32"}, } } @@ -322,7 +324,9 @@ def test_json_file_db_get_marker_styles(): assert result == { "icon_field": "type_of_place", "color_field": "status", - "icons": {"parcel_locker": "M0 0h16v16H0z"}, + "icons": { + "parcel_locker": "https://cdn.jsdelivr.net/npm/@phosphor-icons/core@2/assets/fill/package-fill.svg" + }, "colors": {"open": "#2e7d32"}, } @@ -377,23 +381,26 @@ def test_google_json_db_get_meta_data_empty(mock_cli): @mock.patch("platzky.db.google_json_db.Client") def test_google_json_db_get_marker_styles(mock_cli): - mock_cli.return_value.bucket.return_value.blob.return_value.download_as_text.return_value = ( - json.dumps( - { - "map": { - "marker_styles": { - "icon_field": "type_of_place", - "icons": {"parcel_locker": "M0 0h16v16H0z"}, - } + blob = mock_cli.return_value.bucket.return_value.blob.return_value + blob.download_as_text.return_value = json.dumps( + { + "map": { + "marker_styles": { + "icon_field": "type_of_place", + "icons": { + "parcel_locker": "https://cdn.jsdelivr.net/npm/@phosphor-icons/core@2/assets/fill/package-fill.svg" + }, } } - ) + } ) db = GoogleJsonDb("bucket", "blob") result = google_json_db_get_marker_styles(db) assert result == { "icon_field": "type_of_place", - "icons": {"parcel_locker": "M0 0h16v16H0z"}, + "icons": { + "parcel_locker": "https://cdn.jsdelivr.net/npm/@phosphor-icons/core@2/assets/fill/package-fill.svg" + }, } @@ -1170,7 +1177,9 @@ def test_mongodb_db_get_marker_styles(mock_client): "_id": "map_config", "marker_styles": { "icon_field": "type_of_place", - "icons": {"parcel_locker": "M0 0h16v16H0z"}, + "icons": { + "parcel_locker": "https://cdn.jsdelivr.net/npm/@phosphor-icons/core@2/assets/fill/package-fill.svg" + }, }, } @@ -1178,7 +1187,9 @@ def test_mongodb_db_get_marker_styles(mock_client): result = mongodb_db_get_marker_styles(db) assert result == { "icon_field": "type_of_place", - "icons": {"parcel_locker": "M0 0h16v16H0z"}, + "icons": { + "parcel_locker": "https://cdn.jsdelivr.net/npm/@phosphor-icons/core@2/assets/fill/package-fill.svg" + }, } diff --git a/tests/unit_tests/test_goodmap.py b/tests/unit_tests/test_goodmap.py index f8a4766d..0b89fa1d 100644 --- a/tests/unit_tests/test_goodmap.py +++ b/tests/unit_tests/test_goodmap.py @@ -122,7 +122,9 @@ def test_map_route_includes_marker_styles(): "categories": {"type_of_place": ["parcel_locker", "container"]}, "marker_styles": { "icon_field": "type_of_place", - "icons": {"parcel_locker": "M0 0h16v16H0z"}, + "icons": { + "parcel_locker": "https://cdn.jsdelivr.net/npm/@phosphor-icons/core@2/assets/fill/package-fill.svg" + }, "colors": {}, }, }, From 3457d332b3e53c691829ba6c2a8c11da4a091fcd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Krzysztof=20Ko=C5=82odzi=C5=84ski?= Date: Thu, 20 Aug 2026 14:25:05 +0200 Subject: [PATCH 16/28] some cleanup --- frontend/src/components/MarkerPopup/getTypedMarkerIcon.jsx | 4 ---- 1 file changed, 4 deletions(-) diff --git a/frontend/src/components/MarkerPopup/getTypedMarkerIcon.jsx b/frontend/src/components/MarkerPopup/getTypedMarkerIcon.jsx index a703841a..96b04b3a 100644 --- a/frontend/src/components/MarkerPopup/getTypedMarkerIcon.jsx +++ b/frontend/src/components/MarkerPopup/getTypedMarkerIcon.jsx @@ -2,10 +2,6 @@ import React from 'react'; import PropTypes from 'prop-types'; import { DivIcon } from 'leaflet'; import ReactDOMServer from 'react-dom/server'; -// Custom balloon pin (sharp point, solid head, no third-party asset/CDN) - see -// the .svg file. Its point sits exactly on the viewBox's bottom edge, so the -// anchor below doesn't need any empirical correction the way a borrowed icon -// with its own padding would. import PIN_SHAPE_URL from '../../res/svg/marker-pin.svg'; const PIN_WIDTH = 45; From d2096e47a1b23df720d8e08d7fe0c63a806ee045 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Krzysztof=20Ko=C5=82odzi=C5=84ski?= Date: Thu, 20 Aug 2026 14:38:53 +0200 Subject: [PATCH 17/28] cleanup --- frontend/src/components/MarkerPopup/MarkerPopup.jsx | 4 ---- 1 file changed, 4 deletions(-) diff --git a/frontend/src/components/MarkerPopup/MarkerPopup.jsx b/frontend/src/components/MarkerPopup/MarkerPopup.jsx index 558a13c8..a2844699 100644 --- a/frontend/src/components/MarkerPopup/MarkerPopup.jsx +++ b/frontend/src/components/MarkerPopup/MarkerPopup.jsx @@ -107,10 +107,6 @@ const MarkerPopup = ({ place }) => { eventHandlers: { click: handleMarkerClick, }, - // getTypedMarkerIcon renders as a
, not an , so 'alt' has no - // visible effect once it returns an icon - kept as plain text for the - // one case it still applies to: Leaflet's own default marker below. - alt: 'Marker', }; // getTypedMarkerIcon returns our own pin whenever there's a marker_styles From 121cd64e2f7e119c79c91cb85f28ea04be7ebe9d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Krzysztof=20Ko=C5=82odzi=C5=84ski?= Date: Thu, 20 Aug 2026 14:39:20 +0200 Subject: [PATCH 18/28] cleanup comments --- frontend/src/components/MarkerPopup/MarkerPopup.jsx | 5 ----- 1 file changed, 5 deletions(-) diff --git a/frontend/src/components/MarkerPopup/MarkerPopup.jsx b/frontend/src/components/MarkerPopup/MarkerPopup.jsx index a2844699..9269c777 100644 --- a/frontend/src/components/MarkerPopup/MarkerPopup.jsx +++ b/frontend/src/components/MarkerPopup/MarkerPopup.jsx @@ -109,11 +109,6 @@ const MarkerPopup = ({ place }) => { }, }; - // getTypedMarkerIcon returns our own pin whenever there's a marker_styles - // match or a remark to badge, null only for a plain, unremarked location - - // which then falls back to Leaflet's default marker. Only add an icon prop - // when we actually have a custom icon: passing icon={undefined} causes - // errors in MarkerClusterGroup during cluster zoom animations. const typedIcon = getTypedMarkerIcon(place); if (typedIcon) { markerProps.icon = typedIcon; From 56d1b13d99970fd861a709bbca828b58d283eebd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Krzysztof=20Ko=C5=82odzi=C5=84ski?= Date: Thu, 20 Aug 2026 14:51:36 +0200 Subject: [PATCH 19/28] cleanup --- tests/unit_tests/test_goodmap.py | 36 ++++++++++---------------------- 1 file changed, 11 insertions(+), 25 deletions(-) diff --git a/tests/unit_tests/test_goodmap.py b/tests/unit_tests/test_goodmap.py index 0b89fa1d..7d3e654b 100644 --- a/tests/unit_tests/test_goodmap.py +++ b/tests/unit_tests/test_goodmap.py @@ -107,11 +107,13 @@ def test_frontend_lib_url_uses_bundled_static_when_present(): assert 'src="/static/frontend/index.min.js"' in response.data.decode("utf-8") -def test_map_route_includes_marker_styles(): +def test_map_route_marker_styles(): """The frontend picks pin icon/color per marker_styles.config's iconField/colorField at runtime from window.MARKER_STYLES - a deployment-specific lookup table that lives - in the database (like categories/visible_data), not hardcoded in the frontend build.""" - config = GoodmapConfig( + in the database (like categories/visible_data), not hardcoded in the frontend build. + Deployments that don't configure it get an empty object instead, so the frontend + falls back to Leaflet's default marker - no behavior change.""" + configured_config = GoodmapConfig( APP_NAME="test_app", SECRET_KEY="test_secret", USE_WWW=False, @@ -131,11 +133,10 @@ def test_map_route_includes_marker_styles(): TYPE="json", ), ) - app = goodmap.create_app_from_config(config) - app.config["WTF_CSRF_ENABLED"] = False # NOSONAR - client = app.test_client() + configured_app = goodmap.create_app_from_config(configured_config) + configured_app.config["WTF_CSRF_ENABLED"] = False # NOSONAR - response = client.get("/map") + response = configured_app.test_client().get("/map") assert response.status_code == 200 response_text = response.data.decode("utf-8") @@ -143,25 +144,10 @@ def test_map_route_includes_marker_styles(): assert "icon_field" in response_text assert "parcel_locker" in response_text + unconfigured_app = goodmap.create_app_from_config(_minimal_config()) + unconfigured_app.config["WTF_CSRF_ENABLED"] = False # NOSONAR -def test_map_route_marker_styles_defaults_to_empty(): - """Deployments that don't configure marker_styles get an empty object, so the - frontend falls back to Leaflet's default marker - no behavior change.""" - config = GoodmapConfig( - APP_NAME="test_app", - SECRET_KEY="test_secret", - USE_WWW=False, - BLOG_PREFIX="/blog", - DB=JsonDbConfig( - DATA={"site_content": {"pages": []}, "categories": {}}, - TYPE="json", - ), - ) - app = goodmap.create_app_from_config(config) - app.config["WTF_CSRF_ENABLED"] = False # NOSONAR - client = app.test_client() - - response = client.get("/map") + response = unconfigured_app.test_client().get("/map") assert response.status_code == 200 assert "window.MARKER_STYLES={};" in response.data.decode("utf-8") From d62f19d4ff30a17476a3b260325d070d8aa1dd2d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Krzysztof=20Ko=C5=82odzi=C5=84ski?= Date: Thu, 20 Aug 2026 17:54:45 +0200 Subject: [PATCH 20/28] fixes --- goodmap/data_models/location.py | 7 ++++- goodmap/goodmap.py | 14 ++++++++-- tests/unit_tests/data_models/test_location.py | 28 ++++++++++++++++++- tests/unit_tests/test_core_api.py | 7 +++-- 4 files changed, 49 insertions(+), 7 deletions(-) diff --git a/goodmap/data_models/location.py b/goodmap/data_models/location.py index ea8c4024..e5281525 100644 --- a/goodmap/data_models/location.py +++ b/goodmap/data_models/location.py @@ -6,6 +6,7 @@ """ import warnings +from collections.abc import Iterable from typing import Annotated, Any, ClassVar, Type, cast from annotated_types import Ge, Le @@ -234,6 +235,7 @@ def _build_field_definition(field_type_str: str, allowed_values: frozenset[str]) def create_location_model( obligatory_fields: list[tuple[str, str]] | list[tuple[str, Type[Any]]], categories: dict[str, list[str]], + marker_style_fields: Iterable[str] = (), ) -> Type[BaseModel]: """Dynamically create a Location model with additional required fields. @@ -245,6 +247,9 @@ def create_location_model( - String type name: "str", "list", "int", "float", "bool", "dict" - Python type object: str, list, int, etc. (deprecated) categories: Dict mapping field names to allowed values (enums). + marker_style_fields: Field names referenced by the deployment's marker_styles + config (icon_field/color_field) - the only ones whose values + need to ride along on basic_info() for pin styling. Returns: A Location model class extending LocationBase with additional fields @@ -275,5 +280,5 @@ def create_location_model( __module__="goodmap.data_models.location", **fields, ) - location_model.pin_marker_fields = frozenset(categories.keys()) & fields.keys() + location_model.pin_marker_fields = frozenset(marker_style_fields) & fields.keys() return location_model diff --git a/goodmap/goodmap.py b/goodmap/goodmap.py index 56ee2173..ecb80ebb 100644 --- a/goodmap/goodmap.py +++ b/goodmap/goodmap.py @@ -131,8 +131,18 @@ def _setup_location_model( except (KeyError, AttributeError): categories = {} - if categories: - location_model = create_location_model(obligatory_fields, categories) + try: + marker_styles = extended_db.get_marker_styles() + except (KeyError, AttributeError): + marker_styles = {} + marker_style_fields = { + field + for field in (marker_styles.get("icon_field"), marker_styles.get("color_field")) + if field is not None + } + + if categories or marker_style_fields: + location_model = create_location_model(obligatory_fields, categories, marker_style_fields) extended_db = extend_db_with_goodmap_queries(extended_db, location_model) return obligatory_fields, categories, location_model, extended_db diff --git a/tests/unit_tests/data_models/test_location.py b/tests/unit_tests/data_models/test_location.py index a9cb1d5c..611b4765 100644 --- a/tests/unit_tests/data_models/test_location.py +++ b/tests/unit_tests/data_models/test_location.py @@ -131,11 +131,12 @@ def test_category_validation_rejects_invalid_list_item(): def test_basic_info_includes_category_field_values(): - """basic_info() should surface category field values (for pin icon/color + """basic_info() should surface marker_style_fields' values (for pin icon/color selection) alongside the existing uuid/position/has_remark.""" location_model = create_location_model( obligatory_fields=[("type_of_place", "str"), ("name", "str")], categories={"type_of_place": ["parcel_locker", "container"]}, + marker_style_fields={"type_of_place"}, ) location = location_model( uuid="1", name="test", type_of_place="parcel_locker", position=(50, 50) @@ -158,6 +159,31 @@ def test_basic_info_omits_category_fields_when_none_configured(): assert location.basic_info() == {"uuid": "1", "position": (50, 50), "has_remark": False} +def test_basic_info_omits_category_fields_not_referenced_by_marker_style_fields(): + """A category not used by marker_styles.icon_field/color_field shouldn't ride + along on basic_info() just because it's a category - only marker_style_fields + controls what pin styling needs, not the full category set (a deployment can + have categories unrelated to marker display, e.g. used only for filtering).""" + location_model = create_location_model( + obligatory_fields=[("type_of_place", "str"), ("accessibility", "str")], + categories={ + "type_of_place": ["parcel_locker", "container"], + "accessibility": ["wheelchair", "none"], + }, + marker_style_fields={"type_of_place"}, + ) + location = location_model( + uuid="1", type_of_place="parcel_locker", accessibility="wheelchair", position=(50, 50) + ) + location = cast(LocationBase, location) + assert location.basic_info() == { + "uuid": "1", + "position": (50, 50), + "has_remark": False, + "type_of_place": "parcel_locker", + } + + def test_create_location_model_with_int_field(): """Test that non-str simple fields (like int) are created without max_length.""" location_model = create_location_model(obligatory_fields=[("capacity", "int")], categories={}) diff --git a/tests/unit_tests/test_core_api.py b/tests/unit_tests/test_core_api.py index 2a7ec185..39a54b92 100644 --- a/tests/unit_tests/test_core_api.py +++ b/tests/unit_tests/test_core_api.py @@ -316,13 +316,14 @@ def test_get_locations_accepts_valid_and_undeclared_parameters(test_app, query): def test_get_locations_includes_category_field_for_pin_styling(): - """/api/locations should surface category field values (e.g. a point-type - category), so the frontend can pick a marker icon/color without a full - per-location detail fetch.""" + """/api/locations should surface the field marker_styles.icon_field points at + (e.g. a point-type category), so the frontend can pick a marker icon/color + without a full per-location detail fetch.""" client = create_test_app( db_overrides={ "categories": {"point_type": ["parcel_locker", "container"]}, "location_obligatory_fields": [("point_type", "str"), ("name", "str")], + "marker_styles": {"icon_field": "point_type", "icons": {}, "colors": {}}, "data": [ { "name": "locker-1", From e2a608a434646e995f00732069aa73a2dce69c3f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Krzysztof=20Ko=C5=82odzi=C5=84ski?= Date: Thu, 20 Aug 2026 22:24:01 +0200 Subject: [PATCH 21/28] fixes --- .../components/MarkerPopup/MarkerPopup.jsx | 11 +- frontend/src/services/http/endpoints.js | 8 ++ frontend/src/services/http/httpService.js | 27 ++++ .../tests/MarkerPopup/MarkerPopup.test.jsx | 69 ++++++++++ goodmap/api/api_models.py | 27 ++++ goodmap/api/core_api.py | 29 +++++ goodmap/data_models/location.py | 14 +- tests/unit_tests/data_models/test_location.py | 33 ++--- tests/unit_tests/test_core_api.py | 122 +++++++++++++++++- 9 files changed, 304 insertions(+), 36 deletions(-) diff --git a/frontend/src/components/MarkerPopup/MarkerPopup.jsx b/frontend/src/components/MarkerPopup/MarkerPopup.jsx index 9269c777..60ae7752 100644 --- a/frontend/src/components/MarkerPopup/MarkerPopup.jsx +++ b/frontend/src/components/MarkerPopup/MarkerPopup.jsx @@ -5,11 +5,13 @@ import { isMobile } from 'react-device-detect'; import { useTranslation } from 'react-i18next'; import httpService from '../../services/http/httpService'; import useMapStore from '../Map/store/map.store'; +import useMarkerStylesStore from '../Map/store/markerStyles.store'; import LocationDetailsBox from './LocationDetails'; import MobilePopup from './MobilePopup'; import DesktopPopup from './DesktopPopup'; import getTypedMarkerIcon from './getTypedMarkerIcon'; +import requestMarkerStyle from './requestMarkerStyle'; /** * Wrapper component that fetches full location details and renders them in a popup. @@ -81,6 +83,7 @@ LocationDetailsBoxWrapper.propTypes = { const MarkerPopup = ({ place }) => { const selectedLocationId = useMapStore(state => state.selectedLocationId); const setSelectedLocationId = useMapStore(state => state.setSelectedLocationId); + const lazyMarkerStyle = useMarkerStylesStore(state => state.stylesByUuid[place.uuid]); const [isClicked, setIsClicked] = useState(false); // TODO: this only opens the popup if `place`'s Marker is actually attached to @@ -102,14 +105,20 @@ const MarkerPopup = ({ place }) => { setIsClicked(true); }; + const handleMarkerVisible = () => { + requestMarkerStyle(place.uuid); + }; + const markerProps = { position: place.position, eventHandlers: { click: handleMarkerClick, + add: handleMarkerVisible, }, }; - const typedIcon = getTypedMarkerIcon(place); + const styledPlace = lazyMarkerStyle ? { ...place, ...lazyMarkerStyle } : place; + const typedIcon = getTypedMarkerIcon(styledPlace); if (typedIcon) { markerProps.icon = typedIcon; } diff --git a/frontend/src/services/http/endpoints.js b/frontend/src/services/http/endpoints.js index 282b60fa..6f53f501 100644 --- a/frontend/src/services/http/endpoints.js +++ b/frontend/src/services/http/endpoints.js @@ -28,6 +28,14 @@ export const LOCATIONS = '/api/locations'; */ export const LOCATIONS_CLUSTERED = '/api/locations-clustered'; +/** + * API endpoint for lazily fetching marker styling field values (whatever + * marker_styles.icon_field/color_field point at) for specific locations, by uuid. + * Used once a location's marker becomes individually visible, instead of upfront + * for every location - see lazy-load-marker-styling-plan.md. + */ +export const LOCATIONS_MARKER_STYLES = '/api/locations/marker-styles'; + /** * External API endpoint for address search (forward geocoding) using OpenStreetMap Nominatim. * Converts addresses/place names to geographic coordinates. diff --git a/frontend/src/services/http/httpService.js b/frontend/src/services/http/httpService.js index 01999f3c..0962b8a6 100644 --- a/frontend/src/services/http/httpService.js +++ b/frontend/src/services/http/httpService.js @@ -5,6 +5,7 @@ import { LOCATIONS, SEARCH_ADDRESS, LOCATIONS_CLUSTERED, + LOCATIONS_MARKER_STYLES, } from './endpoints'; import useMapStore from '../../components/Map/store/map.store'; @@ -207,6 +208,32 @@ const httpService = { } }, + /** + * Fetches marker styling field values for specific locations, by uuid. + * Used to lazily fetch pin icon/color data once a marker becomes individually + * visible, instead of upfront for every location. + * + * @param {string[]} uuids - Location UUIDs to fetch styling for + * @returns {Promise>} Promise resolving to a map of + * uuid -> styling field values; uuids with no styling are simply absent + */ + getMarkerStyles: async uuids => { + if (!uuids.length) { + return {}; + } + const params = new URLSearchParams(); + for (const uuid of uuids) { + params.append('uuid', uuid); + } + const response = await fetch(`${LOCATIONS_MARKER_STYLES}?${params.toString()}`, { + method: 'GET', + headers: { + 'Content-Type': 'application/json', + }, + }); + return jsonOrThrow(response, 'marker styles'); + }, + /** * Searches for addresses using OpenStreetMap Nominatim API. * Returns up to 5 results with geocoded coordinates. diff --git a/frontend/tests/MarkerPopup/MarkerPopup.test.jsx b/frontend/tests/MarkerPopup/MarkerPopup.test.jsx index 5beab11b..27807b94 100644 --- a/frontend/tests/MarkerPopup/MarkerPopup.test.jsx +++ b/frontend/tests/MarkerPopup/MarkerPopup.test.jsx @@ -4,6 +4,7 @@ import { render, screen, fireEvent, act, waitFor } from '@testing-library/react' import { MapContainer } from 'react-leaflet'; import MarkerPopup from '../../src/components/MarkerPopup/MarkerPopup'; import httpService from '../../src/services/http/httpService'; +import useMarkerStylesStore from '../../src/components/Map/store/markerStyles.store'; jest.mock('../../src/services/http/httpService'); @@ -153,3 +154,71 @@ describe('MarkerPopup with remark', () => { expect(style.height).toBe('50px'); }); }); + +describe('MarkerPopup lazy marker styling', () => { + beforeEach(() => { + jest.useFakeTimers(); + useMarkerStylesStore.setState({ stylesByUuid: {} }); + globalThis.MARKER_STYLES = { + icon_field: 'pointType', // eslint-disable-line camelcase -- matches backend API schema property name + icons: { parcelLocker: 'https://cdn.example.com/parcel-locker.svg' }, + }; + httpService.getMarkerStyles.mockResolvedValue({ + [location.uuid]: { pointType: 'parcelLocker' }, + }); + }); + + afterEach(() => { + jest.useRealTimers(); + delete globalThis.MARKER_STYLES; + }); + + it('fetches marker styling once the marker becomes individually visible', async () => { + await act(async () => { + render( + + + , + ); + }); + + expect(httpService.getMarkerStyles).not.toHaveBeenCalled(); + + await act(async () => { + jest.advanceTimersByTime(200); + await Promise.resolve(); + }); + + expect(httpService.getMarkerStyles).toHaveBeenCalledWith([location.uuid]); + }); + + it('re-renders the marker with the lazily-fetched icon once it arrives', async () => { + await act(async () => { + render( + + + , + ); + }); + + // Nothing matched yet - default Leaflet icon, no custom pin + expect(document.querySelector('.custom-typed-marker-icon')).not.toBeInTheDocument(); + + await act(async () => { + jest.advanceTimersByTime(200); + await Promise.resolve(); + }); + + const marker = document.querySelector('.custom-typed-marker-icon'); + expect(marker).toBeInTheDocument(); + expect(marker.innerHTML).toContain('https://cdn.example.com/parcel-locker.svg'); + }); +}); diff --git a/goodmap/api/api_models.py b/goodmap/api/api_models.py index e59b7915..cbcb0c72 100644 --- a/goodmap/api/api_models.py +++ b/goodmap/api/api_models.py @@ -82,6 +82,33 @@ class LocationList(RootModel[list[LocationBasicInfo]]): """List of points, each with identity and position only.""" +class LocationMarkerStyles(RootModel[dict[str, dict[str, Any]]]): + """Map of uuid -> pin styling field values (whatever marker_styles.icon_field/ + color_field point at), for lazily fetching styling once a marker becomes + individually visible instead of getting it upfront for every location. + Unknown/missing uuids are simply absent from the response, not an error.""" + + +class MarkerStylesQueryParams(BaseModel): + """Query parameters of the marker styles lazy-loading endpoint.""" + + uuid: list[str] = Field(default_factory=list, description="Location UUIDs to fetch styling for") + + +def marker_style_values(location: BaseModel, style_fields: frozenset[str]) -> dict[str, Any]: + """Pin styling field values for `location`, as /api/locations/marker-styles returns them. + + This is API response shaping, not something the location domain model needs to + know how to do itself - it belongs alongside the models it fills, not on + LocationBase. + """ + return { + field: value + for field in sorted(style_fields) + if (value := getattr(location, field, None)) is not None + } + + class ClusterInfo(BaseModel): """One entry of the clustered list: either a single point or a cluster of them.""" diff --git a/goodmap/api/core_api.py b/goodmap/api/core_api.py index 09be7455..5423ed18 100644 --- a/goodmap/api/core_api.py +++ b/goodmap/api/core_api.py @@ -25,12 +25,15 @@ LanguagesResponse, LocationDetail, LocationList, + LocationMarkerStyles, LocationQueryParams, LocationReportRequest, LocationReportResponse, LocationSchemaResponse, + MarkerStylesQueryParams, SuccessResponse, VersionResponse, + marker_style_values, ) from goodmap.clustering import ( MAX_ZOOM, @@ -418,6 +421,32 @@ def get_location(location_id): formatted_data = prepare_pin(location.model_dump(), visible_data, meta_data, shortcodes) return jsonify(formatted_data) + @core_api_blueprint.route("/locations/marker-styles", methods=["GET"]) + @spec.validate( + tags=[TAG_MAP_DATA], + query=MarkerStylesQueryParams, + resp=Response(HTTP_200=LocationMarkerStyles), + ) + def get_locations_marker_styles(): + """Get pin styling field values for specific locations, by uuid. + + For lazily fetching marker_styles-relevant field values only once a + client-side-clustered marker becomes individually visible, instead of + the frontend getting them upfront for every location (see + lazy-load-marker-styling-plan.md). Unknown or missing uuids are + silently omitted from the response rather than erroring the whole + request - a marker that's re-clustered mid-flight isn't a client bug. + """ + result: dict[str, dict[str, Any]] = {} + for location_uuid in request.args.getlist("uuid"): + location = database.get_location(location_uuid) + if location is None: + continue + styling = marker_style_values(location, location_model.pin_marker_fields) + if styling: + result[location_uuid] = styling + return jsonify(result) + @core_api_blueprint.route("/version", methods=["GET"]) @spec.validate(tags=[TAG_META], resp=Response(HTTP_200=VersionResponse)) def get_version(): diff --git a/goodmap/data_models/location.py b/goodmap/data_models/location.py index e5281525..6646bd8e 100644 --- a/goodmap/data_models/location.py +++ b/goodmap/data_models/location.py @@ -91,18 +91,16 @@ def model_dump(self, **kwargs) -> dict[str, Any]: return super().model_dump(**kwargs) def basic_info(self) -> dict[str, Any]: - """Get basic location information summary. + """Get basic location information summary: identity and position only. - Includes the uuid/position/remark flag always shown on the map, plus the - value of any category field named in ``pin_marker_fields`` - enough for the - frontend to choose a pin icon/color without fetching full location detail. + Includes the uuid/position/remark flag always shown on the map. Marker + styling field values are deliberately not here - see + ``goodmap.api.api_models.marker_style_values``, read separately and only + once a point is individually visible (not folded into a cluster), so + points that aren't don't pay for it. """ data = self.model_dump(include={"uuid", "position"}) data["has_remark"] = bool(self.remark) - for field in sorted(self.pin_marker_fields): - value = getattr(self, field, None) - if value is not None: - data[field] = value return data diff --git a/tests/unit_tests/data_models/test_location.py b/tests/unit_tests/data_models/test_location.py index 611b4765..9af9a3e2 100644 --- a/tests/unit_tests/data_models/test_location.py +++ b/tests/unit_tests/data_models/test_location.py @@ -1,5 +1,5 @@ import warnings -from typing import cast +from typing import Type, cast import pytest @@ -130,9 +130,10 @@ def test_category_validation_rejects_invalid_list_item(): location_model(uuid="2", tags=["red", "yellow"], position=(50, 50)) -def test_basic_info_includes_category_field_values(): - """basic_info() should surface marker_style_fields' values (for pin icon/color - selection) alongside the existing uuid/position/has_remark.""" +def test_basic_info_omits_marker_style_field_values(): + """basic_info() carries identity/position only - marker styling values are + fetched separately (see goodmap.api.api_models.marker_style_values and + lazy-load-marker-styling-plan.md), only once a marker is actually visible.""" location_model = create_location_model( obligatory_fields=[("type_of_place", "str"), ("name", "str")], categories={"type_of_place": ["parcel_locker", "container"]}, @@ -142,12 +143,7 @@ def test_basic_info_includes_category_field_values(): uuid="1", name="test", type_of_place="parcel_locker", position=(50, 50) ) location = cast(LocationBase, location) - assert location.basic_info() == { - "uuid": "1", - "position": (50, 50), - "has_remark": False, - "type_of_place": "parcel_locker", - } + assert location.basic_info() == {"uuid": "1", "position": (50, 50), "has_remark": False} def test_basic_info_omits_category_fields_when_none_configured(): @@ -159,9 +155,9 @@ def test_basic_info_omits_category_fields_when_none_configured(): assert location.basic_info() == {"uuid": "1", "position": (50, 50), "has_remark": False} -def test_basic_info_omits_category_fields_not_referenced_by_marker_style_fields(): - """A category not used by marker_styles.icon_field/color_field shouldn't ride - along on basic_info() just because it's a category - only marker_style_fields +def test_pin_marker_fields_omits_categories_not_referenced_by_marker_style_fields(): + """A category not used by marker_styles.icon_field/color_field shouldn't land + in pin_marker_fields just because it's a category - only marker_style_fields controls what pin styling needs, not the full category set (a deployment can have categories unrelated to marker display, e.g. used only for filtering).""" location_model = create_location_model( @@ -172,16 +168,9 @@ def test_basic_info_omits_category_fields_not_referenced_by_marker_style_fields( }, marker_style_fields={"type_of_place"}, ) - location = location_model( - uuid="1", type_of_place="parcel_locker", accessibility="wheelchair", position=(50, 50) + assert cast(Type[LocationBase], location_model).pin_marker_fields == frozenset( + {"type_of_place"} ) - location = cast(LocationBase, location) - assert location.basic_info() == { - "uuid": "1", - "position": (50, 50), - "has_remark": False, - "type_of_place": "parcel_locker", - } def test_create_location_model_with_int_field(): diff --git a/tests/unit_tests/test_core_api.py b/tests/unit_tests/test_core_api.py index 39a54b92..d0659c83 100644 --- a/tests/unit_tests/test_core_api.py +++ b/tests/unit_tests/test_core_api.py @@ -315,10 +315,11 @@ def test_get_locations_accepts_valid_and_undeclared_parameters(test_app, query): assert response.status_code == 200 -def test_get_locations_includes_category_field_for_pin_styling(): - """/api/locations should surface the field marker_styles.icon_field points at - (e.g. a point-type category), so the frontend can pick a marker icon/color - without a full per-location detail fetch.""" +def test_get_locations_omits_marker_style_field_values(): + """/api/locations should not surface the field marker_styles.icon_field + points at (e.g. a point-type category) - that value is fetched lazily via + /api/locations/marker-styles, only once a marker is individually visible, + instead of upfront for every location (see lazy-load-marker-styling-plan.md).""" client = create_test_app( db_overrides={ "categories": {"point_type": ["parcel_locker", "container"]}, @@ -344,11 +345,122 @@ def test_get_locations_includes_category_field_for_pin_styling(): "uuid": "11111111-1111-1111-1111-111111111111", "position": [50, 50], "has_remark": False, - "point_type": "parcel_locker", }, ] +def test_get_locations_marker_styles_returns_requested_uuids_styling(): + """The lazy marker-styles endpoint returns just the marker_styles-relevant + field values for the requested uuids, not the full location.""" + client = create_test_app( + db_overrides={ + "categories": {"point_type": ["parcel_locker", "container"]}, + "location_obligatory_fields": [("point_type", "str"), ("name", "str")], + "marker_styles": {"icon_field": "point_type", "icons": {}, "colors": {}}, + "data": [ + { + "name": "locker-1", + "position": [50, 50], + "point_type": "parcel_locker", + "uuid": "11111111-1111-1111-1111-111111111111", + }, + { + "name": "locker-2", + "position": [51, 51], + "point_type": "container", + "uuid": "22222222-2222-2222-2222-222222222222", + }, + ], + "visible_data": ["name", "point_type"], + } + ) + + response = client.get("/api/locations/marker-styles?uuid=11111111-1111-1111-1111-111111111111") + + assert response.status_code == 200 + assert response.json == { + "11111111-1111-1111-1111-111111111111": {"point_type": "parcel_locker"}, + } + + +def test_get_locations_marker_styles_supports_multiple_uuids(): + client = create_test_app( + db_overrides={ + "categories": {"point_type": ["parcel_locker", "container"]}, + "location_obligatory_fields": [("point_type", "str"), ("name", "str")], + "marker_styles": {"icon_field": "point_type", "icons": {}, "colors": {}}, + "data": [ + { + "name": "locker-1", + "position": [50, 50], + "point_type": "parcel_locker", + "uuid": "11111111-1111-1111-1111-111111111111", + }, + { + "name": "locker-2", + "position": [51, 51], + "point_type": "container", + "uuid": "22222222-2222-2222-2222-222222222222", + }, + ], + "visible_data": ["name", "point_type"], + } + ) + + response = client.get( + "/api/locations/marker-styles" + "?uuid=11111111-1111-1111-1111-111111111111" + "&uuid=22222222-2222-2222-2222-222222222222" + ) + + assert response.status_code == 200 + assert response.json == { + "11111111-1111-1111-1111-111111111111": {"point_type": "parcel_locker"}, + "22222222-2222-2222-2222-222222222222": {"point_type": "container"}, + } + + +def test_get_locations_marker_styles_omits_unknown_uuids(): + """An unknown/re-clustered-away uuid doesn't error the whole request - it's + just absent from the response.""" + client = create_test_app( + db_overrides={ + "categories": {"point_type": ["parcel_locker"]}, + "location_obligatory_fields": [("point_type", "str"), ("name", "str")], + "marker_styles": {"icon_field": "point_type", "icons": {}, "colors": {}}, + "data": [ + { + "name": "locker-1", + "position": [50, 50], + "point_type": "parcel_locker", + "uuid": "11111111-1111-1111-1111-111111111111", + }, + ], + "visible_data": ["name", "point_type"], + } + ) + + response = client.get( + "/api/locations/marker-styles" + "?uuid=11111111-1111-1111-1111-111111111111" + "&uuid=99999999-9999-9999-9999-999999999999" + ) + + assert response.status_code == 200 + assert response.json == { + "11111111-1111-1111-1111-111111111111": {"point_type": "parcel_locker"}, + } + + +def test_get_locations_marker_styles_empty_query_returns_empty_object(): + client = create_test_app(db_overrides={"categories": {}}) + + response = client.get("/api/locations/marker-styles") + + assert response.status_code == 200 + assert response.json == {} + + def test_get_locations_multi_value_same_category_uses_or_semantics(): """Selecting several checkboxes within one category should return the union of matches, not only entries that have every selected value.""" From 34a1f8420ba0bc554dd104d06d22da53f9cdbbb6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Krzysztof=20Ko=C5=82odzi=C5=84ski?= Date: Thu, 20 Aug 2026 22:44:26 +0200 Subject: [PATCH 22/28] fixes --- goodmap/api/core_api.py | 3 ++- goodmap/data_models/location.py | 16 ++---------- goodmap/goodmap.py | 21 +++++++++++---- tests/unit_tests/data_models/test_location.py | 26 +++---------------- 4 files changed, 24 insertions(+), 42 deletions(-) diff --git a/goodmap/api/core_api.py b/goodmap/api/core_api.py index 5423ed18..4ff501b9 100644 --- a/goodmap/api/core_api.py +++ b/goodmap/api/core_api.py @@ -198,6 +198,7 @@ def core_pages( photo_attachment_config: AttachmentConfig, feature_flags: FeatureFlagSet, shortcodes: dict[str, Shortcode], + pin_marker_fields: frozenset[str] = frozenset(), ) -> Blueprint: core_api_blueprint = Blueprint("api", __name__, url_prefix="/api") @@ -442,7 +443,7 @@ def get_locations_marker_styles(): location = database.get_location(location_uuid) if location is None: continue - styling = marker_style_values(location, location_model.pin_marker_fields) + styling = marker_style_values(location, pin_marker_fields) if styling: result[location_uuid] = styling return jsonify(result) diff --git a/goodmap/data_models/location.py b/goodmap/data_models/location.py index 6646bd8e..6fa5f934 100644 --- a/goodmap/data_models/location.py +++ b/goodmap/data_models/location.py @@ -6,8 +6,7 @@ """ import warnings -from collections.abc import Iterable -from typing import Annotated, Any, ClassVar, Type, cast +from typing import Annotated, Any, Type, cast from annotated_types import Ge, Le from pydantic import ( @@ -38,11 +37,6 @@ class LocationBase(BaseModel, extra="allow"): uuid: str = Field(..., max_length=100) # TODO make this UUID and deprecate string remark: str | None = None - # Names of category fields whose values should ride along on basic_info(), - # e.g. so the frontend can pick a pin icon/color without a full detail fetch. - # Populated by create_location_model(); empty for the base class. - pin_marker_fields: ClassVar[frozenset[str]] = frozenset() - @model_validator(mode="before") @classmethod def validate_uuid_exists(cls, data: Any) -> Any: @@ -233,7 +227,6 @@ def _build_field_definition(field_type_str: str, allowed_values: frozenset[str]) def create_location_model( obligatory_fields: list[tuple[str, str]] | list[tuple[str, Type[Any]]], categories: dict[str, list[str]], - marker_style_fields: Iterable[str] = (), ) -> Type[BaseModel]: """Dynamically create a Location model with additional required fields. @@ -245,9 +238,6 @@ def create_location_model( - String type name: "str", "list", "int", "float", "bool", "dict" - Python type object: str, list, int, etc. (deprecated) categories: Dict mapping field names to allowed values (enums). - marker_style_fields: Field names referenced by the deployment's marker_styles - config (icon_field/color_field) - the only ones whose values - need to ride along on basic_info() for pin styling. Returns: A Location model class extending LocationBase with additional fields @@ -272,11 +262,9 @@ def create_location_model( allowed = frozenset() fields[field_name] = _build_field_definition(field_type_str, allowed) - location_model = create_model( + return create_model( "Location", __base__=LocationBase, __module__="goodmap.data_models.location", **fields, ) - location_model.pin_marker_fields = frozenset(marker_style_fields) & fields.keys() - return location_model diff --git a/goodmap/goodmap.py b/goodmap/goodmap.py index ecb80ebb..ff7fe89e 100644 --- a/goodmap/goodmap.py +++ b/goodmap/goodmap.py @@ -112,14 +112,18 @@ def _add_cors(response): def _setup_location_model( db: Any, -) -> tuple[list[Any], dict[str, Any], type[BaseModel], Any]: +) -> tuple[list[Any], dict[str, Any], type[BaseModel], Any, frozenset[str]]: """Configure location model and db with lazy-loading and categories support. Args: db: The database instance to extend with location queries. Returns: - Tuple of (obligatory_fields, categories, location_model, db). + Tuple of (obligatory_fields, categories, location_model, db, pin_marker_fields). + pin_marker_fields is app-wiring knowledge - which of this deployment's fields + the marker_styles config (icon_field/color_field) actually points at - not + something the location model itself needs to know; it's threaded to core_pages() + for goodmap.api.api_models.marker_style_values() to use. """ obligatory_fields = get_location_obligatory_fields(db) location_model = create_location_model(obligatory_fields, {}) @@ -142,10 +146,13 @@ def _setup_location_model( } if categories or marker_style_fields: - location_model = create_location_model(obligatory_fields, categories, marker_style_fields) + location_model = create_location_model(obligatory_fields, categories) extended_db = extend_db_with_goodmap_queries(extended_db, location_model) - return obligatory_fields, categories, location_model, extended_db + field_names = {name for name, _ in obligatory_fields} + pin_marker_fields = frozenset(marker_style_fields) & field_names + + return obligatory_fields, categories, location_model, extended_db, pin_marker_fields def create_app(config_path: str) -> platzky.Engine: @@ -206,11 +213,14 @@ def create_app_from_config(config: GoodmapConfig) -> platzky.Engine: app.config["MAX_CONTENT_LENGTH"] = config.attachment.max_size + MULTIPART_OVERHEAD_ALLOWANCE if app.is_enabled(UseLazyLoading): - location_obligatory_fields, _, location_model, app.db = _setup_location_model(app.db) + location_obligatory_fields, _, location_model, app.db, pin_marker_fields = ( + _setup_location_model(app.db) + ) else: location_obligatory_fields = [] location_model = create_location_model([], {}) app.db = extend_db_with_goodmap_queries(app.db, location_model) + pin_marker_fields = frozenset() app.extensions["goodmap"] = {"location_obligatory_fields": location_obligatory_fields} @@ -278,6 +288,7 @@ def handle_csrf_error(error): photo_attachment_config=photo_attachment_config, feature_flags=config.feature_flags, shortcodes=shortcodes, + pin_marker_fields=pin_marker_fields, ) app.register_blueprint(cp) diff --git a/tests/unit_tests/data_models/test_location.py b/tests/unit_tests/data_models/test_location.py index 9af9a3e2..60ce982a 100644 --- a/tests/unit_tests/data_models/test_location.py +++ b/tests/unit_tests/data_models/test_location.py @@ -1,5 +1,5 @@ import warnings -from typing import Type, cast +from typing import cast import pytest @@ -130,14 +130,14 @@ def test_category_validation_rejects_invalid_list_item(): location_model(uuid="2", tags=["red", "yellow"], position=(50, 50)) -def test_basic_info_omits_marker_style_field_values(): - """basic_info() carries identity/position only - marker styling values are +def test_basic_info_omits_category_field_values(): + """basic_info() carries identity/position only, even for a category field a + deployment's marker_styles config might reference - marker styling values are fetched separately (see goodmap.api.api_models.marker_style_values and lazy-load-marker-styling-plan.md), only once a marker is actually visible.""" location_model = create_location_model( obligatory_fields=[("type_of_place", "str"), ("name", "str")], categories={"type_of_place": ["parcel_locker", "container"]}, - marker_style_fields={"type_of_place"}, ) location = location_model( uuid="1", name="test", type_of_place="parcel_locker", position=(50, 50) @@ -155,24 +155,6 @@ def test_basic_info_omits_category_fields_when_none_configured(): assert location.basic_info() == {"uuid": "1", "position": (50, 50), "has_remark": False} -def test_pin_marker_fields_omits_categories_not_referenced_by_marker_style_fields(): - """A category not used by marker_styles.icon_field/color_field shouldn't land - in pin_marker_fields just because it's a category - only marker_style_fields - controls what pin styling needs, not the full category set (a deployment can - have categories unrelated to marker display, e.g. used only for filtering).""" - location_model = create_location_model( - obligatory_fields=[("type_of_place", "str"), ("accessibility", "str")], - categories={ - "type_of_place": ["parcel_locker", "container"], - "accessibility": ["wheelchair", "none"], - }, - marker_style_fields={"type_of_place"}, - ) - assert cast(Type[LocationBase], location_model).pin_marker_fields == frozenset( - {"type_of_place"} - ) - - def test_create_location_model_with_int_field(): """Test that non-str simple fields (like int) are created without max_length.""" location_model = create_location_model(obligatory_fields=[("capacity", "int")], categories={}) From 41cfcac8658f76895d20d1f4a928b5e4996bbd7b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Krzysztof=20Ko=C5=82odzi=C5=84ski?= Date: Thu, 20 Aug 2026 23:11:06 +0200 Subject: [PATCH 23/28] fix --- .../MarkerPopup/getTypedMarkerIcon.jsx | 5 +- .../tests/MarkerPopup/MarkerPopup.test.jsx | 46 +++++++++++++---- goodmap/api/api_models.py | 32 ++++++------ goodmap/api/core_api.py | 17 +++---- goodmap/data_models/location.py | 9 ++-- tests/unit_tests/data_models/test_location.py | 26 ++++------ tests/unit_tests/test_core_api.py | 50 ++++++++++++++++--- 7 files changed, 123 insertions(+), 62 deletions(-) diff --git a/frontend/src/components/MarkerPopup/getTypedMarkerIcon.jsx b/frontend/src/components/MarkerPopup/getTypedMarkerIcon.jsx index 96b04b3a..cdbc1dc1 100644 --- a/frontend/src/components/MarkerPopup/getTypedMarkerIcon.jsx +++ b/frontend/src/components/MarkerPopup/getTypedMarkerIcon.jsx @@ -89,7 +89,10 @@ PinIcon.propTypes = { * matched, or `null` (falls back to Leaflet's default marker) when there's * neither a match nor a remark to show. * - * @param {Object} place - Location data, as returned by GET /api/locations + * @param {Object} place - Location data from GET /api/locations, merged with any + * styling lazily fetched for it from GET /api/locations/marker-styles (has_remark + * and marker_styles field values aren't in the initial /api/locations response - + * see lazy-load-marker-styling-plan.md) * @returns {import('leaflet').DivIcon|null} */ const getTypedMarkerIcon = place => { diff --git a/frontend/tests/MarkerPopup/MarkerPopup.test.jsx b/frontend/tests/MarkerPopup/MarkerPopup.test.jsx index 27807b94..35491106 100644 --- a/frontend/tests/MarkerPopup/MarkerPopup.test.jsx +++ b/frontend/tests/MarkerPopup/MarkerPopup.test.jsx @@ -36,6 +36,23 @@ const locationData = { }; httpService.getLocation.mockResolvedValue(locationData); +// Every mount now fires a lazy marker-styles request (see requestMarkerStyle.js) - +// a harmless default so it always resolves, even in tests that don't care about it. +httpService.getMarkerStyles.mockResolvedValue({}); + +/** + * requestMarkerStyle.js debounces/batches uuids through module-level state shared + * by every test in this file. Describes below that render with real timers must + * drain that debounce window before finishing, or its still-pending timer fires + * during a later (fake-timer) describe and merges its uuid into that batch. + */ +const flushMarkerStyleDebounce = () => + act( + () => + new Promise(resolve => { + setTimeout(resolve, 200); + }), + ); describe('MarkerPopup', () => { beforeEach(() => { @@ -55,8 +72,9 @@ describe('MarkerPopup', () => { ); }); - afterEach(() => { + afterEach(async () => { globalThis.fetch.mockRestore(); + await flushMarkerStyleDebounce(); }); it('should render marker without popup', () => { @@ -102,8 +120,9 @@ describe('MarkerPopup with remark', () => { }); }); - afterEach(() => { + afterEach(async () => { globalThis.fetch.mockRestore(); + await flushMarkerStyleDebounce(); }); it('should render our own pin with an asterisk badge when remark is true', () => { @@ -156,6 +175,15 @@ describe('MarkerPopup with remark', () => { }); describe('MarkerPopup lazy marker styling', () => { + // A uuid distinct from `location`'s (used by the describes above, which run with + // real timers) so a leftover real setTimeout from those can't resolve into this + // describe's store state mid-test and make "already known" skip our own request. + const lazyLocation = { + position: [51.2, 17.1], + uuid: 'lazy-marker-styling-uuid', + has_remark: false, // eslint-disable-line camelcase -- matches backend API schema property name + }; + beforeEach(() => { jest.useFakeTimers(); useMarkerStylesStore.setState({ stylesByUuid: {} }); @@ -164,7 +192,7 @@ describe('MarkerPopup lazy marker styling', () => { icons: { parcelLocker: 'https://cdn.example.com/parcel-locker.svg' }, }; httpService.getMarkerStyles.mockResolvedValue({ - [location.uuid]: { pointType: 'parcelLocker' }, + [lazyLocation.uuid]: { pointType: 'parcelLocker' }, }); }); @@ -177,34 +205,34 @@ describe('MarkerPopup lazy marker styling', () => { await act(async () => { render( - + , ); }); - expect(httpService.getMarkerStyles).not.toHaveBeenCalled(); + expect(httpService.getMarkerStyles).not.toHaveBeenCalledWith([lazyLocation.uuid]); await act(async () => { jest.advanceTimersByTime(200); await Promise.resolve(); }); - expect(httpService.getMarkerStyles).toHaveBeenCalledWith([location.uuid]); + expect(httpService.getMarkerStyles).toHaveBeenCalledWith([lazyLocation.uuid]); }); it('re-renders the marker with the lazily-fetched icon once it arrives', async () => { await act(async () => { render( - + , ); }); diff --git a/goodmap/api/api_models.py b/goodmap/api/api_models.py index cbcb0c72..eb5806ca 100644 --- a/goodmap/api/api_models.py +++ b/goodmap/api/api_models.py @@ -73,9 +73,6 @@ class LocationBasicInfo(BaseModel): uuid: str = Field(..., description="Location UUID") position: tuple[Latitude, Longitude] = Field(..., description=_POSITION_DESCRIPTION) - has_remark: bool = Field( - ..., description="Whether the point has a remark, not the remark itself" - ) class LocationList(RootModel[list[LocationBasicInfo]]): @@ -83,10 +80,11 @@ class LocationList(RootModel[list[LocationBasicInfo]]): class LocationMarkerStyles(RootModel[dict[str, dict[str, Any]]]): - """Map of uuid -> pin styling field values (whatever marker_styles.icon_field/ - color_field point at), for lazily fetching styling once a marker becomes - individually visible instead of getting it upfront for every location. - Unknown/missing uuids are simply absent from the response, not an error.""" + """Map of uuid -> pin styling data: has_remark (drives the asterisk badge) plus + whatever marker_styles.icon_field/color_field point at (drive icon/color) - for + lazily fetching it once a marker becomes individually visible instead of getting + it upfront for every location. Unknown/missing uuids are simply absent from the + response, not an error.""" class MarkerStylesQueryParams(BaseModel): @@ -96,17 +94,19 @@ class MarkerStylesQueryParams(BaseModel): def marker_style_values(location: BaseModel, style_fields: frozenset[str]) -> dict[str, Any]: - """Pin styling field values for `location`, as /api/locations/marker-styles returns them. + """Pin styling data for `location`, as /api/locations/marker-styles returns it. - This is API response shaping, not something the location domain model needs to - know how to do itself - it belongs alongside the models it fills, not on - LocationBase. + Always includes has_remark (drives the asterisk badge), plus the value of any + of `style_fields` this location actually has (drive icon/color). This is API + response shaping, not something the location domain model needs to know how to + do itself - it belongs alongside the models it fills, not on LocationBase. """ - return { - field: value - for field in sorted(style_fields) - if (value := getattr(location, field, None)) is not None - } + data: dict[str, Any] = {"has_remark": bool(getattr(location, "remark", None))} + for field in sorted(style_fields): + value = getattr(location, field, None) + if value is not None: + data[field] = value + return data class ClusterInfo(BaseModel): diff --git a/goodmap/api/core_api.py b/goodmap/api/core_api.py index 4ff501b9..ac4285c1 100644 --- a/goodmap/api/core_api.py +++ b/goodmap/api/core_api.py @@ -345,8 +345,9 @@ def report_location(): def get_locations(): """Get list of locations with basic info. - Returns locations filtered by query parameters, - showing only uuid, position, and whether each has a remark. + Returns locations filtered by query parameters, showing only uuid and + position. Pin styling (has_remark, marker_styles field values) is fetched + separately, per-uuid, via /api/locations/marker-styles. """ locations = get_locations_from_request(database, request.args) return jsonify(locations) @@ -429,11 +430,11 @@ def get_location(location_id): resp=Response(HTTP_200=LocationMarkerStyles), ) def get_locations_marker_styles(): - """Get pin styling field values for specific locations, by uuid. + """Get pin styling data for specific locations, by uuid. - For lazily fetching marker_styles-relevant field values only once a - client-side-clustered marker becomes individually visible, instead of - the frontend getting them upfront for every location (see + For lazily fetching has_remark and marker_styles-relevant field values + only once a client-side-clustered marker becomes individually visible, + instead of the frontend getting them upfront for every location (see lazy-load-marker-styling-plan.md). Unknown or missing uuids are silently omitted from the response rather than erroring the whole request - a marker that's re-clustered mid-flight isn't a client bug. @@ -443,9 +444,7 @@ def get_locations_marker_styles(): location = database.get_location(location_uuid) if location is None: continue - styling = marker_style_values(location, pin_marker_fields) - if styling: - result[location_uuid] = styling + result[location_uuid] = marker_style_values(location, pin_marker_fields) return jsonify(result) @core_api_blueprint.route("/version", methods=["GET"]) diff --git a/goodmap/data_models/location.py b/goodmap/data_models/location.py index 6fa5f934..98d5b412 100644 --- a/goodmap/data_models/location.py +++ b/goodmap/data_models/location.py @@ -87,15 +87,14 @@ def model_dump(self, **kwargs) -> dict[str, Any]: def basic_info(self) -> dict[str, Any]: """Get basic location information summary: identity and position only. - Includes the uuid/position/remark flag always shown on the map. Marker - styling field values are deliberately not here - see + Everything about how this point's marker should look - whether it has a + remark (drives the asterisk badge), any marker_styles field values (drive + icon/color) - is deliberately not here; see ``goodmap.api.api_models.marker_style_values``, read separately and only once a point is individually visible (not folded into a cluster), so points that aren't don't pay for it. """ - data = self.model_dump(include={"uuid", "position"}) - data["has_remark"] = bool(self.remark) - return data + return self.model_dump(include={"uuid", "position"}) _TYPE_MAPPING: dict[str, type] = { diff --git a/tests/unit_tests/data_models/test_location.py b/tests/unit_tests/data_models/test_location.py index 60ce982a..adbbf8db 100644 --- a/tests/unit_tests/data_models/test_location.py +++ b/tests/unit_tests/data_models/test_location.py @@ -130,29 +130,25 @@ def test_category_validation_rejects_invalid_list_item(): location_model(uuid="2", tags=["red", "yellow"], position=(50, 50)) -def test_basic_info_omits_category_field_values(): - """basic_info() carries identity/position only, even for a category field a - deployment's marker_styles config might reference - marker styling values are - fetched separately (see goodmap.api.api_models.marker_style_values and +def test_basic_info_is_identity_and_position_only(): + """basic_info() carries uuid/position only, even for a category field a + deployment's marker_styles config might reference and even when the location + has a remark - both has_remark and marker styling values are fetched + separately (see goodmap.api.api_models.marker_style_values and lazy-load-marker-styling-plan.md), only once a marker is actually visible.""" location_model = create_location_model( obligatory_fields=[("type_of_place", "str"), ("name", "str")], categories={"type_of_place": ["parcel_locker", "container"]}, ) location = location_model( - uuid="1", name="test", type_of_place="parcel_locker", position=(50, 50) + uuid="1", + name="test", + type_of_place="parcel_locker", + position=(50, 50), + remark="a remark", ) location = cast(LocationBase, location) - assert location.basic_info() == {"uuid": "1", "position": (50, 50), "has_remark": False} - - -def test_basic_info_omits_category_fields_when_none_configured(): - """Backward compatibility: deployments without categories get the original - uuid/position/has_remark shape, unchanged.""" - location_model = create_location_model(obligatory_fields=[("name", "str")], categories={}) - location = location_model(uuid="1", name="test", position=(50, 50)) - location = cast(LocationBase, location) - assert location.basic_info() == {"uuid": "1", "position": (50, 50), "has_remark": False} + assert location.basic_info() == {"uuid": "1", "position": (50, 50)} def test_create_location_model_with_int_field(): diff --git a/tests/unit_tests/test_core_api.py b/tests/unit_tests/test_core_api.py index d0659c83..4185348d 100644 --- a/tests/unit_tests/test_core_api.py +++ b/tests/unit_tests/test_core_api.py @@ -272,12 +272,10 @@ def test_get_locations(test_app): { "uuid": "11111111-1111-1111-1111-111111111111", "position": [50, 50], - "has_remark": True, }, { "uuid": "22222222-2222-2222-2222-222222222222", "position": [60, 60], - "has_remark": False, }, ] @@ -344,7 +342,6 @@ def test_get_locations_omits_marker_style_field_values(): { "uuid": "11111111-1111-1111-1111-111111111111", "position": [50, 50], - "has_remark": False, }, ] @@ -379,7 +376,10 @@ def test_get_locations_marker_styles_returns_requested_uuids_styling(): assert response.status_code == 200 assert response.json == { - "11111111-1111-1111-1111-111111111111": {"point_type": "parcel_locker"}, + "11111111-1111-1111-1111-111111111111": { + "has_remark": False, + "point_type": "parcel_locker", + }, } @@ -415,8 +415,14 @@ def test_get_locations_marker_styles_supports_multiple_uuids(): assert response.status_code == 200 assert response.json == { - "11111111-1111-1111-1111-111111111111": {"point_type": "parcel_locker"}, - "22222222-2222-2222-2222-222222222222": {"point_type": "container"}, + "11111111-1111-1111-1111-111111111111": { + "has_remark": False, + "point_type": "parcel_locker", + }, + "22222222-2222-2222-2222-222222222222": { + "has_remark": False, + "point_type": "container", + }, } @@ -448,7 +454,37 @@ def test_get_locations_marker_styles_omits_unknown_uuids(): assert response.status_code == 200 assert response.json == { - "11111111-1111-1111-1111-111111111111": {"point_type": "parcel_locker"}, + "11111111-1111-1111-1111-111111111111": { + "has_remark": False, + "point_type": "parcel_locker", + }, + } + + +def test_get_locations_marker_styles_includes_has_remark_without_marker_styles_config(): + """has_remark drives the asterisk badge independently of marker_styles - + deployments with no icon_field/color_field configured still need it fetched + lazily, the same as everyone else.""" + client = create_test_app( + db_overrides={ + "categories": {}, + "location_obligatory_fields": [("name", "str")], + "data": [ + { + "name": "test", + "position": [50, 50], + "uuid": "11111111-1111-1111-1111-111111111111", + "remark": "this is a remark", + }, + ], + } + ) + + response = client.get("/api/locations/marker-styles?uuid=11111111-1111-1111-1111-111111111111") + + assert response.status_code == 200 + assert response.json == { + "11111111-1111-1111-1111-111111111111": {"has_remark": True}, } From 309a79df9cd00705068d7d469bd4f47f410df25d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Krzysztof=20Ko=C5=82odzi=C5=84ski?= Date: Thu, 20 Aug 2026 23:14:11 +0200 Subject: [PATCH 24/28] added missing files --- .../Map/store/markerStyles.store.js | 14 ++++ .../MarkerPopup/requestMarkerStyle.js | 54 +++++++++++++++ .../MarkerPopup/requestMarkerStyle.test.js | 66 +++++++++++++++++++ tests/unit_tests/test_api_models.py | 43 ++++++++++++ 4 files changed, 177 insertions(+) create mode 100644 frontend/src/components/Map/store/markerStyles.store.js create mode 100644 frontend/src/components/MarkerPopup/requestMarkerStyle.js create mode 100644 frontend/tests/MarkerPopup/requestMarkerStyle.test.js create mode 100644 tests/unit_tests/test_api_models.py diff --git a/frontend/src/components/Map/store/markerStyles.store.js b/frontend/src/components/Map/store/markerStyles.store.js new file mode 100644 index 00000000..8608056a --- /dev/null +++ b/frontend/src/components/Map/store/markerStyles.store.js @@ -0,0 +1,14 @@ +import { create } from 'zustand'; + +/** + * uuid -> resolved marker-styling field values (whatever marker_styles.icon_field/ + * color_field point at), lazily fetched once a client-side-clustered marker becomes + * individually visible - see lazy-load-marker-styling-plan.md. A uuid with no + * matching styling is still recorded, as {}, so it isn't re-requested forever. + */ +const useMarkerStylesStore = create(set => ({ + stylesByUuid: {}, + mergeStyles: styles => set(state => ({ stylesByUuid: { ...state.stylesByUuid, ...styles } })), +})); + +export default useMarkerStylesStore; diff --git a/frontend/src/components/MarkerPopup/requestMarkerStyle.js b/frontend/src/components/MarkerPopup/requestMarkerStyle.js new file mode 100644 index 00000000..59d6029d --- /dev/null +++ b/frontend/src/components/MarkerPopup/requestMarkerStyle.js @@ -0,0 +1,54 @@ +import httpService from '../../services/http/httpService'; +import useMarkerStylesStore from '../Map/store/markerStyles.store'; + +const BATCH_DEBOUNCE_MS = 150; + +let pendingUuids = new Set(); +let timer = null; + +/** + * Queues `uuid` for a batched GET /api/locations/marker-styles fetch, once its + * marker becomes individually visible (not folded into a cluster) - see + * lazy-load-marker-styling-plan.md. Fetches pin styling data (has_remark plus any + * marker_styles field values), so it's needed regardless of whether marker_styles + * is even configured - has_remark alone still drives the asterisk badge. Debounced + * so that markers becoming visible in quick succession (panning, zooming, a + * cluster spiderfying) share one request instead of firing one per marker. + * + * Scoped to client-side clustering for now - server-side clustering's own + * lazy-loading trigger is a separate follow-up (see the plan doc). + * + * @param {string} uuid - Location UUID whose marker just became individually visible + */ +const requestMarkerStyle = uuid => { + if (globalThis.FEATURE_FLAGS?.USE_SERVER_SIDE_CLUSTERING) { + return; + } + + const alreadyKnown = uuid in useMarkerStylesStore.getState().stylesByUuid; + if (alreadyKnown || pendingUuids.has(uuid)) { + return; + } + pendingUuids.add(uuid); + + if (timer) { + clearTimeout(timer); + } + timer = setTimeout(() => { + const uuids = [...pendingUuids]; + pendingUuids = new Set(); + timer = null; + + httpService + .getMarkerStyles(uuids) + .then(styles => { + // Every requested uuid is recorded, even with no matching styling + // ({}), so it isn't queued again on the next re-cluster. + const withDefaults = Object.fromEntries(uuids.map(u => [u, styles[u] ?? {}])); + useMarkerStylesStore.getState().mergeStyles(withDefaults); + }) + .catch(error => console.error('Failed to fetch marker styles:', error)); + }, BATCH_DEBOUNCE_MS); +}; + +export default requestMarkerStyle; diff --git a/frontend/tests/MarkerPopup/requestMarkerStyle.test.js b/frontend/tests/MarkerPopup/requestMarkerStyle.test.js new file mode 100644 index 00000000..7f2abebf --- /dev/null +++ b/frontend/tests/MarkerPopup/requestMarkerStyle.test.js @@ -0,0 +1,66 @@ +import requestMarkerStyle from '../../src/components/MarkerPopup/requestMarkerStyle'; +import httpService from '../../src/services/http/httpService'; +import useMarkerStylesStore from '../../src/components/Map/store/markerStyles.store'; + +jest.mock('../../src/services/http/httpService'); + +describe('requestMarkerStyle', () => { + beforeEach(() => { + jest.useFakeTimers(); + jest.clearAllMocks(); + useMarkerStylesStore.setState({ stylesByUuid: {} }); + globalThis.MARKER_STYLES = { icon_field: 'pointType' }; // eslint-disable-line camelcase -- matches backend API schema property name + delete globalThis.FEATURE_FLAGS; + }); + + afterEach(() => { + jest.runOnlyPendingTimers(); + jest.useRealTimers(); + delete globalThis.MARKER_STYLES; + delete globalThis.FEATURE_FLAGS; + }); + + it('still fetches when marker styling is not configured, for has_remark', () => { + globalThis.MARKER_STYLES = {}; + httpService.getMarkerStyles.mockResolvedValue({ 'uuid-1': { has_remark: true } }); // eslint-disable-line camelcase -- matches backend API schema property name + requestMarkerStyle('uuid-1'); + jest.runAllTimers(); + expect(httpService.getMarkerStyles).toHaveBeenCalledWith(['uuid-1']); + }); + + it('does nothing when server-side clustering is enabled', () => { + globalThis.FEATURE_FLAGS = { USE_SERVER_SIDE_CLUSTERING: true }; + requestMarkerStyle('uuid-1'); + jest.runAllTimers(); + expect(httpService.getMarkerStyles).not.toHaveBeenCalled(); + }); + + it('batches uuids requested within the debounce window into one request', async () => { + httpService.getMarkerStyles.mockResolvedValue({ 'uuid-1': { pointType: 'a' } }); + requestMarkerStyle('uuid-1'); + requestMarkerStyle('uuid-2'); + jest.runAllTimers(); + await Promise.resolve(); + expect(httpService.getMarkerStyles).toHaveBeenCalledTimes(1); + expect(httpService.getMarkerStyles).toHaveBeenCalledWith(['uuid-1', 'uuid-2']); + }); + + it('merges results into the store, defaulting unmatched uuids to {}', async () => { + httpService.getMarkerStyles.mockResolvedValue({ 'uuid-1': { pointType: 'a' } }); + requestMarkerStyle('uuid-1'); + requestMarkerStyle('uuid-2'); + jest.runAllTimers(); + await Promise.resolve(); + expect(useMarkerStylesStore.getState().stylesByUuid).toEqual({ + 'uuid-1': { pointType: 'a' }, + 'uuid-2': {}, + }); + }); + + it('does not re-request a uuid already known, even with no matching styling', () => { + useMarkerStylesStore.setState({ stylesByUuid: { 'uuid-1': {} } }); + requestMarkerStyle('uuid-1'); + jest.runAllTimers(); + expect(httpService.getMarkerStyles).not.toHaveBeenCalled(); + }); +}); diff --git a/tests/unit_tests/test_api_models.py b/tests/unit_tests/test_api_models.py new file mode 100644 index 00000000..2f5af344 --- /dev/null +++ b/tests/unit_tests/test_api_models.py @@ -0,0 +1,43 @@ +from typing import cast + +from goodmap.api.api_models import marker_style_values +from goodmap.data_models.location import LocationBase, create_location_model + + +def test_marker_style_values_includes_has_remark_and_configured_field_values(): + """marker_style_values() always includes has_remark (drives the asterisk + badge), plus the requested style_fields' values (drive icon/color) off the + given location.""" + location_model = create_location_model( + obligatory_fields=[("type_of_place", "str"), ("name", "str")], + categories={"type_of_place": ["parcel_locker", "container"]}, + ) + location = location_model( + uuid="1", + name="test", + type_of_place="parcel_locker", + position=(50, 50), + remark="a remark", + ) + location = cast(LocationBase, location) + assert marker_style_values(location, frozenset({"type_of_place"})) == { + "has_remark": True, + "type_of_place": "parcel_locker", + } + + +def test_marker_style_values_has_remark_false_and_empty_when_no_style_fields(): + location_model = create_location_model(obligatory_fields=[("name", "str")], categories={}) + location = location_model(uuid="1", name="test", position=(50, 50)) + location = cast(LocationBase, location) + assert marker_style_values(location, frozenset()) == {"has_remark": False} + + +def test_marker_style_values_ignores_style_fields_the_location_does_not_have(): + """A style field that isn't actually one of this location's attributes (e.g. + misconfigured marker_styles, or narrowed away upstream) is simply skipped, + not an error.""" + location_model = create_location_model(obligatory_fields=[("name", "str")], categories={}) + location = location_model(uuid="1", name="test", position=(50, 50)) + location = cast(LocationBase, location) + assert marker_style_values(location, frozenset({"nonexistent_field"})) == {"has_remark": False} From 0063c97a75256db3979fe3932d46061947d26431 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Krzysztof=20Ko=C5=82odzi=C5=84ski?= Date: Thu, 20 Aug 2026 23:17:24 +0200 Subject: [PATCH 25/28] fix --- goodmap/data_models/location.py | 7 ------- 1 file changed, 7 deletions(-) diff --git a/goodmap/data_models/location.py b/goodmap/data_models/location.py index 98d5b412..8993eca3 100644 --- a/goodmap/data_models/location.py +++ b/goodmap/data_models/location.py @@ -86,13 +86,6 @@ def model_dump(self, **kwargs) -> dict[str, Any]: def basic_info(self) -> dict[str, Any]: """Get basic location information summary: identity and position only. - - Everything about how this point's marker should look - whether it has a - remark (drives the asterisk badge), any marker_styles field values (drive - icon/color) - is deliberately not here; see - ``goodmap.api.api_models.marker_style_values``, read separately and only - once a point is individually visible (not folded into a cluster), so - points that aren't don't pay for it. """ return self.model_dump(include={"uuid", "position"}) From fe0fb270789844448a930e4e428e405d3c7a1845 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Krzysztof=20Ko=C5=82odzi=C5=84ski?= Date: Thu, 20 Aug 2026 23:23:13 +0200 Subject: [PATCH 26/28] refactor --- .../tests/MarkerPopup/MarkerPopup.test.jsx | 38 ++++++++----------- 1 file changed, 16 insertions(+), 22 deletions(-) diff --git a/frontend/tests/MarkerPopup/MarkerPopup.test.jsx b/frontend/tests/MarkerPopup/MarkerPopup.test.jsx index 35491106..79f2dfa2 100644 --- a/frontend/tests/MarkerPopup/MarkerPopup.test.jsx +++ b/frontend/tests/MarkerPopup/MarkerPopup.test.jsx @@ -201,18 +201,22 @@ describe('MarkerPopup lazy marker styling', () => { delete globalThis.MARKER_STYLES; }); + // render() already wraps itself in act(), so there's nothing left for a caller + // to flush - wrapping it again is redundant (and duplicated across the two + // tests below, which is what this helper avoids). + const renderLazyLocationMarker = () => + render( + + + , + ); + it('fetches marker styling once the marker becomes individually visible', async () => { - await act(async () => { - render( - - - , - ); - }); + renderLazyLocationMarker(); expect(httpService.getMarkerStyles).not.toHaveBeenCalledWith([lazyLocation.uuid]); @@ -225,17 +229,7 @@ describe('MarkerPopup lazy marker styling', () => { }); it('re-renders the marker with the lazily-fetched icon once it arrives', async () => { - await act(async () => { - render( - - - , - ); - }); + renderLazyLocationMarker(); // Nothing matched yet - default Leaflet icon, no custom pin expect(document.querySelector('.custom-typed-marker-icon')).not.toBeInTheDocument(); From 50fad555e728812c16094022b13055d54fc57ff4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Krzysztof=20Ko=C5=82odzi=C5=84ski?= Date: Thu, 20 Aug 2026 23:36:12 +0200 Subject: [PATCH 27/28] refactor --- goodmap/data_models/location.py | 3 +- tests/unit_tests/test_core_api.py | 112 ++++++++++-------------------- 2 files changed, 38 insertions(+), 77 deletions(-) diff --git a/goodmap/data_models/location.py b/goodmap/data_models/location.py index 8993eca3..8b363cdd 100644 --- a/goodmap/data_models/location.py +++ b/goodmap/data_models/location.py @@ -85,8 +85,7 @@ def model_dump(self, **kwargs) -> dict[str, Any]: return super().model_dump(**kwargs) def basic_info(self) -> dict[str, Any]: - """Get basic location information summary: identity and position only. - """ + """Get basic location information summary: identity and position only.""" return self.model_dump(include={"uuid", "position"}) diff --git a/tests/unit_tests/test_core_api.py b/tests/unit_tests/test_core_api.py index 4185348d..4a8712a9 100644 --- a/tests/unit_tests/test_core_api.py +++ b/tests/unit_tests/test_core_api.py @@ -313,27 +313,44 @@ def test_get_locations_accepts_valid_and_undeclared_parameters(test_app, query): assert response.status_code == 200 +# Fixture shared by the /api/locations and /api/locations/marker-styles tests +# below: point_type-categorized locker locations with a matching marker_styles +# config. Kept as data + a small factory, not one big db_overrides literal per +# test, so each test only states what it actually varies. +_LOCKER_LOCATIONS = [ + { + "name": "locker-1", + "position": [50, 50], + "point_type": "parcel_locker", + "uuid": "11111111-1111-1111-1111-111111111111", + }, + { + "name": "locker-2", + "position": [51, 51], + "point_type": "container", + "uuid": "22222222-2222-2222-2222-222222222222", + }, +] + + +def _create_marker_styles_test_app(data=_LOCKER_LOCATIONS, **db_overrides): + overrides = { + "categories": {"point_type": ["parcel_locker", "container"]}, + "location_obligatory_fields": [("point_type", "str"), ("name", "str")], + "marker_styles": {"icon_field": "point_type", "icons": {}, "colors": {}}, + "data": data, + "visible_data": ["name", "point_type"], + } + overrides.update(db_overrides) + return create_test_app(db_overrides=overrides) + + def test_get_locations_omits_marker_style_field_values(): """/api/locations should not surface the field marker_styles.icon_field points at (e.g. a point-type category) - that value is fetched lazily via /api/locations/marker-styles, only once a marker is individually visible, instead of upfront for every location (see lazy-load-marker-styling-plan.md).""" - client = create_test_app( - db_overrides={ - "categories": {"point_type": ["parcel_locker", "container"]}, - "location_obligatory_fields": [("point_type", "str"), ("name", "str")], - "marker_styles": {"icon_field": "point_type", "icons": {}, "colors": {}}, - "data": [ - { - "name": "locker-1", - "position": [50, 50], - "point_type": "parcel_locker", - "uuid": "11111111-1111-1111-1111-111111111111", - }, - ], - "visible_data": ["name", "point_type"], - } - ) + client = _create_marker_styles_test_app(data=_LOCKER_LOCATIONS[:1]) response = client.get("/api/locations") @@ -349,28 +366,7 @@ def test_get_locations_omits_marker_style_field_values(): def test_get_locations_marker_styles_returns_requested_uuids_styling(): """The lazy marker-styles endpoint returns just the marker_styles-relevant field values for the requested uuids, not the full location.""" - client = create_test_app( - db_overrides={ - "categories": {"point_type": ["parcel_locker", "container"]}, - "location_obligatory_fields": [("point_type", "str"), ("name", "str")], - "marker_styles": {"icon_field": "point_type", "icons": {}, "colors": {}}, - "data": [ - { - "name": "locker-1", - "position": [50, 50], - "point_type": "parcel_locker", - "uuid": "11111111-1111-1111-1111-111111111111", - }, - { - "name": "locker-2", - "position": [51, 51], - "point_type": "container", - "uuid": "22222222-2222-2222-2222-222222222222", - }, - ], - "visible_data": ["name", "point_type"], - } - ) + client = _create_marker_styles_test_app() response = client.get("/api/locations/marker-styles?uuid=11111111-1111-1111-1111-111111111111") @@ -384,28 +380,7 @@ def test_get_locations_marker_styles_returns_requested_uuids_styling(): def test_get_locations_marker_styles_supports_multiple_uuids(): - client = create_test_app( - db_overrides={ - "categories": {"point_type": ["parcel_locker", "container"]}, - "location_obligatory_fields": [("point_type", "str"), ("name", "str")], - "marker_styles": {"icon_field": "point_type", "icons": {}, "colors": {}}, - "data": [ - { - "name": "locker-1", - "position": [50, 50], - "point_type": "parcel_locker", - "uuid": "11111111-1111-1111-1111-111111111111", - }, - { - "name": "locker-2", - "position": [51, 51], - "point_type": "container", - "uuid": "22222222-2222-2222-2222-222222222222", - }, - ], - "visible_data": ["name", "point_type"], - } - ) + client = _create_marker_styles_test_app() response = client.get( "/api/locations/marker-styles" @@ -429,21 +404,8 @@ def test_get_locations_marker_styles_supports_multiple_uuids(): def test_get_locations_marker_styles_omits_unknown_uuids(): """An unknown/re-clustered-away uuid doesn't error the whole request - it's just absent from the response.""" - client = create_test_app( - db_overrides={ - "categories": {"point_type": ["parcel_locker"]}, - "location_obligatory_fields": [("point_type", "str"), ("name", "str")], - "marker_styles": {"icon_field": "point_type", "icons": {}, "colors": {}}, - "data": [ - { - "name": "locker-1", - "position": [50, 50], - "point_type": "parcel_locker", - "uuid": "11111111-1111-1111-1111-111111111111", - }, - ], - "visible_data": ["name", "point_type"], - } + client = _create_marker_styles_test_app( + data=_LOCKER_LOCATIONS[:1], categories={"point_type": ["parcel_locker"]} ) response = client.get( From 3c0b60d574e709dd249f143007552e002548f24d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Krzysztof=20Ko=C5=82odzi=C5=84ski?= Date: Fri, 21 Aug 2026 00:38:22 +0200 Subject: [PATCH 28/28] lot of files changes --- README.md | 1 - docs/configuration.rst | 18 +----- docs/data-source.rst | 11 +--- docs/quickstart.rst | 1 - e2e-tests/e2e_stress_test_config.yml | 1 - e2e-tests/e2e_test_config.template.yml | 1 - examples/e2e_test_config.yml | 1 - goodmap/db.py | 8 +-- goodmap/feature_flags.py | 5 -- goodmap/goodmap.py | 81 ++++++++------------------ goodmap/templates/goodmap-admin.html | 1 - goodmap/templates/map.html | 1 - tests/unit_tests/conftest.py | 4 +- tests/unit_tests/test_core_api.py | 1 - tests/unit_tests/test_goodmap.py | 22 ++++--- 15 files changed, 50 insertions(+), 107 deletions(-) diff --git a/README.md b/README.md index 4faf2e3d..fdec3d60 100644 --- a/README.md +++ b/README.md @@ -62,7 +62,6 @@ Afterwards run it with: | Option | Description | |--------------------------|------------------------------------------------------------------------------------------------------------------------------------| -| USE_LAZY_LOADING | Loads point data only after the user clicks a point. If set to false, point data is loaded together with the initial map. | | FAKE_LOGIN | If set to true, allows access to the admin panel by simply selecting the role instead of logging in. **DO NOT USE IN PRODUCTION!** | | SHOW_ACCESSIBILITY_TABLE | If set as true it shows special view to help with accessing application. | diff --git a/docs/configuration.rst b/docs/configuration.rst index 4562b0d2..5bc840a5 100644 --- a/docs/configuration.rst +++ b/docs/configuration.rst @@ -60,7 +60,6 @@ Everything below in one file — copy it and delete what you do not need: max_size: 5242880 # 5 MiB FEATURE_FLAGS: - USE_LAZY_LOADING: true CATEGORIES_HELP: true SHOW_SEARCH_BAR: true SHOW_SUGGEST_NEW_POINT_BUTTON: true @@ -165,8 +164,7 @@ Basic keys Feature flags ------------- -``FEATURE_FLAGS`` is a flat mapping of flag name to boolean. Unset flags are off, with one -exception: ``USE_LAZY_LOADING`` defaults to on. +``FEATURE_FLAGS`` is a flat mapping of flag name to boolean. Unset flags are off. Flags fall into two groups: some change what the backend does, others are handed to the frontend to decide what to render. Both are set the same way. @@ -178,13 +176,6 @@ frontend to decide what to render. Both are set the same way. * - Flag - Acts on - Effect - * - ``USE_LAZY_LOADING`` - - backend - - **On by default.** Builds the location model from ``location_obligatory_fields`` - and ``categories`` in your data source, so submitted points are validated against - them, and the "suggest a new point" form is generated from them. Set it to - ``false`` and only ``uuid``, ``position`` and ``remark`` are validated, and the - suggest form has no fields — see the note below. * - ``CATEGORIES_HELP`` - both - Enables the help-tooltip data in ``/api/categories-full``, and makes the frontend @@ -219,13 +210,6 @@ frontend to decide what to render. Both are set the same way. Never enable ``FAKE_LOGIN`` in production. It hands a logged-in session to anyone who asks for one. -.. note:: - - ``USE_LAZY_LOADING`` is named for behaviour that is now unconditional: point details - have their own endpoint (``/api/location/``) whether the flag is set or not. - What the flag still controls is schema validation, as described above. Leave it on - unless you have a reason not to. - The frontend receives the whole ``FEATURE_FLAGS`` mapping, so a plugin or a custom build can read flags Goodmap itself does not know about. diff --git a/docs/data-source.rst b/docs/data-source.rst index 1c567376..bdf0e37d 100644 --- a/docs/data-source.rst +++ b/docs/data-source.rst @@ -27,8 +27,9 @@ and their schema, alongside platzky's ``site_content`` section: Note that ``plugins`` is a **sibling** of ``map``, not a key inside it. -Only ``data`` and ``categories`` are structurally required; ``suggestions`` and -``reports`` are created by the app as users submit things. +Only ``data`` is structurally required. ``categories`` defaults to no categories +if omitted (a map with only plain, unfiltered points is a valid setup); ``suggestions`` +and ``reports`` are created by the app as users submit things. Points ------ @@ -108,12 +109,6 @@ This drives three things at once: - **Length limits.** String fields are capped at 200 characters, lists at 20 items of at most 100 characters each. -.. important:: - - This key is only read when the ``USE_LAZY_LOADING`` feature flag is on. With it off, - nothing beyond ``uuid``/``position``/``remark`` is validated and the suggest form comes - up empty. See :ref:`config-feature-flags`. - .. _data-model-visible_data: ``visible_data`` and ``meta_data`` diff --git a/docs/quickstart.rst b/docs/quickstart.rst index e6c2fe08..7da7b986 100644 --- a/docs/quickstart.rst +++ b/docs/quickstart.rst @@ -131,7 +131,6 @@ Create ``config.yml`` next to it: PATH: data.json FEATURE_FLAGS: - USE_LAZY_LOADING: true SHOW_SEARCH_BAR: true SHOW_SUGGEST_NEW_POINT_BUTTON: true diff --git a/e2e-tests/e2e_stress_test_config.yml b/e2e-tests/e2e_stress_test_config.yml index 8cf34d8c..50aba4fe 100644 --- a/e2e-tests/e2e_stress_test_config.yml +++ b/e2e-tests/e2e_stress_test_config.yml @@ -19,7 +19,6 @@ LANGUAGES: country: PL FEATURE_FLAGS: - USE_LAZY_LOADING: True SHOW_ACCESSIBILITY_TABLE: True USE_SERVER_SIDE_CLUSTERING: False CATEGORIES_HELP: True diff --git a/e2e-tests/e2e_test_config.template.yml b/e2e-tests/e2e_test_config.template.yml index e73ba88d..f1571613 100644 --- a/e2e-tests/e2e_test_config.template.yml +++ b/e2e-tests/e2e_test_config.template.yml @@ -21,7 +21,6 @@ LANGUAGES: country: PL FEATURE_FLAGS: - USE_LAZY_LOADING: True SHOW_ACCESSIBILITY_TABLE: True USE_SERVER_SIDE_CLUSTERING: False CATEGORIES_HELP: True diff --git a/examples/e2e_test_config.yml b/examples/e2e_test_config.yml index f80e4970..4276a68c 100644 --- a/examples/e2e_test_config.yml +++ b/examples/e2e_test_config.yml @@ -16,7 +16,6 @@ LANGUAGES: country: PL FEATURE_FLAGS: - USE_LAZY_LOADING: True USE_SERVER_SIDE_CLUSTERING: False SHOW_ACCESSIBILITY_TABLE: True FAKE_LOGIN: False diff --git a/goodmap/db.py b/goodmap/db.py index cdd3ec8b..a1213650 100644 --- a/goodmap/db.py +++ b/goodmap/db.py @@ -667,7 +667,7 @@ def json_db_get_category_data(self, category_type=None): """Return category data from in-memory JSON database, optionally filtered by type.""" if category_type: return { - "categories": {category_type: self.data["categories"].get(category_type, [])}, + "categories": {category_type: self.data.get("categories", {}).get(category_type, [])}, "categories_help": self.data.get("categories_help", []), "categories_options_help": { category_type: self.data.get("categories_options_help", {}).get(category_type, []) @@ -682,7 +682,7 @@ def json_db_get_category_data(self, category_type=None): }, } return { - "categories": self.data["categories"], + "categories": self.data.get("categories", {}), "categories_help": self.data.get("categories_help", []), "categories_options_help": self.data.get("categories_options_help", {}), "categories_default_checked": self.data.get("categories_default_checked", {}), @@ -696,7 +696,7 @@ def json_file_db_get_category_data(self, category_type=None): data = json.load(file)["map"] if category_type: return { - "categories": {category_type: data["categories"].get(category_type, [])}, + "categories": {category_type: data.get("categories", {}).get(category_type, [])}, "categories_help": data.get("categories_help", []), "categories_options_help": { category_type: data.get("categories_options_help", {}).get(category_type, []) @@ -709,7 +709,7 @@ def json_file_db_get_category_data(self, category_type=None): }, } return { - "categories": data["categories"], + "categories": data.get("categories", {}), "categories_help": data.get("categories_help", []), "categories_options_help": data.get("categories_options_help", {}), "categories_default_checked": data.get("categories_default_checked", {}), diff --git a/goodmap/feature_flags.py b/goodmap/feature_flags.py index 61ffe81c..2d6abb0f 100644 --- a/goodmap/feature_flags.py +++ b/goodmap/feature_flags.py @@ -5,15 +5,10 @@ Flags: CategoriesHelp: Display help text alongside map categories to guide users. - UseLazyLoading: Defer loading of location fields until they are needed, - improving initial page load performance. EnableAdminPanel: Expose the admin panel for managing map data. """ from platzky import FeatureFlag CategoriesHelp = FeatureFlag(alias="CATEGORIES_HELP", description="Show category help text") -UseLazyLoading = FeatureFlag( - alias="USE_LAZY_LOADING", default=True, description="Enable lazy loading of location fields" -) EnableAdminPanel = FeatureFlag(alias="ENABLE_ADMIN_PANEL", description="Enable admin panel") diff --git a/goodmap/goodmap.py b/goodmap/goodmap.py index ff7fe89e..9bca9f96 100644 --- a/goodmap/goodmap.py +++ b/goodmap/goodmap.py @@ -13,7 +13,6 @@ from platzky.models import CmsModule from platzky.plugin.content_transformer import ContentTransformerPluginBase from platzky.shortcodes import Shortcode -from pydantic import BaseModel from goodmap.api.admin_api import admin_pages from goodmap.api.core_api import core_pages @@ -21,9 +20,11 @@ from goodmap.data_models.location import create_location_model from goodmap.db import ( extend_db_with_goodmap_queries, + get_category_data, get_location_obligatory_fields, + get_marker_styles, ) -from goodmap.feature_flags import EnableAdminPanel, UseLazyLoading +from goodmap.feature_flags import EnableAdminPanel from goodmap.plugin import CAPABILITY_BASES, GoodmapPluginBase logger = logging.getLogger(__name__) @@ -110,51 +111,6 @@ def _add_cors(response): return None, [] -def _setup_location_model( - db: Any, -) -> tuple[list[Any], dict[str, Any], type[BaseModel], Any, frozenset[str]]: - """Configure location model and db with lazy-loading and categories support. - - Args: - db: The database instance to extend with location queries. - - Returns: - Tuple of (obligatory_fields, categories, location_model, db, pin_marker_fields). - pin_marker_fields is app-wiring knowledge - which of this deployment's fields - the marker_styles config (icon_field/color_field) actually points at - not - something the location model itself needs to know; it's threaded to core_pages() - for goodmap.api.api_models.marker_style_values() to use. - """ - obligatory_fields = get_location_obligatory_fields(db) - location_model = create_location_model(obligatory_fields, {}) - extended_db = extend_db_with_goodmap_queries(db, location_model) - - try: - category_data = extended_db.get_category_data() - categories = category_data.get("categories", {}) - except (KeyError, AttributeError): - categories = {} - - try: - marker_styles = extended_db.get_marker_styles() - except (KeyError, AttributeError): - marker_styles = {} - marker_style_fields = { - field - for field in (marker_styles.get("icon_field"), marker_styles.get("color_field")) - if field is not None - } - - if categories or marker_style_fields: - location_model = create_location_model(obligatory_fields, categories) - extended_db = extend_db_with_goodmap_queries(extended_db, location_model) - - field_names = {name for name, _ in obligatory_fields} - pin_marker_fields = frozenset(marker_style_fields) & field_names - - return obligatory_fields, categories, location_model, extended_db, pin_marker_fields - - def create_app(config_path: str) -> platzky.Engine: """Create Goodmap application from YAML configuration file. @@ -212,15 +168,28 @@ def create_app_from_config(config: GoodmapConfig) -> platzky.Engine: if app.config.get("MAX_CONTENT_LENGTH") is None: app.config["MAX_CONTENT_LENGTH"] = config.attachment.max_size + MULTIPART_OVERHEAD_ALLOWANCE - if app.is_enabled(UseLazyLoading): - location_obligatory_fields, _, location_model, app.db, pin_marker_fields = ( - _setup_location_model(app.db) - ) - else: - location_obligatory_fields = [] - location_model = create_location_model([], {}) - app.db = extend_db_with_goodmap_queries(app.db, location_model) - pin_marker_fields = frozenset() + # Build this deployment's location model from its data source and extend app.db + # with the query functions it needs. categories/marker_styles are both optional + # (see docs/data-source.rst) - every backend's get_category_data()/ + # get_marker_styles() already defaults them to {} internally. + location_obligatory_fields = get_location_obligatory_fields(app.db) + categories = get_category_data(app.db)(app.db)["categories"] + marker_styles = get_marker_styles(app.db)(app.db) + marker_style_fields = { + field + for field in (marker_styles.get("icon_field"), marker_styles.get("color_field")) + if field is not None + } + + location_model = create_location_model(location_obligatory_fields, categories) + app.db = extend_db_with_goodmap_queries(app.db, location_model) + + obligatory_field_names = {name for name, _ in location_obligatory_fields} + # pin_marker_fields is app-wiring knowledge - which of this deployment's fields + # marker_styles.icon_field/color_field actually point at - not something the + # location model itself needs to know; threaded to core_pages() for + # goodmap.api.api_models.marker_style_values() to use. + pin_marker_fields = frozenset(marker_style_fields) & obligatory_field_names app.extensions["goodmap"] = {"location_obligatory_fields": location_obligatory_fields} diff --git a/goodmap/templates/goodmap-admin.html b/goodmap/templates/goodmap-admin.html index efd6bc85..3d4bcaf4 100644 --- a/goodmap/templates/goodmap-admin.html +++ b/goodmap/templates/goodmap-admin.html @@ -741,7 +741,6 @@

{{ gettext("Reports") }}

window.SHOW_SUGGEST_NEW_POINT_BUTTON = {{ feature_flags.SHOW_SUGGEST_NEW_POINT_BUTTON | default(false) | tojson }}; window.SHOW_SEARCH_BAR = {{ feature_flags.SHOW_SEARCH_BAR | default(false) | tojson }}; - window.USE_LAZY_LOADING = {{ feature_flags.USE_LAZY_LOADING | default(false) | tojson }}; window.SHOW_ACCESSIBILITY_TABLE = {{ feature_flags.SHOW_ACCESSIBILITY_TABLE | default(false) | tojson }}; diff --git a/goodmap/templates/map.html b/goodmap/templates/map.html index 7ada444f..4852a8a9 100644 --- a/goodmap/templates/map.html +++ b/goodmap/templates/map.html @@ -116,7 +116,6 @@ window.SHOW_SUGGEST_NEW_POINT_BUTTON = {{ feature_flags.SHOW_SUGGEST_NEW_POINT_BUTTON | default(false) | tojson }}; window.SHOW_SEARCH_BAR = {{ feature_flags.SHOW_SEARCH_BAR | default(false) | tojson }}; -window.USE_LAZY_LOADING = {{ feature_flags.USE_LAZY_LOADING | default(false) | tojson }}; window.USE_SERVER_SIDE_CLUSTERING = {{ feature_flags.USE_SERVER_SIDE_CLUSTERING | default(false) | tojson }}; window.SHOW_ACCESSIBILITY_TABLE = {{ feature_flags.SHOW_ACCESSIBILITY_TABLE | default(false) | tojson }}; window.FEATURE_FLAGS = {{ feature_flags | tojson }}; diff --git a/tests/unit_tests/conftest.py b/tests/unit_tests/conftest.py index 55155568..2f97c2d7 100644 --- a/tests/unit_tests/conftest.py +++ b/tests/unit_tests/conftest.py @@ -5,7 +5,7 @@ from platzky import FeatureFlag, FeatureFlagSet from goodmap.config import GoodmapConfig -from goodmap.feature_flags import CategoriesHelp, EnableAdminPanel, UseLazyLoading +from goodmap.feature_flags import CategoriesHelp, EnableAdminPanel from goodmap.goodmap import create_app_from_config @@ -101,7 +101,7 @@ def multipart_suggest_post(client, location, photo=None): def create_test_app( - feature_flags=make_flag_set(CategoriesHelp, UseLazyLoading, EnableAdminPanel), + feature_flags=make_flag_set(CategoriesHelp, EnableAdminPanel), db_overrides=None, ): """Create a test app with optional feature flags and db overrides.""" diff --git a/tests/unit_tests/test_core_api.py b/tests/unit_tests/test_core_api.py index 4a8712a9..d8a3aab8 100644 --- a/tests/unit_tests/test_core_api.py +++ b/tests/unit_tests/test_core_api.py @@ -1173,7 +1173,6 @@ def test_issue_options_defaults_to_empty_when_missing(): config_data = get_test_config_data() config_data["FEATURE_FLAGS"] = { "CATEGORIES_HELP": True, - "USE_LAZY_LOADING": True, "ENABLE_ADMIN_PANEL": True, } config_data["DB"]["DATA"].pop("reported_issue_types", None) diff --git a/tests/unit_tests/test_goodmap.py b/tests/unit_tests/test_goodmap.py index 7d3e654b..aa7b79dd 100644 --- a/tests/unit_tests/test_goodmap.py +++ b/tests/unit_tests/test_goodmap.py @@ -14,7 +14,7 @@ from goodmap import goodmap from goodmap.config import GoodmapConfig -from goodmap.feature_flags import EnableAdminPanel, UseLazyLoading +from goodmap.feature_flags import EnableAdminPanel from goodmap.plugin import ( CAPABILITY_BASES, MapOverlayPluginBase, @@ -37,7 +37,14 @@ def test_create_app(): def test_create_app_from_config(): with patch("platzky.platzky.create_app_from_config", MagicMock()) as mock_platzky_app_creation: mock_platzky_app_creation.return_value.is_enabled.return_value = False - with patch("goodmap.goodmap.extend_db_with_goodmap_queries", MagicMock()) as mock_extend_db: + with ( + patch("goodmap.goodmap.extend_db_with_goodmap_queries", MagicMock()) as mock_extend_db, + patch("goodmap.goodmap.get_location_obligatory_fields", return_value=[]), + patch("goodmap.goodmap.get_category_data") as mock_get_category_data, + patch("goodmap.goodmap.get_marker_styles") as mock_get_marker_styles, + ): + mock_get_category_data.return_value.return_value = {"categories": {}} + mock_get_marker_styles.return_value.return_value = {} goodmap.create_app_from_config(config) mock_platzky_app_creation.assert_called_once_with( config, @@ -56,12 +63,13 @@ def test_create_app_delegation(mock_parse_yaml, mock_create_app_from_config): @mock.patch("goodmap.goodmap.get_location_obligatory_fields") -def test_use_lazy_loading_branch(mock_get_location_obligatory_fields): +def test_location_model_is_always_built_from_the_data_source(mock_get_location_obligatory_fields): + """Building the location model from location_obligatory_fields/categories is + unconditional - there's no flag that skips it (see feature_flags.py).""" config = GoodmapConfig( APP_NAME="test_lazy", SECRET_KEY="secret", DB=JsonDbConfig(DATA={"site_content": {}, "location_obligatory_fields": []}, TYPE="json"), - FEATURE_FLAGS=make_flag_set(UseLazyLoading), ) app = goodmap.create_app_from_config(config) @@ -284,8 +292,9 @@ def test_map_route_overrides_photo_constraints(): assert photo["allowed_extensions"] == ["jpeg", "jpg", "png"] -def test_location_schema_endpoint_with_lazy_loading(): - """The schema includes obligatory_fields when USE_LAZY_LOADING is enabled.""" +def test_location_schema_endpoint_includes_obligatory_fields(): + """The schema includes this deployment's obligatory_fields - unconditional, + there's no flag that skips building the location model from them.""" config = GoodmapConfig( APP_NAME="test_app", SECRET_KEY="test_secret", @@ -303,7 +312,6 @@ def test_location_schema_endpoint_with_lazy_loading(): }, TYPE="json", ), - FEATURE_FLAGS=make_flag_set(UseLazyLoading), ) app = goodmap.create_app_from_config(config) # CSRF protection must be disabled in test environment to allow API testing