From e9a06417ad64019ac4d1277c90627988149d2785 Mon Sep 17 00:00:00 2001 From: Andras Schmelczer Date: Fri, 15 May 2026 08:17:05 +0100 Subject: [PATCH] deploy --- frontend/src/components/map/AreaPane.tsx | 18 +- frontend/src/components/map/Filters.tsx | 69 ++- .../src/components/map/TravelTimeCard.tsx | 4 +- .../map/filters/ActiveFilterList.tsx | 394 +++++++++-------- .../map/filters/ActiveFiltersPanel.tsx | 21 +- .../components/map/filters/AddFilterPanel.tsx | 6 +- .../filters/ElectionVoteShareFilterCard.tsx | 4 +- .../map/filters/EthnicityFilterCard.tsx | 4 +- .../map/filters/NumericFeatureFilterCard.tsx | 4 +- .../map/filters/PoiDistanceFilterCard.tsx | 4 +- .../map/filters/SchoolFilterCard.tsx | 4 +- .../map/filters/SpecificCrimeFilterCard.tsx | 4 +- .../map/filters/TravelTimeFilterCards.tsx | 4 +- .../map/map-page/useExportController.ts | 4 +- frontend/src/hooks/useFilters.test.ts | 32 +- frontend/src/hooks/useFilters.ts | 8 +- frontend/src/hooks/useMapData.ts | 181 +++++--- frontend/src/hooks/useTravelTime.test.ts | 47 ++ frontend/src/hooks/useTravelTime.ts | 21 +- frontend/src/i18n/locales/de.ts | 5 +- frontend/src/i18n/locales/en.ts | 4 +- frontend/src/i18n/locales/fr.ts | 5 +- frontend/src/i18n/locales/hi.ts | 4 +- frontend/src/i18n/locales/hu.ts | 5 +- frontend/src/i18n/locales/zh.ts | 4 +- frontend/src/lib/travel-params.test.ts | 97 ++++ frontend/src/lib/travel-params.ts | 41 +- frontend/src/lib/url-state.test.ts | 39 ++ server-rs/src/routes/filter_counts.rs | 7 + server-rs/src/routes/hexagon_stats.rs | 414 ++++++++++++++++-- server-rs/src/routes/shorten.rs | 115 ++++- server-rs/src/routes/stripe_webhook.rs | 365 +++++++++++++-- 32 files changed, 1531 insertions(+), 407 deletions(-) create mode 100644 frontend/src/lib/travel-params.test.ts diff --git a/frontend/src/components/map/AreaPane.tsx b/frontend/src/components/map/AreaPane.tsx index a37addb..28c5e2b 100644 --- a/frontend/src/components/map/AreaPane.tsx +++ b/frontend/src/components/map/AreaPane.tsx @@ -75,12 +75,6 @@ function filterValueFormat(feature?: FeatureMeta) { }; } -function formatExclusionPercent(value: number): string { - const percent = value * 100; - if (percent < 10) return `${percent.toFixed(1)}%`; - return `${Math.round(percent)}%`; -} - export default function AreaPane({ stats, globalFeatures, @@ -144,6 +138,9 @@ export default function AreaPane({ }; const getExclusionAdjustment = (exclusion: FilterExclusion) => { + if (exclusion.direction === 'missing_value') { + return t('areaPane.missingFilterValue'); + } if (exclusion.direction === 'allow_value') { return t('areaPane.allowCategory', { value: ts(exclusion.category ?? '') }); } @@ -264,14 +261,7 @@ export default function AreaPane({ key={`${exclusion.kind}:${exclusion.name}:${exclusion.direction}:${exclusion.category ?? ''}`} className="rounded bg-white/70 px-2 py-1.5 dark:bg-navy-950/40" > -
- - {getExclusionLabel(exclusion)} - - - {formatExclusionPercent(exclusion.relative_difference)} - -
+
{getExclusionLabel(exclusion)}

{getExclusionAdjustment(exclusion)}

diff --git a/frontend/src/components/map/Filters.tsx b/frontend/src/components/map/Filters.tsx index 3a3bf7d..9d60e9b 100644 --- a/frontend/src/components/map/Filters.tsx +++ b/frontend/src/components/map/Filters.tsx @@ -5,6 +5,7 @@ import type { FeatureMeta, FeatureFilters } from '../../types'; import { findActiveFilterElement } from '../../lib/active-filter-scroll'; import { buildPercentileScale } from '../../lib/format'; import type { PercentileScale } from '../../lib/format'; +import { useCollapsibleGroups } from '../../hooks/useCollapsibleGroups'; import InfoPopup from '../ui/InfoPopup'; import { FeatureInfoPopup } from '../ui/FeatureInfoPopup'; import type { AiFilterErrorType } from '../../hooks/useAiFilters'; @@ -69,7 +70,7 @@ interface FiltersProps { onAddFilter: (name: string) => void; onRemoveFilter: (name: string) => void; onFilterChange: (name: string, value: [number, number] | string[]) => void; - onDragStart: (name: string) => void; + onDragStart: (name: string, initialValue?: [number, number]) => void; onDragChange: (value: [number, number]) => void; onDragEnd: () => void; pinnedFeature: string | null; @@ -423,46 +424,88 @@ export default memo(function Filters({ const [activeInfoFeature, setActiveInfoFeature] = useState(null); const [activeFilterCollapsed, setActiveFilterCollapsed] = useState(false); const [addFilterCollapsed, setAddFilterCollapsed] = useState(false); + const [isActiveFilterGroupExpanded, toggleActiveFilterGroup, expandActiveFilterGroup] = + useCollapsibleGroups(); const activeEntryCount = travelTimeEntries.length; const pendingScrollRef = useRef(null); const highlightTimeoutRef = useRef(null); + const queueActiveFilterScroll = useCallback( + (filterName: string, groupName: string | null | undefined) => { + if (groupName) expandActiveFilterGroup(groupName); + pendingScrollRef.current = filterName; + }, + [expandActiveFilterGroup] + ); + + const getAddFilterGroupName = useCallback( + (name: string): string | null => { + if (name === SCHOOL_FILTER_NAME) return schoolMeta.group ?? 'Education'; + if (name === SPECIFIC_CRIMES_FILTER_NAME) return specificCrimeMeta.group ?? 'Crime'; + if (name === ELECTION_VOTE_SHARE_FILTER_NAME) { + return electionVoteShareMeta.group ?? 'Neighbours'; + } + if (name === ETHNICITIES_FILTER_NAME) return ethnicityMeta.group ?? 'Neighbours'; + if (POI_FILTER_NAMES.includes(name as PoiFilterName)) { + return poiFilterMetas[name as PoiFilterName].group ?? null; + } + return features.find((feature) => feature.name === name)?.group ?? null; + }, + [ + electionVoteShareMeta.group, + ethnicityMeta.group, + features, + poiFilterMetas, + schoolMeta.group, + specificCrimeMeta.group, + ] + ); + const handleAddAndScroll = useCallback( (name: string) => { if (name === SCHOOL_FILTER_NAME) { if (!defaultSchoolFeatureName) return; - pendingScrollRef.current = SCHOOL_FILTER_NAME; + queueActiveFilterScroll(SCHOOL_FILTER_NAME, getAddFilterGroupName(SCHOOL_FILTER_NAME)); onAddFilter(SCHOOL_FILTER_NAME); return; } if (name === SPECIFIC_CRIMES_FILTER_NAME) { if (!defaultSpecificCrimeFeatureName) return; - pendingScrollRef.current = SPECIFIC_CRIMES_FILTER_NAME; + queueActiveFilterScroll( + SPECIFIC_CRIMES_FILTER_NAME, + getAddFilterGroupName(SPECIFIC_CRIMES_FILTER_NAME) + ); onAddFilter(SPECIFIC_CRIMES_FILTER_NAME); return; } if (name === ELECTION_VOTE_SHARE_FILTER_NAME) { if (!defaultElectionVoteShareFeatureName) return; - pendingScrollRef.current = ELECTION_VOTE_SHARE_FILTER_NAME; + queueActiveFilterScroll( + ELECTION_VOTE_SHARE_FILTER_NAME, + getAddFilterGroupName(ELECTION_VOTE_SHARE_FILTER_NAME) + ); onAddFilter(ELECTION_VOTE_SHARE_FILTER_NAME); return; } if (name === ETHNICITIES_FILTER_NAME) { if (!defaultEthnicityFeatureName) return; - pendingScrollRef.current = ETHNICITIES_FILTER_NAME; + queueActiveFilterScroll( + ETHNICITIES_FILTER_NAME, + getAddFilterGroupName(ETHNICITIES_FILTER_NAME) + ); onAddFilter(ETHNICITIES_FILTER_NAME); return; } if (POI_FILTER_NAMES.includes(name as PoiFilterName)) { const filterName = name as PoiFilterName; if (!defaultPoiFilterFeatureNames[filterName]) return; - pendingScrollRef.current = filterName; + queueActiveFilterScroll(filterName, getAddFilterGroupName(filterName)); onAddFilter(filterName); return; } - pendingScrollRef.current = name; + queueActiveFilterScroll(name, getAddFilterGroupName(name)); onAddFilter(name); }, [ @@ -471,16 +514,18 @@ export default memo(function Filters({ defaultElectionVoteShareFeatureName, defaultEthnicityFeatureName, defaultPoiFilterFeatureNames, + getAddFilterGroupName, onAddFilter, + queueActiveFilterScroll, ] ); const handleAddTravelTimeAndScroll = useCallback( (mode: TransportMode) => { - pendingScrollRef.current = `tt_${travelTimeEntries.length}`; + queueActiveFilterScroll(`tt_${travelTimeEntries.length}`, 'Transport'); onTravelTimeAddEntry(mode); }, - [onTravelTimeAddEntry, travelTimeEntries.length] + [onTravelTimeAddEntry, queueActiveFilterScroll, travelTimeEntries.length] ); useEffect(() => { @@ -516,9 +561,6 @@ export default memo(function Filters({ return scales; }, [features]); - // Keep commute controls at the top of active filters, before other Transport filters. - const travelInsertIdx = 0; - const badgeCount = enabledFeatureList.length + activeEntryCount; const [showClearPopup, setShowClearPopup] = useState(false); @@ -574,10 +616,11 @@ export default memo(function Filters({ dragValue={dragValue} pinnedFeature={pinnedFeature} travelTimeEntries={travelTimeEntries} - travelInsertIdx={travelInsertIdx} filterImpacts={filterImpacts} percentileScales={percentileScales} destinationDropdownPortal={destinationDropdownPortal} + isGroupExpanded={isActiveFilterGroupExpanded} + onToggleGroup={toggleActiveFilterGroup} aiFilterLoading={aiFilterLoading} aiFilterError={aiFilterError} aiFilterErrorType={aiFilterErrorType} diff --git a/frontend/src/components/map/TravelTimeCard.tsx b/frontend/src/components/map/TravelTimeCard.tsx index d644d06..db5b415 100644 --- a/frontend/src/components/map/TravelTimeCard.tsx +++ b/frontend/src/components/map/TravelTimeCard.tsx @@ -25,7 +25,7 @@ interface TravelTimeCardProps { onTogglePin: () => void; onSetDestination: (slug: string, label: string, lat: number, lon: number) => void; onTimeRangeChange: (range: [number, number]) => void; - onDragStart: () => void; + onDragStart: (range: [number, number]) => void; onDragChange: (value: [number, number]) => void; onDragEnd: () => void; onToggleBest: () => void; @@ -152,7 +152,7 @@ export function TravelTimeCard({ step={1} value={[displayRange[0], displayRange[1]]} onValueChange={([min, max]) => onDragChange([min, max])} - onPointerDown={() => onDragStart()} + onPointerDown={() => onDragStart(displayRange)} onPointerUp={() => onDragEnd()} />
diff --git a/frontend/src/components/map/filters/ActiveFilterList.tsx b/frontend/src/components/map/filters/ActiveFilterList.tsx index 8edd781..4cb41d1 100644 --- a/frontend/src/components/map/filters/ActiveFilterList.tsx +++ b/frontend/src/components/map/filters/ActiveFilterList.tsx @@ -1,7 +1,8 @@ -import { Fragment } from 'react'; +import { useMemo } from 'react'; import type { FeatureFilters, FeatureMeta } from '../../../types'; import type { PercentileScale } from '../../../lib/format'; +import { groupFeaturesByCategory } from '../../../lib/features'; import { getSpecificCrimeFeatureName, isSpecificCrimeFilterName } from '../../../lib/crime-filter'; import { getElectionVoteShareFeatureName, @@ -22,6 +23,9 @@ import { ElectionVoteShareFilterCard } from './ElectionVoteShareFilterCard'; import { EnumFeatureFilterCard } from './EnumFeatureFilterCard'; import { NumericFeatureFilterCard } from './NumericFeatureFilterCard'; import { TravelTimeFilterCards } from './TravelTimeFilterCards'; +import { CollapsibleGroupHeader } from '../../ui/CollapsibleGroupHeader'; + +const TRANSPORT_GROUP = 'Transport'; interface ActiveFilterListProps { features: FeatureMeta[]; @@ -31,13 +35,14 @@ interface ActiveFilterListProps { dragValue: [number, number] | null; pinnedFeature: string | null; travelTimeEntries: TravelTimeEntry[]; - travelInsertIdx: number; filterImpacts?: Record; percentileScales: Map; destinationDropdownPortal: boolean; + isGroupExpanded: (name: string) => boolean; + onToggleGroup: (name: string) => void; onFilterChange: (name: string, value: [number, number] | string[]) => void; onRemoveFilter: (name: string) => void; - onDragStart: (name: string) => void; + onDragStart: (name: string, initialValue?: [number, number]) => void; onDragChange: (value: [number, number]) => void; onDragEnd: () => void; onTogglePin: (name: string) => void; @@ -63,10 +68,11 @@ export function ActiveFilterList({ dragValue, pinnedFeature, travelTimeEntries, - travelInsertIdx, filterImpacts, percentileScales, destinationDropdownPortal, + isGroupExpanded, + onToggleGroup, onFilterChange, onRemoveFilter, onDragStart, @@ -99,194 +105,208 @@ export function ActiveFilterList({ /> ); + const groupedFeatures = useMemo(() => { + const groups = groupFeaturesByCategory(enabledFeatureList); + const transportGroup = groups.find((group) => group.name === TRANSPORT_GROUP); + const otherGroups = groups.filter((group) => group.name !== TRANSPORT_GROUP); + + if (transportGroup) return [transportGroup, ...otherGroups]; + if (travelTimeEntries.length > 0) + return [{ name: TRANSPORT_GROUP, features: [] }, ...otherGroups]; + return otherGroups; + }, [enabledFeatureList, travelTimeEntries.length]); + + const renderFeatureCard = (feature: FeatureMeta) => { + if (isSchoolFilterName(feature.name)) { + const schoolBackendName = getSchoolBackendFeatureName(feature.name); + return ( + onRemoveFilter(feature.name)} + /> + ); + } + + if (isSpecificCrimeFilterName(feature.name)) { + const specificCrimeBackendName = getSpecificCrimeFeatureName(feature.name); + return ( + onRemoveFilter(feature.name)} + /> + ); + } + + if (isElectionVoteShareFilterName(feature.name)) { + const electionVoteShareBackendName = getElectionVoteShareFeatureName(feature.name); + return ( + onRemoveFilter(feature.name)} + /> + ); + } + + if (isEthnicityFilterName(feature.name)) { + const ethnicityBackendName = getEthnicityFeatureName(feature.name); + return ( + onRemoveFilter(feature.name)} + /> + ); + } + + if (isPoiDistanceFilterName(feature.name)) { + const poiBackendName = getPoiDistanceFeatureName(feature.name); + return ( + onRemoveFilter(feature.name)} + /> + ); + } + + return feature.type === 'enum' ? ( + + ) : ( + + ); + }; + return ( -
- {enabledFeatureList.map((feature, featureIdx) => { - const insertTravelCards = featureIdx === travelInsertIdx; - - if (isSchoolFilterName(feature.name)) { - const schoolBackendName = getSchoolBackendFeatureName(feature.name); - return ( - - {insertTravelCards && travelCards} - onRemoveFilter(feature.name)} - /> - - ); - } - - if (isSpecificCrimeFilterName(feature.name)) { - const specificCrimeBackendName = getSpecificCrimeFeatureName(feature.name); - return ( - - {insertTravelCards && travelCards} - onRemoveFilter(feature.name)} - /> - - ); - } - - if (isElectionVoteShareFilterName(feature.name)) { - const electionVoteShareBackendName = getElectionVoteShareFeatureName(feature.name); - return ( - - {insertTravelCards && travelCards} - onRemoveFilter(feature.name)} - /> - - ); - } - - if (isEthnicityFilterName(feature.name)) { - const ethnicityBackendName = getEthnicityFeatureName(feature.name); - return ( - - {insertTravelCards && travelCards} - onRemoveFilter(feature.name)} - /> - - ); - } - - if (isPoiDistanceFilterName(feature.name)) { - const poiBackendName = getPoiDistanceFeatureName(feature.name); - return ( - - {insertTravelCards && travelCards} - onRemoveFilter(feature.name)} - /> - - ); - } - +
+ {groupedFeatures.map((group) => { + const travelCount = group.name === TRANSPORT_GROUP ? travelTimeEntries.length : 0; + const count = group.features.length + travelCount; + if (count === 0) return null; + const expanded = isGroupExpanded(group.name); return ( - - {insertTravelCards && travelCards} - {feature.type === 'enum' ? ( - - ) : ( - +
+ onToggleGroup(group.name)} + className="sticky top-0 z-10 px-3 py-2.5 text-sm font-bold text-navy-950 bg-warm-200 dark:bg-navy-900 dark:text-warm-100 hover:bg-warm-200 dark:hover:bg-warm-800" + > + {count} + + {expanded && ( +
+ {group.name === TRANSPORT_GROUP && travelCards} + {group.features.map((feature) => renderFeatureCard(feature))} +
)} - +
); })} - {travelInsertIdx >= enabledFeatureList.length && travelCards}
); } diff --git a/frontend/src/components/map/filters/ActiveFiltersPanel.tsx b/frontend/src/components/map/filters/ActiveFiltersPanel.tsx index 99b0b15..f1470fc 100644 --- a/frontend/src/components/map/filters/ActiveFiltersPanel.tsx +++ b/frontend/src/components/map/filters/ActiveFiltersPanel.tsx @@ -22,10 +22,11 @@ interface ActiveFiltersPanelProps { dragValue: [number, number] | null; pinnedFeature: string | null; travelTimeEntries: TravelTimeEntry[]; - travelInsertIdx: number; filterImpacts?: Record; percentileScales: Map; destinationDropdownPortal: boolean; + isGroupExpanded: (name: string) => boolean; + onToggleGroup: (name: string) => void; aiFilterLoading: boolean; aiFilterError: string | null; aiFilterErrorType: AiFilterErrorType | null; @@ -39,7 +40,7 @@ interface ActiveFiltersPanelProps { onLoginRequired: () => void; onFilterChange: (name: string, value: [number, number] | string[]) => void; onRemoveFilter: (name: string) => void; - onDragStart: (name: string) => void; + onDragStart: (name: string, initialValue?: [number, number]) => void; onDragChange: (value: [number, number]) => void; onDragEnd: () => void; onTogglePin: (name: string) => void; @@ -70,10 +71,11 @@ export function ActiveFiltersPanel({ dragValue, pinnedFeature, travelTimeEntries, - travelInsertIdx, filterImpacts, percentileScales, destinationDropdownPortal, + isGroupExpanded, + onToggleGroup, aiFilterLoading, aiFilterError, aiFilterErrorType, @@ -108,14 +110,14 @@ export function ActiveFiltersPanel({ > @@ -182,10 +184,11 @@ export function ActiveFiltersPanel({ dragValue={dragValue} pinnedFeature={pinnedFeature} travelTimeEntries={travelTimeEntries} - travelInsertIdx={travelInsertIdx} filterImpacts={filterImpacts} percentileScales={percentileScales} destinationDropdownPortal={destinationDropdownPortal} + isGroupExpanded={isGroupExpanded} + onToggleGroup={onToggleGroup} onFilterChange={onFilterChange} onRemoveFilter={onRemoveFilter} onDragStart={onDragStart} diff --git a/frontend/src/components/map/filters/AddFilterPanel.tsx b/frontend/src/components/map/filters/AddFilterPanel.tsx index 7fef896..e3c2a87 100644 --- a/frontend/src/components/map/filters/AddFilterPanel.tsx +++ b/frontend/src/components/map/filters/AddFilterPanel.tsx @@ -110,14 +110,14 @@ export function AddFilterPanel({ > {(!collapsed || !isLicensed) && ( diff --git a/frontend/src/components/map/filters/ElectionVoteShareFilterCard.tsx b/frontend/src/components/map/filters/ElectionVoteShareFilterCard.tsx index 1bb56dc..6fdc876 100644 --- a/frontend/src/components/map/filters/ElectionVoteShareFilterCard.tsx +++ b/frontend/src/components/map/filters/ElectionVoteShareFilterCard.tsx @@ -45,7 +45,7 @@ export function ElectionVoteShareFilterCard({ filterImpact?: number; percentileScale?: PercentileScale; onFilterChange: (name: string, value: [number, number] | string[]) => void; - onDragStart: (name: string) => void; + onDragStart: (name: string, initialValue?: [number, number]) => void; onDragChange: (value: [number, number]) => void; onDragEnd: () => void; onTogglePin: (name: string) => void; @@ -201,7 +201,7 @@ export function ElectionVoteShareFilterCard({ max >= (selectedFeature.max ?? dataMax) ? dataMax : max, ]) } - onPointerDown={() => onDragStart(voteShareFeature.name)} + onPointerDown={() => onDragStart(voteShareFeature.name, displayValue)} onPointerUp={() => onDragEnd()} /> void; - onDragStart: (name: string) => void; + onDragStart: (name: string, initialValue?: [number, number]) => void; onDragChange: (value: [number, number]) => void; onDragEnd: () => void; onTogglePin: (name: string) => void; @@ -197,7 +197,7 @@ export function EthnicityFilterCard({ max >= (selectedFeature.max ?? dataMax) ? dataMax : max, ]) } - onPointerDown={() => onDragStart(ethnicityFeature.name)} + onPointerDown={() => onDragStart(ethnicityFeature.name, displayValue)} onPointerUp={() => onDragEnd()} /> void; - onDragStart: (name: string) => void; + onDragStart: (name: string, initialValue?: [number, number]) => void; onDragChange: (value: [number, number]) => void; onDragEnd: () => void; onTogglePin: (name: string) => void; @@ -114,7 +114,7 @@ export function NumericFeatureFilterCard({ max >= feature.max! ? (hist?.max ?? feature.max!) : max, ]) } - onPointerDown={() => onDragStart(feature.name)} + onPointerDown={() => onDragStart(feature.name, displayValue)} onPointerUp={() => onDragEnd()} /> void; - onDragStart: (name: string) => void; + onDragStart: (name: string, initialValue?: [number, number]) => void; onDragChange: (value: [number, number]) => void; onDragEnd: () => void; onTogglePin: (name: string) => void; @@ -184,7 +184,7 @@ export function PoiDistanceFilterCard({ max >= sliderMax ? sliderMax : max, ]) } - onPointerDown={() => onDragStart(poiFeature.name)} + onPointerDown={() => onDragStart(poiFeature.name, displayValue)} onPointerUp={() => onDragEnd()} /> void; - onDragStart: (name: string) => void; + onDragStart: (name: string, initialValue?: [number, number]) => void; onDragChange: (value: [number, number]) => void; onDragEnd: () => void; onTogglePin: (name: string) => void; @@ -216,7 +216,7 @@ export function SchoolFilterCard({ max >= (backendFeature?.max ?? dataMax) ? dataMax : max, ]) } - onPointerDown={() => onDragStart(schoolFeature.name)} + onPointerDown={() => onDragStart(schoolFeature.name, displayValue)} onPointerUp={() => onDragEnd()} /> void; - onDragStart: (name: string) => void; + onDragStart: (name: string, initialValue?: [number, number]) => void; onDragChange: (value: [number, number]) => void; onDragEnd: () => void; onTogglePin: (name: string) => void; @@ -197,7 +197,7 @@ export function SpecificCrimeFilterCard({ max >= (selectedFeature.max ?? dataMax) ? dataMax : max, ]) } - onPointerDown={() => onDragStart(crimeFeature.name)} + onPointerDown={() => onDragStart(crimeFeature.name, displayValue)} onPointerUp={() => onDragEnd()} /> void; onTravelTimeDragEnd: (index: number) => void; onTravelTimeToggleBest: (index: number) => void; - onDragStart: (name: string) => void; + onDragStart: (name: string, initialValue?: [number, number]) => void; onDragChange: (value: [number, number]) => void; } @@ -60,7 +60,7 @@ export function TravelTimeFilterCards({ onTravelTimeSetDestination(index, slug, label, lat, lon) } onTimeRangeChange={(range) => onTravelTimeRangeChange(index, range)} - onDragStart={() => onDragStart(fieldKey)} + onDragStart={(range) => onDragStart(fieldKey, range)} onDragChange={onDragChange} onDragEnd={() => onTravelTimeDragEnd(index)} onToggleBest={() => onTravelTimeToggleBest(index)} diff --git a/frontend/src/components/map/map-page/useExportController.ts b/frontend/src/components/map/map-page/useExportController.ts index 64076bc..9fa9bcc 100644 --- a/frontend/src/components/map/map-page/useExportController.ts +++ b/frontend/src/components/map/map-page/useExportController.ts @@ -6,7 +6,7 @@ import { apiUrl, authHeaders, buildFilterString, logNonAbortError } from '../../ import { trackEvent } from '../../../lib/analytics'; import type { ExportNotice, ExportState } from './types'; import type { TravelTimeEntry } from '../../../hooks/useTravelTime'; -import { buildTravelParam } from '../../../lib/travel-params'; +import { buildTravelParam, dedupeTravelTimeEntries } from '../../../lib/travel-params'; const EXPORT_FILE_NAME = 'perfect-postcode-export.xlsx'; const EXPORT_TIMEOUT_MS = 150_000; @@ -68,7 +68,7 @@ function triggerExportDownload(blob: Blob, fileName: string): void { } function appendTravelStateParams(params: URLSearchParams, entries: TravelTimeEntry[]): void { - for (const entry of entries) { + for (const entry of dedupeTravelTimeEntries(entries)) { if (!entry.slug) continue; let value = `${entry.mode}:${entry.slug}:${encodeURIComponent(entry.label)}`; if (entry.useBest) value += ':b'; diff --git a/frontend/src/hooks/useFilters.test.ts b/frontend/src/hooks/useFilters.test.ts index 5d8133b..2a14e8f 100644 --- a/frontend/src/hooks/useFilters.test.ts +++ b/frontend/src/hooks/useFilters.test.ts @@ -27,21 +27,24 @@ describe('useFilters', () => { ); act(() => { - result.current.handleDragStart('price'); + result.current.handleDragStart('price', [0, 100]); }); expect(result.current.activeFeature).toBe('price'); expect(result.current.viewSource).toBe('drag'); + expect(result.current.dragValue).toEqual([0, 100]); + expect(result.current.filterRange).toEqual([0, 100]); act(() => { result.current.handleDragEnd(); }); expect(result.current.activeFeature).toBeNull(); + expect(result.current.dragValue).toBeNull(); expect(result.current.filters.price).toEqual([0, 100]); act(() => { - result.current.handleDragStart('price'); + result.current.handleDragStart('price', [0, 100]); result.current.handleDragChange([10, 90]); }); @@ -55,4 +58,29 @@ describe('useFilters', () => { expect(result.current.activeFeature).toBeNull(); expect(result.current.filters.price).toEqual([10, 90]); }); + + it('uses the provided initial range for drag-only feature keys', () => { + const { result } = renderHook(() => + useFilters({ + initialFilters: {}, + features, + }) + ); + + act(() => { + result.current.handleDragStart('tt_car_station', [15, 45]); + }); + + expect(result.current.activeFeature).toBe('tt_car_station'); + expect(result.current.dragValue).toEqual([15, 45]); + expect(result.current.filterRange).toEqual([15, 45]); + + act(() => { + result.current.handleDragEnd(); + }); + + expect(result.current.activeFeature).toBeNull(); + expect(result.current.dragValue).toBeNull(); + expect(result.current.filters).toEqual({}); + }); }); diff --git a/frontend/src/hooks/useFilters.ts b/frontend/src/hooks/useFilters.ts index a426e39..b455905 100644 --- a/frontend/src/hooks/useFilters.ts +++ b/frontend/src/hooks/useFilters.ts @@ -416,10 +416,12 @@ export function useFilters({ initialFilters, features }: UseFiltersOptions) { }, []); const handleDragStart = useCallback( - (name: string) => { + (name: string, initialValue?: [number, number]) => { const meta = features.find((f) => f.name === name); if (meta?.type === 'enum') return; pendingDragRef.current = name; + setDragValue(initialValue ?? null); + dragValueRef.current = initialValue ?? null; setActiveFeature(name); }, [features] @@ -440,6 +442,8 @@ export function useFilters({ initialFilters, features }: UseFiltersOptions) { // Click without drag — no filter value was changed, just clear preview state. pendingDragRef.current = null; setActiveFeature(null); + setDragValue(null); + dragValueRef.current = null; return; } const af = dragActiveRef.current; @@ -458,6 +462,8 @@ export function useFilters({ initialFilters, features }: UseFiltersOptions) { if (pendingDragRef.current) { pendingDragRef.current = null; setActiveFeature(null); + setDragValue(null); + dragValueRef.current = null; return null; } const dv = dragValueRef.current; diff --git a/frontend/src/hooks/useMapData.ts b/frontend/src/hooks/useMapData.ts index 7cbc841..b6f5b03 100644 --- a/frontend/src/hooks/useMapData.ts +++ b/frontend/src/hooks/useMapData.ts @@ -24,6 +24,7 @@ import { getPoiDistanceFeatureName } from '../lib/poi-distance-filter'; import { POSTCODE_ZOOM_THRESHOLD } from '../lib/consts'; import { COLOR_RANGE_LOW_PERCENTILE, COLOR_RANGE_HIGH_PERCENTILE } from '../lib/consts'; import { type TravelTimeEntry } from './useTravelTime'; +import { buildTravelParam as serializeTravelParam } from '../lib/travel-params'; /** Return the p-th percentile (0–100) from a sorted array via linear interpolation. */ function percentile(sorted: number[], p: number): number { @@ -70,12 +71,18 @@ export function useMapData({ longitude: number; zoom: number; } | null>(null); + const [currentVisibleView, setCurrentVisibleView] = useState<{ + latitude: number; + longitude: number; + zoom: number; + } | null>(null); const [licenseRequired, setLicenseRequired] = useState(false); const [freeZone, setFreeZone] = useState(null); // Drag preview state const [dragHexData, setDragHexData] = useState(null); const [dragPostcodeData, setDragPostcodeData] = useState(null); + const [dragDataKey, setDragDataKey] = useState(''); const dragFeatureRef = useRef(null); const dragAbortRef = useRef(null); const activeFeatureRef = useRef(null); @@ -119,32 +126,19 @@ export function useMapData({ ); const filtersParam = useMemo(() => buildFilterParam(), [buildFilterParam]); - // Build the travel param string from entries with destinations. - // Format: mode:slug[:best][:min:max] — server filters rows outside [min,max]. - // When excludeFieldKey is set, that entry uses a wide range (0:1440) instead of - // the committed range. This still filters out rows with no travel data (the server - // skips rows where minutes=None when any range is set) while including all actual values. + // Format: mode:slug[:best][:min:max]. For drag preview, the active travel + // filter uses an unbounded range so rows with travel data stay visible. const buildTravelParam = useCallback( - (excludeFieldKey?: string): string => { - const segments: string[] = []; - for (const entry of travelTimeEntries) { - if (!entry.slug) continue; - let seg = `${entry.mode}:${entry.slug}`; - if (entry.useBest) seg += ':best'; - const isExcluded = excludeFieldKey === `tt_${entry.mode}_${entry.slug}`; - if (isExcluded) { - seg += ':0:1440'; - } else if (entry.timeRange) { - seg += `:${entry.timeRange[0]}:${entry.timeRange[1]}`; - } - segments.push(seg); - } - return segments.join('|'); - }, + (excludeFieldKey?: string): string => + serializeTravelParam(travelTimeEntries, excludeFieldKey, true), [travelTimeEntries] ); const travelParam = useMemo(() => buildTravelParam(), [buildTravelParam]); + const filterStateKey = useMemo( + () => `${filtersParam}|${travelParam}`, + [filtersParam, travelParam] + ); const boundsParam = useMemo( () => (bounds ? `${bounds.south},${bounds.west},${bounds.north},${bounds.east}` : ''), [bounds] @@ -176,28 +170,13 @@ export function useMapData({ ] ); const [loadedDataKey, setLoadedDataKey] = useState(''); - - // Keep activeFeatureRef in sync - useEffect(() => { - activeFeatureRef.current = activeFeature; - }, [activeFeature]); - - useEffect(() => { - if (activeFeature) return; - latestDragRequestKeyRef.current = ''; - dragFeatureRef.current = null; - setDragHexData(null); - setDragPostcodeData(null); - }, [activeFeature]); - - // Drag prefetch: when activeFeature starts, fetch data excluding that filter. - // For regular filters: excludes the filter from the filter string. - // For travel time: excludes the time range from that entry's travel param segment. - useEffect(() => { - if (!activeFeature || !bounds) return; - - if (dragAbortRef.current) dragAbortRef.current.abort(); - dragAbortRef.current = new AbortController(); + const previousDragStateRef = useRef<{ activeFeature: string | null; filterStateKey: string }>({ + activeFeature: null, + filterStateKey, + }); + const resetPreviewScaleAfterSliderRef = useRef(false); + const activeDragRequest = useMemo(() => { + if (!activeFeature || !bounds) return null; const filtersStr = buildFilterString(filters, features, activeFeature); const boundsStr = `${bounds.south},${bounds.west},${bounds.north},${bounds.east}`; @@ -218,7 +197,58 @@ export function useMapData({ viewFeatureIsEnum && dataViewFeature ? dataViewFeature : '', shareCode ?? '', ].join('|'); + + return { boundsStr, dragTravelParam, fieldsParam, filtersStr, requestKey }; + }, [ + activeFeature, + bounds, + buildTravelParam, + dataViewFeature, + features, + filters, + getBackendFeatureName, + resolution, + shareCode, + travelParam, + usePostcodeView, + viewFeatureIsEnum, + ]); + + // Keep activeFeatureRef in sync + useEffect(() => { + activeFeatureRef.current = activeFeature; + }, [activeFeature]); + + useEffect(() => { + const previous = previousDragStateRef.current; + if (!activeFeature && previous.activeFeature && previous.filterStateKey !== filterStateKey) { + resetPreviewScaleAfterSliderRef.current = true; + } + previousDragStateRef.current = { activeFeature, filterStateKey }; + }, [activeFeature, filterStateKey]); + + useEffect(() => { + if (activeFeature) return; + latestDragRequestKeyRef.current = ''; + dragFeatureRef.current = null; + setDragDataKey(''); + setDragHexData(null); + setDragPostcodeData(null); + }, [activeFeature]); + + // Drag prefetch: when activeFeature starts, fetch data excluding that filter. + // For regular filters: excludes the filter from the filter string. + // For travel time: excludes the time range from that entry's travel param segment. + useEffect(() => { + if (!activeFeature || !activeDragRequest) return; + + if (dragAbortRef.current) dragAbortRef.current.abort(); + dragAbortRef.current = new AbortController(); + + const { boundsStr, dragTravelParam, fieldsParam, filtersStr, requestKey } = activeDragRequest; latestDragRequestKeyRef.current = requestKey; + setDragDataKey(''); + dragFeatureRef.current = null; if (usePostcodeView) { const params = new URLSearchParams({ bounds: boundsStr }); @@ -234,6 +264,7 @@ export function useMapData({ if (latestDragRequestKeyRef.current !== requestKey) return; setDragPostcodeData(json.features); setDragHexData(null); + setDragDataKey(requestKey); dragFeatureRef.current = activeFeature; }) .catch((err) => logNonAbortError('Failed to fetch drag postcode data', err)); @@ -254,6 +285,7 @@ export function useMapData({ if (latestDragRequestKeyRef.current !== requestKey) return; setDragHexData(json.features); setDragPostcodeData(null); + setDragDataKey(requestKey); dragFeatureRef.current = activeFeature; }) .catch((err) => logNonAbortError('Failed to fetch drag hex data', err)); @@ -270,15 +302,9 @@ export function useMapData({ }; }, [ activeFeature, - bounds, - resolution, - filters, - features, - usePostcodeView, - travelParam, - buildTravelParam, + activeDragRequest, dataViewFeature, - getBackendFeatureName, + usePostcodeView, viewFeatureIsEnum, shareCode, ]); @@ -386,6 +412,7 @@ export function useMapData({ if (!activeFeatureRef.current) { setDragHexData(null); setDragPostcodeData(null); + setDragDataKey(''); dragFeatureRef.current = null; } setLoading(false); @@ -420,18 +447,16 @@ export function useMapData({ shareCode, ]); - // Use drag data when it matches the current view feature, otherwise fall back to rawData - const data = - (activeFeature && viewFeature && dragFeatureRef.current === viewFeature ? dragHexData : null) ?? - rawData; - const effectivePostcodeData = - (activeFeature && viewFeature && dragFeatureRef.current === viewFeature - ? dragPostcodeData - : null) ?? postcodeData; + // Use drag data only when it matches the current view feature and request key. + const hasMatchingDragData = + Boolean(activeFeature && viewFeature && activeDragRequest) && + dragFeatureRef.current === viewFeature && + dragDataKey === activeDragRequest?.requestKey; + const data = (hasMatchingDragData ? dragHexData : null) ?? rawData; + const effectivePostcodeData = (hasMatchingDragData ? dragPostcodeData : null) ?? postcodeData; - // Compute p5/p95 from committed data for the viewed feature. - // Always uses rawData/postcodeData (not drag preview data) so the color - // scale stays stable while dragging a filter slider. + // Compute p5/p95 from the data currently being drawn. During slider drags + // this uses the drag-preview data so the colour scale resets to that preview. const dataRange = useMemo((): [number, number] | null => { if (!dataViewFeature) return null; @@ -445,8 +470,8 @@ export function useMapData({ const vals: number[] = []; if (usePostcodeView) { - if (postcodeData.length === 0) return null; - for (const feat of postcodeData) { + if (effectivePostcodeData.length === 0) return null; + for (const feat of effectivePostcodeData) { if (bounds) { const [lng, lat] = feat.properties.centroid as [number, number]; if (lat < bounds.south || lat > bounds.north || lng < bounds.west || lng > bounds.east) @@ -456,8 +481,8 @@ export function useMapData({ if (typeof val === 'number' && !isNaN(val)) vals.push(val); } } else { - if (rawData.length === 0) return null; - for (const item of rawData) { + if (data.length === 0) return null; + for (const item of data) { if (bounds) { const { lat, lon } = item; if (lat < bounds.south || lat > bounds.north || lon < bounds.west || lon > bounds.east) @@ -474,7 +499,7 @@ export function useMapData({ percentile(vals, COLOR_RANGE_LOW_PERCENTILE), percentile(vals, COLOR_RANGE_HIGH_PERCENTILE), ]; - }, [dataViewFeature, rawData, postcodeData, usePostcodeView, features, bounds]); + }, [dataViewFeature, data, effectivePostcodeData, usePostcodeView, features, bounds]); // Live color range for the legend and hex coloring. const liveColorRange = useMemo((): [number, number] | null => { @@ -505,7 +530,10 @@ export function useMapData({ useEffect(() => { setFrozenPreviewRange((prev) => { - if (!pinnedDataViewFeature) return prev ? null : prev; + if (!pinnedDataViewFeature) { + resetPreviewScaleAfterSliderRef.current = false; + return prev ? null : prev; + } return prev?.feature === pinnedDataViewFeature ? prev : null; }); }, [pinnedDataViewFeature]); @@ -524,11 +552,13 @@ export function useMapData({ : null; if (!rangeToFreeze) return; + const resetAfterSlider = resetPreviewScaleAfterSliderRef.current; setFrozenPreviewRange((prev) => - prev?.feature === pinnedDataViewFeature - ? prev - : { feature: pinnedDataViewFeature, range: rangeToFreeze } + resetAfterSlider || prev?.feature !== pinnedDataViewFeature + ? { feature: pinnedDataViewFeature, range: rangeToFreeze } + : prev ); + if (resetAfterSlider) resetPreviewScaleAfterSliderRef.current = false; }, [ dataRange, dataRequestKey, @@ -583,6 +613,8 @@ export function useMapData({ zoom: newZoom, latitude, longitude, + visibleLatitude, + visibleLongitude, }: ViewChangeParams) => { const boundsKey = `${newBounds.south},${newBounds.west},${newBounds.north},${newBounds.east},${newRes}`; if (boundsKey !== prevBoundsRef.current) { @@ -592,6 +624,11 @@ export function useMapData({ } setZoom(newZoom); setCurrentView({ latitude, longitude, zoom: newZoom }); + setCurrentVisibleView({ + latitude: visibleLatitude ?? latitude, + longitude: visibleLongitude ?? longitude, + zoom: newZoom, + }); }, [] ); @@ -599,6 +636,7 @@ export function useMapData({ const setInitialView = useCallback( (view: { latitude: number; longitude: number; zoom: number }) => { setCurrentView(view); + setCurrentVisibleView(view); setZoom(view.zoom); }, [] @@ -613,6 +651,7 @@ export function useMapData({ loading, zoom, currentView, + currentVisibleView, usePostcodeView, colorRange, canResetPreviewScale, diff --git a/frontend/src/hooks/useTravelTime.test.ts b/frontend/src/hooks/useTravelTime.test.ts index d598495..a5b40fb 100644 --- a/frontend/src/hooks/useTravelTime.test.ts +++ b/frontend/src/hooks/useTravelTime.test.ts @@ -65,4 +65,51 @@ describe('useTravelTime', () => { expect(result.current.entries).toEqual([replacement]); expect(result.current.activeEntries).toEqual([replacement]); }); + + it('deduplicates initial and replacement entries using the tightest range', () => { + const wide: TravelTimeEntry = { + mode: 'transit', + slug: 'bank-tube-station', + label: 'Bank', + timeRange: [0, 60], + useBest: false, + }; + const tight: TravelTimeEntry = { + mode: 'transit', + slug: 'bank-tube-station', + label: 'Bank', + timeRange: [10, 45], + useBest: false, + }; + const replacement: TravelTimeEntry = { + mode: 'transit', + slug: 'bank-tube-station', + label: 'Bank', + timeRange: [20, 40], + useBest: true, + }; + const { result } = renderHook(() => useTravelTime({ entries: [wide, tight] })); + + expect(result.current.entries).toEqual([ + { + mode: 'transit', + slug: 'bank-tube-station', + label: 'Bank', + timeRange: [10, 45], + useBest: false, + }, + ]); + + act(() => result.current.handleSetEntries([wide, replacement])); + + expect(result.current.entries).toEqual([ + { + mode: 'transit', + slug: 'bank-tube-station', + label: 'Bank', + timeRange: [20, 40], + useBest: true, + }, + ]); + }); }); diff --git a/frontend/src/hooks/useTravelTime.ts b/frontend/src/hooks/useTravelTime.ts index 1ae2c49..c49c8f0 100644 --- a/frontend/src/hooks/useTravelTime.ts +++ b/frontend/src/hooks/useTravelTime.ts @@ -2,6 +2,7 @@ import { useState, useCallback, useMemo } from 'react'; import type { ComponentType } from 'react'; import { useTranslation } from 'react-i18next'; import { CarIcon, BicycleIcon, WalkingIcon, TransitIcon } from '../components/ui/icons'; +import { dedupeTravelTimeEntries } from '../lib/travel-params'; export type TransportMode = 'car' | 'bicycle' | 'walking' | 'transit'; @@ -75,7 +76,9 @@ export interface TravelTimeInitial { } export function useTravelTime(initial?: TravelTimeInitial) { - const [entries, setEntries] = useState(initial?.entries ?? []); + const [entries, setEntries] = useState(() => + dedupeTravelTimeEntries(initial?.entries ?? []) + ); const handleAddEntry = useCallback((mode: TransportMode) => { setEntries((prev) => [...prev, { mode, slug: '', label: '', timeRange: null, useBest: false }]); @@ -87,26 +90,32 @@ export function useTravelTime(initial?: TravelTimeInitial) { const handleSetDestination = useCallback((index: number, slug: string, label: string) => { setEntries((prev) => - prev.map((entry, i) => - i === index ? { ...entry, slug, label, timeRange: slug ? [0, 120] : null } : entry + dedupeTravelTimeEntries( + prev.map((entry, i) => + i === index ? { ...entry, slug, label, timeRange: slug ? [0, 120] : null } : entry + ) ) ); }, []); const handleTimeRangeChange = useCallback((index: number, range: [number, number]) => { setEntries((prev) => - prev.map((entry, i) => (i === index ? { ...entry, timeRange: range } : entry)) + dedupeTravelTimeEntries( + prev.map((entry, i) => (i === index ? { ...entry, timeRange: range } : entry)) + ) ); }, []); const handleToggleBest = useCallback((index: number) => { setEntries((prev) => - prev.map((entry, i) => (i === index ? { ...entry, useBest: !entry.useBest } : entry)) + dedupeTravelTimeEntries( + prev.map((entry, i) => (i === index ? { ...entry, useBest: !entry.useBest } : entry)) + ) ); }, []); const handleSetEntries = useCallback((newEntries: TravelTimeEntry[]) => { - setEntries(newEntries); + setEntries(dedupeTravelTimeEntries(newEntries)); }, []); /** Entries that have a destination selected (slug is set) */ diff --git a/frontend/src/i18n/locales/de.ts b/frontend/src/i18n/locales/de.ts index d47c8f9..e415258 100644 --- a/frontend/src/i18n/locales/de.ts +++ b/frontend/src/i18n/locales/de.ts @@ -829,10 +829,13 @@ const de: Translations = { showAllStatsFallback: 'Wechseln Sie zu allen Immobilien, um dieses Gebiet ohne aktive Filter zu prüfen.', showAllStats: 'Alle Immobilien anzeigen', - closestBlockingFilters: 'Nächste Filter, die dieses Gebiet ausschließen', + closestBlockingFilters: 'Nächste Änderungen, um dieses Gebiet einzuschließen', lowerMinTo: 'Minimum auf {{value}} senken', raiseMaxTo: 'Maximum auf {{value}} erhöhen', allowCategory: '{{value}} zulassen', + missingFilterValue: + 'Kein Wert für diesen Filter; entfernen Sie ihn oder lassen Sie fehlende Werte zu', + noFilterDataShort: 'Keine Daten', travelTo: 'Fahrt zu {{destination}}', viewProperties: '{{count}} Immobilien ansehen', viewPropertiesShort: 'Immobilien ansehen', diff --git a/frontend/src/i18n/locales/en.ts b/frontend/src/i18n/locales/en.ts index 286d8af..dc53a27 100644 --- a/frontend/src/i18n/locales/en.ts +++ b/frontend/src/i18n/locales/en.ts @@ -802,10 +802,12 @@ const en = { showAllStatsFallback: 'Switch to all properties to inspect this area without the active filters.', showAllStats: 'Show all properties', - closestBlockingFilters: 'Closest filters excluding this area', + closestBlockingFilters: 'Closest changes to include this area', lowerMinTo: 'Lower minimum to {{value}}', raiseMaxTo: 'Raise maximum to {{value}}', allowCategory: 'Allow {{value}}', + missingFilterValue: 'No value for this filter; remove it or allow missing values', + noFilterDataShort: 'No data', travelTo: 'Travel to {{destination}}', viewProperties: 'View {{count}} Properties', viewPropertiesShort: 'View properties', diff --git a/frontend/src/i18n/locales/fr.ts b/frontend/src/i18n/locales/fr.ts index 65f8c14..140d471 100644 --- a/frontend/src/i18n/locales/fr.ts +++ b/frontend/src/i18n/locales/fr.ts @@ -834,10 +834,13 @@ const fr: Translations = { showAllStatsFallback: 'Passez à toutes les propriétés pour inspecter cette zone sans les filtres actifs.', showAllStats: 'Afficher toutes les propriétés', - closestBlockingFilters: 'Filtres les plus proches qui excluent cette zone', + closestBlockingFilters: 'Modifications les plus proches pour inclure cette zone', lowerMinTo: 'Abaisser le minimum à {{value}}', raiseMaxTo: 'Augmenter le maximum à {{value}}', allowCategory: 'Autoriser {{value}}', + missingFilterValue: + 'Aucune valeur pour ce filtre ; supprimez-le ou autorisez les valeurs manquantes', + noFilterDataShort: 'Aucune donnée', travelTo: 'Trajet vers {{destination}}', viewProperties: 'Voir {{count}} propriétés', viewPropertiesShort: 'Voir les propriétés', diff --git a/frontend/src/i18n/locales/hi.ts b/frontend/src/i18n/locales/hi.ts index 2b4adc9..910dcce 100644 --- a/frontend/src/i18n/locales/hi.ts +++ b/frontend/src/i18n/locales/hi.ts @@ -792,10 +792,12 @@ const hi: Translations = { showAllStatsFallback: 'सक्रिय फिल्टर के बिना इस क्षेत्र को देखने के लिए सभी संपत्तियों पर जाएं.', showAllStats: 'सभी संपत्तियां दिखाएं', - closestBlockingFilters: 'इस क्षेत्र को बाहर करने वाले निकटतम फिल्टर', + closestBlockingFilters: 'इस क्षेत्र को शामिल करने के निकटतम बदलाव', lowerMinTo: 'न्यूनतम को {{value}} तक घटाएं', raiseMaxTo: 'अधिकतम को {{value}} तक बढ़ाएं', allowCategory: '{{value}} की अनुमति दें', + missingFilterValue: 'इस फिल्टर के लिए कोई मान नहीं है; इसे हटाएं या गायब मानों की अनुमति दें', + noFilterDataShort: 'कोई डेटा नहीं', travelTo: '{{destination}} तक यात्रा', viewProperties: '{{count}} संपत्तियां देखें', viewPropertiesShort: 'संपत्तियां देखें', diff --git a/frontend/src/i18n/locales/hu.ts b/frontend/src/i18n/locales/hu.ts index ab500ae..491004f 100644 --- a/frontend/src/i18n/locales/hu.ts +++ b/frontend/src/i18n/locales/hu.ts @@ -815,10 +815,13 @@ const hu: Translations = { showAllStatsFallback: 'Váltson az összes ingatlanra, hogy aktív szűrők nélkül tekintse át ezt a területet.', showAllStats: 'Összes ingatlan mutatása', - closestBlockingFilters: 'A területet kizáró legközelebbi szűrők', + closestBlockingFilters: 'A terület bevonásához legközelebbi módosítások', lowerMinTo: 'Minimum csökkentése erre: {{value}}', raiseMaxTo: 'Maximum növelése erre: {{value}}', allowCategory: '{{value}} engedélyezése', + missingFilterValue: + 'Ehhez a szűrőhöz nincs érték; távolítsa el, vagy engedélyezze a hiányzó értékeket', + noFilterDataShort: 'Nincs adat', travelTo: 'Utazás ide: {{destination}}', viewProperties: '{{count}} ingatlan megtekintése', viewPropertiesShort: 'Ingatlanok megtekintése', diff --git a/frontend/src/i18n/locales/zh.ts b/frontend/src/i18n/locales/zh.ts index 8482678..c48d174 100644 --- a/frontend/src/i18n/locales/zh.ts +++ b/frontend/src/i18n/locales/zh.ts @@ -762,10 +762,12 @@ const zh: Translations = { showAllStatsHint: '筛选前这里有 {{count}} 处房产。切换到全部房产即可查看该区域。', showAllStatsFallback: '切换到全部房产即可在不应用当前筛选条件的情况下查看该区域。', showAllStats: '显示全部房产', - closestBlockingFilters: '最接近的排除此区域的筛选条件', + closestBlockingFilters: '纳入该区域所需的最小调整', lowerMinTo: '将最小值降至 {{value}}', raiseMaxTo: '将最大值提高至 {{value}}', allowCategory: '允许 {{value}}', + missingFilterValue: '此筛选条件没有值；请移除它或允许缺失值', + noFilterDataShort: '无数据', travelTo: '前往 {{destination}} 的出行', viewProperties: '查看 {{count}} 处房产', viewPropertiesShort: '查看房产', diff --git a/frontend/src/lib/travel-params.test.ts b/frontend/src/lib/travel-params.test.ts new file mode 100644 index 0000000..50ce5d1 --- /dev/null +++ b/frontend/src/lib/travel-params.test.ts @@ -0,0 +1,97 @@ +import { describe, expect, it } from 'vitest'; + +import type { TravelTimeEntry } from '../hooks/useTravelTime'; +import { buildTravelParam, dedupeTravelTimeEntries } from './travel-params'; + +const bankMedian: TravelTimeEntry = { + mode: 'transit', + slug: 'bank-tube-station', + label: 'Bank', + timeRange: [0, 60], + useBest: false, +}; + +describe('travel-params', () => { + it('deduplicates travel entries by backend key and keeps the tightest range', () => { + const entries = dedupeTravelTimeEntries([ + bankMedian, + { + mode: 'transit', + slug: 'bank-tube-station', + label: 'Bank tube station', + timeRange: [15, 45], + useBest: false, + }, + { + mode: 'walking', + slug: 'bank-tube-station', + label: 'Bank', + timeRange: [0, 20], + useBest: false, + }, + ]); + + expect(entries).toEqual([ + { + mode: 'transit', + slug: 'bank-tube-station', + label: 'Bank', + timeRange: [15, 45], + useBest: false, + }, + { + mode: 'walking', + slug: 'bank-tube-station', + label: 'Bank', + timeRange: [0, 20], + useBest: false, + }, + ]); + }); + + it('serializes deduplicated entries before backend requests', () => { + expect( + buildTravelParam([ + bankMedian, + { + mode: 'transit', + slug: 'bank-tube-station', + label: 'Bank', + timeRange: [10, 50], + useBest: false, + }, + ]) + ).toBe('transit:bank-tube-station:10:50'); + }); + + it('keeps duplicate blank entries because they are editable placeholders', () => { + const blank: TravelTimeEntry = { + mode: 'transit', + slug: '', + label: '', + timeRange: null, + useBest: false, + }; + + expect(dedupeTravelTimeEntries([blank, blank])).toHaveLength(2); + }); + + it('uses an unbounded range when excluding a deduplicated travel filter', () => { + expect( + buildTravelParam( + [ + bankMedian, + { + mode: 'transit', + slug: 'bank-tube-station', + label: 'Bank', + timeRange: [10, 50], + useBest: false, + }, + ], + 'tt_transit_bank-tube-station', + true + ) + ).toBe('transit:bank-tube-station:0:1440'); + }); +}); diff --git a/frontend/src/lib/travel-params.ts b/frontend/src/lib/travel-params.ts index 11676a6..f13753a 100644 --- a/frontend/src/lib/travel-params.ts +++ b/frontend/src/lib/travel-params.ts @@ -1,5 +1,44 @@ import type { TravelTimeEntry } from '../hooks/useTravelTime'; +function mergeTimeRanges( + current: [number, number] | null, + next: [number, number] | null +): [number, number] | null { + if (!current) return next; + if (!next) return current; + return [Math.max(current[0], next[0]), Math.min(current[1], next[1])]; +} + +export function dedupeTravelTimeEntries(entries: TravelTimeEntry[]): TravelTimeEntry[] { + const result: TravelTimeEntry[] = []; + const indexByKey = new Map(); + + for (const entry of entries) { + if (!entry.slug) { + result.push(entry); + continue; + } + + const key = `${entry.mode}:${entry.slug}`; + const existingIndex = indexByKey.get(key); + if (existingIndex == null) { + indexByKey.set(key, result.length); + result.push({ ...entry }); + continue; + } + + const existing = result[existingIndex]; + result[existingIndex] = { + ...existing, + label: existing.label || entry.label, + timeRange: mergeTimeRanges(existing.timeRange, entry.timeRange), + useBest: existing.useBest || entry.useBest, + }; + } + + return result; +} + export function buildTravelParam( entries: TravelTimeEntry[], excludeFieldKey?: string, @@ -7,7 +46,7 @@ export function buildTravelParam( ): string { const segments: string[] = []; - for (const entry of entries) { + for (const entry of dedupeTravelTimeEntries(entries)) { if (!entry.slug) continue; let segment = `${entry.mode}:${entry.slug}`; diff --git a/frontend/src/lib/url-state.test.ts b/frontend/src/lib/url-state.test.ts index 6e5d6e2..097bd49 100644 --- a/frontend/src/lib/url-state.test.ts +++ b/frontend/src/lib/url-state.test.ts @@ -99,6 +99,45 @@ describe('url-state', () => { expect(params.getAll('tt')).toEqual(['bicycle:bank:Bank:5:25']); }); + it('deduplicates travel-time URL params with the tightest range', () => { + window.history.replaceState( + {}, + '', + '/?tt=transit:bank-tube-station:Bank:0:60&tt=transit:bank-tube-station:Bank:10:45' + ); + + const state = parseUrlState(); + + expect(state.travelTime?.entries).toEqual([ + { + mode: 'transit', + slug: 'bank-tube-station', + label: 'Bank', + timeRange: [10, 45], + useBest: false, + }, + ]); + + const params = stateToParams(null, {}, [], new Set(), 'area', [ + { + mode: 'transit', + slug: 'bank-tube-station', + label: 'Bank', + useBest: false, + timeRange: [0, 60], + }, + { + mode: 'transit', + slug: 'bank-tube-station', + label: 'Bank', + useBest: false, + timeRange: [10, 45], + }, + ]); + + expect(params.getAll('tt')).toEqual(['transit:bank-tube-station:Bank:10:45']); + }); + it('round-trips an explicitly empty POI selection', () => { const params = stateToParams(null, {}, [], new Set(), 'area'); diff --git a/server-rs/src/routes/filter_counts.rs b/server-rs/src/routes/filter_counts.rs index 7e96989..991ba53 100644 --- a/server-rs/src/routes/filter_counts.rs +++ b/server-rs/src/routes/filter_counts.rs @@ -3,12 +3,15 @@ use std::sync::Arc; use axum::extract::{Query, State}; use axum::http::StatusCode; use axum::response::{IntoResponse, Json}; +use axum::Extension; use rustc_hash::FxHashMap; use serde::{Deserialize, Serialize}; use tracing::info; +use crate::auth::OptionalUser; use crate::consts::NAN_U16; use crate::data::travel_time::TravelData; +use crate::licensing::{check_license_bounds, resolve_share_code}; use crate::parsing::{parse_filters_with_poi, require_bounds}; use crate::routes::travel_time::parse_optional_travel; use crate::state::SharedState; @@ -18,6 +21,7 @@ pub struct FilterCountsParams { bounds: Option, filters: Option, travel: Option, + share: Option, } #[derive(Serialize)] @@ -28,12 +32,15 @@ pub struct FilterCountsResponse { pub async fn get_filter_counts( State(shared): State>, + Extension(user): Extension, Query(params): Query, ) -> Result, axum::response::Response> { let state = shared.load_state(); let (south, west, north, east) = require_bounds(params.bounds).map_err(IntoResponse::into_response)?; + let share_bounds = resolve_share_code(&state, params.share.as_deref()).await; + check_license_bounds(&user.0, (south, west, north, east), share_bounds)?; let quant = state.data.quant_ref(); let poi_quant = state.data.poi_metrics.quant_ref(); diff --git a/server-rs/src/routes/hexagon_stats.rs b/server-rs/src/routes/hexagon_stats.rs index 01226a1..44085f7 100644 --- a/server-rs/src/routes/hexagon_stats.rs +++ b/server-rs/src/routes/hexagon_stats.rs @@ -1,4 +1,4 @@ -use std::collections::HashMap; +use std::collections::{HashMap, HashSet}; use std::str::FromStr; use std::sync::Arc; @@ -11,15 +11,25 @@ use serde::{Deserialize, Serialize}; use tracing::{info, warn}; use crate::auth::OptionalUser; +use crate::consts::NAN_U16; +use crate::data::travel_time::TravelData; +use crate::data::PropertyData; +use crate::features::{Feature, FEATURE_GROUPS}; use crate::licensing::{check_license_bounds, resolve_share_code}; use crate::parsing::{ cell_for_row_cached, h3_cell_bounds, needs_parent, parse_field_set, parse_filters_with_poi, - row_passes_filters, row_passes_poi_filters, validate_h3_resolution, + row_passes_filters, row_passes_poi_filters, validate_h3_resolution, ParsedEnumFilter, + ParsedFilter, ParsedPoiFilter, }; use crate::state::SharedState; use super::stats; -use super::travel_time::{load_travel_data, parse_optional_travel, row_passes_travel_filters}; +use super::travel_time::{ + load_travel_data, parse_optional_travel, row_passes_travel_filters, TravelEntry, +}; + +const AREA_STATS_EXCLUDED_GROUPS: &[&str] = &["Amenities"]; +const MAX_FILTER_EXCLUSIONS: usize = 5; #[derive(Serialize)] pub struct HistogramStats { @@ -54,6 +64,47 @@ pub struct PricePoint { pub price: f32, } +#[derive(Serialize)] +pub struct FilterExclusion { + pub name: String, + pub kind: String, + pub direction: String, + #[serde(skip_serializing_if = "Option::is_none")] + pub value: Option, + #[serde(skip_serializing_if = "Option::is_none")] + pub min: Option, + #[serde(skip_serializing_if = "Option::is_none")] + pub max: Option, + #[serde(skip_serializing_if = "Option::is_none")] + pub category: Option, + pub relative_difference: f32, + pub rejected_count: usize, +} + +fn filter_exclusion_key(exclusion: &FilterExclusion) -> String { + format!( + "{}\u{1f}{}\u{1f}{}\u{1f}{}", + exclusion.kind, + exclusion.name, + exclusion.direction, + exclusion.category.as_deref().unwrap_or("") + ) +} + +fn missing_filter_exclusion(name: String, kind: &str) -> FilterExclusion { + FilterExclusion { + name, + kind: kind.to_string(), + direction: "missing_value".to_string(), + value: None, + min: None, + max: None, + category: None, + relative_difference: 1.0, + rejected_count: 0, + } +} + #[derive(Serialize)] pub struct HexagonStatsResponse { pub count: usize, @@ -63,6 +114,8 @@ pub struct HexagonStatsResponse { pub price_history: Vec, #[serde(skip_serializing_if = "Option::is_none")] pub central_postcode: Option, + #[serde(skip_serializing_if = "Vec::is_empty")] + pub filter_exclusions: Vec, } #[derive(Deserialize)] @@ -70,8 +123,9 @@ pub struct HexagonStatsParams { pub h3: String, pub resolution: u8, pub filters: Option, - /// Comma-separated feature names to include in stats response. - /// Only listed features are computed; if absent or empty, no features are returned. + /// `;;`-separated feature names to include in stats response. + /// Only listed features are computed. If absent, area stats default to + /// displayable groups; if empty, no feature stats are returned. pub fields: Option, /// When set (with journey_slug), pick central_postcode as the postcode with the /// shortest travel time for this mode+slug (so it has journey data). @@ -84,6 +138,271 @@ pub struct HexagonStatsParams { pub share: Option, } +fn default_area_stat_field_set() -> HashSet { + FEATURE_GROUPS + .iter() + .filter(|group| !AREA_STATS_EXCLUDED_GROUPS.contains(&group.name)) + .flat_map(|group| group.features.iter()) + .map(|feature| match feature { + Feature::Numeric(config) => config.name.to_string(), + Feature::Enum(config) => config.name.to_string(), + }) + .collect() +} + +pub(super) fn parse_area_stats_field_set(fields: Option<&str>) -> (bool, HashSet) { + let (fields_specified, field_set) = parse_field_set(fields); + if fields_specified { + return (fields_specified, field_set); + } + + (true, default_area_stat_field_set()) +} + +#[inline] +fn relative_difference(value: f32, min: f32, max: f32) -> Option<(String, f32)> { + let distance = if value < min { + min - value + } else if value > max { + value - max + } else { + return None; + }; + + let range = (max - min).abs(); + let denominator = if range.is_finite() && range > f32::EPSILON { + range + } else { + min.abs().max(max.abs()).max(1.0) + }; + let direction = if value < min { + "lower_min".to_string() + } else { + "raise_max".to_string() + }; + Some((direction, distance / denominator)) +} + +pub(super) fn top_filter_exclusions( + area_rows: &[usize], + numeric_filters: &[ParsedFilter], + enum_filters: &[ParsedEnumFilter], + poi_filters: &[ParsedPoiFilter], + travel_entries: &[TravelEntry], + travel_data: &[TravelData], + data: &PropertyData, +) -> Vec { + if area_rows.is_empty() + || (numeric_filters.is_empty() + && enum_filters.is_empty() + && poi_filters.is_empty() + && !travel_entries + .iter() + .any(|entry| entry.filter_min.is_some() && entry.filter_max.is_some())) + { + return Vec::new(); + } + + let feature_data = &data.feature_data; + let num_features = data.num_features; + let quant = data.quant_ref(); + let poi_quant = data.poi_metrics.quant_ref(); + let mut rejection_counts: HashMap = HashMap::new(); + let mut best_path: Option> = None; + + for &row in area_rows { + let mut path = Vec::new(); + + for filter in numeric_filters { + let min = quant.decode(filter.feat_idx, filter.min_u16); + let max = quant.decode(filter.feat_idx, filter.max_u16); + let raw = feature_data[row * num_features + filter.feat_idx]; + if raw == NAN_U16 { + path.push(missing_filter_exclusion( + data.feature_names[filter.feat_idx].clone(), + "numeric", + )); + continue; + } + + let value = quant.decode(filter.feat_idx, raw); + let Some((direction, rel_diff)) = relative_difference(value, min, max) else { + continue; + }; + + path.push(FilterExclusion { + name: data.feature_names[filter.feat_idx].clone(), + kind: "numeric".to_string(), + direction, + value: Some(value), + min: Some(min), + max: Some(max), + category: None, + relative_difference: rel_diff, + rejected_count: 0, + }); + } + + for filter in enum_filters { + let raw = feature_data[row * num_features + filter.feat_idx]; + if raw == NAN_U16 { + path.push(missing_filter_exclusion( + data.feature_names[filter.feat_idx].clone(), + "enum", + )); + continue; + } + if filter.allowed.contains(&raw) { + continue; + } + + let Some(values) = data.enum_values.get(&filter.feat_idx) else { + continue; + }; + let Some(category) = values.get(raw as usize) else { + continue; + }; + + path.push(FilterExclusion { + name: data.feature_names[filter.feat_idx].clone(), + kind: "enum".to_string(), + direction: "allow_value".to_string(), + value: None, + min: None, + max: None, + category: Some(category.clone()), + relative_difference: 1.0, + rejected_count: 0, + }); + } + + for filter in poi_filters { + let min = poi_quant.decode(filter.metric_idx, filter.min_u16); + let max = poi_quant.decode(filter.metric_idx, filter.max_u16); + let raw = data + .poi_metrics + .raw_for_property_row(row, filter.metric_idx); + if raw == NAN_U16 { + path.push(missing_filter_exclusion( + data.poi_metrics.feature_names[filter.metric_idx].clone(), + "poi", + )); + continue; + } + + let value = poi_quant.decode(filter.metric_idx, raw); + let Some((direction, rel_diff)) = relative_difference(value, min, max) else { + continue; + }; + + path.push(FilterExclusion { + name: data.poi_metrics.feature_names[filter.metric_idx].clone(), + kind: "poi".to_string(), + direction, + value: Some(value), + min: Some(min), + max: Some(max), + category: None, + relative_difference: rel_diff, + rejected_count: 0, + }); + } + + for (filter_index, entry) in travel_entries.iter().enumerate() { + let (Some(min), Some(max)) = (entry.filter_min, entry.filter_max) else { + continue; + }; + + let postcode = data.postcode(row); + let Some(row_data) = travel_data + .get(filter_index) + .and_then(|travel| travel.get(postcode)) + else { + path.push(missing_filter_exclusion( + format!("tt_{}_{}", entry.mode, entry.slug), + "travel", + )); + continue; + }; + + let minutes = if entry.use_best { + row_data.best_minutes.unwrap_or(row_data.minutes) + } else { + row_data.minutes + } as f32; + let Some((direction, rel_diff)) = relative_difference(minutes, min, max) else { + continue; + }; + + path.push(FilterExclusion { + name: format!("tt_{}_{}", entry.mode, entry.slug), + kind: "travel".to_string(), + direction, + value: Some(minutes), + min: Some(min), + max: Some(max), + category: None, + relative_difference: rel_diff, + rejected_count: 0, + }); + } + + if path.is_empty() { + continue; + } + + for exclusion in &path { + *rejection_counts + .entry(filter_exclusion_key(exclusion)) + .or_default() += 1; + } + + let path_score = path + .iter() + .map(|exclusion| exclusion.relative_difference) + .sum::(); + let current_score = best_path + .as_ref() + .map(|current| { + current + .iter() + .map(|exclusion| exclusion.relative_difference) + .sum::() + }) + .unwrap_or(f32::INFINITY); + + let replace = path_score < current_score + || (path_score == current_score + && best_path + .as_ref() + .map_or(true, |current| path.len() < current.len())); + if replace { + best_path = Some(path); + } + } + + let Some(mut exclusions) = best_path else { + return Vec::new(); + }; + + for exclusion in &mut exclusions { + exclusion.rejected_count = rejection_counts + .get(&filter_exclusion_key(exclusion)) + .copied() + .unwrap_or(0); + } + + exclusions.sort_by(|a, b| { + a.relative_difference + .partial_cmp(&b.relative_difference) + .unwrap_or(std::cmp::Ordering::Equal) + .then_with(|| b.rejected_count.cmp(&a.rejected_count)) + .then_with(|| a.name.cmp(&b.name)) + }); + exclusions.truncate(MAX_FILTER_EXCLUSIONS); + exclusions +} + pub async fn get_hexagon_stats( State(shared): State>, Extension(user): Extension, @@ -124,7 +443,7 @@ pub async fn get_hexagon_stats( let filters_str = params.filters; let has_poi_filters = !parsed_poi_filters.is_empty(); - let (fields_specified, field_set) = parse_field_set(params.fields.as_deref()); + let (fields_specified, field_set) = parse_area_stats_field_set(params.fields.as_deref()); let travel_entries = parse_optional_travel(params.travel.as_deref()) .map_err(|err| (StatusCode::BAD_REQUEST, err).into_response())?; @@ -151,38 +470,55 @@ pub async fn get_hexagon_stats( let (min_lat, min_lon, max_lat, max_lon) = h3_cell_bounds(cell, 0.001); let mut h3_cache: FxHashMap = FxHashMap::default(); + let mut area_rows: Vec = Vec::new(); let mut matching_rows: Vec = Vec::new(); state .grid .for_each_in_bounds(min_lat, min_lon, max_lat, max_lon, |row_idx| { let row = row_idx as usize; if cell_for_row_cached(row, precomputed, h3_res, need_parent, &mut h3_cache) - == cell_u64 - && row_passes_filters( - row, - &parsed_filters, - &parsed_enum_filters, - feature_data, - num_features, - ) - && (!has_poi_filters - || row_passes_poi_filters( - row, - &parsed_poi_filters, - &state.data.poi_metrics, - )) + != cell_u64 { - if has_travel { - let postcode = state.data.postcode(row); - if !row_passes_travel_filters(postcode, &travel_entries, &travel_data) { - return; - } + return; + } + + area_rows.push(row); + if row_passes_filters( + row, + &parsed_filters, + &parsed_enum_filters, + feature_data, + num_features, + ) && (!has_poi_filters + || row_passes_poi_filters(row, &parsed_poi_filters, &state.data.poi_metrics)) + { + if has_travel + && !row_passes_travel_filters( + state.data.postcode(row), + &travel_entries, + &travel_data, + ) + { + return; } matching_rows.push(row); } }); let total_count = matching_rows.len(); + let filter_exclusions = if total_count == 0 { + top_filter_exclusions( + &area_rows, + &parsed_filters, + &parsed_enum_filters, + &parsed_poi_filters, + &travel_entries, + &travel_data, + &state.data, + ) + } else { + Vec::new() + }; // Pick central_postcode: prefer the postcode with the shortest travel time // for the requested journey destination (so it has journey data). Fall back @@ -277,6 +613,7 @@ pub async fn get_hexagon_stats( enum_features: enum_features_out, price_history, central_postcode, + filter_exclusions, }) }) .await @@ -285,3 +622,30 @@ pub async fn get_hexagon_stats( Ok(Json(response)) } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn default_area_stat_fields_skip_amenities() { + let (fields_specified, field_set) = parse_area_stats_field_set(None); + + assert!(fields_specified); + assert!(field_set.contains("Property type")); + assert!(!field_set.contains("Noise (dB)")); + assert!(!field_set.contains("Max available download speed (Mbps)")); + assert!(!field_set.contains("Distance to nearest amenity (Cafe) (km)")); + } + + #[test] + fn explicit_area_stat_fields_are_respected() { + let (fields_specified, field_set) = + parse_area_stats_field_set(Some("Noise (dB);;Property type")); + + assert!(fields_specified); + assert!(field_set.contains("Noise (dB)")); + assert!(field_set.contains("Property type")); + assert_eq!(field_set.len(), 2); + } +} diff --git a/server-rs/src/routes/shorten.rs b/server-rs/src/routes/shorten.rs index 6380154..53611db 100644 --- a/server-rs/src/routes/shorten.rs +++ b/server-rs/src/routes/shorten.rs @@ -1,7 +1,7 @@ use std::sync::Arc; use axum::extract::{Path, State}; -use axum::http::{header, StatusCode}; +use axum::http::{header, HeaderMap, StatusCode}; use axum::response::{Html, IntoResponse, Response}; use axum::Extension; use axum::Json; @@ -11,7 +11,8 @@ use tracing::warn; use url::form_urlencoded; use crate::auth::OptionalUser; -use crate::licensing::{is_valid_share_bounds, share_bounds_from_params, ShareBounds}; +use crate::language::{language_from_accept_language, query_string_with_language}; +use crate::licensing::{is_valid_share_bounds, share_params_and_bounds_from_params, ShareBounds}; use crate::pocketbase::get_superuser_token; use crate::state::SharedState; @@ -136,6 +137,7 @@ fn is_allowed_param_key(key: &str) -> bool { | "tab" | "pc" | "tt" + | "lang" | "share" ) } @@ -180,15 +182,42 @@ fn record_share_bounds(item: &serde_json::Value) -> Option { } fn dashboard_redirect_url(params: &str, code: &str, include_share: bool) -> String { - match (params.is_empty(), include_share) { - (true, false) => "/dashboard".to_string(), - (true, true) => format!("/dashboard?share={code}"), - (false, false) => format!("/dashboard?{params}"), - (false, true) => format!("/dashboard?{params}&share={code}"), + let params = match include_share { + true => params_with_share(params, code), + false => params.to_string(), + }; + + if params.is_empty() { + "/dashboard".to_string() + } else { + format!("/dashboard?{params}") } } -fn og_image_url(public_url: &str, params: &str) -> String { +fn params_with_share(params: &str, code: &str) -> String { + let mut out = form_urlencoded::Serializer::new(String::new()); + for (key, value) in form_urlencoded::parse(params.as_bytes()) { + if key == "share" { + continue; + } + out.append_pair(&key, &value); + } + out.append_pair("share", code); + out.finish() +} + +fn og_image_url( + public_url: &str, + params: &str, + language: &str, + share_code: Option<&str>, +) -> String { + let params = query_string_with_language(params, language); + let params = match share_code { + Some(code) => params_with_share(¶ms, code), + None => params, + }; + if params.is_empty() { format!("{}/api/screenshot?og=1", public_url.trim_end_matches('/')) } else { @@ -208,7 +237,7 @@ pub async fn post_shorten( let pb_url = state.pocketbase_url.trim_end_matches('/'); let can_create_share_grant = user_can_create_share_grant(&user); - let params = match sanitized_query_params(&req.params, !can_create_share_grant) { + let mut params = match sanitized_query_params(&req.params, !can_create_share_grant) { Ok(params) => params, Err(reason) => { warn!("Rejected short URL params: {reason}"); @@ -226,7 +255,10 @@ pub async fn post_shorten( let code = generate_code(); let share_bounds = if can_create_share_grant { - share_bounds_from_params(¶ms) + share_params_and_bounds_from_params(¶ms).map(|(share_params, share_bounds)| { + params = share_params; + share_bounds + }) } else { None }; @@ -275,6 +307,7 @@ pub async fn post_shorten( pub async fn get_share_links( State(shared): State>, Extension(user): Extension, + headers: HeaderMap, ) -> Response { let state = shared.load_state(); let user = match user.0 { @@ -328,6 +361,11 @@ pub async fn get_share_links( }; let public_url = state.public_url.trim_end_matches('/'); + let language = language_from_accept_language( + headers + .get(header::ACCEPT_LANGUAGE) + .and_then(|value| value.to_str().ok()), + ); let links: Vec = body["items"] .as_array() .map(|arr| { @@ -335,10 +373,17 @@ pub async fn get_share_links( .map(|item| { let code = item["code"].as_str().unwrap_or("").to_string(); let params = item["params"].as_str().unwrap_or("").to_string(); + let has_share_grant = record_share_bounds(item).is_some(); + let og_image_url = og_image_url( + public_url, + ¶ms, + language, + has_share_grant.then_some(code.as_str()), + ); ShareLinkListItem { url: format!("{public_url}/s/{code}"), code, - og_image_url: og_image_url(public_url, ¶ms), + og_image_url, params, click_count: json_number_as_u64(&item["click_count"]), created: item["created"].as_str().unwrap_or("").to_string(), @@ -354,8 +399,14 @@ pub async fn get_share_links( pub async fn get_short_url( State(shared): State>, Path(code): Path, + headers: HeaderMap, ) -> Response { let state = shared.load_state(); + let language = language_from_accept_language( + headers + .get(header::ACCEPT_LANGUAGE) + .and_then(|value| value.to_str().ok()), + ); if code.is_empty() || code.len() > 20 || !code.bytes().all(|b| b.is_ascii_alphanumeric()) { return StatusCode::BAD_REQUEST.into_response(); @@ -428,9 +479,14 @@ pub async fn get_short_url( Err(err) => warn!("PocketBase click count update failed: {err}"), } } - let redirect_url = - dashboard_redirect_url(¶ms, &code, record_share_bounds(item).is_some()); - let og_image_url = og_image_url(&state.public_url, ¶ms); + let has_share_grant = record_share_bounds(item).is_some(); + let redirect_url = dashboard_redirect_url(¶ms, &code, has_share_grant); + let og_image_url = og_image_url( + &state.public_url, + ¶ms, + language, + has_share_grant.then_some(code.as_str()), + ); let og_url = format!("{}/s/{code}", state.public_url.trim_end_matches('/')); let og_title = "Perfect Postcode | Every neighbourhood in England"; let og_description = "Explore property prices, energy ratings, crime stats, school ratings, and more across England on one interactive map."; @@ -454,6 +510,7 @@ pub async fn get_short_url( + {og_title} "# @@ -517,4 +574,34 @@ mod tests { fn escapes_html_attributes() { assert_eq!(escape_attr(r#""'><&"#), ""'><&"); } + + #[test] + fn og_image_url_includes_language_and_share_grant() { + assert_eq!( + og_image_url( + "http://localhost:3001/", + "lat=51.5&lon=-0.1&zoom=12", + "de", + Some("abc123") + ), + "http://localhost:3001/api/screenshot?og=1&lat=51.5&lon=-0.1&zoom=12&lang=de&share=abc123" + ); + } + + #[test] + fn share_grant_replaces_existing_share_param() { + assert_eq!( + dashboard_redirect_url("lat=51.5&share=oldcode&zoom=12", "newcode", true), + "/dashboard?lat=51.5&zoom=12&share=newcode" + ); + assert_eq!( + og_image_url( + "https://perfect-postcodes.co.uk", + "lat=51.5&share=oldcode&zoom=12", + "en", + Some("newcode") + ), + "https://perfect-postcodes.co.uk/api/screenshot?og=1&lat=51.5&zoom=12&lang=en&share=newcode" + ); + } } diff --git a/server-rs/src/routes/stripe_webhook.rs b/server-rs/src/routes/stripe_webhook.rs index 209ce73..f0dfd2a 100644 --- a/server-rs/src/routes/stripe_webhook.rs +++ b/server-rs/src/routes/stripe_webhook.rs @@ -9,10 +9,11 @@ use sha2::Sha256; use tracing::{info, warn}; use crate::checkout_sessions::{ - complete_verified_checkout, reverse_license_for_payment_intent, verify_checkout_completion, - CheckoutCompletion, + complete_verified_checkout, reinstate_license_for_payment_intent, + reverse_license_for_payment_intent, verify_checkout_completion, CheckoutCompletion, + PaymentReinstatementOutcome, PaymentReversalOutcome, }; -use crate::state::SharedState; +use crate::state::{AppState, SharedState}; type HmacSha256 = Hmac; @@ -73,12 +74,17 @@ fn verify_signature(payload: &[u8], sig_header: &str, secret: &str) -> bool { matched } -fn payment_intent_id_from_object(object: &serde_json::Value) -> Option<&str> { - object["payment_intent"] +fn stripe_id_from_value(value: &serde_json::Value) -> Option<&str> { + value .as_str() + .or_else(|| value["id"].as_str()) .filter(|id| is_safe_stripe_id(id)) } +fn payment_intent_id_from_object(object: &serde_json::Value) -> Option<&str> { + stripe_id_from_value(&object["payment_intent"]) +} + fn is_safe_stripe_id(id: &str) -> bool { !id.is_empty() && id.len() <= 128 @@ -87,21 +93,169 @@ fn is_safe_stripe_id(id: &str) -> bool { .all(|b| b.is_ascii_alphanumeric() || b == b'_' || b == b'-') } -fn reversal_event_is_actionable(event_type: &str, object: &serde_json::Value) -> bool { +fn integer_field(value: &serde_json::Value, field: &str) -> Option { + value[field].as_u64().or_else(|| { + value[field] + .as_f64() + .filter(|n| n.is_finite() && *n >= 0.0 && n.fract() == 0.0) + .map(|n| n as u64) + }) +} + +fn reversal_refund_amount_pence( + event_type: &str, + object: &serde_json::Value, +) -> Option> { match event_type { "charge.refunded" => { - object["refunded"].as_bool().unwrap_or(false) - || object["amount_refunded"].as_u64().unwrap_or(0) > 0 + let amount_refunded = integer_field(object, "amount_refunded").unwrap_or(0); + let amount = integer_field(object, "amount").unwrap_or(0); + if object["refunded"].as_bool().unwrap_or(false) + || (amount > 0 && amount_refunded >= amount) + { + Some(Some(amount_refunded)) + } else { + None + } } "charge.refund.updated" | "refund.created" | "refund.updated" => { - matches!(object["status"].as_str(), Some("succeeded")) + if matches!(object["status"].as_str(), Some("succeeded")) { + integer_field(object, "amount").map(Some) + } else { + None + } } - "charge.dispute.created" | "charge.dispute.funds_withdrawn" => true, - "charge.dispute.closed" => matches!(object["status"].as_str(), Some("lost")), + "charge.dispute.created" | "charge.dispute.funds_withdrawn" => Some(None), + "charge.dispute.closed" => { + if matches!(object["status"].as_str(), Some("lost")) { + Some(None) + } else { + None + } + } + _ => None, + } +} + +fn reinstatement_event_is_actionable(event_type: &str, object: &serde_json::Value) -> bool { + match event_type { + "charge.dispute.funds_reinstated" => true, + "charge.dispute.closed" => matches!(object["status"].as_str(), Some("won")), _ => false, } } +async fn process_stripe_reversal_event( + state: &AppState, + payment_intent_id: &str, + event_type: &str, + refunded_amount_pence: Option, +) -> Result<(), StatusCode> { + match reverse_license_for_payment_intent( + state, + payment_intent_id, + event_type, + refunded_amount_pence, + ) + .await + { + Ok(PaymentReversalOutcome::Applied { user_id }) => { + info!( + user_id, + payment_intent_id, event_type, "Processed Stripe payment reversal event" + ); + } + Ok(PaymentReversalOutcome::AlreadyHandled { user_id }) => { + info!( + user_id, + payment_intent_id, event_type, "Stripe payment reversal was already handled" + ); + } + Ok(PaymentReversalOutcome::IgnoredPartialRefund { + user_id, + refunded_amount_pence, + paid_amount_pence, + }) => { + info!( + user_id, + payment_intent_id, + refunded_amount_pence, + paid_amount_pence, + "Ignoring partial Stripe refund" + ); + } + Ok(PaymentReversalOutcome::NotReversible { user_id, status }) => { + warn!( + user_id, + payment_intent_id, + status, + event_type, + "Stripe reversal event matched a non-reversible checkout" + ); + } + Ok(PaymentReversalOutcome::NoMatchingCheckout) => { + warn!( + payment_intent_id, + event_type, "Stripe reversal event had no matching checkout reservation" + ); + } + Err(err) => { + warn!( + payment_intent_id, + event_type, "Failed to process Stripe payment reversal event: {err:?}" + ); + return Err(StatusCode::INTERNAL_SERVER_ERROR); + } + } + + Ok(()) +} + +async fn process_stripe_reinstatement_event( + state: &AppState, + payment_intent_id: &str, + event_type: &str, +) -> Result<(), StatusCode> { + match reinstate_license_for_payment_intent(state, payment_intent_id, event_type).await { + Ok(PaymentReinstatementOutcome::Applied { user_id }) => { + info!( + user_id, + payment_intent_id, event_type, "Processed Stripe payment reinstatement event" + ); + } + Ok(PaymentReinstatementOutcome::AlreadyHandled { user_id }) => { + info!( + user_id, + payment_intent_id, event_type, "Stripe payment reinstatement was already handled" + ); + } + Ok(PaymentReinstatementOutcome::Ignored { user_id, reason }) => { + info!( + user_id, + payment_intent_id, + event_type, + reason, + "Ignoring Stripe payment reinstatement event" + ); + } + Ok(PaymentReinstatementOutcome::NoMatchingCheckout) => { + warn!( + payment_intent_id, + event_type, "Stripe reinstatement event had no matching checkout reservation" + ); + } + Err(err) => { + warn!( + payment_intent_id, + event_type, "Failed to process Stripe payment reinstatement event: {err:?}" + ); + return Err(StatusCode::INTERNAL_SERVER_ERROR); + } + } + + Ok(()) +} + /// Handle Stripe webhook events. /// On `checkout.session.completed`, updates the user's subscription to "licensed". pub async fn post_stripe_webhook( @@ -159,7 +313,15 @@ pub async fn post_stripe_webhook( "User subscription updated to licensed via verified Stripe checkout" ); } - Ok(CheckoutCompletion::AlreadyHandled) => { + Ok(CheckoutCompletion::AlreadyHandled(checkout)) => { + if let Err(err) = complete_verified_checkout(&state, &checkout).await { + warn!( + user_id = %checkout.user_id, + reservation_id = %checkout.reservation_id, + "Failed to finish idempotent Stripe checkout side effects: {err:?}" + ); + return StatusCode::INTERNAL_SERVER_ERROR.into_response(); + } info!("Stripe checkout session was already handled"); } Ok(CheckoutCompletion::Rejected(reason)) => { @@ -179,44 +341,173 @@ pub async fn post_stripe_webhook( | "charge.dispute.created" | "charge.dispute.closed" | "charge.dispute.funds_withdrawn" + | "charge.dispute.funds_reinstated" ) { let object = &event["data"]["object"]; let Some(payment_intent_id) = payment_intent_id_from_object(object) else { warn!( event_id, - event_type, "Stripe reversal event missing payment intent id" + event_type, "Stripe payment adjustment event missing payment intent id" ); return StatusCode::OK.into_response(); }; - if !reversal_event_is_actionable(event_type, object) { - info!( - payment_intent_id, - event_type, "Ignoring non-final Stripe reversal event" - ); + + if reinstatement_event_is_actionable(event_type, object) { + if let Err(status) = + process_stripe_reinstatement_event(&state, payment_intent_id, event_type).await + { + return status.into_response(); + } return StatusCode::OK.into_response(); } - match reverse_license_for_payment_intent(&state, payment_intent_id, event_type).await { - Ok(Some(user_id)) => { - info!( - user_id, - payment_intent_id, event_type, "Processed Stripe payment reversal event" - ); - } - Ok(None) => { - warn!( - payment_intent_id, - event_type, "Stripe reversal event had no matching checkout reservation" - ); - } - Err(err) => { - warn!( - payment_intent_id, - event_type, "Failed to process Stripe payment reversal event: {err:?}" - ); - return StatusCode::INTERNAL_SERVER_ERROR.into_response(); + + if let Some(refunded_amount_pence) = reversal_refund_amount_pence(event_type, object) { + if let Err(status) = process_stripe_reversal_event( + &state, + payment_intent_id, + event_type, + refunded_amount_pence, + ) + .await + { + return status.into_response(); } + return StatusCode::OK.into_response(); } + + info!( + payment_intent_id, + event_type, "Ignoring non-final Stripe payment adjustment event" + ); } StatusCode::OK.into_response() } + +#[cfg(test)] +mod tests { + use super::*; + + fn signed_header(payload: &[u8], secret: &str, timestamp: i64) -> String { + let signed_payload = format!("{timestamp}.{}", String::from_utf8_lossy(payload)); + let mut mac = HmacSha256::new_from_slice(secret.as_bytes()).unwrap(); + mac.update(signed_payload.as_bytes()); + let signature = hex::encode(mac.finalize().into_bytes()); + format!("t={timestamp},v1={signature}") + } + + #[test] + fn verify_signature_accepts_valid_header() { + let payload = br#"{"id":"evt_123","type":"checkout.session.completed"}"#; + let secret = "whsec_test_secret"; + let now = std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .unwrap() + .as_secs() as i64; + let header = signed_header(payload, secret, now); + + assert!(verify_signature(payload, &header, secret)); + } + + #[test] + fn verify_signature_rejects_tampered_payload() { + let payload = br#"{"id":"evt_123","type":"checkout.session.completed"}"#; + let secret = "whsec_test_secret"; + let now = std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .unwrap() + .as_secs() as i64; + let header = signed_header(payload, secret, now); + + assert!(!verify_signature( + br#"{"id":"evt_123","type":"invoice.paid"}"#, + &header, + secret + )); + } + + #[test] + fn verify_signature_rejects_stale_header() { + let payload = br#"{"id":"evt_123","type":"checkout.session.completed"}"#; + let secret = "whsec_test_secret"; + let header = signed_header(payload, secret, 1); + + assert!(!verify_signature(payload, &header, secret)); + } + + #[test] + fn reversal_refund_amount_only_accepts_full_charge_refunds() { + let partial = serde_json::json!({ + "amount": 1000, + "amount_refunded": 500, + "refunded": false, + }); + let full = serde_json::json!({ + "amount": 1000, + "amount_refunded": 1000, + "refunded": true, + }); + + assert_eq!( + reversal_refund_amount_pence("charge.refunded", &partial), + None + ); + assert_eq!( + reversal_refund_amount_pence("charge.refunded", &full), + Some(Some(1000)) + ); + } + + #[test] + fn reversal_refund_amount_requires_succeeded_refund() { + let pending = serde_json::json!({ + "amount": 1000, + "status": "pending", + }); + let succeeded = serde_json::json!({ + "amount": 1000, + "status": "succeeded", + }); + + assert_eq!( + reversal_refund_amount_pence("refund.created", &pending), + None + ); + assert_eq!( + reversal_refund_amount_pence("refund.created", &succeeded), + Some(Some(1000)) + ); + } + + #[test] + fn payment_intent_id_accepts_expanded_objects() { + let object = serde_json::json!({ + "payment_intent": { "id": "pi_123" } + }); + + assert_eq!(payment_intent_id_from_object(&object), Some("pi_123")); + } + + #[test] + fn dispute_closed_routes_lost_and_won_differently() { + let lost = serde_json::json!({ "status": "lost" }); + let won = serde_json::json!({ "status": "won" }); + + assert_eq!( + reversal_refund_amount_pence("charge.dispute.closed", &lost), + Some(None) + ); + assert!(!reinstatement_event_is_actionable( + "charge.dispute.closed", + &lost + )); + assert_eq!( + reversal_refund_amount_pence("charge.dispute.closed", &won), + None + ); + assert!(reinstatement_event_is_actionable( + "charge.dispute.closed", + &won + )); + } +}