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(); + }); +});