mirror of
https://github.com/mauriceboe/TREK.git
synced 2026-10-20 22:16:26 +00:00
fix(maps): name the country and drop duplicate places in index suggestions
The suggestion list under the place search showed the index's raw ISO code
("福岡市博多区, JP") and listed a place twice when the index and its
OpenStreetMap layer both carried it, which the search list after Enter
already merges. Suggestions now name the country through Intl in the
language of the search, and a layer row at the spot of an index row with a
matching name is dropped by the same rule mergeSearchResults uses, now
shared as samePlace.
Refs #2592
This commit is contained in:
@@ -4,7 +4,7 @@
|
||||
* need no branch per source. Pure: the calls themselves are trek-places.client's,
|
||||
* the decision to make them is MapsService's.
|
||||
*/
|
||||
import { buildOsmDetails, parsePoiCategories } from './maps.helpers';
|
||||
import { buildOsmDetails, parsePoiCategories, samePlace, toApiLang } from './maps.helpers';
|
||||
import { POI_RESULT_CAP, type PoiSearchResult } from './providers/osm.client';
|
||||
import {
|
||||
POI_CATEGORY_TO_TREK,
|
||||
@@ -144,9 +144,49 @@ export function indexPoiAnswer(
|
||||
};
|
||||
}
|
||||
|
||||
/** Autocomplete rows out of the index and its OpenStreetMap layer, interleaved as they came. */
|
||||
export function indexSuggestions(found: TrekSearchHit[]): MapsAutocompleteResult['suggestions'] {
|
||||
return found.map((p) =>
|
||||
const regionNamesByLang = new Map<string, Intl.DisplayNames | null>();
|
||||
|
||||
/**
|
||||
* The index hands over an ISO code ("JP"), which is not what a person reads a
|
||||
* place by. Named through Intl in the language of the search, the way
|
||||
* journey-domain names countries. A code the runtime cannot name, or a locale
|
||||
* it does not know, leaves the code as it came.
|
||||
*/
|
||||
function countryName(code: string | null | undefined, lang: string | undefined): string | null {
|
||||
if (!code) return null;
|
||||
const locale = toApiLang(lang);
|
||||
if (!regionNamesByLang.has(locale)) {
|
||||
let names: Intl.DisplayNames | null = null;
|
||||
try {
|
||||
names = new Intl.DisplayNames([locale], { type: 'region', fallback: 'code' });
|
||||
} catch {
|
||||
// Not a locale tag, or a Node built without full ICU.
|
||||
}
|
||||
regionNamesByLang.set(locale, names);
|
||||
}
|
||||
try {
|
||||
return regionNamesByLang.get(locale)?.of(code) ?? code;
|
||||
} catch {
|
||||
return code;
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Autocomplete rows out of the index and its OpenStreetMap layer, interleaved as they came.
|
||||
*
|
||||
* A layer row for a place the index already listed is dropped, by the same
|
||||
* rule the search list merges its two sources with (#2592). Both copies at the
|
||||
* same spot read as two shops, and the explicit search showed it once. The
|
||||
* index's copy stays, because it carries the stable id and the contact details.
|
||||
*/
|
||||
export function indexSuggestions(found: TrekSearchHit[], lang?: string): MapsAutocompleteResult['suggestions'] {
|
||||
const indexed = found.filter((p) => !isOsmHit(p));
|
||||
const listed = (p: TrekSearchHit): boolean =>
|
||||
!isOsmHit(p) ||
|
||||
!indexed.some(
|
||||
(i) => samePlace(i, p) || (!!p.local_name && samePlace(i, { lat: p.lat, lng: p.lng, name: p.local_name })),
|
||||
);
|
||||
return found.filter(listed).map((p) =>
|
||||
isOsmHit(p)
|
||||
? {
|
||||
// The service's own id form, translated into the one this
|
||||
@@ -174,7 +214,7 @@ export function indexSuggestions(found: TrekSearchHit[]): MapsAutocompleteResult
|
||||
: {
|
||||
placeId: `gers:${p.gers}`,
|
||||
mainText: p.name,
|
||||
secondaryText: [p.address?.locality, p.address?.country].filter(Boolean).join(', '),
|
||||
secondaryText: [p.address?.locality, countryName(p.address?.country, lang)].filter(Boolean).join(', '),
|
||||
source: 'trek-places',
|
||||
lat: p.lat,
|
||||
lng: p.lng,
|
||||
|
||||
@@ -118,21 +118,7 @@ export function mergeSearchResults(
|
||||
fromOsm: Record<string, unknown>[],
|
||||
limit = 10,
|
||||
): Record<string, unknown>[] {
|
||||
const sameThing = (a: Record<string, unknown>, b: Record<string, unknown>): boolean => {
|
||||
const [aLat, aLng, bLat, bLng] = [a.lat, a.lng, b.lat, b.lng];
|
||||
if (![aLat, aLng, bLat, bLng].every((v) => typeof v === 'number')) return false;
|
||||
// 60 m is close enough to be the same shop. It is NOT enough on its own:
|
||||
// a temple and the coffee shop at its gate are fifty metres apart, and
|
||||
// deduping on distance alone swallowed the temple, which is the exact
|
||||
// result this function exists to surface. Two things in one spot with
|
||||
// unrelated names are two things.
|
||||
if (haversineMetres(aLat as number, aLng as number, bLat as number, bLng as number) >= 60) {
|
||||
return false;
|
||||
}
|
||||
return typeof a.name === 'string' && typeof b.name === 'string' && namesOverlap(a.name, b.name);
|
||||
};
|
||||
|
||||
const extra = fromOsm.filter((o) => !fromIndex.some((i) => sameThing(i, o)));
|
||||
const extra = fromOsm.filter((o) => !fromIndex.some((i) => samePlace(i, o)));
|
||||
|
||||
const out: Record<string, unknown>[] = [];
|
||||
for (let i = 0; i < Math.max(fromIndex.length, extra.length) && out.length < limit; i++) {
|
||||
@@ -142,6 +128,25 @@ export function mergeSearchResults(
|
||||
return out;
|
||||
}
|
||||
|
||||
/**
|
||||
* Whether two rows from different sources are one place. The search list and
|
||||
* the suggestion list both dedupe on this, so a place shown once in one is
|
||||
* shown once in the other.
|
||||
*/
|
||||
export function samePlace(a: { lat?: unknown; lng?: unknown; name?: unknown }, b: typeof a): boolean {
|
||||
const [aLat, aLng, bLat, bLng] = [a.lat, a.lng, b.lat, b.lng];
|
||||
if (![aLat, aLng, bLat, bLng].every((v) => typeof v === 'number')) return false;
|
||||
// 60 m is close enough to be the same shop. It is NOT enough on its own:
|
||||
// a temple and the coffee shop at its gate are fifty metres apart, and
|
||||
// deduping on distance alone swallowed the temple, which is the exact
|
||||
// result this function exists to surface. Two things in one spot with
|
||||
// unrelated names are two things.
|
||||
if (haversineMetres(aLat as number, aLng as number, bLat as number, bLng as number) >= 60) {
|
||||
return false;
|
||||
}
|
||||
return typeof a.name === 'string' && typeof b.name === 'string' && namesOverlap(a.name, b.name);
|
||||
}
|
||||
|
||||
export function namesOverlap(a: string, b: string): boolean {
|
||||
const words = (value: string): Set<string> =>
|
||||
new Set(
|
||||
|
||||
@@ -520,7 +520,7 @@ export class MapsService {
|
||||
limit: 8,
|
||||
sources: 'index,osm',
|
||||
});
|
||||
if (found.length > 0) return { suggestions: indexSuggestions(found), source: 'trek-places' };
|
||||
if (found.length > 0) return { suggestions: indexSuggestions(found, lang), source: 'trek-places' };
|
||||
} catch (err: unknown) {
|
||||
console.warn('TREK Places autocomplete failed, falling back:', (err as Error).message);
|
||||
}
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
/**
|
||||
* MAPS-AUTO-001..013: the suggestion list behind the place search box.
|
||||
* MAPS-AUTO-001..016: the suggestion list behind the place search box.
|
||||
*
|
||||
* This is the path the index was built for. Nominatim's usage policy names
|
||||
* autocomplete as unacceptable use in its own words, whatever the rate, and the
|
||||
@@ -130,7 +130,8 @@ describe('MapsService.autocompletePlaces', () => {
|
||||
expect(result.suggestions[0].placeId).toBe('gers:abc-123');
|
||||
expect(result.suggestions[1].placeId).toBe('gers:def-456');
|
||||
expect(result.suggestions[0].mainText).toBe('Café Kröpeliner');
|
||||
expect(result.suggestions[0].secondaryText).toBe('Rostock, DE');
|
||||
// The country by name, not its ISO code (#2592).
|
||||
expect(result.suggestions[0].secondaryText).toBe('Rostock, Germany');
|
||||
});
|
||||
|
||||
it('MAPS-AUTO-002: a place with no locality gets an empty line, not the word "undefined"', async () => {
|
||||
@@ -288,4 +289,98 @@ describe('MapsService.autocompletePlaces', () => {
|
||||
expect(mockSearch).toHaveBeenCalledTimes(1);
|
||||
expect(off.source).toBe('trek-places');
|
||||
});
|
||||
|
||||
it('MAPS-AUTO-014: the country is named in the language the search was made in (#2592)', async () => {
|
||||
const hakata = hit({ name: 'Kirby Cafe Hakata', address: { locality: '福岡市博多区', country: 'JP' } });
|
||||
mockSearch.mockResolvedValue([hakata]);
|
||||
|
||||
const named = async (lang?: string) =>
|
||||
(await make().autocompletePlaces(1, INPUT, lang)).suggestions[0].secondaryText;
|
||||
|
||||
expect(await named('en')).toBe('福岡市博多区, Japan');
|
||||
expect(await named('de')).toBe('福岡市博多区, Japan');
|
||||
expect(await named('fr')).toBe('福岡市博多区, Japon');
|
||||
expect(await named('ja')).toBe('福岡市博多区, 日本');
|
||||
// TREK's own codes for Brazilian Portuguese and Greek are not BCP 47 tags.
|
||||
expect(await named('br')).toBe('福岡市博多区, Japão');
|
||||
expect(await named('gr')).toBe('福岡市博多区, Ιαπωνία');
|
||||
expect(await named(undefined)).toBe('福岡市博多区, Japan');
|
||||
});
|
||||
|
||||
it('MAPS-AUTO-015: a country the runtime cannot name stays as the index sent it', async () => {
|
||||
mockSearch.mockResolvedValue([
|
||||
hit({ address: { locality: 'Rostock', country: 'JPN' } }),
|
||||
hit({ gers: 'x', address: { locality: 'Rostock', country: '' } }),
|
||||
]);
|
||||
|
||||
const { suggestions } = await make().autocompletePlaces(1, INPUT, 'not a locale!');
|
||||
|
||||
expect(suggestions[0].secondaryText).toBe('Rostock, JPN');
|
||||
expect(suggestions[1].secondaryText).toBe('Rostock');
|
||||
});
|
||||
|
||||
it('MAPS-AUTO-016: a layer row for a place the index already listed is dropped, as the search list does (#2592)', async () => {
|
||||
// The reported case: the index's Kirby Cafe Hakata and the layer's Kirby
|
||||
// Cafe six metres away are one shop, and the list showed it twice. The
|
||||
// layer's Kirby Cafe in Tokyo is another shop and stays.
|
||||
mockSearch.mockResolvedValue([
|
||||
hit({
|
||||
gers: 'hakata',
|
||||
name: 'Kirby Cafe Hakata',
|
||||
lat: 33.59052848815918,
|
||||
lng: 130.4105987548828,
|
||||
address: { locality: '福岡市博多区', country: 'JP' },
|
||||
}),
|
||||
osmHit({
|
||||
id: 'osm:node/1',
|
||||
osm_id: 1,
|
||||
name: 'Kirby Cafe',
|
||||
local_name: 'カービィカフェ',
|
||||
lat: 35.7103685,
|
||||
lng: 139.8118977,
|
||||
}),
|
||||
hit({ gers: 'emmetsburg', name: "Kirby's Cafe", lat: 43.11215019226074, lng: -94.69049072265625 }),
|
||||
osmHit({
|
||||
id: 'osm:node/2',
|
||||
osm_id: 2,
|
||||
name: 'Kirby Cafe',
|
||||
local_name: 'カービィカフェ',
|
||||
lat: 33.5905723,
|
||||
lng: 130.4106152,
|
||||
}),
|
||||
// The same name a hundred metres off is kept, exactly as the search
|
||||
// list keeps it: index coordinates are not that precise, but a chain's
|
||||
// second branch round the corner is not a duplicate either.
|
||||
osmHit({
|
||||
id: 'osm:node/3',
|
||||
osm_id: 3,
|
||||
name: "Kirby's Café",
|
||||
local_name: null,
|
||||
lat: 43.1122282,
|
||||
lng: -94.6917042,
|
||||
}),
|
||||
// The index carries the local name, the layer labels it with the exonym
|
||||
// that matched: compared on the name written on the building instead.
|
||||
hit({ gers: 'hasedera', name: '長谷寺', lat: 35.3124, lng: 139.5333 }),
|
||||
osmHit({
|
||||
id: 'osm:way/4',
|
||||
osm_type: 'way',
|
||||
osm_id: 4,
|
||||
name: 'Hase-dera',
|
||||
local_name: '長谷寺',
|
||||
lat: 35.31245,
|
||||
lng: 139.53335,
|
||||
}),
|
||||
]);
|
||||
|
||||
const { suggestions } = await make().autocompletePlaces(1, INPUT, 'en');
|
||||
|
||||
expect(suggestions.map((s) => s.placeId)).toEqual([
|
||||
'gers:hakata',
|
||||
'node:1',
|
||||
'gers:emmetsburg',
|
||||
'node:3',
|
||||
'gers:hasedera',
|
||||
]);
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user