fix(places): a track is a duplicate only of a track, never of a sight of that name

A GPX or KML import skipped every row whose name was already a place of the
trip. A watch names a hike after where it went, so "Hokitika Gorge" was
refused as already in the trip because the gorge itself was a planned
place, with no track to show for it (#2810). A track now matches only the
tracks of that name; a pin still matches every place of its name.

The Places filter also hid its "Tracks" choice whenever Tours was on, on the
grounds that tracks are tours then. A track imported before Tours is a
plain place and stays in the list, so the choice now counts the tracks that
are no tour.

client/src/components/Planner/usePlacesPool.test.ts

@@ -48,13 +48,17 @@ afterEach(() => {
 });
 
 describe('places pool helpers', () => {
-  it('FE-PLANNER-POOL-001: the Tracks choice needs a track and stays away while tours are on', () => {
+  it('FE-PLANNER-POOL-001: the Tracks choice needs a track that is no tour while tours are on (#2810)', () => {
     const track = buildPlace({ route_geometry: '[[1,2],[3,4]]' });
+    const tour = buildPlace({ route_geometry: '[[1,2],[3,4]]' });
+    tour.tour_place_id = tour.id;
     const pin = buildPlace();
     expect(poolHasTracks([pin], false)).toBe(false);
     expect(poolHasTracks([pin, track], false)).toBe(true);
     expect(poolHasTracks([pin, track], undefined)).toBe(true);
-    expect(poolHasTracks([pin, track], true)).toBe(false);
+    expect(poolHasTracks([pin, tour], true)).toBe(false);
+    expect(poolHasTracks([pin, tour], false)).toBe(true);
+    expect(poolHasTracks([pin, tour, track], true)).toBe(true);
   });
 
   it('FE-PLANNER-POOL-002: the show choices list all, unplanned and planned, then tracks when there are some', () => {

client/src/components/Planner/usePlacesPool.ts

@@ -12,9 +12,13 @@ export type PlacesFilter = 'all' | 'unplanned' | 'planned' | 'tracks';
 
 type Translate = ReturnType<typeof useTranslation>['t'];
 
-/** Whether the "Tracks" choice belongs in the pool: never while tours are on, since tracks are tours then. */
+/**
+ * Whether the "Tracks" choice belongs in the pool: a place with a line that is
+ * no tour. While tours are on a tour lists under Tours, but a track imported
+ * before them (4.x) is a plain place and stays here (#2810).
+ */
 export function poolHasTracks(places: Place[], toursEnabled: boolean | undefined): boolean {
-  return !toursEnabled && places.some((p) => p.route_geometry);
+  return places.some((p) => p.route_geometry && !(toursEnabled && p.tour_place_id === p.id));
 }
 
 /** The "show" choices; tracks only once a place has one. */

server/src/db/repositories/Places.repository.ts

@@ -590,6 +590,12 @@ export class PlacesRepository extends TrekRepository<Places> {
       .execute<{ name: string; description: string | null; address: string | null; lat: number | null; lng: number | null; route_geometry: string | null; category: string | null }[]>('all', false);
   }
 
+  /** The names of the trip's places that carry a line, which a track to import is matched against (#2810). */
+  async listTrackNames(trip_id: number): Promise<string[]> {
+    const rows = await this.find({ trip: trip_id, route_geometry: { $ne: null } }, { fields: ['name'] });
+    return rows.map((row) => row.name);
+  }
+
   /**
    * PL24 (`PlacesService.buildDedupSet`'s read) — `SELECT name, lat, lng,
    * google_place_id, google_ftid, osm_id, amap_poi_id FROM places WHERE

server/src/nest/places/places.helpers.ts

@@ -86,11 +86,16 @@ export async function reclaimPhotoCache(
 
 export interface DedupSet {
   names: Set<string>;
+  /** The names of the places with a line: a track to import matches only these (#2810). Absent where none is read. */
+  trackNames?: Set<string>;
   coords: Array<{ lat: number; lng: number }>;
   /** Provider ids (google_place_id, google_ftid, osm_id, amap_poi_id) of the places already in the trip. */
   externalIds: Set<string>;
 }
 
+/** A candidate for the dedup set; `track` when it is a line (a GPX track or route, a KML path). */
+export type DedupCandidate = PlaceMatchCandidate & { track?: boolean };
+
 /**
  * Returns true if a candidate place is already represented in the dedup set.
  *
@@ -104,12 +109,14 @@ export interface DedupSet {
  * JavaScript's is not, and only unnamed rows contribute coordinates to a dedup
  * set. `findDuplicatePlace` spells out where the two still answer differently.
  */
-export function isPlaceDuplicate(candidate: PlaceMatchCandidate, dedup: DedupSet): boolean {
+export function isPlaceDuplicate(candidate: DedupCandidate, dedup: DedupSet): boolean {
   for (const strategy of placeMatchStrategies(candidate)) {
     if (strategy.by === 'externalId') {
       if (dedup.externalIds.has(strategy.id)) return true;
     } else if (strategy.by === 'name') {
-      if (dedup.names.has(strategy.name)) return true;
+      // A track is the same as a track of that name, never as a sight of that name:
+      // a watch names a hike after the place it went to (#2810).
+      if ((candidate.track ? dedup.trackNames : dedup.names)?.has(strategy.name)) return true;
     } else if (
       dedup.coords.some(
         (c) =>
@@ -132,6 +139,7 @@ export function dedupSetOf(
   rows: ReadonlyArray<
     { name: string | null; lat: number | null; lng: number | null } & Parameters<typeof externalIdsOf>[0]
   >,
+  trackNames: ReadonlyArray<string | null> = [],
 ): DedupSet {
   const names = new Set<string>();
   const coords: Array<{ lat: number; lng: number }> = [];
@@ -144,15 +152,17 @@ export function dedupSetOf(
       coords.push({ lat: row.lat, lng: row.lng });
     }
   }
-  return { names, coords, externalIds };
+  const tracks = trackNames.map(normalizePlaceName).filter((name): name is string => !!name);
+  return { names, trackNames: new Set(tracks), coords, externalIds };
 }
 
 /** Record a newly inserted place so subsequent candidates in the same batch are checked against it. */
-export function trackInsertedInDedupSet(place: PlaceMatchCandidate, dedup: DedupSet): void {
+export function trackInsertedInDedupSet(place: DedupCandidate, dedup: DedupSet): void {
   for (const id of externalIdsOf(place)) dedup.externalIds.add(id);
   const normalizedName = normalizePlaceName(place.name);
   if (normalizedName) {
     dedup.names.add(normalizedName);
+    if (place.track) (dedup.trackNames ??= new Set()).add(normalizedName);
   } else if (place.lat != null && place.lng != null) {
     dedup.coords.push({ lat: place.lat, lng: place.lng });
   }

server/src/nest/places/places.service.ts

@@ -751,14 +751,14 @@ export class PlacesService {
   /** Build a lookup of names/coords for places already in a trip. */
   private async buildDedupSet(tripId: string): Promise<DedupSet> {
     // PL24 — `PlacesRepository.listDedupInputs` replaces the raw statement (Task 5 review L6 ruling).
-    return dedupSetOf(await this.placesRepo.listDedupInputs(tripId));
+    return dedupSetOf(await this.placesRepo.listDedupInputs(tripId), await this.placesRepo.listTrackNames(toRowId(tripId) ?? -1));
   }
 
   /** Which prepared GPX rows the import would skip as already in the trip: its own rule, read only. */
   async gpxDuplicates(tripId: string, rows: PreparedGpxPlace[]): Promise<boolean[]> {
     const dedup = await this.buildDedupSet(tripId);
     return rows.map((wp) => {
-      const candidate = { name: wp.name, lat: wp.lat, lng: wp.lng };
+      const candidate = { name: wp.name, lat: wp.lat, lng: wp.lng, track: !!wp.routeGeometry };
       if (isPlaceDuplicate(candidate, dedup)) return true;
       trackInsertedInDedupSet(candidate, dedup);
       return false;
@@ -925,7 +925,7 @@ export class PlacesService {
     // `findWithTagsAndRatings` INSIDE the caller's transaction, so the read
     // sees the still-uncommitted row.
     for (const wp of waypoints) {
-      if (isPlaceDuplicate({ name: wp.name, lat: wp.lat, lng: wp.lng }, dedup)) {
+      if (isPlaceDuplicate({ name: wp.name, lat: wp.lat, lng: wp.lng, track: !!wp.routeGeometry }, dedup)) {
         skipped++;
         continue;
       }
@@ -960,7 +960,7 @@ export class PlacesService {
       });
       const place = (await this.placesRepo.findWithTagsAndRatings(placeId))!;
       created.push(place);
-      trackInsertedInDedupSet({ name: wp.name, lat: wp.lat, lng: wp.lng }, dedup);
+      trackInsertedInDedupSet({ name: wp.name, lat: wp.lat, lng: wp.lng, track: !!wp.routeGeometry }, dedup);
     }
 
     return { places: created, count: created.length, skipped };
@@ -1033,7 +1033,7 @@ export class PlacesService {
         const fallbackName = `Placemark ${fallbackIndex}`;
         const name = parsedPlacemark.name || fallbackName;
 
-        if (isPlaceDuplicate({ name, lat: parsedPlacemark.lat, lng: parsedPlacemark.lng }, dedup)) {
+        if (isPlaceDuplicate({ name, lat: parsedPlacemark.lat, lng: parsedPlacemark.lng, track: isPath }, dedup)) {
           summary.skippedCount += 1;
           dupCount++;
           fallbackIndex += 1;
@@ -1079,7 +1079,7 @@ export class PlacesService {
 
         const place = (await this.placesRepo.findWithTagsAndRatings(placeId))!;
         created.push(place);
-        trackInsertedInDedupSet({ name, lat: parsedPlacemark.lat, lng: parsedPlacemark.lng }, dedup);
+        trackInsertedInDedupSet({ name, lat: parsedPlacemark.lat, lng: parsedPlacemark.lng, track: isPath }, dedup);
         summary.createdCount += 1;
         fallbackIndex += 1;
       }

server/tests/e2e/tours.e2e.test.ts

@@ -565,6 +565,21 @@ describe('Tours e2e (real guards + temp SQLite)', () => {
       expect((await importGpx(OWNER, TRACK_GPX)).body).toEqual({ tours: [], caution: false, skipped: 1 });
     });
 
+    it('imports a hike named after a sight of the trip instead of skipping it as a duplicate (#2810)', async () => {
+      // A watch names the hike after where it went, and that place is already planned.
+      const sight = (await makePlace(orm, 5, { name: 'alpine loop', lat: 47, lng: 11 })).id;
+
+      const preview = await previewGpx(OWNER, TRACK_GPX);
+      expect(preview.body.tracks[0]).toMatchObject({ name: 'Alpine loop', duplicate: false });
+
+      const res = await importGpx(OWNER, TRACK_GPX);
+      expect(res.status).toBe(201);
+      expect(res.body).toMatchObject({ skipped: 0 });
+      expect(res.body.tours).toHaveLength(1);
+      expect(res.body.tours[0].place_id).not.toBe(sight);
+      expect(await placeColumns(sight, 'route_geometry')).toEqual({ route_geometry: null });
+    });
+
     it('400 without a file, and for a file with no track or route', async () => {
       const missing = await importGpx(OWNER, null);
       expect(missing.status).toBe(400);

server/tests/unit/db/repositories/Places.repository.test.ts

@@ -1587,4 +1587,16 @@ describe('PlacesRepository.isTour / listForTrip tour_place_id', () => {
     expect(byId.get(tour.id)).toBe(tour.id);
     expect(byId.get(plain.id)).toBeNull();
   });
+
+  it("PLACEREPO-TOUR-005: listTrackNames lists the names of the trip's places with a line (#2810)", async () => {
+    const { user } = createUser(testDb);
+    const trip = createTrip(testDb, user.id);
+    const other = createTrip(testDb, user.id);
+    const line = '[[49,12],[49.01,12.01]]';
+    const tracks = [createPlace(testDb, trip.id, { name: 'Garmin hike' }), createPlace(testDb, trip.id, { name: 'Ridge walk' }), createPlace(testDb, other.id, { name: 'Elsewhere' })];
+    await updateRows(t.orm, Places, { id: { $in: tracks.map((p) => p.id) } }, { route_geometry: line });
+    createPlace(testDb, trip.id, { name: 'Hokitika Gorge' });
+
+    expect((await places.listTrackNames(trip.id)).sort()).toEqual(['Garmin hike', 'Ridge walk']);
+  });
 });

server/tests/unit/nest/places.helpers.test.ts

@@ -5,6 +5,7 @@
  */
 import {
   COORD_DEDUP_TOLERANCE,
+  dedupSetOf,
   externalIdsOf,
   isPlaceDuplicate,
   mapWithConcurrency,
@@ -25,6 +26,32 @@ vi.mock('../../../src/db/database', () => ({
 const emptyDedup = (): DedupSet => ({ names: new Set(), coords: [], externalIds: new Set() });
 
 describe('isPlaceDuplicate / trackInsertedInDedupSet', () => {
+  it('matches a track only against the tracks of that name, never a sight of that name (#2810)', () => {
+    // The watch named the hike after the gorge, and the gorge is already a place of the trip.
+    const dedup = dedupSetOf(
+      [
+        { name: 'Hokitika Gorge', lat: -42.95, lng: 171.0 },
+        { name: '008', lat: 1, lng: 2 },
+      ],
+      [' 008 ', null],
+    );
+    expect(isPlaceDuplicate({ name: 'hokitika gorge', lat: -42.9, lng: 171.0, track: true }, dedup)).toBe(false);
+    expect(isPlaceDuplicate({ name: '008', lat: 1, lng: 2, track: true }, dedup)).toBe(true);
+    // A pin still matches every place of its name, a track included.
+    expect(isPlaceDuplicate({ name: 'Hokitika Gorge', lat: null, lng: null }, dedup)).toBe(true);
+    expect(isPlaceDuplicate({ name: '008', lat: null, lng: null }, dedup)).toBe(true);
+
+    // A track imported earlier in the same file counts for the next one.
+    trackInsertedInDedupSet({ name: 'Hokitika Gorge', lat: -42.9, lng: 171.0, track: true }, dedup);
+    expect(isPlaceDuplicate({ name: 'Hokitika gorge ', lat: null, lng: null, track: true }, dedup)).toBe(true);
+    // A set built by hand without tracks matches no track by name.
+    const bare = emptyDedup();
+    trackInsertedInDedupSet({ name: 'Ridge', lat: 1, lng: 2 }, bare);
+    expect(isPlaceDuplicate({ name: 'Ridge', lat: 1, lng: 2, track: true }, bare)).toBe(false);
+    trackInsertedInDedupSet({ name: 'Lake', lat: 1, lng: 2, track: true }, bare);
+    expect([...(bare.trackNames ?? [])]).toEqual(['lake']);
+  });
+
   it('matches a named place case- and whitespace-insensitively', () => {
     const dedup = emptyDedup();
     trackInsertedInDedupSet({ name: '  Eiffel Tower ', lat: 1, lng: 2 }, dedup);