From 292ae1e02c04c82688e7d8abfa3d7120e4179db9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Krzysztof=20Ko=C5=82odzi=C5=84ski?= Date: Wed, 19 Aug 2026 14:07:38 +0200 Subject: [PATCH 01/18] chore: refactor location schema --- frontend/src/components/Map/Map.jsx | 13 ++-- .../Map/components/SuggestNewPointDialog.jsx | 23 ++++--- .../MarkerPopup/ReportProblemForm.jsx | 10 +-- frontend/src/services/http/endpoints.js | 6 ++ frontend/src/services/http/httpService.js | 19 ++++++ frontend/tests/CategoriesContext.test.jsx | 19 ++++-- .../Map/components/SuggestNewPoint.test.jsx | 17 +++-- .../MarkerPopup/ReportProblemForm.test.jsx | 23 +++++-- goodmap/goodmap.py | 41 ++---------- goodmap/templates/map.html | 1 - tests/unit_tests/test_goodmap.py | 65 ++++++++++--------- 11 files changed, 133 insertions(+), 104 deletions(-) diff --git a/frontend/src/components/Map/Map.jsx b/frontend/src/components/Map/Map.jsx index ba009829..0e76ce4d 100644 --- a/frontend/src/components/Map/Map.jsx +++ b/frontend/src/components/Map/Map.jsx @@ -4,12 +4,15 @@ import { createPortal } from 'react-dom'; import FiltersForm from '../FiltersForm/FiltersForm'; import MapComponent from './MapComponent'; import { CategoriesProvider } from '../Categories/CategoriesContext'; +import { LocationSchemaProvider } from './context/LocationSchemaContext'; import AppToaster from '../common/AppToaster'; /** * Wrapper component that renders the map and filters form into their respective DOM placeholders. * Uses React portals to render components into pre-existing DOM elements outside the React tree. - * Wraps both components with CategoriesProvider for shared filter state management. + * Wraps both components with CategoriesProvider for shared filter state management, and + * with LocationSchemaProvider so the suggest and report forms know what this deployment + * accepts without each fetching it. * * @returns {React.ReactElement|null} Portals for FiltersForm and MapComponent, or null if placeholders not found */ @@ -24,9 +27,11 @@ const MapWrap = () => { return ( - - {createPortal(, filtersPlaceholder)} - {createPortal(, mapPlaceholder)} + + + {createPortal(, filtersPlaceholder)} + {createPortal(, mapPlaceholder)} + ); }; diff --git a/frontend/src/components/Map/components/SuggestNewPointDialog.jsx b/frontend/src/components/Map/components/SuggestNewPointDialog.jsx index 47a7e41f..3226cd2a 100644 --- a/frontend/src/components/Map/components/SuggestNewPointDialog.jsx +++ b/frontend/src/components/Map/components/SuggestNewPointDialog.jsx @@ -26,6 +26,7 @@ import axios from 'axios'; import { useTranslation } from 'react-i18next'; import imageCompression from 'browser-image-compression'; import getCsrfToken from '../../../utils/csrf'; +import { useLocationSchema } from '../context/LocationSchemaContext'; import { useLocation } from '../context/LocationContext'; import { useCategories } from '../../Categories/CategoriesContext'; import toast from '../../../utils/toast'; @@ -107,9 +108,9 @@ const useScrollToTop = trigger => { }; /** - * Dialog form for suggesting a new map point. Fields are generated dynamically from - * window.LOCATION_SCHEMA and include the user's position, an optional photo, and every - * obligatory location attribute defined by the backend. + * Dialog form for suggesting a new map point. Fields are generated dynamically from the + * deployment's location schema and include the user's position, an optional photo, and + * every obligatory location attribute defined by the backend. * * @param {{open: boolean, onClose: () => void}} props * @returns {React.ReactElement} Dialog with the new point suggestion form @@ -140,10 +141,7 @@ const SuggestNewPointDialog = ({ open, onClose }) => { } }, [open]); - const locationSchema = globalThis.LOCATION_SCHEMA || { - obligatory_fields: [], - categories: {}, - }; + const { locationSchema } = useLocationSchema(); const { allowed_extensions: allowedPhotoExtensions = [], allowed_mime_types: allowedPhotoMimeTypes = [], @@ -173,6 +171,13 @@ const SuggestNewPointDialog = ({ open, onClose }) => { const [formFields, setFormFields] = useState(initializeFormFields); + // The schema is fetched, so it is empty on the first render and the initial state + // above has nothing to build from. Rebuild the fields once it arrives. + useEffect(() => { + setFormFields(initializeFormFields()); + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [locationSchema]); + const handleLocateMe = () => { requestLocationWithFeedback(); }; @@ -317,7 +322,9 @@ const SuggestNewPointDialog = ({ open, onClose }) => { > {categoryOptions.map(option => ( - + ))} diff --git a/frontend/src/components/MarkerPopup/ReportProblemForm.jsx b/frontend/src/components/MarkerPopup/ReportProblemForm.jsx index 9a2405b0..ff323b89 100644 --- a/frontend/src/components/MarkerPopup/ReportProblemForm.jsx +++ b/frontend/src/components/MarkerPopup/ReportProblemForm.jsx @@ -4,6 +4,7 @@ import styled from 'styled-components'; import PropTypes from 'prop-types'; import { useTranslation } from 'react-i18next'; import getCsrfToken from '../../utils/csrf'; +import { useLocationSchema } from '../Map/context/LocationSchemaContext'; /** * Styled form component with flexbox column layout. @@ -123,14 +124,14 @@ const SuccessMessage = styled.div` */ /** * Get issue type options from backend configuration or fall back to defaults. - * Dynamic types come from LOCATION_SCHEMA.reported_issue_types (configured per deployment). + * Dynamic types come from the deployment's location schema (reported_issue_types). * Default types are kept for backward compatibility with backends that don't provide this field (until 2.0.0). * * @param {Function} t - Translation function + * @param {Array<{value: string, label: string}>} [dynamicTypes] - Types this deployment declares * @returns {Array<{value: string, label: string}>} Issue type options (without "other", which is always appended) */ -const getIssueTypeOptions = t => { - const dynamicTypes = globalThis.LOCATION_SCHEMA?.reported_issue_types; +const getIssueTypeOptions = (t, dynamicTypes) => { if (dynamicTypes && dynamicTypes.length > 0) { return dynamicTypes.map(type => ({ value: type.value, @@ -147,12 +148,13 @@ const getIssueTypeOptions = t => { const ReportProblemForm = ({ placeId }) => { const { t } = useTranslation(); + const { locationSchema } = useLocationSchema(); const [problem, setProblem] = useState(''); const [problemType, setProblemType] = useState(''); const [isSubmitted, setIsSubmitted] = useState(false); const [responseMessage, setResponseMessage] = useState(''); - const issueTypeOptions = getIssueTypeOptions(t); + const issueTypeOptions = getIssueTypeOptions(t, locationSchema.reported_issue_types); const handleSubmit = async event => { event.preventDefault(); diff --git a/frontend/src/services/http/endpoints.js b/frontend/src/services/http/endpoints.js index b8d0bef1..282b60fa 100644 --- a/frontend/src/services/http/endpoints.js +++ b/frontend/src/services/http/endpoints.js @@ -4,6 +4,12 @@ */ export const CATEGORIES_FULL = '/api/categories-full'; +/** + * API endpoint describing what this deployment accepts for a new point: + * the fields, their allowed values, reportable issue types and photo limits. + */ +export const LOCATION_SCHEMA = '/api/location-schema'; + /** * API endpoint for fetching a single location by ID. * Use with location UUID appended: /api/location/{uuid} diff --git a/frontend/src/services/http/httpService.js b/frontend/src/services/http/httpService.js index 6a12e810..01999f3c 100644 --- a/frontend/src/services/http/httpService.js +++ b/frontend/src/services/http/httpService.js @@ -1,5 +1,6 @@ import { CATEGORIES_FULL, + LOCATION_SCHEMA, LOCATION, LOCATIONS, SEARCH_ADDRESS, @@ -74,6 +75,24 @@ const httpService = { * category data plus a map of category key to the option values that should be * pre-checked by default. */ + /** + * Fetches the schema this deployment accepts for a new point. + * + * The accepted fields are configured per deployment, so they are read from the + * running instance rather than assumed. + * + * @returns {Promise} Promise resolving to the location schema + */ + getLocationSchema: async () => { + const response = await fetch(LOCATION_SCHEMA, { + method: 'GET', + headers: { + 'Content-Type': 'application/json', + }, + }); + return jsonOrThrow(response, 'location schema'); + }, + getCategoriesData: async () => { const response = await fetch(CATEGORIES_FULL).then(res => res.json()); const useCategoriesHelp = Boolean(globalThis.FEATURE_FLAGS?.CATEGORIES_HELP); diff --git a/frontend/tests/CategoriesContext.test.jsx b/frontend/tests/CategoriesContext.test.jsx index d8780fab..986bbef1 100644 --- a/frontend/tests/CategoriesContext.test.jsx +++ b/frontend/tests/CategoriesContext.test.jsx @@ -5,12 +5,17 @@ import FiltersForm from '../src/components/FiltersForm/FiltersForm'; import SuggestNewPointButton from '../src/components/Map/components/SuggestNewPointButton'; import { CategoriesProvider } from '../src/components/Categories/CategoriesContext'; import { LocationProvider } from '../src/components/Map/context/LocationContext'; +import { LocationSchemaProvider } from '../src/components/Map/context/LocationSchemaContext'; import httpService from '../src/services/http/httpService'; jest.mock('axios'); jest.mock('../src/services/http/httpService', () => ({ __esModule: true, - default: { getCategoriesData: jest.fn(), getLocations: jest.fn() }, + default: { + getCategoriesData: jest.fn(), + getLocations: jest.fn(), + getLocationSchema: jest.fn(), + }, })); jest.mock('../src/utils/toast', () => ({ __esModule: true, @@ -22,14 +27,16 @@ jest.mock('browser-image-compression'); // The provider owns the fetch so they share one request instead of making two. test('categories are fetched once for all consumers', async () => { httpService.getCategoriesData.mockResolvedValue({ categories: [], defaultChecked: {} }); - globalThis.LOCATION_SCHEMA = { obligatory_fields: [['name', 'str']], categories: {} }; + httpService.getLocationSchema.mockResolvedValue({}); render( - - - - + + + + + + , ); diff --git a/frontend/tests/Map/components/SuggestNewPoint.test.jsx b/frontend/tests/Map/components/SuggestNewPoint.test.jsx index 789aad1f..fa223cc6 100644 --- a/frontend/tests/Map/components/SuggestNewPoint.test.jsx +++ b/frontend/tests/Map/components/SuggestNewPoint.test.jsx @@ -6,6 +6,7 @@ import imageCompression from 'browser-image-compression'; import SuggestNewPointButton from '../../../src/components/Map/components/SuggestNewPointButton'; import { LocationProvider } from '../../../src/components/Map/context/LocationContext'; import { CategoriesProvider } from '../../../src/components/Categories/CategoriesContext'; +import { LocationSchemaProvider } from '../../../src/components/Map/context/LocationSchemaContext'; import { mockGeolocationSuccess, mockGeolocationError, @@ -25,7 +26,9 @@ import toast from '../../../src/utils/toast'; const renderWithProvider = component => render( - {component} + + {component} + , ); @@ -34,6 +37,7 @@ jest.mock('../../../src/services/http/httpService', () => ({ __esModule: true, default: { getCategoriesData: jest.fn(), + getLocationSchema: jest.fn(), }, })); jest.mock('../../../src/utils/toast', () => ({ @@ -49,7 +53,7 @@ beforeEach(() => { metaTag.setAttribute('content', 'test-csrf-token'); document.head.appendChild(metaTag); - globalThis.LOCATION_SCHEMA = FULL_SCHEMA; + httpService.getLocationSchema.mockResolvedValue(FULL_SCHEMA); // Mock categories data matching httpService.getCategoriesData()'s real // { categories: [{ categoryKey, categoryName, options }] } shape. @@ -82,7 +86,6 @@ afterEach(() => { if (metaTag) { metaTag.remove(); } - delete globalThis.LOCATION_SCHEMA; jest.clearAllMocks(); }); @@ -447,7 +450,7 @@ describe('SuggestNewPointButton', () => { axios.post.mockRejectedValue(new Error('Network error')); mockGeolocationSuccess(); - globalThis.LOCATION_SCHEMA = SIMPLE_SCHEMA; + httpService.getLocationSchema.mockResolvedValue(SIMPLE_SCHEMA); renderWithProvider(); await openDialog(); @@ -474,7 +477,7 @@ describe('SuggestNewPointButton', () => { axios.post.mockRejectedValue({ response: { data: { message: backendMessage } } }); mockGeolocationSuccess(); - globalThis.LOCATION_SCHEMA = SIMPLE_SCHEMA; + httpService.getLocationSchema.mockResolvedValue(SIMPLE_SCHEMA); renderWithProvider(); await openDialog(); @@ -499,7 +502,7 @@ describe('SuggestNewPointButton', () => { }), ); mockGeolocationSuccess(); - globalThis.LOCATION_SCHEMA = SIMPLE_SCHEMA; + httpService.getLocationSchema.mockResolvedValue(SIMPLE_SCHEMA); renderWithProvider(); await openDialog(); @@ -526,7 +529,7 @@ describe('SuggestNewPointButton', () => { it('closes dialog and resets form on successful submission', async () => { axios.post.mockResolvedValue({ data: { message: 'Success' } }); mockGeolocationSuccess(); - globalThis.LOCATION_SCHEMA = SIMPLE_SCHEMA; + httpService.getLocationSchema.mockResolvedValue(SIMPLE_SCHEMA); renderWithProvider(); await openDialog(); diff --git a/frontend/tests/MarkerPopup/ReportProblemForm.test.jsx b/frontend/tests/MarkerPopup/ReportProblemForm.test.jsx index 72f44012..035b03e0 100644 --- a/frontend/tests/MarkerPopup/ReportProblemForm.test.jsx +++ b/frontend/tests/MarkerPopup/ReportProblemForm.test.jsx @@ -5,6 +5,15 @@ import ReportProblemForm from '../../src/components/MarkerPopup/ReportProblemFor jest.mock('axios'); const axios = require('axios'); +// The schema comes from a provider that fetches it; these tests exercise the form, so +// the hook is stubbed to keep them synchronous and focused. +jest.mock('../../src/components/Map/context/LocationSchemaContext', () => ({ + useLocationSchema: jest.fn(), +})); +const { useLocationSchema } = require('../../src/components/Map/context/LocationSchemaContext'); + +const mockSchema = (schema = {}) => useLocationSchema.mockReturnValue({ locationSchema: schema }); + axios.post.mockResolvedValue({ data: { success: true } }); const PLACE_ID = 'test-id'; @@ -45,6 +54,7 @@ beforeEach(() => { metaTag.setAttribute('name', 'csrf-token'); metaTag.setAttribute('content', CSRF_TOKEN); document.head.appendChild(metaTag); + mockSchema(); }); afterEach(() => { @@ -52,7 +62,6 @@ afterEach(() => { if (metaTag) { metaTag.remove(); } - delete globalThis.LOCATION_SCHEMA; }); describe('ReportProblemForm', () => { @@ -81,8 +90,8 @@ describe('ReportProblemForm', () => { expect(queryByText(/Submit/i)).toBeNull(); }); - it('renders dynamic issue types from LOCATION_SCHEMA', () => { - globalThis.LOCATION_SCHEMA = CUSTOM_ISSUE_TYPES; + it('renders dynamic issue types from the deployment schema', () => { + mockSchema(CUSTOM_ISSUE_TYPES); const { getByText, queryByText, select } = renderForm(); const optionTexts = Array.from(select.querySelectorAll('option')).map(o => o.textContent); @@ -97,7 +106,7 @@ describe('ReportProblemForm', () => { }); it('submits dynamic issue type value as description', () => { - globalThis.LOCATION_SCHEMA = CUSTOM_ISSUE_TYPES; + mockSchema(CUSTOM_ISSUE_TYPES); const { getByText, select } = renderForm(); fireEvent.change(select, { target: { value: 'under construction' } }); fireEvent.click(getByText(/Submit/i)); @@ -105,8 +114,8 @@ describe('ReportProblemForm', () => { return expectReportSubmitted('under construction'); }); - it('falls back to default options when LOCATION_SCHEMA is undefined', () => { - delete globalThis.LOCATION_SCHEMA; + it('falls back to default options when the schema has not loaded', () => { + mockSchema({}); const { getByText } = renderForm(); expect(getByText('this point is not here')).toBeTruthy(); @@ -117,7 +126,7 @@ describe('ReportProblemForm', () => { it('falls back to default options when reported_issue_types is empty', () => { // eslint-disable-next-line camelcase -- matches backend API schema property name - globalThis.LOCATION_SCHEMA = { reported_issue_types: [] }; + mockSchema({ reported_issue_types: [] }); const { getByText } = renderForm(); expect(getByText('this point is not here')).toBeTruthy(); diff --git a/goodmap/goodmap.py b/goodmap/goodmap.py index 113038e4..03bcd086 100644 --- a/goodmap/goodmap.py +++ b/goodmap/goodmap.py @@ -7,7 +7,6 @@ from typing import Any from flask import Blueprint, jsonify, redirect, render_template, session -from flask_babel import gettext from flask_wtf.csrf import CSRFError from platzky import platzky from platzky.config import languages_dict @@ -276,55 +275,23 @@ def handle_csrf_error(error): @goodmap.route("/map") def index(): - """Render main map interface with location schema. + """Render the main map interface. Registered at /map rather than / because platzky (>=2.0.0a8) reserves the root path for its own homepage dispatch (see db.get_home_page_path()). Deployments set site_content.home_page_path to "/map" so visiting / still renders this view, with no redirect. - Prepares and passes location schema including obligatory fields and - categories to the frontend for dynamic form generation. + The frontend reads what this deployment accepts from /api/location-schema + rather than from an inlined copy, so there is one source of truth for it. Returns: - Rendered map.html template with feature flags and location schema + Rendered map.html template with feature flags and the plugin manifest """ - # Prepare location schema for frontend dynamic forms - # Include full schema from Pydantic model for better type information - category_data = app.db.get_category_data() # type: ignore[attr-defined] - categories = category_data.get("categories", {}) - - # Get full JSON schema from Pydantic model - model_json_schema = location_model.model_json_schema() - properties = model_json_schema.get("properties", {}) - - # Filter out uuid and position from properties for frontend form - form_fields = { - name: spec for name, spec in properties.items() if name not in ("uuid", "position") - } - - issue_options_raw = app.db.get_issue_options() # type: ignore[attr-defined] - reported_issue_types = [{"value": t, "label": gettext(t)} for t in issue_options_raw] - - location_schema = { # TODO remove backward compatibility - deprecation - "obligatory_fields": app.extensions["goodmap"][ - "location_obligatory_fields" - ], # Backward compatibility - "categories": categories, # Backward compatibility - "fields": form_fields, - "reported_issue_types": reported_issue_types, - "photo": { - "allowed_extensions": sorted(photo_attachment_config.allowed_extensions or []), - "allowed_mime_types": sorted(photo_attachment_config.allowed_mime_types or []), - "max_size_bytes": photo_attachment_config.max_size, - }, - } - return render_template( "map.html", feature_flags=config.feature_flags, goodmap_frontend_lib_url=config.goodmap_frontend_lib_url, - location_schema=location_schema, plugin_manifest=plugin_manifest, ) diff --git a/goodmap/templates/map.html b/goodmap/templates/map.html index b165a99b..4c646620 100644 --- a/goodmap/templates/map.html +++ b/goodmap/templates/map.html @@ -122,7 +122,6 @@ window.FEATURE_FLAGS = {{ feature_flags | tojson }}; // Location schema for dynamic form building // Contains required fields and available categories for new location suggestions -globalThis.LOCATION_SCHEMA = {{ location_schema | tojson }}; window.PLUGIN_MANIFEST = {{ plugin_manifest | tojson }}; diff --git a/tests/unit_tests/test_goodmap.py b/tests/unit_tests/test_goodmap.py index 488fc01a..34774abb 100644 --- a/tests/unit_tests/test_goodmap.py +++ b/tests/unit_tests/test_goodmap.py @@ -107,8 +107,9 @@ 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_returns_location_schema(): - """Test that the /map route returns successfully with location_schema""" +def test_location_schema_endpoint_reports_configured_categories(): + """The schema the frontend builds its form from comes from /api/location-schema, + not from anything inlined into the map page.""" config = GoodmapConfig( APP_NAME="test_app", SECRET_KEY="test_secret", @@ -131,22 +132,19 @@ def test_map_route_returns_location_schema(): app.config["WTF_CSRF_ENABLED"] = False # NOSONAR client = app.test_client() - response = client.get("/map") + response = client.get("/api/location-schema") assert response.status_code == 200 - # Verify location_schema is present in the response - response_text = response.data.decode("utf-8") - assert "LOCATION_SCHEMA" in response_text - assert "obligatory_fields" in response_text - assert "categories" in response_text - assert "accessibility" in response_text - assert "amenities" in response_text + schema = response.json + assert schema is not None + assert "obligatory_fields" in schema + assert set(schema["categories"]) == {"accessibility", "amenities"} 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 - guards the `photo` key in location_schema that makes that possible. + guards the `photo` key in /api/location-schema that makes that possible. """ config = GoodmapConfig( APP_NAME="test_app", @@ -162,13 +160,14 @@ def test_map_route_includes_photo_constraints(): app.config["WTF_CSRF_ENABLED"] = False # NOSONAR client = app.test_client() - response = client.get("/map") + response = client.get("/api/location-schema") assert response.status_code == 200 - response_text = response.data.decode("utf-8") - assert '"max_size_bytes":5242880' in response_text - assert '"allowed_mime_types":["image/jpeg"]' in response_text - assert '"allowed_extensions":["jpeg","jpg"]' in response_text + assert response.json is not None + photo = response.json["photo"] + assert photo["max_size_bytes"] == 5242880 + assert photo["allowed_mime_types"] == ["image/jpeg"] + assert photo["allowed_extensions"] == ["jpeg", "jpg"] def _minimal_config() -> GoodmapConfig: @@ -264,17 +263,18 @@ def test_map_route_overrides_photo_constraints(): app.config["WTF_CSRF_ENABLED"] = False # NOSONAR client = app.test_client() - response = client.get("/map") + response = client.get("/api/location-schema") assert response.status_code == 200 - response_text = response.data.decode("utf-8") - assert '"max_size_bytes":8388608' in response_text - assert '"allowed_mime_types":["image/jpeg","image/png"]' in response_text - assert '"allowed_extensions":["jpeg","jpg","png"]' in response_text + assert response.json is not None + photo = response.json["photo"] + assert photo["max_size_bytes"] == 8388608 + assert photo["allowed_mime_types"] == ["image/jpeg", "image/png"] + assert photo["allowed_extensions"] == ["jpeg", "jpg", "png"] -def test_map_route_location_schema_with_lazy_loading(): - """Test that location_schema includes obligatory_fields when USE_LAZY_LOADING is enabled""" +def test_location_schema_endpoint_with_lazy_loading(): + """The schema includes obligatory_fields when USE_LAZY_LOADING is enabled.""" config = GoodmapConfig( APP_NAME="test_app", SECRET_KEY="test_secret", @@ -300,15 +300,20 @@ def test_map_route_location_schema_with_lazy_loading(): app.config["WTF_CSRF_ENABLED"] = False # NOSONAR client = app.test_client() - response = client.get("/map") + response = client.get("/api/location-schema") assert response.status_code == 200 - # Verify location_schema includes obligatory_fields - response_text = response.data.decode("utf-8") - assert "LOCATION_SCHEMA" in response_text - assert "name" in response_text - assert "position" in response_text - assert "test_category" in response_text + schema = response.json + assert schema is not None + assert [f[0] for f in schema["obligatory_fields"]] == [ + "name", + "position", + "test_category", + ] + # position is client-supplied, so it must be offered as a field; uuid is not + assert "position" in schema["fields"] + assert "uuid" not in schema["fields"] + assert "test_category" in schema["categories"] def _plugin_ep(name: str, plugin_dir: str | None, base: type = MapOverlayPluginBase): From 26af58e0f88d714f6dcf137ce55a6e8ab434d345 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Krzysztof=20Ko=C5=82odzi=C5=84ski?= Date: Wed, 19 Aug 2026 14:11:16 +0200 Subject: [PATCH 02/18] fix: add the LocationSchemaContext the previous commit imports The provider file was still untracked when "chore: refactor location schema" was committed, so it was left behind while six files that import it were not. --- .../Map/context/LocationSchemaContext.jsx | 80 +++++++++++++++++++ 1 file changed, 80 insertions(+) create mode 100644 frontend/src/components/Map/context/LocationSchemaContext.jsx diff --git a/frontend/src/components/Map/context/LocationSchemaContext.jsx b/frontend/src/components/Map/context/LocationSchemaContext.jsx new file mode 100644 index 00000000..0bde79ee --- /dev/null +++ b/frontend/src/components/Map/context/LocationSchemaContext.jsx @@ -0,0 +1,80 @@ +import React, { createContext, useCallback, useContext, useEffect, useMemo, useState } from 'react'; +import { httpService } from '../../../services/http/httpService'; + +/** + * React Context holding the schema this deployment accepts for a new point. + * + * The accepted fields, their allowed values, the reportable issue types and the photo + * limits are all configured per deployment, so they are fetched from the running + * instance once and shared, rather than assumed or inlined into the page. + */ +const LocationSchemaContext = createContext(); +LocationSchemaContext.displayName = 'LocationSchemaContext'; + +// Used until the fetch resolves, and if it fails: an empty schema renders an empty +// form rather than crashing on a missing key. +const EMPTY_SCHEMA = { + obligatory_fields: [], + categories: {}, + fields: {}, + reported_issue_types: [], + photo: {}, +}; + +/** + * Provider that fetches the location schema once and shares it with every child. + * + * @param {Object} props - Component props + * @param {React.ReactNode} props.children - Components that need the schema + * @returns {React.ReactElement} Context provider with the location schema + */ +export const LocationSchemaProvider = ({ children }) => { + const [locationSchema, setLocationSchema] = useState(EMPTY_SCHEMA); + const [isLoading, setIsLoading] = useState(true); + const [hasError, setHasError] = useState(false); + + const fetchLocationSchema = useCallback(async () => { + setIsLoading(true); + setHasError(false); + try { + // Merged over the empty schema so every key is present whatever the + // instance returns, and no consumer has to guard each one. + setLocationSchema({ ...EMPTY_SCHEMA, ...(await httpService.getLocationSchema()) }); + } catch (error) { + console.error('Failed to load location schema:', error); + setLocationSchema(EMPTY_SCHEMA); + setHasError(true); + } finally { + setIsLoading(false); + } + }, []); + + useEffect(() => { + fetchLocationSchema(); + }, [fetchLocationSchema]); + + const value = useMemo( + () => ({ locationSchema, isLoading, hasError }), + [locationSchema, isLoading, hasError], + ); + + return ( + {children} + ); +}; + +/** + * Access the deployment's location schema. + * + * Must be used within a LocationSchemaProvider. + * + * @returns {{locationSchema: Object, isLoading: boolean, hasError: boolean}} Schema state + * @throws {Error} If used outside of LocationSchemaProvider + */ +export const useLocationSchema = () => { + const context = useContext(LocationSchemaContext); + if (context === undefined) { + throw new Error('useLocationSchema must be used within a LocationSchemaProvider'); + } + return context; +}; From e6d6d9ce05488d0c7ae9de1e4ef1d964e6bc6abe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Krzysztof=20Ko=C5=82odzi=C5=84ski?= Date: Wed, 19 Aug 2026 15:14:06 +0200 Subject: [PATCH 03/18] comment added --- goodmap/templates/map.html | 2 -- 1 file changed, 2 deletions(-) diff --git a/goodmap/templates/map.html b/goodmap/templates/map.html index 4c646620..d1b1eacb 100644 --- a/goodmap/templates/map.html +++ b/goodmap/templates/map.html @@ -120,8 +120,6 @@ 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 }}; -// Location schema for dynamic form building -// Contains required fields and available categories for new location suggestions window.PLUGIN_MANIFEST = {{ plugin_manifest | tojson }}; From d5116a79aee04135d2c2ee3b2b22a714a37c1e94 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Krzysztof=20Ko=C5=82odzi=C5=84ski?= Date: Wed, 19 Aug 2026 16:22:25 +0200 Subject: [PATCH 04/18] fixes after review --- .../Categories/CategoriesContext.jsx | 97 ------------ .../components/FiltersForm/FiltersForm.jsx | 22 +-- frontend/src/components/Map/Map.jsx | 18 +-- .../Map/components/AccessibilityTable.jsx | 8 +- .../src/components/Map/components/Markers.jsx | 8 +- .../Map/components/SuggestNewPointDialog.jsx | 62 +++++--- .../Map/context/LocationSchemaContext.jsx | 80 ---------- .../MarkerPopup/ReportProblemForm.jsx | 6 +- .../src/context/DeploymentDataContext.jsx | 144 ++++++++++++++++++ frontend/src/context/FiltersContext.jsx | 71 +++++++++ ...est.jsx => DeploymentDataContext.test.jsx} | 17 +-- frontend/tests/FiltersForm.test.jsx | 26 ++-- frontend/tests/Map/MapComponent.test.jsx | 22 ++- .../components/AccessibilityTable.test.jsx | 6 +- .../tests/Map/components/Markers.test.jsx | 9 +- .../Map/components/SuggestNewPoint.test.jsx | 37 ++++- .../MarkerPopup/ReportProblemForm.test.jsx | 8 +- frontend/tests/utils/providers.jsx | 23 +++ 18 files changed, 386 insertions(+), 278 deletions(-) delete mode 100644 frontend/src/components/Categories/CategoriesContext.jsx delete mode 100644 frontend/src/components/Map/context/LocationSchemaContext.jsx create mode 100644 frontend/src/context/DeploymentDataContext.jsx create mode 100644 frontend/src/context/FiltersContext.jsx rename frontend/tests/{CategoriesContext.test.jsx => DeploymentDataContext.test.jsx} (74%) create mode 100644 frontend/tests/utils/providers.jsx diff --git a/frontend/src/components/Categories/CategoriesContext.jsx b/frontend/src/components/Categories/CategoriesContext.jsx deleted file mode 100644 index 624bbce4..00000000 --- a/frontend/src/components/Categories/CategoriesContext.jsx +++ /dev/null @@ -1,97 +0,0 @@ -import React, { useState, useContext, createContext, useMemo, useEffect, useCallback } from 'react'; -import PropTypes from 'prop-types'; -import httpService from '../../services/http/httpService'; - -/** - * React Context for managing categories state across the application. - * Provides categories data and setter function to all child components. - */ -const CategoriesContext = createContext(); -CategoriesContext.displayName = 'CategoriesContext'; - -/** - * Provider component that wraps the application to provide categories context. - * Fetches the category definitions once and shares them, alongside the currently - * selected filter values, with every child component. - * - * @param {Object} props - Component props - * @param {React.ReactNode} props.children - Child components that will have access to categories context - * @returns {React.ReactElement} Context provider with categories state - */ -export const CategoriesProvider = ({ children }) => { - const [categories, setCategories] = useState({}); - const [isInitialized, setIsInitialized] = useState(false); - const [categoriesData, setCategoriesData] = useState([]); - const [isLoading, setIsLoading] = useState(true); - const [hasError, setHasError] = useState(false); - - const fetchCategories = useCallback(async () => { - setIsLoading(true); - setHasError(false); - try { - const { categories: fetchedCategories, defaultChecked } = - await httpService.getCategoriesData(); - setCategoriesData(fetchedCategories); - if (Object.keys(defaultChecked).length > 0) { - setCategories(defaultChecked); - } - setIsInitialized(true); - } catch (error) { - console.error('Failed to load categories:', error); - // Establish an explicit fallback (no filters) instead of silently - // signaling initialization with unknown/missing category data. - setCategoriesData([]); - setHasError(true); - } finally { - setIsLoading(false); - } - }, []); - - useEffect(() => { - fetchCategories(); - }, [fetchCategories]); - - const value = useMemo( - () => ({ - categories, - setCategories, - isInitialized, - setIsInitialized, - categoriesData, - isLoading, - hasError, - refetchCategories: fetchCategories, - }), - [categories, isInitialized, categoriesData, isLoading, hasError, fetchCategories], - ); - - return {children}; -}; - -CategoriesProvider.propTypes = { - children: PropTypes.node.isRequired, -}; - -/** - * Custom hook to access categories context. - * Must be used within a CategoriesProvider component. - * - * @throws {Error} If used outside of CategoriesProvider - * @returns {Object} Object containing categories map and setCategories function - * @returns {Object} return.categories - Currently selected filter values keyed by category - * @returns {Function} return.setCategories - Function to update categories - * @returns {boolean} return.isInitialized - True once initial filter state (including - * default-checked options) has been loaded, so consumers can wait before fetching - * @returns {Function} return.setIsInitialized - Marks the initial filter state as loaded - * @returns {Array} return.categoriesData - Category definitions fetched from the backend - * @returns {boolean} return.isLoading - True while the category definitions are in flight - * @returns {boolean} return.hasError - True if fetching the category definitions failed - * @returns {Function} return.refetchCategories - Retries the category definitions fetch - */ -export const useCategories = () => { - const context = useContext(CategoriesContext); - if (!context) { - throw new Error('useCategories must be used within a CategoriesProvider'); - } - return context; -}; diff --git a/frontend/src/components/FiltersForm/FiltersForm.jsx b/frontend/src/components/FiltersForm/FiltersForm.jsx index 3b9ab257..c14c5615 100644 --- a/frontend/src/components/FiltersForm/FiltersForm.jsx +++ b/frontend/src/components/FiltersForm/FiltersForm.jsx @@ -2,7 +2,8 @@ import React from 'react'; import styled, { keyframes } from 'styled-components'; import { useTranslation } from 'react-i18next'; import { Tooltip } from '@mui/material'; -import { useCategories } from '../Categories/CategoriesContext'; +import { useDeploymentData } from '../../context/DeploymentDataContext'; +import { useFilters } from '../../context/FiltersContext'; import FiltersTooltip from './FiltersTooltip'; const shimmer = keyframes` @@ -242,19 +243,18 @@ const LoadingSkeleton = () => ( const FiltersForm = () => { const { t } = useTranslation(); const { - categories: selectedFilters, - setCategories, categoriesData, - isLoading, - hasError, + categoriesLoading, + categoriesError, refetchCategories, - } = useCategories(); + } = useDeploymentData(); + const { selectedFilters, setSelectedFilters } = useFilters(); const handleCheckboxChange = event => { const { value, checked } = event.target; const { category } = event.target.dataset; - setCategories(prevSelectedFilters => { + setSelectedFilters(prevSelectedFilters => { const newSelectedFilters = { ...prevSelectedFilters }; if (checked) { @@ -275,14 +275,14 @@ const FiltersForm = () => { const { value } = event.target; const { category } = event.target.dataset; - setCategories(prevSelectedFilters => ({ + setSelectedFilters(prevSelectedFilters => ({ ...prevSelectedFilters, [category]: [value], })); }; const handleClearFilters = () => { - setCategories({}); + setSelectedFilters({}); }; const renderModeBadge = mode => { @@ -423,7 +423,7 @@ const FiltersForm = () => { ); } - if (isLoading) { + if (categoriesLoading) { return (
@@ -431,7 +431,7 @@ const FiltersForm = () => { ); } - if (hasError) { + if (categoriesError) { return ( {t('loadFiltersError')} diff --git a/frontend/src/components/Map/Map.jsx b/frontend/src/components/Map/Map.jsx index 0e76ce4d..34b353e2 100644 --- a/frontend/src/components/Map/Map.jsx +++ b/frontend/src/components/Map/Map.jsx @@ -3,16 +3,16 @@ import React from 'react'; import { createPortal } from 'react-dom'; import FiltersForm from '../FiltersForm/FiltersForm'; import MapComponent from './MapComponent'; -import { CategoriesProvider } from '../Categories/CategoriesContext'; -import { LocationSchemaProvider } from './context/LocationSchemaContext'; +import { DeploymentDataProvider } from '../../context/DeploymentDataContext'; +import { FiltersProvider } from '../../context/FiltersContext'; import AppToaster from '../common/AppToaster'; /** * Wrapper component that renders the map and filters form into their respective DOM placeholders. * Uses React portals to render components into pre-existing DOM elements outside the React tree. - * Wraps both components with CategoriesProvider for shared filter state management, and - * with LocationSchemaProvider so the suggest and report forms know what this deployment - * accepts without each fetching it. + * Wraps both components with DeploymentDataProvider, which fetches this deployment's + * fixed data (category definitions and the new-point schema) once for every consumer, + * and with FiltersProvider, which owns the one thing that changes as the app runs. * * @returns {React.ReactElement|null} Portals for FiltersForm and MapComponent, or null if placeholders not found */ @@ -26,13 +26,13 @@ const MapWrap = () => { } return ( - - + + {createPortal(, filtersPlaceholder)} {createPortal(, mapPlaceholder)} - - + + ); }; diff --git a/frontend/src/components/Map/components/AccessibilityTable.jsx b/frontend/src/components/Map/components/AccessibilityTable.jsx index 3961669f..062e02e1 100644 --- a/frontend/src/components/Map/components/AccessibilityTable.jsx +++ b/frontend/src/components/Map/components/AccessibilityTable.jsx @@ -12,7 +12,7 @@ import { IconButton } from '@mui/material'; import PropTypes from 'prop-types'; import httpService from '../../../services/http/httpService'; import FieldRenderer from '../../MarkerPopup/FieldRenderer'; -import { useCategories } from '../../Categories/CategoriesContext'; +import { useFilters } from '../../../context/FiltersContext'; /** * Accessibility table component that displays location data in a tabular format. @@ -28,7 +28,7 @@ import { useCategories } from '../../Categories/CategoriesContext'; * @returns {React.ReactElement} Table container with location data and back button */ const AccessibilityTable = ({ userPosition, setIsAccessibilityTableOpen }) => { - const { categories } = useCategories(); + const { selectedFilters } = useFilters(); const { t } = useTranslation(); const [data, setData] = useState(null); @@ -37,11 +37,11 @@ const AccessibilityTable = ({ userPosition, setIsAccessibilityTableOpen }) => { useEffect(() => { httpService - .getLocationsData(userPosition.lat, userPosition.lng, categories) + .getLocationsData(userPosition.lat, userPosition.lng, selectedFilters) .then(places => { setData(places); }); - }, [categories, userPosition]); + }, [selectedFilters, userPosition]); useEffect(() => { if (!data) { diff --git a/frontend/src/components/Map/components/Markers.jsx b/frontend/src/components/Map/components/Markers.jsx index 038df6e5..a3dd3216 100644 --- a/frontend/src/components/Map/components/Markers.jsx +++ b/frontend/src/components/Map/components/Markers.jsx @@ -4,7 +4,7 @@ import { useMap } from 'react-leaflet'; import MarkerClusterGroup from 'react-leaflet-cluster'; import httpService from '../../../services/http/httpService'; import MarkerPopup from '../../MarkerPopup/MarkerPopup'; -import { useCategories } from '../../Categories/CategoriesContext'; +import { useFilters } from '../../../context/FiltersContext'; import ClusterMarker from '../../MarkerPopup/ClusterMarker'; /** @@ -44,7 +44,7 @@ const getMarkers = locations => { * @returns {React.ReactElement|Array} MarkerClusterGroup containing location markers, or empty array while loading */ const Markers = ({ onLoadingChange = null }) => { - const { categories, isInitialized } = useCategories(); + const { selectedFilters, isInitialized } = useFilters(); const [markers, setMarkers] = useState([]); const [areMarkersLoaded, setAreMarkersLoaded] = useState(false); const map = useMap(); @@ -59,7 +59,7 @@ const Markers = ({ onLoadingChange = null }) => { const fetchMarkers = async () => { let locations; try { - locations = await httpService.getLocations(categories); + locations = await httpService.getLocations(selectedFilters); } catch (error) { console.error('Failed to load locations:', error); setMarkers([]); @@ -100,7 +100,7 @@ const Markers = ({ onLoadingChange = null }) => { return () => { setMarkers([]); }; - }, [categories, isInitialized]); + }, [selectedFilters, isInitialized]); useEffect(() => { const mapContainer = map.getContainer(); diff --git a/frontend/src/components/Map/components/SuggestNewPointDialog.jsx b/frontend/src/components/Map/components/SuggestNewPointDialog.jsx index 3226cd2a..c2c4e415 100644 --- a/frontend/src/components/Map/components/SuggestNewPointDialog.jsx +++ b/frontend/src/components/Map/components/SuggestNewPointDialog.jsx @@ -26,9 +26,8 @@ import axios from 'axios'; import { useTranslation } from 'react-i18next'; import imageCompression from 'browser-image-compression'; import getCsrfToken from '../../../utils/csrf'; -import { useLocationSchema } from '../context/LocationSchemaContext'; import { useLocation } from '../context/LocationContext'; -import { useCategories } from '../../Categories/CategoriesContext'; +import { useDeploymentData } from '../../../context/DeploymentDataContext'; import toast from '../../../utils/toast'; // Map a category's options to a { key: translation } object. @@ -47,16 +46,15 @@ const mapCategoryOptions = categoryOptions => { }; // Build { fieldNames, options } translation maps from the fetched category definitions. +// Every category key is registered, even when it has no options, so a category with an +// empty option list stays distinguishable from a field that is not a category at all. const buildCategoryTranslations = categoriesData => { const fieldNames = {}; const options = {}; categoriesData.forEach(({ categoryKey, categoryName, options: categoryOptions }) => { fieldNames[categoryKey] = categoryName; - - if (categoryOptions?.length) { - options[categoryKey] = mapCategoryOptions(categoryOptions); - } + options[categoryKey] = mapCategoryOptions(categoryOptions ?? []); }); return { fieldNames, options }; @@ -108,17 +106,16 @@ const useScrollToTop = trigger => { }; /** - * Dialog form for suggesting a new map point. Fields are generated dynamically from the - * deployment's location schema and include the user's position, an optional photo, and - * every obligatory location attribute defined by the backend. + * The suggestion form itself. Mounted only once the location schema is known, so the + * fields it generates can be built in one pass from the deployment's obligatory fields. * - * @param {{open: boolean, onClose: () => void}} props + * @param {{open: boolean, onClose: () => void, locationSchema: Object}} props * @returns {React.ReactElement} Dialog with the new point suggestion form */ -const SuggestNewPointDialog = ({ open, onClose }) => { +const SuggestNewPointForm = ({ open, onClose, locationSchema }) => { const { t } = useTranslation(); const { userPosition, requestLocationWithFeedback } = useLocation(); - const { categoriesData } = useCategories(); + const { categoriesData } = useDeploymentData(); const [photo, setPhoto] = useState(null); const [photoURL, setPhotoURL] = useState(null); // Shown inline in the dialog: a toast here can end up behind it. @@ -141,7 +138,6 @@ const SuggestNewPointDialog = ({ open, onClose }) => { } }, [open]); - const { locationSchema } = useLocationSchema(); const { allowed_extensions: allowedPhotoExtensions = [], allowed_mime_types: allowedPhotoMimeTypes = [], @@ -171,13 +167,6 @@ const SuggestNewPointDialog = ({ open, onClose }) => { const [formFields, setFormFields] = useState(initializeFormFields); - // The schema is fetched, so it is empty on the first render and the initial state - // above has nothing to build from. Rebuild the fields once it arrives. - useEffect(() => { - setFormFields(initializeFormFields()); - // eslint-disable-next-line react-hooks/exhaustive-deps - }, [locationSchema]); - const handleLocateMe = () => { requestLocationWithFeedback(); }; @@ -302,8 +291,11 @@ const SuggestNewPointDialog = ({ open, onClose }) => { // Render form field based on field type and whether it's a category const renderFormField = (fieldName, fieldType) => { - const isCategory = fieldName in locationSchema.categories; - const categoryOptions = isCategory ? locationSchema.categories[fieldName] : []; + // Values and labels both come from the category definitions, so an option can + // never render with a label this component has no entry for. + const optionLabels = categoryTranslations.options[fieldName]; + const isCategory = Boolean(optionLabels); + const categoryOptions = isCategory ? Object.keys(optionLabels) : []; const fieldLabel = getFieldLabel(fieldName); if (fieldType === 'list' && isCategory) { @@ -444,6 +436,32 @@ const SuggestNewPointDialog = ({ open, onClose }) => { ); }; +SuggestNewPointForm.propTypes = { + open: PropTypes.bool.isRequired, + onClose: PropTypes.func.isRequired, + locationSchema: PropTypes.object.isRequired, +}; + +/** + * Dialog for suggesting a new map point. + * + * The form's fields are generated from the deployment's location schema, so there is + * nothing to render until that has arrived - holding the form back until then is what + * lets it build its fields once, instead of rebuilding them when the schema lands. + * + * @param {{open: boolean, onClose: () => void}} props + * @returns {React.ReactElement|null} The suggestion form, or null while the schema loads + */ +const SuggestNewPointDialog = ({ open, onClose }) => { + const { locationSchema } = useDeploymentData(); + + if (!locationSchema) { + return null; + } + + return ; +}; + SuggestNewPointDialog.propTypes = { open: PropTypes.bool.isRequired, onClose: PropTypes.func.isRequired, diff --git a/frontend/src/components/Map/context/LocationSchemaContext.jsx b/frontend/src/components/Map/context/LocationSchemaContext.jsx deleted file mode 100644 index 0bde79ee..00000000 --- a/frontend/src/components/Map/context/LocationSchemaContext.jsx +++ /dev/null @@ -1,80 +0,0 @@ -import React, { createContext, useCallback, useContext, useEffect, useMemo, useState } from 'react'; -import { httpService } from '../../../services/http/httpService'; - -/** - * React Context holding the schema this deployment accepts for a new point. - * - * The accepted fields, their allowed values, the reportable issue types and the photo - * limits are all configured per deployment, so they are fetched from the running - * instance once and shared, rather than assumed or inlined into the page. - */ -const LocationSchemaContext = createContext(); -LocationSchemaContext.displayName = 'LocationSchemaContext'; - -// Used until the fetch resolves, and if it fails: an empty schema renders an empty -// form rather than crashing on a missing key. -const EMPTY_SCHEMA = { - obligatory_fields: [], - categories: {}, - fields: {}, - reported_issue_types: [], - photo: {}, -}; - -/** - * Provider that fetches the location schema once and shares it with every child. - * - * @param {Object} props - Component props - * @param {React.ReactNode} props.children - Components that need the schema - * @returns {React.ReactElement} Context provider with the location schema - */ -export const LocationSchemaProvider = ({ children }) => { - const [locationSchema, setLocationSchema] = useState(EMPTY_SCHEMA); - const [isLoading, setIsLoading] = useState(true); - const [hasError, setHasError] = useState(false); - - const fetchLocationSchema = useCallback(async () => { - setIsLoading(true); - setHasError(false); - try { - // Merged over the empty schema so every key is present whatever the - // instance returns, and no consumer has to guard each one. - setLocationSchema({ ...EMPTY_SCHEMA, ...(await httpService.getLocationSchema()) }); - } catch (error) { - console.error('Failed to load location schema:', error); - setLocationSchema(EMPTY_SCHEMA); - setHasError(true); - } finally { - setIsLoading(false); - } - }, []); - - useEffect(() => { - fetchLocationSchema(); - }, [fetchLocationSchema]); - - const value = useMemo( - () => ({ locationSchema, isLoading, hasError }), - [locationSchema, isLoading, hasError], - ); - - return ( - {children} - ); -}; - -/** - * Access the deployment's location schema. - * - * Must be used within a LocationSchemaProvider. - * - * @returns {{locationSchema: Object, isLoading: boolean, hasError: boolean}} Schema state - * @throws {Error} If used outside of LocationSchemaProvider - */ -export const useLocationSchema = () => { - const context = useContext(LocationSchemaContext); - if (context === undefined) { - throw new Error('useLocationSchema must be used within a LocationSchemaProvider'); - } - return context; -}; diff --git a/frontend/src/components/MarkerPopup/ReportProblemForm.jsx b/frontend/src/components/MarkerPopup/ReportProblemForm.jsx index ff323b89..e8bf4545 100644 --- a/frontend/src/components/MarkerPopup/ReportProblemForm.jsx +++ b/frontend/src/components/MarkerPopup/ReportProblemForm.jsx @@ -4,7 +4,7 @@ import styled from 'styled-components'; import PropTypes from 'prop-types'; import { useTranslation } from 'react-i18next'; import getCsrfToken from '../../utils/csrf'; -import { useLocationSchema } from '../Map/context/LocationSchemaContext'; +import { useDeploymentData } from '../../context/DeploymentDataContext'; /** * Styled form component with flexbox column layout. @@ -148,13 +148,13 @@ const getIssueTypeOptions = (t, dynamicTypes) => { const ReportProblemForm = ({ placeId }) => { const { t } = useTranslation(); - const { locationSchema } = useLocationSchema(); + const { locationSchema } = useDeploymentData(); const [problem, setProblem] = useState(''); const [problemType, setProblemType] = useState(''); const [isSubmitted, setIsSubmitted] = useState(false); const [responseMessage, setResponseMessage] = useState(''); - const issueTypeOptions = getIssueTypeOptions(t, locationSchema.reported_issue_types); + const issueTypeOptions = getIssueTypeOptions(t, locationSchema?.reported_issue_types); const handleSubmit = async event => { event.preventDefault(); diff --git a/frontend/src/context/DeploymentDataContext.jsx b/frontend/src/context/DeploymentDataContext.jsx new file mode 100644 index 00000000..cd0199cb --- /dev/null +++ b/frontend/src/context/DeploymentDataContext.jsx @@ -0,0 +1,144 @@ +import React, { createContext, useCallback, useContext, useEffect, useMemo, useState } from 'react'; +import PropTypes from 'prop-types'; +import httpService from '../services/http/httpService'; + +/** + * React Context for everything this deployment is configured with: the category + * definitions and the schema a new point has to satisfy. + * + * None of it changes while the app runs - the backend reads it from config and from a + * map_config document nothing in the app ever writes - so it is fetched once here and + * treated as constant afterwards. The one thing that does change, the user's own filter + * selections, lives in FiltersContext instead, so toggling a filter cannot re-render + * the consumers of this data. + */ +const DeploymentDataContext = createContext(); +DeploymentDataContext.displayName = 'DeploymentDataContext'; + +// The shape the schema is merged onto, so every key is present whatever the instance +// returns and no consumer has to guard each one. This is a floor for a schema that has +// arrived - "not arrived yet" is null, and consumers wait for that rather than render +// against an empty skeleton. +/* eslint-disable camelcase -- these are the API's own field names */ +const EMPTY_SCHEMA = { + obligatory_fields: [], + categories: {}, + fields: {}, + reported_issue_types: [], + photo: {}, +}; +/* eslint-enable camelcase */ + +/** + * Provider that fetches this deployment's fixed data once and shares it with every child. + * + * @param {Object} props - Component props + * @param {React.ReactNode} props.children - Components that need the deployment data + * @return {React.ReactElement} Context provider with the deployment data + */ +export const DeploymentDataProvider = ({ children }) => { + const [categoriesData, setCategoriesData] = useState([]); + const [defaultChecked, setDefaultChecked] = useState({}); + const [categoriesLoading, setCategoriesLoading] = useState(true); + const [categoriesError, setCategoriesError] = useState(false); + + // Held as null rather than an empty skeleton so consumers can tell "not here yet" + // from "here and empty", and wait rather than build a form out of nothing. + const [locationSchema, setLocationSchema] = useState(null); + const [schemaError, setSchemaError] = useState(false); + + const fetchCategories = useCallback(async () => { + setCategoriesLoading(true); + setCategoriesError(false); + try { + const { categories, defaultChecked: fetchedDefaults } = + await httpService.getCategoriesData(); + setCategoriesData(categories); + setDefaultChecked(fetchedDefaults); + } catch (error) { + console.error('Failed to load categories:', error); + // Establish an explicit fallback (no filters) instead of silently + // signaling initialization with unknown/missing category data. + setCategoriesData([]); + setCategoriesError(true); + } finally { + setCategoriesLoading(false); + } + }, []); + + const fetchLocationSchema = useCallback(async () => { + setSchemaError(false); + try { + setLocationSchema({ ...EMPTY_SCHEMA, ...(await httpService.getLocationSchema()) }); + } catch (error) { + console.error('Failed to load location schema:', error); + // Settle on the empty schema rather than leaving it null: null means "still + // loading" and holds the suggest form back, which would turn a failed fetch + // into a button that silently does nothing. + setLocationSchema(EMPTY_SCHEMA); + setSchemaError(true); + } + }, []); + + // Tracked separately so one failure does not deny consumers the other: the filters + // panel is still usable without the schema, and the forms without the categories. + useEffect(() => { + fetchCategories(); + }, [fetchCategories]); + + useEffect(() => { + fetchLocationSchema(); + }, [fetchLocationSchema]); + + const value = useMemo( + () => ({ + categoriesData, + defaultChecked, + categoriesLoading, + categoriesError, + refetchCategories: fetchCategories, + locationSchema, + schemaError, + }), + [ + categoriesData, + defaultChecked, + categoriesLoading, + categoriesError, + fetchCategories, + locationSchema, + schemaError, + ], + ); + + return ( + {children} + ); +}; + +DeploymentDataProvider.propTypes = { + children: PropTypes.node.isRequired, +}; + +/** + * Access this deployment's fixed configuration. + * + * Must be used within a DeploymentDataProvider. + * + * - categoriesData: category definitions fetched from the backend + * - defaultChecked: options pre-selected by the deployment, keyed by category + * - categoriesLoading / categoriesError: state of the category definitions fetch + * - refetchCategories: retries that fetch + * - locationSchema: schema for a new point, null until it arrives + * - schemaError: true if fetching the location schema failed + * + * @throws {Error} If used outside of DeploymentDataProvider + * @return {Object} The deployment data described above + */ +export const useDeploymentData = () => { + const context = useContext(DeploymentDataContext); + if (!context) { + throw new Error('useDeploymentData must be used within a DeploymentDataProvider'); + } + return context; +}; diff --git a/frontend/src/context/FiltersContext.jsx b/frontend/src/context/FiltersContext.jsx new file mode 100644 index 00000000..47abb7dc --- /dev/null +++ b/frontend/src/context/FiltersContext.jsx @@ -0,0 +1,71 @@ +import React, { createContext, useContext, useEffect, useMemo, useState } from 'react'; +import PropTypes from 'prop-types'; +import { useDeploymentData } from './DeploymentDataContext'; + +/** + * React Context for the filter values the user currently has selected. + * + * This is the only map state that changes while the app runs, which is why it is kept + * apart from the deployment's fixed data in DeploymentDataContext: a filter toggle then + * re-renders the map and the filters panel, and nothing else. + */ +const FiltersContext = createContext(); +FiltersContext.displayName = 'FiltersContext'; + +/** + * Provider that owns the selected filters and seeds them from the deployment's defaults. + * + * @param {Object} props - Component props + * @param {React.ReactNode} props.children - Components that read or set the filters + * @return {React.ReactElement} Context provider with the selected filter state + */ +export const FiltersProvider = ({ children }) => { + const { defaultChecked, categoriesLoading, categoriesError } = useDeploymentData(); + const [selectedFilters, setSelectedFilters] = useState({}); + const [isInitialized, setIsInitialized] = useState(false); + + // The deployment's default-checked options are part of the initial filter state, so + // consumers must not fetch against an empty selection before those are known. + // Initialization stays false while they are in flight, and stays false for good if + // they could not be loaded at all - an unfiltered fetch is not a safe stand-in. + useEffect(() => { + if (categoriesLoading || categoriesError) { + return; + } + if (Object.keys(defaultChecked).length > 0) { + setSelectedFilters(defaultChecked); + } + setIsInitialized(true); + }, [defaultChecked, categoriesLoading, categoriesError]); + + const value = useMemo( + () => ({ selectedFilters, setSelectedFilters, isInitialized }), + [selectedFilters, isInitialized], + ); + + return {children}; +}; + +FiltersProvider.propTypes = { + children: PropTypes.node.isRequired, +}; + +/** + * Access the currently selected filters. + * + * Must be used within a FiltersProvider. + * + * - selectedFilters: selected filter values keyed by category + * - setSelectedFilters: updates the selected filters + * - isInitialized: true once the initial selection is known + * + * @throws {Error} If used outside of FiltersProvider + * @return {Object} The filter state described above + */ +export const useFilters = () => { + const context = useContext(FiltersContext); + if (!context) { + throw new Error('useFilters must be used within a FiltersProvider'); + } + return context; +}; diff --git a/frontend/tests/CategoriesContext.test.jsx b/frontend/tests/DeploymentDataContext.test.jsx similarity index 74% rename from frontend/tests/CategoriesContext.test.jsx rename to frontend/tests/DeploymentDataContext.test.jsx index 986bbef1..92e025c6 100644 --- a/frontend/tests/CategoriesContext.test.jsx +++ b/frontend/tests/DeploymentDataContext.test.jsx @@ -3,9 +3,8 @@ import { render, waitFor } from '@testing-library/react'; import '@testing-library/jest-dom'; import FiltersForm from '../src/components/FiltersForm/FiltersForm'; import SuggestNewPointButton from '../src/components/Map/components/SuggestNewPointButton'; -import { CategoriesProvider } from '../src/components/Categories/CategoriesContext'; import { LocationProvider } from '../src/components/Map/context/LocationContext'; -import { LocationSchemaProvider } from '../src/components/Map/context/LocationSchemaContext'; +import AppProviders from './utils/providers'; import httpService from '../src/services/http/httpService'; jest.mock('axios'); @@ -30,14 +29,12 @@ test('categories are fetched once for all consumers', async () => { httpService.getLocationSchema.mockResolvedValue({}); render( - - - - - - - - , + + + + + + , ); await waitFor(() => expect(httpService.getCategoriesData).toHaveBeenCalled()); diff --git a/frontend/tests/FiltersForm.test.jsx b/frontend/tests/FiltersForm.test.jsx index 1a896c41..fa32d104 100644 --- a/frontend/tests/FiltersForm.test.jsx +++ b/frontend/tests/FiltersForm.test.jsx @@ -2,7 +2,7 @@ import React from 'react'; import '@testing-library/jest-dom'; import { fireEvent, render, waitFor, within } from '@testing-library/react'; import FiltersForm from '../src/components/FiltersForm/FiltersForm'; -import { CategoriesProvider } from '../src/components/Categories/CategoriesContext'; +import AppProviders from './utils/providers'; import httpService from '../src/services/http/httpService'; jest.mock('../src/services/http/httpService'); @@ -32,9 +32,9 @@ describe('Creates good filter_form box', () => { json: jest.fn().mockResolvedValue(categories), }); render( - + - , + , ); await waitFor(() => expect(document.querySelector('#filter-label-types')).not.toBeNull()); }); @@ -99,9 +99,9 @@ describe('Pre-checks options configured as default-checked', () => { defaultChecked: { types: ['shoes'] }, }); render( - + - , + , ); await waitFor(() => expect(document.querySelector('#shoes')).not.toBeNull()); }); @@ -143,9 +143,9 @@ describe('Renders exclusive (single-select) categories as radio buttons', () => defaultChecked: {}, }); render( - + - , + , ); await waitFor(() => expect(document.querySelector('#free')).not.toBeNull()); }); @@ -204,9 +204,9 @@ describe('Groups boolean categories into a shared "Others" section', () => { defaultChecked: {}, }); render( - + - , + , ); await waitFor(() => expect(document.querySelector('#is_free')).not.toBeNull()); }); @@ -265,9 +265,9 @@ describe('Renders threshold categories as radio buttons too', () => { defaultChecked: {}, }); render( - + - , + , ); await waitFor(() => expect(document.getElementById('10')).not.toBeNull()); }); @@ -311,9 +311,9 @@ describe('Distinguishes "and" categories with a visible hint, but keeps checkbox defaultChecked: {}, }); render( - + - , + , ); await waitFor(() => expect(document.querySelector('#lighting')).not.toBeNull()); }); diff --git a/frontend/tests/Map/MapComponent.test.jsx b/frontend/tests/Map/MapComponent.test.jsx index 2f307e68..0b391eac 100644 --- a/frontend/tests/Map/MapComponent.test.jsx +++ b/frontend/tests/Map/MapComponent.test.jsx @@ -3,7 +3,7 @@ import { render, waitFor, screen } from '@testing-library/react'; import '@testing-library/jest-dom/extend-expect'; import MapComponent from '../../src/components/Map/MapComponent'; import FiltersForm from '../../src/components/FiltersForm/FiltersForm'; -import { CategoriesProvider } from '../../src/components/Categories/CategoriesContext'; +import AppProviders from '../utils/providers'; import httpService from '../../src/services/http/httpService'; jest.mock('../../src/services/http/httpService'); @@ -32,6 +32,8 @@ const locations = [ httpService.getLocations.mockResolvedValue(locations); httpService.getCategoriesData.mockResolvedValue({ categories, defaultChecked: {} }); +// Fetched by the provider alongside the categories; no component here reads it. +httpService.getLocationSchema.mockResolvedValue({}); describe('MapComponent', () => { beforeAll(() => { @@ -48,25 +50,29 @@ describe('MapComponent', () => { it('renders without crashing', async () => { render( - + - , + , ); await waitFor(() => expect(screen.getAllByRole('presentation').length).toBeGreaterThan(0)); }); - it('does not fetch locations before filter state is initialized', () => { + // eslint-disable-next-line es-x/no-async-functions -- needed to await waitFor + it('does not fetch locations before filter state is initialized', async () => { httpService.getLocations.mockClear(); // Never resolves, so the provider leaves the filter state uninitialized. httpService.getCategoriesData.mockReturnValueOnce(new Promise(() => {})); render( - + - , + , ); + // Let the provider's other fetch settle first, so this asserts on the filter + // state rather than on merely having run before anything could happen. + await waitFor(() => expect(httpService.getLocationSchema).toHaveBeenCalled()); expect(httpService.getLocations).not.toHaveBeenCalled(); }); }); @@ -84,10 +90,10 @@ describe('MapComponent with FiltersForm', () => { }); render( - + - , + , ); await waitFor(() => expect(httpService.getLocations).toHaveBeenCalledTimes(1)); diff --git a/frontend/tests/Map/components/AccessibilityTable.test.jsx b/frontend/tests/Map/components/AccessibilityTable.test.jsx index 25930438..8c81b0b5 100644 --- a/frontend/tests/Map/components/AccessibilityTable.test.jsx +++ b/frontend/tests/Map/components/AccessibilityTable.test.jsx @@ -4,7 +4,7 @@ import { render, screen, act, waitFor } from '@testing-library/react'; import AccessibilityTable from '../../../src/components/Map/components/AccessibilityTable'; import httpService from '../../../src/services/http/httpService'; -import { CategoriesProvider } from '../../../src/components/Categories/CategoriesContext'; +import AppProviders from '../../utils/providers'; jest.mock('../../../src/services/http/httpService'); @@ -44,12 +44,12 @@ describe('Accessibility Table', () => { const lng = 17.0555; return act(() => { render( - + {}} /> - , + , ); }); }); diff --git a/frontend/tests/Map/components/Markers.test.jsx b/frontend/tests/Map/components/Markers.test.jsx index 44edffb8..a04fd2d0 100644 --- a/frontend/tests/Map/components/Markers.test.jsx +++ b/frontend/tests/Map/components/Markers.test.jsx @@ -3,7 +3,7 @@ import '@testing-library/jest-dom'; import { render, waitFor } from '@testing-library/react'; import { MapContainer } from 'react-leaflet'; import Markers from '../../../src/components/Map/components/Markers'; -import { CategoriesProvider } from '../../../src/components/Categories/CategoriesContext'; +import AppProviders from '../../utils/providers'; import httpService from '../../../src/services/http/httpService'; jest.mock('../../../src/services/http/httpService', () => ({ @@ -11,20 +11,23 @@ jest.mock('../../../src/services/http/httpService', () => ({ default: { getCategoriesData: jest.fn(), getLocations: jest.fn(), + getLocationSchema: jest.fn(), }, })); const renderMarkers = onLoadingChange => render( - + - , + , ); beforeEach(() => { httpService.getCategoriesData.mockResolvedValue({ categories: [], defaultChecked: {} }); + // The provider fetches this alongside the categories; Markers itself never reads it. + httpService.getLocationSchema.mockResolvedValue({}); // Server-side clustering settles the loading state directly, rather than waiting on // a Leaflet cluster event, which keeps these assertions about Markers itself. globalThis.FEATURE_FLAGS = { USE_SERVER_SIDE_CLUSTERING: true }; diff --git a/frontend/tests/Map/components/SuggestNewPoint.test.jsx b/frontend/tests/Map/components/SuggestNewPoint.test.jsx index fa223cc6..c98a04b1 100644 --- a/frontend/tests/Map/components/SuggestNewPoint.test.jsx +++ b/frontend/tests/Map/components/SuggestNewPoint.test.jsx @@ -5,8 +5,7 @@ import axios from 'axios'; import imageCompression from 'browser-image-compression'; import SuggestNewPointButton from '../../../src/components/Map/components/SuggestNewPointButton'; import { LocationProvider } from '../../../src/components/Map/context/LocationContext'; -import { CategoriesProvider } from '../../../src/components/Categories/CategoriesContext'; -import { LocationSchemaProvider } from '../../../src/components/Map/context/LocationSchemaContext'; +import AppProviders from '../../utils/providers'; import { mockGeolocationSuccess, mockGeolocationError, @@ -25,11 +24,9 @@ import toast from '../../../src/utils/toast'; const renderWithProvider = component => render( - - - {component} - - , + + {component} + , ); jest.mock('axios'); @@ -545,4 +542,30 @@ describe('SuggestNewPointButton', () => { expect(axios.post).toHaveBeenCalledTimes(1); }); }); + it('offers only category options the definitions can label', async () => { + mockGeolocationSuccess(); + // The schema advertises a value the category definitions carry no label for. + // Options and their labels must come from one source, so the stale value is + // simply not offered - rather than rendered with its raw key as the label. + httpService.getLocationSchema.mockResolvedValue({ + ...FULL_SCHEMA, + categories: { + ...FULL_SCHEMA.categories, + accessible_by: [...FULL_SCHEMA.categories.accessible_by, 'hovercrafts'], + }, + }); + + renderWithProvider(); + await openDialog(); + + fireEvent.mouseDown(screen.getByRole('combobox', { name: /accessible by/i })); + + const options = await screen.findAllByRole('option'); + expect(options.map(option => option.textContent)).toEqual([ + 'Bikes', + 'Cars', + 'Pedestrians', + ]); + expect(screen.queryByText('hovercrafts')).not.toBeInTheDocument(); + }); }); diff --git a/frontend/tests/MarkerPopup/ReportProblemForm.test.jsx b/frontend/tests/MarkerPopup/ReportProblemForm.test.jsx index 035b03e0..0d6c9538 100644 --- a/frontend/tests/MarkerPopup/ReportProblemForm.test.jsx +++ b/frontend/tests/MarkerPopup/ReportProblemForm.test.jsx @@ -7,12 +7,12 @@ const axios = require('axios'); // The schema comes from a provider that fetches it; these tests exercise the form, so // the hook is stubbed to keep them synchronous and focused. -jest.mock('../../src/components/Map/context/LocationSchemaContext', () => ({ - useLocationSchema: jest.fn(), +jest.mock('../../src/context/DeploymentDataContext', () => ({ + useDeploymentData: jest.fn(), })); -const { useLocationSchema } = require('../../src/components/Map/context/LocationSchemaContext'); +const { useDeploymentData } = require('../../src/context/DeploymentDataContext'); -const mockSchema = (schema = {}) => useLocationSchema.mockReturnValue({ locationSchema: schema }); +const mockSchema = (schema = {}) => useDeploymentData.mockReturnValue({ locationSchema: schema }); axios.post.mockResolvedValue({ data: { success: true } }); diff --git a/frontend/tests/utils/providers.jsx b/frontend/tests/utils/providers.jsx new file mode 100644 index 00000000..558acbd7 --- /dev/null +++ b/frontend/tests/utils/providers.jsx @@ -0,0 +1,23 @@ +import React from 'react'; +import PropTypes from 'prop-types'; +import { DeploymentDataProvider } from '../../src/context/DeploymentDataContext'; +import { FiltersProvider } from '../../src/context/FiltersContext'; + +/** + * Wraps a component in the same provider nesting the app itself uses, so tests exercise + * the real arrangement instead of each re-declaring it and drifting from it. + * + * @param {{children: React.ReactNode}} props + * @return {React.ReactElement} The children inside the app's providers + */ +const AppProviders = ({ children }) => ( + + {children} + +); + +AppProviders.propTypes = { + children: PropTypes.node.isRequired, +}; + +export default AppProviders; From d557988b3a0407f32513425727340e9d56e2c832 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Krzysztof=20Ko=C5=82odzi=C5=84ski?= Date: Wed, 19 Aug 2026 16:48:02 +0200 Subject: [PATCH 05/18] rebase --- frontend/src/components/FiltersForm/FiltersForm.jsx | 8 ++------ .../Map/components/SuggestNewPointDialog.jsx | 10 +++++++++- frontend/tests/Map/MapComponent.test.jsx | 1 - frontend/tests/Map/components/SuggestNewPoint.test.jsx | 6 +----- 4 files changed, 12 insertions(+), 13 deletions(-) diff --git a/frontend/src/components/FiltersForm/FiltersForm.jsx b/frontend/src/components/FiltersForm/FiltersForm.jsx index c14c5615..9ef89f1e 100644 --- a/frontend/src/components/FiltersForm/FiltersForm.jsx +++ b/frontend/src/components/FiltersForm/FiltersForm.jsx @@ -242,12 +242,8 @@ const LoadingSkeleton = () => ( const FiltersForm = () => { const { t } = useTranslation(); - const { - categoriesData, - categoriesLoading, - categoriesError, - refetchCategories, - } = useDeploymentData(); + const { categoriesData, categoriesLoading, categoriesError, refetchCategories } = + useDeploymentData(); const { selectedFilters, setSelectedFilters } = useFilters(); const handleCheckboxChange = event => { diff --git a/frontend/src/components/Map/components/SuggestNewPointDialog.jsx b/frontend/src/components/Map/components/SuggestNewPointDialog.jsx index c2c4e415..e978fb5c 100644 --- a/frontend/src/components/Map/components/SuggestNewPointDialog.jsx +++ b/frontend/src/components/Map/components/SuggestNewPointDialog.jsx @@ -439,7 +439,15 @@ const SuggestNewPointForm = ({ open, onClose, locationSchema }) => { SuggestNewPointForm.propTypes = { open: PropTypes.bool.isRequired, onClose: PropTypes.func.isRequired, - locationSchema: PropTypes.object.isRequired, + // Only the parts this form reads; the schema itself carries more. + locationSchema: PropTypes.shape({ + obligatory_fields: PropTypes.arrayOf(PropTypes.arrayOf(PropTypes.string)), + photo: PropTypes.shape({ + allowed_extensions: PropTypes.arrayOf(PropTypes.string), + allowed_mime_types: PropTypes.arrayOf(PropTypes.string), + max_size_bytes: PropTypes.number, + }), + }).isRequired, }; /** diff --git a/frontend/tests/Map/MapComponent.test.jsx b/frontend/tests/Map/MapComponent.test.jsx index 0b391eac..02fa9ae5 100644 --- a/frontend/tests/Map/MapComponent.test.jsx +++ b/frontend/tests/Map/MapComponent.test.jsx @@ -58,7 +58,6 @@ describe('MapComponent', () => { await waitFor(() => expect(screen.getAllByRole('presentation').length).toBeGreaterThan(0)); }); - // eslint-disable-next-line es-x/no-async-functions -- needed to await waitFor it('does not fetch locations before filter state is initialized', async () => { httpService.getLocations.mockClear(); // Never resolves, so the provider leaves the filter state uninitialized. diff --git a/frontend/tests/Map/components/SuggestNewPoint.test.jsx b/frontend/tests/Map/components/SuggestNewPoint.test.jsx index c98a04b1..0bc1f1dd 100644 --- a/frontend/tests/Map/components/SuggestNewPoint.test.jsx +++ b/frontend/tests/Map/components/SuggestNewPoint.test.jsx @@ -561,11 +561,7 @@ describe('SuggestNewPointButton', () => { fireEvent.mouseDown(screen.getByRole('combobox', { name: /accessible by/i })); const options = await screen.findAllByRole('option'); - expect(options.map(option => option.textContent)).toEqual([ - 'Bikes', - 'Cars', - 'Pedestrians', - ]); + expect(options.map(option => option.textContent)).toEqual(['Bikes', 'Cars', 'Pedestrians']); expect(screen.queryByText('hovercrafts')).not.toBeInTheDocument(); }); }); From c0c6b18724ea92bf2800d86726a0b4b6bca5a405 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Krzysztof=20Ko=C5=82odzi=C5=84ski?= Date: Wed, 19 Aug 2026 16:52:52 +0200 Subject: [PATCH 06/18] little fix --- goodmap/goodmap.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/goodmap/goodmap.py b/goodmap/goodmap.py index 03bcd086..64139404 100644 --- a/goodmap/goodmap.py +++ b/goodmap/goodmap.py @@ -282,9 +282,6 @@ def index(): db.get_home_page_path()). Deployments set site_content.home_page_path to "/map" so visiting / still renders this view, with no redirect. - The frontend reads what this deployment accepts from /api/location-schema - rather than from an inlined copy, so there is one source of truth for it. - Returns: Rendered map.html template with feature flags and the plugin manifest """ From 2a640b12cb52614f6ddaf00ef43b6cb6738d2ca4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Krzysztof=20Ko=C5=82odzi=C5=84ski?= Date: Wed, 19 Aug 2026 17:22:37 +0200 Subject: [PATCH 07/18] cleanup --- docs/http-api.rst | 12 ++++--- .../src/context/DeploymentDataContext.jsx | 1 - .../Map/components/SuggestNewPoint.test.jsx | 16 +++------ frontend/tests/utils/testConstants.js | 5 --- goodmap/api/api_models.py | 3 -- goodmap/api/core_api.py | 10 +++--- tests/unit_tests/test_core_api.py | 3 +- tests/unit_tests/test_goodmap.py | 35 ------------------- 8 files changed, 20 insertions(+), 65 deletions(-) diff --git a/docs/http-api.rst b/docs/http-api.rst index 215558f3..43eaa463 100644 --- a/docs/http-api.rst +++ b/docs/http-api.rst @@ -211,10 +211,14 @@ buttons for ``exclusive``/``threshold``, a single checkbox for ``boolean`` ~~~~~~~~~~~~~~~~~~~~~~~~~~~~ What this instance accepts for a new point: the fields of its location model (all of them -except the server-assigned ``uuid``), the allowed values per category, the reportable -issue types, and the photo limits. This is how a client learns what to put in -``/api/suggest-new-point``'s ``location`` payload rather than assuming — it is the same -schema the built-in suggest form is generated from. +except the server-assigned ``uuid``), the reportable issue types, and the photo limits. +This is how a client learns what to put in ``/api/suggest-new-point``'s ``location`` +payload rather than assuming — it is the same schema the built-in suggest form is +generated from. + +The allowed values for a category are **not** repeated here. ``/api/categories-full`` +above reports them, with their translated labels, and is the single place to read them +from — so a client cannot end up holding two versions of the same list. ``GET /api/languages`` ~~~~~~~~~~~~~~~~~~~~~~ diff --git a/frontend/src/context/DeploymentDataContext.jsx b/frontend/src/context/DeploymentDataContext.jsx index cd0199cb..50359ea3 100644 --- a/frontend/src/context/DeploymentDataContext.jsx +++ b/frontend/src/context/DeploymentDataContext.jsx @@ -22,7 +22,6 @@ DeploymentDataContext.displayName = 'DeploymentDataContext'; /* eslint-disable camelcase -- these are the API's own field names */ const EMPTY_SCHEMA = { obligatory_fields: [], - categories: {}, fields: {}, reported_issue_types: [], photo: {}, diff --git a/frontend/tests/Map/components/SuggestNewPoint.test.jsx b/frontend/tests/Map/components/SuggestNewPoint.test.jsx index 0bc1f1dd..1967629c 100644 --- a/frontend/tests/Map/components/SuggestNewPoint.test.jsx +++ b/frontend/tests/Map/components/SuggestNewPoint.test.jsx @@ -542,26 +542,18 @@ describe('SuggestNewPointButton', () => { expect(axios.post).toHaveBeenCalledTimes(1); }); }); - it('offers only category options the definitions can label', async () => { + it('builds category options from the category definitions', async () => { mockGeolocationSuccess(); - // The schema advertises a value the category definitions carry no label for. - // Options and their labels must come from one source, so the stale value is - // simply not offered - rather than rendered with its raw key as the label. - httpService.getLocationSchema.mockResolvedValue({ - ...FULL_SCHEMA, - categories: { - ...FULL_SCHEMA.categories, - accessible_by: [...FULL_SCHEMA.categories.accessible_by, 'hovercrafts'], - }, - }); renderWithProvider(); await openDialog(); fireEvent.mouseDown(screen.getByRole('combobox', { name: /accessible by/i })); + // The schema says accessible_by is obligatory but no longer carries its values: + // both the values and their labels come from the category definitions, which + // are now the only place either is reported. const options = await screen.findAllByRole('option'); expect(options.map(option => option.textContent)).toEqual(['Bikes', 'Cars', 'Pedestrians']); - expect(screen.queryByText('hovercrafts')).not.toBeInTheDocument(); }); }); diff --git a/frontend/tests/utils/testConstants.js b/frontend/tests/utils/testConstants.js index 83502235..af47c831 100644 --- a/frontend/tests/utils/testConstants.js +++ b/frontend/tests/utils/testConstants.js @@ -50,7 +50,6 @@ export const PHOTO_SCHEMA = { */ export const SIMPLE_SCHEMA = { obligatory_fields: [['name', 'str']], - categories: {}, photo: PHOTO_SCHEMA, }; @@ -63,9 +62,5 @@ export const FULL_SCHEMA = { ['accessible_by', 'list'], ['type_of_place', 'str'], ], - categories: { - accessible_by: ['bikes', 'cars', 'pedestrians'], - type_of_place: ['big bridge', 'small bridge'], - }, photo: PHOTO_SCHEMA, }; diff --git a/goodmap/api/api_models.py b/goodmap/api/api_models.py index fefb06a5..e59b7915 100644 --- a/goodmap/api/api_models.py +++ b/goodmap/api/api_models.py @@ -190,9 +190,6 @@ class LocationSchemaResponse(BaseModel): obligatory_fields: list[Any] = Field( ..., description="[name, type] pairs every point must carry" ) - categories: dict[str, list[str]] = Field( - ..., description="Filterable fields and their allowed values" - ) reported_issue_types: list[IssueType] = Field( ..., description="Accepted values for /api/report-location" ) diff --git a/goodmap/api/core_api.py b/goodmap/api/core_api.py index b2c1280d..ac77b258 100644 --- a/goodmap/api/core_api.py +++ b/goodmap/api/core_api.py @@ -436,10 +436,13 @@ def get_location_schema(): The fields a point may carry are configured per deployment, so there is no fixed payload for /api/suggest-new-point. This returns the accepted fields (excluding only the server-assigned uuid - position is required and must be - supplied by the client), the allowed values for each category, the reportable - issue types and the photo limits, as the built-in suggest form uses them. + supplied by the client), the reportable issue types and the photo limits, as + the built-in suggest form uses them. + + The allowed values per category are deliberately not repeated here: + /api/categories-full is the one place that reports them, so a client cannot + read two versions of the same list and find them disagreeing. """ - category_data = database.get_category_data() properties = location_model.model_json_schema().get("properties", {}) # Matches the fallback /api/report-location applies: an unconfigured # reported_issue_types must not make this endpoint advertise fewer accepted @@ -451,7 +454,6 @@ def get_location_schema(): "obligatory_fields": current_app.extensions.get("goodmap", {}).get( "location_obligatory_fields", [] ), - "categories": category_data.get("categories", {}), "reported_issue_types": [{"value": t, "label": gettext(t)} for t in issue_options], "photo": { "allowed_extensions": sorted(photo_attachment_config.allowed_extensions or []), diff --git a/tests/unit_tests/test_core_api.py b/tests/unit_tests/test_core_api.py index 06844bb7..b44396af 100644 --- a/tests/unit_tests/test_core_api.py +++ b/tests/unit_tests/test_core_api.py @@ -40,10 +40,11 @@ def test_location_schema_endpoint_describes_this_instance(test_app): assert set(body) == { "fields", "obligatory_fields", - "categories", "reported_issue_types", "photo", } + # Category values live in /api/categories-full alone, so they must not reappear here. + assert "categories" not in body # uuid is server-assigned and must not be offered as a form field; position is # required and client-supplied, same as /api/suggest-new-point, so it must be. assert "uuid" not in body["fields"] diff --git a/tests/unit_tests/test_goodmap.py b/tests/unit_tests/test_goodmap.py index 34774abb..5fab3be2 100644 --- a/tests/unit_tests/test_goodmap.py +++ b/tests/unit_tests/test_goodmap.py @@ -107,40 +107,6 @@ 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_location_schema_endpoint_reports_configured_categories(): - """The schema the frontend builds its form from comes from /api/location-schema, - not from anything inlined into the map page.""" - config = GoodmapConfig( - APP_NAME="test_app", - SECRET_KEY="test_secret", - USE_WWW=False, - BLOG_PREFIX="/blog", - DB=JsonDbConfig( - DATA={ - "site_content": {"pages": []}, - "categories": { - "accessibility": ["wheelchair", "elevator"], - "amenities": ["wifi", "parking"], - }, - }, - TYPE="json", - ), - ) - app = goodmap.create_app_from_config(config) - # CSRF protection must be disabled in test environment to allow API testing - # This is safe because tests run in isolation, not in production - app.config["WTF_CSRF_ENABLED"] = False # NOSONAR - client = app.test_client() - - response = client.get("/api/location-schema") - assert response.status_code == 200 - - schema = response.json - assert schema is not None - assert "obligatory_fields" in schema - assert set(schema["categories"]) == {"accessibility", "amenities"} - - 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 @@ -313,7 +279,6 @@ def test_location_schema_endpoint_with_lazy_loading(): # position is client-supplied, so it must be offered as a field; uuid is not assert "position" in schema["fields"] assert "uuid" not in schema["fields"] - assert "test_category" in schema["categories"] def _plugin_ep(name: str, plugin_dir: str | None, base: type = MapOverlayPluginBase): From 22aa2458faf6ee0d797022112071bc253f096cc5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Krzysztof=20Ko=C5=82odzi=C5=84ski?= Date: Wed, 19 Aug 2026 17:40:05 +0200 Subject: [PATCH 08/18] removed extra comment --- docs/http-api.rst | 4 ---- 1 file changed, 4 deletions(-) diff --git a/docs/http-api.rst b/docs/http-api.rst index 43eaa463..b42b0a7c 100644 --- a/docs/http-api.rst +++ b/docs/http-api.rst @@ -216,10 +216,6 @@ This is how a client learns what to put in ``/api/suggest-new-point``'s ``locati payload rather than assuming — it is the same schema the built-in suggest form is generated from. -The allowed values for a category are **not** repeated here. ``/api/categories-full`` -above reports them, with their translated labels, and is the single place to read them -from — so a client cannot end up holding two versions of the same list. - ``GET /api/languages`` ~~~~~~~~~~~~~~~~~~~~~~ From 5aa8eb8e11fc2dc0164667c3c74cbfffa4aebe19 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Krzysztof=20Ko=C5=82odzi=C5=84ski?= Date: Wed, 19 Aug 2026 17:48:23 +0200 Subject: [PATCH 09/18] fixes --- .../src/components/Map/components/SuggestNewPointDialog.jsx | 2 -- goodmap/api/core_api.py | 1 + 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/frontend/src/components/Map/components/SuggestNewPointDialog.jsx b/frontend/src/components/Map/components/SuggestNewPointDialog.jsx index e978fb5c..ce1c70e0 100644 --- a/frontend/src/components/Map/components/SuggestNewPointDialog.jsx +++ b/frontend/src/components/Map/components/SuggestNewPointDialog.jsx @@ -46,8 +46,6 @@ const mapCategoryOptions = categoryOptions => { }; // Build { fieldNames, options } translation maps from the fetched category definitions. -// Every category key is registered, even when it has no options, so a category with an -// empty option list stays distinguishable from a field that is not a category at all. const buildCategoryTranslations = categoriesData => { const fieldNames = {}; const options = {}; diff --git a/goodmap/api/core_api.py b/goodmap/api/core_api.py index ac77b258..279de2e7 100644 --- a/goodmap/api/core_api.py +++ b/goodmap/api/core_api.py @@ -479,6 +479,7 @@ def get_categories_full(): categories_default_checked = categories_data.get("categories_default_checked", {}) categories_filter_mode = categories_data.get("categories_filter_mode", {}) + # TODO: reject empty categories at startup - they make obligatory fields unfillable for key, options in categories_data["categories"].items(): category_entry = { "key": key, From 409bbc9b582488b0d7fbd349dc5135da8f0d9d76 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Krzysztof=20Ko=C5=82odzi=C5=84ski?= Date: Wed, 19 Aug 2026 17:53:04 +0200 Subject: [PATCH 10/18] removed extra comment --- goodmap/api/core_api.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/goodmap/api/core_api.py b/goodmap/api/core_api.py index 279de2e7..0e4dd955 100644 --- a/goodmap/api/core_api.py +++ b/goodmap/api/core_api.py @@ -439,9 +439,6 @@ def get_location_schema(): supplied by the client), the reportable issue types and the photo limits, as the built-in suggest form uses them. - The allowed values per category are deliberately not repeated here: - /api/categories-full is the one place that reports them, so a client cannot - read two versions of the same list and find them disagreeing. """ properties = location_model.model_json_schema().get("properties", {}) # Matches the fallback /api/report-location applies: an unconfigured From bdfe80b32811c64c2d52d8b63705dfbee60bb050 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Krzysztof=20Ko=C5=82odzi=C5=84ski?= Date: Wed, 19 Aug 2026 17:54:34 +0200 Subject: [PATCH 11/18] removed some comments --- tests/unit_tests/test_core_api.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/unit_tests/test_core_api.py b/tests/unit_tests/test_core_api.py index b44396af..c23eb64b 100644 --- a/tests/unit_tests/test_core_api.py +++ b/tests/unit_tests/test_core_api.py @@ -43,7 +43,6 @@ def test_location_schema_endpoint_describes_this_instance(test_app): "reported_issue_types", "photo", } - # Category values live in /api/categories-full alone, so they must not reappear here. assert "categories" not in body # uuid is server-assigned and must not be offered as a form field; position is # required and client-supplied, same as /api/suggest-new-point, so it must be. From 105c178d8e5a92c7e44a775859bd7e64952f4588 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Krzysztof=20Ko=C5=82odzi=C5=84ski?= Date: Wed, 19 Aug 2026 21:47:31 +0200 Subject: [PATCH 12/18] fixes comments --- .../components/Map/components/SuggestNewPointDialog.jsx | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/frontend/src/components/Map/components/SuggestNewPointDialog.jsx b/frontend/src/components/Map/components/SuggestNewPointDialog.jsx index ce1c70e0..5a1431a3 100644 --- a/frontend/src/components/Map/components/SuggestNewPointDialog.jsx +++ b/frontend/src/components/Map/components/SuggestNewPointDialog.jsx @@ -104,8 +104,11 @@ const useScrollToTop = trigger => { }; /** - * The suggestion form itself. Mounted only once the location schema is known, so the - * fields it generates can be built in one pass from the deployment's obligatory fields. + * The dialog and its schema-driven form, mounted by SuggestNewPointDialog only once + * the location schema has loaded. + * + * Its inputs are generated from the schema's obligatory_fields, so mounting earlier + * would seed a form with no fields and force a rebuild when the schema arrived. * * @param {{open: boolean, onClose: () => void, locationSchema: Object}} props * @returns {React.ReactElement} Dialog with the new point suggestion form From 97085b9a70af76eec2d3fbf5c4678b508b048352 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Krzysztof=20Ko=C5=82odzi=C5=84ski?= Date: Wed, 19 Aug 2026 22:08:47 +0200 Subject: [PATCH 13/18] fixes after review --- .../Map/components/SuggestNewPointDialog.jsx | 28 ++++++++++++++-- .../src/context/DeploymentDataContext.jsx | 12 ++++--- frontend/src/locales/en/map.json | 1 + frontend/src/locales/pl/map.json | 1 + frontend/src/locales/ua/map.json | 1 + frontend/tests/DeploymentDataContext.test.jsx | 32 +++++++++++++++++++ .../Map/components/SuggestNewPoint.test.jsx | 22 +++++++++++++ 7 files changed, 89 insertions(+), 8 deletions(-) diff --git a/frontend/src/components/Map/components/SuggestNewPointDialog.jsx b/frontend/src/components/Map/components/SuggestNewPointDialog.jsx index 5a1431a3..f0df16e6 100644 --- a/frontend/src/components/Map/components/SuggestNewPointDialog.jsx +++ b/frontend/src/components/Map/components/SuggestNewPointDialog.jsx @@ -456,13 +456,35 @@ SuggestNewPointForm.propTypes = { * * The form's fields are generated from the deployment's location schema, so there is * nothing to render until that has arrived - holding the form back until then is what - * lets it build its fields once, instead of rebuilding them when the schema lands. + * lets it build its fields once, instead of rebuilding them when the schema lands. If + * the schema could not be fetched at all, this says so instead of showing a form that + * has no fields to fill and could never be submitted. * * @param {{open: boolean, onClose: () => void}} props - * @returns {React.ReactElement|null} The suggestion form, or null while the schema loads + * @returns {React.ReactElement|null} The form, a retry prompt, or null while it loads */ const SuggestNewPointDialog = ({ open, onClose }) => { - const { locationSchema } = useDeploymentData(); + const { t } = useTranslation(); + const { locationSchema, schemaError, refetchLocationSchema } = useDeploymentData(); + + // Without the schema there are no fields to fill, so offering the form anyway would + // only produce a submission the backend is bound to reject. Say so and offer a retry. + if (schemaError) { + return ( + + {t('suggestNewPointDialogTitle')} + + {t('loadSuggestFormError')} + + + + + + + ); + } if (!locationSchema) { return null; diff --git a/frontend/src/context/DeploymentDataContext.jsx b/frontend/src/context/DeploymentDataContext.jsx index 50359ea3..201a810c 100644 --- a/frontend/src/context/DeploymentDataContext.jsx +++ b/frontend/src/context/DeploymentDataContext.jsx @@ -71,10 +71,9 @@ export const DeploymentDataProvider = ({ children }) => { setLocationSchema({ ...EMPTY_SCHEMA, ...(await httpService.getLocationSchema()) }); } catch (error) { console.error('Failed to load location schema:', error); - // Settle on the empty schema rather than leaving it null: null means "still - // loading" and holds the suggest form back, which would turn a failed fetch - // into a button that silently does nothing. - setLocationSchema(EMPTY_SCHEMA); + // Left null: a failed request must not read as a resolved schema, or the + // suggest form renders with no fields and can never be submitted. Consumers + // tell the two apart by schemaError - null alone only means "not yet". setSchemaError(true); } }, []); @@ -98,6 +97,7 @@ export const DeploymentDataProvider = ({ children }) => { refetchCategories: fetchCategories, locationSchema, schemaError, + refetchLocationSchema: fetchLocationSchema, }), [ categoriesData, @@ -107,6 +107,7 @@ export const DeploymentDataProvider = ({ children }) => { fetchCategories, locationSchema, schemaError, + fetchLocationSchema, ], ); @@ -128,8 +129,9 @@ DeploymentDataProvider.propTypes = { * - defaultChecked: options pre-selected by the deployment, keyed by category * - categoriesLoading / categoriesError: state of the category definitions fetch * - refetchCategories: retries that fetch - * - locationSchema: schema for a new point, null until it arrives + * - locationSchema: schema for a new point, null until it arrives (and if it failed) * - schemaError: true if fetching the location schema failed + * - refetchLocationSchema: retries that fetch * * @throws {Error} If used outside of DeploymentDataProvider * @return {Object} The deployment data described above diff --git a/frontend/src/locales/en/map.json b/frontend/src/locales/en/map.json index 26f745d1..8cc1af83 100644 --- a/frontend/src/locales/en/map.json +++ b/frontend/src/locales/en/map.json @@ -40,6 +40,7 @@ "linkCopied": "Link copied to clipboard", "linkCopyFailed": "Failed to copy link", "loadFiltersError": "Failed to load filters.", + "loadSuggestFormError": "Failed to load the suggestion form.", "retry": "Retry", "clearFilters": "Clear filters", "clearAllFiltersAriaLabel": "Clear all filters", diff --git a/frontend/src/locales/pl/map.json b/frontend/src/locales/pl/map.json index 84615adf..6980e5ce 100644 --- a/frontend/src/locales/pl/map.json +++ b/frontend/src/locales/pl/map.json @@ -40,6 +40,7 @@ "linkCopied": "Link skopiowany do schowka", "linkCopyFailed": "Nie udało się skopiować linku", "loadFiltersError": "Nie udało się załadować filtrów.", + "loadSuggestFormError": "Nie udało się załadować formularza propozycji.", "retry": "Spróbuj ponownie", "clearFilters": "Wyczyść filtry", "clearAllFiltersAriaLabel": "Wyczyść wszystkie filtry", diff --git a/frontend/src/locales/ua/map.json b/frontend/src/locales/ua/map.json index 3b0a4341..9d1b5597 100644 --- a/frontend/src/locales/ua/map.json +++ b/frontend/src/locales/ua/map.json @@ -39,6 +39,7 @@ "linkCopied": "Посилання скопійовано", "linkCopyFailed": "Не вдалося скопіювати посилання", "loadFiltersError": "Не вдалося завантажити фільтри.", + "loadSuggestFormError": "Не вдалося завантажити форму пропозиції.", "retry": "Спробувати ще раз", "clearFilters": "Очистити фільтри", "clearAllFiltersAriaLabel": "Очистити всі фільтри", diff --git a/frontend/tests/DeploymentDataContext.test.jsx b/frontend/tests/DeploymentDataContext.test.jsx index 92e025c6..eac46196 100644 --- a/frontend/tests/DeploymentDataContext.test.jsx +++ b/frontend/tests/DeploymentDataContext.test.jsx @@ -5,6 +5,7 @@ import FiltersForm from '../src/components/FiltersForm/FiltersForm'; import SuggestNewPointButton from '../src/components/Map/components/SuggestNewPointButton'; import { LocationProvider } from '../src/components/Map/context/LocationContext'; import AppProviders from './utils/providers'; +import { useDeploymentData } from '../src/context/DeploymentDataContext'; import httpService from '../src/services/http/httpService'; jest.mock('axios'); @@ -40,3 +41,34 @@ test('categories are fetched once for all consumers', async () => { await waitFor(() => expect(httpService.getCategoriesData).toHaveBeenCalled()); expect(httpService.getCategoriesData).toHaveBeenCalledTimes(1); }); + +// A probe is the only way to see what the provider hands consumers, rather than what +// one particular consumer happens to render from it. +const SchemaProbe = () => { + const { locationSchema, schemaError } = useDeploymentData(); + return ( +
+ {locationSchema === null ? 'null' : 'resolved'} + {String(schemaError)} +
+ ); +}; + +// A failed request must not be handed on as a schema: consumers would read an empty one +// as a real answer and build a form with no fields from it. +test('a failed schema request is not exposed as a resolved schema', async () => { + const consoleErrorSpy = jest.spyOn(console, 'error').mockImplementation(); + httpService.getCategoriesData.mockResolvedValue({ categories: [], defaultChecked: {} }); + httpService.getLocationSchema.mockRejectedValue(new Error('schema unavailable')); + + const { getByTestId } = render( + + + , + ); + + await waitFor(() => expect(getByTestId('error')).toHaveTextContent('true')); + expect(getByTestId('schema')).toHaveTextContent('null'); + + consoleErrorSpy.mockRestore(); +}); diff --git a/frontend/tests/Map/components/SuggestNewPoint.test.jsx b/frontend/tests/Map/components/SuggestNewPoint.test.jsx index 1967629c..937c17b6 100644 --- a/frontend/tests/Map/components/SuggestNewPoint.test.jsx +++ b/frontend/tests/Map/components/SuggestNewPoint.test.jsx @@ -556,4 +556,26 @@ describe('SuggestNewPointButton', () => { const options = await screen.findAllByRole('option'); expect(options.map(option => option.textContent)).toEqual(['Bikes', 'Cars', 'Pedestrians']); }); + it('offers a retry instead of the form when the schema fails to load', async () => { + const consoleErrorSpy = jest.spyOn(console, 'error').mockImplementation(); + mockGeolocationSuccess(); + httpService.getLocationSchema.mockRejectedValue(new Error('schema unavailable')); + + renderWithProvider(); + await openDialog(); + + // With no schema there are no fields to fill, so a submitted form could only be + // rejected by the backend: the failure is stated instead of offering one. + expect(screen.getByText('Failed to load the suggestion form.')).toBeInTheDocument(); + expect(screen.queryByRole('button', { name: /submit/i })).not.toBeInTheDocument(); + + httpService.getLocationSchema.mockResolvedValue(FULL_SCHEMA); + fireEvent.click(screen.getByRole('button', { name: /retry/i })); + + await waitFor(() => + expect(screen.getByRole('button', { name: /submit/i })).toBeInTheDocument(), + ); + + consoleErrorSpy.mockRestore(); + }); }); From ebbbcf7327db79424a605d91c7f4ddaf0c6a0d0c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Krzysztof=20Ko=C5=82odzi=C5=84ski?= Date: Wed, 19 Aug 2026 23:18:40 +0200 Subject: [PATCH 14/18] removed fallbacks --- .../Map/components/SuggestNewPointDialog.jsx | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/frontend/src/components/Map/components/SuggestNewPointDialog.jsx b/frontend/src/components/Map/components/SuggestNewPointDialog.jsx index f0df16e6..54f1d5b6 100644 --- a/frontend/src/components/Map/components/SuggestNewPointDialog.jsx +++ b/frontend/src/components/Map/components/SuggestNewPointDialog.jsx @@ -307,7 +307,7 @@ const SuggestNewPointForm = ({ open, onClose, locationSchema }) => { @@ -350,7 +348,7 @@ const SuggestNewPointForm = ({ open, onClose, locationSchema }) => { Date: Wed, 19 Aug 2026 23:34:26 +0200 Subject: [PATCH 15/18] fixes after review --- docs/http-api.rst | 4 ++++ .../MarkerPopup/ReportProblemForm.jsx | 17 ++++++++++++--- .../MarkerPopup/ReportProblemForm.test.jsx | 12 +++++++++++ goodmap/api/core_api.py | 4 +++- tests/unit_tests/test_core_api.py | 21 ++++++++++++++++++- 5 files changed, 53 insertions(+), 5 deletions(-) diff --git a/docs/http-api.rst b/docs/http-api.rst index b42b0a7c..49023dd6 100644 --- a/docs/http-api.rst +++ b/docs/http-api.rst @@ -216,6 +216,10 @@ This is how a client learns what to put in ``/api/suggest-new-point``'s ``locati payload rather than assuming — it is the same schema the built-in suggest form is generated from. +A field's own allowed values are part of its schema, reported as ``enum_items``. For the +same values with translated labels, ready to render a filter panel, use +``/api/categories-full`` above. + ``GET /api/languages`` ~~~~~~~~~~~~~~~~~~~~~~ diff --git a/frontend/src/components/MarkerPopup/ReportProblemForm.jsx b/frontend/src/components/MarkerPopup/ReportProblemForm.jsx index e8bf4545..33004cb5 100644 --- a/frontend/src/components/MarkerPopup/ReportProblemForm.jsx +++ b/frontend/src/components/MarkerPopup/ReportProblemForm.jsx @@ -154,7 +154,14 @@ const ReportProblemForm = ({ placeId }) => { const [isSubmitted, setIsSubmitted] = useState(false); const [responseMessage, setResponseMessage] = useState(''); - const issueTypeOptions = getIssueTypeOptions(t, locationSchema?.reported_issue_types); + // Until the schema arrives, this deployment's own issue types are unknown, and the + // legacy fallbacks below are not a stand-in for them: offering those would let a + // report be filed against a value this instance may not accept. A loaded schema that + // simply declares none is a different case, and does fall back. + const isSchemaLoaded = Boolean(locationSchema); + const issueTypeOptions = isSchemaLoaded + ? getIssueTypeOptions(t, locationSchema.reported_issue_types) + : []; const handleSubmit = async event => { event.preventDefault(); @@ -186,14 +193,18 @@ const ReportProblemForm = ({ placeId }) => { {problemType === 'other' && ( diff --git a/frontend/tests/MarkerPopup/ReportProblemForm.test.jsx b/frontend/tests/MarkerPopup/ReportProblemForm.test.jsx index 0d6c9538..ed7face9 100644 --- a/frontend/tests/MarkerPopup/ReportProblemForm.test.jsx +++ b/frontend/tests/MarkerPopup/ReportProblemForm.test.jsx @@ -134,3 +134,15 @@ describe('ReportProblemForm', () => { expect(getByText("it's broken")).toBeTruthy(); }); }); + +// A schema that has not arrived is not the same as one declaring no issue types: the +// legacy fallbacks are not a stand-in for values this deployment may not accept. +test('offers no issue types until the schema has loaded', () => { + useDeploymentData.mockReturnValue({ locationSchema: null }); + + const { select } = renderForm(); + + expect(select.disabled).toBe(true); + expect(select.querySelectorAll('option')).toHaveLength(1); + expect(select.textContent).not.toMatch(/not here|overload|broken/i); +}); diff --git a/goodmap/api/core_api.py b/goodmap/api/core_api.py index 0e4dd955..09be7455 100644 --- a/goodmap/api/core_api.py +++ b/goodmap/api/core_api.py @@ -439,6 +439,9 @@ def get_location_schema(): supplied by the client), the reportable issue types and the photo limits, as the built-in suggest form uses them. + A field's own allowed values are part of its schema under `fields`. What this no + longer carries is a separate top-level `categories` map repeating them in another + shape; /api/categories-full reports the same values with translated labels. """ properties = location_model.model_json_schema().get("properties", {}) # Matches the fallback /api/report-location applies: an unconfigured @@ -476,7 +479,6 @@ def get_categories_full(): categories_default_checked = categories_data.get("categories_default_checked", {}) categories_filter_mode = categories_data.get("categories_filter_mode", {}) - # TODO: reject empty categories at startup - they make obligatory fields unfillable for key, options in categories_data["categories"].items(): category_entry = { "key": key, diff --git a/tests/unit_tests/test_core_api.py b/tests/unit_tests/test_core_api.py index c23eb64b..92bfd0b2 100644 --- a/tests/unit_tests/test_core_api.py +++ b/tests/unit_tests/test_core_api.py @@ -43,7 +43,6 @@ def test_location_schema_endpoint_describes_this_instance(test_app): "reported_issue_types", "photo", } - assert "categories" not in body # uuid is server-assigned and must not be offered as a form field; position is # required and client-supplied, same as /api/suggest-new-point, so it must be. assert "uuid" not in body["fields"] @@ -52,6 +51,26 @@ def test_location_schema_endpoint_describes_this_instance(test_app): assert set(body["photo"]) == {"allowed_extensions", "allowed_mime_types", "max_size_bytes"} +def test_location_schema_reports_allowed_values_inside_each_field(): + """A field's allowed values are part of its own schema under `fields`. + + Removing the top-level `categories` key removed a second copy of them in a different + shape, not the values themselves - a client building a suggest payload still needs + them, and this is where it reads them from. + """ + test_app = create_test_app( + db_overrides={ + "categories": {"accessible_by": ["bikes", "cars"]}, + "location_obligatory_fields": [("accessible_by", "list"), ("name", "str")], + } + ) + response = test_app.get("/api/location-schema") + assert response.status_code == 200 + body = response.json + # frozenset-backed, so the order carries no meaning + assert set(body["fields"]["accessible_by"]["enum_items"]) == {"bikes", "cars"} + + def test_location_schema_endpoint_falls_back_to_default_issue_options(): """An unconfigured reported_issue_types must not undersell what /api/report-location actually accepts (it falls back to the same defaults). From e6e71af5c375d06dcdf4e9b3ab75010401f1de6b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Krzysztof=20Ko=C5=82odzi=C5=84ski?= Date: Thu, 20 Aug 2026 00:07:19 +0200 Subject: [PATCH 16/18] fix lint --- tests/unit_tests/test_core_api.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/unit_tests/test_core_api.py b/tests/unit_tests/test_core_api.py index 92bfd0b2..6dbb03e5 100644 --- a/tests/unit_tests/test_core_api.py +++ b/tests/unit_tests/test_core_api.py @@ -67,6 +67,7 @@ def test_location_schema_reports_allowed_values_inside_each_field(): response = test_app.get("/api/location-schema") assert response.status_code == 200 body = response.json + assert body is not None # frozenset-backed, so the order carries no meaning assert set(body["fields"]["accessible_by"]["enum_items"]) == {"bikes", "cars"} From 96349a4279483da30a542cac303985a69719fb4a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Krzysztof=20Ko=C5=82odzi=C5=84ski?= Date: Thu, 20 Aug 2026 00:11:51 +0200 Subject: [PATCH 17/18] added early exi --- .../MarkerPopup/ReportProblemForm.jsx | 63 ++++++++++++++----- frontend/src/locales/en/map.json | 1 + frontend/src/locales/pl/map.json | 1 + frontend/src/locales/ua/map.json | 1 + .../MarkerPopup/ReportProblemForm.test.jsx | 24 +++++-- 5 files changed, 70 insertions(+), 20 deletions(-) diff --git a/frontend/src/components/MarkerPopup/ReportProblemForm.jsx b/frontend/src/components/MarkerPopup/ReportProblemForm.jsx index 33004cb5..6c46004a 100644 --- a/frontend/src/components/MarkerPopup/ReportProblemForm.jsx +++ b/frontend/src/components/MarkerPopup/ReportProblemForm.jsx @@ -101,6 +101,28 @@ const SubmitButton = styled.input` /** * Styled success message component. */ +const ErrorMessage = styled.div` + padding: 12px 15px; + background-color: #fdecea; + border: 1px solid #f5c6cb; + border-radius: 8px; + color: #b71c1c; + font-size: 13px; + margin: 10px 15px; + text-align: center; +`; + +const RetryButton = styled.button` + margin-top: 8px; + font-size: 13px; + padding: 6px 12px; + border-radius: 6px; + border: 1px solid currentColor; + background: transparent; + color: inherit; + cursor: pointer; +`; + const SuccessMessage = styled.div` padding: 12px 15px; background-color: #e8f5e9; @@ -148,20 +170,15 @@ const getIssueTypeOptions = (t, dynamicTypes) => { const ReportProblemForm = ({ placeId }) => { const { t } = useTranslation(); - const { locationSchema } = useDeploymentData(); + const { locationSchema, schemaError, refetchLocationSchema } = useDeploymentData(); const [problem, setProblem] = useState(''); const [problemType, setProblemType] = useState(''); const [isSubmitted, setIsSubmitted] = useState(false); const [responseMessage, setResponseMessage] = useState(''); - // Until the schema arrives, this deployment's own issue types are unknown, and the - // legacy fallbacks below are not a stand-in for them: offering those would let a - // report be filed against a value this instance may not accept. A loaded schema that - // simply declares none is a different case, and does fall back. - const isSchemaLoaded = Boolean(locationSchema); - const issueTypeOptions = isSchemaLoaded - ? getIssueTypeOptions(t, locationSchema.reported_issue_types) - : []; + // Only reached once the schema is known, so a loaded schema declaring no issue + // types is the one case that falls back to the legacy defaults. + const issueTypeOptions = getIssueTypeOptions(t, locationSchema?.reported_issue_types); const handleSubmit = async event => { event.preventDefault(); @@ -189,22 +206,38 @@ const ReportProblemForm = ({ placeId }) => { return {responseMessage}; } + // This deployment's own issue types are the only ones it is guaranteed to accept, so + // the form is withheld until they are known: the legacy fallbacks are not a stand-in + // for them. A failed fetch has to say so, or the form would just never appear. + if (schemaError) { + return ( + + {t('loadReportFormError')} +
+ + {t('retry')} + +
+ + ); + } + + if (!locationSchema) { + return null; + } + return ( {problemType === 'other' && ( diff --git a/frontend/src/locales/en/map.json b/frontend/src/locales/en/map.json index 8cc1af83..f836798b 100644 --- a/frontend/src/locales/en/map.json +++ b/frontend/src/locales/en/map.json @@ -41,6 +41,7 @@ "linkCopyFailed": "Failed to copy link", "loadFiltersError": "Failed to load filters.", "loadSuggestFormError": "Failed to load the suggestion form.", + "loadReportFormError": "Failed to load the report form.", "retry": "Retry", "clearFilters": "Clear filters", "clearAllFiltersAriaLabel": "Clear all filters", diff --git a/frontend/src/locales/pl/map.json b/frontend/src/locales/pl/map.json index 6980e5ce..f6f8a2a4 100644 --- a/frontend/src/locales/pl/map.json +++ b/frontend/src/locales/pl/map.json @@ -41,6 +41,7 @@ "linkCopyFailed": "Nie udało się skopiować linku", "loadFiltersError": "Nie udało się załadować filtrów.", "loadSuggestFormError": "Nie udało się załadować formularza propozycji.", + "loadReportFormError": "Nie udało się załadować formularza zgłoszenia.", "retry": "Spróbuj ponownie", "clearFilters": "Wyczyść filtry", "clearAllFiltersAriaLabel": "Wyczyść wszystkie filtry", diff --git a/frontend/src/locales/ua/map.json b/frontend/src/locales/ua/map.json index 9d1b5597..b5f9356c 100644 --- a/frontend/src/locales/ua/map.json +++ b/frontend/src/locales/ua/map.json @@ -40,6 +40,7 @@ "linkCopyFailed": "Не вдалося скопіювати посилання", "loadFiltersError": "Не вдалося завантажити фільтри.", "loadSuggestFormError": "Не вдалося завантажити форму пропозиції.", + "loadReportFormError": "Не вдалося завантажити форму скарги.", "retry": "Спробувати ще раз", "clearFilters": "Очистити фільтри", "clearAllFiltersAriaLabel": "Очистити всі фільтри", diff --git a/frontend/tests/MarkerPopup/ReportProblemForm.test.jsx b/frontend/tests/MarkerPopup/ReportProblemForm.test.jsx index ed7face9..32eda6ec 100644 --- a/frontend/tests/MarkerPopup/ReportProblemForm.test.jsx +++ b/frontend/tests/MarkerPopup/ReportProblemForm.test.jsx @@ -137,12 +137,26 @@ describe('ReportProblemForm', () => { // A schema that has not arrived is not the same as one declaring no issue types: the // legacy fallbacks are not a stand-in for values this deployment may not accept. -test('offers no issue types until the schema has loaded', () => { +test('renders nothing until the schema has loaded', () => { useDeploymentData.mockReturnValue({ locationSchema: null }); - const { select } = renderForm(); + const { container } = render(); - expect(select.disabled).toBe(true); - expect(select.querySelectorAll('option')).toHaveLength(1); - expect(select.textContent).not.toMatch(/not here|overload|broken/i); + expect(container.innerHTML).toBe(''); +}); + +// Withholding the form on a failed fetch would otherwise leave it never appearing at all. +test('offers a retry when the schema could not be loaded', () => { + const refetchLocationSchema = jest.fn(); + useDeploymentData.mockReturnValue({ + locationSchema: null, + schemaError: true, + refetchLocationSchema, + }); + + const { getByText, getByRole } = render(); + + expect(getByText(/failed to load the report form/i)).toBeTruthy(); + fireEvent.click(getByRole('button', { name: /retry/i })); + expect(refetchLocationSchema).toHaveBeenCalledTimes(1); }); From 81ed75305abb17d0ce29020fe25007491af34bdc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Krzysztof=20Ko=C5=82odzi=C5=84ski?= Date: Thu, 20 Aug 2026 00:26:45 +0200 Subject: [PATCH 18/18] more linting --- frontend/.eslintrc.json | 3 ++- frontend/src/components/MarkerPopup/ClusterMarker.jsx | 3 +++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/frontend/.eslintrc.json b/frontend/.eslintrc.json index 26e4203e..b9d45efa 100644 --- a/frontend/.eslintrc.json +++ b/frontend/.eslintrc.json @@ -4,7 +4,8 @@ "plugin:react/recommended", "plugin:prettier/recommended", "airbnb", - "prettier"], + "prettier", + "plugin:react-hooks/recommended"], "plugins": ["react", "prettier", "eslint-plugin-react", "eslint-plugin-react-hooks"], "parser": "@babel/eslint-parser", "parserOptions": { diff --git a/frontend/src/components/MarkerPopup/ClusterMarker.jsx b/frontend/src/components/MarkerPopup/ClusterMarker.jsx index d247c69a..bc0b112f 100644 --- a/frontend/src/components/MarkerPopup/ClusterMarker.jsx +++ b/frontend/src/components/MarkerPopup/ClusterMarker.jsx @@ -32,6 +32,9 @@ const ClusterMarker = ({ cluster }) => { iconSize: [30, 30], iconAnchor: [15, 15], }), + // ClusterMarkerIcon renders cluster_count alone, so depending on the whole + // cluster would rebuild the icon on every new object identity for nothing. + // eslint-disable-next-line react-hooks/exhaustive-deps [cluster.cluster_count], );