fix(roadtrip): re-pin a via to the leg it was dragged onto

Dragging a via somewhere else on the map changed where the point IS and never where it
sits in the chain. The anchor that decides which two stops it is threaded between was
worked out once, when the point was dropped, and the drag sent only coordinates —
`roadtripViaUpdateRequestSchema` carried a lat and a lng and nothing else.

So a via pulled past the stop it used to precede kept claiming the earlier leg. The
router then drove out to it and turned back before carrying on, which on screen is not a
subtly wrong route: it is a spur that reads as the drag having done nothing at all, with
the original road still sitting there unchanged.

The anchor is not a property of the point but of where the point sits along the drive, so
it is recomputed on a drag the same way it is on a drop. The projection that did that was
inline in addRoadtripVia; it moves to a shared `anchorFor` and both callers use it.

A drag differs from a drop in two ways, both deliberate. It measures against its OWN day:
letting the nearest day win, which is right for a click on open map, would hand the via to
a neighbouring day whose road happens to pass closer and make it disappear from the day it
was dragged in. And it has no 2 km guard: pulling a via well off the current road is
exactly what the gesture is for, and the anchor only says which leg gets bent.

`after_order_index` is optional on the way in, so a drag inside one leg and an older
client still send just the coordinates, and the stored pin is then left alone rather
than cleared.
This commit is contained in:
Maurice
2026-09-09 18:25:23 +02:00
committed by Maurice
parent 77162aa8d5
commit 59b7c35e87
8 changed files with 158 additions and 15 deletions
@@ -66,7 +66,7 @@ export interface RoadtripVias {
/** Absent leaves the day's track alone, null clears it, an object records a new one. */
track?: { place_id: number; stray_km?: number | null } | null,
) => Promise<void>
move: (dayId: number, id: number, lat: number, lng: number) => Promise<void>
move: (dayId: number, id: number, lat: number, lng: number, afterOrderIndex?: number) => Promise<void>
remove: (dayId: number, id: number) => Promise<void>
/**
* Correct a day's anchors after its stops changed shape.
@@ -171,9 +171,16 @@ export function useRoadtripVias(tripId: number | string | null, active: boolean)
await reload()
}, [tripId, reload])
const move = useCallback(async (dayId: number, id: number, lat: number, lng: number) => {
const move = useCallback(async (dayId: number, id: number, lat: number, lng: number, afterOrderIndex?: number) => {
if (!tripId) return
await roadtripApi.moveVia(tripId, dayId, id, { lat, lng })
// The anchor rides along when the caller worked out a new one: a via dragged past the
// stop it used to sit before belongs to the next leg now, and saying only where it is
// leaves it claiming the old one.
await roadtripApi.moveVia(tripId, dayId, id, {
lat,
lng,
...(afterOrderIndex === undefined ? {} : { after_order_index: afterOrderIndex }),
})
await reload()
}, [tripId, reload])
@@ -1060,13 +1060,65 @@ describe('useTripPlanner road trip: shaping the drive on the map', () => {
const { result } = await renderRoadtrip()
await act(async () => { await result.current.moveRoadtripVia(5, 3, 53.2, 11.7) })
expect(rt.vias.move).toHaveBeenCalledWith(5, 3, 53.2, 11.7)
expect(rt.vias.move).toHaveBeenCalledWith(5, 3, 53.2, 11.7, 0)
rt.vias.move.mockRejectedValue(new Error('stale'))
await act(async () => { await result.current.moveRoadtripVia(5, 3, 53.2, 11.7) })
expect(toasts.some(t => t.type === 'error' && t.message === 'stale')).toBe(true)
})
it('FE-TP-ROAD-053: a via dragged past a stop is re-pinned to the leg it landed on', async () => {
// The bug this exists for. A drag used to send only the new coordinates, so a via
// pulled beyond the stop it used to precede kept claiming the earlier leg: the route
// ran out to the point and back before carrying on, which reads as the drag doing
// nothing at all. Three stops so there are two legs to land between.
seedTrip({ days: [buildDay({ id: 5, day_number: 1 })] })
rt.corridor.day = { dayId: 5, dayNumber: 1 }
rt.routes.days = [{
dayId: 5,
stops: [{ lat: 53.55, lng: 9.99 }, { lat: 53.85, lng: 11.45 }, { lat: 52.52, lng: 13.4 }],
geometry: [[53.55, 9.99], [53.87, 10.7], [53.85, 11.45], [53.87, 11.53], [52.52, 13.4]],
}]
const { result } = await renderRoadtrip()
// Dropped on the first leg, then dragged past the middle stop onto the second.
await act(async () => { await result.current.addRoadtripVia(53.87, 10.7) })
expect(rt.vias.add).toHaveBeenCalledWith(5, 0, 53.87, 10.7)
await act(async () => { await result.current.moveRoadtripVia(5, 9, 53.87, 11.53) })
expect(rt.vias.move).toHaveBeenCalledWith(5, 9, 53.87, 11.53, 1)
})
it('FE-TP-ROAD-054: a drag is measured against its own day, however close another one runs', async () => {
// Placing a via lets the nearest day win, which is right for a click on the map.
// A drag is not that: the via already belongs to a day, and handing it to a
// neighbouring day whose road happens to pass closer would make it vanish from the
// one it was dragged in.
twoRoutedDays()
const { result } = await renderRoadtrip()
// Right on day 6's line, but dragged within day 5.
await act(async () => { await result.current.moveRoadtripVia(5, 3, 47.9, 12.3) })
expect(rt.vias.move).toHaveBeenCalledWith(5, 3, 47.9, 12.3, expect.any(Number))
// Day 6 never saw it.
expect(rt.vias.move).not.toHaveBeenCalledWith(6, expect.anything(), expect.anything(), expect.anything(), expect.anything())
})
it('FE-TP-ROAD-055: a drag far off the road still moves, without an anchor to offer', async () => {
// No distance guard on a drag, unlike on a click: pulling a via well away from the
// current road is the whole point of the gesture. A day with no line to measure
// against simply sends no anchor, and the existing pin stays as it was.
seedTrip({ days: [buildDay({ id: 5, day_number: 1 })] })
rt.corridor.day = { dayId: 5, dayNumber: 1 }
rt.routes.days = [{ dayId: 5, stops: [], geometry: [] }]
const { result } = await renderRoadtrip()
await act(async () => { await result.current.moveRoadtripVia(5, 3, 41.9, 12.5) })
expect(rt.vias.move).toHaveBeenCalledWith(5, 3, 41.9, 12.5, undefined)
})
it('FE-TP-ROAD-052: removing a via lets the drive take the direct road again', async () => {
twoRoutedDays()
const { result } = await renderRoadtrip()
+28 -5
View File
@@ -1148,10 +1148,19 @@ export function useTripPlanner() {
* third stop" means the same thing everywhere. The day is the one whose line was hit,
* found by trying each day's geometry and keeping the closest.
*/
const addRoadtripVia = useCallback(async (lat: number, lng: number) => {
if (!can('day_edit', trip)) return
/**
* Which stop of which day a point belongs behind, measured along the drive.
*
* Shared by placing a via and by dragging one, because it is the same question both
* times and the answer has to be recomputed both times. A drag used to send only the
* new coordinates, so a via pulled past the stop it used to precede kept claiming the
* earlier leg: the route then ran out to the point and back before carrying on, which
* looks exactly like a drag that did nothing.
*/
const anchorFor = useCallback((lat: number, lng: number, onlyDayId?: number) => {
let best: { dayId: number; afterIndex: number; offRouteKm: number } | null = null
for (const day of roadtripRoutes.days) {
if (onlyDayId !== undefined && day.dayId !== onlyDayId) continue
if (day.geometry.length < 2) continue
const spine = day.geometry.map(([la, ln]) => ({ lat: la, lng: ln }))
const hit = projectOntoRoute({ lat, lng }, spine)
@@ -1162,6 +1171,12 @@ export function useTripPlanner() {
const afterIndex = Math.max(0, insertIndexForAlong(stopsAlong, hit.alongKm) - 1)
best = { dayId: day.dayId, afterIndex, offRouteKm: hit.offRouteKm }
}
return best
}, [roadtripRoutes.days])
const addRoadtripVia = useCallback(async (lat: number, lng: number) => {
if (!can('day_edit', trip)) return
const best = anchorFor(lat, lng)
// A click that landed on some other line is not a via anywhere.
if (!best || best.offRouteKm > 2) return
try {
@@ -1169,17 +1184,25 @@ export function useTripPlanner() {
} catch (err: unknown) {
toast.error(err instanceof Error ? err.message : t('common.unknownError'))
}
}, [roadtripRoutes.days, roadtripVias, can, trip, toast, t])
}, [anchorFor, roadtripVias, can, trip, toast, t])
/** Dragging a via redraws the route through its new position. */
const moveRoadtripVia = useCallback(async (dayId: number, id: number, lat: number, lng: number) => {
if (!can('day_edit', trip)) return
// Measured against this day only. A drag is a drag WITHIN a day: letting the nearest
// day win, the way placing one does, would hand the via to a neighbouring day whose
// road happens to pass closer, and it would vanish from the day it was dragged in.
//
// No distance guard either. Dragging a via well off the current road is the whole
// point of dragging it, and refusing that would be refusing the gesture; the anchor
// just says which leg gets bent, and the router answers the rest.
const anchor = anchorFor(lat, lng, dayId)
try {
await roadtripVias.move(dayId, id, lat, lng)
await roadtripVias.move(dayId, id, lat, lng, anchor?.afterIndex)
} catch (err: unknown) {
toast.error(err instanceof Error ? err.message : t('common.unknownError'))
}
}, [roadtripVias, can, trip, toast, t])
}, [anchorFor, roadtripVias, can, trip, toast, t])
/** Removing a via lets the drive take the direct road again. */
const removeRoadtripVia = useCallback(async (dayId: number, id: number) => {
@@ -108,7 +108,7 @@ export class RoadtripController {
@Body() body: RoadtripViaUpdateDto,
): { via: RoadtripVia } {
this.requireDay(dayId, tripId);
const via = this.roadtrip.move(id, dayId, body.lat, body.lng);
const via = this.roadtrip.move(id, dayId, body.lat, body.lng, body.after_order_index);
if (!via) throw new HttpException({ error: 'Via not found' }, 404);
return { via };
}
+22 -2
View File
@@ -154,14 +154,34 @@ export class RoadtripService {
}
/** Moving a via is the whole edit; where it sits in the chain does not change. */
move(id: string | number, dayId: string | number, lat: number, lng: number): RoadtripVia | null {
move(
id: string | number,
dayId: string | number,
lat: number,
lng: number,
afterOrderIndex?: number,
): RoadtripVia | null {
const existing = this.db.get<{ id: number }>(
'SELECT id FROM roadtrip_vias WHERE id = ? AND day_id = ?',
id,
dayId,
);
if (!existing) return null;
this.db.run('UPDATE roadtrip_vias SET lat = ?, lng = ? WHERE id = ?', lat, lng, id);
// The anchor moves with the point when the caller worked out a new one. It is not a
// property of the via but of where the via sits along the drive, so dragging one past
// a stop changes which leg it belongs to — and leaving it behind is what made the
// route run out to the point and back instead of bending through it.
if (afterOrderIndex === undefined) {
this.db.run('UPDATE roadtrip_vias SET lat = ?, lng = ? WHERE id = ?', lat, lng, id);
} else {
this.db.run(
'UPDATE roadtrip_vias SET lat = ?, lng = ?, after_order_index = ? WHERE id = ?',
lat,
lng,
afterOrderIndex,
id,
);
}
return this.byId(Number(id));
}
@@ -160,7 +160,15 @@ describe('RoadtripController — writes', () => {
it('ROADTRIP-CTL-012: moving a via reports where it went', () => {
const s = svc();
expect(new RoadtripController(s).update('7', '4', '5', { lat: 54, lng: 11 })).toEqual({ via: VIA });
expect(s.move).toHaveBeenCalledWith('5', '4', 54, 11);
// Undefined anchor, explicitly: a drag that stayed between the same two stops sends
// no new one, and the service must leave the existing pin alone rather than clear it.
expect(s.move).toHaveBeenCalledWith('5', '4', 54, 11, undefined);
});
it('ROADTRIP-CTL-015: a new anchor is passed on, so a via dragged past a stop is re-pinned', () => {
const s = svc();
new RoadtripController(s).update('7', '4', '5', { lat: 54, lng: 11, after_order_index: 2 });
expect(s.move).toHaveBeenCalledWith('5', '4', 54, 11, 2);
});
it('ROADTRIP-CTL-013: a via that is not on this day is a 404, not a silent no-op', () => {
@@ -82,7 +82,10 @@ describe('RoadtripService', () => {
expect(service.listForTrip(7).map(v => v.day_id)).toEqual([1, 2]);
});
it('ROADTRIP-SVC-005: moving a via changes where it is, not where it sits in the chain', () => {
it('ROADTRIP-SVC-005: a move with no new anchor leaves the chain alone', () => {
// The drag stayed between the same two stops, so there is nothing to re-pin. The
// sequence is never touched by a move either way: it orders the vias that share one
// anchor, and dragging one does not reorder its neighbours.
const via = service.create(1, { after_order_index: 1, lat: 53, lng: 10 });
const moved = service.move(via.id, 1, 52.5, 11.5);
@@ -90,6 +93,26 @@ describe('RoadtripService', () => {
expect(moved).toMatchObject({ lat: 52.5, lng: 11.5, after_order_index: 1, sequence: via.sequence });
});
it('ROADTRIP-SVC-007: a via dragged past a stop is re-pinned to the leg it landed on', () => {
// The bug this exists for: a drag used to carry only the coordinates, so a via pulled
// beyond the stop it used to precede kept claiming the earlier leg. The route then ran
// out to the point and back before carrying on, which reads as the drag doing nothing.
const via = service.create(1, { after_order_index: 0, lat: 53.87, lng: 10.7 });
const moved = service.move(via.id, 1, 53.87, 11.53, 1);
expect(moved).toMatchObject({ lat: 53.87, lng: 11.53, after_order_index: 1 });
});
it('ROADTRIP-SVC-008: a re-pin through the wrong day changes nothing at all', () => {
// The day check has to guard the anchor as well as the coordinates, or an id from
// another day could be renumbered through a day the caller does happen to reach.
const via = service.create(1, { after_order_index: 0, lat: 53, lng: 10 });
expect(service.move(via.id, 2, 51, 12, 3)).toBeNull();
expect(service.listForDay(1)[0]).toMatchObject({ lat: 53, lng: 10, after_order_index: 0 });
});
it('ROADTRIP-SVC-006: a via cannot be moved or removed through the wrong day', () => {
const via = service.create(1, { after_order_index: 0, lat: 53, lng: 10 });
+11 -1
View File
@@ -43,10 +43,20 @@ export const roadtripViaCreateRequestSchema = z.object({
});
export type RoadtripViaCreateRequest = z.infer<typeof roadtripViaCreateRequestSchema>;
/** Moving a via is the whole edit — dragging it somewhere else on the map. */
/**
* Moving a via is the whole edit — dragging it somewhere else on the map.
*
* `after_order_index` travels with the drag because the anchor is not a property of the
* point, it is a property of where the point sits along the drive. Without it a via
* dragged past the stop it used to precede keeps claiming the earlier leg, and the route
* runs out to it and back again before carrying on — which reads as the drag having done
* nothing at all. Optional so an older client, and a drag that did not move the via
* across a stop, still send just the coordinates.
*/
export const roadtripViaUpdateRequestSchema = z.object({
lat: latSchema,
lng: lngSchema,
after_order_index: z.number().int().min(0).optional(),
});
export type RoadtripViaUpdateRequest = z.infer<typeof roadtripViaUpdateRequestSchema>;