k1nq/TREK
1
0
Fork 0
mirror of https://github.com/mauriceboe/TREK.git synced 2026-10-18 21:16:47 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(maps): sag bei jedem Vorschlag, welcher Index geantwortet hat

Browse Source 
Seit die Tipp-Suche beide Schichten fragt, ist ihre Liste gemischt — die
Antwort nennt aber weiterhin EINE Quelle fuer die ganze Liste, und die heisst
'trek-places'. Der Client beschriftet damit jede Zeile, auch die aus
OpenStreetMap. Wer tippt, liest also acht Mal "TREK" und schliesst daraus, die
OSM-Schicht antworte gar nicht. Sie antwortet, sie steht nur unter falschem
Namen da. Auf der Suchstrecke daneben stimmt es laengst: dort traegt jeder Ort
sein eigenes `source`, und genau deshalb liest sich diese Liste richtig.

Also traegt es jetzt auch die Zeile. Das Feld ist optional, weil Google und der
Nominatim-Rueckfall je aus einer Quelle antworten; eine Zeile ohne eigene
Angabe faellt weiter auf den Namen der Liste zurueck.

Dazu die Koordinaten, die der Vorschlag ohnehin schon kennt. Bisher wurden sie
weggeworfen, und wenn der Details-Aufruf nicht antwortete — Kill-Switch, ein
ueberlasteter Overpass-Spiegel, irgendein Fehler oben — baute der Client eine
Textsuche aus Name und zweiter Zeile. Bei einer OSM-Zeile ist die zweite Zeile
aber keine Adresse, sondern der ortsuebliche Name: gesucht wurde dann nach
"Tokio Hauptbahnhof, 東京駅丸の内駅舎", und dessen erstes Ergebnis wurde
kommentarlos als der Ort genommen, den der Nutzer angeklickt hatte. Jetzt
nimmt der Rueckfall den Punkt, den die Zeile mitbringt.

Und das Abzeichen gibt es endlich auch mobil. Die Marke und ihre Tabelle
liegen dafuer in client/src/utils/placeSource.ts, weil beide Oberflaechen
dieselben Listen zeigen und ein Zeichen, das nur auf einer davon steht,
schlimmer ist als keines.

Was hier NICHT drin ist, obwohl es dazugehoert: dass der richtige Bahnhof in
der Liste steht. Das liegt am Dienst, nicht an TREK, und ist dort gemessen und
behoben — Rangfolge nach Art des Ortes plus die Verkehrsknoten, die der
OSM-Schicht ganz fehlten.
This commit is contained in:
Maurice
2026-09-09 17:24:05 +02:00
committed by Maurice
parent 2f71549413
commit 7ba7338f5f
7 changed files with 223 additions and 39 deletions
Download Patch File Download Diff File Expand all files Collapse all files
client/src/components/Planner/PlaceFormModal.test.tsx
+62
View File
@@ -299,6 +299,43 @@ describe('PlaceFormModal', () => {
delete window.__addToast;
});
it('FE-PLANNER-PLACEFORM-021e: an OpenStreetMap suggestion keeps its own coordinates instead of searching for its label', async () => {
// The layer's second line is the name written on the building, not an
// address. Joining the two and searching for it asks a question nobody
// typed, and whatever came back first was taken as the place the user had
// already picked. The row carries coordinates, so the fallback uses those.
const user = userEvent.setup();
let searched = 0;
server.use(
http.post('/api/maps/autocomplete', () =>
HttpResponse.json({
suggestions: [{
placeId: 'node:9712313',
mainText: 'Tokio Hauptbahnhof',
secondaryText: '東京駅丸の内駅舎',
source: 'openstreetmap',
lat: 35.6811816,
lng: 139.76598265,
}],
source: 'trek-places',
}),
),
http.get('/api/maps/details/:placeId', () => HttpResponse.json({ place: null, disabled: true })),
http.post('/api/maps/search', () => {
searched += 1;
return HttpResponse.json({ places: [{ name: 'Etwas ganz anderes', lat: '1', lng: '1' }], source: 'trek-places' });
}),
);
render(<PlaceFormModal {...defaultProps} />);
await user.type(screen.getByPlaceholderText('Search places...'), 'Tokio');
await user.click(await screen.findByText('Tokio Hauptbahnhof'));
expect(await screen.findByDisplayValue('35.6811816')).toBeInTheDocument();
expect(screen.getByDisplayValue('139.76598265')).toBeInTheDocument();
expect(searched).toBe(0);
});
it('FE-PLANNER-PLACEFORM-021c: suggestion click falls back when details is disabled (place: null)', async () => {
const user = userEvent.setup();
server.use(
@@ -375,6 +412,31 @@ describe('PlaceFormModal', () => {
expect(await screen.findByText('TREK')).toBeInTheDocument();
});
it('FE-PLANNER-PLACEFORM-022d: the typed-ahead list marks the OpenStreetMap rows as such', async () => {
// The keystroke path asks both indexes at once, so the name the response
// carries for the whole list ('trek-places') is true of the call and wrong
// for half the rows. Marking every suggestion TREK is how a list that did
// contain OpenStreetMap places read as if it never had.
const user = userEvent.setup();
server.use(
http.post('/api/maps/autocomplete', () =>
HttpResponse.json({
suggestions: [
{ placeId: 'gers:abc', mainText: 'Tokyo Station Beer Stand', secondaryText: 'Chiyoda', source: 'trek-places' },
{ placeId: 'node:9712313', mainText: 'Tokio Hauptbahnhof', secondaryText: '東京駅', source: 'openstreetmap' },
],
source: 'trek-places',
}),
),
);
render(<PlaceFormModal {...defaultProps} />);
await user.type(screen.getByPlaceholderText('Search places...'), 'Tokyo Station');
expect(await screen.findByText('TREK')).toBeInTheDocument();
expect(await screen.findByText('OpenStreetMap')).toBeInTheDocument();
});
it('FE-PLANNER-PLACEFORM-022c: an interleaved result list marks each row with its own index', async () => {
// The whole point of putting it on the row: with both indexes answering,
// a label above the list could only name one of them. The index marks its
client/src/components/Planner/PlaceFormModal.tsx
+32 -33
View File
@@ -18,6 +18,7 @@ import { useTranslation } from '../../i18n'
import CustomTimePicker from '../shared/CustomTimePicker'
import { DEFAULT_FORM, isGoogleMapsUrl, mergeResult, type PlaceFormData, type ResultField } from './PlaceFormModal.helpers'
import { getApiErrorMessage } from '../../utils/apiError'
import { sourceLabelFor } from '../../utils/placeSource'
import { useLocationBias } from '../../hooks/useLocationBias'
import { BookingCostsSection } from './BookingCostsSection'
import type { BookingExpenseRequest } from './BookingCostsSection.types'
@@ -56,36 +57,19 @@ interface PlaceFormModalProps {
/**
* Which index a search result came from, as a short mark beside it.
* One row of the typed-ahead list, as the server sends it.
*
* A place list can be two indexes interleaved, so the source belongs on the row
* rather than above the list: with the TREK index and OpenStreetMap answering
* together, "one of these came from somewhere" is not an answer anyone can use.
*
* The three names are proper nouns, so they are not translated, and that is also
* why there is no fourth: a source without a name people already know would need
* a string in 23 languages to say less than nothing.
* `source`, `lat` and `lng` are optional because not every index fills them:
* Google answers with neither, and the mark falls back to the name the whole
* list carries.
*/
const SOURCE_LABELS: Record<string, string> = {
'trek-places': 'TREK',
openstreetmap: 'OpenStreetMap',
nominatim: 'OpenStreetMap',
google: 'Google',
}
/**
* The label for one row.
*
* A place carries its own source when the index that produced it says so, which
* is what makes an interleaved list readable. Everything else falls back to what
* answered the call: Google never marks its places, and a merged list marks only
* the index side, so an unmarked row in one is OpenStreetMap by elimination.
*/
function sourceLabelFor(place: unknown, listSource: string): string | null {
const own = (place as { source?: unknown } | null)?.source
if (typeof own === 'string' && own) return SOURCE_LABELS[own] ?? null
if (listSource.includes('openstreetmap')) return SOURCE_LABELS.openstreetmap
return SOURCE_LABELS[listSource] ?? null
type Suggestion = {
placeId: string
mainText: string
secondaryText: string
source?: string
lat?: number
lng?: number
}
/** The mark itself. Quiet on purpose: it answers a question, it does not advertise. */
@@ -161,10 +145,10 @@ function usePlaceFormModal(props: PlaceFormModalProps) {
const [pendingFiles, setPendingFiles] = useState([])
const fileRef = useRef(null)
const searchInputRef = useRef<HTMLInputElement>(null)
const [acSuggestions, setAcSuggestions] = useState<{ placeId: string; mainText: string; secondaryText: string }[]>([])
// Which index answered the last keystroke. One call is served by one source,
// so the whole list carries it; the search below is per place, because that
// list can be two indexes interleaved.
const [acSuggestions, setAcSuggestions] = useState<Suggestion[]>([])
// Which index answered the last keystroke, for the rows that do not say so
// themselves. Google and the OpenStreetMap fallback each answer from one
// place; the index path answers from two at once and marks every row.
const [acSource, setAcSource] = useState<string>('')
const [acHighlight, setAcHighlight] = useState(-1)
const acDebounceRef = useRef<ReturnType<typeof setTimeout> | null>(null)
@@ -420,7 +404,7 @@ function usePlaceFormModal(props: PlaceFormModalProps) {
setMapsSearch('')
}
const handleSelectSuggestion = async (suggestion: { placeId: string; mainText: string; secondaryText: string }) => {
const handleSelectSuggestion = async (suggestion: Suggestion) => {
// Read before the list is cleared: this is the rank the user saw.
const acRank = acSuggestions.findIndex(s => s.placeId === suggestion.placeId)
const acCount = acSuggestions.length
@@ -448,6 +432,21 @@ function usePlaceFormModal(props: PlaceFormModalProps) {
} catch (err) {
console.error('Failed to fetch place details:', err)
}
if (!place && suggestion.source === 'openstreetmap' && suggestion.lat != null && suggestion.lng != null) {
// The layer's rows carry no address; their second line is the name
// written on the building. Searching for "Tokio Hauptbahnhof, 東京駅"
// is not a question anybody asked, and its first answer would be
// whatever the index made of it — a different place, chosen silently.
// The suggestion already knows where it is, so use that.
place = {
name: suggestion.mainText,
address: '',
lat: suggestion.lat,
lng: suggestion.lng,
osm_id: suggestion.placeId,
source: 'openstreetmap',
}
}
if (!place) {
const query = [suggestion.mainText, suggestion.secondaryText].filter(Boolean).join(', ')
const search = await mapsApi.search(query, language, locationBiasPoint)
client/src/mobile/screens/trip/sheets/PlPlaceSearch.tsx
+41 -4
View File
@@ -1,6 +1,7 @@
import { useCallback, useEffect, useRef, useState } from 'react'
import { Loader2, Search } from 'lucide-react'
import { mapsApi } from '../../../../api/client'
import { sourceLabelFor } from '../../../../utils/placeSource'
import { recordPlacePick } from '../../../../api/placeShadow'
import { PlacesSession } from '../../../../utils/placesSession'
import { isGoogleMapsUrl } from '../../../../components/Planner/PlaceFormModal.helpers'
@@ -26,10 +27,24 @@ interface Suggestion {
placeId: string
mainText: string
secondaryText: string
/** Which of the two indexes this row came from, when the list is both. */
source?: string
lat?: number
lng?: number
}
type MapsPlace = Record<string, unknown>
/** The same mark the desktop form shows, in the sheet's own tokens. */
function SourceMark({ label }: { label: string | null }) {
if (!label) return null
return (
<span className="shrink-0 rounded-md border border-[color:var(--m-rowbr)] px-1.5 py-0.5 font-geist text-[0.5625rem] font-medium text-m-muted">
{label}
</span>
)
}
interface PlPlaceSearchProps {
planner: TripPlanner
/** Search bias derived from the trip's existing places (trip centre). */
@@ -76,6 +91,8 @@ export default function PlPlaceSearch({ planner, locationBias, onPick, onResolvi
// has to read the values that belonged to that list. Mirrors PlaceFormModal.
const searchMetaRef = useRef<{ query: string; source: string } | null>(null)
const acMetaRef = useRef<{ query: string; source: string } | null>(null)
// The name the whole list carries, for rows that do not name their own index.
const [acSource, setAcSource] = useState('')
const setResolving = useCallback(
(v: boolean) => {
@@ -93,6 +110,7 @@ export default function PlPlaceSearch({ planner, locationBias, onPick, onResolvi
try {
const result = await mapsApi.autocomplete(input, language, locationBias, controller.signal, placesSessionRef.current.current())
acMetaRef.current = { query: input, source: result.source || 'unknown' }
setAcSource(result.source || '')
setSuggestions(result.suggestions || [])
} catch (err: unknown) {
// Superseded request — axios rejects an aborted call with CanceledError.
@@ -211,6 +229,20 @@ export default function PlPlaceSearch({ planner, locationBias, onPick, onResolvi
} catch {
// fall through to text search
}
if (!place && suggestion.source === 'openstreetmap' && suggestion.lat != null && suggestion.lng != null) {
// The layer's second line is the local name, not an address, so joining
// the two makes a query nobody typed — and its first answer would be
// silently taken as the place the user picked. The row already knows
// where it is.
place = {
name: suggestion.mainText,
address: '',
lat: suggestion.lat,
lng: suggestion.lng,
osm_id: suggestion.placeId,
source: 'openstreetmap',
}
}
if (!place) {
const fullQuery = [suggestion.mainText, suggestion.secondaryText].filter(Boolean).join(', ')
const search = await mapsApi.search(fullQuery, language, pointFromBox(locationBias))
@@ -269,10 +301,15 @@ export default function PlPlaceSearch({ planner, locationBias, onPick, onResolvi
onClick={() => handleSelectSuggestion(s)}
className="block w-full border-t border-[color:var(--m-rowbr)] px-[13px] py-[10px] text-left first:border-t-0"
>
<div className="truncate text-[0.8125rem] font-semibold text-m-ink">{s.mainText}</div>
{s.secondaryText && (
<div className="truncate font-geist text-[0.65625rem] text-m-muted">{s.secondaryText}</div>
)}
<div className="flex items-center gap-2">
<div className="min-w-0 flex-1">
<div className="truncate text-[0.8125rem] font-semibold text-m-ink">{s.mainText}</div>
{s.secondaryText && (
<div className="truncate font-geist text-[0.65625rem] text-m-muted">{s.secondaryText}</div>
)}
</div>
<SourceMark label={sourceLabelFor(s, acSource)} />
</div>
</button>
))}
</div>
client/src/utils/placeSource.ts
+39
View File
@@ -0,0 +1,39 @@
/**
* Which index a place came from, as a short mark beside it.
*
* A result list can be two indexes interleaved, so the source belongs on the
* row rather than above the list: with the TREK index and OpenStreetMap
* answering together, "one of these came from somewhere" is not an answer
* anyone can use.
*
* Here rather than beside one screen because both the desktop form and the
* mobile sheet show the same lists, and a mark that says TREK on one and
* nothing on the other is worse than either alone.
*/
/**
* The three names are proper nouns, so they are not translated, and that is
* also why there is no fourth: a source without a name people already know
* would need a string in 23 languages to say less than nothing.
*/
export const SOURCE_LABELS: Record<string, string> = {
'trek-places': 'TREK',
openstreetmap: 'OpenStreetMap',
nominatim: 'OpenStreetMap',
google: 'Google',
}
/**
* The label for one row.
*
* A place carries its own source when the index that produced it says so, which
* is what makes an interleaved list readable. Everything else falls back to what
* answered the call: Google never marks its places, and a merged list marks only
* the index side, so an unmarked row in one is OpenStreetMap by elimination.
*/
export function sourceLabelFor(place: unknown, listSource: string): string | null {
const own = (place as { source?: unknown } | null)?.source
if (typeof own === 'string' && own) return SOURCE_LABELS[own] ?? null
if (listSource.includes('openstreetmap')) return SOURCE_LABELS.openstreetmap
return SOURCE_LABELS[listSource] ?? null
}
server/src/nest/maps/maps.service.ts
+16 -2
View File
@@ -2021,7 +2021,7 @@ export class MapsService {
lang?: string,
locationBias?: { low: { lat: number; lng: number }; high: { lat: number; lng: number } },
sessionToken?: string,
): Promise<{ suggestions: { placeId: string; mainText: string; secondaryText: string }[]; source: string }> {
): Promise<MapsAutocompleteResult> {
const { key: apiKey, source: keySource } = this.resolveMapsKey(userId);
// This is the path that mattered most. Nominatim's usage policy names
@@ -2071,11 +2071,25 @@ export class MapsService {
// place is called on the spot, which is more use under a
// translated label than an empty line.
secondaryText: p.local_name && p.local_name !== p.name ? p.local_name : '',
// Per row, because this list is two indexes interleaved.
// The name above the list says `trek-places`, which is true
// of the call and false of half the rows in it — the layer
// is OpenStreetMap, and a reader deciding whether to trust
// a suggestion is asking exactly that.
source: 'openstreetmap',
// Both indexes hand these over with the row. Carried rather
// than dropped so picking a suggestion has something to fall
// back on when the details lookup cannot answer.
lat: p.lat,
lng: p.lng,
}
: {
placeId: `gers:${p.gers}`,
mainText: p.name,
secondaryText: [p.address?.locality, p.address?.country].filter(Boolean).join(', '),
source: 'trek-places',
lat: p.lat,
lng: p.lng,
},
),
source: 'trek-places',
@@ -2143,7 +2157,7 @@ export class MapsService {
private async autocompleteNominatim(
input: string,
lang?: string,
): Promise<{ suggestions: { placeId: string; mainText: string; secondaryText: string }[]; source: string }> {
): Promise<MapsAutocompleteResult> {
try {
const places = await this.searchNominatim(input, lang);
const suggestions = places
server/tests/unit/nest/maps.autocomplete.test.ts
+13
View File
@@ -196,6 +196,19 @@ describe('MapsService.autocompletePlaces', () => {
expect(second.suggestions[0].secondaryText).toBe('');
});
it('MAPS-AUTO-010: each row says which index it came from, because the list is two', async () => {
mockSearch.mockResolvedValue([hit(), osmHit()]);
const { suggestions, source } = await make().autocompletePlaces(1, INPUT);
// The name above the list describes the call, and the call asked both. Only
// the row can say which of the two answered it — without that the reader is
// told a place from OpenStreetMap came out of the TREK index, which is the
// one thing the mark beside a suggestion exists to answer.
expect(source).toBe('trek-places');
expect(suggestions.map(s => s.source)).toEqual(['trek-places', 'openstreetmap']);
});
it('MAPS-AUTO-007: the keystroke never leaves for the index while the admin has it off', async () => {
mockSearch.mockResolvedValue([hit()]);
shared/src/maps/maps.schema.ts
+20
View File
@@ -68,6 +68,26 @@ export const mapsAutocompleteSuggestionSchema = z.object({
placeId: z.string(),
mainText: z.string(),
secondaryText: z.string(),
/**
* Which index this one row came from, when the answer is more than one index
* interleaved. The list-level `source` cannot say it: the keystroke path asks
* the TREK index and the OpenStreetMap layer together, so a single name above
* the list marks half the rows wrong.
*
* Optional because Google and the Nominatim fallback each answer from one
* place, and a row that names no source falls back to the list's.
*/
source: z.string().optional(),
/**
* Where the place is, when the index that answered already said so.
*
* The OpenStreetMap layer returns coordinates with every row, and throwing
* them away here cost the client a second round trip on every pick — and,
* when that round trip failed, a text search built out of a name and its
* local spelling, which is not a query anybody would type.
*/
lat: z.number().optional(),
lng: z.number().optional(),
});
export const mapsAutocompleteResultSchema = z.object({
suggestions: z.array(mapsAutocompleteSuggestionSchema),