fix(tours): keep the waypoints of a GPX/KML file that also holds a track

With the Tours addon on, a file picked or dropped on the places column goes
into the Tours import as soon as it holds one line, and that import reads
lines only. A Sygic or Trip-o-matic export (a route plus its stops) came in
as one tour and every place was dropped without a word.

The preview now counts a GPX file's waypoints as it already counted a KML
file's pins, and the dialog offers to add those points as places of the
trip, on by default. Confirming runs the Tours import for the lines and the
places import (points only) for the same files. A file whose tracks the
trip already holds can still bring its points.

Closes #2969

client/src/components/Tours/TourGpxPreviewDialog.test.tsx

@@ -1,8 +1,8 @@
-// FE-GPX-PREVIEW-001 to FE-GPX-PREVIEW-004
+// FE-GPX-PREVIEW-001 to FE-GPX-PREVIEW-007
 // The GPX preview as ACT-01 uses it: preset to the tour's type, a city walk
 // imported as one, and when a track was recorded shown where it carried times.
 import { fireEvent, render, screen } from '@testing-library/react';
-import type { TourGpxPreviewTrack } from '@trek/shared';
+import type { TourFileLeftOut, TourGpxPreviewTrack } from '@trek/shared';
 import { describe, expect, it, vi } from 'vitest';
 import TourGpxPreviewDialog from './TourGpxPreviewDialog';
 import type { TourGpxImport } from './useTourGpxImport';
@@ -27,10 +27,10 @@ function track(overrides: Partial<TourGpxPreviewTrack> = {}): TourGpxPreviewTrac
   };
 }
 
-function gpxImport(tracks: TourGpxPreviewTrack[]): TourGpxImport {
+function gpxImport(tracks: TourGpxPreviewTrack[], leftOut?: TourFileLeftOut): TourGpxImport {
   return {
     importing: false,
-    preview: [{ file: new File(['<gpx/>'], 'morning.gpx'), tracks }],
+    preview: [{ file: new File(['<gpx/>'], 'morning.gpx'), tracks, leftOut }],
     confirm: vi.fn(),
     cancel: vi.fn(),
     pick: vi.fn(),
@@ -63,19 +63,54 @@ describe('TourGpxPreviewDialog (ACT-01)', () => {
       />
     );
     fireEvent.click(screen.getByTestId('gpx-preview-import'));
-    expect(current.confirm).toHaveBeenCalledWith({ tour_type: 'kayak', boat_type: 'sup', water_context: 'lake' });
+    expect(current.confirm).toHaveBeenCalledWith(
+      { tour_type: 'kayak', boat_type: 'sup', water_context: 'lake' },
+      { points: false }
+    );
   });
 
   it('FE-GPX-PREVIEW-003: a city walk is imported as a city walk, and without a preset as a hike', () => {
     const walk = gpxImport([track()]);
     const { unmount } = render(<TourGpxPreviewDialog gpxImport={walk} initial={{ tour_type: 'walk' }} />);
     fireEvent.click(screen.getByTestId('gpx-preview-import'));
-    expect(walk.confirm).toHaveBeenCalledWith({ tour_type: 'walk' });
+    expect(walk.confirm).toHaveBeenCalledWith({ tour_type: 'walk' }, { points: false });
     unmount();
 
     const hike = gpxImport([track()]);
     render(<TourGpxPreviewDialog gpxImport={hike} />);
     fireEvent.click(screen.getByTestId('gpx-preview-import'));
-    expect(hike.confirm).toHaveBeenCalledWith({ tour_type: 'hike' });
+    expect(hike.confirm).toHaveBeenCalledWith({ tour_type: 'hike' }, { points: false });
+  });
+});
+
+describe('TourGpxPreviewDialog with points beside the lines (#2969)', () => {
+  it('FE-GPX-PREVIEW-005: offers the points as places, on by default, and imports them with the tour', () => {
+    const current = gpxImport([track()], { points: 4, areas: 0, other: 0 });
+    render(<TourGpxPreviewDialog gpxImport={current} />);
+    expect(screen.getByTestId('gpx-preview-points')).toBeInTheDocument();
+    // Added as places, they are not listed as left out.
+    expect(screen.queryByTestId('gpx-preview-left-out')).toBeNull();
+    fireEvent.click(screen.getByTestId('gpx-preview-import'));
+    expect(current.confirm).toHaveBeenCalledWith({ tour_type: 'hike' }, { points: true });
+  });
+
+  it('FE-GPX-PREVIEW-006: switched off, the points are named as left out and not imported', () => {
+    const current = gpxImport([track()], { points: 4, areas: 1, other: 0 });
+    render(<TourGpxPreviewDialog gpxImport={current} />);
+    fireEvent.click(screen.getByRole('button', { name: /tours\.import\.pointsAsPlaces|Add the points as places/ }));
+    expect(screen.getByTestId('gpx-preview-left-out')).toHaveTextContent(/tours\.import\.leftOutPoints|4 pins/);
+    fireEvent.click(screen.getByTestId('gpx-preview-import'));
+    expect(current.confirm).toHaveBeenCalledWith({ tour_type: 'hike' }, { points: false });
+  });
+
+  it('FE-GPX-PREVIEW-007: a file whose every track the trip holds can still bring its points', () => {
+    const current = gpxImport([track({ duplicate: true })], { points: 2, areas: 0, other: 0 });
+    render(<TourGpxPreviewDialog gpxImport={current} />);
+    const button = screen.getByTestId('gpx-preview-import');
+    expect(button).toBeEnabled();
+    expect(button).toHaveTextContent(/tours\.import\.pointsConfirm|Add places/);
+    expect(screen.queryByText(/tours\.import\.previewAllDuplicates|nothing to import/)).toBeNull();
+    fireEvent.click(button);
+    expect(current.confirm).toHaveBeenCalledWith({ tour_type: 'hike' }, { points: true });
   });
 });

client/src/components/Tours/TourGpxPreviewDialog.tsx

@@ -1,6 +1,6 @@
 import type { TourFileLeftOut, TourGpxImportFields } from '@trek/shared';
 import { AlertTriangle, Ban, Clock, FileDown, Layers, MapPin, Shapes } from 'lucide-react';
-import { useId, useMemo } from 'react';
+import { useId, useMemo, useState } from 'react';
 import { OFM_POSITRON } from '../../constants/mapDefaults';
 import { useTileUrl } from '../../hooks/useTileUrl';
 import { useTranslation } from '../../i18n';
@@ -8,6 +8,7 @@ import { useSettingsStore } from '../../store/settingsStore';
 import { formatDistance, formatElevation } from '../../utils/units';
 import { MapViewAuto } from '../Map/MapViewAuto';
 import { SoftPill } from '../Planner/planParts';
+import ToggleSwitch from '../Settings/ToggleSwitch';
 import {
   DialogButton,
   DialogFooter,
@@ -98,8 +99,8 @@ function PreviewTrackRow({
   );
 }
 
-/** What all the files held besides their lines, summed by kind; null when they held nothing else. */
-function leftOutOf(files: readonly GpxPreviewFile[]): TourFileLeftOut | null {
+/** What all the files held besides their lines, summed by kind. */
+function leftOutOf(files: readonly GpxPreviewFile[]): TourFileLeftOut {
   const sum = { points: 0, areas: 0, other: 0 };
   for (const { leftOut } of files) {
     if (!leftOut) continue;
@@ -107,18 +108,46 @@ function leftOutOf(files: readonly GpxPreviewFile[]): TourFileLeftOut | null {
     sum.areas += leftOut.areas;
     sum.other += leftOut.other;
   }
-  return sum.points + sum.areas + sum.other > 0 ? sum : null;
+  return sum;
 }
 
 /**
- * IO-02: what a KML or KMZ file held besides its lines, named before the
- * import so nothing goes missing unseen: a dashed card with each kind as a
- * counted badge, and why in the info dot.
+ * #2969: the single points of the files (GPX waypoints, KML pins), which no
+ * tour is made of, offered as places of the trip. On by default: a file with
+ * a route and its stops is usually meant as both.
  */
-function PreviewLeftOut({ files }: { files: readonly GpxPreviewFile[] }) {
+function PreviewPoints({ count, on, onToggle }: { count: number; on: boolean; onToggle: () => void }) {
   const { t } = useTranslation();
-  const leftOut = leftOutOf(files);
-  if (!leftOut) return null;
+  const hintId = useId();
+  return (
+    <div
+      data-testid="gpx-preview-points"
+      className="mt-2 flex items-center gap-2.5 rounded-[10px] border border-edge-faint bg-surface-card px-3 py-2"
+    >
+      <MapPin size={14} strokeWidth={2} className="flex-none text-content-muted" aria-hidden="true" />
+      <div className="flex min-w-0 flex-1 flex-col">
+        <span className="flex flex-wrap items-center gap-1.5 font-semibold text-content" style={fs(12.5, 'body')}>
+          {t('tours.import.pointsAsPlaces')}
+          <SoftPill>{t('tours.import.leftOutPoints', { count })}</SoftPill>
+        </span>
+        <span id={hintId} className="text-content-muted" style={fs(11.5, 'body')}>
+          {t('tours.import.pointsAsPlacesHint')}
+        </span>
+      </div>
+      <ToggleSwitch on={on} onToggle={onToggle} label={t('tours.import.pointsAsPlaces')} describedBy={hintId} />
+    </div>
+  );
+}
+
+/**
+ * IO-02: what a file held besides its lines and does not take in, named
+ * before the import so nothing goes missing unseen: a dashed card with each
+ * kind as a counted badge, and why in the info dot. Its points only while
+ * they are not added as places.
+ */
+function PreviewLeftOut({ leftOut }: { leftOut: TourFileLeftOut }) {
+  const { t } = useTranslation();
+  if (leftOut.points + leftOut.areas + leftOut.other === 0) return null;
   const kinds = [
     { key: 'points', count: leftOut.points, icon: MapPin, labelKey: 'tours.import.leftOutPoints' },
     { key: 'areas', count: leftOut.areas, icon: Shapes, labelKey: 'tours.import.leftOutAreas' },
@@ -183,6 +212,9 @@ function PreviewDialog({
   const labelId = useId();
   const tileUrl = useTileUrl(OFM_POSITRON);
   const choices = useGpxImportChoices(initial);
+  const leftOut = useMemo(() => leftOutOf(files), [files]);
+  const [withPoints, setWithPoints] = useState(true);
+  const addsPoints = withPoints && leftOut.points > 0;
 
   const tracks = files.flatMap((file) => file.tracks.map((track) => ({ file: file.file, track })));
   const fresh = tracks.filter(({ track }) => !track.duplicate).length;
@@ -194,7 +226,8 @@ function PreviewDialog({
     [files]
   );
   const focusPoints = useMemo(() => lines.flat(), [lines]);
-  const canImport = fresh > 0 && importMissing(choices.chosen) === null && !gpxImport.importing;
+  const canImport =
+    (fresh > 0 || addsPoints) && (fresh === 0 || importMissing(choices.chosen) === null) && !gpxImport.importing;
   const fields = importFields(choices.chosen);
   const title = files.length === 1 ? files[0].file.name : t('tours.import.previewFiles', { count: files.length });
 
@@ -231,9 +264,9 @@ function PreviewDialog({
             data-testid="gpx-preview-import"
             icon={<FileDown size={14} strokeWidth={2} />}
             disabled={!canImport}
-            onClick={() => void gpxImport.confirm(fields)}
+            onClick={() => void gpxImport.confirm(fields, { points: addsPoints })}
           >
-            {t('tours.import.previewConfirm', { count: fresh })}
+            {fresh > 0 ? t('tours.import.previewConfirm', { count: fresh }) : t('tours.import.pointsConfirm')}
           </DialogButton>
         </DialogFooter>
       }
@@ -258,8 +291,11 @@ function PreviewDialog({
             <PreviewTrackRow key={`${file.name}-${index}`} name={file.name} number={index + 1} track={track} />
           ))}
         </ul>
-        <PreviewLeftOut files={files} />
-        {fresh === 0 && (
+        {leftOut.points > 0 && (
+          <PreviewPoints count={leftOut.points} on={withPoints} onToggle={() => setWithPoints((on) => !on)} />
+        )}
+        <PreviewLeftOut leftOut={withPoints ? { ...leftOut, points: 0 } : leftOut} />
+        {fresh === 0 && !addsPoints && (
           <div className="mt-2">
             <TourNotice tone="warning" icon={<AlertTriangle size={13} strokeWidth={2} />}>
               {t('tours.import.previewAllDuplicates')}

client/src/components/Tours/useTourGpxImport.test.ts

@@ -5,7 +5,9 @@ import { act, renderHook } from '@testing-library/react';
 import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
 
 import { resetAllStores } from '../../../tests/helpers/store';
+import { placesApi } from '../../api/client';
 import { tourRepo } from '../../repo/tourRepo';
+import { useTripStore } from '../../store/tripStore';
 import { useTourGpxImport } from './useTourGpxImport';
 
 // The toasts are caught at useToast, so no window global has to be set for them.
@@ -67,3 +69,66 @@ describe('useTourGpxImport without a line in the file', () => {
     expect(addToast).toHaveBeenCalledWith('tours.import.unreadableIn', 'error', undefined);
   });
 });
+
+describe('useTourGpxImport with points beside the lines (#2969)', () => {
+  const route = new File(['<gpx/>'], 'sygic.gpx');
+  const pins = new File(['<kml/>'], 'pins.kml');
+  const line = {
+    name: 'Route',
+    geometry: [[47, 11] as [number, number], [47.01, 11] as [number, number]],
+    point_count: 2,
+    distance_km: 1,
+    elevation_gain_m: null,
+    elevation_loss_m: null,
+    duplicate: false,
+  };
+
+  beforeEach(() => {
+    useTripStore.setState({ refreshPlaces: vi.fn(async () => {}) } as never);
+    vi.spyOn(tourRepo, 'importGpx').mockResolvedValue({ tours: [], skipped: 0, caution: false } as never);
+  });
+
+  const previewed = async (preview: (picked: File) => object) => {
+    vi.spyOn(tourRepo, 'previewGpx').mockImplementation(async (_tripId, picked) => preview(picked as File) as never);
+    const hook = renderHook(() => useTourGpxImport({ tripId: 2, canEdit: true }));
+    await act(() => hook.result.current.importFiles([route, pins]));
+    return hook;
+  };
+
+  it('adds the points of each file as places through the places import, waypoints only', async () => {
+    const importGpx = vi.spyOn(placesApi, 'importGpx').mockResolvedValue({ count: 3 });
+    const importMap = vi.spyOn(placesApi, 'importMapFile').mockResolvedValue({ count: 2 });
+    const { result } = await previewed((picked) => ({
+      tracks: [line],
+      left_out: { points: picked === route ? 3 : 2, areas: 0, other: 0 },
+    }));
+    await act(() => result.current.confirm({ tour_type: 'hike' }, { points: true }));
+    expect(importGpx).toHaveBeenCalledWith(2, route, { waypoints: true, routes: false, tracks: false });
+    expect(importMap).toHaveBeenCalledWith(2, pins, { points: true, paths: false });
+    expect(tourRepo.importGpx).toHaveBeenCalledTimes(2);
+    expect(addToast).toHaveBeenCalledWith('tours.import.pointsImported', 'success', undefined);
+  });
+
+  it('leaves the points out when they are not asked for, and skips the Tours import for a file of known lines', async () => {
+    const importGpx = vi.spyOn(placesApi, 'importGpx').mockResolvedValue({ count: 3 });
+    const { result } = await previewed(() => ({
+      tracks: [{ ...line, duplicate: true }],
+      left_out: { points: 3, areas: 0, other: 0 },
+    }));
+    await act(() => result.current.confirm({ tour_type: 'hike' }));
+    expect(importGpx).not.toHaveBeenCalled();
+    expect(tourRepo.importGpx).not.toHaveBeenCalled();
+  });
+
+  it('names a file whose points could not be added, and still takes in its tour', async () => {
+    vi.spyOn(placesApi, 'importGpx').mockRejectedValue(new Error('offline'));
+    vi.spyOn(placesApi, 'importMapFile').mockResolvedValue({ count: 0 });
+    const { result } = await previewed((picked) => ({
+      tracks: [line],
+      ...(picked === route ? { left_out: { points: 1, areas: 0, other: 0 } } : {}),
+    }));
+    await act(() => result.current.confirm({ tour_type: 'hike' }, { points: true }));
+    expect(addToast).toHaveBeenCalledWith('tours.import.pointsFailedIn', 'error', undefined);
+    expect(tourRepo.importGpx).toHaveBeenCalledTimes(2);
+  });
+});

client/src/components/Tours/useTourGpxImport.ts

@@ -1,5 +1,6 @@
 import type { TourFileLeftOut, TourGpxImportFields, TourGpxPreviewTrack, TourListItem } from '@trek/shared';
 import { useCallback, useState } from 'react';
+import { placesApi } from '../../api/client';
 import { useTranslation } from '../../i18n';
 import { tourRepo } from '../../repo/tourRepo';
 import { useTripStore } from '../../store/tripStore';
@@ -40,10 +41,25 @@ function fileErrorMessage(error: unknown, name: string, t: Translate): string {
 export interface GpxPreviewFile {
   file: File;
   tracks: TourGpxPreviewTrack[];
-  /** What a KML or KMZ file held besides its lines, which no tour is made of; absent for GPX. */
+  /** What the file held besides its lines, which no tour is made of: GPX waypoints, KML pins, areas and the rest. */
   leftOut?: TourFileLeftOut;
 }
 
+/** The points of one file added as places of the trip, through the places import that has always taken them. */
+async function importPoints(tripId: number, file: File): Promise<number> {
+  const isGpx = file.name.toLowerCase().endsWith('.gpx');
+  const result = isGpx
+    ? await placesApi.importGpx(tripId, file, { waypoints: true, routes: false, tracks: false })
+    : await placesApi.importMapFile(tripId, file, { points: true, paths: false });
+  return result.count ?? 0;
+}
+
+/** What to do with the previewed files besides their lines. */
+export interface TourImportExtras {
+  /** Add each file's points (GPX waypoints, KML pins) as places of the trip (#2969). */
+  points?: boolean;
+}
+
 interface TourGpxImportOptions {
   tripId: number;
   canEdit: boolean;
@@ -115,26 +131,46 @@ export function useTourGpxImport({ tripId, canEdit, onToursChanged, onImported,
     [canEdit, fileError, importing, onNoTrack, preview, t, toast, tripId]
   );
 
-  /** The previewed files imported as the chosen type, summed up in one set of toasts. */
+  /**
+   * The previewed files imported as the chosen type, summed up in one set of
+   * toasts. With `points`, what each file holds as single points comes in as
+   * places as well, so a file with a route and its stops loses neither (#2969).
+   * A file whose every line the trip already holds is not sent to the Tours
+   * import at all, only for its points.
+   */
   const confirm = useCallback(
-    async (fields: TourGpxImportFields) => {
+    async (fields: TourGpxImportFields, extras: TourImportExtras = {}) => {
       if (!canEdit || !preview || importing) return;
       setImporting(true);
       const imported: TourListItem[] = [];
       let skipped = 0;
       let caution = false;
+      let places = 0;
       try {
-        for (const { file } of preview) {
-          try {
-            const result = await tourRepo.importGpx(tripId, file, fields);
-            imported.push(...result.tours);
-            skipped += result.skipped;
-            caution ||= result.caution;
-          } catch (error: unknown) {
-            fileError(error, file);
+        for (const { file, tracks, leftOut } of preview) {
+          if (tracks.some((track) => !track.duplicate)) {
+            try {
+              const result = await tourRepo.importGpx(tripId, file, fields);
+              imported.push(...result.tours);
+              skipped += result.skipped;
+              caution ||= result.caution;
+            } catch (error: unknown) {
+              fileError(error, file);
+            }
+          }
+          if (extras.points && (leftOut?.points ?? 0) > 0) {
+            try {
+              places += await importPoints(tripId, file);
+            } catch {
+              toast.error(t('tours.import.pointsFailedIn', { name: file.name }));
+            }
           }
         }
         setPreview(null);
+        if (places > 0) {
+          if (imported.length === 0) await refreshPlaces(tripId);
+          toast.success(t('tours.import.pointsImported', { count: places }));
+        }
         if (imported.length > 0) {
           // The server's place:created skips this socket, so the places are read
           // again here for the new ones and their lines. Only the places: a whole