diff --git a/client/src/components/Planner/TransportModal.test.tsx b/client/src/components/Planner/TransportModal.test.tsx index 69c6c2ffd..8f3abb8b6 100644 --- a/client/src/components/Planner/TransportModal.test.tsx +++ b/client/src/components/Planner/TransportModal.test.tsx @@ -375,6 +375,31 @@ describe('TransportModal', () => { expect(payload.endpoints.map((e: { role: string }) => e.role)).toEqual(['from', 'to']); }); + it('FE-PLANNER-TRANSMODAL-029: re-saving a joined AirTrail flight keeps metadata.airtrail_ids (#1535)', async () => { + const onSave = vi.fn().mockResolvedValue(undefined); + const res = buildReservation({ title: 'BRU → HEL → JFK', type: 'flight' }) as any; + res.metadata = { + airline: 'Finnair', + flight_number: 'AY1502', + airtrail_ids: ['101', '102'], + legs: [ + { from: 'BRU', to: 'HEL', flight_number: 'AY1502', dep_time: '08:00', arr_time: '12:30' }, + { from: 'HEL', to: 'JFK', flight_number: 'AY15', dep_time: '14:00', arr_time: '15:00' }, + ], + }; + res.endpoints = [ + { role: 'from', sequence: 0, name: 'Brussels', code: 'BRU', lat: 50.9, lng: 4.48, timezone: 'Europe/Brussels', local_date: '2025-06-01', local_time: '08:00' }, + { role: 'stop', sequence: 1, name: 'Helsinki-Vantaa', code: 'HEL', lat: 60.32, lng: 24.96, timezone: 'Europe/Helsinki', local_date: '2025-06-01', local_time: '14:00' }, + { role: 'to', sequence: 2, name: 'JFK', code: 'JFK', lat: 40.64, lng: -73.78, timezone: 'America/New_York', local_date: '2025-06-01', local_time: '15:00' }, + ]; + render(); + // A routine edit (retitle + save) must not cost the booking its AirTrail + // linkage — the import picker relies on it to not re-offer the legs. + await userEvent.click(screen.getByRole('button', { name: /^Update$/i })); + await waitFor(() => expect(onSave).toHaveBeenCalled()); + expect(onSave.mock.calls[0][0].metadata?.airtrail_ids).toEqual(['101', '102']); + }); + // ── Manual / Automated creation switch (#1065) ───────────────────────────── it('FE-PLANNER-TRANSMODAL-022: creating shows the Manual/Automated switch; Automated opens the transit search', async () => { diff --git a/client/src/components/Planner/TransportModal.tsx b/client/src/components/Planner/TransportModal.tsx index 725487340..1bb5ca356 100644 --- a/client/src/components/Planner/TransportModal.tsx +++ b/client/src/components/Planner/TransportModal.tsx @@ -423,6 +423,10 @@ export function TransportModal({ isOpen, onClose, onSave, reservation, days, sel near(prevFrom.lat, fromPick.location.lat) && near(prevFrom.lng, fromPick.location.lng) && near(prevTo.lat, toPick.location.lat) && near(prevTo.lng, toPick.location.lng)) if (keepTransit) metadata.transit = prevMeta.transit + // A joined AirTrail import (#1535) records every source flight id in + // metadata.airtrail_ids so the picker doesn't offer those legs again — + // an edit in this form must not drop that linkage. + if (Array.isArray(prevMeta.airtrail_ids)) metadata.airtrail_ids = prevMeta.airtrail_ids const startDate = startDay?.date ?? null const endDate = (endDay ?? startDay)?.date ?? null diff --git a/server/src/services/airtrail/airtrailSync.ts b/server/src/services/airtrail/airtrailSync.ts index bf76b3a85..7d2d22b12 100644 --- a/server/src/services/airtrail/airtrailSync.ts +++ b/server/src/services/airtrail/airtrailSync.ts @@ -39,6 +39,26 @@ function detach(tripId: number, reservationId: number): void { broadcastUpdated(tripId, reservationId); } +/** + * True when the reservation has grown into a multi-leg booking locally (extra + * stops / metadata.legs) — a shape the single AirTrail flight it is linked to + * cannot represent. Syncing such a row in either direction would corrupt one + * side: a pull flattens the layover chain back to from→to, a push rewrites the + * AirTrail flight to span the whole route (#1535). + */ +function hasLocalMultiLegShape(reservationId: number, metadataJson: string | null | undefined): boolean { + try { + const meta = metadataJson ? JSON.parse(metadataJson) : {}; + if (Array.isArray(meta?.legs) && meta.legs.length > 1) return true; + } catch { + /* malformed metadata — fall through to the endpoint count */ + } + const row = db.prepare('SELECT COUNT(*) AS n FROM reservation_endpoints WHERE reservation_id = ?').get(reservationId) as { + n: number; + }; + return row.n > 2; +} + // ── AirTrail → TREK (poll) ─────────────────────────────────────────────────── /** @@ -81,6 +101,13 @@ async function syncOwner(uid: number): Promise { const current = getReservation(row.id, row.trip_id); if (!current) continue; + if (hasLocalMultiLegShape(row.id, (current as any).metadata)) { + // The user connected this flight into a multi-leg booking; applying the + // remote single-flight shape would flatten it. Stop syncing instead. + detach(row.trip_id, row.id); + changed++; + continue; + } try { updateReservation(row.id, row.trip_id, mapFlightToReservation(flight) as any, current as any); db.prepare('UPDATE reservations SET external_hash = ?, external_synced_at = ? WHERE id = ?').run( @@ -247,6 +274,17 @@ export async function pushReservationToAirtrail(reservationId: number, tripId: n | undefined; if (!row || !row.sync_enabled) return; + // An edit that turned this linked flight into a multi-leg booking severs the + // 1:1 mapping to the AirTrail flight: pushing would rewrite that flight to the + // full span, and the next pull would flatten the layover again. Detach — the + // merge is a deliberate local restructuring, like a joined import (#1535). + const reservation = getReservationWithJoins(row.id); + if (!reservation) return; + if (hasLocalMultiLegShape(row.id, reservation.metadata)) { + detach(tripId, row.id); + return; + } + // AirTrail is read-only by default (#1240). Only push when the flight's owner has // explicitly opted in. A no-op skip (not a detach): the link stays active so the // inbound, AirTrail-wins pull keeps the reservation up to date. @@ -271,9 +309,6 @@ export async function pushReservationToAirtrail(reservationId: number, tripId: n return; } - const reservation = getReservationWithJoins(row.id); - if (!reservation) return; - const payload = buildSavePayload(reservation, existing); if (!payload) return; diff --git a/server/tests/unit/services/airtrailWriteGate.test.ts b/server/tests/unit/services/airtrailWriteGate.test.ts index 18405abdf..81964989a 100644 --- a/server/tests/unit/services/airtrailWriteGate.test.ts +++ b/server/tests/unit/services/airtrailWriteGate.test.ts @@ -31,10 +31,10 @@ vi.mock('../../../src/services/airtrail/airtrailService', () => ({ getAirtrailCredentials: vi.fn(), })); -import { pushReservationToAirtrail } from '../../../src/services/airtrail/airtrailSync'; +import { pushReservationToAirtrail, runAirtrailSyncForUser } from '../../../src/services/airtrail/airtrailSync'; import { db } from '../../../src/db/database'; -import { getReservationWithJoins } from '../../../src/services/reservationService'; -import { getFlight, saveFlight } from '../../../src/services/airtrail/airtrailClient'; +import { getReservation, getReservationWithJoins, updateReservation } from '../../../src/services/reservationService'; +import { getFlight, listFlights, saveFlight } from '../../../src/services/airtrail/airtrailClient'; import { isAirtrailWriteEnabled, getAirtrailCredentials } from '../../../src/services/airtrail/airtrailService'; const linkedRow = { id: 5, trip_id: 9, external_id: '42', external_owner_user_id: 7, sync_enabled: 1 }; @@ -42,10 +42,12 @@ const runSpy = vi.fn(); beforeEach(() => { vi.clearAllMocks(); - // Route db reads: global sync setting + the linked reservation row. + // Route db reads: global sync setting, the linked reservation row and the + // endpoint count the multi-leg guard checks (#1535) — two = plain from/to. (db.prepare as any).mockImplementation((sql: string) => ({ get: () => { if (sql.includes('app_settings')) return { value: 'true' }; + if (sql.includes('FROM reservation_endpoints')) return { n: 2 }; if (sql.includes('FROM reservations')) return { ...linkedRow }; return undefined; }, @@ -89,4 +91,86 @@ describe('pushReservationToAirtrail write gate (#1240)', () => { expect(payload.departureTerminal).toBe('7'); // spread preserved the unmanaged field expect(payload.from).toBe('JFK'); // TREK-managed field still applied as a code }); + + it('#1535 detaches instead of pushing when the reservation grew extra stops', async () => { + (isAirtrailWriteEnabled as any).mockReturnValue(true); + (db.prepare as any).mockImplementation((sql: string) => ({ + get: () => { + if (sql.includes('app_settings')) return { value: 'true' }; + if (sql.includes('FROM reservation_endpoints')) return { n: 3 }; // from + stop + to + if (sql.includes('FROM reservations')) return { ...linkedRow }; + return undefined; + }, + run: (...args: any[]) => { + runSpy(sql, args); + return {}; + }, + all: () => [], + })); + await pushReservationToAirtrail(5, 9); + // Pushing would rewrite the single AirTrail flight to span the whole route. + expect(saveFlight).not.toHaveBeenCalled(); + expect(runSpy).toHaveBeenCalledWith(expect.stringContaining('sync_enabled = 0'), [5]); + }); + + it('#1535 detaches on metadata.legs even when the endpoint count is not available', async () => { + (isAirtrailWriteEnabled as any).mockReturnValue(true); + (getReservationWithJoins as any).mockReturnValue({ + external_id: '42', + reservation_time: '2021-09-01T19:00', + metadata: JSON.stringify({ legs: [{ from: 'BRU', to: 'HEL' }, { from: 'HEL', to: 'JFK' }] }), + endpoints: [], + }); + await pushReservationToAirtrail(5, 9); + expect(saveFlight).not.toHaveBeenCalled(); + expect(runSpy).toHaveBeenCalledWith(expect.stringContaining('sync_enabled = 0'), [5]); + }); +}); + +describe('inbound sync multi-leg guard (#1535)', () => { + it('detaches instead of flattening when a linked reservation grew extra stops locally', async () => { + // A remote change is pending (stored hash differs from canonicalHash's + // 'hash'), but the local reservation has become multi-leg — applying the + // single-flight shape would flatten the layover chain. + (db.prepare as any).mockImplementation((sql: string) => ({ + get: () => { + if (sql.includes('app_settings')) return { value: 'true' }; + if (sql.includes('FROM reservation_endpoints')) return { n: 3 }; + return undefined; + }, + all: () => (sql.includes('sync_enabled = 1') ? [{ id: 5, trip_id: 9, external_id: '42', external_hash: 'stale' }] : []), + run: (...args: any[]) => { + runSpy(sql, args); + return {}; + }, + })); + (listFlights as any).mockResolvedValue([{ id: 42 }]); + (getReservation as any).mockReturnValue({ id: 5, metadata: JSON.stringify({}) }); + + const { changed } = await runAirtrailSyncForUser(7); + expect(updateReservation).not.toHaveBeenCalled(); + expect(runSpy).toHaveBeenCalledWith(expect.stringContaining('sync_enabled = 0'), [5]); + expect(changed).toBe(1); + }); + + it('still applies a remote change to a plain single-leg reservation', async () => { + (db.prepare as any).mockImplementation((sql: string) => ({ + get: () => { + if (sql.includes('app_settings')) return { value: 'true' }; + if (sql.includes('FROM reservation_endpoints')) return { n: 2 }; + return undefined; + }, + all: () => (sql.includes('sync_enabled = 1') ? [{ id: 5, trip_id: 9, external_id: '42', external_hash: 'stale' }] : []), + run: (...args: any[]) => { + runSpy(sql, args); + return {}; + }, + })); + (listFlights as any).mockResolvedValue([{ id: 42 }]); + (getReservation as any).mockReturnValue({ id: 5, metadata: JSON.stringify({}) }); + + await runAirtrailSyncForUser(7); + expect(updateReservation).toHaveBeenCalledTimes(1); + expect(runSpy).not.toHaveBeenCalledWith(expect.stringContaining('sync_enabled = 0'), expect.anything()); + }); });