From 4f7983c89303c219e3957dcb65d87d595bc32dfd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Krzysztof=20Ko=C5=82odzi=C5=84ski?= Date: Mon, 27 Jul 2026 21:38:47 +0200 Subject: [PATCH 01/12] feat: added filter types --- docs/api.rst | 11 + docs/quickstart.rst | 56 ++++- e2e-tests/e2e_test_data_initial.json | 202 +++++++++++++++- .../tests/basic/test_accessibility_table.py | 8 +- e2e-tests/tests/basic/test_language.py | 32 +-- e2e-tests/tests/basic/test_map.py | 141 +++++++++--- e2e-tests/tests/basic/test_mobile_box.py | 36 +-- e2e-tests/tests/basic/test_popup.py | 67 +----- e2e-tests/tests/basic/test_share.py | 70 +----- e2e-tests/tests/conftest.py | 39 +++- .../translations/en/LC_MESSAGES/messages.po | 42 ++++ .../translations/pl/LC_MESSAGES/messages.po | 42 ++++ .../components/FiltersForm/FiltersForm.jsx | 99 +++++++- .../components/MarkerPopup/MarkerPopup.jsx | 8 + frontend/src/locales/en/map.json | 4 +- frontend/src/locales/pl/map.json | 4 +- frontend/src/locales/ua/map.json | 4 +- frontend/src/services/http/httpService.js | 6 +- frontend/tests/FiltersForm.test.jsx | 216 +++++++++++++++++- goodmap/core.py | 71 +++++- goodmap/core_api.py | 2 + goodmap/db.py | 44 +++- tests/unit_tests/test_core.py | 93 ++++++++ tests/unit_tests/test_core_api.py | 139 +++++++++++ tests/unit_tests/test_db.py | 16 +- 25 files changed, 1213 insertions(+), 239 deletions(-) diff --git a/docs/api.rst b/docs/api.rst index 054d942c..72564188 100644 --- a/docs/api.rst +++ b/docs/api.rst @@ -19,6 +19,17 @@ Database Module :undoc-members: :show-inheritance: +Filtering Module +---------------- + +Category filter combination logic (see :ref:`categories-filter-mode` in +:doc:`quickstart` for the config-level guide). + +.. automodule:: goodmap.core + :members: + :undoc-members: + :show-inheritance: + API Endpoints ------------- diff --git a/docs/quickstart.rst b/docs/quickstart.rst index 3fa7f54e..c8ce211a 100644 --- a/docs/quickstart.rst +++ b/docs/quickstart.rst @@ -70,6 +70,8 @@ Example configuration in your data source: ] } +.. _categories-filter-mode: + Categories and Filtering ~~~~~~~~~~~~~~~~~~~~~~~~ @@ -92,14 +94,54 @@ checkboxes, one per allowed value. pre-checked in the filter panel when the app first loads, before the user has made any selection. -Example configuration in your data source: +``categories_filter_mode`` + Dict of category key to how *multiple selected values within that + category* are combined when filtering locations. Categories not listed + here default to ``"or"``. This only affects combination **within** one + category - across different categories, selections are always combined + with AND (a location must match every category that has an active + selection). + + ``"or"`` (default) + A location matches if it has **any** of the selected values. This is + the usual "check more boxes to broaden results" behavior - e.g. + checking both ``bikes`` and ``cars`` on ``accessible_by`` shows + locations that allow bikes *or* cars, not only locations that allow + both (which would often be zero results). + + ``"exclusive"`` + Single-select: the frontend renders the options as radio buttons + instead of checkboxes, so only one value can be active at a time. Use + this for categories with three or more mutually-exclusive states, + e.g. a toll tier: ``free`` / ``discounted`` / ``full_price``. + + ``"boolean"`` + For a field with exactly the two values ``"true"`` and ``"false"``. + Only the ``"true"`` option is rendered, as a single checkbox; leaving + it unchecked already means "show everything" (both true and false + locations), so there's no separate control for isolating ``"false"`` + alone. Use this when nobody would deliberately filter for the + negative case - e.g. a "free only" checkbox for an ``is_free`` field, + since drivers care about "free" or "all", not "paid only". + + ``"threshold"`` + For an ordered, numeric-valued category, e.g. a speed limit in km/h. + Selecting a value matches any location whose value is **at or below** + the highest selected value - e.g. selecting ``30`` also matches + locations with ``10`` or ``30``, but not ``50``. The frontend renders + this as radio buttons too, since selecting more than one option would + be redundant: the highest selection alone determines the cutoff. + +Example configuration combining all four modes: .. code-block:: json { "categories": { "accessible_by": ["bikes", "cars", "pedestrians"], - "type_of_place": ["big bridge", "small bridge"] + "type_of_place": ["big bridge", "small bridge"], + "is_free": ["true", "false"], + "speed_limit": ["10", "30", "50"] }, "categories_help": ["accessible_by"], "categories_options_help": { @@ -107,9 +149,19 @@ Example configuration in your data source: }, "categories_default_checked": { "accessible_by": ["cars"] + }, + "categories_filter_mode": { + "accessible_by": "or", + "type_of_place": "or", + "is_free": "boolean", + "speed_limit": "threshold" } } +The active mode for each category is also exposed as ``filter_mode`` in the +``/api/categories-full`` response, so a custom frontend can render the +right control (checkbox vs. radio) without hardcoding category names. + .. _data-model-visible_data: Database Types diff --git a/e2e-tests/e2e_test_data_initial.json b/e2e-tests/e2e_test_data_initial.json index 37eead36..2af45210 100644 --- a/e2e-tests/e2e_test_data_initial.json +++ b/e2e-tests/e2e_test_data_initial.json @@ -4,7 +4,7 @@ { "name": "Grunwaldzki", "position": [ - 51.1095, + 51.109444, 17.0525 ], "accessible_by": [ @@ -12,6 +12,12 @@ "cars" ], "type_of_place": "big bridge", + "is_free": "true", + "speed_limit": "50", + "amenities": [ + "lighting", + "benches" + ], "uuid": "9264286a-5d33-4e38-ab11-c8e179a7754a", "CTA": { "type": "CTA", @@ -22,16 +28,166 @@ { "name": "Zwierzyniecka", "position": [ - 51.10655, - 17.0555 + 51.108056, + 17.07 ], "accessible_by": [ "bikes", "pedestrians" ], "type_of_place": "small bridge", + "is_free": "true", + "speed_limit": "10", + "amenities": [ + "lighting" + ], "uuid": "c8ecf476-5968-40da-ba5c-e810ad9ff203", "remark": "very old bridge" + }, + { + "name": "Milenijny", + "position": [ + 51.133692, + 16.993103 + ], + "accessible_by": [ + "cars" + ], + "type_of_place": "big bridge", + "is_free": "false", + "speed_limit": "30", + "amenities": [], + "uuid": "1a8f9a2e-4b6d-4a1a-9a8e-2f6c7d0b3e9a", + "remark": "toll bridge, cars only" + }, + { + "name": "Pokoju", + "position": [ + 51.111739, + 17.049236 + ], + "accessible_by": [ + "cars", + "pedestrians" + ], + "type_of_place": "big bridge", + "is_free": "true", + "speed_limit": "50", + "amenities": [ + "lighting", + "benches", + "toilets" + ], + "uuid": "f03afca0-4ef8-45bf-a176-dd477ce48d92" + }, + { + "name": "Tumski", + "position": [ + 51.114708, + 17.042319 + ], + "accessible_by": [ + "pedestrians" + ], + "type_of_place": "small bridge", + "is_free": "true", + "speed_limit": "10", + "amenities": [ + "lighting", + "benches" + ], + "uuid": "e8b8a5b2-91e1-49f9-821b-5c7fa9eceb4b", + "remark": "connects the cathedral island" + }, + { + "name": "Piaskowy", + "position": [ + 51.113542, + 17.039806 + ], + "accessible_by": [ + "pedestrians" + ], + "type_of_place": "small bridge", + "is_free": "true", + "speed_limit": "10", + "amenities": [ + "benches" + ], + "uuid": "5986e755-1eaa-4121-a01c-4fef1d5d1da1" + }, + { + "name": "Uniwersytecki", + "position": [ + 51.11525, + 17.033694 + ], + "accessible_by": [ + "pedestrians" + ], + "type_of_place": "small bridge", + "is_free": "true", + "speed_limit": "30", + "amenities": [ + "lighting" + ], + "uuid": "043dd6ed-1b70-40a1-a7ad-dbfa3424ed8e" + }, + { + "name": "Redzinski", + "position": [ + 51.155556, + 16.958889 + ], + "accessible_by": [ + "cars" + ], + "type_of_place": "big bridge", + "is_free": "false", + "speed_limit": "50", + "amenities": [ + "lighting" + ], + "uuid": "66847536-4ffb-452d-b03d-874831abb233", + "remark": "highway bridge, cars only" + }, + { + "name": "Osobowicki", + "position": [ + 51.131389, + 17.027778 + ], + "accessible_by": [ + "cars", + "pedestrians" + ], + "type_of_place": "big bridge", + "is_free": "true", + "speed_limit": "30", + "amenities": [ + "lighting", + "benches" + ], + "uuid": "1d7ff5c0-ea4d-4701-bf53-49295333e176" + }, + { + "name": "Warszawski", + "position": [ + 51.130256, + 17.059375 + ], + "accessible_by": [ + "cars", + "pedestrians" + ], + "type_of_place": "big bridge", + "is_free": "true", + "speed_limit": "50", + "amenities": [ + "lighting", + "toilets" + ], + "uuid": "e77cac7f-78ba-4a6d-8d99-8018c5057de6" } ], "location_obligatory_fields": [ @@ -46,6 +202,18 @@ [ "type_of_place", "str" + ], + [ + "is_free", + "str" + ], + [ + "speed_limit", + "str" + ], + [ + "amenities", + "list" ] ], "reported_issue_types": [ @@ -62,10 +230,33 @@ "type_of_place": [ "big bridge", "small bridge" + ], + "is_free": [ + "true", + "false" + ], + "speed_limit": [ + "10", + "30", + "50" + ], + "amenities": [ + "lighting", + "benches", + "toilets" ] }, + "categories_filter_mode": { + "accessible_by": "or", + "type_of_place": "or", + "is_free": "boolean", + "speed_limit": "threshold", + "amenities": "and" + }, "categories_help": [ - "accessible_by" + "accessible_by", + "speed_limit", + "amenities" ], "categories_options_help": { "type_of_place": [ @@ -85,6 +276,9 @@ "remark", "accessible_by", "type_of_place", + "is_free", + "speed_limit", + "amenities", "CTA" ], "meta_data": [ diff --git a/e2e-tests/tests/basic/test_accessibility_table.py b/e2e-tests/tests/basic/test_accessibility_table.py index 0c9255ea..24f147a0 100644 --- a/e2e-tests/tests/basic/test_accessibility_table.py +++ b/e2e-tests/tests/basic/test_accessibility_table.py @@ -28,8 +28,8 @@ def setup(self, page: Page, geolocation): # "accessible_by: cars" is checked by default (see categories_default_checked # in the test data), which excludes Zwierzyniecka (bikes/pedestrians only, - # no cars). Uncheck it so both seeded locations are visible, matching what - # these tests assert. + # no cars). Uncheck it so all ten seeded locations are visible, matching + # what these tests assert. page.wait_for_selector("#filter-form", timeout=MARKER_LOAD_TIMEOUT) page.locator("#filter-form input#cars").uncheck() @@ -51,10 +51,10 @@ def setup(self, page: Page, geolocation): def test_should_properly_display_places(self, page: Page): """ Verify table displays correct number of rows. - Should have 1 header row + 2 data rows = 3 total rows. + Should have 1 header row + 10 data rows = 11 total rows. """ rows = page.locator("tr") - expect(rows).to_have_count(3) + expect(rows).to_have_count(11) def test_zwierzyniecka_should_be_first_row(self, page: Page): """ diff --git a/e2e-tests/tests/basic/test_language.py b/e2e-tests/tests/basic/test_language.py index f536a4c6..a5b5b54e 100644 --- a/e2e-tests/tests/basic/test_language.py +++ b/e2e-tests/tests/basic/test_language.py @@ -10,7 +10,7 @@ import pytest from playwright.sync_api import Page, expect -from tests.conftest import BASE_URL, MARKER_LOAD_TIMEOUT, clear_all_checkboxes +from tests.conftest import BASE_URL, open_zwierzyniecka_popup def get_language_button(page: Page): @@ -71,35 +71,7 @@ def test_switch_to_polish_changes_popup_text(self, page: Page): page.get_by_role("link", name="polski").click() page.wait_for_load_state("domcontentloaded") - clear_all_checkboxes(page) - - # Click marker cluster to expand - page.locator(".leaflet-marker-icon").first.click() - - # Wait for markers to appear - markers = page.locator(".leaflet-marker-icon") - expect(markers).to_have_count(2, timeout=MARKER_LOAD_TIMEOUT) - - # Click rightmost marker to open popup - page.evaluate(""" - () => { - const markers = document.querySelectorAll('.leaflet-marker-icon'); - let rightmostMarker = null; - let maxX = -Infinity; - - markers.forEach(marker => { - const rect = marker.getBoundingClientRect(); - if (rect.x > maxX) { - maxX = rect.x; - rightmostMarker = marker; - } - }); - - if (rightmostMarker) { - rightmostMarker.click(); - } - } - """) + open_zwierzyniecka_popup(page) # Verify popup is visible popup = page.locator(".leaflet-popup-content") diff --git a/e2e-tests/tests/basic/test_map.py b/e2e-tests/tests/basic/test_map.py index 5a084286..825943f8 100644 --- a/e2e-tests/tests/basic/test_map.py +++ b/e2e-tests/tests/basic/test_map.py @@ -7,27 +7,60 @@ import pytest from playwright.sync_api import Page, expect -from tests.conftest import BASE_URL +from tests.conftest import BASE_URL, TABLE_LOAD_TIMEOUT, TEST_LOCATIONS class TestMap: """Test suite for map functionality""" @pytest.fixture(autouse=True) - def setup(self, page: Page): - """Navigate to home page before each test""" + def setup(self, page: Page, geolocation): + """Navigate to home page before each test. + + Also grants geolocation (set to WROCLAW_CENTER), since several tests + below use the List View table to verify filter results - it's a + clustering-independent read of exactly what's currently filtered in, + unlike counting `.leaflet-marker-icon` elements on the map, which + depends on Leaflet.markercluster's zoom-dependent grouping and isn't a + reliable way to assert result counts once bridges are spread across + realistic real-world distances (a single cluster can require multiple + zoom-in clicks to fully expand, rather than one click). + """ + location = TEST_LOCATIONS["WROCLAW_CENTER"] + geolocation(location["lat"], location["lon"]) page.goto(BASE_URL, wait_until="domcontentloaded") return - def test_displays_filter_list_with_two_categories_with_5_items(self, page: Page): - """Verify filter list has correct number of checkboxes and category groups""" - # Check number of checkboxes (5 filter options) + def _open_list_view(self, page: Page): + """Switch to List View and return the results table locator.""" + list_view_button = page.locator('button[id="listViewButton"]') + expect(list_view_button).to_be_visible(timeout=5000) + list_view_button.click() + + table = page.locator("table") + expect(table).to_be_visible(timeout=TABLE_LOAD_TIMEOUT) + return table + + def test_displays_filter_list_with_four_categories(self, page: Page): + """Verify filter list has correct number of checkboxes/radios and category groups""" + # accessible_by (3) + type_of_place (2) = 5 "or" checkboxes, plus + # is_free ("boolean") contributes 1 more checkbox (only its "true" + # option is rendered; "false" is hidden - see FiltersForm.jsx). checkboxes = page.get_by_role("checkbox") - expect(checkboxes).to_have_count(5) + expect(checkboxes).to_have_count(6) - # Check that both category groups are present (using translated names) + # speed_limit ("threshold") is single-select, rendered as radios. + radios = page.get_by_role("radio") + expect(radios).to_have_count(3) + + # Check that all category groups are present (using translated names). + # is_free doesn't get its own header - it's grouped into "Others" + # (see FiltersForm.jsx), labeled with its own translated name. expect(page.get_by_text("accessible by")).to_be_visible() expect(page.get_by_text("type of place")).to_be_visible() + expect(page.get_by_text("speed limit")).to_be_visible() + expect(page.get_by_text("Others")).to_be_visible() + expect(page.get_by_text("Free only")).to_be_visible() def test_should_not_have_scrollbars(self, page: Page): """Verify the page has no horizontal or vertical scrollbars""" @@ -57,34 +90,90 @@ def test_should_not_have_scrollbars(self, page: Page): ) def test_filter_checkbox_filters_markers(self, page: Page): - """Verify clicking filter checkbox actually filters the markers on the map""" - # On desktop, filter panel is already visible (no toggle needed). + """Verify clicking filter checkbox actually filters the results""" # "accessible_by: cars" is checked by default (see categories_default_checked - # in the test data), so start by clearing it to see both seeded locations. + # in the test data), so start by clearing it to see all ten seeded locations. cars_checkbox = page.get_by_role("checkbox", name="cars", exact=False) expect(cars_checkbox).to_be_checked() cars_checkbox.click() - # Wait for markers to load - first_marker = page.locator(".leaflet-marker-icon").first - expect(first_marker).to_be_visible(timeout=5000) + table = self._open_list_view(page) + rows = table.locator("tr") + # 1 header + 10 data rows + expect(rows).to_have_count(11) - # Click marker cluster to expand it - first_marker.click() + # Re-check the "cars" filter checkbox - this should filter to only show + # the 6 bridges accessible by cars (1 header + 6 data rows) + cars_checkbox.click() + expect(rows).to_have_count(7) - # Wait for markers to expand - should be 2 markers after expansion - markers = page.locator(".leaflet-marker-icon") - expect(markers).to_have_count(2) + # Uncheck to restore all results + cars_checkbox.click() + expect(rows).to_have_count(11) - # Re-check the "cars" filter checkbox - this should filter to only show - # places accessible by cars (1 marker instead of 2) + def test_or_filter_within_category_broadens_results(self, page: Page): + """Selecting multiple checkboxes within one category (accessible_by) should + return the union of matches (OR semantics), not only bridges that satisfy + every selected option at once (which would incorrectly return nothing here, + since no bridge allows both bikes and cars).""" + cars_checkbox = page.get_by_role("checkbox", name="cars", exact=False) + expect(cars_checkbox).to_be_checked() cars_checkbox.click() - # After filtering, only 1 marker should be visible (the one accessible by cars) - expect(markers).to_have_count(1) + bikes_checkbox = page.get_by_role("checkbox", name="bikes", exact=False) + + table = self._open_list_view(page) + rows = table.locator("tr") + expect(rows).to_have_count(11) + + # bikes alone -> only Zwierzyniecka (1 header + 1 data row) + bikes_checkbox.click() + expect(rows).to_have_count(2) - # Uncheck to restore all markers + # bikes OR cars -> union of both (1 + 6, no overlap since no bridge + # allows both bikes and cars): 1 header + 7 data rows cars_checkbox.click() + expect(rows).to_have_count(8) + + def test_is_free_boolean_filter_toggles_free_only(self, page: Page): + """is_free is a "boolean" filter: a single checkbox for "free only". + Unchecked shows both free and paid bridges (drivers care about "free" + or "all", not "paid only", so there's no separate option for that); + checking it narrows down to free bridges.""" + cars_checkbox = page.get_by_role("checkbox", name="cars", exact=False) + cars_checkbox.click() + + free_checkbox = page.get_by_role("checkbox", name="Free only", exact=False) + expect(free_checkbox).not_to_be_checked() + + table = self._open_list_view(page) + rows = table.locator("tr") + expect(rows).to_have_count(11) + + free_checkbox.click() + expect(free_checkbox).to_be_checked() + # 1 header + 8 free bridges + expect(rows).to_have_count(9) + + # Unchecking goes back to showing both free and paid bridges. + free_checkbox.click() + expect(free_checkbox).not_to_be_checked() + expect(rows).to_have_count(11) + + def test_speed_limit_threshold_filter_includes_lower_values(self, page: Page): + """Selecting a speed limit should also match bridges with a lower limit + (cumulative/threshold semantics), not only an exact match. speed_limit is + single-select (radio), since picking "30" already implies "30 or lower".""" + cars_checkbox = page.get_by_role("checkbox", name="cars", exact=False) + cars_checkbox.click() + + speed_30_radio = page.get_by_role("radio", name="30 km/h", exact=False) + + table = self._open_list_view(page) + rows = table.locator("tr") + expect(rows).to_have_count(11) - # Both markers should be visible again - expect(markers).to_have_count(2) + # 30 km/h also matches the three 10 km/h bridges, but not the 50 km/h + # ones: 1 header + 6 data rows + speed_30_radio.click() + expect(rows).to_have_count(7) diff --git a/e2e-tests/tests/basic/test_mobile_box.py b/e2e-tests/tests/basic/test_mobile_box.py index 9c568efc..2d8cf237 100644 --- a/e2e-tests/tests/basic/test_mobile_box.py +++ b/e2e-tests/tests/basic/test_mobile_box.py @@ -9,7 +9,7 @@ import pytest from playwright.sync_api import Page, expect -from tests.conftest import ALL_MOBILE_DEVICES, BASE_URL, clear_all_checkboxes +from tests.conftest import ALL_MOBILE_DEVICES, BASE_URL, open_zwierzyniecka_popup from tests.helpers import EXPECTED_PLACE_ZWIERZYNIECKA, verify_popup_content, verify_problem_form @@ -32,37 +32,9 @@ def test_displays_title_and_subtitle_in_popup( # Navigate to the page (device emulation already configured by mobile_page fixture) mobile_page.goto(BASE_URL, wait_until="domcontentloaded") - clear_all_checkboxes(mobile_page) - - # Click first marker to expand cluster - # Use JavaScript click to bypass webpack overlay that may intercept clicks on CI - first_marker = mobile_page.locator(".leaflet-marker-icon").first - first_marker.evaluate("el => el.click()") - - # Wait for markers to appear (should be 2 after expansion) - markers = mobile_page.locator(".leaflet-marker-icon") - expect(markers).to_have_count(2) - - # Click the rightmost marker - mobile_page.evaluate(""" - () => { - const markers = document.querySelectorAll('.leaflet-marker-icon'); - let rightmostMarker = null; - let maxX = -Infinity; - - markers.forEach(marker => { - const rect = marker.getBoundingClientRect(); - if (rect.x > maxX) { - maxX = rect.x; - rightmostMarker = marker; - } - }); - - if (rightmostMarker) { - rightmostMarker.click(); - } - } - """) + # Isolate Zwierzyniecka's marker and click it directly, rather than + # expanding a multi-marker cluster and guessing at its layout. + open_zwierzyniecka_popup(mobile_page) # On mobile, popup appears as Material-UI Dialog (bottom sheet) dialog_content = mobile_page.locator(".MuiDialogContent-root") diff --git a/e2e-tests/tests/basic/test_popup.py b/e2e-tests/tests/basic/test_popup.py index cafb4b0f..ba6258bc 100644 --- a/e2e-tests/tests/basic/test_popup.py +++ b/e2e-tests/tests/basic/test_popup.py @@ -6,7 +6,7 @@ from playwright.sync_api import Page, expect -from tests.conftest import BASE_URL, MARKER_LOAD_TIMEOUT, clear_all_checkboxes +from tests.conftest import BASE_URL, open_zwierzyniecka_popup from tests.helpers import EXPECTED_PLACE_ZWIERZYNIECKA, verify_popup_content, verify_problem_form @@ -22,37 +22,9 @@ def test_displays_popup_title_subtitle_categories_and_cta(self, page: Page, wind """ page.goto(BASE_URL, wait_until="domcontentloaded") - clear_all_checkboxes(page) - - # Click first marker to trigger cluster expansion - first_marker = page.locator(".leaflet-marker-icon").first - first_marker.click() - - # Wait for markers to appear (should be 2 after cluster expansion) - markers = page.locator(".leaflet-marker-icon") - expect(markers).to_have_count(2, timeout=MARKER_LOAD_TIMEOUT) - - # Click the rightmost marker - # Note: Using evaluate to find rightmost marker since we don't have data-testid - page.evaluate(""" - () => { - const markers = document.querySelectorAll('.leaflet-marker-icon'); - let rightmostMarker = null; - let maxX = -Infinity; - - markers.forEach(marker => { - const rect = marker.getBoundingClientRect(); - if (rect.x > maxX) { - maxX = rect.x; - rightmostMarker = marker; - } - }); - - if (rightmostMarker) { - rightmostMarker.click(); - } - } - """) + # Isolate Zwierzyniecka's marker and click it directly, rather than + # expanding a multi-marker cluster and guessing at its layout. + open_zwierzyniecka_popup(page) # Verify popup content popup = page.locator(".leaflet-popup-content") @@ -80,36 +52,7 @@ def test_problem_form_on_desktop(self, page: Page, window_open_stub): """ page.goto(BASE_URL, wait_until="domcontentloaded") - clear_all_checkboxes(page) - - # Click first marker to trigger cluster expansion - first_marker = page.locator(".leaflet-marker-icon").first - first_marker.click() - - # Wait for markers to appear (should be 2 after cluster expansion) - markers = page.locator(".leaflet-marker-icon") - expect(markers).to_have_count(2, timeout=MARKER_LOAD_TIMEOUT) - - # Click the rightmost marker - page.evaluate(""" - () => { - const markers = document.querySelectorAll('.leaflet-marker-icon'); - let rightmostMarker = null; - let maxX = -Infinity; - - markers.forEach(marker => { - const rect = marker.getBoundingClientRect(); - if (rect.x > maxX) { - maxX = rect.x; - rightmostMarker = marker; - } - }); - - if (rightmostMarker) { - rightmostMarker.click(); - } - } - """) + open_zwierzyniecka_popup(page) # Verify popup is visible popup = page.locator(".leaflet-popup-content") diff --git a/e2e-tests/tests/basic/test_share.py b/e2e-tests/tests/basic/test_share.py index 43911107..2ccdf43a 100644 --- a/e2e-tests/tests/basic/test_share.py +++ b/e2e-tests/tests/basic/test_share.py @@ -15,6 +15,7 @@ BASE_URL, MARKER_LOAD_TIMEOUT, clear_all_checkboxes, + open_zwierzyniecka_popup, ) @@ -31,36 +32,7 @@ def test_share_button_copies_link_to_clipboard(self, page: Page): # Grant clipboard permissions page.context.grant_permissions(["clipboard-read", "clipboard-write"]) - clear_all_checkboxes(page) - - # Click first marker to trigger cluster expansion - first_marker = page.locator(".leaflet-marker-icon").first - first_marker.click() - - # Wait for markers to appear (should be 2 after cluster expansion) - markers = page.locator(".leaflet-marker-icon") - expect(markers).to_have_count(2, timeout=MARKER_LOAD_TIMEOUT) - - # Click the rightmost marker - page.evaluate(""" - () => { - const markers = document.querySelectorAll('.leaflet-marker-icon'); - let rightmostMarker = null; - let maxX = -Infinity; - - markers.forEach(marker => { - const rect = marker.getBoundingClientRect(); - if (rect.x > maxX) { - maxX = rect.x; - rightmostMarker = marker; - } - }); - - if (rightmostMarker) { - rightmostMarker.click(); - } - } - """) + open_zwierzyniecka_popup(page) # Verify popup is visible popup = page.locator(".leaflet-popup-content") @@ -83,6 +55,13 @@ def test_shared_link_opens_popup_with_correct_content(self, page: Page): """ Verify navigating to a URL with ?locationId= auto-opens the popup with the correct location content. + + Note: this passes because Zwierzyniecka's seeded coordinates keep it + far enough from its nearest neighbor to render as a standalone marker + at the zoom level GoToLocation.jsx navigates to. There is a known, + separate app bug (see TODO in MarkerPopup.jsx) where this same flow + silently fails to open the popup if the target happens to be clustered + under the viewer's current filters/zoom - not exercised by this test. """ page.goto( f"{BASE_URL}/?locationId=c8ecf476-5968-40da-ba5c-e810ad9ff203", @@ -124,36 +103,7 @@ def test_share_button_triggers_native_share(self, mobile_page: Page, device_name mobile_page.goto(BASE_URL, wait_until="domcontentloaded") - clear_all_checkboxes(mobile_page) - - # Click first marker to expand cluster - first_marker = mobile_page.locator(".leaflet-marker-icon").first - first_marker.evaluate("el => el.click()") - - # Wait for markers to appear (should be 2 after expansion) - markers = mobile_page.locator(".leaflet-marker-icon") - expect(markers).to_have_count(2, timeout=MARKER_LOAD_TIMEOUT) - - # Click the rightmost marker - mobile_page.evaluate(""" - () => { - const markers = document.querySelectorAll('.leaflet-marker-icon'); - let rightmostMarker = null; - let maxX = -Infinity; - - markers.forEach(marker => { - const rect = marker.getBoundingClientRect(); - if (rect.x > maxX) { - maxX = rect.x; - rightmostMarker = marker; - } - }); - - if (rightmostMarker) { - rightmostMarker.click(); - } - } - """) + open_zwierzyniecka_popup(mobile_page) # On mobile, popup appears as Material-UI Dialog dialog_content = mobile_page.locator(".MuiDialogContent-root") diff --git a/e2e-tests/tests/conftest.py b/e2e-tests/tests/conftest.py index 544819b5..c5ed2682 100644 --- a/e2e-tests/tests/conftest.py +++ b/e2e-tests/tests/conftest.py @@ -13,7 +13,7 @@ from pathlib import Path import pytest -from playwright.sync_api import BrowserContext, Page +from playwright.sync_api import BrowserContext, Page, expect BASE_URL = "http://localhost:5000" @@ -66,7 +66,9 @@ "lon": 20.088, "tile_pattern": r"https://[abc]\.tile\.openstreetmap\.org/1[3-6]/\d+/\d+\.png", }, - "WROCLAW_CENTER": {"lat": 51.10655, "lon": 17.0555}, + # Matches Zwierzyniecka's seeded position exactly, so it's guaranteed to + # sort first by distance in tests that rely on that ordering. + "WROCLAW_CENTER": {"lat": 51.108056, "lon": 17.07}, } @@ -100,6 +102,39 @@ def clear_all_checkboxes(page: Page) -> None: page.locator('button[aria-label="Close left panel"]').evaluate("el => el.click()") +def open_zwierzyniecka_popup(page: Page) -> None: + """ + Filter down to exactly the Zwierzyniecka bridge and click its marker to + open its popup. + + Clicking into a multi-marker cluster spiderfies it into a layout whose + on-screen ordering isn't guaranteed to match geographic position + (especially on narrow mobile viewports) or to be stable as more + locations are seeded, so tests that need a specific, known location's + popup should isolate it first rather than reaching into an expanded + cluster by pixel position (e.g. "rightmost marker"). + + "bikes" uniquely identifies Zwierzyniecka among the seeded e2e bridges + (the only accessible_by option no other bridge has), so checking it + leaves exactly one marker on the map. + """ + toggle_button = page.locator('button[aria-label="Toggle left panel"]') + opened_dialog = toggle_button.is_visible() + if opened_dialog: + toggle_button.click() + + page.wait_for_selector("#filter-form", timeout=MARKER_LOAD_TIMEOUT) + page.locator("#clear-filters-button").click() + page.locator("#filter-form input#bikes").check() + + if opened_dialog: + page.locator('button[aria-label="Close left panel"]').evaluate("el => el.click()") + + markers = page.locator(".leaflet-marker-icon") + expect(markers).to_have_count(1, timeout=MARKER_LOAD_TIMEOUT) + markers.first.click() + + def _block_hmr(page: Page) -> None: """Block HMR/hot reload requests to prevent page refreshes during tests.""" page.route("**/ws", lambda route: route.abort()) diff --git a/e2e-tests/translations/en/LC_MESSAGES/messages.po b/e2e-tests/translations/en/LC_MESSAGES/messages.po index bcd185f6..a070c492 100644 --- a/e2e-tests/translations/en/LC_MESSAGES/messages.po +++ b/e2e-tests/translations/en/LC_MESSAGES/messages.po @@ -21,6 +21,15 @@ msgstr "accessible by" msgid "type_of_place" msgstr "type of place" +msgid "is_free" +msgstr "Free only" + +msgid "speed_limit" +msgstr "speed limit" + +msgid "amenities" +msgstr "amenities" + msgid "remark" msgstr "remark" @@ -41,6 +50,33 @@ msgstr "big bridge" msgid "small bridge" msgstr "small bridge" +# Category values - is_free +msgid "true" +msgstr "yes" + +msgid "false" +msgstr "no" + +# Category values - speed_limit (km/h, left untranslated on purpose) +msgid "10" +msgstr "10 km/h" + +msgid "30" +msgstr "30 km/h" + +msgid "50" +msgstr "50 km/h" + +# Category values - amenities +msgid "lighting" +msgstr "lighting" + +msgid "benches" +msgstr "benches" + +msgid "toilets" +msgstr "toilets" + # Reported issue types msgid "under construction" msgstr "under construction" @@ -52,6 +88,12 @@ msgstr "has a hole" msgid "categories_help_accessible_by" msgstr "Who can use this bridge" +msgid "categories_help_speed_limit" +msgstr "Selecting a speed limit also shows bridges with a lower limit" + +msgid "categories_help_amenities" +msgstr "Selecting several shows only bridges that have all of them" + # Help texts - category options msgid "categories_options_help_small bridge" msgstr "A smaller pedestrian or bike bridge" diff --git a/e2e-tests/translations/pl/LC_MESSAGES/messages.po b/e2e-tests/translations/pl/LC_MESSAGES/messages.po index c76a5fb1..ed6418c7 100644 --- a/e2e-tests/translations/pl/LC_MESSAGES/messages.po +++ b/e2e-tests/translations/pl/LC_MESSAGES/messages.po @@ -21,6 +21,15 @@ msgstr "dostępny dla" msgid "type_of_place" msgstr "typ miejsca" +msgid "is_free" +msgstr "tylko bezpłatne" + +msgid "speed_limit" +msgstr "ograniczenie prędkości" + +msgid "amenities" +msgstr "udogodnienia" + msgid "remark" msgstr "uwaga" @@ -41,6 +50,33 @@ msgstr "duży most" msgid "small bridge" msgstr "mały most" +# Category values - is_free +msgid "true" +msgstr "tak" + +msgid "false" +msgstr "nie" + +# Category values - speed_limit (km/h) +msgid "10" +msgstr "10 km/h" + +msgid "30" +msgstr "30 km/h" + +msgid "50" +msgstr "50 km/h" + +# Category values - amenities +msgid "lighting" +msgstr "oświetlenie" + +msgid "benches" +msgstr "ławki" + +msgid "toilets" +msgstr "toalety" + # Reported issue types msgid "under construction" msgstr "w budowie" @@ -52,6 +88,12 @@ msgstr "ma dziurę" msgid "categories_help_accessible_by" msgstr "Kto może korzystać z tego mostu" +msgid "categories_help_speed_limit" +msgstr "Wybranie limitu prędkości pokazuje też mosty z niższym limitem" + +msgid "categories_help_amenities" +msgstr "Wybranie kilku pokazuje tylko mosty, które mają je wszystkie" + # Help texts - category options msgid "categories_options_help_small bridge" msgstr "Mniejszy most dla pieszych lub rowerzystów" diff --git a/frontend/src/components/FiltersForm/FiltersForm.jsx b/frontend/src/components/FiltersForm/FiltersForm.jsx index 6255f04a..ff7feaaa 100644 --- a/frontend/src/components/FiltersForm/FiltersForm.jsx +++ b/frontend/src/components/FiltersForm/FiltersForm.jsx @@ -74,6 +74,16 @@ const FilterTitle = styled.span` line-height: 16px; `; +// Visually distinguishes "and" (match-all) categories from the default "or" +// (match-any) ones, the same way "exclusive"/"threshold" are distinguished by +// rendering as radio buttons instead of checkboxes. +const ModeHint = styled.span` + font-size: 11px; + font-weight: 400; + font-style: italic; + opacity: 0.65; +`; + const FilterOption = styled.label` display: flex; align-items: center; @@ -123,6 +133,21 @@ const StyledCheckbox = styled.input` outline: none; box-shadow: 0 0 0 2px rgba(79, 195, 247, 0.4); } + + &[type='radio'] { + border-radius: 50%; + } + + &[type='radio']:checked::after { + left: 4px; + top: 4px; + width: 6px; + height: 6px; + border: none; + border-radius: 50%; + background-color: white; + transform: none; + } `; const OptionText = styled.span` @@ -219,6 +244,18 @@ export const FiltersForm = () => { }); }; + // "exclusive" categories are single-select (radio buttons): picking one + // option replaces any previous selection instead of toggling it. + const handleRadioChange = event => { + const { value } = event.target; + const category = event.target.dataset.category; + + setCategories(prevSelectedFilters => ({ + ...prevSelectedFilters, + [category]: [value], + })); + }; + const handleClearFilters = () => { setCategories({}); }; @@ -249,6 +286,10 @@ export const FiltersForm = () => { }, []); const renderFilterOptions = (filters, category) => { + // "exclusive" (pick one) and "threshold" (pick a cumulative upper bound, + // e.g. speed limit) are both single-select: rendered as radio buttons so + // only one option can be active at a time. + const isSingleSelect = filters[4] === 'exclusive' || filters[4] === 'threshold'; return filters[1].map(([name, translation]) => { const tooltipData = globalThis.FEATURE_FLAGS?.CATEGORIES_HELP ? filters[3].find(it => it[name]) @@ -256,9 +297,10 @@ export const FiltersForm = () => { return ( { }); }; - const sections = categoriesData.map(filtersData => { + // "boolean" categories (e.g. "free only") have exactly one meaningful filter + // state - leaving them unchecked already means "show everything" - so rather + // than giving each its own titled section, they're grouped into one shared + // "Others" section as plain checkboxes labeled with the category's own name. + // This keeps the panel compact as more true/false-style filters are added. + const isBooleanCategory = filtersData => filtersData[4] === 'boolean'; + const booleanCategories = categoriesData.filter(isBooleanCategory); + const otherCategories = categoriesData.filter(f => !isBooleanCategory(f)); + + const renderBooleanFilterOption = filtersData => { + const [categoryKey, categoryName] = filtersData[0]; + const trueOption = filtersData[1].find(([name]) => name === 'true'); + if (!trueOption) { + return null; + } + const [name] = trueOption; + const tooltipData = globalThis.FEATURE_FLAGS?.CATEGORIES_HELP + ? filtersData[3].find(it => it[name]) + : ''; + return ( + + + {categoryName} + {tooltipData && ( + + + + )} + + ); + }; + + const sections = otherCategories.map(filtersData => { const [categoryKey, categoryName] = filtersData[0]; const sectionKey = `${categoryKey}-${categoryName}`; const categoryTooltip = globalThis.FEATURE_FLAGS?.CATEGORIES_HELP @@ -285,6 +366,7 @@ export const FiltersForm = () => { {categoryName} + {filtersData[4] === 'and' && {t('filterModeAndHint')}} {categoryTooltip && } {renderFilterOptions(filtersData, categoryKey)} @@ -292,6 +374,17 @@ export const FiltersForm = () => { ); }); + if (booleanCategories.length > 0) { + sections.push( + + + {t('otherFilters')} + + {booleanCategories.map(renderBooleanFilterOption)} + , + ); + } + if (isLoading) { return (
diff --git a/frontend/src/components/MarkerPopup/MarkerPopup.jsx b/frontend/src/components/MarkerPopup/MarkerPopup.jsx index 96d30ead..e0c5156f 100644 --- a/frontend/src/components/MarkerPopup/MarkerPopup.jsx +++ b/frontend/src/components/MarkerPopup/MarkerPopup.jsx @@ -89,6 +89,14 @@ export const MarkerPopup = ({ place }) => { const setSelectedLocationId = useMapStore(state => state.setSelectedLocationId); const [isClicked, setIsClicked] = useState(false); + // TODO: this only opens the popup if `place`'s Marker is actually attached to + // the map. Leaflet.markercluster detaches individual markers while they sit + // inside an unexpanded cluster bubble, so a shared ?locationId= link to a + // clustered location silently fails to open its popup on desktop (mobile's + // MobilePopup is unaffected - it's a state-driven MUI Dialog, not tied to the + // Leaflet marker). Fix: call the cluster group's zoomToShowLayer(marker, cb) + // before/instead of setIsClicked when the marker is clustered. Covered by the + // xfail'd test_shared_link_opens_popup_with_correct_content in e2e-tests. useEffect(() => { if (selectedLocationId === place.uuid) { setIsClicked(true); diff --git a/frontend/src/locales/en/map.json b/frontend/src/locales/en/map.json index 04752ff4..4fd653ac 100644 --- a/frontend/src/locales/en/map.json +++ b/frontend/src/locales/en/map.json @@ -38,5 +38,7 @@ "loadFiltersError": "Failed to load filters.", "retry": "Retry", "clearFilters": "Clear filters", - "clearAllFiltersAriaLabel": "Clear all filters" + "clearAllFiltersAriaLabel": "Clear all filters", + "otherFilters": "Others", + "filterModeAndHint": "(match all)" } diff --git a/frontend/src/locales/pl/map.json b/frontend/src/locales/pl/map.json index 69747f4c..42b90511 100644 --- a/frontend/src/locales/pl/map.json +++ b/frontend/src/locales/pl/map.json @@ -38,5 +38,7 @@ "loadFiltersError": "Nie udało się załadować filtrów.", "retry": "Spróbuj ponownie", "clearFilters": "Wyczyść filtry", - "clearAllFiltersAriaLabel": "Wyczyść wszystkie filtry" + "clearAllFiltersAriaLabel": "Wyczyść wszystkie filtry", + "otherFilters": "Inne", + "filterModeAndHint": "(wszystkie naraz)" } diff --git a/frontend/src/locales/ua/map.json b/frontend/src/locales/ua/map.json index bcadc554..000cd8b2 100644 --- a/frontend/src/locales/ua/map.json +++ b/frontend/src/locales/ua/map.json @@ -37,5 +37,7 @@ "loadFiltersError": "Не вдалося завантажити фільтри.", "retry": "Спробувати ще раз", "clearFilters": "Очистити фільтри", - "clearAllFiltersAriaLabel": "Очистити всі фільтри" + "clearAllFiltersAriaLabel": "Очистити всі фільтри", + "otherFilters": "Інше", + "filterModeAndHint": "(усі одразу)" } diff --git a/frontend/src/services/http/httpService.js b/frontend/src/services/http/httpService.js index 974f928c..19ffe1e5 100644 --- a/frontend/src/services/http/httpService.js +++ b/frontend/src/services/http/httpService.js @@ -54,10 +54,11 @@ export const httpService = { getCategoriesData: async () => { const response = await fetch(CATEGORIES_FULL).then(res => res.json()); - // Transform to expected format: [[key, name], options, help?, optionsHelp?] + // Transform to expected format: [[key, name], options, help?, optionsHelp?, filterMode] const categories = response.categories.map(category => { const categoryTuple = [category.key, category.name]; const options = category.options; + const filterMode = category.filter_mode ?? 'or'; if (globalThis.FEATURE_FLAGS?.CATEGORIES_HELP) { return [ @@ -65,9 +66,10 @@ export const httpService = { category.options_with_help ?? options, response.categories_help ?? [], category.options_help ?? [], + filterMode, ]; } - return [categoryTuple, options]; + return [categoryTuple, options, undefined, undefined, filterMode]; }); const defaultChecked = Object.fromEntries( diff --git a/frontend/tests/FiltersForm.test.jsx b/frontend/tests/FiltersForm.test.jsx index d983365e..d982fdea 100644 --- a/frontend/tests/FiltersForm.test.jsx +++ b/frontend/tests/FiltersForm.test.jsx @@ -1,6 +1,6 @@ import React from 'react'; import '@testing-library/jest-dom'; -import { render, waitFor, within } from '@testing-library/react'; +import { fireEvent, render, waitFor, within } from '@testing-library/react'; import { FiltersForm } from '../src/components/FiltersForm/FiltersForm'; import { CategoriesProvider } from '../src/components/Categories/CategoriesContext'; import { httpService } from '../src/services/http/httpService'; @@ -116,3 +116,217 @@ describe('Pre-checks options configured as default-checked', () => { expect(clothesCheckbox.checked).toBe(false); }); }); + +describe('Renders exclusive (single-select) categories as radio buttons', () => { + // "exclusive" is for categories with 3+ mutually-exclusive options, e.g. a + // hypothetical toll tier. Boolean yes/no categories use "boolean" mode + // instead (see below), which offers a single checkbox and no separate + // radio-deselection problem. + const exclusiveCategories = [ + [ + ['payment_status', 'payment status'], + [ + ['free', 'free'], + ['discounted', 'discounted'], + ['full_price', 'full price'], + ], + [], + [], + 'exclusive', + ], + ]; + + beforeEach(async () => { + httpService.getCategoriesData.mockResolvedValueOnce({ + categories: exclusiveCategories, + defaultChecked: {}, + }); + render( + + + , + ); + await waitFor(() => expect(document.querySelector('#free')).not.toBeNull()); + }); + + it('renders options as radio inputs sharing the category name', () => { + const freeInput = document.querySelector('#free'); + const discountedInput = document.querySelector('#discounted'); + expect(freeInput.type).toBe('radio'); + expect(discountedInput.type).toBe('radio'); + expect(freeInput.name).toBe('payment_status'); + expect(discountedInput.name).toBe('payment_status'); + }); + + it('selecting one option replaces rather than adds to the selection', () => { + const freeInput = document.querySelector('#free'); + const discountedInput = document.querySelector('#discounted'); + + fireEvent.click(freeInput); + expect(freeInput.checked).toBe(true); + + fireEvent.click(discountedInput); + expect(discountedInput.checked).toBe(true); + expect(freeInput.checked).toBe(false); + }); +}); + +describe('Groups boolean categories into a shared "Others" section', () => { + const mixedCategories = [ + [ + ['types', 'typy'], + [ + ['clothes', 'ciuchy'], + ['shoes', 'buty'], + ], + [], + [], + 'or', + ], + [ + ['is_free', 'Free only'], + [ + ['true', 'yes'], + ['false', 'no'], + ], + [], + [], + 'boolean', + ], + ]; + + beforeEach(async () => { + httpService.getCategoriesData.mockResolvedValueOnce({ + categories: mixedCategories, + defaultChecked: {}, + }); + render( + + + , + ); + await waitFor(() => expect(document.querySelector('#is_free')).not.toBeNull()); + }); + + it('keeps non-boolean categories in their own titled section', () => { + expect(document.querySelector('#filter-label-types-typy')).not.toBeNull(); + expect(document.querySelector('#clothes')).not.toBeNull(); + }); + + it('renders the boolean category as a single checkbox under "Others", labeled with the category name', () => { + expect(document.getElementById('filter-label-others').textContent).toBe('Others'); + + const freeCheckbox = document.querySelector('#is_free'); + expect(freeCheckbox.type).toBe('checkbox'); + expect(freeCheckbox.value).toBe('true'); + + const label = document.querySelector('label[for="is_free"]'); + expect(label.textContent.trim()).toBe('Free only'); + + // "false" is never rendered - unchecked already means "show everything". + expect(document.querySelector('#false')).toBeNull(); + expect(document.querySelector('#true')).toBeNull(); + }); + + it('unchecked by default (shows everything); checking narrows the results', () => { + const freeCheckbox = document.querySelector('#is_free'); + expect(freeCheckbox.checked).toBe(false); + + fireEvent.click(freeCheckbox); + expect(freeCheckbox.checked).toBe(true); + + fireEvent.click(freeCheckbox); + expect(freeCheckbox.checked).toBe(false); + }); +}); + +describe('Renders threshold categories as radio buttons too', () => { + const thresholdCategories = [ + [ + ['speed_limit', 'speed limit'], + [ + ['10', '10 km/h'], + ['30', '30 km/h'], + ['50', '50 km/h'], + ], + [], + [], + 'threshold', + ], + ]; + + beforeEach(async () => { + httpService.getCategoriesData.mockResolvedValueOnce({ + categories: thresholdCategories, + defaultChecked: {}, + }); + render( + + + , + ); + await waitFor(() => expect(document.getElementById('10')).not.toBeNull()); + }); + + it('renders a single-select radio group rather than independent checkboxes', () => { + const low = document.getElementById('10'); + const mid = document.getElementById('30'); + const high = document.getElementById('50'); + expect(low.type).toBe('radio'); + expect(mid.type).toBe('radio'); + expect(high.type).toBe('radio'); + expect(low.name).toBe('speed_limit'); + + fireEvent.click(mid); + expect(mid.checked).toBe(true); + + fireEvent.click(high); + expect(high.checked).toBe(true); + expect(mid.checked).toBe(false); + }); +}); + +describe('Distinguishes "and" categories with a visible hint, but keeps checkboxes', () => { + const andCategories = [ + [ + ['amenities', 'amenities'], + [ + ['lighting', 'lighting'], + ['benches', 'benches'], + ], + [], + [], + 'and', + ], + ]; + + beforeEach(async () => { + httpService.getCategoriesData.mockResolvedValueOnce({ + categories: andCategories, + defaultChecked: {}, + }); + render( + + + , + ); + await waitFor(() => expect(document.querySelector('#lighting')).not.toBeNull()); + }); + + it('still renders checkboxes (multi-select), unlike exclusive/threshold', () => { + const lighting = document.querySelector('#lighting'); + const benches = document.querySelector('#benches'); + expect(lighting.type).toBe('checkbox'); + expect(benches.type).toBe('checkbox'); + + fireEvent.click(lighting); + fireEvent.click(benches); + expect(lighting.checked).toBe(true); + expect(benches.checked).toBe(true); + }); + + it('shows a "match all" hint next to the category title', () => { + const header = document.querySelector('#filter-label-amenities-amenities').parentElement; + expect(header.textContent).toContain('(match all)'); + }); +}); diff --git a/goodmap/core.py b/goodmap/core.py index c3ef7baa..354a2826 100644 --- a/goodmap/core.py +++ b/goodmap/core.py @@ -5,21 +5,79 @@ # TODO move filtering to db site -def does_fulfill_requirement(entry, requirements): +def _as_list(value: Any) -> list: + """Wrap a scalar field value in a list, leaving list values untouched.""" + if value is None: + return [] + if isinstance(value, list): + return value + return [value] + + +def _matches_or(entry_values: list, selected_values: list) -> bool: + """Match if the entry has at least one of the selected values (any-of).""" + return any(value in entry_values for value in selected_values) + + +def _matches_and(entry_values: list, selected_values: list) -> bool: + """Match only if the entry has every selected value (all-of). + + Only meaningful for list-valued categories (an entry can have multiple + simultaneous values), e.g. narrowing down to locations that have both + "lighting" and "benches" among their amenities. For a single-valued + category this can only match when a single value is selected. + """ + return all(value in entry_values for value in selected_values) + + +def _matches_threshold(entry_values: list, selected_values: list) -> bool: + """Match if any entry value is numerically <= the highest selected value. + + Used for ordered numeric categories (e.g. speed limits) where selecting a + value implies "this value or lower" (e.g. selecting 50 also matches 10 and 30). + """ + try: + entry_numbers = [float(value) for value in entry_values] + max_selected = max(float(value) for value in selected_values) + except (TypeError, ValueError): + return False + return any(number <= max_selected for number in entry_numbers) + + +# "exclusive" (radio group) and "boolean" (single checkbox, e.g. "free only") +# categories only ever send a single selected value, so matching is the same +# as "or". +_FILTER_MATCHERS = { + "or": _matches_or, + "and": _matches_and, + "exclusive": _matches_or, + "boolean": _matches_or, + "threshold": _matches_threshold, +} + + +def does_fulfill_requirement(entry, requirements, filter_modes=None): """Check if an entry fulfills all category requirements. Args: entry: Location data entry to check requirements: List of (category, values) tuples to match + filter_modes: Optional dict mapping category name to combination mode + ("or", "and", "exclusive", "boolean", or "threshold"). Categories not + present default to "or" (entry matches if it has any of the + selected values). Returns: bool: True if entry matches all non-empty requirements """ + filter_modes = filter_modes or {} matches = [] for category, values in requirements: if not values: continue - matches.append(all(entry_value in entry[category] for entry_value in values)) + entry_values = _as_list(entry.get(category)) + matcher = _FILTER_MATCHERS.get(filter_modes.get(category, "or"), _matches_or) + matches.append(matcher(entry_values, values)) return all(matches) @@ -64,13 +122,16 @@ def limit(data, query_params): return data -def get_queried_data(all_data, categories, query_params): +def get_queried_data(all_data, categories, query_params, filter_modes=None): """Filter, sort, and limit location data based on query parameters. Args: all_data: Complete list of location data categories: Available categories for filtering query_params: Query parameters for filtering, sorting, and limiting + filter_modes: Optional dict mapping category name to combination mode + ("or", "and", "exclusive", "boolean", or "threshold"), see + does_fulfill_requirement. Returns: Filtered, sorted, and limited location data @@ -79,7 +140,9 @@ def get_queried_data(all_data, categories, query_params): for key in categories.keys(): requirements.append((key, query_params.get(key))) - filtered_data = [x for x in all_data if does_fulfill_requirement(x, requirements)] + filtered_data = [ + x for x in all_data if does_fulfill_requirement(x, requirements, filter_modes) + ] final_data = sort_by_distance(filtered_data, query_params) final_data = limit(final_data, query_params) return final_data diff --git a/goodmap/core_api.py b/goodmap/core_api.py index 13ddb360..c21fcfcf 100644 --- a/goodmap/core_api.py +++ b/goodmap/core_api.py @@ -441,6 +441,7 @@ def get_categories_full(): categories_options_help = categories_data.get("categories_options_help", {}) categories_default_checked = categories_data.get("categories_default_checked", {}) + categories_filter_mode = categories_data.get("categories_filter_mode", {}) for key, options in categories_data["categories"].items(): category_entry = { @@ -452,6 +453,7 @@ def get_categories_full(): for option in categories_default_checked.get(key, []) if option in options ], + "filter_mode": categories_filter_mode.get(key, "or"), } if CategoriesHelp in feature_flags: diff --git a/goodmap/db.py b/goodmap/db.py index 3b745381..199af7eb 100644 --- a/goodmap/db.py +++ b/goodmap/db.py @@ -613,12 +613,18 @@ def json_db_get_category_data(self, category_type=None): category_type, [] ) }, + "categories_filter_mode": { + category_type: self.data.get("categories_filter_mode", {}).get( + category_type, "or" + ) + }, } return { "categories": self.data["categories"], "categories_help": self.data.get("categories_help", []), "categories_options_help": self.data.get("categories_options_help", {}), "categories_default_checked": self.data.get("categories_default_checked", {}), + "categories_filter_mode": self.data.get("categories_filter_mode", {}), } @@ -636,12 +642,16 @@ def json_file_db_get_category_data(self, category_type=None): "categories_default_checked": { category_type: data.get("categories_default_checked", {}).get(category_type, []) }, + "categories_filter_mode": { + category_type: data.get("categories_filter_mode", {}).get(category_type, "or") + }, } return { "categories": data["categories"], "categories_help": data.get("categories_help", []), "categories_options_help": data.get("categories_options_help", {}), "categories_default_checked": data.get("categories_default_checked", {}), + "categories_filter_mode": data.get("categories_filter_mode", {}), } @@ -658,12 +668,16 @@ def google_json_db_get_category_data(self, category_type=None): "categories_default_checked": { category_type: data.get("categories_default_checked", {}).get(category_type, []) }, + "categories_filter_mode": { + category_type: data.get("categories_filter_mode", {}).get(category_type, "or") + }, } return { "categories": data.get("categories", {}), "categories_help": data.get("categories_help", []), "categories_options_help": data.get("categories_options_help", {}), "categories_default_checked": data.get("categories_default_checked", {}), + "categories_filter_mode": data.get("categories_filter_mode", {}), } @@ -687,18 +701,25 @@ def mongodb_db_get_category_data(self, category_type=None): category_type, [] ) }, + "categories_filter_mode": { + category_type: config_doc.get("categories_filter_mode", {}).get( + category_type, "or" + ) + }, } return { "categories": config_doc.get("categories", {}), "categories_help": config_doc.get("categories_help", []), "categories_options_help": config_doc.get("categories_options_help", {}), "categories_default_checked": config_doc.get("categories_default_checked", {}), + "categories_filter_mode": config_doc.get("categories_filter_mode", {}), } return { "categories": {}, "categories_help": [], "categories_options_help": {}, "categories_default_checked": {}, + "categories_filter_mode": {}, } @@ -769,7 +790,12 @@ def get_locations_list_from_raw_data(map_data, query, location_model): Returns: List of validated location model instances. """ - filtered_locations = get_queried_data(map_data["data"], map_data["categories"], query) + filtered_locations = get_queried_data( + map_data["data"], + map_data["categories"], + query, + map_data.get("categories_filter_mode", {}), + ) return [location_model.model_validate(point) for point in filtered_locations] @@ -791,9 +817,23 @@ def json_db_get_locations(self, query, location_model): def mongodb_db_get_locations(self, query, location_model): """Retrieve filtered locations from MongoDB.""" + config_doc = self.db.config.find_one({"_id": "map_config"}) or {} + filter_modes = config_doc.get("categories_filter_mode", {}) + mongo_query = {} for key, values in query.items(): - if values: + if not values: + continue + if filter_modes.get(key) == "threshold": + # Threshold categories (e.g. speed limits) are numeric and ordered: + # selecting a value also matches any stored value at or below it. + # Assumes the field is stored numerically in MongoDB. + try: + mongo_query[key] = {"$lte": max(float(value) for value in values)} + except (TypeError, ValueError): + continue + else: + # "or" and "exclusive" both match any of the selected values. mongo_query[key] = {"$in": values} projection = {"_id": 0, "uuid": 1, "position": 1, "remark": 1} diff --git a/tests/unit_tests/test_core.py b/tests/unit_tests/test_core.py index e5631470..16a003c1 100644 --- a/tests/unit_tests/test_core.py +++ b/tests/unit_tests/test_core.py @@ -65,6 +65,99 @@ def test_category_match_if_not_specified(): assert filtered_data == expected_data +def test_multiple_selected_values_in_same_category_are_or_by_default(): + """Selecting several checkboxes in one category should broaden results (any-of), + not narrow them to entries containing every selected value.""" + requirements = [("gender", ["male", "female"])] + filtered_data = list(filter(lambda x: does_fulfill_requirement(x, requirements), test_data)) + assert filtered_data == test_data + + +def test_or_mode_is_explicit_default(): + requirements = [("gender", ["female"])] + filter_modes = {"gender": "or"} + filtered_data = list( + filter(lambda x: does_fulfill_requirement(x, requirements, filter_modes), test_data) + ) + assert filtered_data == [test_data[0]] + + +def test_and_mode_requires_every_selected_value(): + """"and" narrows results: an entry must have ALL selected values, useful + for list-valued categories like amenities ("lighting" AND "benches").""" + requirements = [("gender", ["male", "female"])] + filter_modes = {"gender": "and"} + + filtered_data = list( + filter(lambda x: does_fulfill_requirement(x, requirements, filter_modes), test_data) + ) + + # Only LASSO has both "male" and "female"; PCK (["male"]) doesn't match. + assert filtered_data == [test_data[0]] + + +def test_and_mode_matches_a_single_selected_value_like_or(): + requirements = [("gender", ["male"])] + filter_modes = {"gender": "and"} + + filtered_data = list( + filter(lambda x: does_fulfill_requirement(x, requirements, filter_modes), test_data) + ) + + assert filtered_data == test_data + + +def test_exclusive_mode_matches_the_single_selected_value(): + requirements = [("gender", ["female"])] + filter_modes = {"gender": "exclusive"} + filtered_data = list( + filter(lambda x: does_fulfill_requirement(x, requirements, filter_modes), test_data) + ) + assert filtered_data == [test_data[0]] + + +def test_threshold_mode_matches_values_at_or_below_the_highest_selected(): + speed_data = [ + {"name": "Zwierzyniecka", "speed_limit": "10"}, + {"name": "Milenijny", "speed_limit": "30"}, + {"name": "Grunwaldzki", "speed_limit": "50"}, + ] + requirements = [("speed_limit", ["30"])] + filter_modes = {"speed_limit": "threshold"} + + filtered_data = list( + filter(lambda x: does_fulfill_requirement(x, requirements, filter_modes), speed_data) + ) + + assert filtered_data == [speed_data[0], speed_data[1]] + + +def test_threshold_mode_uses_the_max_of_multiple_selected_values(): + speed_data = [ + {"name": "Zwierzyniecka", "speed_limit": "10"}, + {"name": "Milenijny", "speed_limit": "30"}, + {"name": "Grunwaldzki", "speed_limit": "50"}, + ] + requirements = [("speed_limit", ["10", "50"])] + filter_modes = {"speed_limit": "threshold"} + + filtered_data = list( + filter(lambda x: does_fulfill_requirement(x, requirements, filter_modes), speed_data) + ) + + assert filtered_data == speed_data + + +def test_get_queried_data_applies_filter_modes(): + categories = {"gender": ["male", "female"]} + query = {"gender": ["female"]} + filter_modes = {"gender": "exclusive"} + + result = get_queried_data(test_data, categories, query, filter_modes) + + assert result == [test_data[0]] + + def test_that_limit_works_properly(): test_data_15_items = [ { diff --git a/tests/unit_tests/test_core_api.py b/tests/unit_tests/test_core_api.py index cd090fa4..10c55b5a 100644 --- a/tests/unit_tests/test_core_api.py +++ b/tests/unit_tests/test_core_api.py @@ -173,6 +173,23 @@ def test_categories_full_endpoint(test_app): # No default-checked options configured for this category assert category["default_checked"] == [] + # Categories without an explicit filter mode default to "or" + assert category["filter_mode"] == "or" + + +@mock.patch("goodmap.core_api.gettext", fake_translation) +def test_categories_full_endpoint_reports_configured_filter_mode(): + test_app = create_test_app( + db_overrides={ + "categories": {"test-category": ["opt1", "opt2"]}, + "categories_filter_mode": {"test-category": "exclusive"}, + } + ) + response = test_app.get("/api/categories-full") + assert response.status_code == 200 + category = response.json["categories"][0] + assert category["filter_mode"] == "exclusive" + @mock.patch("goodmap.core_api.gettext", fake_translation) def test_categories_full_endpoint_with_default_checked(): @@ -304,6 +321,128 @@ def test_get_locations(test_app): ] +def test_get_locations_multi_value_same_category_uses_or_semantics(): + """Selecting several checkboxes within one category should return the union + of matches, not only entries that have every selected value.""" + client = create_test_app( + db_overrides={ + "categories": {"tags": ["red", "blue", "green"]}, + "location_obligatory_fields": [("tags", "list"), ("name", "str")], + "data": [ + { + "name": "red-only", + "position": [50, 50], + "tags": ["red"], + "uuid": "11111111-1111-1111-1111-111111111111", + }, + { + "name": "blue-only", + "position": [60, 60], + "tags": ["blue"], + "uuid": "22222222-2222-2222-2222-222222222222", + }, + { + "name": "green-only", + "position": [70, 70], + "tags": ["green"], + "uuid": "33333333-3333-3333-3333-333333333333", + }, + ], + "visible_data": ["name", "tags"], + } + ) + + response = client.get("/api/locations?tags=red&tags=blue") + + assert response.status_code == 200 + uuids = {loc["uuid"] for loc in response.json} + assert uuids == { + "11111111-1111-1111-1111-111111111111", + "22222222-2222-2222-2222-222222222222", + } + + +def test_get_locations_and_filter_mode_requires_every_selected_value(): + """An "and" category (e.g. amenities) narrows to entries that have every + selected value, not just any of them - the opposite of "or".""" + client = create_test_app( + db_overrides={ + "categories": {"amenities": ["lighting", "benches", "toilets"]}, + "categories_filter_mode": {"amenities": "and"}, + "location_obligatory_fields": [("amenities", "list"), ("name", "str")], + "data": [ + { + "name": "lighting-and-benches", + "position": [50, 50], + "amenities": ["lighting", "benches"], + "uuid": "11111111-1111-1111-1111-111111111111", + }, + { + "name": "lighting-only", + "position": [60, 60], + "amenities": ["lighting"], + "uuid": "22222222-2222-2222-2222-222222222222", + }, + { + "name": "benches-only", + "position": [70, 70], + "amenities": ["benches"], + "uuid": "33333333-3333-3333-3333-333333333333", + }, + ], + "visible_data": ["name", "amenities"], + } + ) + + response = client.get("/api/locations?amenities=lighting&amenities=benches") + + assert response.status_code == 200 + uuids = {loc["uuid"] for loc in response.json} + assert uuids == {"11111111-1111-1111-1111-111111111111"} + + +def test_get_locations_threshold_filter_mode(): + """A "threshold" category (e.g. speed limit) matches any stored value at or + below the highest selected value.""" + client = create_test_app( + db_overrides={ + "categories": {"speed_limit": ["10", "30", "50"]}, + "categories_filter_mode": {"speed_limit": "threshold"}, + "location_obligatory_fields": [("speed_limit", "str"), ("name", "str")], + "data": [ + { + "name": "slow", + "position": [50, 50], + "speed_limit": "10", + "uuid": "11111111-1111-1111-1111-111111111111", + }, + { + "name": "medium", + "position": [60, 60], + "speed_limit": "30", + "uuid": "22222222-2222-2222-2222-222222222222", + }, + { + "name": "fast", + "position": [70, 70], + "speed_limit": "50", + "uuid": "33333333-3333-3333-3333-333333333333", + }, + ], + "visible_data": ["name", "speed_limit"], + } + ) + + response = client.get("/api/locations?speed_limit=30") + + assert response.status_code == 200 + uuids = {loc["uuid"] for loc in response.json} + assert uuids == { + "11111111-1111-1111-1111-111111111111", + "22222222-2222-2222-2222-222222222222", + } + + @mock.patch("goodmap.core_api.gettext", fake_translation) @mock.patch("goodmap.formatter.gettext", fake_translation) @mock.patch("flask_babel.gettext", fake_translation) diff --git a/tests/unit_tests/test_db.py b/tests/unit_tests/test_db.py index 6243166d..d71addd2 100644 --- a/tests/unit_tests/test_db.py +++ b/tests/unit_tests/test_db.py @@ -122,13 +122,15 @@ def initialize_and_assert_db(db, data): location_model = create_location_model(location_obligatory_fields, {}) extend_db_with_goodmap_queries(db, location_model) - query = {"test-category": "searchable"} + # Query values arrive as lists in production (request.args.to_dict(flat=False)); + # a single selected value exactly matches one entry's scalar category field. + query = {"test-category": ["searchable"]} location = db.get_location("1") assert location.position == (50, 50) assert location.uuid == "1" - assert len(db.get_locations(query)) == 2 + assert len(db.get_locations(query)) == 1 assert db.get_data() == data @@ -1424,6 +1426,7 @@ def test_json_db_get_category_data(): "categories_help": ["Help text for categories"], "categories_options_help": {"test-category": ["Help for test category"]}, "categories_default_checked": {"test-category": ["searchable"]}, + "categories_filter_mode": {}, } assert category_data == expected @@ -1437,6 +1440,7 @@ def test_json_db_get_category_data_specific_category(): "categories_help": ["Help text for categories"], "categories_options_help": {"test-category": ["Help for test category"]}, "categories_default_checked": {"test-category": ["searchable"]}, + "categories_filter_mode": {"test-category": "or"}, } assert category_data == expected @@ -1453,6 +1457,7 @@ def test_json_file_db_get_category_data(tmp_path): "categories_help": ["Help text for categories"], "categories_options_help": {"test-category": ["Help for test category"]}, "categories_default_checked": {"test-category": ["searchable"]}, + "categories_filter_mode": {}, } assert category_data == expected @@ -1469,6 +1474,7 @@ def test_json_file_db_get_category_data_specific_category(tmp_path): "categories_help": ["Help text for categories"], "categories_options_help": {"test-category": ["Help for test category"]}, "categories_default_checked": {"test-category": ["searchable"]}, + "categories_filter_mode": {"test-category": "or"}, } assert category_data == expected @@ -1486,6 +1492,7 @@ def test_google_json_db_get_category_data(mock_cli): "categories_help": ["Help text for categories"], "categories_options_help": {"test-category": ["Help for test category"]}, "categories_default_checked": {"test-category": ["searchable"]}, + "categories_filter_mode": {}, } assert category_data == expected @@ -1503,6 +1510,7 @@ def test_google_json_db_get_category_data_specific_category(mock_cli): "categories_help": ["Help text for categories"], "categories_options_help": {"test-category": ["Help for test category"]}, "categories_default_checked": {"test-category": ["searchable"]}, + "categories_filter_mode": {"test-category": "or"}, } assert category_data == expected @@ -1527,6 +1535,7 @@ def test_mongodb_db_get_category_data(mock_client): "categories_help": ["Help text for categories"], "categories_options_help": {"test-category": ["Help for test category"]}, "categories_default_checked": {"test-category": ["searchable"]}, + "categories_filter_mode": {}, } assert category_data == expected @@ -1551,6 +1560,7 @@ def test_mongodb_db_get_category_data_specific_category(mock_client): "categories_help": ["Help text for categories"], "categories_options_help": {"test-category": ["Help for test category"]}, "categories_default_checked": {"test-category": ["searchable"]}, + "categories_filter_mode": {"test-category": "or"}, } assert category_data == expected @@ -1569,6 +1579,7 @@ def test_mongodb_db_get_category_data_no_config(mock_client): "categories_help": [], "categories_options_help": {}, "categories_default_checked": {}, + "categories_filter_mode": {}, } assert category_data == expected @@ -1582,6 +1593,7 @@ def test_get_category_data(): "categories_help": ["Help text for categories"], "categories_options_help": {"test-category": ["Help for test category"]}, "categories_default_checked": {"test-category": ["searchable"]}, + "categories_filter_mode": {}, } assert category_data == expected From a2085df1c81f9d88002d5cd971aaaba7472f5d89 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Krzysztof=20Ko=C5=82odzi=C5=84ski?= Date: Mon, 27 Jul 2026 23:47:05 +0200 Subject: [PATCH 02/12] translations added --- docs/quickstart.rst | 19 +++++- e2e-tests/e2e_test_data_initial.json | 3 +- .../scripts/generate_stress_test_data.py | 1 - e2e-tests/tests/basic/test_map.py | 48 +++++++++++-- examples/e2e_test_data.json | 3 +- .../components/FiltersForm/FiltersForm.jsx | 68 ++++++++++++++++--- frontend/src/locales/en/map.json | 11 ++- frontend/src/locales/pl/map.json | 11 ++- frontend/src/locales/ua/map.json | 11 ++- frontend/tests/FiltersForm.test.jsx | 9 ++- 10 files changed, 157 insertions(+), 27 deletions(-) diff --git a/docs/quickstart.rst b/docs/quickstart.rst index c8ce211a..18b90ffa 100644 --- a/docs/quickstart.rst +++ b/docs/quickstart.rst @@ -109,6 +109,17 @@ checkboxes, one per allowed value. locations that allow bikes *or* cars, not only locations that allow both (which would often be zero results). + ``"and"`` + A location matches only if it has **every** one of the selected + values - narrowing rather than broadening. Only meaningful for + list-valued categories (a location can have several simultaneous + values); for a single-valued category it behaves like ``"or"`` + restricted to one selection at a time. Still rendered as checkboxes + (it's still multi-select), but with a "(match all)" hint next to the + category title so it reads differently from the default "or" + behavior - e.g. an ``amenities`` field where checking ``lighting`` + and ``benches`` should show only bridges that have both, not either. + ``"exclusive"`` Single-select: the frontend renders the options as radio buttons instead of checkboxes, so only one value can be active at a time. Use @@ -132,7 +143,7 @@ checkboxes, one per allowed value. this as radio buttons too, since selecting more than one option would be redundant: the highest selection alone determines the cutoff. -Example configuration combining all four modes: +Example configuration combining all five modes: .. code-block:: json @@ -141,7 +152,8 @@ Example configuration combining all four modes: "accessible_by": ["bikes", "cars", "pedestrians"], "type_of_place": ["big bridge", "small bridge"], "is_free": ["true", "false"], - "speed_limit": ["10", "30", "50"] + "speed_limit": ["10", "30", "50"], + "amenities": ["lighting", "benches", "toilets"] }, "categories_help": ["accessible_by"], "categories_options_help": { @@ -154,7 +166,8 @@ Example configuration combining all four modes: "accessible_by": "or", "type_of_place": "or", "is_free": "boolean", - "speed_limit": "threshold" + "speed_limit": "threshold", + "amenities": "and" } } diff --git a/e2e-tests/e2e_test_data_initial.json b/e2e-tests/e2e_test_data_initial.json index 2af45210..17e8b58d 100644 --- a/e2e-tests/e2e_test_data_initial.json +++ b/e2e-tests/e2e_test_data_initial.json @@ -347,7 +347,6 @@ "url": "https://fonts.googleapis.com/css2?family=Poppins" }, "primary_color": "#FFFFFF", - "secondary_color": "#245466", - "left_bar_width": "300px" + "secondary_color": "#245466" } } diff --git a/e2e-tests/scripts/generate_stress_test_data.py b/e2e-tests/scripts/generate_stress_test_data.py index 7bfbe8e7..0470d2c8 100644 --- a/e2e-tests/scripts/generate_stress_test_data.py +++ b/e2e-tests/scripts/generate_stress_test_data.py @@ -83,7 +83,6 @@ def main(): }, "primary_color": "#FFFFFF", "secondary_color": "#245466", - "left_bar_width": "300px", }, } diff --git a/e2e-tests/tests/basic/test_map.py b/e2e-tests/tests/basic/test_map.py index 825943f8..c0ae040d 100644 --- a/e2e-tests/tests/basic/test_map.py +++ b/e2e-tests/tests/basic/test_map.py @@ -43,11 +43,12 @@ def _open_list_view(self, page: Page): def test_displays_filter_list_with_four_categories(self, page: Page): """Verify filter list has correct number of checkboxes/radios and category groups""" - # accessible_by (3) + type_of_place (2) = 5 "or" checkboxes, plus - # is_free ("boolean") contributes 1 more checkbox (only its "true" - # option is rendered; "false" is hidden - see FiltersForm.jsx). + # accessible_by (3) + type_of_place (2) + amenities (3, "and" mode is + # still multi-select) = 8 "or"/"and" checkboxes, plus is_free + # ("boolean") contributes 1 more checkbox (only its "true" option is + # rendered; "false" is hidden - see FiltersForm.jsx). checkboxes = page.get_by_role("checkbox") - expect(checkboxes).to_have_count(6) + expect(checkboxes).to_have_count(9) # speed_limit ("threshold") is single-select, rendered as radios. radios = page.get_by_role("radio") @@ -62,6 +63,21 @@ def test_displays_filter_list_with_four_categories(self, page: Page): expect(page.get_by_text("Others")).to_be_visible() expect(page.get_by_text("Free only")).to_be_visible() + # Every category shows a small mode badge (a single character, with a + # tooltip on hover/focus explaining what it means) next to its title - + # or:+, and:&, threshold:≤, boolean:• - so "or" and "and" (both + # checkboxes, otherwise visually identical) are as distinguishable as + # exclusive/threshold already are via their radio shape. + expect(page.get_by_text("amenities")).to_be_visible() + + def mode_badge(symbol): + return page.get_by_label("Help:", exact=False).filter(has_text=symbol) + + expect(mode_badge("+")).to_have_count(2) # accessible_by, type_of_place + expect(mode_badge("&")).to_have_count(1) # amenities + expect(mode_badge("≤")).to_have_count(1) # speed_limit + expect(mode_badge("•")).to_have_count(1) # is_free + def test_should_not_have_scrollbars(self, page: Page): """Verify the page has no horizontal or vertical scrollbars""" # Get viewport and document dimensions @@ -177,3 +193,27 @@ def test_speed_limit_threshold_filter_includes_lower_values(self, page: Page): # ones: 1 header + 6 data rows speed_30_radio.click() expect(rows).to_have_count(7) + + def test_and_filter_within_category_narrows_results(self, page: Page): + """Selecting multiple checkboxes within an "and" category (amenities) + should narrow results to locations that have every selected value, + the opposite of the default "or" behavior.""" + cars_checkbox = page.get_by_role("checkbox", name="cars", exact=False) + expect(cars_checkbox).to_be_checked() + cars_checkbox.click() + + lighting_checkbox = page.get_by_role("checkbox", name="lighting", exact=False) + benches_checkbox = page.get_by_role("checkbox", name="benches", exact=False) + + table = self._open_list_view(page) + rows = table.locator("tr") + expect(rows).to_have_count(11) + + # lighting alone -> 8 bridges (1 header + 8 data rows) + lighting_checkbox.click() + expect(rows).to_have_count(9) + + # lighting AND benches -> only bridges with both (4), fewer than + # either alone (8 and 5) - the opposite of OR's broadening. + benches_checkbox.click() + expect(rows).to_have_count(5) diff --git a/examples/e2e_test_data.json b/examples/e2e_test_data.json index f435d0ff..60fc6126 100644 --- a/examples/e2e_test_data.json +++ b/examples/e2e_test_data.json @@ -149,7 +149,6 @@ "url": "https://fonts.googleapis.com/css2?family=Poppins" }, "primary_color": "#FFFFFF", - "secondary_color": "#245466", - "left_bar_width": "300px" + "secondary_color": "#245466" } } diff --git a/frontend/src/components/FiltersForm/FiltersForm.jsx b/frontend/src/components/FiltersForm/FiltersForm.jsx index ff7feaaa..fa4be680 100644 --- a/frontend/src/components/FiltersForm/FiltersForm.jsx +++ b/frontend/src/components/FiltersForm/FiltersForm.jsx @@ -1,6 +1,7 @@ import React, { useState, useEffect } from 'react'; import styled, { keyframes } from 'styled-components'; import { useTranslation } from 'react-i18next'; +import { Tooltip } from '@mui/material'; import { useCategories } from '../Categories/CategoriesContext'; import { httpService } from '../../services/http/httpService'; import FiltersTooltip from './FiltersTooltip'; @@ -68,20 +69,41 @@ const FilterHeader = styled.div` `; const FilterTitle = styled.span` + flex: 1; font-size: 13px; font-weight: 600; letter-spacing: 0.3px; line-height: 16px; `; -// Visually distinguishes "and" (match-all) categories from the default "or" -// (match-any) ones, the same way "exclusive"/"threshold" are distinguished by -// rendering as radio buttons instead of checkboxes. -const ModeHint = styled.span` - font-size: 11px; - font-weight: 400; - font-style: italic; - opacity: 0.65; +// Every filter mode gets a small badge next to its category title, so "or" +// and "and" (both checkboxes, otherwise visually identical at rest) are as +// distinguishable as "exclusive"/"threshold" already are via their radio +// shape. Kept to a single subtle character rather than a word, with the +// full explanation one hover/focus away in the tooltip. +const MODE_BADGES = { + or: { badge: 'filterModeOrBadge', tooltip: 'filterModeOrTooltip' }, + and: { badge: 'filterModeAndBadge', tooltip: 'filterModeAndTooltip' }, + exclusive: { badge: 'filterModeExclusiveBadge', tooltip: 'filterModeExclusiveTooltip' }, + boolean: { badge: 'filterModeBooleanBadge', tooltip: 'filterModeBooleanTooltip' }, + threshold: { badge: 'filterModeThresholdBadge', tooltip: 'filterModeThresholdTooltip' }, +}; + +const ModeBadge = styled.span` + display: inline-flex; + align-items: center; + justify-content: center; + min-width: 15px; + height: 15px; + padding: 0 3px; + border-radius: 50%; + font-size: 10px; + font-weight: 700; + line-height: 1; + background-color: rgba(79, 195, 247, 0.18); + color: #4fc3f7; + border: 1px solid rgba(79, 195, 247, 0.45); + cursor: help; `; const FilterOption = styled.label` @@ -285,10 +307,37 @@ export const FiltersForm = () => { fetchCategories(); }, []); + const renderModeBadge = mode => { + const keys = MODE_BADGES[mode] ?? MODE_BADGES.or; + const tooltipText = t(keys.tooltip); + return ( + + {t(keys.badge)} + + ); + }; + const renderFilterOptions = (filters, category) => { // "exclusive" (pick one) and "threshold" (pick a cumulative upper bound, // e.g. speed limit) are both single-select: rendered as radio buttons so // only one option can be active at a time. + // + // TODO: "threshold" categories would read more naturally as an MUI + // with discrete `marks` at each option value - the filled + // track from min to the thumb is a direct visual match for "everything + // up to here is included," better than a radio group. Needs a design + // decision first: sliders have no natural "nothing selected" state (the + // thumb always sits somewhere), but "no filter" must stay distinct from + // "lowest value selected" - e.g. an explicit "Any" mark left of the + // lowest real value, or relying on the existing "Clear filters" button + // as the only way back to unset. Also needs keyboard/screen-reader + // slider accessibility and updated frontend/e2e tests. const isSingleSelect = filters[4] === 'exclusive' || filters[4] === 'threshold'; return filters[1].map(([name, translation]) => { const tooltipData = globalThis.FEATURE_FLAGS?.CATEGORIES_HELP @@ -346,6 +395,7 @@ export const FiltersForm = () => { checked={Boolean(selectedFilters[categoryKey]?.includes(name))} /> {categoryName} + {renderModeBadge('boolean')} {tooltipData && ( @@ -366,7 +416,7 @@ export const FiltersForm = () => { {categoryName} - {filtersData[4] === 'and' && {t('filterModeAndHint')}} + {renderModeBadge(filtersData[4] ?? 'or')} {categoryTooltip && } {renderFilterOptions(filtersData, categoryKey)} diff --git a/frontend/src/locales/en/map.json b/frontend/src/locales/en/map.json index 4fd653ac..2685cdca 100644 --- a/frontend/src/locales/en/map.json +++ b/frontend/src/locales/en/map.json @@ -40,5 +40,14 @@ "clearFilters": "Clear filters", "clearAllFiltersAriaLabel": "Clear all filters", "otherFilters": "Others", - "filterModeAndHint": "(match all)" + "filterModeOrBadge": "+", + "filterModeOrTooltip": "Selecting several options here shows results that match any of them", + "filterModeAndBadge": "&", + "filterModeAndTooltip": "Selecting several options here shows only results that have all of them, not just any of them", + "filterModeExclusiveBadge": "1", + "filterModeExclusiveTooltip": "Only one option can be selected at a time", + "filterModeBooleanBadge": "•", + "filterModeBooleanTooltip": "Unchecked shows everything; checking it narrows the results to just this", + "filterModeThresholdBadge": "≤", + "filterModeThresholdTooltip": "Selecting a value also includes every lower value" } diff --git a/frontend/src/locales/pl/map.json b/frontend/src/locales/pl/map.json index 42b90511..ec4f59f4 100644 --- a/frontend/src/locales/pl/map.json +++ b/frontend/src/locales/pl/map.json @@ -40,5 +40,14 @@ "clearFilters": "Wyczyść filtry", "clearAllFiltersAriaLabel": "Wyczyść wszystkie filtry", "otherFilters": "Inne", - "filterModeAndHint": "(wszystkie naraz)" + "filterModeOrBadge": "+", + "filterModeOrTooltip": "Wybranie kilku opcji tutaj pokazuje wyniki pasujące do dowolnej z nich", + "filterModeAndBadge": "&", + "filterModeAndTooltip": "Wybranie kilku opcji tutaj pokazuje tylko wyniki, które mają je wszystkie, a nie dowolne", + "filterModeExclusiveBadge": "1", + "filterModeExclusiveTooltip": "Można wybrać tylko jedną opcję naraz", + "filterModeBooleanBadge": "•", + "filterModeBooleanTooltip": "Odznaczone pokazuje wszystko; zaznaczenie zawęża wyniki tylko do tego", + "filterModeThresholdBadge": "≤", + "filterModeThresholdTooltip": "Wybranie wartości obejmuje też każdą niższą wartość" } diff --git a/frontend/src/locales/ua/map.json b/frontend/src/locales/ua/map.json index 000cd8b2..5449fa56 100644 --- a/frontend/src/locales/ua/map.json +++ b/frontend/src/locales/ua/map.json @@ -39,5 +39,14 @@ "clearFilters": "Очистити фільтри", "clearAllFiltersAriaLabel": "Очистити всі фільтри", "otherFilters": "Інше", - "filterModeAndHint": "(усі одразу)" + "filterModeOrBadge": "+", + "filterModeOrTooltip": "Вибір кількох варіантів тут показує результати, що відповідають будь-якому з них", + "filterModeAndBadge": "&", + "filterModeAndTooltip": "Вибір кількох варіантів тут показує лише результати, що мають їх усі, а не будь-який", + "filterModeExclusiveBadge": "1", + "filterModeExclusiveTooltip": "Можна вибрати лише один варіант одночасно", + "filterModeBooleanBadge": "•", + "filterModeBooleanTooltip": "Не позначено - показує все; позначення звужує результати лише до цього", + "filterModeThresholdBadge": "≤", + "filterModeThresholdTooltip": "Вибір значення також включає кожне нижче значення" } diff --git a/frontend/tests/FiltersForm.test.jsx b/frontend/tests/FiltersForm.test.jsx index d982fdea..6b35d57f 100644 --- a/frontend/tests/FiltersForm.test.jsx +++ b/frontend/tests/FiltersForm.test.jsx @@ -221,7 +221,7 @@ describe('Groups boolean categories into a shared "Others" section', () => { expect(freeCheckbox.value).toBe('true'); const label = document.querySelector('label[for="is_free"]'); - expect(label.textContent.trim()).toBe('Free only'); + expect(label.textContent).toContain('Free only'); // "false" is never rendered - unchecked already means "show everything". expect(document.querySelector('#false')).toBeNull(); @@ -325,8 +325,11 @@ describe('Distinguishes "and" categories with a visible hint, but keeps checkbox expect(benches.checked).toBe(true); }); - it('shows a "match all" hint next to the category title', () => { + it('shows an "&" badge with an explanatory tooltip next to the category title', () => { const header = document.querySelector('#filter-label-amenities-amenities').parentElement; - expect(header.textContent).toContain('(match all)'); + expect(header.textContent).toContain('&'); + + const badge = within(header).getByLabelText(/Help:/i); + expect(badge).toBeInTheDocument(); }); }); From 197a2633a9c8ce025f104583cccf78b83bddc925 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Krzysztof=20Ko=C5=82odzi=C5=84ski?= Date: Tue, 28 Jul 2026 00:28:31 +0200 Subject: [PATCH 03/12] renames --- .../tests/basic/test_accessibility_table.py | 16 +++++++++++----- e2e-tests/tests/basic/test_language.py | 4 ++-- e2e-tests/tests/basic/test_map.py | 18 +++++++++--------- e2e-tests/tests/basic/test_mobile_box.py | 4 ++-- e2e-tests/tests/basic/test_popup.py | 6 +++--- e2e-tests/tests/basic/test_share.py | 6 +++--- e2e-tests/tests/conftest.py | 11 ++++++++--- 7 files changed, 38 insertions(+), 27 deletions(-) diff --git a/e2e-tests/tests/basic/test_accessibility_table.py b/e2e-tests/tests/basic/test_accessibility_table.py index 24f147a0..2707c647 100644 --- a/e2e-tests/tests/basic/test_accessibility_table.py +++ b/e2e-tests/tests/basic/test_accessibility_table.py @@ -10,7 +10,13 @@ import pytest from playwright.sync_api import Page, expect -from tests.conftest import BASE_URL, MARKER_LOAD_TIMEOUT, TABLE_LOAD_TIMEOUT, TEST_LOCATIONS +from tests.conftest import ( + BASE_URL, + MARKER_LOAD_TIMEOUT, + SEEDED_LOCATION_COUNT, + TABLE_LOAD_TIMEOUT, + TEST_LOCATIONS, +) class TestAccessibilityTable: @@ -28,7 +34,7 @@ def setup(self, page: Page, geolocation): # "accessible_by: cars" is checked by default (see categories_default_checked # in the test data), which excludes Zwierzyniecka (bikes/pedestrians only, - # no cars). Uncheck it so all ten seeded locations are visible, matching + # no cars). Uncheck it so all seeded locations are visible, matching # what these tests assert. page.wait_for_selector("#filter-form", timeout=MARKER_LOAD_TIMEOUT) page.locator("#filter-form input#cars").uncheck() @@ -50,11 +56,11 @@ def setup(self, page: Page, geolocation): def test_should_properly_display_places(self, page: Page): """ - Verify table displays correct number of rows. - Should have 1 header row + 10 data rows = 11 total rows. + Verify table displays correct number of rows: 1 header row plus one + row per seeded location. """ rows = page.locator("tr") - expect(rows).to_have_count(11) + expect(rows).to_have_count(SEEDED_LOCATION_COUNT + 1) def test_zwierzyniecka_should_be_first_row(self, page: Page): """ diff --git a/e2e-tests/tests/basic/test_language.py b/e2e-tests/tests/basic/test_language.py index a5b5b54e..b0460ba1 100644 --- a/e2e-tests/tests/basic/test_language.py +++ b/e2e-tests/tests/basic/test_language.py @@ -10,7 +10,7 @@ import pytest from playwright.sync_api import Page, expect -from tests.conftest import BASE_URL, open_zwierzyniecka_popup +from tests.conftest import BASE_URL, open_test_popup def get_language_button(page: Page): @@ -71,7 +71,7 @@ def test_switch_to_polish_changes_popup_text(self, page: Page): page.get_by_role("link", name="polski").click() page.wait_for_load_state("domcontentloaded") - open_zwierzyniecka_popup(page) + open_test_popup(page) # Verify popup is visible popup = page.locator(".leaflet-popup-content") diff --git a/e2e-tests/tests/basic/test_map.py b/e2e-tests/tests/basic/test_map.py index c0ae040d..84d18b11 100644 --- a/e2e-tests/tests/basic/test_map.py +++ b/e2e-tests/tests/basic/test_map.py @@ -7,7 +7,7 @@ import pytest from playwright.sync_api import Page, expect -from tests.conftest import BASE_URL, TABLE_LOAD_TIMEOUT, TEST_LOCATIONS +from tests.conftest import BASE_URL, SEEDED_LOCATION_COUNT, TABLE_LOAD_TIMEOUT, TEST_LOCATIONS class TestMap: @@ -115,8 +115,8 @@ def test_filter_checkbox_filters_markers(self, page: Page): table = self._open_list_view(page) rows = table.locator("tr") - # 1 header + 10 data rows - expect(rows).to_have_count(11) + # 1 header + all seeded locations + expect(rows).to_have_count(SEEDED_LOCATION_COUNT + 1) # Re-check the "cars" filter checkbox - this should filter to only show # the 6 bridges accessible by cars (1 header + 6 data rows) @@ -125,7 +125,7 @@ def test_filter_checkbox_filters_markers(self, page: Page): # Uncheck to restore all results cars_checkbox.click() - expect(rows).to_have_count(11) + expect(rows).to_have_count(SEEDED_LOCATION_COUNT + 1) def test_or_filter_within_category_broadens_results(self, page: Page): """Selecting multiple checkboxes within one category (accessible_by) should @@ -140,7 +140,7 @@ def test_or_filter_within_category_broadens_results(self, page: Page): table = self._open_list_view(page) rows = table.locator("tr") - expect(rows).to_have_count(11) + expect(rows).to_have_count(SEEDED_LOCATION_COUNT + 1) # bikes alone -> only Zwierzyniecka (1 header + 1 data row) bikes_checkbox.click() @@ -164,7 +164,7 @@ def test_is_free_boolean_filter_toggles_free_only(self, page: Page): table = self._open_list_view(page) rows = table.locator("tr") - expect(rows).to_have_count(11) + expect(rows).to_have_count(SEEDED_LOCATION_COUNT + 1) free_checkbox.click() expect(free_checkbox).to_be_checked() @@ -174,7 +174,7 @@ def test_is_free_boolean_filter_toggles_free_only(self, page: Page): # Unchecking goes back to showing both free and paid bridges. free_checkbox.click() expect(free_checkbox).not_to_be_checked() - expect(rows).to_have_count(11) + expect(rows).to_have_count(SEEDED_LOCATION_COUNT + 1) def test_speed_limit_threshold_filter_includes_lower_values(self, page: Page): """Selecting a speed limit should also match bridges with a lower limit @@ -187,7 +187,7 @@ def test_speed_limit_threshold_filter_includes_lower_values(self, page: Page): table = self._open_list_view(page) rows = table.locator("tr") - expect(rows).to_have_count(11) + expect(rows).to_have_count(SEEDED_LOCATION_COUNT + 1) # 30 km/h also matches the three 10 km/h bridges, but not the 50 km/h # ones: 1 header + 6 data rows @@ -207,7 +207,7 @@ def test_and_filter_within_category_narrows_results(self, page: Page): table = self._open_list_view(page) rows = table.locator("tr") - expect(rows).to_have_count(11) + expect(rows).to_have_count(SEEDED_LOCATION_COUNT + 1) # lighting alone -> 8 bridges (1 header + 8 data rows) lighting_checkbox.click() diff --git a/e2e-tests/tests/basic/test_mobile_box.py b/e2e-tests/tests/basic/test_mobile_box.py index 2d8cf237..63ecbf14 100644 --- a/e2e-tests/tests/basic/test_mobile_box.py +++ b/e2e-tests/tests/basic/test_mobile_box.py @@ -9,7 +9,7 @@ import pytest from playwright.sync_api import Page, expect -from tests.conftest import ALL_MOBILE_DEVICES, BASE_URL, open_zwierzyniecka_popup +from tests.conftest import ALL_MOBILE_DEVICES, BASE_URL, open_test_popup from tests.helpers import EXPECTED_PLACE_ZWIERZYNIECKA, verify_popup_content, verify_problem_form @@ -34,7 +34,7 @@ def test_displays_title_and_subtitle_in_popup( # Isolate Zwierzyniecka's marker and click it directly, rather than # expanding a multi-marker cluster and guessing at its layout. - open_zwierzyniecka_popup(mobile_page) + open_test_popup(mobile_page) # On mobile, popup appears as Material-UI Dialog (bottom sheet) dialog_content = mobile_page.locator(".MuiDialogContent-root") diff --git a/e2e-tests/tests/basic/test_popup.py b/e2e-tests/tests/basic/test_popup.py index ba6258bc..abff7da6 100644 --- a/e2e-tests/tests/basic/test_popup.py +++ b/e2e-tests/tests/basic/test_popup.py @@ -6,7 +6,7 @@ from playwright.sync_api import Page, expect -from tests.conftest import BASE_URL, open_zwierzyniecka_popup +from tests.conftest import BASE_URL, open_test_popup from tests.helpers import EXPECTED_PLACE_ZWIERZYNIECKA, verify_popup_content, verify_problem_form @@ -24,7 +24,7 @@ def test_displays_popup_title_subtitle_categories_and_cta(self, page: Page, wind # Isolate Zwierzyniecka's marker and click it directly, rather than # expanding a multi-marker cluster and guessing at its layout. - open_zwierzyniecka_popup(page) + open_test_popup(page) # Verify popup content popup = page.locator(".leaflet-popup-content") @@ -52,7 +52,7 @@ def test_problem_form_on_desktop(self, page: Page, window_open_stub): """ page.goto(BASE_URL, wait_until="domcontentloaded") - open_zwierzyniecka_popup(page) + open_test_popup(page) # Verify popup is visible popup = page.locator(".leaflet-popup-content") diff --git a/e2e-tests/tests/basic/test_share.py b/e2e-tests/tests/basic/test_share.py index 2ccdf43a..674528cb 100644 --- a/e2e-tests/tests/basic/test_share.py +++ b/e2e-tests/tests/basic/test_share.py @@ -15,7 +15,7 @@ BASE_URL, MARKER_LOAD_TIMEOUT, clear_all_checkboxes, - open_zwierzyniecka_popup, + open_test_popup, ) @@ -32,7 +32,7 @@ def test_share_button_copies_link_to_clipboard(self, page: Page): # Grant clipboard permissions page.context.grant_permissions(["clipboard-read", "clipboard-write"]) - open_zwierzyniecka_popup(page) + open_test_popup(page) # Verify popup is visible popup = page.locator(".leaflet-popup-content") @@ -103,7 +103,7 @@ def test_share_button_triggers_native_share(self, mobile_page: Page, device_name mobile_page.goto(BASE_URL, wait_until="domcontentloaded") - open_zwierzyniecka_popup(mobile_page) + open_test_popup(mobile_page) # On mobile, popup appears as Material-UI Dialog dialog_content = mobile_page.locator(".MuiDialogContent-root") diff --git a/e2e-tests/tests/conftest.py b/e2e-tests/tests/conftest.py index c5ed2682..51d5f840 100644 --- a/e2e-tests/tests/conftest.py +++ b/e2e-tests/tests/conftest.py @@ -21,6 +21,11 @@ TABLE_LOAD_TIMEOUT = 5000 FLY_TO_TIMEOUT = 20000 +# Number of bridges in e2e_test_data_initial.json's "data" array. Kept as a +# single constant so tests don't hardcode this count (and its +1-for-header +# variant) in multiple places. +SEEDED_LOCATION_COUNT = 10 + MOBILE_DEVICES = { "iphone-x": { "viewport": {"width": 375, "height": 812}, @@ -102,10 +107,10 @@ def clear_all_checkboxes(page: Page) -> None: page.locator('button[aria-label="Close left panel"]').evaluate("el => el.click()") -def open_zwierzyniecka_popup(page: Page) -> None: +def open_test_popup(page: Page) -> None: """ - Filter down to exactly the Zwierzyniecka bridge and click its marker to - open its popup. + Filter down to exactly one known seeded location (Zwierzyniecka) and + click its marker to open its popup. Clicking into a multi-marker cluster spiderfies it into a layout whose on-screen ordering isn't guaranteed to match geographic position From b9aad2b4802f7e79d2ba5ce7d9d2ae72c53ceff2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Krzysztof=20Ko=C5=82odzi=C5=84ski?= Date: Tue, 28 Jul 2026 00:46:22 +0200 Subject: [PATCH 04/12] remove some comments --- e2e-tests/tests/basic/test_map.py | 2 +- e2e-tests/tests/basic/test_mobile_box.py | 2 -- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/e2e-tests/tests/basic/test_map.py b/e2e-tests/tests/basic/test_map.py index 84d18b11..d156c7c5 100644 --- a/e2e-tests/tests/basic/test_map.py +++ b/e2e-tests/tests/basic/test_map.py @@ -108,7 +108,7 @@ def test_should_not_have_scrollbars(self, page: Page): def test_filter_checkbox_filters_markers(self, page: Page): """Verify clicking filter checkbox actually filters the results""" # "accessible_by: cars" is checked by default (see categories_default_checked - # in the test data), so start by clearing it to see all ten seeded locations. + # in the test data), so start by clearing it to see all seeded locations. cars_checkbox = page.get_by_role("checkbox", name="cars", exact=False) expect(cars_checkbox).to_be_checked() cars_checkbox.click() diff --git a/e2e-tests/tests/basic/test_mobile_box.py b/e2e-tests/tests/basic/test_mobile_box.py index 63ecbf14..0cc43133 100644 --- a/e2e-tests/tests/basic/test_mobile_box.py +++ b/e2e-tests/tests/basic/test_mobile_box.py @@ -32,8 +32,6 @@ def test_displays_title_and_subtitle_in_popup( # Navigate to the page (device emulation already configured by mobile_page fixture) mobile_page.goto(BASE_URL, wait_until="domcontentloaded") - # Isolate Zwierzyniecka's marker and click it directly, rather than - # expanding a multi-marker cluster and guessing at its layout. open_test_popup(mobile_page) # On mobile, popup appears as Material-UI Dialog (bottom sheet) From 0ebace1111b4b30f696ea3c5f2cb1b0b54a9f07c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Krzysztof=20Ko=C5=82odzi=C5=84ski?= Date: Tue, 28 Jul 2026 07:53:20 +0200 Subject: [PATCH 05/12] shorter comments --- e2e-tests/tests/basic/test_map.py | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/e2e-tests/tests/basic/test_map.py b/e2e-tests/tests/basic/test_map.py index d156c7c5..d61d61d0 100644 --- a/e2e-tests/tests/basic/test_map.py +++ b/e2e-tests/tests/basic/test_map.py @@ -62,17 +62,12 @@ def test_displays_filter_list_with_four_categories(self, page: Page): expect(page.get_by_text("speed limit")).to_be_visible() expect(page.get_by_text("Others")).to_be_visible() expect(page.get_by_text("Free only")).to_be_visible() - - # Every category shows a small mode badge (a single character, with a - # tooltip on hover/focus explaining what it means) next to its title - - # or:+, and:&, threshold:≤, boolean:• - so "or" and "and" (both - # checkboxes, otherwise visually identical) are as distinguishable as - # exclusive/threshold already are via their radio shape. expect(page.get_by_text("amenities")).to_be_visible() def mode_badge(symbol): return page.get_by_label("Help:", exact=False).filter(has_text=symbol) + # check whether badges are there expect(mode_badge("+")).to_have_count(2) # accessible_by, type_of_place expect(mode_badge("&")).to_have_count(1) # amenities expect(mode_badge("≤")).to_have_count(1) # speed_limit From 1db63e8d45ff977b91293f39c5d90518f82c65b7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Krzysztof=20Ko=C5=82odzi=C5=84ski?= Date: Tue, 28 Jul 2026 09:05:46 +0200 Subject: [PATCH 06/12] type fixes --- .../components/FiltersForm/FiltersForm.jsx | 37 ++++---- frontend/src/services/http/httpService.js | 36 ++++---- frontend/tests/FiltersForm.test.jsx | 89 ++++++++++--------- frontend/tests/Map/MapComponent.test.jsx | 14 +-- 4 files changed, 91 insertions(+), 85 deletions(-) diff --git a/frontend/src/components/FiltersForm/FiltersForm.jsx b/frontend/src/components/FiltersForm/FiltersForm.jsx index fa4be680..28909f92 100644 --- a/frontend/src/components/FiltersForm/FiltersForm.jsx +++ b/frontend/src/components/FiltersForm/FiltersForm.jsx @@ -323,7 +323,7 @@ export const FiltersForm = () => { ); }; - const renderFilterOptions = (filters, category) => { + const renderFilterOptions = category => { // "exclusive" (pick one) and "threshold" (pick a cumulative upper bound, // e.g. speed limit) are both single-select: rendered as radio buttons so // only one option can be active at a time. @@ -338,21 +338,22 @@ export const FiltersForm = () => { // lowest real value, or relying on the existing "Clear filters" button // as the only way back to unset. Also needs keyboard/screen-reader // slider accessibility and updated frontend/e2e tests. - const isSingleSelect = filters[4] === 'exclusive' || filters[4] === 'threshold'; - return filters[1].map(([name, translation]) => { + const { categoryKey, options, optionsHelp, filterMode } = category; + const isSingleSelect = filterMode === 'exclusive' || filterMode === 'threshold'; + return options.map(([name, translation]) => { const tooltipData = globalThis.FEATURE_FLAGS?.CATEGORIES_HELP - ? filters[3].find(it => it[name]) + ? optionsHelp.find(it => it[name]) : ''; return ( - + {translation} {tooltipData && ( @@ -370,19 +371,19 @@ export const FiltersForm = () => { // than giving each its own titled section, they're grouped into one shared // "Others" section as plain checkboxes labeled with the category's own name. // This keeps the panel compact as more true/false-style filters are added. - const isBooleanCategory = filtersData => filtersData[4] === 'boolean'; + const isBooleanCategory = category => category.filterMode === 'boolean'; const booleanCategories = categoriesData.filter(isBooleanCategory); const otherCategories = categoriesData.filter(f => !isBooleanCategory(f)); - const renderBooleanFilterOption = filtersData => { - const [categoryKey, categoryName] = filtersData[0]; - const trueOption = filtersData[1].find(([name]) => name === 'true'); + const renderBooleanFilterOption = category => { + const { categoryKey, categoryName, options, optionsHelp } = category; + const trueOption = options.find(([name]) => name === 'true'); if (!trueOption) { return null; } const [name] = trueOption; const tooltipData = globalThis.FEATURE_FLAGS?.CATEGORIES_HELP - ? filtersData[3].find(it => it[name]) + ? optionsHelp.find(it => it[name]) : ''; return ( @@ -405,21 +406,21 @@ export const FiltersForm = () => { ); }; - const sections = otherCategories.map(filtersData => { - const [categoryKey, categoryName] = filtersData[0]; + const sections = otherCategories.map(category => { + const { categoryKey, categoryName, categoriesHelp, filterMode } = category; const sectionKey = `${categoryKey}-${categoryName}`; const categoryTooltip = globalThis.FEATURE_FLAGS?.CATEGORIES_HELP - ? filtersData[2].find(it => it[categoryKey]) + ? categoriesHelp.find(it => it[categoryKey]) : null; return ( {categoryName} - {renderModeBadge(filtersData[4] ?? 'or')} + {renderModeBadge(filterMode)} {categoryTooltip && } - {renderFilterOptions(filtersData, categoryKey)} + {renderFilterOptions(category)} ); }); diff --git a/frontend/src/services/http/httpService.js b/frontend/src/services/http/httpService.js index 19ffe1e5..e734563d 100644 --- a/frontend/src/services/http/httpService.js +++ b/frontend/src/services/http/httpService.js @@ -47,30 +47,24 @@ export const httpService = { * Fetches complete categories data including subcategories in a single request. * Uses the /api/categories-full endpoint to avoid waterfall requests. * - * @returns {Promise<{categories: Array, defaultChecked: Object}>} Promise resolving to - * the array of category data tuples plus a map of category key to the option values - * that should be pre-checked by default. + * @returns {Promise<{categories: Array<{categoryKey: string, categoryName: string, + * options: Array<[string, string]>, categoriesHelp: Array, optionsHelp: Array, + * filterMode: string}>, defaultChecked: Object}>} Promise resolving to the array of + * category data plus a map of category key to the option values that should be + * pre-checked by default. */ getCategoriesData: async () => { const response = await fetch(CATEGORIES_FULL).then(res => res.json()); - - // Transform to expected format: [[key, name], options, help?, optionsHelp?, filterMode] - const categories = response.categories.map(category => { - const categoryTuple = [category.key, category.name]; - const options = category.options; - const filterMode = category.filter_mode ?? 'or'; - - if (globalThis.FEATURE_FLAGS?.CATEGORIES_HELP) { - return [ - categoryTuple, - category.options_with_help ?? options, - response.categories_help ?? [], - category.options_help ?? [], - filterMode, - ]; - } - return [categoryTuple, options, undefined, undefined, filterMode]; - }); + const useCategoriesHelp = Boolean(globalThis.FEATURE_FLAGS?.CATEGORIES_HELP); + + const categories = response.categories.map(category => ({ + categoryKey: category.key, + categoryName: category.name, + options: (useCategoriesHelp ? category.options_with_help : null) ?? category.options, + categoriesHelp: useCategoriesHelp ? response.categories_help ?? [] : [], + optionsHelp: useCategoriesHelp ? category.options_help ?? [] : [], + filterMode: category.filter_mode ?? 'or', + })); const defaultChecked = Object.fromEntries( response.categories diff --git a/frontend/tests/FiltersForm.test.jsx b/frontend/tests/FiltersForm.test.jsx index 6b35d57f..d779e5b5 100644 --- a/frontend/tests/FiltersForm.test.jsx +++ b/frontend/tests/FiltersForm.test.jsx @@ -8,15 +8,17 @@ import { httpService } from '../src/services/http/httpService'; jest.mock('../src/services/http/httpService'); const categories = [ - [ - ['types', 'typy'], - [ + { + categoryKey: 'types', + categoryName: 'typy', + options: [ ['clothes', 'ciuchy'], ['shoes', 'buty'], ], - [{ types: 'Inaczej rodzaje' }], - [{ shoes: 'Kozaki też' }], - ], + categoriesHelp: [{ types: 'Inaczej rodzaje' }], + optionsHelp: [{ shoes: 'Kozaki też' }], + filterMode: 'or', + }, ]; httpService.getCategoriesData.mockResolvedValue({ categories, defaultChecked: {} }); @@ -123,17 +125,18 @@ describe('Renders exclusive (single-select) categories as radio buttons', () => // instead (see below), which offers a single checkbox and no separate // radio-deselection problem. const exclusiveCategories = [ - [ - ['payment_status', 'payment status'], - [ + { + categoryKey: 'payment_status', + categoryName: 'payment status', + options: [ ['free', 'free'], ['discounted', 'discounted'], ['full_price', 'full price'], ], - [], - [], - 'exclusive', - ], + categoriesHelp: [], + optionsHelp: [], + filterMode: 'exclusive', + }, ]; beforeEach(async () => { @@ -173,26 +176,28 @@ describe('Renders exclusive (single-select) categories as radio buttons', () => describe('Groups boolean categories into a shared "Others" section', () => { const mixedCategories = [ - [ - ['types', 'typy'], - [ + { + categoryKey: 'types', + categoryName: 'typy', + options: [ ['clothes', 'ciuchy'], ['shoes', 'buty'], ], - [], - [], - 'or', - ], - [ - ['is_free', 'Free only'], - [ + categoriesHelp: [], + optionsHelp: [], + filterMode: 'or', + }, + { + categoryKey: 'is_free', + categoryName: 'Free only', + options: [ ['true', 'yes'], ['false', 'no'], ], - [], - [], - 'boolean', - ], + categoriesHelp: [], + optionsHelp: [], + filterMode: 'boolean', + }, ]; beforeEach(async () => { @@ -242,17 +247,18 @@ describe('Groups boolean categories into a shared "Others" section', () => { describe('Renders threshold categories as radio buttons too', () => { const thresholdCategories = [ - [ - ['speed_limit', 'speed limit'], - [ + { + categoryKey: 'speed_limit', + categoryName: 'speed limit', + options: [ ['10', '10 km/h'], ['30', '30 km/h'], ['50', '50 km/h'], ], - [], - [], - 'threshold', - ], + categoriesHelp: [], + optionsHelp: [], + filterMode: 'threshold', + }, ]; beforeEach(async () => { @@ -288,16 +294,17 @@ describe('Renders threshold categories as radio buttons too', () => { describe('Distinguishes "and" categories with a visible hint, but keeps checkboxes', () => { const andCategories = [ - [ - ['amenities', 'amenities'], - [ + { + categoryKey: 'amenities', + categoryName: 'amenities', + options: [ ['lighting', 'lighting'], ['benches', 'benches'], ], - [], - [], - 'and', - ], + categoriesHelp: [], + optionsHelp: [], + filterMode: 'and', + }, ]; beforeEach(async () => { diff --git a/frontend/tests/Map/MapComponent.test.jsx b/frontend/tests/Map/MapComponent.test.jsx index c8bb0320..7886aafd 100644 --- a/frontend/tests/Map/MapComponent.test.jsx +++ b/frontend/tests/Map/MapComponent.test.jsx @@ -9,13 +9,17 @@ import { httpService } from '../../src/services/http/httpService'; jest.mock('../../src/services/http/httpService'); const categories = [ - [ - ['types', 'typy'], - [ + { + categoryKey: 'types', + categoryName: 'typy', + options: [ ['clothes', 'ciuchy'], ['shoes', 'buty'], ], - ], + categoriesHelp: [], + optionsHelp: [], + filterMode: 'or', + }, ]; const locations = [ @@ -27,7 +31,7 @@ const locations = [ ]; httpService.getLocations.mockResolvedValue(locations); -httpService.getCategoriesData.mockResolvedValue(categories); +httpService.getCategoriesData.mockResolvedValue({ categories, defaultChecked: {} }); describe('MapComponent', () => { beforeAll(() => { From ab8fbd46840ad343a546a6b369ccf08f289722c2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Krzysztof=20Ko=C5=82odzi=C5=84ski?= Date: Tue, 28 Jul 2026 09:13:02 +0200 Subject: [PATCH 07/12] default filter_modes --- goodmap/core.py | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/goodmap/core.py b/goodmap/core.py index 354a2826..39f0c01e 100644 --- a/goodmap/core.py +++ b/goodmap/core.py @@ -1,9 +1,16 @@ """Core data filtering and sorting utilities for location queries.""" -from typing import Any, Dict, List +from types import MappingProxyType +from typing import Any, Dict, List, Mapping # TODO move filtering to db site +# Python has no builtin "frozendict" (the way frozenset mirrors set); +# MappingProxyType is the standard-library equivalent - a read-only view that +# raises TypeError on mutation. Used as the filter_modes default below instead +# of a plain {} so a single shared instance is safe to reuse across calls. +_NO_FILTER_MODES: Mapping[str, str] = MappingProxyType({}) + def _as_list(value: Any) -> list: """Wrap a scalar field value in a list, leaving list values untouched.""" @@ -56,21 +63,20 @@ def _matches_threshold(entry_values: list, selected_values: list) -> bool: } -def does_fulfill_requirement(entry, requirements, filter_modes=None): +def does_fulfill_requirement(entry, requirements, filter_modes=_NO_FILTER_MODES): """Check if an entry fulfills all category requirements. Args: entry: Location data entry to check requirements: List of (category, values) tuples to match - filter_modes: Optional dict mapping category name to combination mode - ("or", "and", "exclusive", "boolean", or "threshold"). Categories not + filter_modes: Dict mapping category name to combination mode ("or", + "and", "exclusive", "boolean", or "threshold"). Categories not present default to "or" (entry matches if it has any of the selected values). Returns: bool: True if entry matches all non-empty requirements """ - filter_modes = filter_modes or {} matches = [] for category, values in requirements: if not values: @@ -122,15 +128,15 @@ def limit(data, query_params): return data -def get_queried_data(all_data, categories, query_params, filter_modes=None): +def get_queried_data(all_data, categories, query_params, filter_modes=_NO_FILTER_MODES): """Filter, sort, and limit location data based on query parameters. Args: all_data: Complete list of location data categories: Available categories for filtering query_params: Query parameters for filtering, sorting, and limiting - filter_modes: Optional dict mapping category name to combination mode - ("or", "and", "exclusive", "boolean", or "threshold"), see + filter_modes: Dict mapping category name to combination mode ("or", + "and", "exclusive", "boolean", or "threshold"), see does_fulfill_requirement. Returns: From 34a9ed2412cf672b265fd2c557edef246241626e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Krzysztof=20Ko=C5=82odzi=C5=84ski?= Date: Tue, 28 Jul 2026 09:33:53 +0200 Subject: [PATCH 08/12] refactored filtering --- docs/api.rst | 12 ++- goodmap/core.py | 92 ++------------------ goodmap/filtering.py | 91 ++++++++++++++++++++ tests/unit_tests/test_core.py | 119 +------------------------- tests/unit_tests/test_filtering.py | 133 +++++++++++++++++++++++++++++ 5 files changed, 242 insertions(+), 205 deletions(-) create mode 100644 goodmap/filtering.py create mode 100644 tests/unit_tests/test_filtering.py diff --git a/docs/api.rst b/docs/api.rst index 72564188..856ad768 100644 --- a/docs/api.rst +++ b/docs/api.rst @@ -19,13 +19,23 @@ Database Module :undoc-members: :show-inheritance: +Query Module +------------ + +Sorting, limiting, and query orchestration for location data. + +.. automodule:: goodmap.core + :members: + :undoc-members: + :show-inheritance: + Filtering Module ---------------- Category filter combination logic (see :ref:`categories-filter-mode` in :doc:`quickstart` for the config-level guide). -.. automodule:: goodmap.core +.. automodule:: goodmap.filtering :members: :undoc-members: :show-inheritance: diff --git a/goodmap/core.py b/goodmap/core.py index 39f0c01e..ba4731d0 100644 --- a/goodmap/core.py +++ b/goodmap/core.py @@ -1,90 +1,10 @@ -"""Core data filtering and sorting utilities for location queries.""" +"""Core data sorting, limiting, and query orchestration for location queries.""" -from types import MappingProxyType -from typing import Any, Dict, List, Mapping - -# TODO move filtering to db site - -# Python has no builtin "frozendict" (the way frozenset mirrors set); -# MappingProxyType is the standard-library equivalent - a read-only view that -# raises TypeError on mutation. Used as the filter_modes default below instead -# of a plain {} so a single shared instance is safe to reuse across calls. -_NO_FILTER_MODES: Mapping[str, str] = MappingProxyType({}) - - -def _as_list(value: Any) -> list: - """Wrap a scalar field value in a list, leaving list values untouched.""" - if value is None: - return [] - if isinstance(value, list): - return value - return [value] - - -def _matches_or(entry_values: list, selected_values: list) -> bool: - """Match if the entry has at least one of the selected values (any-of).""" - return any(value in entry_values for value in selected_values) +from typing import Any, Dict, List +from goodmap.filtering import NO_FILTER_MODES, does_fulfill_requirement -def _matches_and(entry_values: list, selected_values: list) -> bool: - """Match only if the entry has every selected value (all-of). - - Only meaningful for list-valued categories (an entry can have multiple - simultaneous values), e.g. narrowing down to locations that have both - "lighting" and "benches" among their amenities. For a single-valued - category this can only match when a single value is selected. - """ - return all(value in entry_values for value in selected_values) - - -def _matches_threshold(entry_values: list, selected_values: list) -> bool: - """Match if any entry value is numerically <= the highest selected value. - - Used for ordered numeric categories (e.g. speed limits) where selecting a - value implies "this value or lower" (e.g. selecting 50 also matches 10 and 30). - """ - try: - entry_numbers = [float(value) for value in entry_values] - max_selected = max(float(value) for value in selected_values) - except (TypeError, ValueError): - return False - return any(number <= max_selected for number in entry_numbers) - - -# "exclusive" (radio group) and "boolean" (single checkbox, e.g. "free only") -# categories only ever send a single selected value, so matching is the same -# as "or". -_FILTER_MATCHERS = { - "or": _matches_or, - "and": _matches_and, - "exclusive": _matches_or, - "boolean": _matches_or, - "threshold": _matches_threshold, -} - - -def does_fulfill_requirement(entry, requirements, filter_modes=_NO_FILTER_MODES): - """Check if an entry fulfills all category requirements. - - Args: - entry: Location data entry to check - requirements: List of (category, values) tuples to match - filter_modes: Dict mapping category name to combination mode ("or", - "and", "exclusive", "boolean", or "threshold"). Categories not - present default to "or" (entry matches if it has any of the - selected values). - - Returns: - bool: True if entry matches all non-empty requirements - """ - matches = [] - for category, values in requirements: - if not values: - continue - entry_values = _as_list(entry.get(category)) - matcher = _FILTER_MATCHERS.get(filter_modes.get(category, "or"), _matches_or) - matches.append(matcher(entry_values, values)) - return all(matches) +# TODO move filtering to db site def sort_by_distance(data: List[Dict[str, Any]], query_params: Dict[str, List[str]]): @@ -128,7 +48,7 @@ def limit(data, query_params): return data -def get_queried_data(all_data, categories, query_params, filter_modes=_NO_FILTER_MODES): +def get_queried_data(all_data, categories, query_params, filter_modes=NO_FILTER_MODES): """Filter, sort, and limit location data based on query parameters. Args: @@ -137,7 +57,7 @@ def get_queried_data(all_data, categories, query_params, filter_modes=_NO_FILTER query_params: Query parameters for filtering, sorting, and limiting filter_modes: Dict mapping category name to combination mode ("or", "and", "exclusive", "boolean", or "threshold"), see - does_fulfill_requirement. + goodmap.filtering.does_fulfill_requirement. Returns: Filtered, sorted, and limited location data diff --git a/goodmap/filtering.py b/goodmap/filtering.py new file mode 100644 index 00000000..a2bc831c --- /dev/null +++ b/goodmap/filtering.py @@ -0,0 +1,91 @@ +"""Category filter combination logic for location queries. + +See the "Categories and Filtering" section of the docs (categories_filter_mode) +for the config-level guide to the modes handled here: "or", "and", "exclusive", +"boolean", and "threshold". +""" + +from types import MappingProxyType +from typing import Mapping + +# Python has no builtin "frozendict" (the way frozenset mirrors set); +# MappingProxyType is the standard-library equivalent - a read-only view that +# raises TypeError on mutation. Used as the filter_modes default below (and in +# goodmap.core.get_queried_data) so a single shared instance is safe to reuse +# across calls instead of a plain {}. +NO_FILTER_MODES: Mapping[str, str] = MappingProxyType({}) + + +def _as_list(value) -> list: + """Wrap a scalar field value in a list, leaving list values untouched.""" + if value is None: + return [] + if isinstance(value, list): + return value + return [value] + + +def _matches_or(entry_values: list, selected_values: list) -> bool: + """Match if the entry has at least one of the selected values (any-of).""" + return any(value in entry_values for value in selected_values) + + +def _matches_and(entry_values: list, selected_values: list) -> bool: + """Match only if the entry has every selected value (all-of). + + Only meaningful for list-valued categories (an entry can have multiple + simultaneous values), e.g. narrowing down to locations that have both + "lighting" and "benches" among their amenities. For a single-valued + category this can only match when a single value is selected. + """ + return all(value in entry_values for value in selected_values) + + +def _matches_threshold(entry_values: list, selected_values: list) -> bool: + """Match if any entry value is numerically <= the highest selected value. + + Used for ordered numeric categories (e.g. speed limits) where selecting a + value implies "this value or lower" (e.g. selecting 50 also matches 10 and 30). + """ + try: + entry_numbers = [float(value) for value in entry_values] + max_selected = max(float(value) for value in selected_values) + except (TypeError, ValueError): + return False + return any(number <= max_selected for number in entry_numbers) + + +# "exclusive" (radio group) and "boolean" (single checkbox, e.g. "free only") +# categories only ever send a single selected value, so matching is the same +# as "or". +_FILTER_MATCHERS = { + "or": _matches_or, + "and": _matches_and, + "exclusive": _matches_or, + "boolean": _matches_or, + "threshold": _matches_threshold, +} + + +def does_fulfill_requirement(entry, requirements, filter_modes=NO_FILTER_MODES): + """Check if an entry fulfills all category requirements. + + Args: + entry: Location data entry to check + requirements: List of (category, values) tuples to match + filter_modes: Dict mapping category name to combination mode ("or", + "and", "exclusive", "boolean", or "threshold"). Categories not + present default to "or" (entry matches if it has any of the + selected values). + + Returns: + bool: True if entry matches all non-empty requirements + """ + matches = [] + for category, values in requirements: + if not values: + continue + entry_values = _as_list(entry.get(category)) + matcher = _FILTER_MATCHERS.get(filter_modes.get(category, "or"), _matches_or) + matches.append(matcher(entry_values, values)) + return all(matches) diff --git a/tests/unit_tests/test_core.py b/tests/unit_tests/test_core.py index 16a003c1..bcc429be 100644 --- a/tests/unit_tests/test_core.py +++ b/tests/unit_tests/test_core.py @@ -1,4 +1,4 @@ -from goodmap.core import does_fulfill_requirement, get_queried_data, limit, sort_by_distance +from goodmap.core import get_queried_data, limit, sort_by_distance test_data = [ { @@ -31,123 +31,6 @@ def test_query(): assert get_queried_data(test_data, categories, query) == expected_data -def test_filtering(): - requirements = [("types", ["clothes"]), ("gender", ["male"])] - expected_data = [ - { - "name": "PCK", - "position": [51.1, 17.05], - "types": ["clothes"], - "gender": ["male"], - } - ] - filtered_data = list(filter(lambda x: does_fulfill_requirement(x, requirements), test_data)) - assert filtered_data == expected_data - - -def test_category_match_if_not_specified(): - requirements = [("types", []), ("gender", ["male"])] - expected_data = [ - { - "name": "LASSO", - "position": [51.113, 17.06], - "types": ["shoes"], - "gender": ["male", "female"], - }, - { - "name": "PCK", - "position": [51.1, 17.05], - "types": ["clothes"], - "gender": ["male"], - }, - ] - filtered_data = list(filter(lambda x: does_fulfill_requirement(x, requirements), test_data)) - assert filtered_data == expected_data - - -def test_multiple_selected_values_in_same_category_are_or_by_default(): - """Selecting several checkboxes in one category should broaden results (any-of), - not narrow them to entries containing every selected value.""" - requirements = [("gender", ["male", "female"])] - filtered_data = list(filter(lambda x: does_fulfill_requirement(x, requirements), test_data)) - assert filtered_data == test_data - - -def test_or_mode_is_explicit_default(): - requirements = [("gender", ["female"])] - filter_modes = {"gender": "or"} - filtered_data = list( - filter(lambda x: does_fulfill_requirement(x, requirements, filter_modes), test_data) - ) - assert filtered_data == [test_data[0]] - - -def test_and_mode_requires_every_selected_value(): - """"and" narrows results: an entry must have ALL selected values, useful - for list-valued categories like amenities ("lighting" AND "benches").""" - requirements = [("gender", ["male", "female"])] - filter_modes = {"gender": "and"} - - filtered_data = list( - filter(lambda x: does_fulfill_requirement(x, requirements, filter_modes), test_data) - ) - - # Only LASSO has both "male" and "female"; PCK (["male"]) doesn't match. - assert filtered_data == [test_data[0]] - - -def test_and_mode_matches_a_single_selected_value_like_or(): - requirements = [("gender", ["male"])] - filter_modes = {"gender": "and"} - - filtered_data = list( - filter(lambda x: does_fulfill_requirement(x, requirements, filter_modes), test_data) - ) - - assert filtered_data == test_data - - -def test_exclusive_mode_matches_the_single_selected_value(): - requirements = [("gender", ["female"])] - filter_modes = {"gender": "exclusive"} - filtered_data = list( - filter(lambda x: does_fulfill_requirement(x, requirements, filter_modes), test_data) - ) - assert filtered_data == [test_data[0]] - - -def test_threshold_mode_matches_values_at_or_below_the_highest_selected(): - speed_data = [ - {"name": "Zwierzyniecka", "speed_limit": "10"}, - {"name": "Milenijny", "speed_limit": "30"}, - {"name": "Grunwaldzki", "speed_limit": "50"}, - ] - requirements = [("speed_limit", ["30"])] - filter_modes = {"speed_limit": "threshold"} - - filtered_data = list( - filter(lambda x: does_fulfill_requirement(x, requirements, filter_modes), speed_data) - ) - - assert filtered_data == [speed_data[0], speed_data[1]] - - -def test_threshold_mode_uses_the_max_of_multiple_selected_values(): - speed_data = [ - {"name": "Zwierzyniecka", "speed_limit": "10"}, - {"name": "Milenijny", "speed_limit": "30"}, - {"name": "Grunwaldzki", "speed_limit": "50"}, - ] - requirements = [("speed_limit", ["10", "50"])] - filter_modes = {"speed_limit": "threshold"} - - filtered_data = list( - filter(lambda x: does_fulfill_requirement(x, requirements, filter_modes), speed_data) - ) - - assert filtered_data == speed_data - - def test_get_queried_data_applies_filter_modes(): categories = {"gender": ["male", "female"]} query = {"gender": ["female"]} diff --git a/tests/unit_tests/test_filtering.py b/tests/unit_tests/test_filtering.py new file mode 100644 index 00000000..d41eecc8 --- /dev/null +++ b/tests/unit_tests/test_filtering.py @@ -0,0 +1,133 @@ +from goodmap.filtering import does_fulfill_requirement + +test_data = [ + { + "name": "LASSO", + "position": [51.113, 17.06], + "types": ["shoes"], + "gender": ["male", "female"], + }, + { + "name": "PCK", + "position": [51.1, 17.05], + "types": ["clothes"], + "gender": ["male"], + }, +] + + +def test_filtering(): + requirements = [("types", ["clothes"]), ("gender", ["male"])] + expected_data = [ + { + "name": "PCK", + "position": [51.1, 17.05], + "types": ["clothes"], + "gender": ["male"], + } + ] + filtered_data = list(filter(lambda x: does_fulfill_requirement(x, requirements), test_data)) + assert filtered_data == expected_data + + +def test_category_match_if_not_specified(): + requirements = [("types", []), ("gender", ["male"])] + expected_data = [ + { + "name": "LASSO", + "position": [51.113, 17.06], + "types": ["shoes"], + "gender": ["male", "female"], + }, + { + "name": "PCK", + "position": [51.1, 17.05], + "types": ["clothes"], + "gender": ["male"], + }, + ] + filtered_data = list(filter(lambda x: does_fulfill_requirement(x, requirements), test_data)) + assert filtered_data == expected_data + + +def test_multiple_selected_values_in_same_category_are_or_by_default(): + """Selecting several checkboxes in one category should broaden results (any-of), + not narrow them to entries containing every selected value.""" + requirements = [("gender", ["male", "female"])] + filtered_data = list(filter(lambda x: does_fulfill_requirement(x, requirements), test_data)) + assert filtered_data == test_data + + +def test_or_mode_is_explicit_default(): + requirements = [("gender", ["female"])] + filter_modes = {"gender": "or"} + filtered_data = list( + filter(lambda x: does_fulfill_requirement(x, requirements, filter_modes), test_data) + ) + assert filtered_data == [test_data[0]] + + +def test_and_mode_requires_every_selected_value(): + """"and" narrows results: an entry must have ALL selected values, useful + for list-valued categories like amenities ("lighting" AND "benches").""" + requirements = [("gender", ["male", "female"])] + filter_modes = {"gender": "and"} + + filtered_data = list( + filter(lambda x: does_fulfill_requirement(x, requirements, filter_modes), test_data) + ) + + # Only LASSO has both "male" and "female"; PCK (["male"]) doesn't match. + assert filtered_data == [test_data[0]] + + +def test_and_mode_matches_a_single_selected_value_like_or(): + requirements = [("gender", ["male"])] + filter_modes = {"gender": "and"} + + filtered_data = list( + filter(lambda x: does_fulfill_requirement(x, requirements, filter_modes), test_data) + ) + + assert filtered_data == test_data + + +def test_exclusive_mode_matches_the_single_selected_value(): + requirements = [("gender", ["female"])] + filter_modes = {"gender": "exclusive"} + filtered_data = list( + filter(lambda x: does_fulfill_requirement(x, requirements, filter_modes), test_data) + ) + assert filtered_data == [test_data[0]] + + +def test_threshold_mode_matches_values_at_or_below_the_highest_selected(): + speed_data = [ + {"name": "Zwierzyniecka", "speed_limit": "10"}, + {"name": "Milenijny", "speed_limit": "30"}, + {"name": "Grunwaldzki", "speed_limit": "50"}, + ] + requirements = [("speed_limit", ["30"])] + filter_modes = {"speed_limit": "threshold"} + + filtered_data = list( + filter(lambda x: does_fulfill_requirement(x, requirements, filter_modes), speed_data) + ) + + assert filtered_data == [speed_data[0], speed_data[1]] + + +def test_threshold_mode_uses_the_max_of_multiple_selected_values(): + speed_data = [ + {"name": "Zwierzyniecka", "speed_limit": "10"}, + {"name": "Milenijny", "speed_limit": "30"}, + {"name": "Grunwaldzki", "speed_limit": "50"}, + ] + requirements = [("speed_limit", ["10", "50"])] + filter_modes = {"speed_limit": "threshold"} + + filtered_data = list( + filter(lambda x: does_fulfill_requirement(x, requirements, filter_modes), speed_data) + ) + + assert filtered_data == speed_data From 0a01dec8436b9c96fe43202d433f642c162477e4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Krzysztof=20Ko=C5=82odzi=C5=84ski?= Date: Tue, 28 Jul 2026 09:40:22 +0200 Subject: [PATCH 09/12] lint fixed --- goodmap/core.py | 4 +--- goodmap/db.py | 4 +--- tests/unit_tests/test_filtering.py | 2 +- 3 files changed, 3 insertions(+), 7 deletions(-) diff --git a/goodmap/core.py b/goodmap/core.py index ba4731d0..123eec0a 100644 --- a/goodmap/core.py +++ b/goodmap/core.py @@ -66,9 +66,7 @@ def get_queried_data(all_data, categories, query_params, filter_modes=NO_FILTER_ for key in categories.keys(): requirements.append((key, query_params.get(key))) - filtered_data = [ - x for x in all_data if does_fulfill_requirement(x, requirements, filter_modes) - ] + filtered_data = [x for x in all_data if does_fulfill_requirement(x, requirements, filter_modes)] final_data = sort_by_distance(filtered_data, query_params) final_data = limit(final_data, query_params) return final_data diff --git a/goodmap/db.py b/goodmap/db.py index 199af7eb..8d2ef51c 100644 --- a/goodmap/db.py +++ b/goodmap/db.py @@ -614,9 +614,7 @@ def json_db_get_category_data(self, category_type=None): ) }, "categories_filter_mode": { - category_type: self.data.get("categories_filter_mode", {}).get( - category_type, "or" - ) + category_type: self.data.get("categories_filter_mode", {}).get(category_type, "or") }, } return { diff --git a/tests/unit_tests/test_filtering.py b/tests/unit_tests/test_filtering.py index d41eecc8..92fd7090 100644 --- a/tests/unit_tests/test_filtering.py +++ b/tests/unit_tests/test_filtering.py @@ -68,7 +68,7 @@ def test_or_mode_is_explicit_default(): def test_and_mode_requires_every_selected_value(): - """"and" narrows results: an entry must have ALL selected values, useful + """ "and" narrows results: an entry must have ALL selected values, useful for list-valued categories like amenities ("lighting" AND "benches").""" requirements = [("gender", ["male", "female"])] filter_modes = {"gender": "and"} From 9b951138b82542a61ee4c12ca7b6675692bed8bd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Krzysztof=20Ko=C5=82odzi=C5=84ski?= Date: Tue, 28 Jul 2026 09:52:12 +0200 Subject: [PATCH 10/12] fixes after review --- e2e-tests/tests/basic/test_map.py | 6 ++- .../components/FiltersForm/FiltersForm.jsx | 17 +++++-- frontend/src/locales/en/map.json | 3 +- frontend/src/locales/pl/map.json | 3 +- frontend/src/locales/ua/map.json | 3 +- frontend/tests/FiltersForm.test.jsx | 15 +++--- goodmap/db.py | 13 +++-- tests/unit_tests/test_db.py | 50 +++++++++++++++++++ 8 files changed, 92 insertions(+), 18 deletions(-) diff --git a/e2e-tests/tests/basic/test_map.py b/e2e-tests/tests/basic/test_map.py index d61d61d0..98db6c8f 100644 --- a/e2e-tests/tests/basic/test_map.py +++ b/e2e-tests/tests/basic/test_map.py @@ -65,7 +65,7 @@ def test_displays_filter_list_with_four_categories(self, page: Page): expect(page.get_by_text("amenities")).to_be_visible() def mode_badge(symbol): - return page.get_by_label("Help:", exact=False).filter(has_text=symbol) + return page.get_by_label("Filter mode:", exact=False).filter(has_text=symbol) # check whether badges are there expect(mode_badge("+")).to_have_count(2) # accessible_by, type_of_place @@ -73,6 +73,10 @@ def mode_badge(symbol): expect(mode_badge("≤")).to_have_count(1) # speed_limit expect(mode_badge("•")).to_have_count(1) # is_free + # Badges are keyboard-focusable (not just hoverable), so their + # tooltip is reachable without a mouse. + expect(mode_badge("&")).to_have_attribute("tabindex", "0") + def test_should_not_have_scrollbars(self, page: Page): """Verify the page has no horizontal or vertical scrollbars""" # Get viewport and document dimensions diff --git a/frontend/src/components/FiltersForm/FiltersForm.jsx b/frontend/src/components/FiltersForm/FiltersForm.jsx index 28909f92..5b4e39b8 100644 --- a/frontend/src/components/FiltersForm/FiltersForm.jsx +++ b/frontend/src/components/FiltersForm/FiltersForm.jsx @@ -318,7 +318,12 @@ export const FiltersForm = () => { enterTouchDelay={0} leaveTouchDelay={3000} > - {t(keys.badge)} + + {t(keys.badge)} + ); }; @@ -408,15 +413,19 @@ export const FiltersForm = () => { const sections = otherCategories.map(category => { const { categoryKey, categoryName, categoriesHelp, filterMode } = category; - const sectionKey = `${categoryKey}-${categoryName}`; + // Built from categoryKey alone (not categoryName): aria-labelledby + // values are parsed as space-separated ID references, so an id built + // from a category's translated name (e.g. "accessible by") would + // silently break the association for any name containing whitespace. + const sectionId = `filter-label-${categoryKey}`; const categoryTooltip = globalThis.FEATURE_FLAGS?.CATEGORIES_HELP ? categoriesHelp.find(it => it[categoryKey]) : null; return ( - + - {categoryName} + {categoryName} {renderModeBadge(filterMode)} {categoryTooltip && } diff --git a/frontend/src/locales/en/map.json b/frontend/src/locales/en/map.json index 2685cdca..8679593b 100644 --- a/frontend/src/locales/en/map.json +++ b/frontend/src/locales/en/map.json @@ -49,5 +49,6 @@ "filterModeBooleanBadge": "•", "filterModeBooleanTooltip": "Unchecked shows everything; checking it narrows the results to just this", "filterModeThresholdBadge": "≤", - "filterModeThresholdTooltip": "Selecting a value also includes every lower value" + "filterModeThresholdTooltip": "Selecting a value also includes every lower value", + "filterModeHelpAriaLabel": "Filter mode: {{description}}" } diff --git a/frontend/src/locales/pl/map.json b/frontend/src/locales/pl/map.json index ec4f59f4..7c33f971 100644 --- a/frontend/src/locales/pl/map.json +++ b/frontend/src/locales/pl/map.json @@ -49,5 +49,6 @@ "filterModeBooleanBadge": "•", "filterModeBooleanTooltip": "Odznaczone pokazuje wszystko; zaznaczenie zawęża wyniki tylko do tego", "filterModeThresholdBadge": "≤", - "filterModeThresholdTooltip": "Wybranie wartości obejmuje też każdą niższą wartość" + "filterModeThresholdTooltip": "Wybranie wartości obejmuje też każdą niższą wartość", + "filterModeHelpAriaLabel": "Tryb filtra: {{description}}" } diff --git a/frontend/src/locales/ua/map.json b/frontend/src/locales/ua/map.json index 5449fa56..dc890d1a 100644 --- a/frontend/src/locales/ua/map.json +++ b/frontend/src/locales/ua/map.json @@ -48,5 +48,6 @@ "filterModeBooleanBadge": "•", "filterModeBooleanTooltip": "Не позначено - показує все; позначення звужує результати лише до цього", "filterModeThresholdBadge": "≤", - "filterModeThresholdTooltip": "Вибір значення також включає кожне нижче значення" + "filterModeThresholdTooltip": "Вибір значення також включає кожне нижче значення", + "filterModeHelpAriaLabel": "Режим фільтра: {{description}}" } diff --git a/frontend/tests/FiltersForm.test.jsx b/frontend/tests/FiltersForm.test.jsx index d779e5b5..5f10c79b 100644 --- a/frontend/tests/FiltersForm.test.jsx +++ b/frontend/tests/FiltersForm.test.jsx @@ -37,7 +37,7 @@ describe('Creates good filter_form box', () => { , ); await waitFor(() => - expect(document.querySelector('#filter-label-types-typy')).not.toBeNull(), + expect(document.querySelector('#filter-label-types')).not.toBeNull(), ); }); @@ -49,7 +49,7 @@ describe('Creates good filter_form box', () => { const form = document.querySelector('form'); expect(form).not.toBeNull(); - const filterLabel = form.querySelector('#filter-label-types-typy'); + const filterLabel = form.querySelector('#filter-label-types'); expect(filterLabel).not.toBeNull(); expect(filterLabel.textContent).toBe('typy'); @@ -88,7 +88,7 @@ describe('Creates good filter_form box', () => { // Category help tooltip is now in FilterHeader, not FilterTitle // Look for it in the parent FilterHeader element - const filterHeader = form.querySelector('#filter-label-types-typy').parentElement; + const filterHeader = form.querySelector('#filter-label-types').parentElement; const { queryByLabelText } = within(filterHeader); expect(queryByLabelText(/Help: Inaczej rodzaje/i)).toBeInTheDocument(); }); @@ -214,7 +214,7 @@ describe('Groups boolean categories into a shared "Others" section', () => { }); it('keeps non-boolean categories in their own titled section', () => { - expect(document.querySelector('#filter-label-types-typy')).not.toBeNull(); + expect(document.querySelector('#filter-label-types')).not.toBeNull(); expect(document.querySelector('#clothes')).not.toBeNull(); }); @@ -332,11 +332,12 @@ describe('Distinguishes "and" categories with a visible hint, but keeps checkbox expect(benches.checked).toBe(true); }); - it('shows an "&" badge with an explanatory tooltip next to the category title', () => { - const header = document.querySelector('#filter-label-amenities-amenities').parentElement; + it('shows an "&" badge with a keyboard-focusable, localized tooltip', () => { + const header = document.querySelector('#filter-label-amenities').parentElement; expect(header.textContent).toContain('&'); - const badge = within(header).getByLabelText(/Help:/i); + const badge = within(header).getByLabelText(/Filter mode:/i); expect(badge).toBeInTheDocument(); + expect(badge).toHaveAttribute('tabIndex', '0'); }); }); diff --git a/goodmap/db.py b/goodmap/db.py index 8d2ef51c..a059f831 100644 --- a/goodmap/db.py +++ b/goodmap/db.py @@ -822,16 +822,23 @@ def mongodb_db_get_locations(self, query, location_model): for key, values in query.items(): if not values: continue - if filter_modes.get(key) == "threshold": + mode = filter_modes.get(key, "or") + if mode == "threshold": # Threshold categories (e.g. speed limits) are numeric and ordered: # selecting a value also matches any stored value at or below it. # Assumes the field is stored numerically in MongoDB. try: mongo_query[key] = {"$lte": max(float(value) for value in values)} except (TypeError, ValueError): - continue + # Match nothing for this category, same as goodmap.filtering's + # _matches_threshold (which returns False on the same error), + # rather than silently dropping the filter and matching everything. + mongo_query[key] = {"$in": []} + elif mode == "and": + # Entry must have every selected value, not just any of them. + mongo_query[key] = {"$all": values} else: - # "or" and "exclusive" both match any of the selected values. + # "or", "exclusive", and "boolean" all match any of the selected values. mongo_query[key] = {"$in": values} projection = {"_id": 0, "uuid": 1, "position": 1, "remark": 1} diff --git a/tests/unit_tests/test_db.py b/tests/unit_tests/test_db.py index d71addd2..67e66fec 100644 --- a/tests/unit_tests/test_db.py +++ b/tests/unit_tests/test_db.py @@ -1625,6 +1625,56 @@ def test_mongodb_db_get_locations(mock_client): ) +@mock.patch("platzky.db.mongodb_db.MongoClient") +def test_mongodb_db_get_locations_and_filter_mode(mock_client): + """An "and" category must use $all (every selected value), not $in (any).""" + mock_db = mock.Mock() + mock_client.return_value.__getitem__.return_value = mock_db + mock_db.config.find_one.return_value = { + "_id": "map_config", + "categories_filter_mode": {"amenities": "and"}, + } + mock_db.locations.find.return_value = [{"uuid": "1", "position": [50, 50]}] + + db = MongoDB("mongodb://localhost:27017", "test_db") + extend_db_with_goodmap_queries(db, LocationBase) + + query = {"amenities": ["lighting", "benches"]} + locations = list(mongodb_db_get_locations(db, query, LocationBase)) + + assert len(locations) == 1 + mock_db.locations.find.assert_called_once_with( + {"amenities": {"$all": ["lighting", "benches"]}}, + {"_id": 0, "uuid": 1, "position": 1, "remark": 1}, + ) + + +@mock.patch("platzky.db.mongodb_db.MongoClient") +def test_mongodb_db_get_locations_threshold_parse_failure_matches_nothing(mock_client): + """A non-numeric threshold value should match nothing for that category, + the same as goodmap.filtering's _matches_threshold, rather than silently + dropping the filter (which would match everything).""" + mock_db = mock.Mock() + mock_client.return_value.__getitem__.return_value = mock_db + mock_db.config.find_one.return_value = { + "_id": "map_config", + "categories_filter_mode": {"speed_limit": "threshold"}, + } + mock_db.locations.find.return_value = [] + + db = MongoDB("mongodb://localhost:27017", "test_db") + extend_db_with_goodmap_queries(db, LocationBase) + + query = {"speed_limit": ["not-a-number"]} + locations = list(mongodb_db_get_locations(db, query, LocationBase)) + + assert locations == [] + mock_db.locations.find.assert_called_once_with( + {"speed_limit": {"$in": []}}, + {"_id": 0, "uuid": 1, "position": 1, "remark": 1}, + ) + + @mock.patch("platzky.db.mongodb_db.MongoClient") def test_mongodb_db_get_locations_empty_query(mock_client): mock_db = mock.Mock() From 14f560f97e58cc7c0781496e6b4e953d277aa7e2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Krzysztof=20Ko=C5=82odzi=C5=84ski?= Date: Tue, 28 Jul 2026 10:06:24 +0200 Subject: [PATCH 11/12] more fixes --- frontend/src/components/FiltersForm/FiltersForm.jsx | 8 ++++---- frontend/tests/FiltersForm.test.jsx | 4 +--- 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/frontend/src/components/FiltersForm/FiltersForm.jsx b/frontend/src/components/FiltersForm/FiltersForm.jsx index 5b4e39b8..6667064e 100644 --- a/frontend/src/components/FiltersForm/FiltersForm.jsx +++ b/frontend/src/components/FiltersForm/FiltersForm.jsx @@ -222,7 +222,7 @@ const ClearFiltersButton = styled.button` * Fetches category data from the API and renders checkboxes for each filter option. * Manages filter state through the Categories context. * - * @returns {React.ReactElement} Form element containing categorized filter checkboxes with optional tooltips + * @return {React.ReactElement} Form element containing categorized filter checkboxes with optional tooltips */ const LoadingSkeleton = () => ( <> @@ -249,7 +249,7 @@ export const FiltersForm = () => { const handleCheckboxChange = event => { const { value, checked } = event.target; - const category = event.target.dataset.category; + const { category } = event.target.dataset; setCategories(prevSelectedFilters => { const newSelectedFilters = { ...prevSelectedFilters }; @@ -270,7 +270,7 @@ export const FiltersForm = () => { // option replaces any previous selection instead of toggling it. const handleRadioChange = event => { const { value } = event.target; - const category = event.target.dataset.category; + const { category } = event.target.dataset; setCategories(prevSelectedFilters => ({ ...prevSelectedFilters, @@ -382,7 +382,7 @@ export const FiltersForm = () => { const renderBooleanFilterOption = category => { const { categoryKey, categoryName, options, optionsHelp } = category; - const trueOption = options.find(([name]) => name === 'true'); + const trueOption = options.find(([optionValue]) => optionValue === 'true'); if (!trueOption) { return null; } diff --git a/frontend/tests/FiltersForm.test.jsx b/frontend/tests/FiltersForm.test.jsx index 5f10c79b..6eae3bbf 100644 --- a/frontend/tests/FiltersForm.test.jsx +++ b/frontend/tests/FiltersForm.test.jsx @@ -36,9 +36,7 @@ describe('Creates good filter_form box', () => { , ); - await waitFor(() => - expect(document.querySelector('#filter-label-types')).not.toBeNull(), - ); + await waitFor(() => expect(document.querySelector('#filter-label-types')).not.toBeNull()); }); afterEach(() => { From ce8ccce8b26aa17819266cbf76af14deb971af5b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Krzysztof=20Ko=C5=82odzi=C5=84ski?= Date: Tue, 28 Jul 2026 10:17:59 +0200 Subject: [PATCH 12/12] linting fixes --- goodmap/core.py | 6 ++++-- goodmap/filtering.py | 14 ++++++++------ tests/unit_tests/test_core_api.py | 4 ++++ 3 files changed, 16 insertions(+), 8 deletions(-) diff --git a/goodmap/core.py b/goodmap/core.py index 123eec0a..c323560c 100644 --- a/goodmap/core.py +++ b/goodmap/core.py @@ -1,6 +1,6 @@ """Core data sorting, limiting, and query orchestration for location queries.""" -from typing import Any, Dict, List +from typing import Any, Dict, List, Mapping from goodmap.filtering import NO_FILTER_MODES, does_fulfill_requirement @@ -48,7 +48,9 @@ def limit(data, query_params): return data -def get_queried_data(all_data, categories, query_params, filter_modes=NO_FILTER_MODES): +def get_queried_data( + all_data, categories, query_params, filter_modes: Mapping[str, str] = NO_FILTER_MODES +): """Filter, sort, and limit location data based on query parameters. Args: diff --git a/goodmap/filtering.py b/goodmap/filtering.py index a2bc831c..351d39e1 100644 --- a/goodmap/filtering.py +++ b/goodmap/filtering.py @@ -6,7 +6,7 @@ """ from types import MappingProxyType -from typing import Mapping +from typing import Any, Mapping # Python has no builtin "frozendict" (the way frozenset mirrors set); # MappingProxyType is the standard-library equivalent - a read-only view that @@ -16,7 +16,7 @@ NO_FILTER_MODES: Mapping[str, str] = MappingProxyType({}) -def _as_list(value) -> list: +def _as_list(value: Any) -> list[str]: """Wrap a scalar field value in a list, leaving list values untouched.""" if value is None: return [] @@ -25,12 +25,12 @@ def _as_list(value) -> list: return [value] -def _matches_or(entry_values: list, selected_values: list) -> bool: +def _matches_or(entry_values: list[str], selected_values: list[str]) -> bool: """Match if the entry has at least one of the selected values (any-of).""" return any(value in entry_values for value in selected_values) -def _matches_and(entry_values: list, selected_values: list) -> bool: +def _matches_and(entry_values: list[str], selected_values: list[str]) -> bool: """Match only if the entry has every selected value (all-of). Only meaningful for list-valued categories (an entry can have multiple @@ -41,7 +41,7 @@ def _matches_and(entry_values: list, selected_values: list) -> bool: return all(value in entry_values for value in selected_values) -def _matches_threshold(entry_values: list, selected_values: list) -> bool: +def _matches_threshold(entry_values: list[str], selected_values: list[str]) -> bool: """Match if any entry value is numerically <= the highest selected value. Used for ordered numeric categories (e.g. speed limits) where selecting a @@ -67,7 +67,9 @@ def _matches_threshold(entry_values: list, selected_values: list) -> bool: } -def does_fulfill_requirement(entry, requirements, filter_modes=NO_FILTER_MODES): +def does_fulfill_requirement( + entry, requirements, filter_modes: Mapping[str, str] = NO_FILTER_MODES +): """Check if an entry fulfills all category requirements. Args: diff --git a/tests/unit_tests/test_core_api.py b/tests/unit_tests/test_core_api.py index 10c55b5a..76ad9cd0 100644 --- a/tests/unit_tests/test_core_api.py +++ b/tests/unit_tests/test_core_api.py @@ -187,6 +187,7 @@ def test_categories_full_endpoint_reports_configured_filter_mode(): ) response = test_app.get("/api/categories-full") assert response.status_code == 200 + assert response.json is not None category = response.json["categories"][0] assert category["filter_mode"] == "exclusive" @@ -355,6 +356,7 @@ def test_get_locations_multi_value_same_category_uses_or_semantics(): response = client.get("/api/locations?tags=red&tags=blue") assert response.status_code == 200 + assert response.json is not None uuids = {loc["uuid"] for loc in response.json} assert uuids == { "11111111-1111-1111-1111-111111111111", @@ -397,6 +399,7 @@ def test_get_locations_and_filter_mode_requires_every_selected_value(): response = client.get("/api/locations?amenities=lighting&amenities=benches") assert response.status_code == 200 + assert response.json is not None uuids = {loc["uuid"] for loc in response.json} assert uuids == {"11111111-1111-1111-1111-111111111111"} @@ -436,6 +439,7 @@ def test_get_locations_threshold_filter_mode(): response = client.get("/api/locations?speed_limit=30") assert response.status_code == 200 + assert response.json is not None uuids = {loc["uuid"] for loc in response.json} assert uuids == { "11111111-1111-1111-1111-111111111111",