mirror of
https://github.com/mauriceboe/TREK.git
synced 2026-10-19 05:26:47 +00:00
fix(airtrail): stop syncing a booking once it grows extra stops
A linked flight that becomes multi-leg locally no longer matches the single AirTrail flight it was imported from: pushing would rewrite that flight to span the whole route, and the next pull would flatten the layover chain back to a plain from/to. Both sync directions now detach the link instead — the same state a joined import starts in, surfaced by the existing "Not synced" badge. TransportModal also carries metadata.airtrail_ids through re-saves (like it already does for transit itineraries and day positions), so editing a joined booking cannot cost it its import dedupe and get its legs re-offered in the picker.
This commit is contained in:
@@ -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(<TransportModal {...defaultProps} reservation={res} onSave={onSave} />);
|
||||
// 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 () => {
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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<number> {
|
||||
|
||||
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;
|
||||
|
||||
|
||||
@@ -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());
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user