From 60a42cfac1310b9f2eaef325fb2f5136d964025a Mon Sep 17 00:00:00 2001 From: Maurice Date: Thu, 8 Oct 2026 15:55:57 +0200 Subject: [PATCH] fix(airtrail): import a flight with its AirTrail link in one transaction The import created the booking and wrote its external_* link as two commits. A failed link left the booking in the trip while the import reported the flight as skipped. The flights are fetched before the loop, so nothing between the two writes waits on the network: each flight or joined connection now goes in with its link in one transaction, and the method is marked as one transaction per imported booking instead of being held in the lint:tx baseline. --- server/scripts/tx-writes-baseline.json | 1 - .../integrations/airtrail-import.service.ts | 25 +++++++++++-- server/tests/helpers/mcp-test-controllers.ts | 1 + .../unit/services/airtrailImport.test.ts | 37 +++++++++++++++++++ 4 files changed, 59 insertions(+), 5 deletions(-) diff --git a/server/scripts/tx-writes-baseline.json b/server/scripts/tx-writes-baseline.json index cff814139..a1db60507 100644 --- a/server/scripts/tx-writes-baseline.json +++ b/server/scripts/tx-writes-baseline.json @@ -27,7 +27,6 @@ "src/nest/doc-sync/doc-sync.service.ts#DocSyncService.resolveConflict": 3, "src/nest/doc-sync/doc-sync.service.ts#DocSyncService.runLink": 4, "src/nest/doc-sync/doc-sync.service.ts#DocSyncService.syncLink": 2, - "src/nest/integrations/airtrail-import.service.ts#AirtrailImportService.importAirtrailFlights": 8, "src/nest/integrations/airtrail-link.service.ts#AirtrailLinkService.pushReservationToAirtrail": 2, "src/nest/integrations/airtrail-sync.service.ts#AirtrailSyncService.runAirtrailSync": 2, "src/nest/integrations/airtrail-sync.service.ts#AirtrailSyncService.syncOwner": 8, diff --git a/server/src/nest/integrations/airtrail-import.service.ts b/server/src/nest/integrations/airtrail-import.service.ts index d675d1cd6..01d69891b 100644 --- a/server/src/nest/integrations/airtrail-import.service.ts +++ b/server/src/nest/integrations/airtrail-import.service.ts @@ -9,6 +9,7 @@ import { Days } from '../../db/entities/Days.entity'; import { DaysRepository } from '../../db/repositories/Days.repository'; import { RealtimeService } from '../realtime/realtime.service'; import { ReservationsService } from '../reservations/reservations.service'; +import { UnitOfWork } from '../database/unit-of-work'; import { AirtrailRequestError, type AirtrailFlightRaw } from './airtrail.client'; import { AirtrailClient } from './airtrail.client'; import { AirtrailService } from './airtrail.service'; @@ -121,6 +122,7 @@ export class AirtrailImportService { private readonly reservations: ReservationsService, private readonly client: AirtrailClient, private readonly airtrail: AirtrailService, + private readonly uow: UnitOfWork, ) {} /** @@ -135,6 +137,11 @@ export class AirtrailImportService { return Number.isFinite(n) ? n : -1; } + /** + * @txIndependent one transaction per imported booking: a flight or a joined + * connection lands with its AirTrail link or not at all, and one that fails is + * reported skipped while the others stay imported. + */ async importAirtrailFlights( tripId: string | number, userId: number, @@ -245,9 +252,15 @@ export class AirtrailImportService { const ids = chain.map(f => String(f.id)); try { const mapped = mapFlightsToMultiLegReservation(chain, resolveDayId); - const { reservation } = await this.reservations.create(tripId, mapped as any); const now = new Date().toISOString(); - await this.reservationsRepo.linkAirtrailMultiLeg(Number(reservation.id), ids[0], userId, now); + // The booking and its AirTrail link are one write: a failed link must not + // leave a booking behind that this import then reports as skipped. The + // flights were fetched above, so nothing in here waits on the network. + const reservation = await this.uow.transactional(async () => { + const created = (await this.reservations.create(tripId, mapped as any)).reservation; + await this.reservationsRepo.linkAirtrailMultiLeg(Number(created.id), ids[0], userId, now); + return created; + }); reservation.external_source = 'airtrail'; reservation.external_id = ids[0]; @@ -286,9 +299,13 @@ export class AirtrailImportService { } try { - const { reservation } = await this.reservations.create(tripId, mapped as any); const now = new Date().toISOString(); - await this.reservationsRepo.linkAirtrailSingleFlight(Number(reservation.id), fid, userId, canonicalHash(flight), now); + // One write with its link, as for a joined connection above. + const reservation = await this.uow.transactional(async () => { + const created = (await this.reservations.create(tripId, mapped as any)).reservation; + await this.reservationsRepo.linkAirtrailSingleFlight(Number(created.id), fid, userId, canonicalHash(flight), now); + return created; + }); // Carry the linkage on the broadcast payload so members see the badge live. reservation.external_source = 'airtrail'; diff --git a/server/tests/helpers/mcp-test-controllers.ts b/server/tests/helpers/mcp-test-controllers.ts index 71db16df8..9a3c6e989 100644 --- a/server/tests/helpers/mcp-test-controllers.ts +++ b/server/tests/helpers/mcp-test-controllers.ts @@ -519,6 +519,7 @@ export async function createMcpTestRegistry(): Promise { new ReservationImportMcp(new AirtrailImportService( await createTestReservationsRepo(db), await createTestReservationEndpointsRepo(db), await createTestDaysRepo(db), realtimeService, reservationsService, new AirtrailClient(), new AirtrailService(usersRepo, new AuditService(auditLogRepo, usersRepo), new AirtrailClient()), + await createTestUnitOfWork(db), ), await createTestTripsRepo(db), authService, guards, addonsService), new SettingsMcp(new SettingsService(await createTestUnitOfWork(db), appSettings, await createTestSettingsRepo(db)), authService), new HelpMcp(), new AddonsMcp(addonsService), diff --git a/server/tests/unit/services/airtrailImport.test.ts b/server/tests/unit/services/airtrailImport.test.ts index a1bf3eb60..aedf5556e 100644 --- a/server/tests/unit/services/airtrailImport.test.ts +++ b/server/tests/unit/services/airtrailImport.test.ts @@ -57,6 +57,7 @@ async function makeImportService(): Promise { { getAirtrailCredentials: () => ({ baseUrl: 'https://at.example', apiKey: 'k', allowInsecureTls: false }), } as unknown as AirtrailService, + await createTestUnitOfWork(db), ); } @@ -262,3 +263,39 @@ describe('importAirtrailFlights connection joining (#1535)', () => { expect(JSON.parse(r.metadata).airtrail_ids).toBeUndefined(); }); }); + +describe('importAirtrailFlights writes a booking with its link', () => { + it('rolls the booking back when the single-flight link fails, so a flight reported skipped is not in the trip', async () => { + listFlights.mockResolvedValue([legBruHel()]); + const quiet = vi.spyOn(console, 'error').mockImplementation(() => {}); + const repo = await createTestReservationsRepo(db); + const spy = vi.spyOn(repo, 'linkAirtrailSingleFlight').mockRejectedValueOnce(new Error('boom')); + try { + const result = await importAirtrailFlights(tripId, userId, ['101'], undefined); + expect(result.imported).toEqual([]); + expect(result.skipped).toEqual([{ flightId: '101', reason: 'invalid', detail: 'boom' }]); + } finally { + spy.mockRestore(); + quiet.mockRestore(); + } + expect(tripReservations(tripId)).toEqual([]); + expect(broadcast).not.toHaveBeenCalled(); + }); + + it('rolls a joined connection back when its link fails', async () => { + listFlights.mockResolvedValue([legBruHel(), legHelJfk()]); + const quiet = vi.spyOn(console, 'error').mockImplementation(() => {}); + const repo = await createTestReservationsRepo(db); + const spy = vi.spyOn(repo, 'linkAirtrailMultiLeg').mockRejectedValueOnce(new Error('boom')); + try { + const result = await importAirtrailFlights(tripId, userId, ['101', '102'], undefined, [['101', '102']]); + expect(result.imported).toEqual([]); + expect(result.skipped.map(s => [s.flightId, s.reason])).toEqual([['101', 'invalid'], ['102', 'invalid']]); + } finally { + spy.mockRestore(); + quiet.mockRestore(); + } + expect(tripReservations(tripId)).toEqual([]); + expect(broadcast).not.toHaveBeenCalled(); + }); +});