From c2dd66986c1b1a658b91e81020edad6f9a09d378 Mon Sep 17 00:00:00 2001 From: Maurice Date: Mon, 12 Oct 2026 11:43:50 +0200 Subject: [PATCH] fix(maps): the day route leaves an imported gpx track to the track and carries on from its end A GPX or KML import stores the track's first point as the place's position, so the day route treated it as a single stop there and routed from the track start to the next stop, a second road running alongside the track. Any assigned place with a usable track now gets the gap a Tour already had: the route runs up to the track start and continues from the track end, on the desktop map and day list, the mobile timeline, the trip overview and the PDF map, which now draws the track as a line. A place without a usable track stays a plain stop and never shows as an invalid Tour. Refs #2600 --- .../src/components/Map/dayRoutePlan.test.ts | 69 +++++++++++++++++++ .../src/components/Map/dayTourProjection.ts | 14 +++- client/src/components/PDF/TripPDF.map.test.ts | 15 ++++ .../Planner/DayPlanSidebar.test.tsx | 2 +- .../src/hooks/useRouteCalculation.b5.test.ts | 3 +- .../trip/plan/planTimelineModel.test.ts | 20 +++++- 6 files changed, 119 insertions(+), 4 deletions(-) diff --git a/client/src/components/Map/dayRoutePlan.test.ts b/client/src/components/Map/dayRoutePlan.test.ts index 406a72adb..d4a868d1a 100644 --- a/client/src/components/Map/dayRoutePlan.test.ts +++ b/client/src/components/Map/dayRoutePlan.test.ts @@ -436,3 +436,72 @@ describe('Tour day itinerary segments', () => { expect(projectDayItinerary(list, true)[1]).toMatchObject({ geometry: [points.S, points.E] }) }) }) + +// #2600: an imported GPX/KML place sits at its track's first point, so routing it as a +// plain stop drew a second road from the track start to the next stop, alongside the +// track itself. Any assigned place with a track now leaves that stretch to the track. +describe('Imported track places', () => { + const points = { + A: [35.2015, 24.1395], S: [35.201, 24.118], M: [35.199, 24.1], E: [35.1965, 24.079], C: [35.19, 24.07], + } as const + type Name = keyof typeof points + const stop = (name: Name, order: number, placeId: number) => + buildAssignment({ id: 900 + order, day_id: 1, order_index: order, + place: buildPlace({ id: placeId, lat: points[name][0], lng: points[name][1] }) }) + const trackPlace = (id: number, geometry: string | null, at: readonly [number, number] = points.S) => + buildPlace({ id, lat: at[0], lng: at[1], route_geometry: geometry }) + const track = JSON.stringify([points.S, points.M, points.E]) + const runsOf = (list: AssignmentsMap['1'], places: ReturnType[], toursEnabled = false) => + buildDayRouteRuns(1, { ...inputs({ assignments: { '1': list } }), toursEnabled, places }) + .map(run => run.map(point => [point.lat, point.lng])) + + it('routes up to the track start and carries on from its end, without the Tours addon', () => { + const list = [stop('A', 0, 1), stop('S', 1, 2), stop('C', 2, 3)] + const places = [buildPlace({ id: 1 }), trackPlace(2, track), buildPlace({ id: 3 })] + expect(runsOf(list, places)).toEqual([[points.A, points.S], [points.E, points.C]]) + expect(runsOf(list, places, true)).toEqual([[points.A, points.S], [points.E, points.C]]) + expect(projectDayItinerary(list, false, places)[1]).toMatchObject({ kind: 'tour', valid: true, geometry: [points.S, points.M, points.E] }) + }) + + it('routes to the track start even when the marker was moved off it', () => { + const list = [stop('A', 0, 1), stop('C', 1, 2), stop('C', 2, 3)] + const places = [buildPlace({ id: 1 }), trackPlace(2, track, points.C), buildPlace({ id: 3 })] + expect(runsOf(list, places)).toEqual([[points.A, points.S], [points.E, points.C]]) + }) + + it('takes the track as recorded, so one walked the other way still starts at its first point', () => { + const list = [stop('A', 0, 1), stop('E', 1, 2), stop('C', 2, 3)] + const places = [buildPlace({ id: 1 }), trackPlace(2, track, points.E), buildPlace({ id: 3 })] + expect(runsOf(list, places)).toEqual([[points.A, points.S], [points.E, points.C]]) + }) + + it('keeps a place without a usable track as a plain stop, never as an invalid tour', () => { + const list = [stop('A', 0, 1), stop('S', 1, 2), stop('C', 2, 3)] + for (const geometry of [null, '{bad', JSON.stringify([points.S])]) { + const places = [buildPlace({ id: 1 }), trackPlace(2, geometry), buildPlace({ id: 3 })] + expect(runsOf(list, places)).toEqual([[points.A, points.S, points.C]]) + expect(projectDayItinerary(list, true, places)[1]).toMatchObject({ kind: 'stop' }) + } + // Places not loaded yet: the assignment alone carries no track, so nothing is guessed. + expect(runsOf(list, [])).toEqual([[points.A, points.S, points.C]]) + }) + + it('starts the evening drive back to the hotel at the end of the track', () => { + const hotel = [35.5, 24.0] + const days = [1, 2, 3].map(id => buildDay({ id, day_number: id })) + const accommodations = [{ id: 1, trip_id: 1, start_day_id: 1, end_day_id: 3, + place_lat: hotel[0], place_lng: hotel[1] }] as Accommodation[] + const list = [stop('A', 0, 1), stop('S', 1, 2)].map(assignment => ({ ...assignment, day_id: 2 })) + const places = [buildPlace({ id: 1 }), trackPlace(2, track)] + expect(buildDayRouteRuns(2, { ...inputs({ days, assignments: { '2': list }, accommodations, + optimizeFromAccommodation: true }), places }).map(run => run.map(point => [point.lat, point.lng]))) + .toEqual([[hotel, points.A], [points.A, points.S], [points.E, hotel]]) + }) + + it('draws the track as a line in the trip overview and the PDF map', () => { + const list = [stop('A', 0, 1), stop('S', 1, 2), stop('C', 2, 3)] + const places = [buildPlace({ id: 1 }), trackPlace(2, track), buildPlace({ id: 3 })] + const plan = planTripRoute({ ...inputs({ assignments: { '1': list } }), places }, 'driving') + expect(plan[0].tourLines).toEqual([[points.S, points.M, points.E]]) + }) +}) diff --git a/client/src/components/Map/dayTourProjection.ts b/client/src/components/Map/dayTourProjection.ts index 4866fa3ab..fac690f00 100644 --- a/client/src/components/Map/dayTourProjection.ts +++ b/client/src/components/Map/dayTourProjection.ts @@ -22,8 +22,20 @@ export function projectDayItinerary( const placesById = new Map(places.map(place => [place.id, place])) return assignments.map(assignment => { const placeId = assignment.place?.id - if (!toursEnabled || placeId == null || assignment.tour_place_id !== placeId) return { kind: 'stop', assignment } + if (placeId == null) return { kind: 'stop', assignment } const currentPlace = placesById.get(placeId) + if (!toursEnabled || assignment.tour_place_id !== placeId) { + // Any other place with a track (GPX/KML import) sits at the track's first point, + // so routing it as a plain stop draws a second road alongside the track (#2600). + // It gets the same gap as a Tour; without a usable track it stays a stop and can + // never turn into an invalid Tour. + const track = analyzeRouteGeometry(currentPlace?.route_geometry)?.routeCoordinates ?? [] + if (track.length < 2) return { kind: 'stop', assignment } + const start = { lat: track[0][0], lng: track[0][1] } + const end = { lat: track[track.length - 1][0], lng: track[track.length - 1][1] } + return { kind: 'tour', assignment, placeId, start, end, geometry: track, + loop: start.lat === end.lat && start.lng === end.lng, valid: true } as const + } const analysis = analyzeRouteGeometry(currentPlace ? currentPlace.route_geometry : assignment.tour_route_geometry) const geometry = analysis?.routeCoordinates ?? [] const start = geometry.length >= 2 diff --git a/client/src/components/PDF/TripPDF.map.test.ts b/client/src/components/PDF/TripPDF.map.test.ts index 17e8f144d..3f7639f5f 100644 --- a/client/src/components/PDF/TripPDF.map.test.ts +++ b/client/src/components/PDF/TripPDF.map.test.ts @@ -161,6 +161,21 @@ describe('trip route map in the PDF', () => { expect(srcdoc()).not.toContain('stroke="#14805e"') }) + it('leaves an imported track to its line instead of routing alongside it (#2600)', async () => { + const a = assign(41, 1, 0, 48.1, 11.1) + const trackPlace = assign(42, 1, 1, 48.2, 11.2) + const c = assign(43, 1, 2, 48.4, 11.4) + const places = [{ ...place(42, 48.2, 11.2), route_geometry: JSON.stringify([[48.2, 11.2], [48.3, 11.3]]) }] as never + await downloadTripPDF({ ...args, days: [args.days[0]], places, assignments: { '1': [a, trackPlace, c] } }) + + expect(vi.mocked(routeTrip).mock.calls[0][0].toursEnabled).toBe(false) + expect(vi.mocked(calculateRouteWithLegs).mock.calls.map(([points]) => + points.map(point => [point.lat, point.lng]))) + .toEqual([[[48.1, 11.1], [48.2, 11.2]], [[48.3, 11.3], [48.4, 11.4]]]) + expect(srcdoc()).toContain('stroke="#14805e"') + expect(srcdoc()).not.toContain('tours.dayRoute.endpointUnknown') + }) + it('prints one localized warning for an invalid Tour and does not invent its outgoing endpoint', async () => { useAddonStore.setState({ addons: [{ id: 'tours', enabled: true }] as never }) const a = assign(31, 1, 0, 48.1, 11.1) diff --git a/client/src/components/Planner/DayPlanSidebar.test.tsx b/client/src/components/Planner/DayPlanSidebar.test.tsx index 16d2d05fd..27178b1d0 100644 --- a/client/src/components/Planner/DayPlanSidebar.test.tsx +++ b/client/src/components/Planner/DayPlanSidebar.test.tsx @@ -2917,7 +2917,7 @@ describe('DayPlanSidebar', () => { it.each([ { enabled: true, excluded: false, pairs: [[48.85, 48.86], [48.88, 48.89]] }, { enabled: true, excluded: true, pairs: [[48.85, 48.89]] }, - { enabled: false, excluded: false, pairs: [[48.85, 48.86], [48.86, 48.89]] }, + { enabled: false, excluded: false, pairs: [[48.85, 48.86], [48.88, 48.89]] }, ])('B5 sidebar Tour connector parity enabled=$enabled excluded=$excluded', async ({ enabled, excluded, pairs }) => { useAddonStore.setState({ addons: [{ id: 'tours', name: 'Tours', icon: 'Route', type: 'feature', enabled }] }) const day = buildDay({ id: 10, date: '2025-06-01' }) diff --git a/client/src/hooks/useRouteCalculation.b5.test.ts b/client/src/hooks/useRouteCalculation.b5.test.ts index 541713fa5..891ab3ff5 100644 --- a/client/src/hooks/useRouteCalculation.b5.test.ts +++ b/client/src/hooks/useRouteCalculation.b5.test.ts @@ -45,7 +45,8 @@ describe('B5 useRouteCalculation', () => { it.each([ { enabled: true, excluded: false, expected: [[48.1, 48.2], [48.3, 48.4]] }, { enabled: true, excluded: true, expected: [[48.1, 48.4]] }, - { enabled: false, excluded: false, expected: [[48.1, 48.2, 48.4]] }, + // Off, the place still has its track, so the drive breaks there all the same (#2600). + { enabled: false, excluded: false, expected: [[48.1, 48.2], [48.3, 48.4]] }, ])('routes enabled=$enabled excluded=$excluded using the shared day topology', async ({ enabled, excluded, expected }) => { seed(enabled, excluded) const { result } = renderHook(() => useRouteCalculation(useTripStore(), 1, true, 'walking')) diff --git a/client/src/mobile/screens/trip/plan/planTimelineModel.test.ts b/client/src/mobile/screens/trip/plan/planTimelineModel.test.ts index 52d9ba45a..488fe6354 100644 --- a/client/src/mobile/screens/trip/plan/planTimelineModel.test.ts +++ b/client/src/mobile/screens/trip/plan/planTimelineModel.test.ts @@ -1,7 +1,7 @@ import { describe, expect, it } from 'vitest' import { buildPlanRows } from './planTimelineModel' import type { MergedItem } from '../../../../utils/dayMerge' -import type { Reservation, RouteSegment } from '../../../../types' +import type { Place, Reservation, RouteSegment } from '../../../../types' // FE-MOBILE-PLANROWS-001 to FE-MOBILE-PLANROWS-003 @@ -65,4 +65,22 @@ describe('buildPlanRows', () => { expect(rows.filter(row => row.kind === 'conn').map(row => row.seg)).toEqual(segments) expect(buildPlanRows({ merged, reservations: [], routeSegments: segments, dayId: 5 }).some(row => row.kind === 'conn' && row.seg.from[0] === end[0])).toBe(false) }) + + it('connects an imported track place at its track start and end without the Tours addon (#2600)', () => { + const start: [number, number] = [35.201, 24.118] + const end: [number, number] = [35.1965, 24.079] + const before: [number, number] = [35.2015, 24.1395] + const after: [number, number] = [35.19, 24.07] + const merged = [ + { type: 'place', sortKey: 0, data: { id: 11, place: { id: 1, lat: before[0], lng: before[1] } } }, + { type: 'place', sortKey: 1, data: { id: 12, tour_place_id: null, place: { id: 2, lat: start[0], lng: start[1] } } }, + { type: 'place', sortKey: 2, data: { id: 13, place: { id: 3, lat: after[0], lng: after[1] } } }, + ] as unknown as MergedItem[] + const places = [{ id: 2, lat: start[0], lng: start[1], route_geometry: JSON.stringify([start, [35.199, 24.1], end]) }] as unknown as Place[] + const segment = (from: [number, number], to: [number, number]) => ({ from, to }) as RouteSegment + const segments = [segment(before, start), segment(end, after)] + const rows = buildPlanRows({ merged, reservations: [], routeSegments: segments, dayId: 5, places }) + expect(rows.filter(row => row.kind === 'conn').map(row => row.seg)).toEqual(segments) + expect(rows.some(row => row.kind === 'place' && row.invalidTour)).toBe(false) + }) })