mirror of
https://github.com/mauriceboe/TREK.git
synced 2026-06-19 13:21:46 +00:00
* feat(places): enrich list-imported places via the Places API (#886) Google/Naver list imports only carry a name and coordinates, so the places open as bare pins — the Maps tab jumps to coordinates, with no photo, address or open/closed. Add an opt-in "Enrich places via Google" toggle to the list-import dialog, shown only when a Google Maps key is configured. When enabled, after the (fast, unchanged) import the server runs a background pass that re-resolves each place by name — biased to and validated against the imported coordinates so a common-name search cannot overwrite the wrong place — and fills the empty address/website/phone/photo columns plus the resolved google_place_id, pushing each row over the live sync. Opening hours and the proper Maps link then work on demand from the stored id. Enrichment only fills empty fields, runs detached so a long list never blocks the import, and no-ops when no key is configured. * fix(places): use the ToggleSwitch component for the enrich toggle Match the rest of the app — the import-enrichment opt-in used a raw checkbox; swap it for the shared ToggleSwitch (text left, switch right) like the settings toggles.
This commit is contained in:
@@ -163,27 +163,30 @@ export class PlacesController {
|
||||
}
|
||||
|
||||
@Post('import/google-list')
|
||||
async importGoogle(@CurrentUser() user: User, @Param('tripId') tripId: string, @Body('url') url: unknown, @Headers('x-socket-id') socketId?: string) {
|
||||
return this.importList('google', user, tripId, url, socketId);
|
||||
async importGoogle(@CurrentUser() user: User, @Param('tripId') tripId: string, @Body('url') url: unknown, @Body('enrich') enrich: unknown, @Headers('x-socket-id') socketId?: string) {
|
||||
return this.importList('google', user, tripId, url, enrich, socketId);
|
||||
}
|
||||
|
||||
@Post('import/naver-list')
|
||||
async importNaver(@CurrentUser() user: User, @Param('tripId') tripId: string, @Body('url') url: unknown, @Headers('x-socket-id') socketId?: string) {
|
||||
return this.importList('naver', user, tripId, url, socketId);
|
||||
async importNaver(@CurrentUser() user: User, @Param('tripId') tripId: string, @Body('url') url: unknown, @Body('enrich') enrich: unknown, @Headers('x-socket-id') socketId?: string) {
|
||||
return this.importList('naver', user, tripId, url, enrich, socketId);
|
||||
}
|
||||
|
||||
/** Shared google/naver list import — identical flow, different provider + error string. */
|
||||
private async importList(provider: 'google' | 'naver', user: User, tripId: string, url: unknown, socketId?: string) {
|
||||
private async importList(provider: 'google' | 'naver', user: User, tripId: string, url: unknown, enrich: unknown, socketId?: string) {
|
||||
const trip = this.requireTrip(tripId, user);
|
||||
this.requireEdit(trip, user);
|
||||
if (!url || typeof url !== 'string') {
|
||||
throw new HttpException({ error: 'URL is required' }, 400);
|
||||
}
|
||||
// Opt-in: re-resolve each imported place via the Places API to fill in
|
||||
// photo / address / website / phone and persist a google_place_id (#886).
|
||||
const opts = { enrich: parseBool(enrich, false), userId: user.id };
|
||||
const label = provider === 'google' ? 'Google' : 'Naver';
|
||||
try {
|
||||
const result = provider === 'google'
|
||||
? await this.places.importGoogleList(tripId, url)
|
||||
: await this.places.importNaverList(tripId, url);
|
||||
? await this.places.importGoogleList(tripId, url, opts)
|
||||
: await this.places.importNaverList(tripId, url, opts);
|
||||
if ('error' in result) {
|
||||
throw new HttpException({ error: result.error }, result.status);
|
||||
}
|
||||
|
||||
@@ -64,12 +64,12 @@ export class PlacesService {
|
||||
return svc.importMapFile(tripId, buffer, filename, opts);
|
||||
}
|
||||
|
||||
importGoogleList(tripId: string, url: string) {
|
||||
return svc.importGoogleList(tripId, url);
|
||||
importGoogleList(tripId: string, url: string, opts?: Parameters<typeof svc.importGoogleList>[2]) {
|
||||
return svc.importGoogleList(tripId, url, opts);
|
||||
}
|
||||
|
||||
importNaverList(tripId: string, url: string) {
|
||||
return svc.importNaverList(tripId, url);
|
||||
importNaverList(tripId: string, url: string, opts?: Parameters<typeof svc.importNaverList>[2]) {
|
||||
return svc.importNaverList(tripId, url, opts);
|
||||
}
|
||||
|
||||
searchImage(tripId: string, id: string, userId: number) {
|
||||
|
||||
@@ -0,0 +1,164 @@
|
||||
import { db, getPlaceWithTags } from '../db/database';
|
||||
import { broadcast } from '../websocket';
|
||||
import { getMapsKey, searchPlaces, getPlacePhoto } from './mapsService';
|
||||
|
||||
/**
|
||||
* Background enrichment for list-imported places (#886).
|
||||
*
|
||||
* Google/Naver list imports only carry name + coordinates, so the imported
|
||||
* places open as bare pins (the Maps tab jumps to coordinates, no photo, no
|
||||
* open/closed). When the importer opts in and a Google Maps key is configured,
|
||||
* we re-resolve each place by name — biased to and validated against the
|
||||
* imported coordinates — to a real Google place, then fill in the empty fields
|
||||
* and persist the resolved `google_place_id` (which is what powers on-demand
|
||||
* opening hours / the proper Maps link going forward).
|
||||
*
|
||||
* This runs detached from the import request (fire-and-forget) so a long list
|
||||
* never blocks the response, and pushes each enriched row over the websocket so
|
||||
* the sidebar fills in progressively. It only ever fills EMPTY columns, so it
|
||||
* can never clobber data the import already captured (e.g. a Naver address).
|
||||
*/
|
||||
|
||||
/** A place the import produced — only the fields enrichment reads/writes. */
|
||||
export interface EnrichablePlace {
|
||||
id: number;
|
||||
name: string;
|
||||
lat: number;
|
||||
lng: number;
|
||||
google_place_id?: string | null;
|
||||
address?: string | null;
|
||||
website?: string | null;
|
||||
phone?: string | null;
|
||||
image_url?: string | null;
|
||||
}
|
||||
|
||||
/** How close a search hit must be to the imported coordinates to be trusted. */
|
||||
const MATCH_RADIUS_METERS = 250;
|
||||
/** Bias the text search to roughly the imported area. */
|
||||
const SEARCH_BIAS_RADIUS_METERS = 2000;
|
||||
/** Concurrent enrichment lookups — small, to stay friendly to the Maps quota. */
|
||||
const ENRICH_CONCURRENCY = 3;
|
||||
|
||||
function haversineMeters(a: { lat: number; lng: number }, b: { lat: number; lng: number }): number {
|
||||
const R = 6371000;
|
||||
const toRad = (d: number) => (d * Math.PI) / 180;
|
||||
const dLat = toRad(b.lat - a.lat);
|
||||
const dLng = toRad(b.lng - a.lng);
|
||||
const lat1 = toRad(a.lat);
|
||||
const lat2 = toRad(b.lat);
|
||||
const h = Math.sin(dLat / 2) ** 2 + Math.cos(lat1) * Math.cos(lat2) * Math.sin(dLng / 2) ** 2;
|
||||
return 2 * R * Math.asin(Math.sqrt(h));
|
||||
}
|
||||
|
||||
/**
|
||||
* Pick the search result that is the same place as the import: it must be a
|
||||
* Google result (have a google_place_id) with coordinates within
|
||||
* MATCH_RADIUS_METERS of the imported point. Returns the closest such hit, or
|
||||
* null when nothing is close enough — in which case the place is left as
|
||||
* imported rather than risking a wrong-place overwrite (common-name / romanized
|
||||
* lists). Exported for unit testing.
|
||||
*/
|
||||
export function pickEnrichmentMatch(
|
||||
candidates: Record<string, unknown>[],
|
||||
target: { lat: number; lng: number },
|
||||
maxMeters: number = MATCH_RADIUS_METERS,
|
||||
): Record<string, unknown> | null {
|
||||
let best: { c: Record<string, unknown>; dist: number } | null = null;
|
||||
for (const c of candidates || []) {
|
||||
const gpid = c.google_place_id;
|
||||
const lat = c.lat;
|
||||
const lng = c.lng;
|
||||
if (typeof gpid !== 'string' || !gpid) continue;
|
||||
if (typeof lat !== 'number' || typeof lng !== 'number') continue;
|
||||
const dist = haversineMeters(target, { lat, lng });
|
||||
if (dist > maxMeters) continue;
|
||||
if (!best || dist < best.dist) best = { c, dist };
|
||||
}
|
||||
return best?.c ?? null;
|
||||
}
|
||||
|
||||
async function mapWithConcurrency<T>(items: T[], limit: number, fn: (item: T) => Promise<void>): Promise<void> {
|
||||
let cursor = 0;
|
||||
const workers = Array.from({ length: Math.min(limit, items.length) }, async () => {
|
||||
while (cursor < items.length) {
|
||||
const item = items[cursor++];
|
||||
await fn(item);
|
||||
}
|
||||
});
|
||||
await Promise.all(workers);
|
||||
}
|
||||
|
||||
const str = (v: unknown): string | null => (typeof v === 'string' && v.trim() ? v.trim() : null);
|
||||
|
||||
async function enrichOne(tripId: string, userId: number, place: EnrichablePlace, lang?: string): Promise<void> {
|
||||
// Already linked (shouldn't happen for list imports) — nothing to resolve.
|
||||
if (place.google_place_id) return;
|
||||
if (typeof place.lat !== 'number' || typeof place.lng !== 'number') return;
|
||||
|
||||
const { places: results } = await searchPlaces(userId, place.name, lang, {
|
||||
lat: place.lat,
|
||||
lng: place.lng,
|
||||
radius: SEARCH_BIAS_RADIUS_METERS,
|
||||
});
|
||||
const match = pickEnrichmentMatch(results, { lat: place.lat, lng: place.lng });
|
||||
if (!match) return;
|
||||
|
||||
const gpid = str(match.google_place_id);
|
||||
if (!gpid) return;
|
||||
|
||||
// COALESCE so enrichment only fills empty columns — never overwrites data the
|
||||
// import already captured (e.g. Naver's address) or anything the user edited.
|
||||
db.prepare(
|
||||
`UPDATE places
|
||||
SET google_place_id = COALESCE(google_place_id, ?),
|
||||
address = COALESCE(address, ?),
|
||||
website = COALESCE(website, ?),
|
||||
phone = COALESCE(phone, ?),
|
||||
updated_at = CURRENT_TIMESTAMP
|
||||
WHERE id = ? AND trip_id = ?`,
|
||||
).run(gpid, str(match.address), str(match.website), str(match.phone), place.id, tripId);
|
||||
|
||||
// Photo is best-effort: Google often has none, and getPlacePhoto throws 404 in
|
||||
// that case — a missing photo must never abort the rest of the enrichment.
|
||||
try {
|
||||
const photo = await getPlacePhoto(userId, gpid, place.lat, place.lng, place.name);
|
||||
if (photo?.photoUrl) {
|
||||
db.prepare(
|
||||
'UPDATE places SET image_url = COALESCE(image_url, ?), updated_at = CURRENT_TIMESTAMP WHERE id = ? AND trip_id = ?',
|
||||
).run(photo.photoUrl, place.id, tripId);
|
||||
}
|
||||
} catch {
|
||||
/* no photo — leave image_url as-is */
|
||||
}
|
||||
|
||||
// Push the enriched row to every connected client (no socket exclusion: the
|
||||
// importer's own client should also receive the late update).
|
||||
const updated = getPlaceWithTags(place.id);
|
||||
if (updated) broadcast(tripId, 'place:updated', { place: updated }, undefined);
|
||||
}
|
||||
|
||||
/**
|
||||
* Enrich a batch of just-imported places in the background. Never throws —
|
||||
* any per-place failure is swallowed so one bad lookup can't take down the
|
||||
* detached task or the process. No-ops when no Google Maps key is configured.
|
||||
*/
|
||||
export async function enrichImportedPlaces(
|
||||
tripId: string,
|
||||
userId: number,
|
||||
places: EnrichablePlace[],
|
||||
lang?: string,
|
||||
): Promise<void> {
|
||||
try {
|
||||
if (!places.length) return;
|
||||
if (!getMapsKey(userId)) return;
|
||||
await mapWithConcurrency(places, ENRICH_CONCURRENCY, async (place) => {
|
||||
try {
|
||||
await enrichOne(tripId, userId, place, lang);
|
||||
} catch (err) {
|
||||
console.error(`[Places] enrichment failed for place ${place.id}:`, err instanceof Error ? err.message : err);
|
||||
}
|
||||
});
|
||||
} catch (err) {
|
||||
console.error('[Places] import enrichment pass failed:', err instanceof Error ? err.message : err);
|
||||
}
|
||||
}
|
||||
@@ -13,6 +13,14 @@ import {
|
||||
resolveCategoryIdForFolder,
|
||||
type KmlImportSummary,
|
||||
} from './kmlImport';
|
||||
import { enrichImportedPlaces, type EnrichablePlace } from './placeEnrichment';
|
||||
|
||||
/** Opt-in Places-API enrichment for list imports (#886). */
|
||||
export interface ListImportOptions {
|
||||
enrich?: boolean;
|
||||
userId?: number;
|
||||
lang?: string;
|
||||
}
|
||||
|
||||
interface PlaceWithCategory extends Place {
|
||||
category_name: string | null;
|
||||
@@ -595,7 +603,7 @@ export async function importMapFile(tripId: string, fileBuffer: Buffer, filename
|
||||
// Import Google Maps list
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
export async function importGoogleList(tripId: string, url: string) {
|
||||
export async function importGoogleList(tripId: string, url: string, opts?: ListImportOptions) {
|
||||
let listId: string | null = null;
|
||||
let resolvedUrl = url;
|
||||
|
||||
@@ -697,6 +705,10 @@ export async function importGoogleList(tripId: string, url: string) {
|
||||
});
|
||||
insertAll();
|
||||
|
||||
if (opts?.enrich && opts.userId && created.length) {
|
||||
void enrichImportedPlaces(tripId, opts.userId, created as EnrichablePlace[], opts.lang);
|
||||
}
|
||||
|
||||
return { places: created, listName, skipped };
|
||||
}
|
||||
|
||||
@@ -707,6 +719,7 @@ export async function importGoogleList(tripId: string, url: string) {
|
||||
export async function importNaverList(
|
||||
tripId: string,
|
||||
url: string,
|
||||
opts?: ListImportOptions,
|
||||
): Promise<{ places: any[]; listName: string; skipped: number } | { error: string; status: number }> {
|
||||
let resolvedUrl = url;
|
||||
const limit = 20;
|
||||
@@ -826,6 +839,10 @@ export async function importNaverList(
|
||||
});
|
||||
insertAll();
|
||||
|
||||
if (opts?.enrich && opts.userId && created.length) {
|
||||
void enrichImportedPlaces(tripId, opts.userId, created as EnrichablePlace[], opts.lang);
|
||||
}
|
||||
|
||||
return { places: created, listName, skipped };
|
||||
}
|
||||
|
||||
|
||||
@@ -0,0 +1,49 @@
|
||||
/**
|
||||
* Unit tests for the import-enrichment match selector (#886).
|
||||
* Covers PENRICH-001 to PENRICH-004 — the coordinate-validation guard that
|
||||
* prevents a name search from overwriting an imported place with the wrong POI.
|
||||
*/
|
||||
import { describe, it, expect, vi } from 'vitest';
|
||||
|
||||
// placeEnrichment pulls in the DB, websocket and maps service at import time;
|
||||
// stub them so the pure match selector can be tested in isolation.
|
||||
vi.mock('../../../src/db/database', () => ({ db: {}, getPlaceWithTags: () => null }));
|
||||
vi.mock('../../../src/websocket', () => ({ broadcast: () => {} }));
|
||||
vi.mock('../../../src/services/mapsService', () => ({
|
||||
getMapsKey: () => null,
|
||||
searchPlaces: async () => ({ places: [], source: 'none' }),
|
||||
getPlacePhoto: async () => ({ photoUrl: '', attribution: null }),
|
||||
}));
|
||||
|
||||
import { pickEnrichmentMatch } from '../../../src/services/placeEnrichment';
|
||||
|
||||
const target = { lat: 48.85, lng: 2.35 };
|
||||
|
||||
describe('pickEnrichmentMatch', () => {
|
||||
it('PENRICH-001: picks the closest Google candidate within the radius', () => {
|
||||
const candidates = [
|
||||
{ google_place_id: 'far', lat: 48.8512, lng: 2.3512 }, // ~170 m
|
||||
{ google_place_id: 'near', lat: 48.85, lng: 2.35 }, // exact
|
||||
];
|
||||
const match = pickEnrichmentMatch(candidates, target);
|
||||
expect(match?.google_place_id).toBe('near');
|
||||
});
|
||||
|
||||
it('PENRICH-002: returns null when every candidate is beyond the radius', () => {
|
||||
const candidates = [{ google_place_id: 'A', lat: 48.86, lng: 2.36 }]; // ~1.2 km
|
||||
expect(pickEnrichmentMatch(candidates, target)).toBeNull();
|
||||
});
|
||||
|
||||
it('PENRICH-003: ignores candidates without a google_place_id (e.g. OSM results)', () => {
|
||||
const candidates = [
|
||||
{ google_place_id: null, lat: 48.85, lng: 2.35 },
|
||||
{ name: 'no id', lat: 48.85, lng: 2.35 },
|
||||
];
|
||||
expect(pickEnrichmentMatch(candidates, target)).toBeNull();
|
||||
});
|
||||
|
||||
it('PENRICH-004: ignores candidates with non-numeric coordinates', () => {
|
||||
const candidates = [{ google_place_id: 'A', lat: 'x', lng: 'y' }];
|
||||
expect(pickEnrichmentMatch(candidates as never, target)).toBeNull();
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user