From 1e1dd9b814062c7dafea44e1944b071d4720b8d2 Mon Sep 17 00:00:00 2001 From: Maurice Date: Mon, 7 Sep 2026 14:00:22 +0200 Subject: [PATCH] fix(roadtrip): keep the shaping a traveller did, through every path that moves a stop MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- client/src/api/client.ts | 9 ++- .../Roadtrip/RoadtripStopPopup.test.tsx | 32 +++++++++ .../components/Roadtrip/RoadtripStopPopup.tsx | 16 +++-- client/src/components/Roadtrip/corridor.ts | 21 +++++- .../components/Roadtrip/reanchorVias.test.ts | 51 ++++++++++++++ .../components/Roadtrip/roadtripModel.test.ts | 49 ++++++++++++++ .../src/components/Roadtrip/roadtripModel.ts | 66 +++++++++++++++++-- .../Roadtrip/useFollowTrack.test.ts | 1 + .../Roadtrip/useRoadtripVias.test.ts | 27 +++++++- .../components/Roadtrip/useRoadtripVias.ts | 45 +++++++++++-- .../src/pages/tripPlanner/useTripPlanner.ts | 50 ++++++++++++-- server/src/nest/places/places.service.ts | 21 +++++- server/src/nest/roadtrip/roadtrip.service.ts | 50 ++++++++++++++ server/src/nest/trips/trips.service.ts | 39 ++++++++++- .../tests/unit/nest/places.controller.test.ts | 7 +- .../tests/unit/nest/places.directions.test.ts | 36 ++++++++++ .../tests/unit/nest/roadtrip.service.test.ts | 41 ++++++++++++ server/tests/unit/nest/trips.service.test.ts | 46 +++++++++++++ 18 files changed, 579 insertions(+), 28 deletions(-) diff --git a/client/src/api/client.ts b/client/src/api/client.ts index 743bd0263..ede0c14b3 100644 --- a/client/src/api/client.ts +++ b/client/src/api/client.ts @@ -472,8 +472,15 @@ export const placesApi = { if (opts?.paths !== undefined) fd.append('importPaths', String(opts.paths)) return postMultipart(`/trips/${tripId}/places/import/map`, fd) }, + // A longer timeout than the shared 8 s, like the other routes here that wait + // on somebody else's service. A directions link whose stops are only named + // has to be geocoded one at a time behind a 1.1 s throttle, so a route with + // eight stops needs about ten seconds. Giving up at eight left the server + // finishing the import and writing the places while the browser reported a + // failure, and a retry then spent the whole geocoding budget again only to + // have the dedupe skip every stop. importGoogleList: (tripId: number | string, url: string, enrich?: boolean) => - apiClient.post(`/trips/${tripId}/places/import/google-list`, { url, enrich } satisfies PlaceImportListRequest).then(r => r.data), + apiClient.post(`/trips/${tripId}/places/import/google-list`, { url, enrich } satisfies PlaceImportListRequest, { timeout: 60000 }).then(r => r.data), importNaverList: (tripId: number | string, url: string, enrich?: boolean) => apiClient.post(`/trips/${tripId}/places/import/naver-list`, { url, enrich } satisfies PlaceImportListRequest).then(r => r.data), bulkDelete: (tripId: number | string, ids: number[]) => diff --git a/client/src/components/Roadtrip/RoadtripStopPopup.test.tsx b/client/src/components/Roadtrip/RoadtripStopPopup.test.tsx index 6a990f938..07b35b3ae 100644 --- a/client/src/components/Roadtrip/RoadtripStopPopup.test.tsx +++ b/client/src/components/Roadtrip/RoadtripStopPopup.test.tsx @@ -234,6 +234,38 @@ describe('RoadtripStopPopup', () => { expect(onSave).toHaveBeenCalled() }) + it('FE-ROADTRIP-STOPPOPUP-026: a scheme-less OSM website still leaves the app', () => { + // OSM `website` tags are routinely written without a scheme. As a raw href + // `www.hotel.de` is a RELATIVE url, so the click navigated the planner to + // /trips//www.hotel.de and the user lost the page they were on. + wrap( + , + ) + + expect(screen.getByRole('link', { name: 'Website' })).toHaveAttribute('href', 'https://www.hotel-adlon.de') + }) + + it('FE-ROADTRIP-STOPPOPUP-027: a website that is not a link is not rendered as one', () => { + // Anything that is not http(s) loses the link rather than being handed to + // the browser. The CSP stops a javascript: navigation on a stock deployment, + // but this is defended everywhere else in the app and belongs here too. + for (const bad of ['javascript:alert(1)', 'data:text/html,