mirror of
https://github.com/mauriceboe/TREK.git
synced 2026-10-18 21:16:47 +00:00
fix(roadtrip): keep the shaping a traveller did, through every path that moves a stop
A via records which stop it follows by POSITION, so anything that renumbers a day's stops moves the ground under it. The road-trip rail corrects for that; almost nothing else did. reanchor() never renumbered `sequence`. It is allocated per leg and starts at 0 in each, and re-anchoring MERGES two legs whenever the stop between them goes away — so leg A's 0,1 ended up beside leg B's 0,1, and everything that draws the route orders by sequence. The drive ran A0, B0, A1, B1: forward, back, and forward again. Persisted, surviving a reload, and unrepairable except by deleting the vias. The old anchor is read before the updates and used as the primary sort key, so the vias of the earlier leg stay ahead of the ones that merged into it. Reordering the only two stops of a day deleted every via on it. A reorder is modelled as remove-then-insert, and the removal half refuses to treat a two-stop day as still having a drive — right when a stop really goes away, catastrophic when nothing was removed. Two stops is exactly the shape "follow this track" produces, up to nine vias on that one leg, gone to a single drag with no prompt and no undo. Removing a stop, reordering within a day, and assigning a place mid-day never re-anchored at all, though the helpers exist and three other paths use them. The detour then sat on a leg the traveller never chose, or on a leg that no longer existed and vanished — until a later stop brought it back bending a road nobody asked for. Reordering hands a whole new ordering rather than one move, so the anchors there follow the stop they were pinned behind instead of arithmetic on the index, which is correct for any permutation. Duplicating a trip dropped all of it: `stop_type` was missing from the place insert, and neither via nor day-track table was copied. The duplicate looked complete and quietly drove somewhere else. A failed read of the via list emptied it. That read runs after every write and feeds the routing key, so a 502, a dropped connection or a tab going offline re-routed the whole trip along the roads the traveller had steered away from, with nothing said and nothing retried — indistinguishable from having deleted them. Only a 404 or 403 empties now; anything else keeps the last known list and says it may be stale.
This commit is contained in:
@@ -204,7 +204,12 @@ describe('PlacesController (parity with the legacy /api/trips/:tripId/places rou
|
||||
expect(importGoogleDirections).toHaveBeenCalledWith('5', url, { enrich: false, userId: 1 });
|
||||
expect(importGoogleList).not.toHaveBeenCalled();
|
||||
});
|
||||
it('leaves a list link with the list importer', async () => {
|
||||
it('sends a short link to the list importer, which knows where it really goes', async () => {
|
||||
// A short link's path is `/<code>`, so nothing about the raw URL says
|
||||
// whether it is a route or a list. The dispatch cannot know before the
|
||||
// redirect is followed, and the list importer already follows it — so it
|
||||
// is the one that hands a route on. Deciding here would mean a second
|
||||
// network hop and a second copy of the SSRF handling.
|
||||
const importGoogleDirections = vi.fn();
|
||||
const importGoogleList = vi.fn().mockResolvedValue({ places: [], listName: 'L', skipped: 0 });
|
||||
const s = svc({ importGoogleDirections, importGoogleList, broadcast: vi.fn() } as Partial<PlacesService>);
|
||||
|
||||
@@ -202,6 +202,42 @@ describe('PlacesService.importGoogleDirections', () => {
|
||||
expect((result as { error: string }).error).toMatch(/Share button/);
|
||||
});
|
||||
|
||||
it('PLACES-DIR-012: the bulk geocoding yields to whoever is typing', async () => {
|
||||
// searchNominatim's own contract asks a bulk caller for the background
|
||||
// lane, naming this exact shape: up to thirty sequential lookups, each
|
||||
// taking the next slot on a 1.1 s process-wide throttle. On the interactive
|
||||
// lane every other member of the instance typing in the place search box
|
||||
// queues behind the whole import.
|
||||
const search = vi.fn(async () => [{ lat: 52.52, lng: 13.405 }]) as unknown as MapsService['searchNominatim'];
|
||||
await svc(search).importGoogleDirections(tripId, 'https://www.google.com/maps/dir/Berlin/Dresden');
|
||||
|
||||
expect(search).toHaveBeenCalled();
|
||||
for (const call of (search as unknown as { mock: { calls: unknown[][] } }).mock.calls) {
|
||||
expect(call[2]).toBe('background');
|
||||
}
|
||||
});
|
||||
|
||||
it('PLACES-DIR-011: a route shared as a short link is imported as a route', async () => {
|
||||
// The path of a short link is `/<code>`: nothing in it says route or list,
|
||||
// so the dispatch upstream cannot tell and sent every one of them to the
|
||||
// list importer, which answered "could not extract list ID" — the complaint
|
||||
// this whole import was written to remove. The Share sheet in the Google
|
||||
// Maps phone app produces exactly this shape, and the box beside it says a
|
||||
// directions link works.
|
||||
safeFetchFollow.mockResolvedValue({
|
||||
url: 'https://www.google.com/maps/dir/52.52,13.405/51.05,13.74',
|
||||
} as never);
|
||||
|
||||
const result = await svc(geocoder()).importGoogleList(tripId, 'https://maps.app.goo.gl/aBcDeF12345');
|
||||
|
||||
expect('error' in result).toBe(false);
|
||||
if ('error' in result) return;
|
||||
expect(result.places).toHaveLength(2);
|
||||
// Resolved once, not once per importer: the route is handed on with the URL
|
||||
// the redirect already produced.
|
||||
expect(safeFetchFollow).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it('PLACES-DIR-010: a route where only one stop can be placed is not half an import', async () => {
|
||||
const search = vi.fn(async () => []) as unknown as MapsService['searchNominatim'];
|
||||
const url = 'https://www.google.com/maps/dir/52.52,13.405/Nowhere/Nowhere+Else';
|
||||
|
||||
@@ -151,6 +151,47 @@ describe('RoadtripService', () => {
|
||||
expect(after.map(v => v.after_order_index)).toEqual([1]);
|
||||
});
|
||||
|
||||
it('ROADTRIP-SVC-016: merging two populated legs keeps the drive going one way', () => {
|
||||
// The case none of the four above reach: they all re-anchor onto legs that
|
||||
// are empty. `sequence` is allocated per leg and starts at 0 in each, so
|
||||
// merging two populated legs put leg A's 0,1 beside leg B's 0,1 — and
|
||||
// everything that draws the route orders by sequence, so the drive ran A0,
|
||||
// B0, A1, B1: forward, back, and forward again. Persisted, surviving a
|
||||
// reload, and unrepairable except by deleting the vias.
|
||||
const a0 = service.create(1, { after_order_index: 1, lat: 50.0, lng: 10 });
|
||||
const a1 = service.create(1, { after_order_index: 1, lat: 50.1, lng: 10 });
|
||||
const b0 = service.create(1, { after_order_index: 2, lat: 50.2, lng: 10 });
|
||||
const b1 = service.create(1, { after_order_index: 2, lat: 50.3, lng: 10 });
|
||||
expect([a0.sequence, a1.sequence, b0.sequence, b1.sequence]).toEqual([0, 1, 0, 1]);
|
||||
|
||||
// The stop between the two legs is removed, so leg 2 merges into leg 1 —
|
||||
// exactly what reanchorAfterRemove computes on the client.
|
||||
const after = service.reanchor(1, {
|
||||
vias: [{ id: b0.id, after_order_index: 1 }, { id: b1.id, after_order_index: 1 }],
|
||||
});
|
||||
|
||||
const onLeg = after.filter(v => v.after_order_index === 1).sort((x, y) => x.sequence - y.sequence);
|
||||
expect(onLeg.map(v => v.id)).toEqual([a0.id, a1.id, b0.id, b1.id]);
|
||||
// Strictly increasing across the whole leg, with no value used twice.
|
||||
expect(onLeg.map(v => v.sequence)).toEqual([0, 1, 2, 3]);
|
||||
// Which is the geographic order the day is actually driven in.
|
||||
expect(onLeg.map(v => v.lat)).toEqual([50.0, 50.1, 50.2, 50.3]);
|
||||
});
|
||||
|
||||
it('ROADTRIP-SVC-017: a leg nothing merged into keeps the order it had', () => {
|
||||
const first = service.create(1, { after_order_index: 0, lat: 53, lng: 10 });
|
||||
const second = service.create(1, { after_order_index: 0, lat: 54, lng: 11 });
|
||||
const elsewhere = service.create(1, { after_order_index: 3, lat: 55, lng: 12 });
|
||||
|
||||
const after = service.reanchor(1, { vias: [{ id: elsewhere.id, after_order_index: 2 }] });
|
||||
|
||||
const leg0 = after.filter(v => v.after_order_index === 0).sort((x, y) => x.sequence - y.sequence);
|
||||
expect(leg0.map(v => v.id)).toEqual([first.id, second.id]);
|
||||
expect(leg0.map(v => v.sequence)).toEqual([0, 1]);
|
||||
// And the one that moved is alone on its new leg, numbered from zero.
|
||||
expect(after.find(v => v.id === elsewhere.id)?.sequence).toBe(0);
|
||||
});
|
||||
|
||||
it('ROADTRIP-SVC-012: a chain lands in the order it was sent, per leg', () => {
|
||||
const vias = service.createMany(1, { vias: [
|
||||
{ after_order_index: 0, lat: 53.0, lng: 10.0 },
|
||||
|
||||
@@ -754,6 +754,52 @@ describe('folded trip CRUD', () => {
|
||||
expect((testDb.prepare('SELECT title FROM trips WHERE id = ?').get(secondCopy) as any).title).toBe('Origin');
|
||||
});
|
||||
|
||||
it('TRIP-SVC-061: copy carries the road-trip shaping, not just the places', () => {
|
||||
// A via is the road the traveller chose over the one the router prefers, and
|
||||
// a day track is the line a day was fitted to. Leaving them behind gave back
|
||||
// a trip that looks complete and quietly drives somewhere else — noticed
|
||||
// only once somebody edits the copy, with nothing left to recover from.
|
||||
const { user } = createUser(testDb);
|
||||
const trip = createTrip(testDb, user.id, { title: 'Norway', start_date: '2025-06-01', end_date: '2025-06-02' });
|
||||
const days = getDays(trip.id);
|
||||
const stop = createPlace(testDb, trip.id, { name: 'Geiranger' });
|
||||
const track = createPlace(testDb, trip.id, { name: 'Scenic route' });
|
||||
testDb.prepare("UPDATE places SET stop_type = 'fuel' WHERE id = ?").run(stop.id);
|
||||
testDb.prepare("UPDATE places SET route_geometry = '[[1,2],[3,4]]' WHERE id = ?").run(track.id);
|
||||
createDayAssignment(testDb, days[0].id, stop.id);
|
||||
testDb.prepare(
|
||||
'INSERT INTO roadtrip_vias (day_id, after_order_index, sequence, lat, lng) VALUES (?, 0, 0, 62.1, 7.2), (?, 0, 1, 62.2, 7.3)',
|
||||
).run(days[0].id, days[0].id);
|
||||
testDb.prepare('INSERT INTO roadtrip_day_tracks (day_id, place_id, stray_km) VALUES (?, ?, 1.5)')
|
||||
.run(days[0].id, track.id);
|
||||
|
||||
const newTripId = svc.copy(trip.id, user.id, 'Clone');
|
||||
const newDays = getDays(newTripId);
|
||||
|
||||
// The kind of stop each place is survives the copy.
|
||||
const copiedStop = testDb.prepare("SELECT stop_type FROM places WHERE trip_id = ? AND name = 'Geiranger'")
|
||||
.get(newTripId) as { stop_type: string | null };
|
||||
expect(copiedStop.stop_type).toBe('fuel');
|
||||
|
||||
const vias = testDb.prepare('SELECT after_order_index, sequence, lat, lng FROM roadtrip_vias WHERE day_id = ? ORDER BY sequence')
|
||||
.all(newDays[0].id) as { after_order_index: number; sequence: number; lat: number; lng: number }[];
|
||||
expect(vias).toEqual([
|
||||
{ after_order_index: 0, sequence: 0, lat: 62.1, lng: 7.2 },
|
||||
{ after_order_index: 0, sequence: 1, lat: 62.2, lng: 7.3 },
|
||||
]);
|
||||
|
||||
// The track points at the COPY's place, never back at the original.
|
||||
const copiedTrack = testDb.prepare('SELECT place_id, stray_km FROM roadtrip_day_tracks WHERE day_id = ?')
|
||||
.get(newDays[0].id) as { place_id: number; stray_km: number };
|
||||
const copiedTrackPlace = testDb.prepare("SELECT id FROM places WHERE trip_id = ? AND name = 'Scenic route'")
|
||||
.get(newTripId) as { id: number };
|
||||
expect(copiedTrack.place_id).toBe(copiedTrackPlace.id);
|
||||
expect(copiedTrack.stray_km).toBe(1.5);
|
||||
|
||||
// And the original keeps exactly what it had.
|
||||
expect(testDb.prepare('SELECT COUNT(*) c FROM roadtrip_vias WHERE day_id = ?').get(days[0].id)).toEqual({ c: 2 });
|
||||
});
|
||||
|
||||
it('TRIP-SVC-060: copying a trip keeps a staged booking staged', () => {
|
||||
const { user } = createUser(testDb);
|
||||
const trip = createTrip(testDb, user.id, { title: 'Origin', start_date: '2025-06-01', end_date: '2025-06-02' });
|
||||
|
||||
Reference in New Issue
Block a user