diff --git a/server/src/nest/accommodations/accommodations.controller.ts b/server/src/nest/accommodations/accommodations.controller.ts index 60f68c888..a05db5c3a 100644 --- a/server/src/nest/accommodations/accommodations.controller.ts +++ b/server/src/nest/accommodations/accommodations.controller.ts @@ -68,8 +68,8 @@ export class AccommodationsController { } @Get() - list(@CurrentUser() user: User, @Param('tripId') tripId: string) { - return { accommodations: this.accommodations.list(tripId) }; + async list(@CurrentUser() user: User, @Param('tripId') tripId: string) { + return { accommodations: await this.accommodations.list(tripId) }; } @RequirePermission('day_edit') @@ -85,7 +85,7 @@ export class AccommodationsController { if (!place_id || !start_day_id || !end_day_id) { throw new HttpException({ error: 'place_id, start_day_id, and end_day_id are required' }, 400); } - const errors = this.accommodations.validateRefs(tripId, place_id, start_day_id, end_day_id); + const errors = await this.accommodations.validateRefs(tripId, place_id, start_day_id, end_day_id); if (errors.length > 0) { throw new HttpException({ error: errors[0].message }, 404); } @@ -108,12 +108,12 @@ export class AccommodationsController { @Headers('x-socket-id') socketId?: string, ) { const body = rawBody as AccommodationBody; - const existing = this.accommodations.get(id, tripId); + const existing = await this.accommodations.get(id, tripId); if (!existing) { throw new HttpException({ error: 'Accommodation not found' }, 404); } const { place_id, start_day_id, end_day_id, check_in, check_in_end, check_out, confirmation, notes } = body; - const errors = this.accommodations.validateRefs(tripId, place_id, start_day_id, end_day_id); + const errors = await this.accommodations.validateRefs(tripId, place_id, start_day_id, end_day_id); if (errors.length > 0) { throw new HttpException({ error: errors[0].message }, 404); } @@ -132,7 +132,7 @@ export class AccommodationsController { @Headers('x-socket-id') socketId?: string, @Query('keepStop') keepStop?: string, ) { - if (!this.accommodations.get(id, tripId)) { + if (!(await this.accommodations.get(id, tripId))) { throw new HttpException({ error: 'Accommodation not found' }, 404); } // Turning a night back into a pause in road trip mode: the booking goes, the diff --git a/server/src/nest/accommodations/accommodations.mcp.ts b/server/src/nest/accommodations/accommodations.mcp.ts index 7c5d60d91..e626685ed 100644 --- a/server/src/nest/accommodations/accommodations.mcp.ts +++ b/server/src/nest/accommodations/accommodations.mcp.ts @@ -69,9 +69,9 @@ export class AccommodationsMcp { ctx: McpContext, ) { if (await this.auth.isDemoUser(ctx.userId)) return demoDenied(); - if (!this.accommodations.verifyTripAccess(tripId, ctx.userId)) return noAccess(); + if (!(await this.accommodations.verifyTripAccess(tripId, ctx.userId))) return noAccess(); if (!(await this.guards.hasTripPermission('day_edit', tripId, ctx.userId))) return permissionDenied(); - const errors = this.accommodations.validateAccommodationRefs(tripId, place_id, start_day_id, end_day_id); + const errors = await this.accommodations.validateAccommodationRefs(tripId, place_id, start_day_id, end_day_id); if (errors.length > 0) return { content: [{ type: 'text' as const, text: errors.map(e => e.message).join(', ') }], isError: true }; const { accommodation, mirror } = await this.accommodations.createAccommodation(tripId, { place_id, start_day_id, end_day_id, check_in, check_in_end, check_out, confirmation, notes }); this.guards.safeBroadcast(tripId, 'accommodation:created', { accommodation }); @@ -120,13 +120,13 @@ export class AccommodationsMcp { ctx: McpContext, ) { if (await this.auth.isDemoUser(ctx.userId)) return demoDenied(); - if (!this.accommodations.verifyTripAccess(tripId, ctx.userId)) return noAccess(); + if (!(await this.accommodations.verifyTripAccess(tripId, ctx.userId))) return noAccess(); if (!(await this.guards.hasTripPermission('day_edit', tripId, ctx.userId))) return permissionDenied(); - const dayErrors = this.accommodations.validateAccommodationRefs(tripId, undefined, start_day_id, end_day_id); + const dayErrors = await this.accommodations.validateAccommodationRefs(tripId, undefined, start_day_id, end_day_id); if (dayErrors.length > 0) return { content: [{ type: 'text' as const, text: dayErrors.map(e => e.message).join(', ') }], isError: true }; try { const result = await this.uow.transactional(async () => { - const place = this.places.create(String(tripId), { name, description, lat, lng, address, category_id, google_place_id, google_ftid, osm_id, notes: place_notes, website, phone, price, currency }); + const place = await this.places.create(String(tripId), { name, description, lat, lng, address, category_id, google_place_id, google_ftid, osm_id, notes: place_notes, website, phone, price, currency }); const { accommodation, mirror } = await this.accommodations.createAccommodation(tripId, { place_id: place.id, start_day_id, end_day_id, check_in, check_in_end, check_out, confirmation, notes: accommodation_notes }); return { place, accommodation, mirror }; }); @@ -168,9 +168,9 @@ export class AccommodationsMcp { ctx: McpContext, ) { if (await this.auth.isDemoUser(ctx.userId)) return demoDenied(); - if (!this.accommodations.verifyTripAccess(tripId, ctx.userId)) return noAccess(); + if (!(await this.accommodations.verifyTripAccess(tripId, ctx.userId))) return noAccess(); if (!(await this.guards.hasTripPermission('day_edit', tripId, ctx.userId))) return permissionDenied(); - const existing = this.accommodations.getAccommodation(accommodationId, tripId); + const existing = await this.accommodations.getAccommodation(accommodationId, tripId); if (!existing) return { content: [{ type: 'text' as const, text: 'Accommodation not found.' }], isError: true }; const { accommodation, mirror } = await this.accommodations.updateAccommodation(accommodationId, existing, { place_id, start_day_id, end_day_id, check_in, check_in_end, check_out, confirmation, notes }); this.guards.safeBroadcast(tripId, 'accommodation:updated', { accommodation }); @@ -192,9 +192,9 @@ export class AccommodationsMcp { }) async deleteAccommodation({ tripId, accommodationId }: { tripId: number; accommodationId: number }, ctx: McpContext) { if (await this.auth.isDemoUser(ctx.userId)) return demoDenied(); - if (!this.accommodations.verifyTripAccess(tripId, ctx.userId)) return noAccess(); + if (!(await this.accommodations.verifyTripAccess(tripId, ctx.userId))) return noAccess(); if (!(await this.guards.hasTripPermission('day_edit', tripId, ctx.userId))) return permissionDenied(); - if (!this.accommodations.getAccommodation(accommodationId, tripId)) return { content: [{ type: 'text' as const, text: 'Accommodation not found.' }], isError: true }; + if (!(await this.accommodations.getAccommodation(accommodationId, tripId))) return { content: [{ type: 'text' as const, text: 'Accommodation not found.' }], isError: true }; // linkedReservationId stays the first one so the tool's answer keeps its shape; // linkedReservationIds carries the rest for a block that had more than one booking. const { linkedReservationId, linkedReservationIds, mirror } = await this.accommodations.deleteAccommodation(accommodationId); @@ -212,7 +212,7 @@ export class AccommodationsMcp { }) async tripAccommodationsResource(uri: URL, { tripId }: { tripId: string | string[] }, ctx: McpContext) { const id = parseId(tripId); - if (id === null || !this.accommodations.verifyTripAccess(id, ctx.userId)) { + if (id === null || !(await this.accommodations.verifyTripAccess(id, ctx.userId))) { return { contents: [{ uri: uri.href, @@ -221,7 +221,7 @@ export class AccommodationsMcp { }], }; } - const accommodations = this.accommodations.listAccommodations(id); + const accommodations = await this.accommodations.listAccommodations(id); return { contents: [{ uri: uri.href, diff --git a/server/src/nest/accommodations/accommodations.rpc.ts b/server/src/nest/accommodations/accommodations.rpc.ts index 1e3a2df95..53051e408 100644 --- a/server/src/nest/accommodations/accommodations.rpc.ts +++ b/server/src/nest/accommodations/accommodations.rpc.ts @@ -54,7 +54,7 @@ export class AccommodationsRpc { const endDayId = Math.trunc(Number(input.end_day_id)); if (!placeId || !startDayId || !endDayId) throw new BadParams('place_id, start_day_id, and end_day_id are required'); // Verifies the place and both days belong to this trip. - const errors = this.days.validateAccommodationRefs(tripId, placeId, startDayId, endDayId); + const errors = await this.days.validateAccommodationRefs(tripId, placeId, startDayId, endDayId); if (errors.length > 0) throw new ForbiddenResource(errors[0].message); const { accommodation, mirror } = await this.days.createAccommodation(tripId, { place_id: placeId, @@ -81,10 +81,10 @@ export class AccommodationsRpc { const parsed = accommodationUpdateRequestSchema.safeParse(params.input); if (!parsed.success) throw new BadParams(`invalid accommodation: ${schemaMessage(parsed.error)}`); await this.guards.requireTripEdit(tripId, actor, ACCOMMODATION_EDIT_ACTION); - const existing = this.days.getAccommodation(accommodationId, tripId); + const existing = await this.days.getAccommodation(accommodationId, tripId); if (!existing) throw new ForbiddenResource(`no accommodation ${accommodationId} on trip ${tripId}`); const input = parsed.data as { place_id?: number; start_day_id?: number; end_day_id?: number; check_in?: string; check_in_end?: string; check_out?: string; confirmation?: string; notes?: string }; - const errors = this.days.validateAccommodationRefs(tripId, input.place_id, input.start_day_id, input.end_day_id); + const errors = await this.days.validateAccommodationRefs(tripId, input.place_id, input.start_day_id, input.end_day_id); if (errors.length > 0) throw new ForbiddenResource(errors[0].message); const { accommodation, mirror } = await this.days.updateAccommodation(accommodationId, existing, input); this.realtime.broadcast(tripId, 'accommodation:updated', { accommodation }); @@ -98,7 +98,7 @@ export class AccommodationsRpc { const accommodationId = num(params.accommodationId, 'accommodationId'); const actor = this.guards.requireActor(ctx, 'accommodation'); await this.guards.requireTripEdit(tripId, actor, ACCOMMODATION_EDIT_ACTION); - if (!this.days.getAccommodation(accommodationId, tripId)) { + if (!(await this.days.getAccommodation(accommodationId, tripId))) { throw new ForbiddenResource(`no accommodation ${accommodationId} on trip ${tripId}`); } // Deleting a block can take its partner reservation and budget item with it. diff --git a/server/src/nest/accommodations/accommodations.service.ts b/server/src/nest/accommodations/accommodations.service.ts index 1867dd8e4..9405bc426 100644 --- a/server/src/nest/accommodations/accommodations.service.ts +++ b/server/src/nest/accommodations/accommodations.service.ts @@ -105,7 +105,7 @@ export class AccommodationsService { /** Owner or member, returning the trip. Takes a number too: the MCP tools pass * the parsed id, the REST path the raw param. */ - verifyTripAccess(tripId: string | number, userId: number) { + async verifyTripAccess(tripId: string | number, userId: number) { return this.dbs.canAccessTrip(Number(tripId), userId); } @@ -235,7 +235,7 @@ export class AccommodationsService { // ------------------------------------------------------------------------- - private getAccommodationWithPlace(id: number | bigint) { + private async getAccommodationWithPlace(id: number | bigint) { return this.db.get(` SELECT a.*, p.name as place_name, p.address as place_address, p.image_url as place_image, p.lat as place_lat, p.lng as place_lng FROM day_accommodations a @@ -244,7 +244,7 @@ export class AccommodationsService { `, id); } - listAccommodations(tripId: string | number) { + async listAccommodations(tripId: string | number) { return this.db.all(` SELECT a.*, p.name as place_name, p.address as place_address, p.image_url as place_image, p.lat as place_lat, p.lng as place_lng, r.title as reservation_title @@ -256,7 +256,7 @@ export class AccommodationsService { `, tripId); } - validateAccommodationRefs(tripId: string | number, placeId?: number, startDayId?: number, endDayId?: number) { + async validateAccommodationRefs(tripId: string | number, placeId?: number, startDayId?: number, endDayId?: number) { const errors: { field: string; message: string }[] = []; if (placeId !== undefined) { const place = this.db.get('SELECT id FROM places WHERE id = ? AND trip_id = ?', placeId, tripId); @@ -294,7 +294,7 @@ export class AccommodationsService { * pulled it forward, and a night booked for ten in the morning sat behind a whole * day of unpinned stops, reached at a quarter past twelve. */ - private positionForCheckIn(dayId: number, checkIn: string | null | undefined, excludeId?: number): number { + private async positionForCheckIn(dayId: number, checkIn: string | null | undefined, excludeId?: number): Promise { if (!checkIn) return 0; // excludeId leaves the row being re-seated out of the chain it is measured // against. Without it a night parked at the end of the day can find itself. @@ -321,7 +321,7 @@ export class AccommodationsService { * without an hour are passed over in both directions: the night may have been * dragged past them on purpose, and a change of notes is no reason to undo that. */ - private seatedByCheckIn(dayId: number, ownId: number, checkIn: string | null | undefined): boolean { + private async seatedByCheckIn(dayId: number, ownId: number, checkIn: string | null | undefined): Promise { if (!checkIn) return true; const rows = this.db.all<{ id: number; at: string | null }>(` SELECT da.id, COALESCE(da.assignment_time, p.place_time, other.check_in) AS at @@ -345,7 +345,7 @@ export class AccommodationsService { * road trip. Only ever filled in when it is empty: a type the traveller picked * (a campsite, say) is theirs. */ - private stampLodging(placeId: number): PlaceWithTags | null { + private async stampLodging(placeId: number): Promise { const place = this.db.get<{ stop_type: string | null }>('SELECT stop_type FROM places WHERE id = ?', placeId); if (!place || place.stop_type) return null; this.db.run("UPDATE places SET stop_type = 'hotel' WHERE id = ?", placeId); @@ -359,11 +359,15 @@ export class AccommodationsService { * the n-th stop that has coordinates, not behind a row id. Stops without * coordinates are never routed and so never counted. */ - private stopOrders(dayIds: number[]): Map { - return new Map([...new Set(dayIds)].map((dayId): [number, number[]] => [dayId, this.locatedStopIds(dayId)])); + private async stopOrders(dayIds: number[]): Promise> { + const result = new Map(); + for (const dayId of new Set(dayIds)) { + result.set(dayId, await this.locatedStopIds(dayId)); + } + return result; } - private locatedStopIds(dayId: number): number[] { + private async locatedStopIds(dayId: number): Promise { return this.db.all<{ id: number }>(` SELECT da.id FROM day_assignments da JOIN places p ON p.id = da.place_id WHERE da.day_id = ? AND p.lat IS NOT NULL AND p.lng IS NOT NULL @@ -385,9 +389,9 @@ export class AccommodationsService { * * Runs inside the caller's transaction. */ - private reanchorVias(mirror: AccommodationMirror, before: Map): void { + private async reanchorVias(mirror: AccommodationMirror, before: Map): Promise { for (const [dayId, previousIds] of before) { - const nextIds = this.locatedStopIds(dayId); + const nextIds = await this.locatedStopIds(dayId); if (previousIds.length === nextIds.length && previousIds.every((id, i) => id === nextIds[i])) continue; const vias = this.db.all('SELECT id, after_order_index, sequence FROM roadtrip_vias WHERE day_id = ?', dayId); if (!vias.length) continue; @@ -411,7 +415,7 @@ export class AccommodationsService { for (const viaId of remove) this.db.run('DELETE FROM roadtrip_vias WHERE id = ? AND day_id = ?', viaId, dayId); for (const via of moved) this.db.run('UPDATE roadtrip_vias SET after_order_index = ? WHERE id = ? AND day_id = ?', via.after_order_index, via.id, dayId); - this.renumberMergedLegs(dayId, vias.filter(via => !remove.includes(via.id)), moved); + await this.renumberMergedLegs(dayId, vias.filter(via => !remove.includes(via.id)), moved); this.noteVias(mirror, { dayId, vias: this.db.all( @@ -445,7 +449,7 @@ export class AccommodationsService { * `RoadtripService.reanchor` does it, the earlier leg's points first, then by * their old sequence, then by id. Only a leg that received a via is touched. */ - private renumberMergedLegs(dayId: number, kept: PinnedVia[], moved: { id: number; after_order_index: number }[]): void { + private async renumberMergedLegs(dayId: number, kept: PinnedVia[], moved: { id: number; after_order_index: number }[]): Promise { const landed = new Map(moved.map(via => [via.id, via.after_order_index])); const byLeg = new Map(); for (const via of kept) { @@ -506,7 +510,7 @@ export class AccommodationsService { const end = (max.max !== null ? max.max : -1) + 1; this.db.run('UPDATE day_assignments SET day_id = ?, place_id = ?, order_index = ? WHERE id = ?', dayId, placeId, end, stop.id); - const seat = this.positionForCheckIn(dayId, checkIn, stop.id); + const seat = await this.positionForCheckIn(dayId, checkIn, stop.id); if (seat < end) { this.db.run( 'UPDATE day_assignments SET order_index = order_index + 1 WHERE day_id = ? AND order_index >= ? AND id != ?', @@ -524,7 +528,7 @@ export class AccommodationsService { // form writes stays that never had one. Nothing to put on the map then. if (!placeId) return mirror; - mirror.stamped = this.stampLodging(placeId); + mirror.stamped = await this.stampLodging(placeId); // The road-trip flow assigns the place to the day and only then books the night. // Claiming that row would make cancelling the booking delete a stop the traveller @@ -532,7 +536,7 @@ export class AccommodationsService { // answer for a place already planned for that day by hand. if (this.db.get('SELECT id FROM day_assignments WHERE day_id = ? AND place_id = ?', dayId, placeId)) return mirror; - const before = this.stopOrders([dayId]); + const before = await this.stopOrders([dayId]); // Through AssignmentsService, seated where the check-in says (see // positionForCheckIn), with everything behind it moved up one. // @@ -543,14 +547,14 @@ export class AccommodationsService { // time until the next reload. mirror.created = await this.assignments.createAssignment(dayId, placeId, null, { accommodationId, - orderIndex: this.positionForCheckIn(dayId, checkIn), + orderIndex: await this.positionForCheckIn(dayId, checkIn), }); - this.reanchorVias(mirror, before); + await this.reanchorVias(mirror, before); return mirror; } /** The day stops this booking, and only this booking, put on the plan. */ - private ownStops(accommodationId: number) { + private async ownStops(accommodationId: number) { return this.db.all<{ id: number; day_id: number; place_id: number; order_index: number }>( 'SELECT id, day_id, place_id, order_index FROM day_assignments WHERE accommodation_id = ?', accommodationId ); @@ -572,8 +576,8 @@ export class AccommodationsService { */ private async releaseStops(accommodationId: number, opts: { keepStop?: boolean }): Promise { const mirror = noMirror(); - const own = this.ownStops(accommodationId); - const before = this.stopOrders(own.map(stop => stop.day_id)); + const own = await this.ownStops(accommodationId); + const before = await this.stopOrders(own.map(stop => stop.day_id)); for (const stop of own) { if (opts.keepStop) { this.db.run('UPDATE day_assignments SET accommodation_id = NULL WHERE id = ?', stop.id); @@ -587,7 +591,7 @@ export class AccommodationsService { this.db.run('DELETE FROM day_assignments WHERE id = ?', stop.id); mirror.removed.push({ id: stop.id, dayId: stop.day_id }); } - this.reanchorVias(mirror, before); + await this.reanchorVias(mirror, before); return mirror; } @@ -610,7 +614,7 @@ export class AccommodationsService { checkIn?: string | null, opts: { checkInChanged?: boolean } = {}, ): Promise { - const own = this.ownStops(accommodationId); + const own = await this.ownStops(accommodationId); if (own.length === 0) return noMirror(); if (own.length === 1 && own[0].day_id === dayId && own[0].place_id === placeId) { // Same place, same day. A check-in given a new hour seats the night afresh, the @@ -618,8 +622,8 @@ export class AccommodationsService { // it is unless the clocks around it say it is in the wrong place, so a night the // traveller dragged somewhere stays there through a change of notes. const settled = opts.checkInChanged - ? this.positionForCheckIn(dayId, checkIn, own[0].id) === own[0].order_index - : this.seatedByCheckIn(dayId, own[0].id, checkIn); + ? (await this.positionForCheckIn(dayId, checkIn, own[0].id)) === own[0].order_index + : await this.seatedByCheckIn(dayId, own[0].id, checkIn); if (settled) return noMirror(); } @@ -630,22 +634,22 @@ export class AccommodationsService { // note, its hour, its end-of-day flag and the road-trip day boundary anchored // on its id, all of which a DELETE takes with it. if (own.length === 1 && placeId) { - const before = this.stopOrders([own[0].day_id, dayId]); + const before = await this.stopOrders([own[0].day_id, dayId]); const moved = await this.relocateOwnStop(own[0], placeId, dayId, checkIn); if (moved) { mirror.moved = { assignment: moved, oldDayId: own[0].day_id }; - mirror.stamped = this.stampLodging(placeId); - this.reanchorVias(mirror, before); + mirror.stamped = await this.stampLodging(placeId); + await this.reanchorVias(mirror, before); return mirror; } } - const beforeRebuild = this.stopOrders(own.map(stop => stop.day_id)); + const beforeRebuild = await this.stopOrders(own.map(stop => stop.day_id)); for (const stop of own) { this.db.run('DELETE FROM day_assignments WHERE id = ?', stop.id); mirror.removed.push({ id: stop.id, dayId: stop.day_id }); } - this.reanchorVias(mirror, beforeRebuild); + await this.reanchorVias(mirror, beforeRebuild); const fresh = await this.mirrorStay(accommodationId, placeId, dayId, checkIn); mirror.created = fresh.created; mirror.stamped = fresh.stamped; @@ -685,10 +689,10 @@ export class AccommodationsService { return { accommodationId: newId, mirror: await this.mirrorStay(Number(newId), place_id ?? null, start_day_id, check_in) }; }); - return { accommodation: this.getAccommodationWithPlace(written.accommodationId), mirror: written.mirror }; + return { accommodation: await this.getAccommodationWithPlace(written.accommodationId), mirror: written.mirror }; } - getAccommodation(id: string | number, tripId: string | number) { + async getAccommodation(id: string | number, tripId: string | number) { return this.db.get('SELECT * FROM day_accommodations WHERE id = ? AND trip_id = ?', id, tripId); } @@ -731,7 +735,7 @@ export class AccommodationsService { JSON.stringify(meta), newConfirmation || null, res.id); } - return { accommodation: this.getAccommodationWithPlace(Number(id)), mirror }; + return { accommodation: await this.getAccommodationWithPlace(Number(id)), mirror }; } /** diff --git a/server/src/nest/assignments/assignments.controller.ts b/server/src/nest/assignments/assignments.controller.ts index 09ce6f6a3..f075d78c6 100644 --- a/server/src/nest/assignments/assignments.controller.ts +++ b/server/src/nest/assignments/assignments.controller.ts @@ -49,7 +49,7 @@ export class DayAssignmentsController { @Get() async list(@CurrentUser() user: User, @Param('tripId') tripId: string, @Param('dayId') dayId: string) { - if (!this.assignments.dayExists(dayId, tripId)) { + if (!await this.assignments.dayExists(dayId, tripId)) { throw new HttpException({ error: 'Day not found' }, 404); } return { assignments: await this.assignments.listDayAssignments(dayId) }; @@ -64,10 +64,10 @@ export class DayAssignmentsController { @Body() body: AssignmentCreateDto, @Headers('x-socket-id') socketId?: string, ) { - if (!this.assignments.dayExists(dayId, tripId)) { + if (!await this.assignments.dayExists(dayId, tripId)) { throw new HttpException({ error: 'Day not found' }, 404); } - if (!this.assignments.placeExists(body.place_id, tripId)) { + if (!await this.assignments.placeExists(body.place_id, tripId)) { throw new HttpException({ error: 'Place not found' }, 404); } const assignment = await this.assignments.createAssignment(dayId, body.place_id, body.notes); @@ -78,17 +78,17 @@ export class DayAssignmentsController { @RequirePermission('day_edit') @Put('reorder') - reorder( + async reorder( @CurrentUser() user: User, @Param('tripId') tripId: string, @Param('dayId') dayId: string, @Body() body: AssignmentReorderDto, @Headers('x-socket-id') socketId?: string, ) { - if (!this.assignments.dayExists(dayId, tripId)) { + if (!await this.assignments.dayExists(dayId, tripId)) { throw new HttpException({ error: 'Day not found' }, 404); } - this.assignments.reorderAssignments(dayId, body.orderedIds); + await this.assignments.reorderAssignments(dayId, body.orderedIds); this.assignments.broadcast(tripId, 'assignment:reordered', { dayId: Number(dayId), orderedIds: body.orderedIds }, socketId); return { success: true }; } @@ -102,10 +102,10 @@ export class DayAssignmentsController { @Param('id') id: string, @Headers('x-socket-id') socketId?: string, ) { - if (!this.assignments.assignmentExistsInDay(id, dayId, tripId)) { + if (!await this.assignments.assignmentExistsInDay(id, dayId, tripId)) { throw new HttpException({ error: 'Assignment not found' }, 404); } - this.assignments.deleteAssignment(id); + await this.assignments.deleteAssignment(id); this.assignments.broadcast(tripId, 'assignment:deleted', { assignmentId: Number(id), dayId: Number(dayId) }, socketId); await this.assignments.reconcile(tripId, socketId); return { success: true }; @@ -135,10 +135,10 @@ export class AssignmentOpsController { @Body() body: AssignmentMoveDto, @Headers('x-socket-id') socketId?: string, ) { - if (!this.assignments.getAssignmentForTrip(id, tripId)) { + if (!await this.assignments.getAssignmentForTrip(id, tripId)) { throw new HttpException({ error: 'Assignment not found' }, 404); } - if (!this.assignments.dayExists(String(body.new_day_id), tripId)) { + if (!await this.assignments.dayExists(String(body.new_day_id), tripId)) { throw new HttpException({ error: 'Target day not found' }, 404); } const { assignment, oldDayId } = await this.assignments.moveAssignment(id, body.new_day_id, body.order_index); @@ -148,11 +148,11 @@ export class AssignmentOpsController { } @Get(':id/participants') - participants(@CurrentUser() user: User, @Param('tripId') tripId: string, @Param('id') id: string) { - if (!this.assignments.getAssignmentForTrip(id, tripId)) { + async participants(@CurrentUser() user: User, @Param('tripId') tripId: string, @Param('id') id: string) { + if (!await this.assignments.getAssignmentForTrip(id, tripId)) { throw new HttpException({ error: 'Assignment not found' }, 404); } - return { participants: this.assignments.getParticipants(id) }; + return { participants: await this.assignments.getParticipants(id) }; } @RequirePermission('day_edit') @@ -164,7 +164,7 @@ export class AssignmentOpsController { @Body() body: AssignmentTimeDto, @Headers('x-socket-id') socketId?: string, ) { - if (!this.assignments.getAssignmentForTrip(id, tripId)) { + if (!await this.assignments.getAssignmentForTrip(id, tripId)) { throw new HttpException({ error: 'Assignment not found' }, 404); } const { assignment, reordered, vias } = await this.assignments.updateTime(id, body.place_time, body.end_time); @@ -190,7 +190,7 @@ export class AssignmentOpsController { @Body() body: AssignmentEndDayDto, @Headers('x-socket-id') socketId?: string, ) { - if (!this.assignments.getAssignmentForTrip(id, tripId)) { + if (!await this.assignments.getAssignmentForTrip(id, tripId)) { throw new HttpException({ error: 'Assignment not found' }, 404); } const assignment = await this.assignments.setEndDay(id, body.end_day); @@ -210,7 +210,7 @@ export class AssignmentOpsController { @Body() body: AssignmentNotesDto, @Headers('x-socket-id') socketId?: string, ) { - if (!this.assignments.getAssignmentForTrip(id, tripId)) { + if (!await this.assignments.getAssignmentForTrip(id, tripId)) { throw new HttpException({ error: 'Assignment not found' }, 404); } const assignment = await this.assignments.updateNotes(id, body.notes); @@ -227,7 +227,7 @@ export class AssignmentOpsController { @Body() body: AssignmentTransportDto, @Headers('x-socket-id') socketId?: string, ) { - if (!this.assignments.getAssignmentForTrip(id, tripId)) { + if (!await this.assignments.getAssignmentForTrip(id, tripId)) { throw new HttpException({ error: 'Assignment not found' }, 404); } const assignment = body.direction === 'incoming' @@ -239,17 +239,17 @@ export class AssignmentOpsController { @RequirePermission('day_edit') @Put(':id/participants') - setParticipants( + async setParticipants( @CurrentUser() user: User, @Param('tripId') tripId: string, @Param('id') id: string, @Body() body: AssignmentParticipantsDto, @Headers('x-socket-id') socketId?: string, ) { - if (!this.assignments.getAssignmentForTrip(id, tripId)) { + if (!await this.assignments.getAssignmentForTrip(id, tripId)) { throw new HttpException({ error: 'Assignment not found' }, 404); } - const participants = this.assignments.setParticipants(id, body.user_ids, tripId); + const participants = await this.assignments.setParticipants(id, body.user_ids, tripId); this.assignments.broadcast(tripId, 'assignment:participants', { assignmentId: Number(id), participants }, socketId); return { participants }; } diff --git a/server/src/nest/assignments/assignments.mcp.ts b/server/src/nest/assignments/assignments.mcp.ts index ca999d715..67ff4c91f 100644 --- a/server/src/nest/assignments/assignments.mcp.ts +++ b/server/src/nest/assignments/assignments.mcp.ts @@ -47,10 +47,10 @@ export class AssignmentsMcp { ctx: McpContext, ) { if (await this.auth.isDemoUser(ctx.userId)) return demoDenied(); - if (!this.assignments.verifyTripAccess(tripId, ctx.userId)) return noAccess(); + if (!(await this.assignments.verifyTripAccess(tripId, ctx.userId))) return noAccess(); if (!(await this.guards.hasTripPermission('day_edit', tripId, ctx.userId))) return permissionDenied(); - if (!this.assignments.dayExists(dayId, tripId)) return errorResult('Day not found.'); - if (!this.assignments.placeExists(placeId, tripId)) return errorResult('Place not found.'); + if (!(await this.assignments.dayExists(dayId, tripId))) return errorResult('Day not found.'); + if (!(await this.assignments.placeExists(placeId, tripId))) return errorResult('Place not found.'); const assignment = await this.assignments.createAssignment(dayId, placeId, notes || null); this.guards.safeBroadcast(tripId, 'assignment:created', { assignment }); await this.assignments.reconcile(tripId); @@ -73,9 +73,9 @@ export class AssignmentsMcp { ctx: McpContext, ) { if (await this.auth.isDemoUser(ctx.userId)) return demoDenied(); - if (!this.assignments.verifyTripAccess(tripId, ctx.userId)) return noAccess(); + if (!(await this.assignments.verifyTripAccess(tripId, ctx.userId))) return noAccess(); if (!(await this.guards.hasTripPermission('day_edit', tripId, ctx.userId))) return permissionDenied(); - if (!this.assignments.getAssignmentForTrip(assignmentId, tripId)) return errorResult('Assignment not found.'); + if (!(await this.assignments.getAssignmentForTrip(assignmentId, tripId))) return errorResult('Assignment not found.'); const assignment = await this.assignments.setEndDay(assignmentId, end_day); this.guards.safeBroadcast(tripId, 'assignment:updated', { assignment }); return ok({ assignment }); @@ -97,11 +97,11 @@ export class AssignmentsMcp { ctx: McpContext, ) { if (await this.auth.isDemoUser(ctx.userId)) return demoDenied(); - if (!this.assignments.verifyTripAccess(tripId, ctx.userId)) return noAccess(); + if (!(await this.assignments.verifyTripAccess(tripId, ctx.userId))) return noAccess(); if (!(await this.guards.hasTripPermission('day_edit', tripId, ctx.userId))) return permissionDenied(); - if (!this.assignments.assignmentExistsInDay(assignmentId, dayId, tripId)) + if (!(await this.assignments.assignmentExistsInDay(assignmentId, dayId, tripId))) return errorResult('Assignment not found.'); - this.assignments.deleteAssignment(assignmentId); + await this.assignments.deleteAssignment(assignmentId); this.guards.safeBroadcast(tripId, 'assignment:deleted', { assignmentId, dayId }); await this.assignments.reconcile(tripId); return ok({ success: true }); @@ -126,9 +126,9 @@ export class AssignmentsMcp { ctx: McpContext, ) { if (await this.auth.isDemoUser(ctx.userId)) return demoDenied(); - if (!this.assignments.verifyTripAccess(tripId, ctx.userId)) return noAccess(); + if (!(await this.assignments.verifyTripAccess(tripId, ctx.userId))) return noAccess(); if (!(await this.guards.hasTripPermission('day_edit', tripId, ctx.userId))) return permissionDenied(); - const existing = this.assignments.getAssignmentForTrip(assignmentId, tripId); + const existing = await this.assignments.getAssignmentForTrip(assignmentId, tripId); if (!existing) return errorResult('Assignment not found.'); const { assignment, reordered, vias } = await this.assignments.updateTime( assignmentId, @@ -162,9 +162,9 @@ export class AssignmentsMcp { ctx: McpContext, ) { if (await this.auth.isDemoUser(ctx.userId)) return demoDenied(); - if (!this.assignments.verifyTripAccess(tripId, ctx.userId)) return noAccess(); + if (!(await this.assignments.verifyTripAccess(tripId, ctx.userId))) return noAccess(); if (!(await this.guards.hasTripPermission('day_edit', tripId, ctx.userId))) return permissionDenied(); - if (!this.assignments.getAssignmentForTrip(assignmentId, tripId)) return errorResult('Assignment not found.'); + if (!(await this.assignments.getAssignmentForTrip(assignmentId, tripId))) return errorResult('Assignment not found.'); const assignment = await this.assignments.updateNotes(assignmentId, notes); this.guards.safeBroadcast(tripId, 'assignment:updated', { assignment }); return ok({ assignment }); @@ -189,9 +189,9 @@ export class AssignmentsMcp { ctx: McpContext, ) { if (await this.auth.isDemoUser(ctx.userId)) return demoDenied(); - if (!this.assignments.verifyTripAccess(tripId, ctx.userId)) return noAccess(); + if (!(await this.assignments.verifyTripAccess(tripId, ctx.userId))) return noAccess(); if (!(await this.guards.hasTripPermission('day_edit', tripId, ctx.userId))) return permissionDenied(); - if (!this.assignments.getAssignmentForTrip(assignmentId, tripId)) return errorResult('Assignment not found.'); + if (!(await this.assignments.getAssignmentForTrip(assignmentId, tripId))) return errorResult('Assignment not found.'); const assignment = direction === 'incoming' ? await this.assignments.setIncomingLegTransportMode(assignmentId, transport_mode ?? null) : await this.assignments.setLegTransportMode(assignmentId, transport_mode ?? null); @@ -219,9 +219,9 @@ export class AssignmentsMcp { ctx: McpContext, ) { if (await this.auth.isDemoUser(ctx.userId)) return demoDenied(); - if (!this.assignments.verifyTripAccess(tripId, ctx.userId)) return noAccess(); + if (!(await this.assignments.verifyTripAccess(tripId, ctx.userId))) return noAccess(); if (!(await this.guards.hasTripPermission('day_edit', tripId, ctx.userId))) return permissionDenied(); - if (!this.assignments.getAssignmentForTrip(assignmentId, tripId)) return errorResult('Assignment not found.'); + if (!(await this.assignments.getAssignmentForTrip(assignmentId, tripId))) return errorResult('Assignment not found.'); if (!(await this.days.getDay(newDayId, tripId))) return errorResult('Day not found.'); const result = await this.assignments.moveAssignment(assignmentId, newDayId, orderIndex ?? 0); // REST parity shape ({ assignment, oldDayId, newDayId }) — the client keys its @@ -246,9 +246,9 @@ export class AssignmentsMcp { { tripId, assignmentId }: { tripId: number; assignmentId: number }, ctx: McpContext, ) { - if (!this.assignments.verifyTripAccess(tripId, ctx.userId)) return noAccess(); - if (!this.assignments.getAssignmentForTrip(assignmentId, tripId)) return errorResult('Assignment not found.'); - const participants = this.assignments.getParticipants(assignmentId); + if (!(await this.assignments.verifyTripAccess(tripId, ctx.userId))) return noAccess(); + if (!(await this.assignments.getAssignmentForTrip(assignmentId, tripId))) return errorResult('Assignment not found.'); + const participants = await this.assignments.getParticipants(assignmentId); return ok({ participants }); } @@ -268,10 +268,10 @@ export class AssignmentsMcp { ctx: McpContext, ) { if (await this.auth.isDemoUser(ctx.userId)) return demoDenied(); - if (!this.assignments.verifyTripAccess(tripId, ctx.userId)) return noAccess(); + if (!(await this.assignments.verifyTripAccess(tripId, ctx.userId))) return noAccess(); if (!(await this.guards.hasTripPermission('day_edit', tripId, ctx.userId))) return permissionDenied(); - if (!this.assignments.getAssignmentForTrip(assignmentId, tripId)) return errorResult('Assignment not found.'); - const participants = this.assignments.setParticipants(assignmentId, userIds, tripId); + if (!(await this.assignments.getAssignmentForTrip(assignmentId, tripId))) return errorResult('Assignment not found.'); + const participants = await this.assignments.setParticipants(assignmentId, userIds, tripId); this.guards.safeBroadcast(tripId, 'assignment:participants', { assignmentId, participants }); return ok({ participants }); } @@ -292,10 +292,10 @@ export class AssignmentsMcp { ctx: McpContext, ) { if (await this.auth.isDemoUser(ctx.userId)) return demoDenied(); - if (!this.assignments.verifyTripAccess(tripId, ctx.userId)) return noAccess(); + if (!(await this.assignments.verifyTripAccess(tripId, ctx.userId))) return noAccess(); if (!(await this.guards.hasTripPermission('day_edit', tripId, ctx.userId))) return permissionDenied(); if (!(await this.days.getDay(dayId, tripId))) return errorResult('Day not found.'); - this.assignments.reorderAssignments(dayId, assignmentIds); + await this.assignments.reorderAssignments(dayId, assignmentIds); // REST parity shape ({ dayId, orderedIds }) — the client only reads orderedIds, // so broadcasting the tool-input key name emptied the day for collaborators. this.guards.safeBroadcast(tripId, 'assignment:reordered', { dayId, orderedIds: assignmentIds }); diff --git a/server/src/nest/assignments/assignments.service.ts b/server/src/nest/assignments/assignments.service.ts index 96909a301..581fe644e 100644 --- a/server/src/nest/assignments/assignments.service.ts +++ b/server/src/nest/assignments/assignments.service.ts @@ -8,6 +8,7 @@ import { QueryHelpersService } from '../query-helpers/query-helpers.service'; import { formatAssignmentWithPlace } from '../common/rowShape'; import type { AssignmentRow, DayAssignment, User, Participant } from '../../types'; import { JourneyDomainService } from '../journey/journey-domain.service'; +import { UnitOfWork } from '../database/unit-of-work'; type Trip = TripAccess; @@ -64,9 +65,10 @@ export class AssignmentsService { private readonly realtime: RealtimeService, private readonly queryHelpers: QueryHelpersService, private readonly journey: JourneyDomainService, + private readonly uow: UnitOfWork, ) {} - verifyTripAccess(tripId: string | number, userId: number) { + async verifyTripAccess(tripId: string | number, userId: number) { return this.dbs.canAccessTrip(Number(tripId), userId); } @@ -157,11 +159,11 @@ export class AssignmentsService { }); } - dayExists(dayId: string | number, tripId: string | number) { + async dayExists(dayId: string | number, tripId: string | number) { return !!this.dbs.get('SELECT id FROM days WHERE id = ? AND trip_id = ?', dayId, tripId); } - placeExists(placeId: unknown, tripId: string | number) { + async placeExists(placeId: unknown, tripId: string | number) { return !!this.dbs.get('SELECT id FROM places WHERE id = ? AND trip_id = ?', placeId, tripId); } @@ -172,8 +174,8 @@ export class AssignmentsService { * stop that reaches it without its booking id is one the day list cannot tell * from a place the traveller added, so it draws the hotel a second time. */ - createAssignment(dayId: string | number, placeId: unknown, notes?: string | null, opts: { accommodationId?: number; orderIndex?: number } = {}) { - const result = this.dbs.transaction(() => { + async createAssignment(dayId: string | number, placeId: unknown, notes?: string | null, opts: { accommodationId?: number; orderIndex?: number } = {}) { + const result = await this.uow.transactional(async () => { const maxOrder = this.dbs.get<{ max: number | null }>('SELECT MAX(order_index) as max FROM day_assignments WHERE day_id = ?', dayId)!; const end = (maxOrder.max !== null ? maxOrder.max : -1) + 1; // Somewhere in the middle when the caller says so, which means everything from @@ -190,30 +192,30 @@ export class AssignmentsService { ); }); - return this.getAssignmentWithPlace(result.lastInsertRowid); + return await this.getAssignmentWithPlace(result.lastInsertRowid); } - assignmentExistsInDay(id: string | number, dayId: string | number, tripId: string | number) { + async assignmentExistsInDay(id: string | number, dayId: string | number, tripId: string | number) { return !!this.dbs.get( 'SELECT da.id FROM day_assignments da JOIN days d ON da.day_id = d.id WHERE da.id = ? AND da.day_id = ? AND d.trip_id = ?', id, dayId, tripId ); } - deleteAssignment(id: string | number): void { + async deleteAssignment(id: string | number): Promise { this.dbs.run('DELETE FROM day_assignments WHERE id = ?', id); } - reorderAssignments(dayId: string | number, orderedIds: number[]): void { + async reorderAssignments(dayId: string | number, orderedIds: number[]): Promise { const update = this.dbs.prepare('UPDATE day_assignments SET order_index = ? WHERE id = ? AND day_id = ?'); - this.dbs.transaction(() => { + await this.uow.transactional(async () => { orderedIds.forEach((id: number, index: number) => { update.run(index, id, dayId); }); }); } - getAssignmentForTrip(id: string | number, tripId: string | number) { + async getAssignmentForTrip(id: string | number, tripId: string | number) { return this.dbs.get(` SELECT da.* FROM day_assignments da JOIN days d ON da.day_id = d.id @@ -224,7 +226,7 @@ export class AssignmentsService { async moveAssignment(id: string | number, newDayId: unknown, orderIndex: number | null | undefined) { // The source day comes from the row, not the caller — callers can't lie // about (or race on) where the assignment was. - const oldDayId = this.dbs.transaction(() => { + const oldDayId = await this.uow.transactional(async () => { const row = this.dbs.get<{ day_id: number }>('SELECT day_id FROM day_assignments WHERE id = ?', id); this.dbs.run('UPDATE day_assignments SET day_id = ?, order_index = ? WHERE id = ?', newDayId, orderIndex ?? 0, id); return row?.day_id; @@ -233,7 +235,7 @@ export class AssignmentsService { return { assignment: updated, oldDayId }; } - getParticipants(assignmentId: string | number) { + async getParticipants(assignmentId: string | number) { return this.dbs.all(` SELECT ap.user_id, COALESCE(u.display_name, u.username) AS username, u.avatar FROM assignment_participants ap @@ -256,7 +258,7 @@ export class AssignmentsService { * same. */ async updateTime(id: string | number, placeTime: unknown, endTime: unknown): Promise { - const sorted = this.dbs.transaction(() => { + const sorted = await this.uow.transactional(async () => { const stored = this.dbs.get<{ day_id: number; start: string | null }>(` SELECT da.day_id, COALESCE(da.assignment_time, p.place_time, acc.check_in) AS start FROM day_assignments da @@ -278,13 +280,15 @@ export class AssignmentsService { // cleared start leaves the day alone too. if (!placeTime || !stored) return null; if (sortMinutes(String(placeTime)) === sortMinutes(stored.start)) return null; - return this.sortDayByTime(stored.day_id); + return await this.sortDayByTime(stored.day_id); }); + const vias = sorted?.viasMoved ? { dayId: sorted.dayId, vias: await this.listDayVias(sorted.dayId) } : null; + return { assignment: await this.getAssignmentWithPlace(Number(id)), reordered: sorted ? { dayId: sorted.dayId, orderedIds: sorted.orderedIds } : null, - vias: sorted?.viasMoved ? { dayId: sorted.dayId, vias: this.listDayVias(sorted.dayId) } : null, + vias, }; } @@ -292,7 +296,7 @@ export class AssignmentsService { * Puts one day in time order. Writes nothing when it already is, which is the usual * case: most starts are typed in the order the day is planned. */ - private sortDayByTime(dayId: number): { dayId: number; orderedIds: number[]; viasMoved: boolean } | null { + private async sortDayByTime(dayId: number): Promise<{ dayId: number; orderedIds: number[]; viasMoved: boolean } | null> { // A booked night's hour lives on the booking, not on the stop: nobody types a // time into a hotel row, they type a check-in. Left out of this, the night // counted as untimed and stayed wherever it had been dropped, so pinning an @@ -321,7 +325,7 @@ export class AssignmentsService { if (row.order_index !== i) update.run(i, row.id); }); - return { dayId, orderedIds: sorted.map(row => row.id), viasMoved: this.reanchorVias(dayId, rows, sorted) }; + return { dayId, orderedIds: sorted.map(row => row.id), viasMoved: await this.reanchorVias(dayId, rows, sorted) }; } /** @@ -334,7 +338,7 @@ export class AssignmentsService { * No sequence renumbering, unlike RoadtripService.reanchor: a reorder maps each leg * onto a different one, so two legs' vias never end up on the same leg. */ - private reanchorVias(dayId: number, before: DayStopRow[], after: DayStopRow[]): boolean { + private async reanchorVias(dayId: number, before: DayStopRow[], after: DayStopRow[]): Promise { const located = (rows: DayStopRow[]) => rows.filter(row => row.located).map(row => row.id); const previousIds = located(before); const nextIds = located(after); @@ -364,7 +368,7 @@ export class AssignmentsService { * The day's vias in the shape the road trip routes broadcast them. RoadtripService * has this query too, but its module imports this one, so it cannot be injected here. */ - private listDayVias(dayId: number): RoadtripVia[] { + private async listDayVias(dayId: number): Promise { return this.dbs.all( `SELECT id, day_id, after_order_index, sequence, lat, lng, created_at FROM roadtrip_vias @@ -374,9 +378,9 @@ export class AssignmentsService { ); } - setEndDay(id: string | number, endDay: boolean) { + async setEndDay(id: string | number, endDay: boolean) { this.dbs.run('UPDATE day_assignments SET end_day = ? WHERE id = ?', endDay ? 1 : 0, id); - return this.getAssignmentWithPlace(Number(id)); + return await this.getAssignmentWithPlace(Number(id)); } /** @@ -387,9 +391,9 @@ export class AssignmentsService { * journey reconcile: the note affects neither the day order nor the skeleton * mirror (same as the transport-mode writes). */ - updateNotes(id: string | number, notes: string | null | undefined) { + async updateNotes(id: string | number, notes: string | null | undefined) { this.dbs.run('UPDATE day_assignments SET notes = ? WHERE id = ?', notes || null, id); - return this.getAssignmentWithPlace(Number(id)); + return await this.getAssignmentWithPlace(Number(id)); } /** @@ -398,9 +402,9 @@ export class AssignmentsService { * sticky by design: changing the whole-day default never touches a leg that * carries its own explicit mode. */ - setLegTransportMode(id: string | number, mode: string | null) { + async setLegTransportMode(id: string | number, mode: string | null) { this.dbs.run('UPDATE day_assignments SET leg_transport_mode = ? WHERE id = ?', mode ?? null, id); - return this.getAssignmentWithPlace(Number(id)); + return await this.getAssignmentWithPlace(Number(id)); } /** @@ -409,9 +413,9 @@ export class AssignmentsService { * when the previous timeline element is a place (the column is only read for * non-place origins like a booking arrival or a morning hotel departure). */ - setIncomingLegTransportMode(id: string | number, mode: string | null) { + async setIncomingLegTransportMode(id: string | number, mode: string | null) { this.dbs.run('UPDATE day_assignments SET incoming_leg_transport_mode = ? WHERE id = ?', mode ?? null, id); - return this.getAssignmentWithPlace(Number(id)); + return await this.getAssignmentWithPlace(Number(id)); } /** @@ -421,10 +425,10 @@ export class AssignmentsService { * the participants box sends the whole list back on every edit, so rejecting * the request would strand a trip whose membership changed underneath it. */ - setParticipants(assignmentId: string | number, userIds: number[], tripId: string | number) { + async setParticipants(assignmentId: string | number, userIds: number[], tripId: string | number) { const roster = this.dbs.rosterUserIds(tripId); const scoped = userIds.filter(id => roster.has(id)); - this.dbs.transaction(() => { + await this.uow.transactional(async () => { this.dbs.run('DELETE FROM assignment_participants WHERE assignment_id = ?', assignmentId); if (scoped.length > 0) { const insert = this.dbs.prepare('INSERT OR IGNORE INTO assignment_participants (assignment_id, user_id) VALUES (?, ?)'); diff --git a/server/src/nest/assignments/itinerary.rpc.ts b/server/src/nest/assignments/itinerary.rpc.ts index a3823a8ec..eecd0bbd6 100644 --- a/server/src/nest/assignments/itinerary.rpc.ts +++ b/server/src/nest/assignments/itinerary.rpc.ts @@ -32,8 +32,8 @@ export class ItineraryRpc { const actor = this.guards.requireActor(ctx, 'itinerary'); const notes = params.notes === undefined || params.notes === null ? null : str(params.notes, 'notes'); await this.guards.requireTripEdit(tripId, actor, DAY_EDIT_ACTION); - if (!this.assignments.dayExists(dayId, tripId)) throw new ForbiddenResource(`no day ${dayId} on trip ${tripId}`); - if (!this.assignments.placeExists(placeId, tripId)) throw new ForbiddenResource(`no place ${placeId} on trip ${tripId}`); + if (!(await this.assignments.dayExists(dayId, tripId))) throw new ForbiddenResource(`no day ${dayId} on trip ${tripId}`); + if (!(await this.assignments.placeExists(placeId, tripId))) throw new ForbiddenResource(`no place ${placeId} on trip ${tripId}`); const assignment = await this.assignments.createAssignment(dayId, placeId, notes); this.realtime.broadcast(tripId, 'assignment:created', { assignment }); await this.assignments.reconcile(tripId); @@ -46,9 +46,9 @@ export class ItineraryRpc { const assignmentId = num(params.assignmentId, 'assignmentId'); const actor = this.guards.requireActor(ctx, 'itinerary'); await this.guards.requireTripEdit(tripId, actor, DAY_EDIT_ACTION); - const existing = this.assignments.getAssignmentForTrip(assignmentId, tripId); + const existing = await this.assignments.getAssignmentForTrip(assignmentId, tripId); if (!existing) throw new ForbiddenResource(`no assignment ${assignmentId} on trip ${tripId}`); - this.assignments.deleteAssignment(assignmentId); + await this.assignments.deleteAssignment(assignmentId); // The dayId is what the client reducer keys the eviction on. Without it nothing // leaves the day, so keep the payload shape identical to the REST/MCP delete. this.realtime.broadcast(tripId, 'assignment:deleted', { assignmentId, dayId: existing.day_id }); diff --git a/server/src/nest/booking-import/booking-import.service.ts b/server/src/nest/booking-import/booking-import.service.ts index 022c5dffd..f8f392f4b 100644 --- a/server/src/nest/booking-import/booking-import.service.ts +++ b/server/src/nest/booking-import/booking-import.service.ts @@ -252,7 +252,7 @@ export class BookingImportService { } } - const place = this.places.create(tripId, { + const place = await this.places.create(tripId, { name: _venue.name, lat, lng, diff --git a/server/src/nest/integrations/dawarich-suggestions.service.ts b/server/src/nest/integrations/dawarich-suggestions.service.ts index 193ac7eb1..e45286778 100644 --- a/server/src/nest/integrations/dawarich-suggestions.service.ts +++ b/server/src/nest/integrations/dawarich-suggestions.service.ts @@ -225,7 +225,7 @@ export class DawarichSuggestionsService { if (body.dayId !== undefined) { await this.requirePermission('day_edit', access.user_id, userId); - if (!this.assignments.dayExists(body.dayId, tripId)) { + if (!(await this.assignments.dayExists(body.dayId, tripId))) { throw new AcceptError('day_not_on_trip', 'Day does not belong to this trip', 400); } } @@ -238,7 +238,7 @@ export class DawarichSuggestionsService { // worse than none: the stay would come back as unhandled while the place it // already produced sat on the trip. const { created, placeId, assignment } = await this.uow.transactional(async () => { - const place = this.places.create(String(tripId), { + const place = await this.places.create(String(tripId), { name: body.name?.trim() || row.name, lat, lng, diff --git a/server/src/nest/places/places.controller.ts b/server/src/nest/places/places.controller.ts index b81879594..3f083d555 100644 --- a/server/src/nest/places/places.controller.ts +++ b/server/src/nest/places/places.controller.ts @@ -118,15 +118,15 @@ export class PlacesController { private readonly storage: StorageService, ) {} - private requireTrip(tripId: string, user: User) { - const trip = this.places.verifyTripAccess(tripId, user.id); + private async requireTrip(tripId: string, user: User) { + const trip = await this.places.verifyTripAccess(tripId, user.id); if (!trip) { throw new HttpException({ error: 'Trip not found' }, 404); } return trip; } - private async requireEdit(trip: NonNullable>, user: User): Promise { + private async requireEdit(trip: NonNullable>>, user: User): Promise { if (!(await this.places.canEdit(trip, user))) { throw new HttpException({ error: 'No permission' }, 403); } @@ -151,12 +151,12 @@ export class PlacesController { @Body() body: PlaceCreateDto, @Headers('x-socket-id') socketId?: string, ) { - const trip = this.requireTrip(tripId, user); + const trip = await this.requireTrip(tripId, user); validateLengths(body); validateRouteColor(body); validateUrlFields(body); await this.requireEdit(trip, user); - const place = this.places.create(tripId, body as never); + const place = await this.places.create(tripId, body as never); this.places.broadcast(tripId, 'place:created', { place }, socketId); await this.places.onCreated(tripId, place.id); return { place }; @@ -171,7 +171,7 @@ export class PlacesController { @Body() body: PlaceImportGpxDto, @Headers('x-socket-id') socketId?: string, ) { - const trip = this.requireTrip(tripId, user); + const trip = await this.requireTrip(tripId, user); await this.requireEdit(trip, user); if (!file) { throw new HttpException({ error: 'No file uploaded' }, 400); @@ -182,7 +182,7 @@ export class PlacesController { if (!importWaypoints && !importRoutes && !importTracks) { throw new HttpException({ error: 'No import types selected' }, 400); } - const result = this.places.importGpx(tripId, file.buffer, { importWaypoints, importRoutes, importTracks, defaultName: file.originalname }); + const result = await this.places.importGpx(tripId, file.buffer, { importWaypoints, importRoutes, importTracks, defaultName: file.originalname }); if (!result) { throw new HttpException({ error: 'No matching places found in GPX file' }, 400); } @@ -198,20 +198,20 @@ export class PlacesController { * before the `:id` routes so the literal path wins. */ @Get('export.gpx') - exportGpx( + async exportGpx( @CurrentUser() user: User, @Param('tripId') tripId: string, @Query() query: PlaceExportGpxDto, @Res() res: Response, ) { - this.requireTrip(tripId, user); + await this.requireTrip(tripId, user); const waypoints = parseBool(query.waypoints, true); const tracks = parseBool(query.tracks, true); const dayRoutes = parseBool(query.dayRoutes, true); if (!waypoints && !tracks && !dayRoutes) { throw new HttpException({ error: 'No export types selected' }, 400); } - const result = this.places.exportGpx(tripId, { waypoints, tracks, dayRoutes }); + const result = await this.places.exportGpx(tripId, { waypoints, tracks, dayRoutes }); if (!result) { throw new HttpException({ error: 'Nothing to export' }, 404); } @@ -229,7 +229,7 @@ export class PlacesController { @Body() body: PlaceImportMapDto, @Headers('x-socket-id') socketId?: string, ) { - const trip = this.requireTrip(tripId, user); + const trip = await this.requireTrip(tripId, user); await this.requireEdit(trip, user); if (!file) { throw new HttpException({ error: 'No file uploaded' }, 400); @@ -267,7 +267,7 @@ export class PlacesController { /** Shared google/naver list import — identical flow, different provider + error string. */ private async importList(provider: 'google' | 'naver', user: User, tripId: string, body: PlaceImportListDto, socketId?: string) { - const trip = this.requireTrip(tripId, user); + const trip = await this.requireTrip(tripId, user); await this.requireEdit(trip, user); const { url, enrich } = body; // Opt-in: re-resolve each imported place via the Places API to fill in @@ -306,7 +306,7 @@ export class PlacesController { @Body() body: PlaceBulkDeleteDto, @Headers('x-socket-id') socketId?: string, ) { - const trip = this.requireTrip(tripId, user); + const trip = await this.requireTrip(tripId, user); await this.requireEdit(trip, user); const { ids } = body; if (ids.length === 0) { @@ -317,10 +317,10 @@ export class PlacesController { // journey entries even though removeMany below refuses it. Still ahead of // the DELETE — journey_entries.source_place_id is ON DELETE SET NULL, so // afterwards there is nothing left to detach. - const scoped = this.places.scopedIds(tripId, ids); + const scoped = await this.places.scopedIds(tripId, ids); for (const id of scoped) await this.places.onDeleted(id); // Read the linked expenses before the delete — afterwards the link is gone (#1298). - const expenseIds = this.places.linkedExpenseIds(tripId, scoped); + const expenseIds = await this.places.linkedExpenseIds(tripId, scoped); const { deleted, cancelled } = await this.places.removeMany(tripId, ids); for (const id of deleted) { this.places.broadcast(tripId, 'place:deleted', { placeId: id }, socketId); @@ -350,7 +350,7 @@ export class PlacesController { @Body() body: PlaceBulkUpdateDto, @Headers('x-socket-id') socketId?: string, ) { - const trip = this.requireTrip(tripId, user); + const trip = await this.requireTrip(tripId, user); await this.requireEdit(trip, user); const { ids } = body; if (ids.length === 0) { @@ -371,8 +371,8 @@ export class PlacesController { @Get(':id') @UseGuards(TripAccessGuard) - get(@CurrentUser() user: User, @Param('tripId') tripId: string, @Param('id') id: string) { - const place = this.places.get(tripId, id); + async get(@CurrentUser() user: User, @Param('tripId') tripId: string, @Param('id') id: string) { + const place = await this.places.get(tripId, id); if (!place) { throw new HttpException({ error: 'Place not found' }, 404); } @@ -389,7 +389,7 @@ export class PlacesController { @UploadedFile() file: Express.Multer.File | undefined, @Headers('x-socket-id') socketId?: string, ) { - const trip = this.requireTrip(tripId, user); + const trip = await this.requireTrip(tripId, user); await this.requireEdit(trip, user); // Inline rather than DemoWriteGuard: requireEdit above answers 404/403 for a // trip the caller cannot reach, and a guard would run before it. @@ -415,7 +415,7 @@ export class PlacesController { } @Put(':id/rating') - rate( + async rate( @CurrentUser() user: User, @Param('tripId') tripId: string, @Param('id') id: string, @@ -424,8 +424,8 @@ export class PlacesController { ) { // Deliberately no place_edit check: every trip member may cast their own // vote (#1435), even when an admin restricts place editing. - this.requireTrip(tripId, user); - const place = this.places.rate(tripId, id, user.id, body.rating); + await this.requireTrip(tripId, user); + const place = await this.places.rate(tripId, id, user.id, body.rating); if (!place) { throw new HttpException({ error: 'Place not found' }, 404); } @@ -435,8 +435,8 @@ export class PlacesController { @Delete(':id/rating') @UseGuards(TripAccessGuard) - unrate(@CurrentUser() user: User, @Param('tripId') tripId: string, @Param('id') id: string, @Headers('x-socket-id') socketId?: string) { - const place = this.places.rate(tripId, id, user.id, null); + async unrate(@CurrentUser() user: User, @Param('tripId') tripId: string, @Param('id') id: string, @Headers('x-socket-id') socketId?: string) { + const place = await this.places.rate(tripId, id, user.id, null); if (!place) { throw new HttpException({ error: 'Place not found' }, 404); } @@ -469,7 +469,7 @@ export class PlacesController { @Headers('x-socket-id') socketId?: string, @Headers('x-base-updated-at') ifMatch?: string, ) { - const trip = this.requireTrip(tripId, user); + const trip = await this.requireTrip(tripId, user); validateLengths(body); validateRouteColor(body); validateUrlFields(body); @@ -495,11 +495,11 @@ export class PlacesController { async remove(@CurrentUser() user: User, @Param('tripId') tripId: string, @Param('id') id: string, @Headers('x-socket-id') socketId?: string) { // Scope the id to the trip before the hook (see bulkDelete), then sync the // journey ahead of the actual delete. - if (!this.places.get(tripId, id)) { + if (!(await this.places.get(tripId, id))) { throw new HttpException({ error: 'Place not found' }, 404); } await this.places.onDeleted(Number(id)); - const expenseIds = this.places.linkedExpenseIds(tripId, [id]); + const expenseIds = await this.places.linkedExpenseIds(tripId, [id]); const { deleted, cancelled } = await this.places.remove(tripId, id); if (!deleted) { throw new HttpException({ error: 'Place not found' }, 404); diff --git a/server/src/nest/places/places.mcp.ts b/server/src/nest/places/places.mcp.ts index e6f9b9168..1fa345c34 100644 --- a/server/src/nest/places/places.mcp.ts +++ b/server/src/nest/places/places.mcp.ts @@ -99,7 +99,7 @@ export class PlacesMcp { if (await this.auth.isDemoUser(ctx.userId)) return demoDenied(); if (!this.db.canAccessTrip(tripId, ctx.userId)) return noAccess(); if (!(await this.guards.hasTripPermission('place_edit', tripId, ctx.userId))) return permissionDenied(); - const place = this.places.create(String(tripId), { name, description, lat, lng, address, category_id, google_place_id, google_ftid, osm_id, amap_poi_id, notes, website, phone, image_url, price, currency, stop_type }); + const place = await this.places.create(String(tripId), { name, description, lat, lng, address, category_id, google_place_id, google_ftid, osm_id, amap_poi_id, notes, website, phone, image_url, price, currency, stop_type }); this.guards.safeBroadcast(tripId, 'place:created', { place }); return ok({ place }); } @@ -144,10 +144,10 @@ export class PlacesMcp { if (await this.auth.isDemoUser(ctx.userId)) return demoDenied(); if (!this.db.canAccessTrip(tripId, ctx.userId)) return noAccess(); if (!(await this.guards.hasTripPermission('place_edit', tripId, ctx.userId))) return permissionDenied(); - if (!this.assignments.dayExists(dayId, tripId)) return { content: [{ type: 'text' as const, text: 'Day not found.' }], isError: true }; + if (!(await this.assignments.dayExists(dayId, tripId))) return { content: [{ type: 'text' as const, text: 'Day not found.' }], isError: true }; try { const result = await this.uow.transactional(async () => { - const place = this.places.create(String(tripId), { name, description, lat, lng, address, category_id, google_place_id, google_ftid, osm_id, amap_poi_id, notes: place_notes, website, phone, image_url, price, currency, stop_type }); + const place = await this.places.create(String(tripId), { name, description, lat, lng, address, category_id, google_place_id, google_ftid, osm_id, amap_poi_id, notes: place_notes, website, phone, image_url, price, currency, stop_type }); const assignment = await this.assignments.createAssignment(dayId, place.id, assignment_notes ?? null); return { place, assignment }; }); @@ -231,7 +231,7 @@ export class PlacesMcp { if (await this.auth.isDemoUser(ctx.userId)) return demoDenied(); // Rating is a personal vote — any trip member may cast one, place_edit not required. if (!this.db.canAccessTrip(tripId, ctx.userId)) return noAccess(); - const place = this.places.rate(String(tripId), String(placeId), ctx.userId, rating ?? null); + const place = await this.places.rate(String(tripId), String(placeId), ctx.userId, rating ?? null); if (!place) return { content: [{ type: 'text' as const, text: 'Place not found.' }], isError: true }; this.guards.safeBroadcast(tripId, 'place:updated', { place }); return ok({ place }); @@ -254,12 +254,12 @@ export class PlacesMcp { // Scope the id to the trip before the hook: onPlaceDeleted keys on the place // id alone, so a foreign id would detach that trip's journey entries even // though the delete below refuses it. - if (!this.places.get(String(tripId), String(placeId))) { + if (!(await this.places.get(String(tripId), String(placeId)))) { return { content: [{ type: 'text' as const, text: 'Place not found.' }], isError: true }; } try { await this.journey.onPlaceDeleted(placeId); } catch { /* non-fatal */ } // sync journeys before the row is gone // The link is gone once the place is, so read it first (#1298). - const expenseIds = this.places.linkedExpenseIds(tripId, [placeId]); + const expenseIds = await this.places.linkedExpenseIds(tripId, [placeId]); const { deleted, cancelled } = await this.places.remove(String(tripId), String(placeId)); if (!deleted) return { content: [{ type: 'text' as const, text: 'Place not found.' }], isError: true }; this.guards.safeBroadcast(tripId, 'place:deleted', { placeId }); @@ -378,7 +378,7 @@ export class PlacesMcp { if (!(await this.guards.hasTripPermission('place_edit', input.tripId, ctx.userId))) return permissionDenied(); if (!input.importWaypoints && !input.importRoutes && !input.importTracks) return errorResult('No import types selected.'); try { - const imported = this.places.importGpx(String(input.tripId), Buffer.from(input.gpx, 'utf8'), { importWaypoints: input.importWaypoints, importRoutes: input.importRoutes, importTracks: input.importTracks, defaultName: input.name }); + const imported = await this.places.importGpx(String(input.tripId), Buffer.from(input.gpx, 'utf8'), { importWaypoints: input.importWaypoints, importRoutes: input.importRoutes, importTracks: input.importTracks, defaultName: input.name }); if (!imported) return errorResult('No matching places found in GPX.'); for (const place of imported.places) this.guards.safeBroadcast(input.tripId, 'place:created', { place }); return ok(imported); @@ -408,7 +408,7 @@ export class PlacesMcp { // A read, like the REST route: seeing the trip is enough, no place_edit. if (!this.db.canAccessTrip(tripId, ctx.userId)) return noAccess(); if (!waypoints && !tracks && !dayRoutes) return errorResult('No export types selected.'); - const result = this.places.exportGpx(String(tripId), { waypoints, tracks, dayRoutes }); + const result = await this.places.exportGpx(String(tripId), { waypoints, tracks, dayRoutes }); if (!result) return errorResult('Nothing to export.'); return ok({ gpx: result.gpx, filename: result.filename }); } @@ -431,12 +431,12 @@ export class PlacesMcp { // Trip-scoped, and ahead of the DELETE: journey_entries.source_place_id is // ON DELETE SET NULL, so a hook that ran afterwards found nothing left to // detach and left the entries as orphans. - const scoped = this.places.scopedIds(String(tripId), placeIds); + const scoped = await this.places.scopedIds(String(tripId), placeIds); for (const id of scoped) { try { await this.journey.onPlaceDeleted(id); } catch { /* non-fatal */ } } // The link is gone once the places are, so read it first (#1298). - const expenseIds = this.places.linkedExpenseIds(tripId, scoped); + const expenseIds = await this.places.linkedExpenseIds(tripId, scoped); const { deleted, cancelled } = await this.places.removeMany(String(tripId), placeIds); for (const id of deleted) this.guards.safeBroadcast(tripId, 'place:deleted', { placeId: id }); // A night booked at this place went with it, and took its partner booking and diff --git a/server/src/nest/places/places.rpc.ts b/server/src/nest/places/places.rpc.ts index 3aae2749c..6a8002122 100644 --- a/server/src/nest/places/places.rpc.ts +++ b/server/src/nest/places/places.rpc.ts @@ -60,7 +60,7 @@ export class PlacesRpc { this.guards.capStrings(parsed.data as Record, PLACE_STR_LIMITS); capUrls(parsed.data as Record); await this.guards.requireTripEdit(tripId, actor, PLACE_EDIT_ACTION); - const place = this.places.create(String(tripId), parsed.data as unknown as PlaceCreateInput); + const place = await this.places.create(String(tripId), parsed.data as unknown as PlaceCreateInput); this.realtime.broadcast(tripId, 'place:created', { place }); await this.mirrorJourneys(() => this.journey.onPlaceCreated(tripId, place.id)); return place; @@ -95,7 +95,7 @@ export class PlacesRpc { // Scope the id to the trip before anything else: onPlaceDeleted keys on the place // alone, so an id belonging to a foreign trip would detach THAT trip's journey // entries even though the delete below refuses it. - if (!this.places.get(String(tripId), String(placeId))) { + if (!(await this.places.get(String(tripId), String(placeId)))) { throw new ForbiddenResource(`no place ${placeId} on trip ${tripId}`); } // Ahead of the DELETE, like the REST route and the MCP tool: @@ -103,7 +103,7 @@ export class PlacesRpc { // finds nothing left to detach and the entries linger as orphans. await this.mirrorJourneys(() => this.journey.onPlaceDeleted(placeId)); // The link is gone once the place is, so read it first (#1298). - const expenseIds = this.places.linkedExpenseIds(tripId, [placeId]); + const expenseIds = await this.places.linkedExpenseIds(tripId, [placeId]); // remove is async (it deletes the place's storage object): await it so the // refusal check sees the resolved answer, not an always-truthy Promise. const { deleted, cancelled } = await this.places.remove(String(tripId), String(placeId)); diff --git a/server/src/nest/places/places.service.ts b/server/src/nest/places/places.service.ts index aa9985753..c70ee4de2 100644 --- a/server/src/nest/places/places.service.ts +++ b/server/src/nest/places/places.service.ts @@ -137,7 +137,7 @@ export class PlacesService { private readonly uow: UnitOfWork, ) {} - verifyTripAccess(tripId: string, userId: number) { + async verifyTripAccess(tripId: string, userId: number) { return this.dbs.canAccessTrip(Number(tripId), userId); } @@ -157,7 +157,7 @@ export class PlacesService { * arrive from an older client that had no business knowing it existed, and the * read-back join hands `tags.user_id` straight to the caller. */ - private tagsOnTrip(tripId: string | number, tagIds: number[]): number[] { + private async tagsOnTrip(tripId: string | number, tagIds: number[]): Promise { const unique = [...new Set(tagIds)]; if (unique.length === 0) return []; const roster = this.dbs.rosterUserIds(tripId); @@ -240,7 +240,7 @@ export class PlacesService { // Create place // ------------------------------------------------------------------------- - create(tripId: string, body: PlaceCreateInput) { + async create(tripId: string, body: PlaceCreateInput) { const { name, description, lat, lng, address, category_id, price, currency, place_time, end_time, @@ -278,7 +278,7 @@ export class PlacesService { if (tags && tags.length > 0) { const insertTag = this.dbs.prepare('INSERT OR IGNORE INTO place_tags (place_id, tag_id) VALUES (?, ?)'); - for (const tagId of this.tagsOnTrip(tripId, tags)) { + for (const tagId of await this.tagsOnTrip(tripId, tags)) { insertTag.run(placeId, tagId); } } @@ -290,7 +290,7 @@ export class PlacesService { // Get single place // ------------------------------------------------------------------------- - get(tripId: string, placeId: string) { + async get(tripId: string, placeId: string) { const placeCheck = this.dbs.get('SELECT id FROM places WHERE id = ? AND trip_id = ?', placeId, tripId); if (!placeCheck) return null; return this.dbs.getPlaceWithTags(placeId); @@ -306,22 +306,22 @@ export class PlacesService { body: PlaceUpdateInput, ifMatch?: string, ): Promise { - const { result, reclaim } = this.applyUpdate(tripId, placeId, body, ifMatch); + const { result, reclaim } = await this.applyUpdate(tripId, placeId, body, ifMatch); if (reclaim !== undefined) await reclaimPlaceImage(this.storage, reclaim); return result; } /** - * The synchronous DB half of update(). Split out so updateMany() can run it - * inside a better-sqlite3 transaction (which cannot await) and settle the - * storage reclaims after the transaction commits. + * The DB half of update(). Split out so updateMany() can run it inside the + * same UnitOfWork transaction and settle the storage reclaims after the + * transaction commits. */ - private applyUpdate( + private async applyUpdate( tripId: string, placeId: string, body: PlaceUpdateInput, ifMatch?: string, - ): { result: PlaceWithTags | UpdateConflict | null; reclaim?: string | null } { + ): Promise<{ result: PlaceWithTags | UpdateConflict | null; reclaim?: string | null }> { const existingPlace = this.dbs.get('SELECT * FROM places WHERE id = ? AND trip_id = ?', placeId, tripId); if (!existingPlace) return { result: null }; @@ -405,7 +405,7 @@ export class PlacesService { this.dbs.run('DELETE FROM place_tags WHERE place_id = ?', placeId); if (tags.length > 0) { const insertTag = this.dbs.prepare('INSERT OR IGNORE INTO place_tags (place_id, tag_id) VALUES (?, ?)'); - for (const tagId of this.tagsOnTrip(tripId, tags)) { + for (const tagId of await this.tagsOnTrip(tripId, tags)) { insertTag.run(placeId, tagId); } } @@ -430,7 +430,7 @@ export class PlacesService { * budget:deleted events for the rows remove()/removeMany() are about to take * with them. Read it BEFORE deleting — afterwards the link is gone. */ - linkedExpenseIds(tripId: string | number, placeIds: Array): number[] { + async linkedExpenseIds(tripId: string | number, placeIds: Array): Promise { if (placeIds.length === 0) return []; const rows = this.dbs.all<{ id: number }>( `SELECT id FROM budget_items WHERE trip_id = ? AND place_id IN (${placeIds.map(() => '?').join(',')})`, @@ -518,7 +518,7 @@ export class PlacesService { * those key on the place id alone, so an id from another trip would detach * that trip's journey entries even though the delete itself refuses it. */ - scopedIds(tripId: string, ids: number[]): number[] { + async scopedIds(tripId: string, ids: number[]): Promise { if (ids.length === 0) return []; const placeholders = ids.map(() => '?').join(','); const rows = this.dbs.all<{ id: number }>( @@ -542,11 +542,11 @@ export class PlacesService { if (ids.length === 0) return []; const updated: PlaceWithTags[] = []; const reclaims: (string | null)[] = []; - this.dbs.transaction(() => { + await this.uow.transactional(async () => { for (const id of ids) { // Bulk update sends no If-Match, so applyUpdate() never returns a // conflict here; the guard keeps the types honest. - const { result: place, reclaim } = this.applyUpdate(tripId, String(id), body); + const { result: place, reclaim } = await this.applyUpdate(tripId, String(id), body); if (place && !isUpdateConflict(place)) updated.push(place); if (reclaim !== undefined) reclaims.push(reclaim); } @@ -562,7 +562,7 @@ export class PlacesService { // ------------------------------------------------------------------------- /** Build a lookup of names/coords for places already in a trip. */ - private buildDedupSet(tripId: string): DedupSet { + private async buildDedupSet(tripId: string): Promise { const rows = this.dbs.all<{ name: string | null; lat: number | null; lng: number | null; google_place_id: string | null; google_ftid: string | null; osm_id: string | null; amap_poi_id: string | null; @@ -595,8 +595,8 @@ export class PlacesService { * `google_ftid` for the bulk importer's backfill, which is a detail of that * caller and not part of the question "which place is this?". */ - findMatchingPlaceId(tripId: string, candidate: PlaceMatchCandidate): number | null { - return this.findDuplicatePlace(tripId, candidate)?.id ?? null; + async findMatchingPlaceId(tripId: string, candidate: PlaceMatchCandidate): Promise { + return (await this.findDuplicatePlace(tripId, candidate))?.id ?? null; } /** @@ -623,10 +623,10 @@ export class PlacesService { * `findMatchingPlaceId` wants — a booking with no place name should link to * the hotel that has one — so it is stated rather than removed. */ - private findDuplicatePlace( + private async findDuplicatePlace( tripId: string, place: PlaceMatchCandidate, - ): { id: number; google_ftid: string | null } | null { + ): Promise<{ id: number; google_ftid: string | null } | null> { for (const strategy of placeMatchStrategies(place)) { let hit: { id: number; google_ftid: string | null } | undefined; if (strategy.by === 'externalId') { @@ -663,9 +663,9 @@ export class PlacesService { // Import GPX // ------------------------------------------------------------------------- - importGpx(tripId: string, fileBuffer: Buffer, opts: GpxImportOptions = {}): GpxImportResult | null { - const result = this.importGpxRows(tripId, fileBuffer, opts); - this.colorizeImportedTracks(tripId, result); + async importGpx(tripId: string, fileBuffer: Buffer, opts: GpxImportOptions = {}): Promise { + const result = await this.importGpxRows(tripId, fileBuffer, opts); + await this.colorizeImportedTracks(tripId, result); return result; } @@ -678,7 +678,7 @@ export class PlacesService { * Returns null when the selection yields nothing, so the caller answers 404 rather * than handing over a file that imports as nothing on the other end. */ - exportGpx(tripId: string, opts: GpxExportOptions = {}): { gpx: string; filename: string } | null { + async exportGpx(tripId: string, opts: GpxExportOptions = {}): Promise<{ gpx: string; filename: string } | null> { const trip = this.dbs.get<{ title: string }>('SELECT title FROM trips WHERE id = ?', tripId); if (!trip) return null; @@ -717,7 +717,7 @@ export class PlacesService { return gpx ? { gpx, filename: gpxFilename(trip.title) } : null; } - private importGpxRows(tripId: string, fileBuffer: Buffer, opts: GpxImportOptions = {}): GpxImportResult | null { + private async importGpxRows(tripId: string, fileBuffer: Buffer, opts: GpxImportOptions = {}): Promise { const { importWaypoints = true, importRoutes = true, importTracks = true, defaultName } = opts; const parsed = gpxParser.parse(fileBuffer.toString('utf-8')); @@ -789,14 +789,14 @@ export class PlacesService { if (waypoints.length === 0) return null; - const dedup = this.buildDedupSet(tripId); + const dedup = await this.buildDedupSet(tripId); const insertStmt = this.dbs.prepare(` INSERT INTO places (trip_id, name, description, lat, lng, transport_mode, route_geometry) VALUES (?, ?, ?, ?, ?, 'walking', ?) `); const created: PlaceWithTags[] = []; let skipped = 0; - this.dbs.transaction(() => { + await this.uow.transactional(async () => { for (const wp of waypoints) { if (isPlaceDuplicate({ name: wp.name, lat: wp.lat, lng: wp.lng }, dedup)) { skipped++; @@ -818,7 +818,7 @@ export class PlacesService { async importMapFile(tripId: string, fileBuffer: Buffer, filename: string, opts: KmlImportOptions = {}): Promise { const result = await this.importMapFileRows(tripId, fileBuffer, filename, opts); - this.colorizeImportedTracks(tripId, result); + await this.colorizeImportedTracks(tripId, result); return result; } @@ -834,7 +834,7 @@ export class PlacesService { return this.importKmlPlaces(tripId, kmlBuffer, opts); } - importKmlPlaces(tripId: string, fileBuffer: Buffer, opts: KmlImportOptions = {}): PlaceImportResult { + async importKmlPlaces(tripId: string, fileBuffer: Buffer, opts: KmlImportOptions = {}): Promise { const { importPoints = true, importPaths = true } = opts; const decoded = decodeUtf8WithWarning(fileBuffer); @@ -859,7 +859,7 @@ export class PlacesService { const categories = this.dbs.all<{ id: number; name: string }>('SELECT id, name FROM categories'); const categoryLookup = buildCategoryNameLookup(categories); - const dedup = this.buildDedupSet(tripId); + const dedup = await this.buildDedupSet(tripId); const created: PlaceWithTags[] = []; let dupCount = 0; @@ -868,7 +868,7 @@ export class PlacesService { VALUES (?, ?, ?, ?, ?, ?, 'walking', ?) `); - this.dbs.transaction(() => { + await this.uow.transactional(async () => { let fallbackIndex = 1; for (const node of placemarkNodes) { const parsedPlacemark = parsePlacemarkNode(node); @@ -955,25 +955,24 @@ export class PlacesService { * it used to hand the same object back, which read like a transformation and * was none. */ - private colorizeImportedTracks(tripId: string, result: { places: ImportedPlace[] } | null): void { + private async colorizeImportedTracks(tripId: string, result: { places: ImportedPlace[] } | null): Promise { const tracks = result?.places?.filter((p) => p.route_geometry && !p.route_color) ?? []; if (tracks.length === 0) return; // Read and write in one transaction so two concurrent imports cannot both // read the same set of free colours. - this.dbs.transaction((conn) => { + await this.uow.transactional(async () => { const taken = new Set( - (conn - .prepare('SELECT DISTINCT route_color AS c FROM places WHERE trip_id = ? AND route_color IS NOT NULL') - .all(tripId) as { c: string }[]).map((r) => r.c), + this.dbs.all<{ c: string }>( + 'SELECT DISTINCT route_color AS c FROM places WHERE trip_id = ? AND route_color IS NOT NULL', tripId, + ).map((r) => r.c), ); const free = TRACK_COLORS.filter((c) => !taken.has(c)); - const stmt = conn.prepare('UPDATE places SET route_color = ? WHERE id = ?'); tracks.forEach((track, i) => { // Free ones first, then wrap through the whole palette — never reuse a // free colour twice within the same import. const color = i < free.length ? free[i] : TRACK_COLORS[(i - free.length) % TRACK_COLORS.length]; - stmt.run(color, track.id); + this.dbs.run('UPDATE places SET route_color = ? WHERE id = ?', color, track.id); track.route_color = color; }); }); @@ -1103,7 +1102,7 @@ export class PlacesService { return { error: 'No places with coordinates found in list', status: 400 }; } - const { created, skipped } = this.storeGooglePlaces(tripId, places); + const { created, skipped } = await this.storeGooglePlaces(tripId, places); if (created.length) { void this.enrichImportedList(tripId, created as EnrichablePlace[], opts); @@ -1119,11 +1118,11 @@ export class PlacesService { * the same rules — a place already on the trip is skipped rather than doubled, and a * row that matches but carries no provider id is given the one this import knows. */ - private storeGooglePlaces( + private async storeGooglePlaces( tripId: string, places: { name: string; lat: number; lng: number; notes: string | null; googleFtid: string | null }[], - ): { created: PlaceWithTags[]; skipped: number } { - const dedup = this.buildDedupSet(tripId); + ): Promise<{ created: PlaceWithTags[]; skipped: number }> { + const dedup = await this.buildDedupSet(tripId); const insertStmt = this.dbs.prepare(` INSERT INTO places (trip_id, name, lat, lng, notes, google_ftid, transport_mode) VALUES (?, ?, ?, ?, ?, ?, 'walking') @@ -1131,7 +1130,7 @@ export class PlacesService { const updateGoogleFtidStmt = this.dbs.prepare('UPDATE places SET google_ftid = ?, updated_at = CURRENT_TIMESTAMP WHERE id = ?'); const created: PlaceWithTags[] = []; let skipped = 0; - this.dbs.transaction(() => { + await this.uow.transactional(async () => { for (const p of places) { // One candidate for both halves. Passing the raw parser object to the SQL // half used to mean its provider id never arrived — the field is @@ -1140,7 +1139,7 @@ export class PlacesService { // this candidate's ftid on the backfill below. const candidate = { name: p.name, lat: p.lat, lng: p.lng, google_ftid: p.googleFtid }; if (isPlaceDuplicate(candidate, dedup)) { - const duplicate = this.findDuplicatePlace(tripId, candidate); + const duplicate = await this.findDuplicatePlace(tripId, candidate); if (duplicate && !duplicate.google_ftid && p.googleFtid) { updateGoogleFtidStmt.run(p.googleFtid, duplicate.id); } @@ -1246,7 +1245,7 @@ export class PlacesService { return { error: 'None of the stops in that link could be placed on the map.', status: 400 }; } - const { created, skipped } = this.storeGooglePlaces(tripId, places); + const { created, skipped } = await this.storeGooglePlaces(tripId, places); if (created.length) { void this.enrichImportedList(tripId, created as EnrichablePlace[], opts); } @@ -1374,14 +1373,14 @@ export class PlacesService { return { error: 'No places with coordinates found in list', status: 400 }; } - const dedup = this.buildDedupSet(tripId); + const dedup = await this.buildDedupSet(tripId); const insertStmt = this.dbs.prepare(` INSERT INTO places (trip_id, name, lat, lng, address, notes, transport_mode) VALUES (?, ?, ?, ?, ?, ?, 'walking') `); const created: PlaceWithTags[] = []; let skipped = 0; - this.dbs.transaction(() => { + await this.uow.transactional(async () => { for (const p of places) { if (isPlaceDuplicate({ name: p.name, lat: p.lat, lng: p.lng }, dedup)) { skipped++; @@ -1579,7 +1578,7 @@ export class PlacesService { * a vote must not 409 another member's offline edit. Returns the refreshed * place (with the new aggregate) or null when the place isn't in the trip. */ - rate(tripId: string, placeId: string, userId: number, rating: number | null): PlaceWithTags | null { + async rate(tripId: string, placeId: string, userId: number, rating: number | null): Promise { const place = this.dbs.get('SELECT id FROM places WHERE id = ? AND trip_id = ?', placeId, tripId); if (!place) return null; if (rating === null) { diff --git a/server/src/nest/public-api/public-api.controller.ts b/server/src/nest/public-api/public-api.controller.ts index f3e12cd30..5843f2257 100644 --- a/server/src/nest/public-api/public-api.controller.ts +++ b/server/src/nest/public-api/public-api.controller.ts @@ -66,10 +66,10 @@ export class PublicApiController { * this is the one list in TREK it can answer questions about. */ @Get('bucket-list') - listBucketList(@Req() req: Request): PublicApiBucketList { + async listBucketList(@Req() req: Request): Promise { this.limit(req); requireScope(req, 'bucket-list'); - return { items: this.api.listBucketList(requireUserId(req)) }; + return { items: await this.api.listBucketList(requireUserId(req)) }; } /** @@ -80,11 +80,11 @@ export class PublicApiController { * endpoint into a way to count someone else's trips. */ @Get('trips/:id') - getTrip( + async getTrip( @Req() req: Request, @Param('id') id: string, @Query('include') include?: string, - ): PublicApiTrip { + ): Promise { this.limit(req); requireScope(req, 'trips'); const tripId = parseTripId(id); @@ -102,7 +102,7 @@ export class PublicApiController { // The grant goes in as well as the narrowed include: days are implied by any // section that hangs off them, and the implied block must still be measured // against what the key may read. - const trip = this.api.getTrip(tripId, requireUserId(req), allowed, grantedScopes(req)); + const trip = await this.api.getTrip(tripId, requireUserId(req), allowed, grantedScopes(req)); if (!trip) { throw new HttpException({ error: 'Trip not found' }, 404); } diff --git a/server/src/nest/public-api/public-api.service.ts b/server/src/nest/public-api/public-api.service.ts index 022d132e7..d9223e067 100644 --- a/server/src/nest/public-api/public-api.service.ts +++ b/server/src/nest/public-api/public-api.service.ts @@ -61,7 +61,7 @@ export class PublicApiService { * turns both into the same 404, so the endpoint cannot be used to probe which * trip ids exist. */ - getTrip(tripId: number, userId: number, include: PublicApiInclude[], granted: readonly string[] = include): PublicApiTrip | null { + async getTrip(tripId: number, userId: number, include: PublicApiInclude[], granted: readonly string[] = include): Promise { if (!this.db.canAccessTrip(tripId, userId)) return null; const row = this.db.get( `SELECT id, title, description, start_date, end_date, currency, is_archived, updated_at @@ -80,19 +80,19 @@ export class PublicApiService { // through the container those places arrive in. The shell it does get is the // join key the children are useless without. if (DAY_SCOPED.some((section) => include.includes(section))) { - trip.days = this.buildDays(tripId, include, granted.includes('days')); + trip.days = await this.buildDays(tripId, include, granted.includes('days')); } if (include.includes('places')) { - trip.unplanned_places = this.buildUnplannedPlaces(tripId); + trip.unplanned_places = await this.buildUnplannedPlaces(tripId); } if (include.includes('reservations')) { - trip.unscheduled_reservations = this.buildUnscheduledReservations(tripId); + trip.unscheduled_reservations = await this.buildUnscheduledReservations(tripId); } if (include.includes('accommodations')) { - trip.accommodations = this.buildAccommodations(tripId); + trip.accommodations = await this.buildAccommodations(tripId); } if (include.includes('travellers')) { - trip.travellers = this.buildTravellers(tripId); + trip.travellers = await this.buildTravellers(tripId); } return trip; } @@ -105,7 +105,7 @@ export class PublicApiService { * rather than queried per day — a two-week trip would otherwise cost 42 round * trips for the same rows. */ - private buildDays(tripId: number, include: PublicApiInclude[], dayFields: boolean): PublicApiDay[] { + private async buildDays(tripId: number, include: PublicApiInclude[], dayFields: boolean): Promise { const days = this.db.all( `SELECT id, day_number, date, title, notes FROM days WHERE trip_id = ? ORDER BY day_number ASC`, @@ -113,10 +113,10 @@ export class PublicApiService { ); if (days.length === 0) return []; - const placesByDay = include.includes('places') ? this.placesByDay(tripId) : new Map(); - const notesByDay = include.includes('notes') ? this.dayNotesByDay(tripId) : new Map(); + const placesByDay = include.includes('places') ? await this.placesByDay(tripId) : new Map(); + const notesByDay = include.includes('notes') ? await this.dayNotesByDay(tripId) : new Map(); const reservationsByDay = include.includes('reservations') - ? this.reservationsByDay(tripId) + ? await this.reservationsByDay(tripId) : new Map(); return days.map((day) => ({ @@ -143,7 +143,7 @@ export class PublicApiService { * listed here as well it would read as two different intentions. Same rule as * the shortlist below. */ - private placesByDay(tripId: number): Map { + private async placesByDay(tripId: number): Promise> { const rows = this.db.all( `SELECT da.day_id, p.name, p.address, p.lat, p.lng, p.place_time, p.end_time, @@ -160,7 +160,7 @@ export class PublicApiService { return groupBy(rows, (r: PlaceRow) => r.day_id, toPlace); } - private dayNotesByDay(tripId: number): Map { + private async dayNotesByDay(tripId: number): Promise> { const rows = this.db.all( `SELECT day_id, text, time FROM day_notes WHERE trip_id = ? @@ -180,7 +180,7 @@ export class PublicApiService { * repeated on each — a consumer that sees the same flight on three days has no * way to tell that from three flights. */ - private reservationsByDay(tripId: number): Map { + private async reservationsByDay(tripId: number): Promise> { const rows = this.db.all( `SELECT day_id, type, title, location, reservation_time, reservation_end_time, status, notes @@ -198,7 +198,7 @@ export class PublicApiService { * Stored as day ids, reported as ISO dates: a consumer has no way to look up a * TREK day id, and the dates are what it actually needs to match its own nights. */ - private buildAccommodations(tripId: number): PublicApiAccommodation[] { + private async buildAccommodations(tripId: number): Promise { const rows = this.db.all( `SELECT p.name, p.address, p.lat, p.lng, ds.date AS start_date, de.date AS end_date, @@ -240,7 +240,7 @@ export class PublicApiService { * but it is not a shortlist entry and it is already reported in full under * `accommodations` — listing it twice would read as two different intentions. */ - private buildUnplannedPlaces(tripId: number): PublicApiPlace[] { + private async buildUnplannedPlaces(tripId: number): Promise { const rows = this.db.all>( `SELECT p.name, p.address, p.lat, p.lng, p.place_time, p.end_time, p.duration_minutes, p.notes, p.transport_mode, @@ -261,7 +261,7 @@ export class PublicApiService { * sets it null rather than cascading, so a flight can outlive the day it was * pinned to. Reporting only day-bound bookings would quietly lose those. */ - private buildUnscheduledReservations(tripId: number): PublicApiReservation[] { + private async buildUnscheduledReservations(tripId: number): Promise { const rows = this.db.all>( `SELECT type, title, location, reservation_time, reservation_end_time, status, notes @@ -285,7 +285,7 @@ export class PublicApiService { * whether TREK shows the feature, not whether the rows exist, and a key whose * answers change when an unrelated toggle moves is a key nobody can build on. */ - listBucketList(userId: number): PublicApiBucketListItem[] { + async listBucketList(userId: number): Promise { const rows = this.db.all( `SELECT name, lat, lng, country_code, notes, target_date FROM bucket_list WHERE user_id = ? @@ -310,7 +310,7 @@ export class PublicApiService { * itinerary, not for enumerating the people around them. What it returns is * exactly what those people already see on the trip in TREK. */ - private buildTravellers(tripId: number): PublicApiTraveller[] { + private async buildTravellers(tripId: number): Promise { const rows = this.db.all( `SELECT u.username, 1 AS is_owner FROM trips t JOIN users u ON u.id = t.user_id diff --git a/server/src/nest/reservations/reservations.controller.ts b/server/src/nest/reservations/reservations.controller.ts index 439889955..4e873ee37 100644 --- a/server/src/nest/reservations/reservations.controller.ts +++ b/server/src/nest/reservations/reservations.controller.ts @@ -68,7 +68,7 @@ export class ReservationsController { @Headers('x-socket-id') socketId?: string, ) { const body = rawBody as ReservationBody & { title: string }; - this.rejectForeignReferences(tripId, body); + await this.rejectForeignReferences(tripId, body); const { reservation, accommodationCreated } = await this.reservations.create(tripId, body as never); if (accommodationCreated) { this.reservations.broadcast(tripId, 'accommodation:created', {}, socketId); @@ -108,7 +108,7 @@ export class ReservationsController { if (!current) { throw new HttpException({ error: 'Reservation not found' }, 404); } - this.rejectForeignReferences(tripId, body); + await this.rejectForeignReferences(tripId, body); const { reservation, accommodationChanged } = await this.reservations.update(id, tripId, body as never, current as never); if (accommodationChanged) { this.reservations.broadcast(tripId, 'accommodation:updated', {}, socketId); @@ -180,12 +180,12 @@ export class ReservationsController { * trip" about an id that is part of nothing would send the caller looking in * the wrong place. */ - private rejectForeignReferences(tripId: string, body: ReservationBody): void { + private async rejectForeignReferences(tripId: string, body: ReservationBody): Promise { const offenders = this.reservations.referencesOutsideTrip(tripId, body as never); if (offenders.length > 0) { throw new HttpException({ error: `Not part of this trip: ${offenders.join(', ')}` }, 400); } - const unknown = this.reservations.unresolvedReferences(tripId, body as never); + const unknown = await this.reservations.unresolvedReferences(tripId, body as never); if (unknown.length > 0) { throw new HttpException({ error: `Unknown reference: ${unknown.join(', ')}` }, 400); } diff --git a/server/src/nest/reservations/reservations.mcp.ts b/server/src/nest/reservations/reservations.mcp.ts index f768e1b1e..94ba40441 100644 --- a/server/src/nest/reservations/reservations.mcp.ts +++ b/server/src/nest/reservations/reservations.mcp.ts @@ -368,13 +368,13 @@ export class ReservationsMcp { // Validate that all referenced IDs belong to this trip if (day_id && !(await this.days.getDay(day_id, tripId))) return errorResult('day_id does not belong to this trip.'); - if (place_id && !this.assignments.placeExists(place_id, tripId)) + if (place_id && !(await this.assignments.placeExists(place_id, tripId))) return errorResult('place_id does not belong to this trip.'); if (start_day_id && !(await this.days.getDay(start_day_id, tripId))) return errorResult('start_day_id does not belong to this trip.'); if (end_day_id && !(await this.days.getDay(end_day_id, tripId))) return errorResult('end_day_id does not belong to this trip.'); - if (assignment_id && !this.assignments.getAssignmentForTrip(assignment_id, tripId)) + if (assignment_id && !(await this.assignments.getAssignmentForTrip(assignment_id, tripId))) return errorResult('assignment_id does not belong to this trip.'); const createAccommodation = (type === 'hotel' && place_id && start_day_id && end_day_id) @@ -443,9 +443,9 @@ export class ReservationsMcp { const existing = this.reservations.getReservation(reservationId, tripId); if (!existing) return errorResult('Reservation not found.'); - if (place_id != null && !this.assignments.placeExists(place_id, tripId)) + if (place_id != null && !(await this.assignments.placeExists(place_id, tripId))) return errorResult('place_id does not belong to this trip.'); - if (assignment_id != null && !this.assignments.getAssignmentForTrip(assignment_id, tripId)) + if (assignment_id != null && !(await this.assignments.getAssignmentForTrip(assignment_id, tripId))) return errorResult('assignment_id does not belong to this trip.'); const { reservation } = await this.reservations.update(reservationId, tripId, { @@ -577,7 +577,7 @@ export class ReservationsMcp { if (!current) return errorResult('Reservation not found.'); if (current.type !== 'hotel') return errorResult('Reservation is not of type hotel.'); - if (!this.assignments.placeExists(place_id, tripId)) + if (!(await this.assignments.placeExists(place_id, tripId))) return errorResult('place_id does not belong to this trip.'); if (!(await this.days.getDay(start_day_id, tripId))) return errorResult('start_day_id does not belong to this trip.'); diff --git a/server/src/nest/reservations/reservations.rpc.ts b/server/src/nest/reservations/reservations.rpc.ts index 687d9a15c..868717163 100644 --- a/server/src/nest/reservations/reservations.rpc.ts +++ b/server/src/nest/reservations/reservations.rpc.ts @@ -38,7 +38,7 @@ export class ReservationsRpc { const input = parsed.data as Record; this.requireValidEndpoints(input.endpoints); await this.guards.requireTripEdit(tripId, actor, RESERVATION_EDIT_ACTION); - this.requireOwnReferences(tripId, input); + await this.requireOwnReferences(tripId, input); const { reservation, accommodationCreated } = await this.reservations.create(String(tripId), input as never); if (accommodationCreated) this.realtime.broadcast(tripId, 'accommodation:created', {}, undefined); const i = input as { title?: string; type?: string; create_budget_entry?: unknown }; @@ -60,7 +60,7 @@ export class ReservationsRpc { await this.guards.requireTripEdit(tripId, actor, RESERVATION_EDIT_ACTION); const current = this.reservations.getReservation(String(reservationId), String(tripId)); if (!current) throw new ForbiddenResource(`no reservation ${reservationId} on trip ${tripId}`); - this.requireOwnReferences(tripId, input); + await this.requireOwnReferences(tripId, input); const { reservation, accommodationChanged } = await this.reservations.update(String(reservationId), String(tripId), input as never, current as never); if (accommodationChanged) this.realtime.broadcast(tripId, 'accommodation:updated', {}, undefined); const cur = current as { title: string; type?: string }; @@ -109,10 +109,10 @@ export class ReservationsRpc { * resolves to nothing is a foreign-key error the plugin reads as a crash (#2355), * so it is named here as well, after the ownership check. */ - private requireOwnReferences(tripId: number, input: Record): void { + private async requireOwnReferences(tripId: number, input: Record): Promise { const offenders = this.reservations.referencesOutsideTrip(String(tripId), input as never); if (offenders.length > 0) throw new ForbiddenResource(`not part of trip ${tripId}: ${offenders.join(', ')}`); - const unknown = this.reservations.unresolvedReferences(String(tripId), input as never); + const unknown = await this.reservations.unresolvedReferences(String(tripId), input as never); if (unknown.length > 0) throw new BadParams(`unknown reference: ${unknown.join(', ')}`); } diff --git a/server/src/nest/reservations/reservations.service.ts b/server/src/nest/reservations/reservations.service.ts index f4c2cf288..a15f45ccd 100644 --- a/server/src/nest/reservations/reservations.service.ts +++ b/server/src/nest/reservations/reservations.service.ts @@ -587,7 +587,7 @@ export class ReservationsService { * that belongs to another trip is named here too — the REST controller asks * the older guard first, so that case keeps its own answer. */ - unresolvedReferences(tripId: string | number, data: CreateReservationData | UpdateReservationData): string[] { + async unresolvedReferences(tripId: string | number, data: CreateReservationData | UpdateReservationData): Promise { const offenders: string[] = []; const onTrip = (table: 'days' | 'places', id: unknown) => !!this.db.get(`SELECT id FROM ${table} WHERE id = ? AND trip_id = ?`, id, tripId); @@ -610,7 +610,7 @@ export class ReservationsService { // on a write that currently succeeds is not what this guard is for. if (data.create_accommodation && data.type === 'hotel') { const acc = data.create_accommodation; - const errors = this.accommodations.validateAccommodationRefs( + const errors = await this.accommodations.validateAccommodationRefs( tripId, acc.place_id || undefined, acc.start_day_id || undefined, acc.end_day_id || undefined, ); for (const { field } of errors) offenders.push(`create_accommodation.${field}`); diff --git a/server/src/nest/roadtrip/google-route.service.ts b/server/src/nest/roadtrip/google-route.service.ts index 6e3a68d2b..2ded31c48 100644 --- a/server/src/nest/roadtrip/google-route.service.ts +++ b/server/src/nest/roadtrip/google-route.service.ts @@ -65,13 +65,13 @@ export class GoogleRouteService { if (!(await this.permissions.checkPermission(action, role, access.user_id, userId, access.user_id !== userId))) throw new HttpException({ error: 'Permission denied' }, 403); } - if (!this.assignments.dayExists(String(input.dayId), String(tripId))) throw new HttpException({ error: 'Day not found' }, 404); + if (!(await this.assignments.dayExists(String(input.dayId), String(tripId)))) throw new HttpException({ error: 'Day not found' }, 404); // `map` cannot await the now-async assignment write, so the same per-stop // sequence runs as an explicit loop inside the transaction. const imported = await this.uow.transactional(async () => { - const rows: { place: ReturnType; assignment: Awaited> }[] = []; + const rows: { place: Awaited>; assignment: Awaited> }[] = []; for (const stop of input.stops) { - const place = this.places.create(String(tripId), { ...stop, transport_mode: 'car', duration_minutes: 0 }); + const place = await this.places.create(String(tripId), { ...stop, transport_mode: 'car', duration_minutes: 0 }); const assignment = await this.assignments.createAssignment(input.dayId, place.id); rows.push({ place, assignment }); } diff --git a/server/src/nest/trip-read-model/trip-read-model.service.ts b/server/src/nest/trip-read-model/trip-read-model.service.ts index 98c03f165..4f1aad605 100644 --- a/server/src/nest/trip-read-model/trip-read-model.service.ts +++ b/server/src/nest/trip-read-model/trip-read-model.service.ts @@ -60,7 +60,7 @@ export class TripReadModelService { const { days: rawDays } = await this.days.list(tripId); const days = rawDays.map(({ notes_items, ...day }) => ({ ...day, notes: notes_items })); - const accommodations = this.accommodations.list(tripId); + const accommodations = await this.accommodations.list(tripId); const budgetItems = await this.budget.listBudgetItems(tripId); const budget = { @@ -111,7 +111,7 @@ export class TripReadModelService { budgetItems: await this.budget.listBudgetItems(tripId), reservations: this.reservations.list(tripId), files: await this.files.listFiles(tripId, false), - accommodations: this.accommodations.list(tripId), + accommodations: await this.accommodations.list(tripId), members: [owner, ...(members || [])].filter(Boolean), }; } diff --git a/server/src/nest/trips/trips.rpc.ts b/server/src/nest/trips/trips.rpc.ts index 3c1d5e8f8..9b22ce30d 100644 --- a/server/src/nest/trips/trips.rpc.ts +++ b/server/src/nest/trips/trips.rpc.ts @@ -83,7 +83,7 @@ export class TripsRpc { @PluginMethod('trips.getAccommodations', { permission: 'db:read:trips' }) getAccommodations(params: Record, ctx: PluginRpcContext): unknown { - return this.guards.tripRead(params, ctx, () => this.accommodations.list(num(params.tripId, 'tripId')) as unknown[]); + return this.guards.tripRead(params, ctx, async () => (await this.accommodations.list(num(params.tripId, 'tripId'))) as unknown[]); } @PluginMethod('trips.listMine', { permission: 'db:read:trips' }) diff --git a/server/tests/helpers/accommodations-service.ts b/server/tests/helpers/accommodations-service.ts index 1b15d96a1..3ac7e891e 100644 --- a/server/tests/helpers/accommodations-service.ts +++ b/server/tests/helpers/accommodations-service.ts @@ -36,6 +36,7 @@ export async function accommodationsOver(dbs: DatabaseService): Promise { // dependency fails the build — pass them for real regardless of the gate. // One instance, four consumers: AssignmentsMcp, ReservationsMcp, PlacesMcp and // AccommodationsService, which writes the day stop a booked night implies. - const assignmentsService = new AssignmentsService(dbService, permissionsService, realtimeService, queryHelpersService, journeyDomain); + const assignmentsService = new AssignmentsService(dbService, permissionsService, realtimeService, queryHelpersService, journeyDomain, await createTestUnitOfWork(dbService.connection)); const accommodationsService = new AccommodationsService(dbService, permissionsService, realtimeService, assignmentsService, await createTestUnitOfWork(dbService.connection)); // Built after it: deleting a place cancels the nights booked at it through this one. const placesService = new PlacesService( diff --git a/server/tests/helpers/plugin-host.ts b/server/tests/helpers/plugin-host.ts index fc88b3646..65291e1af 100644 --- a/server/tests/helpers/plugin-host.ts +++ b/server/tests/helpers/plugin-host.ts @@ -105,7 +105,7 @@ export async function createPluginRpcHostFactory(dbs: DatabaseService): Promise< const collections = new CollectionsService(dbs, permissions, realtime, notificationsStub(), generalStorage, await createTestUnitOfWork(dbs.connection)); const atlas = new AtlasService(dbs, await createTestUnitOfWork(dbs.connection)); const dayNotes = new DayNotesService(dbs, permissions, realtime); - const assignments = new AssignmentsService(dbs, permissions, realtime, queryHelpers, journey); + const assignments = new AssignmentsService(dbs, permissions, realtime, queryHelpers, journey, await createTestUnitOfWork(dbs.connection)); const membership = new TripMembershipService(dbs); const notifications = await makeNotificationsService(dbs, realtime); const llmConfig = new LlmConfigResolver(new SettingsService(dbs, await createTestUnitOfWork(dbs.connection)), dbs, addons); diff --git a/server/tests/integration/journey-place-order.test.ts b/server/tests/integration/journey-place-order.test.ts index fa98b114a..ec1a8efbc 100644 --- a/server/tests/integration/journey-place-order.test.ts +++ b/server/tests/integration/journey-place-order.test.ts @@ -5,10 +5,16 @@ * FK, ON DELETE SET NULL) is actually deleted — otherwise the hook either * never runs, or races the delete and finds nothing left to annotate. * - * Wires PlacesController → PlacesService → JourneyDomainService for real - * (no journey mock), through the real buildApp() HTTP surface, so a future - * change that turns the await back into fire-and-forget shows up here rather - * than only in a unit test with a stub. + * Wires PlacesController → PlacesService → JourneyDomainService for real (no + * journey mock), through the real buildApp() HTTP surface, and pins the + * end-to-end detach-then-annotate behaviour that path produces. It is NOT by + * itself a guard against a revert to fire-and-forget: the hook body today has + * no internal await, so it still runs to completion inside the same + * synchronous/microtask turn even if the controller stopped awaiting it, and + * this test would not notice. It becomes a true regression guard once the + * hook body awaits real async DB work of its own — see + * tests/unit/nest/places.controller.test.ts for the unit-level ordering test + * that stubs a macrotask hop and does catch a detached hook today. */ import { describe, it, expect, vi, beforeAll, beforeEach, afterAll } from 'vitest'; import request from 'supertest'; diff --git a/server/tests/unit/nest/accommodations.controller.test.ts b/server/tests/unit/nest/accommodations.controller.test.ts index acac3e213..06a4e0773 100644 --- a/server/tests/unit/nest/accommodations.controller.test.ts +++ b/server/tests/unit/nest/accommodations.controller.test.ts @@ -32,9 +32,9 @@ async function thrownAsync(fn: () => Promise): Promise<{ status: number describe('AccommodationsController (parity with the legacy accommodations sub-router)', () => { - it('GET / lists (no permission gate)', () => { + it('GET / lists (no permission gate)', async () => { const svc = makeService({ list: vi.fn().mockReturnValue([{ id: 1 }]) } as Partial); - expect(new AccommodationsController(svc).list(user, '5')).toEqual({ accommodations: [{ id: 1 }] }); + expect(await new AccommodationsController(svc).list(user, '5')).toEqual({ accommodations: [{ id: 1 }] }); }); describe('POST /', () => { diff --git a/server/tests/unit/nest/accommodations.service.test.ts b/server/tests/unit/nest/accommodations.service.test.ts index cc2651e7a..8f5145a15 100644 --- a/server/tests/unit/nest/accommodations.service.test.ts +++ b/server/tests/unit/nest/accommodations.service.test.ts @@ -72,48 +72,48 @@ afterAll(() => { }); describe('validateAccommodationRefs', () => { - it('DAY-SVC-016 — returns no errors when all refs are valid', () => { + it('DAY-SVC-016 — returns no errors when all refs are valid', async () => { const { user } = createUser(testDb); const trip = createTrip(testDb, user.id); const day = createDay(testDb, trip.id) as any; const place = createPlace(testDb, trip.id, { name: 'Hotel' }) as any; - const errors = svc.validateAccommodationRefs(trip.id, place.id, day.id, day.id); + const errors = await svc.validateAccommodationRefs(trip.id, place.id, day.id, day.id); expect(errors).toHaveLength(0); }); - it('DAY-SVC-017 — returns error when place does not exist in trip', () => { + it('DAY-SVC-017 — returns error when place does not exist in trip', async () => { const { user } = createUser(testDb); const trip = createTrip(testDb, user.id); const day = createDay(testDb, trip.id) as any; - const errors = svc.validateAccommodationRefs(trip.id, 99999, day.id, day.id); + const errors = await svc.validateAccommodationRefs(trip.id, 99999, day.id, day.id); expect(errors.some((e: any) => e.field === 'place_id')).toBe(true); }); - it('DAY-SVC-018 — returns error when start_day_id is invalid', () => { + it('DAY-SVC-018 — returns error when start_day_id is invalid', async () => { const { user } = createUser(testDb); const trip = createTrip(testDb, user.id); const day = createDay(testDb, trip.id) as any; const place = createPlace(testDb, trip.id, { name: 'Hotel' }) as any; - const errors = svc.validateAccommodationRefs(trip.id, place.id, 99999, day.id); + const errors = await svc.validateAccommodationRefs(trip.id, place.id, 99999, day.id); expect(errors.some((e: any) => e.field === 'start_day_id')).toBe(true); }); - it('ACC-003 — reports every bad ref, end_day_id included', () => { + it('ACC-003 — reports every bad ref, end_day_id included', async () => { // The controller only surfaces errors[0], so an unchecked end_day_id would let a // stay point at a day from another trip and never be reported. const { user } = createUser(testDb); const trip = createTrip(testDb, user.id); - const errors = svc.validateAccommodationRefs(trip.id, 99999, 99998, 99997); + const errors = await svc.validateAccommodationRefs(trip.id, 99999, 99998, 99997); expect(errors.map((e: any) => e.field)).toEqual(['place_id', 'start_day_id', 'end_day_id']); }); - it('ACC-004 — skips the refs the caller left undefined', () => { + it('ACC-004 — skips the refs the caller left undefined', async () => { // The plugin update path (accommodations.rpc) passes only the fields it changes. // If an absent ref were validated, every partial update would 403 "Place not found". const { user } = createUser(testDb); const trip = createTrip(testDb, user.id); - expect(svc.validateAccommodationRefs(trip.id)).toHaveLength(0); - expect(svc.validateAccommodationRefs(trip.id, undefined, undefined, undefined)).toHaveLength(0); + expect(await svc.validateAccommodationRefs(trip.id)).toHaveLength(0); + expect(await svc.validateAccommodationRefs(trip.id, undefined, undefined, undefined)).toHaveLength(0); }); }); @@ -171,21 +171,21 @@ describe('createAccommodation', () => { }); describe('getAccommodation', () => { - it('DAY-SVC-021 — returns accommodation for valid id and trip', () => { + it('DAY-SVC-021 — returns accommodation for valid id and trip', async () => { const { user } = createUser(testDb); const trip = createTrip(testDb, user.id); const day = createDay(testDb, trip.id) as any; const place = createPlace(testDb, trip.id, { name: 'Hotel' }) as any; const accom = createDayAccommodation(testDb, trip.id, place.id, day.id, day.id) as any; - const found = svc.getAccommodation(accom.id, trip.id) as any; + const found = await svc.getAccommodation(accom.id, trip.id) as any; expect(found).toBeDefined(); expect(found.id).toBe(accom.id); }); - it('DAY-SVC-022 — returns undefined for non-existent accommodation', () => { + it('DAY-SVC-022 — returns undefined for non-existent accommodation', async () => { const { user } = createUser(testDb); const trip = createTrip(testDb, user.id); - expect(svc.getAccommodation(99999, trip.id)).toBeUndefined(); + expect(await svc.getAccommodation(99999, trip.id)).toBeUndefined(); }); }); @@ -199,7 +199,7 @@ describe('updateAccommodation', () => { place_id: place.id, start_day_id: day.id, end_day_id: day.id, })) as any; - const existing = svc.getAccommodation(accom.id, trip.id)!; + const existing = (await svc.getAccommodation(accom.id, trip.id))!; const { accommodation: updated } = (await svc.updateAccommodation(accom.id, existing as any, { check_in: '16:00', check_out: '12:00' })) as any; expect(updated).toBeDefined(); @@ -221,10 +221,10 @@ describe('updateAccommodation', () => { confirmation: 'ABC123', })) as any; - const existing = svc.getAccommodation(accom.id, trip.id)!; + const existing = (await svc.getAccommodation(accom.id, trip.id))!; await svc.updateAccommodation(accom.id, existing as any, { check_in: '14:00' }); - const row = svc.getAccommodation(accom.id, trip.id) as any; + const row = await svc.getAccommodation(accom.id, trip.id) as any; expect(row.confirmation).toBe('ABC123'); }); @@ -238,7 +238,7 @@ describe('updateAccommodation', () => { // The factory writes the stay row directly, without the partner reservation. const accom = createDayAccommodation(testDb, trip.id, place.id, day.id, day.id) as any; - const existing = svc.getAccommodation(accom.id, trip.id)!; + const existing = (await svc.getAccommodation(accom.id, trip.id))!; const { accommodation: updated } = (await svc.updateAccommodation(accom.id, existing as any, { check_in: '15:00' })) as any; expect(updated.check_in).toBe('15:00'); @@ -257,7 +257,7 @@ describe('updateAccommodation', () => { place_id: place.id, start_day_id: day.id, end_day_id: day.id, check_in: '15:00', })) as any; - const existing = svc.getAccommodation(accom.id, trip.id)!; + const existing = (await svc.getAccommodation(accom.id, trip.id))!; await svc.updateAccommodation(accom.id, existing as any, { check_in_end: '20:00', check_out: '11:00' }); const reservation = testDb.prepare('SELECT metadata FROM reservations WHERE accommodation_id = ?').get(accom.id) as any; @@ -278,7 +278,7 @@ describe('updateAccommodation', () => { })) as any; testDb.prepare('UPDATE reservations SET confirmation_number = ? WHERE accommodation_id = ?').run('RES-9', accom.id); - const existing = svc.getAccommodation(accom.id, trip.id)!; + const existing = (await svc.getAccommodation(accom.id, trip.id))!; await svc.updateAccommodation(accom.id, existing as any, { check_in: '15:00' }); const reservation = testDb.prepare('SELECT confirmation_number FROM reservations WHERE accommodation_id = ?').get(accom.id) as any; @@ -302,7 +302,7 @@ describe('deleteAccommodation', () => { expect(result.linkedReservationId).toBe(reservation.id); // Accommodation is gone - expect(svc.getAccommodation(accom.id, trip.id)).toBeUndefined(); + expect(await svc.getAccommodation(accom.id, trip.id)).toBeUndefined(); // Reservation is gone const deletedRes = testDb.prepare('SELECT id FROM reservations WHERE id = ?').get(reservation.id); @@ -400,13 +400,13 @@ describe('deleteAccommodation', () => { }); describe('listAccommodations', () => { - it('DAY-SVC-029 — listAccommodations returns the hydrated stays', () => { + it('DAY-SVC-029 — listAccommodations returns the hydrated stays', async () => { const { user } = createUser(testDb); const trip = createTrip(testDb, user.id); const day = createDay(testDb, trip.id); const place = createPlace(testDb, trip.id, { name: 'Ryokan' }); createDayAccommodation(testDb, trip.id, place.id, day.id, day.id); - const rows = svc.listAccommodations(trip.id) as { place_name: string }[]; + const rows = await svc.listAccommodations(trip.id) as { place_name: string }[]; expect(rows).toHaveLength(1); expect(rows[0].place_name).toBe('Ryokan'); }); @@ -438,26 +438,26 @@ describe('route-facing delegators', () => { // on the same SQL, and the short ones get raw route params — strings where the long // ones get numbers. - it('ACC-010 — list() returns what listAccommodations() returns, for a string and a number trip id', () => { + it('ACC-010 — list() returns what listAccommodations() returns, for a string and a number trip id', async () => { const { user } = createUser(testDb); const trip = createTrip(testDb, user.id); const day = createDay(testDb, trip.id) as any; const place = createPlace(testDb, trip.id, { name: 'Pension Alpen' }) as any; createDayAccommodation(testDb, trip.id, place.id, day.id, day.id); - expect(svc.list(String(trip.id))).toEqual(svc.listAccommodations(trip.id)); - expect((svc.list(trip.id) as any[])[0].place_name).toBe('Pension Alpen'); + expect(await svc.list(String(trip.id))).toEqual(await svc.listAccommodations(trip.id)); + expect((await svc.list(trip.id) as any[])[0].place_name).toBe('Pension Alpen'); }); - it('ACC-011 — validateRefs() returns the same errors validateAccommodationRefs() does', () => { + it('ACC-011 — validateRefs() returns the same errors validateAccommodationRefs() does', async () => { const { user } = createUser(testDb); const trip = createTrip(testDb, user.id); const day = createDay(testDb, trip.id) as any; const place = createPlace(testDb, trip.id, { name: 'Hotel' }) as any; - expect(svc.validateRefs(trip.id, place.id, day.id, day.id)).toHaveLength(0); + expect(await svc.validateRefs(trip.id, place.id, day.id, day.id)).toHaveLength(0); // The controller 404s with errors[0].message, so the shape matters, not just the count. - expect(svc.validateRefs(trip.id, 99999, day.id, day.id)).toEqual([{ field: 'place_id', message: 'Place not found' }]); + expect(await svc.validateRefs(trip.id, 99999, day.id, day.id)).toEqual([{ field: 'place_id', message: 'Place not found' }]); }); it('ACC-012 — create() writes the stay, get() reads it back trip-scoped', async () => { @@ -472,9 +472,9 @@ describe('route-facing delegators', () => { place_id: place.id, start_day_id: day.id, end_day_id: day.id, confirmation: 'XY-1', })) as any; - expect(svc.get(String(created.id), String(trip.id))).toMatchObject({ id: created.id, confirmation: 'XY-1' }); + expect(await svc.get(String(created.id), String(trip.id))).toMatchObject({ id: created.id, confirmation: 'XY-1' }); // Trip-scoped: a stay must not be readable through another trip's URL. - expect(svc.get(created.id, otherTrip.id)).toBeUndefined(); + expect(await svc.get(created.id, otherTrip.id)).toBeUndefined(); }); it('ACC-013 — update() syncs the partner reservation even when the id is a string', async () => { @@ -486,7 +486,7 @@ describe('route-facing delegators', () => { const place = createPlace(testDb, trip.id, { name: 'Hotel' }) as any; const { accommodation: accom } = (await svc.create(trip.id, { place_id: place.id, start_day_id: day.id, end_day_id: day.id })) as any; - const existing = svc.get(accom.id, trip.id)!; + const existing = (await svc.get(accom.id, trip.id))!; const { accommodation: updated } = (await svc.update(String(accom.id), existing as any, { check_in: '16:00', notes: 'late arrival' })) as any; expect(updated).toMatchObject({ check_in: '16:00', notes: 'late arrival' }); @@ -510,33 +510,33 @@ describe('route-facing delegators', () => { deletedBudgetItemIds: [], mirror: { created: null, moved: null, updated: [], removed: [{ id: expect.any(Number), dayId: day.id }], stamped: null }, }); - expect(svc.get(accom.id, trip.id)).toBeUndefined(); + expect(await svc.get(accom.id, trip.id)).toBeUndefined(); }); }); describe('trip access and edit permission', () => { - it('ACC-015 — verifyTripAccess resolves the trip from a string id and from a number', () => { + it('ACC-015 — verifyTripAccess resolves the trip from a string id and from a number', async () => { // The REST path hands over the raw :tripId param, the MCP tools the parsed number. // Without the Number() coercion one of the two callers stops finding its trip. const { user } = createUser(testDb); const trip = createTrip(testDb, user.id); - expect(svc.verifyTripAccess(String(trip.id), user.id)).toMatchObject({ id: trip.id, user_id: user.id }); - expect(svc.verifyTripAccess(trip.id, user.id)).toMatchObject({ id: trip.id, user_id: user.id }); + expect(await svc.verifyTripAccess(String(trip.id), user.id)).toMatchObject({ id: trip.id, user_id: user.id }); + expect(await svc.verifyTripAccess(trip.id, user.id)).toMatchObject({ id: trip.id, user_id: user.id }); }); - it('ACC-016 — verifyTripAccess returns nothing for a user who is neither owner nor member', () => { + it('ACC-016 — verifyTripAccess returns nothing for a user who is neither owner nor member', async () => { const { user: owner } = createUser(testDb); const { user: stranger } = createUser(testDb); const trip = createTrip(testDb, owner.id); - expect(svc.verifyTripAccess(trip.id, stranger.id)).toBeUndefined(); + expect(await svc.verifyTripAccess(trip.id, stranger.id)).toBeUndefined(); }); it('ACC-017 — canEdit lets the owner through on the default day_edit level', async () => { const { user } = createUser(testDb); const trip = createTrip(testDb, user.id); - const access = svc.verifyTripAccess(trip.id, user.id)!; + const access = (await svc.verifyTripAccess(trip.id, user.id))!; expect(await svc.canEdit(access, user as any)).toBe(true); }); @@ -549,7 +549,7 @@ describe('trip access and edit permission', () => { const { user: member } = createUser(testDb); const trip = createTrip(testDb, owner.id); addTripMember(testDb, trip.id, member.id); - const access = svc.verifyTripAccess(trip.id, member.id)!; + const access = (await svc.verifyTripAccess(trip.id, member.id))!; expect(await svc.canEdit(access, member as any)).toBe(true); }); @@ -563,9 +563,9 @@ describe('trip access and edit permission', () => { try { await permissions.savePermissions({ day_edit: 'trip_owner' }); - expect(await svc.canEdit(svc.verifyTripAccess(trip.id, member.id)!, member as any)).toBe(false); + expect(await svc.canEdit((await svc.verifyTripAccess(trip.id, member.id))!, member as any)).toBe(false); // The owner keeps the right, otherwise the setting would lock out the whole trip. - expect(await svc.canEdit(svc.verifyTripAccess(trip.id, owner.id)!, owner as any)).toBe(true); + expect(await svc.canEdit((await svc.verifyTripAccess(trip.id, owner.id))!, owner as any)).toBe(true); } finally { // The permission cache is module-scoped and outlives resetTestDb — drop the row and // the cache together, or every later case in this file inherits the raised level. @@ -718,12 +718,12 @@ describe('the day stop a booking implies', () => { testDb.prepare('UPDATE day_assignments SET order_index = 2 WHERE id = ?').run(own.id); expect(stopsOn(day.id).map(a => a.place_id)).toEqual([harbour.id, market.id, hotel.id]); - let existing = svc.getAccommodation(accommodation.id, trip.id)!; + let existing = (await svc.getAccommodation(accommodation.id, trip.id))!; const quiet = (await svc.updateAccommodation(accommodation.id, existing, { notes: 'late arrival' })) as any; expect(quiet.mirror.moved).toBeNull(); expect(stopsOn(day.id).map(a => a.place_id)).toEqual([harbour.id, market.id, hotel.id]); - existing = svc.getAccommodation(accommodation.id, trip.id)!; + existing = (await svc.getAccommodation(accommodation.id, trip.id))!; const reseated = (await svc.updateAccommodation(accommodation.id, existing, { check_in: '10:00' })) as any; expect(reseated.mirror.moved).toMatchObject({ oldDayId: day.id, assignment: { id: own.id } }); expect(stopsOn(day.id).map(a => a.place_id)).toEqual([hotel.id, harbour.id, market.id]); @@ -742,7 +742,7 @@ describe('the day stop a booking implies', () => { })) as any; expect(stopsOn(day.id).map(a => a.place_id)).toEqual([hotel.id, afternoon.id]); - const existing = svc.getAccommodation(accommodation.id, trip.id)!; + const existing = (await svc.getAccommodation(accommodation.id, trip.id))!; await svc.updateAccommodation(accommodation.id, existing, { check_in: '20:00' }); expect(stopsOn(day.id).map(a => a.place_id)).toEqual([afternoon.id, hotel.id]); @@ -781,7 +781,7 @@ describe('the day stop a booking implies', () => { // Behind the harbour only: allowed, and left alone. testDb.prepare('UPDATE day_assignments SET order_index = 0 WHERE day_id = ? AND place_id = ?').run(day.id, harbour.id); testDb.prepare('UPDATE day_assignments SET order_index = 1 WHERE id = ?').run(own.id); - let existing = svc.getAccommodation(accommodation.id, trip.id)!; + let existing = (await svc.getAccommodation(accommodation.id, trip.id))!; expect(((await svc.updateAccommodation(accommodation.id, existing, { notes: 'late' })) as any).mirror.moved).toBeNull(); expect(stopsOn(day.id).map(a => a.place_id)).toEqual([harbour.id, hotel.id, museum.id]); @@ -789,7 +789,7 @@ describe('the day stop a booking implies', () => { // the front, ahead of the harbour that has no hour of its own. testDb.prepare('UPDATE day_assignments SET order_index = 1 WHERE day_id = ? AND place_id = ?').run(day.id, museum.id); testDb.prepare('UPDATE day_assignments SET order_index = 2 WHERE id = ?').run(own.id); - existing = svc.getAccommodation(accommodation.id, trip.id)!; + existing = (await svc.getAccommodation(accommodation.id, trip.id))!; expect(((await svc.updateAccommodation(accommodation.id, existing, { notes: 'later' })) as any).mirror.moved).not.toBeNull(); expect(stopsOn(day.id).map(a => a.place_id)).toEqual([hotel.id, harbour.id, museum.id]); }); @@ -877,7 +877,7 @@ describe('the day stop a booking implies', () => { const { accommodation } = await book(trip.id, place.id, day1.id, day1.id); const before = stopsOn(day1.id)[0]; - const existing = svc.getAccommodation(accommodation.id, trip.id)!; + const existing = (await svc.getAccommodation(accommodation.id, trip.id))!; const { mirror } = (await svc.updateAccommodation(accommodation.id, existing, { start_day_id: day2.id, end_day_id: day2.id })) as any; expect(stopsOn(day1.id)).toHaveLength(0); @@ -900,7 +900,7 @@ describe('the day stop a booking implies', () => { testDb.prepare('UPDATE day_assignments SET notes = ?, assignment_time = ?, end_day = 1 WHERE id = ?') .run('ask for the quiet side', '15:30', stop.id); - const existing = svc.getAccommodation(accommodation.id, trip.id)!; + const existing = (await svc.getAccommodation(accommodation.id, trip.id))!; await svc.updateAccommodation(accommodation.id, existing, { start_day_id: day2.id, end_day_id: day2.id }); const after = testDb.prepare('SELECT * FROM day_assignments WHERE id = ?').get(stop.id) as Record; @@ -919,7 +919,7 @@ describe('the day stop a booking implies', () => { // re-insert would take these with it without a word. testDb.prepare('INSERT INTO assignment_participants (assignment_id, user_id) VALUES (?, ?)').run(stop.id, user.id); - const existing = svc.getAccommodation(accommodation.id, trip.id)!; + const existing = (await svc.getAccommodation(accommodation.id, trip.id))!; await svc.updateAccommodation(accommodation.id, existing, { start_day_id: day2.id, end_day_id: day2.id }); const kept = testDb.prepare('SELECT user_id FROM assignment_participants WHERE assignment_id = ?').all(stop.id); @@ -936,7 +936,7 @@ describe('the day stop a booking implies', () => { const own = stopsOn(day1.id)[0]; const byHand = createDayAssignment(testDb, day2.id, place.id) as { id: number }; - const existing = svc.getAccommodation(accommodation.id, trip.id)!; + const existing = (await svc.getAccommodation(accommodation.id, trip.id))!; const { mirror } = (await svc.updateAccommodation(accommodation.id, existing, { start_day_id: day2.id, end_day_id: day2.id })) as any; expect(stopsOn(day1.id)).toHaveLength(0); @@ -955,7 +955,7 @@ describe('the day stop a booking implies', () => { const { accommodation } = await book(trip.id, hotel.id, day1.id, day1.id); createDayAssignment(testDb, day1.id, museum.id); - const existing = svc.getAccommodation(accommodation.id, trip.id)!; + const existing = (await svc.getAccommodation(accommodation.id, trip.id))!; await svc.updateAccommodation(accommodation.id, existing, { start_day_id: day2.id, end_day_id: day2.id }); expect(stopsOn(day1.id).map((row: { order_index: number }) => row.order_index)).toEqual([0]); @@ -969,7 +969,7 @@ describe('the day stop a booking implies', () => { const { accommodation } = await book(trip.id, place.id, day.id, day.id); const before = stopsOn(day.id); - const existing = svc.getAccommodation(accommodation.id, trip.id)!; + const existing = (await svc.getAccommodation(accommodation.id, trip.id))!; const { mirror } = (await svc.updateAccommodation(accommodation.id, existing, { check_in: '16:00' })) as any; expect(stopsOn(day.id)).toEqual(before); @@ -994,7 +994,7 @@ describe('the day stop a booking implies', () => { testDb.prepare('UPDATE day_assignments SET order_index = 1 WHERE id = ?').run(stop.id); expect(stopsOn(day.id).map((row: { place_id: number }) => row.place_id)).toEqual([dinner.id, hotel.id]); - const existing = svc.getAccommodation(accommodation.id, trip.id)!; + const existing = (await svc.getAccommodation(accommodation.id, trip.id))!; const { mirror } = (await svc.updateAccommodation(accommodation.id, existing, { check_in: '15:00' })) as any; expect(stopsOn(day.id).map((row: { place_id: number }) => row.place_id)).toEqual([hotel.id, dinner.id]); @@ -1014,7 +1014,7 @@ describe('the day stop a booking implies', () => { createDayAssignment(testDb, day.id, place.id); const stay = createDayAccommodation(testDb, trip.id, place.id, day.id, day.id) as any; - const existing = svc.getAccommodation(stay.id, trip.id)!; + const existing = (await svc.getAccommodation(stay.id, trip.id))!; await svc.updateAccommodation(stay.id, existing, { start_day_id: second.id, end_day_id: second.id }); expect(stopsOn(second.id)).toEqual([]); @@ -1219,7 +1219,7 @@ describe('the drawn roads a booking moves', () => { const afterB = addVia(day.id, 2); const afterC = addVia(day.id, 3); - const existing = svc.getAccommodation(accommodation.id, trip.id)!; + const existing = (await svc.getAccommodation(accommodation.id, trip.id))!; const { mirror } = (await svc.updateAccommodation(accommodation.id, existing, { check_in: '20:00' })) as any; // The night is last now. The road out of it has no leg left and goes; the roads @@ -1284,7 +1284,7 @@ describe('the drawn roads a booking moves', () => { const afterHotel = addVia(day1.id, 1); const outOfC = addVia(day2.id, 0); - const existing = svc.getAccommodation(accommodation.id, trip.id)!; + const existing = (await svc.getAccommodation(accommodation.id, trip.id))!; const { mirror } = (await svc.updateAccommodation(accommodation.id, existing, { start_day_id: day2.id, end_day_id: day2.id })) as any; expect(stopsOn(day1.id)).toEqual([a.id, b.id]); @@ -1322,7 +1322,7 @@ describe('the drawn roads a booking moves', () => { const afterB = addVia(day.id, 2); addVia(day.id, 3); - const existing = svc.getAccommodation(accommodation.id, trip.id)!; + const existing = (await svc.getAccommodation(accommodation.id, trip.id))!; const { mirror } = (await svc.updateAccommodation(accommodation.id, existing, { notes: 'late arrival' })) as any; expect(stopsOn(day.id)).toEqual([a.id, hotel.id, b.id, c.id]); diff --git a/server/tests/unit/nest/assignments.controller.test.ts b/server/tests/unit/nest/assignments.controller.test.ts index d465f4415..07706f29a 100644 --- a/server/tests/unit/nest/assignments.controller.test.ts +++ b/server/tests/unit/nest/assignments.controller.test.ts @@ -15,15 +15,6 @@ function svc(o: Partial = {}): AssignmentsService { } as unknown as AssignmentsService; } -function thrown(fn: () => unknown): { status: number; body: unknown } { - try { fn(); } catch (err) { - expect(err).toBeInstanceOf(HttpException); - const e = err as HttpException; - return { status: e.getStatus(), body: e.getResponse() }; - } - throw new Error('expected throw'); -} - async function thrownAsync(fn: () => Promise): Promise<{ status: number; body: unknown }> { try { await fn(); } catch (err) { expect(err).toBeInstanceOf(HttpException); @@ -58,10 +49,10 @@ describe('DayAssignmentsController (parity with the legacy day-assignments route }); }); - it('PUT /reorder 404 day, else reorders + broadcasts', () => { - expect(thrown(() => new DayAssignmentsController(svc({ dayExists: vi.fn().mockReturnValue(false) } as Partial)).reorder(user, '5', '3', { orderedIds: [1, 2] }))).toEqual({ status: 404, body: { error: 'Day not found' } }); + it('PUT /reorder 404 day, else reorders + broadcasts', async () => { + expect(await thrownAsync(() => new DayAssignmentsController(svc({ dayExists: vi.fn().mockReturnValue(false) } as Partial)).reorder(user, '5', '3', { orderedIds: [1, 2] }))).toEqual({ status: 404, body: { error: 'Day not found' } }); const reorderAssignments = vi.fn(); const broadcast = vi.fn(); - expect(new DayAssignmentsController(svc({ reorderAssignments, broadcast } as Partial)).reorder(user, '5', '3', { orderedIds: [2, 1] }, 'sock')).toEqual({ success: true }); + expect(await new DayAssignmentsController(svc({ reorderAssignments, broadcast } as Partial)).reorder(user, '5', '3', { orderedIds: [2, 1] }, 'sock')).toEqual({ success: true }); expect(reorderAssignments).toHaveBeenCalledWith('3', [2, 1]); expect(broadcast).toHaveBeenCalledWith('5', 'assignment:reordered', { dayId: 3, orderedIds: [2, 1] }, 'sock'); }); @@ -99,11 +90,11 @@ describe('AssignmentOpsController (parity with the per-assignment op routes)', ( expect(reconcile).toHaveBeenCalledWith('5', 'sock'); }); - it('GET /:id/participants 404 when the assignment is on another trip, else returns participants (access-only)', () => { - expect(thrown(() => new AssignmentOpsController(svc({ getAssignmentForTrip: vi.fn().mockReturnValue(undefined) } as Partial)).participants(user, '5', '9'))) + it('GET /:id/participants 404 when the assignment is on another trip, else returns participants (access-only)', async () => { + expect(await thrownAsync(() => new AssignmentOpsController(svc({ getAssignmentForTrip: vi.fn().mockReturnValue(undefined) } as Partial)).participants(user, '5', '9'))) .toEqual({ status: 404, body: { error: 'Assignment not found' } }); const s = svc({ getAssignmentForTrip: vi.fn().mockReturnValue({ id: 9 }), getParticipants: vi.fn().mockReturnValue([{ user_id: 2 }]) } as Partial); - expect(new AssignmentOpsController(s).participants(user, '5', '9')).toEqual({ participants: [{ user_id: 2 }] }); + expect(await new AssignmentOpsController(s).participants(user, '5', '9')).toEqual({ participants: [{ user_id: 2 }] }); }); it('PUT /:id/time 404 missing, else updates', async () => { @@ -145,13 +136,13 @@ describe('AssignmentOpsController (parity with the per-assignment op routes)', ( expect(reconcile).not.toHaveBeenCalled(); }); - it('PUT /:id/participants 404 on a foreign assignment, else sets + broadcasts (non-array bodies are the Zod pipe\'s 400, covered in e2e)', () => { + it('PUT /:id/participants 404 on a foreign assignment, else sets + broadcasts (non-array bodies are the Zod pipe\'s 400, covered in e2e)', async () => { const setParticipants = vi.fn().mockReturnValue([{ user_id: 2 }]); const broadcast = vi.fn(); - expect(thrown(() => new AssignmentOpsController(svc({ getAssignmentForTrip: vi.fn().mockReturnValue(undefined), setParticipants } as Partial)).setParticipants(user, '5', '9', { user_ids: [2] }))) + expect(await thrownAsync(() => new AssignmentOpsController(svc({ getAssignmentForTrip: vi.fn().mockReturnValue(undefined), setParticipants } as Partial)).setParticipants(user, '5', '9', { user_ids: [2] }))) .toEqual({ status: 404, body: { error: 'Assignment not found' } }); expect(setParticipants).not.toHaveBeenCalled(); const s = svc({ getAssignmentForTrip: vi.fn().mockReturnValue({ id: 9 }), setParticipants, broadcast } as Partial); - expect(new AssignmentOpsController(s).setParticipants(user, '5', '9', { user_ids: [2] }, 'sock')).toEqual({ participants: [{ user_id: 2 }] }); + expect(await new AssignmentOpsController(s).setParticipants(user, '5', '9', { user_ids: [2] }, 'sock')).toEqual({ participants: [{ user_id: 2 }] }); // The trip comes along so the service can confine the ids to its roster. expect(setParticipants).toHaveBeenCalledWith('9', [2], '5'); expect(broadcast).toHaveBeenCalledWith('5', 'assignment:participants', { assignmentId: 9, participants: [{ user_id: 2 }] }, 'sock'); diff --git a/server/tests/unit/nest/assignments.service.test.ts b/server/tests/unit/nest/assignments.service.test.ts index 2017118ff..72a870735 100644 --- a/server/tests/unit/nest/assignments.service.test.ts +++ b/server/tests/unit/nest/assignments.service.test.ts @@ -70,6 +70,7 @@ beforeAll(async () => { // Real collaborator rather than a stub: reconcile() runs after every mutation // and needs the same connection to see the rows these cases write. new JourneyDomainService(dbs, realtime, new TrekPhotosRepository(dbs), await createTestUnitOfWork(dbs.connection)), + await createTestUnitOfWork(dbs.connection), ); }); @@ -107,17 +108,17 @@ function fixture() { // ── verifyTripAccess / canEdit ──────────────────────────────────────────────── describe('verifyTripAccess', () => { - it('ASG-SVC-001: returns trip for owner', () => { + it('ASG-SVC-001: returns trip for owner', async () => { const { user, trip } = fixture(); - const result = svc.verifyTripAccess(trip.id, user.id); + const result = await svc.verifyTripAccess(trip.id, user.id); expect(result).toBeDefined(); expect((result as { user_id: number }).user_id).toBe(user.id); }); - it('ASG-SVC-002: returns falsy for non-member', () => { + it('ASG-SVC-002: returns falsy for non-member', async () => { const { trip } = fixture(); const { user: stranger } = createUser(testDb); - expect(svc.verifyTripAccess(trip.id, stranger.id)).toBeFalsy(); + expect(await svc.verifyTripAccess(trip.id, stranger.id)).toBeFalsy(); }); }); @@ -134,30 +135,30 @@ describe('canEdit', () => { // ── existence checks ────────────────────────────────────────────────────────── describe('dayExists / placeExists / assignmentExistsInDay', () => { - it('ASG-SVC-004: dayExists is trip-scoped', () => { + it('ASG-SVC-004: dayExists is trip-scoped', async () => { const { trip, day } = fixture(); const { user: other } = createUser(testDb); const otherTrip = createTrip(testDb, other.id); - expect(svc.dayExists(day.id, trip.id)).toBe(true); - expect(svc.dayExists(day.id, otherTrip.id)).toBe(false); - expect(svc.dayExists(9999, trip.id)).toBe(false); + expect(await svc.dayExists(day.id, trip.id)).toBe(true); + expect(await svc.dayExists(day.id, otherTrip.id)).toBe(false); + expect(await svc.dayExists(9999, trip.id)).toBe(false); }); - it('ASG-SVC-005: placeExists is trip-scoped', () => { + it('ASG-SVC-005: placeExists is trip-scoped', async () => { const { trip, place } = fixture(); const { user: other } = createUser(testDb); const otherTrip = createTrip(testDb, other.id); - expect(svc.placeExists(place.id, trip.id)).toBe(true); - expect(svc.placeExists(place.id, otherTrip.id)).toBe(false); + expect(await svc.placeExists(place.id, trip.id)).toBe(true); + expect(await svc.placeExists(place.id, otherTrip.id)).toBe(false); }); - it('ASG-SVC-006: assignmentExistsInDay requires matching day AND trip', () => { + it('ASG-SVC-006: assignmentExistsInDay requires matching day AND trip', async () => { const { trip, day, place } = fixture(); const otherDay = createDay(testDb, trip.id); const a = createDayAssignment(testDb, day.id, place.id); - expect(svc.assignmentExistsInDay(a.id, day.id, trip.id)).toBe(true); - expect(svc.assignmentExistsInDay(a.id, otherDay.id, trip.id)).toBe(false); - expect(svc.assignmentExistsInDay(a.id, day.id, trip.id + 1)).toBe(false); + expect(await svc.assignmentExistsInDay(a.id, day.id, trip.id)).toBe(true); + expect(await svc.assignmentExistsInDay(a.id, otherDay.id, trip.id)).toBe(false); + expect(await svc.assignmentExistsInDay(a.id, day.id, trip.id + 1)).toBe(false); }); }); @@ -166,10 +167,10 @@ describe('dayExists / placeExists / assignmentExistsInDay', () => { describe('createAssignment', () => { it('ASG-SVC-007: first assignment gets order_index 0, then MAX+1 appends', async () => { const { day, place } = fixture(); - const first = svc.createAssignment(day.id, place.id, null); - const second = svc.createAssignment(day.id, place.id, null); - expect((await first!).order_index).toBe(0); - expect((await second!).order_index).toBe(1); + const first = await svc.createAssignment(day.id, place.id, null); + const second = await svc.createAssignment(day.id, place.id, null); + expect(first!.order_index).toBe(0); + expect(second!.order_index).toBe(1); }); it('ASG-SVC-008: empty-string notes coerce to null (`notes || null`)', async () => { @@ -210,7 +211,7 @@ describe('listDayAssignments', () => { testDb.prepare('INSERT INTO place_tags (place_id, tag_id) VALUES (?, ?)').run(place.id, tag.id); const a1 = createDayAssignment(testDb, day.id, place.id, { order_index: 1 }); const a2 = createDayAssignment(testDb, day.id, second.id, { order_index: 0 }); - svc.setParticipants(a1.id, [user.id], trip.id); + await svc.setParticipants(a1.id, [user.id], trip.id); const list = await svc.listDayAssignments(day.id); expect(list.map(a => a.id)).toEqual([a2.id, a1.id]); @@ -230,21 +231,21 @@ describe('listDayAssignments', () => { // ── delete / reorder ────────────────────────────────────────────────────────── describe('deleteAssignment / reorderAssignments', () => { - it('ASG-SVC-012: deleteAssignment removes the row', () => { + it('ASG-SVC-012: deleteAssignment removes the row', async () => { const { day, place } = fixture(); const a = createDayAssignment(testDb, day.id, place.id); - svc.deleteAssignment(a.id); + await svc.deleteAssignment(a.id); expect(testDb.prepare('SELECT id FROM day_assignments WHERE id = ?').get(a.id)).toBeUndefined(); }); - it('ASG-SVC-013: reorderAssignments applies positional order, scoped to the day', () => { + it('ASG-SVC-013: reorderAssignments applies positional order, scoped to the day', async () => { const { trip, day, place } = fixture(); const otherDay = createDay(testDb, trip.id); const a1 = createDayAssignment(testDb, day.id, place.id, { order_index: 0 }); const a2 = createDayAssignment(testDb, day.id, place.id, { order_index: 1 }); const foreign = createDayAssignment(testDb, otherDay.id, place.id, { order_index: 5 }); - svc.reorderAssignments(day.id, [a2.id, a1.id, foreign.id]); + await svc.reorderAssignments(day.id, [a2.id, a1.id, foreign.id]); const order = (id: number) => (testDb.prepare('SELECT order_index FROM day_assignments WHERE id = ?').get(id) as { order_index: number }).order_index; expect(order(a2.id)).toBe(0); @@ -257,11 +258,11 @@ describe('deleteAssignment / reorderAssignments', () => { // ── getAssignmentForTrip / moveAssignment ───────────────────────────────────── describe('getAssignmentForTrip', () => { - it('ASG-SVC-014: returns the raw row for the trip, undefined cross-trip', () => { + it('ASG-SVC-014: returns the raw row for the trip, undefined cross-trip', async () => { const { trip, day, place } = fixture(); const a = createDayAssignment(testDb, day.id, place.id); - expect(svc.getAssignmentForTrip(a.id, trip.id)).toMatchObject({ id: a.id, day_id: day.id }); - expect(svc.getAssignmentForTrip(a.id, trip.id + 1)).toBeUndefined(); + expect(await svc.getAssignmentForTrip(a.id, trip.id)).toMatchObject({ id: a.id, day_id: day.id }); + expect(await svc.getAssignmentForTrip(a.id, trip.id + 1)).toBeUndefined(); }); }); @@ -288,37 +289,37 @@ describe('moveAssignment', () => { // ── participants ────────────────────────────────────────────────────────────── describe('getParticipants / setParticipants', () => { - it('ASG-SVC-017: setParticipants replaces the list and returns the joined rows', () => { + it('ASG-SVC-017: setParticipants replaces the list and returns the joined rows', async () => { const { user, trip, day, place } = fixture(); const { user: peer } = createUser(testDb); addTripMember(testDb, trip.id, peer.id); const a = createDayAssignment(testDb, day.id, place.id); - svc.setParticipants(a.id, [user.id], trip.id); - const replaced = svc.setParticipants(a.id, [peer.id], trip.id); + await svc.setParticipants(a.id, [user.id], trip.id); + const replaced = await svc.setParticipants(a.id, [peer.id], trip.id); expect(replaced).toEqual([{ user_id: peer.id, username: peer.username, avatar: null }]); - expect(svc.getParticipants(a.id)).toEqual(replaced); + expect(await svc.getParticipants(a.id)).toEqual(replaced); }); - it('ASG-SVC-018: empty array clears all participants', () => { + it('ASG-SVC-018: empty array clears all participants', async () => { const { user, trip, day, place } = fixture(); const a = createDayAssignment(testDb, day.id, place.id); - svc.setParticipants(a.id, [user.id], trip.id); - expect(svc.setParticipants(a.id, [], trip.id)).toEqual([]); - expect(svc.getParticipants(a.id)).toEqual([]); + await svc.setParticipants(a.id, [user.id], trip.id); + expect(await svc.setParticipants(a.id, [], trip.id)).toEqual([]); + expect(await svc.getParticipants(a.id)).toEqual([]); }); - it('ASG-SVC-019: username COALESCEs display_name over username', () => { + it('ASG-SVC-019: username COALESCEs display_name over username', async () => { const { user, trip, day, place } = fixture(); testDb.prepare('UPDATE users SET display_name = ? WHERE id = ?').run('Fancy Name', user.id); const a = createDayAssignment(testDb, day.id, place.id); - const rows = svc.setParticipants(a.id, [user.id], trip.id); + const rows = await svc.setParticipants(a.id, [user.id], trip.id); expect(rows).toEqual([{ user_id: user.id, username: 'Fancy Name', avatar: null }]); }); - it('ASG-SVC-020: duplicate user ids collapse via INSERT OR IGNORE', () => { + it('ASG-SVC-020: duplicate user ids collapse via INSERT OR IGNORE', async () => { const { user, trip, day, place } = fixture(); const a = createDayAssignment(testDb, day.id, place.id); - const rows = svc.setParticipants(a.id, [user.id, user.id], trip.id); + const rows = await svc.setParticipants(a.id, [user.id, user.id], trip.id); expect(rows).toHaveLength(1); }); @@ -328,22 +329,22 @@ describe('getParticipants / setParticipants', () => { // list, and the delete stands. The rollback itself is still covered — the // transaction wraps delete and insert together, and ASG-SVC-018 pins that an // explicit empty list clears. - it('ASG-SVC-028: a list of ids that are not on the trip clears rather than throwing', () => { + it('ASG-SVC-028: a list of ids that are not on the trip clears rather than throwing', async () => { const { user, trip, day, place } = fixture(); const a = createDayAssignment(testDb, day.id, place.id); - svc.setParticipants(a.id, [user.id], trip.id); - expect(() => svc.setParticipants(a.id, [999999], trip.id)).not.toThrow(); - expect(svc.getParticipants(a.id)).toEqual([]); + await svc.setParticipants(a.id, [user.id], trip.id); + await expect(svc.setParticipants(a.id, [999999], trip.id)).resolves.toBeDefined(); + expect(await svc.getParticipants(a.id)).toEqual([]); }); - it('ASG-SVC-029: keeps the trip members and drops the stranger in one call', () => { + it('ASG-SVC-029: keeps the trip members and drops the stranger in one call', async () => { const { user, trip, day, place } = fixture(); const { user: member } = createUser(testDb); const { user: stranger } = createUser(testDb); addTripMember(testDb, trip.id, member.id); const a = createDayAssignment(testDb, day.id, place.id); - const rows = svc.setParticipants(a.id, [user.id, stranger.id, member.id], trip.id) as { user_id: number }[]; + const rows = await svc.setParticipants(a.id, [user.id, stranger.id, member.id], trip.id) as { user_id: number }[]; expect(rows.map(r => r.user_id).sort()).toEqual([user.id, member.id].sort()); expect(JSON.stringify(rows)).not.toContain(stranger.username); @@ -429,7 +430,7 @@ describe('updateTime', () => { expect(dayOrder(day.id)).toEqual([b, a, c]); // Dragged back out of time order on purpose: a sort now would put b first again, // with c behind it. - svc.reorderAssignments(day.id, [a, b, c]); + await svc.reorderAssignments(day.id, [a, b, c]); const { assignment } = await svc.updateTime(c, null, null); // clear, no re-sort expect((await assignment!).assignment_time).toBeNull(); @@ -440,7 +441,7 @@ describe('updateTime', () => { const { day, ids: [a, b, c] } = dayOf([['09:00', 0], [null, 1], ['12:00', 2]]); await svc.updateTime(b, '06:00', '07:00'); // sorts b first expect(dayOrder(day.id)).toEqual([b, a, c]); - svc.reorderAssignments(day.id, [a, b, c]); + await svc.reorderAssignments(day.id, [a, b, c]); const { assignment } = await svc.updateTime(c, '', ''); expect((await assignment!).assignment_time).toBeNull(); diff --git a/server/tests/unit/nest/places.controller.test.ts b/server/tests/unit/nest/places.controller.test.ts index 2bd34e547..ea7c1c5d8 100644 --- a/server/tests/unit/nest/places.controller.test.ts +++ b/server/tests/unit/nest/places.controller.test.ts @@ -23,14 +23,6 @@ function svc(o: Partial = {}): PlacesService { } as unknown as PlacesService; } -function thrown(fn: () => unknown): { status: number; body: unknown } { - try { fn(); } catch (err) { - expect(err).toBeInstanceOf(HttpException); - const e = err as HttpException; - return { status: e.getStatus(), body: e.getResponse() }; - } - throw new Error('expected throw'); -} async function thrownAsync(fn: () => Promise): Promise<{ status: number; body: unknown }> { try { await fn(); } catch (err) { expect(err).toBeInstanceOf(HttpException); @@ -151,18 +143,18 @@ describe('PlacesController (parity with the legacy /api/trips/:tripId/places rou const makeRes = () => ({ setHeader: vi.fn(), send: vi.fn() }); const exportQuery = {} as never; - it('an ASCII filename keeps the exact legacy header', () => { + it('an ASCII filename keeps the exact legacy header', async () => { const res = makeRes(); const s = svc({ exportGpx: vi.fn().mockReturnValue({ gpx: '', filename: 'Alpine-week.gpx' }) } as Partial); - new PlacesController(s, new RuntimeEnvService(), storageStub).exportGpx(user, '5', exportQuery, res as never); + await new PlacesController(s, new RuntimeEnvService(), storageStub).exportGpx(user, '5', exportQuery, res as never); expect(res.setHeader).toHaveBeenCalledWith('Content-Disposition', 'attachment; filename="Alpine-week.gpx"'); expect(res.send).toHaveBeenCalledWith(''); }); - it('a non-ASCII filename no longer reaches setHeader raw: ASCII fallback + filename*', () => { + it('a non-ASCII filename no longer reaches setHeader raw: ASCII fallback + filename*', async () => { const res = makeRes(); const s = svc({ exportGpx: vi.fn().mockReturnValue({ gpx: '', filename: '沖縄-4泊5日.gpx' }) } as Partial); - new PlacesController(s, new RuntimeEnvService(), storageStub).exportGpx(user, '5', exportQuery, res as never); + await new PlacesController(s, new RuntimeEnvService(), storageStub).exportGpx(user, '5', exportQuery, res as never); expect(res.setHeader).toHaveBeenCalledWith( 'Content-Disposition', 'attachment; filename="__-4_5_.gpx"; filename*=UTF-8\'\'%E6%B2%96%E7%B8%84-4%E6%B3%8A5%E6%97%A5.gpx', @@ -264,6 +256,26 @@ describe('PlacesController (parity with the legacy /api/trips/:tripId/places rou expect(onDeleted.mock.invocationCallOrder[0]).toBeLessThan(removeMany.mock.invocationCallOrder[0]); }); + // invocationCallOrder above only proves the hook was CALLED first — a mock + // that resolves synchronously can't tell an awaited hook from a detached + // (fire-and-forget) one, since both invoke onDeleted() before removeMany() is + // invoked either way. A hook stubbed with a real macrotask hop, and a delete + // that records when it actually RUNS, catches a regression to detached. + it('the journey hook actually finishes before the delete runs, not just gets called first', async () => { + const order: string[] = []; + const onDeleted = vi.fn(async () => { + await new Promise((resolve) => setTimeout(resolve, 0)); + order.push('hook'); + }); + const removeMany = vi.fn(async () => { + order.push('delete'); + return { deleted: [1], cancelled: { reservationIds: [], budgetItemIds: [] } }; + }); + const s = svc({ removeMany, onDeleted, scopedIds: vi.fn().mockReturnValue([1]), broadcast: vi.fn() } as Partial); + await new PlacesController(s, new RuntimeEnvService(), storageStub).bulkDelete(user, '5', { ids: [1] }); + expect(order).toEqual(['hook', 'delete']); + }); + // #1298: the link is gone once the place is, so the ids have to be read first. it('announces the expenses the deleted places took with them', async () => { const removeMany = vi.fn().mockReturnValue({ deleted: [1, 2], cancelled: { reservationIds: [], budgetItemIds: [] } }); @@ -325,10 +337,10 @@ describe('PlacesController (parity with the legacy /api/trips/:tripId/places rou }); }); - it('GET /:id returns the place when found, 404 when missing', () => { - expect(thrown(() => new PlacesController(svc({ get: vi.fn().mockReturnValue(undefined) } as Partial), new RuntimeEnvService(), storageStub).get(user, '5', '9'))).toEqual({ status: 404, body: { error: 'Place not found' } }); + it('GET /:id returns the place when found, 404 when missing', async () => { + expect(await thrownAsync(() => new PlacesController(svc({ get: vi.fn().mockReturnValue(undefined) } as Partial), new RuntimeEnvService(), storageStub).get(user, '5', '9'))).toEqual({ status: 404, body: { error: 'Place not found' } }); const s = svc({ get: vi.fn().mockReturnValue({ id: 9 }) } as Partial); - expect(new PlacesController(s, new RuntimeEnvService(), storageStub).get(user, '5', '9')).toEqual({ place: { id: 9 } }); + expect(await new PlacesController(s, new RuntimeEnvService(), storageStub).get(user, '5', '9')).toEqual({ place: { id: 9 } }); }); it('PUT /:id 404 when missing, else updates + hooks', async () => { @@ -446,6 +458,26 @@ describe('PlacesController (parity with the legacy /api/trips/:tripId/places rou expect(await new PlacesController(s, new RuntimeEnvService(), storageStub).remove(user, '5', '9')).toEqual({ success: true }); }); + // invocationCallOrder in the case above only proves the hook was CALLED + // first — a synchronously-resolving mock can't tell an awaited hook from a + // detached (fire-and-forget) one. A hook stubbed with a real macrotask hop, + // and a delete that records when it actually RUNS, catches a regression to + // detached (the trap this pins: onDeleted must finish before the DELETE). + it('DELETE /:id — the journey hook actually finishes before the delete runs, not just gets called first', async () => { + const order: string[] = []; + const onDeleted = vi.fn(async () => { + await new Promise((resolve) => setTimeout(resolve, 0)); + order.push('hook'); + }); + const remove = vi.fn(async () => { + order.push('delete'); + return { deleted: true, cancelled: { reservationIds: [], budgetItemIds: [] } }; + }); + const s = svc({ remove, onDeleted, broadcast: vi.fn() } as Partial); + await new PlacesController(s, new RuntimeEnvService(), storageStub).remove(user, '5', '9'); + expect(order).toEqual(['hook', 'delete']); + }); + it('DELETE /:id announces the booking and the expense a cancelled night took down', async () => { // place:deleted says nothing about either, and an expense linked by // reservation_id is not one linkedExpenseIds finds. diff --git a/server/tests/unit/nest/places.service.test.ts b/server/tests/unit/nest/places.service.test.ts index fecae4f77..d6330638b 100644 --- a/server/tests/unit/nest/places.service.test.ts +++ b/server/tests/unit/nest/places.service.test.ts @@ -197,20 +197,20 @@ describe('list', () => { // ── create ──────────────────────────────────────────────────────────────────── describe('create', () => { - it('PLACE-SVC-007 — creates a place and returns it with tags array', () => { + it('PLACE-SVC-007 — creates a place and returns it with tags array', async () => { const { user } = createUser(testDb); const trip = createTrip(testDb, user.id); - const place = svc.create(String(trip.id), { name: 'New Place', lat: 48.8, lng: 2.3 }) as any; + const place = await svc.create(String(trip.id), { name: 'New Place', lat: 48.8, lng: 2.3 }) as any; expect(place).toBeDefined(); expect(place.name).toBe('New Place'); expect(Array.isArray(place.tags)).toBe(true); }); - it('PLACE-SVC-008 — creates a place with tags', () => { + it('PLACE-SVC-008 — creates a place with tags', async () => { const { user } = createUser(testDb); const trip = createTrip(testDb, user.id); const tag = createTag(testDb, user.id, { name: 'Highlight' }) as any; - const place = svc.create(String(trip.id), { name: 'Tagged Place', tags: [tag.id] }) as any; + const place = await svc.create(String(trip.id), { name: 'Tagged Place', tags: [tag.id] }) as any; expect(place.tags).toHaveLength(1); expect(place.tags[0].id).toBe(tag.id); }); @@ -218,34 +218,34 @@ describe('create', () => { // A tag belongs to a user, not a trip, and the place body is an open record, so // an id from someone outside the trip could be attached and then read straight // back — the tag projection carries the owner's user_id with it. - it('PLACE-SVC-008a — drops a tag owned by someone outside the trip', () => { + it('PLACE-SVC-008a — drops a tag owned by someone outside the trip', async () => { const { user } = createUser(testDb); const { user: outsider } = createUser(testDb); const trip = createTrip(testDb, user.id); const mine = createTag(testDb, user.id, { name: 'Mine' }) as any; const theirs = createTag(testDb, outsider.id, { name: 'Theirs' }) as any; - const place = svc.create(String(trip.id), { name: 'Tagged Place', tags: [mine.id, theirs.id] }) as any; + const place = await svc.create(String(trip.id), { name: 'Tagged Place', tags: [mine.id, theirs.id] }) as any; expect(place.tags.map((t: any) => t.id)).toEqual([mine.id]); }); - it('PLACE-SVC-008b — keeps a co-traveller tag, so a shared place survives a foreign re-save', () => { + it('PLACE-SVC-008b — keeps a co-traveller tag, so a shared place survives a foreign re-save', async () => { const { user: owner } = createUser(testDb); const { user: member } = createUser(testDb); const trip = createTrip(testDb, owner.id); addTripMember(testDb, trip.id, member.id); const theirTag = createTag(testDb, member.id, { name: 'Theirs' }) as any; - const place = svc.create(String(trip.id), { name: 'Shared Place', tags: [theirTag.id] }) as any; + const place = await svc.create(String(trip.id), { name: 'Shared Place', tags: [theirTag.id] }) as any; expect(place.tags.map((t: any) => t.id)).toEqual([theirTag.id]); }); - it('PLACE-SVC-009 — place is associated with correct trip', () => { + it('PLACE-SVC-009 — place is associated with correct trip', async () => { const { user } = createUser(testDb); const trip = createTrip(testDb, user.id); - const place = svc.create(String(trip.id), { name: 'My Place' }) as any; + const place = await svc.create(String(trip.id), { name: 'My Place' }) as any; const row = testDb.prepare('SELECT trip_id FROM places WHERE id = ?').get(place.id) as any; expect(row.trip_id).toBe(trip.id); }); @@ -254,27 +254,27 @@ describe('create', () => { // ── get ─────────────────────────────────────────────────────────────────────── describe('get', () => { - it('PLACE-SVC-010 — returns the place when tripId and placeId match', () => { + it('PLACE-SVC-010 — returns the place when tripId and placeId match', async () => { const { user } = createUser(testDb); const trip = createTrip(testDb, user.id); const place = createPlace(testDb, trip.id, { name: 'Find Me' }) as any; - const found = svc.get(String(trip.id), String(place.id)) as any; + const found = await svc.get(String(trip.id), String(place.id)) as any; expect(found).toBeDefined(); expect(found.name).toBe('Find Me'); }); - it('PLACE-SVC-011 — returns null when place belongs to different trip', () => { + it('PLACE-SVC-011 — returns null when place belongs to different trip', async () => { const { user } = createUser(testDb); const t1 = createTrip(testDb, user.id); const t2 = createTrip(testDb, user.id); const place = createPlace(testDb, t1.id, { name: 'T1 Place' }) as any; - expect(svc.get(String(t2.id), String(place.id))).toBeNull(); + expect(await svc.get(String(t2.id), String(place.id))).toBeNull(); }); - it('PLACE-SVC-012 — returns null for non-existent placeId', () => { + it('PLACE-SVC-012 — returns null for non-existent placeId', async () => { const { user } = createUser(testDb); const trip = createTrip(testDb, user.id); - expect(svc.get(String(trip.id), '99999')).toBeNull(); + expect(await svc.get(String(trip.id), '99999')).toBeNull(); }); }); @@ -302,7 +302,7 @@ describe('update', () => { const trip = createTrip(testDb, user.id); const tag1 = createTag(testDb, user.id, { name: 'Old Tag' }) as any; const tag2 = createTag(testDb, user.id, { name: 'New Tag' }) as any; - const place = svc.create(String(trip.id), { name: 'Taggable', tags: [tag1.id] }) as any; + const place = await svc.create(String(trip.id), { name: 'Taggable', tags: [tag1.id] }) as any; const updated = await svc.update(String(trip.id), String(place.id), { tags: [tag2.id] }) as any; expect(updated.tags).toHaveLength(1); @@ -313,7 +313,7 @@ describe('update', () => { const { user } = createUser(testDb); const trip = createTrip(testDb, user.id); const tag = createTag(testDb, user.id, { name: 'Temp' }) as any; - const place = svc.create(String(trip.id), { name: 'Untaggable', tags: [tag.id] }) as any; + const place = await svc.create(String(trip.id), { name: 'Untaggable', tags: [tag.id] }) as any; const updated = await svc.update(String(trip.id), String(place.id), { tags: [] }) as any; expect(updated.tags).toHaveLength(0); @@ -350,10 +350,10 @@ describe('update', () => { expect(renamed.route_color).toBe('#059669'); }); - it('PLACE-SVC-055 — create carries geometry and colour instead of dropping them', () => { + it('PLACE-SVC-055 — create carries geometry and colour instead of dropping them', async () => { const { user } = createUser(testDb); const trip = createTrip(testDb, user.id); - const created = svc.create(String(trip.id), { + const created = await svc.create(String(trip.id), { name: 'Restored track', route_geometry: '[[48.0,2.0],[49.0,3.0]]', route_color: '#7c3aed', @@ -396,7 +396,7 @@ describe('updateMany', () => { expect(updated).toHaveLength(1); expect((updated[0] as any).id).toBe(mine.id); // The place from the other trip stays untouched. - expect((svc.get(String(other.id), String(foreign.id)) as any).notes).toBeNull(); + expect((await svc.get(String(other.id), String(foreign.id)) as any).notes).toBeNull(); }); it('PLACE-SVC-041 — returns [] for an empty id list', async () => { @@ -414,7 +414,7 @@ describe('remove', () => { const trip = createTrip(testDb, user.id); const place = createPlace(testDb, trip.id, { name: 'To Delete' }) as any; expect((await svc.remove(String(trip.id), String(place.id))).deleted).toBe(true); - expect(svc.get(String(trip.id), String(place.id))).toBeNull(); + expect(await svc.get(String(trip.id), String(place.id))).toBeNull(); }); it('PLACE-SVC-018 — returns false for non-existent place', async () => { @@ -480,7 +480,7 @@ describe('remove', () => { const standalone = Number(testDb.prepare("INSERT INTO budget_items (trip_id, name, total_price) VALUES (?, 'Coffee', 3)").run(trip.id).lastInsertRowid); // Read the link before the delete — that is what the controller broadcasts. - expect(svc.linkedExpenseIds(trip.id, [place.id])).toEqual([linked]); + expect(await svc.linkedExpenseIds(trip.id, [place.id])).toEqual([linked]); expect((await svc.remove(String(trip.id), String(place.id))).deleted).toBe(true); const rows = testDb.prepare('SELECT id FROM budget_items ORDER BY id').all() as { id: number }[]; @@ -497,22 +497,22 @@ describe('remove', () => { testDb.prepare("INSERT INTO budget_items (trip_id, name, total_price, place_id) VALUES (?, 'x', 1, ?)").run(trip.id, p.id); } - expect(svc.linkedExpenseIds(trip.id, [a.id, b.id])).toHaveLength(2); + expect(await svc.linkedExpenseIds(trip.id, [a.id, b.id])).toHaveLength(2); await svc.removeMany(String(trip.id), [a.id, b.id]); const rows = testDb.prepare('SELECT place_id FROM budget_items').all() as { place_id: number }[]; expect(rows.map(r => r.place_id)).toEqual([keep.id]); }); - it('PLACE-SVC-019e — linkedExpenseIds ignores places of another trip and an empty list', () => { + it('PLACE-SVC-019e — linkedExpenseIds ignores places of another trip and an empty list', async () => { const { user } = createUser(testDb); const trip = createTrip(testDb, user.id); const other = createTrip(testDb, user.id); const place = createPlace(testDb, other.id, { name: 'Elsewhere' }) as any; testDb.prepare("INSERT INTO budget_items (trip_id, name, total_price, place_id) VALUES (?, 'x', 1, ?)").run(other.id, place.id); - expect(svc.linkedExpenseIds(trip.id, [place.id])).toEqual([]); - expect(svc.linkedExpenseIds(trip.id, [])).toEqual([]); + expect(await svc.linkedExpenseIds(trip.id, [place.id])).toEqual([]); + expect(await svc.linkedExpenseIds(trip.id, [])).toEqual([]); }); it('PLACE-SVC-019b — reclaims the photo cache for the deleted place', async () => { @@ -542,7 +542,7 @@ describe('removeMany', () => { const { deleted } = await svc.removeMany(String(trip.id), [a.id, b.id, foreign.id, 99999]); expect(deleted.sort()).toEqual([a.id, b.id].sort()); - expect(svc.get(String(other.id), String(foreign.id))).not.toBeNull(); + expect(await svc.get(String(other.id), String(foreign.id))).not.toBeNull(); }); it('PLACE-SVC-057b — a place delete reports the booking and expense its cancelled night took down', async () => { @@ -580,27 +580,27 @@ describe('removeMany', () => { // ── importGpx ───────────────────────────────────────────────────────────────── describe('importGpx', () => { - it('PLACE-SVC-020 — returns null when buffer has no root', () => { + it('PLACE-SVC-020 — returns null when buffer has no root', async () => { const { user } = createUser(testDb); const trip = createTrip(testDb, user.id); - const result = svc.importGpx(String(trip.id), Buffer.from('')); + const result = await svc.importGpx(String(trip.id), Buffer.from('')); expect(result).toBeNull(); }); - it('PLACE-SVC-021 — imports waypoints as places', () => { + it('PLACE-SVC-021 — imports waypoints as places', async () => { const { user } = createUser(testDb); const trip = createTrip(testDb, user.id); const gpx = Buffer.from(` Paris London `); - const result = svc.importGpx(String(trip.id), gpx) as any; + const result = await svc.importGpx(String(trip.id), gpx) as any; expect(result.places).toHaveLength(2); expect(result.places[0].name).toBe('Paris'); expect(result.places[1].name).toBe('London'); }); - it('PLACE-SVC-022 — imports as a single polyline-place with routeGeometry', () => { + it('PLACE-SVC-022 — imports as a single polyline-place with routeGeometry', async () => { const { user } = createUser(testDb); const trip = createTrip(testDb, user.id); const gpx = Buffer.from(` @@ -610,7 +610,7 @@ describe('importGpx', () => { End `); - const result = svc.importGpx(String(trip.id), gpx) as any; + const result = await svc.importGpx(String(trip.id), gpx) as any; expect(result.places).toHaveLength(1); expect(result.places[0].name).toBe('My Route'); expect(result.places[0].lat).toBe(48.8566); @@ -620,7 +620,7 @@ describe('importGpx', () => { expect(coords).toHaveLength(2); }); - it('PLACE-SVC-023 — imports track as a single place with routeGeometry', () => { + it('PLACE-SVC-023 — imports track as a single place with routeGeometry', async () => { const { user } = createUser(testDb); const trip = createTrip(testDb, user.id); const gpx = Buffer.from(` @@ -632,7 +632,7 @@ describe('importGpx', () => { `); - const result = svc.importGpx(String(trip.id), gpx) as any; + const result = await svc.importGpx(String(trip.id), gpx) as any; expect(result.places).toHaveLength(1); expect(result.places[0].name).toBe('My Track'); const geometry = JSON.parse(result.places[0].route_geometry); @@ -640,7 +640,7 @@ describe('importGpx', () => { expect(geometry).toHaveLength(2); }); - it('PLACE-SVC-024 — and together: waypoints plus track appended', () => { + it('PLACE-SVC-024 — and together: waypoints plus track appended', async () => { const { user } = createUser(testDb); const trip = createTrip(testDb, user.id); const gpx = Buffer.from(` @@ -653,7 +653,7 @@ describe('importGpx', () => { `); - const result = svc.importGpx(String(trip.id), gpx) as any; + const result = await svc.importGpx(String(trip.id), gpx) as any; // 1 wpt + 1 trk expect(result.places).toHaveLength(2); const trackPlace = result.places.find((p: any) => p.name === 'Track') as any; @@ -662,15 +662,15 @@ describe('importGpx', () => { expect(geometry).toHaveLength(2); }); - it('PLACE-SVC-025 — returns null when GPX has no usable elements', () => { + it('PLACE-SVC-025 — returns null when GPX has no usable elements', async () => { const { user } = createUser(testDb); const trip = createTrip(testDb, user.id); const gpx = Buffer.from(``); - const result = svc.importGpx(String(trip.id), gpx); + const result = await svc.importGpx(String(trip.id), gpx); expect(result).toBeNull(); }); - it('PLACE-SVC-037 — multiple unnamed tracks in one file get distinct names instead of collapsing to one', () => { + it('PLACE-SVC-037 — multiple unnamed tracks in one file get distinct names instead of collapsing to one', async () => { const { user } = createUser(testDb); const trip = createTrip(testDb, user.id); const gpx = Buffer.from(` @@ -683,13 +683,13 @@ describe('importGpx', () => { `); - const result = svc.importGpx(String(trip.id), gpx) as any; + const result = await svc.importGpx(String(trip.id), gpx) as any; expect(result.places).toHaveLength(2); const names = result.places.map((p: any) => p.name); expect(new Set(names).size).toBe(2); }); - it('PLACE-SVC-038 — unnamed tracks fall back to the source filename', () => { + it('PLACE-SVC-038 — unnamed tracks fall back to the source filename', async () => { const { user } = createUser(testDb); const trip = createTrip(testDb, user.id); const gpx = Buffer.from(` @@ -698,7 +698,7 @@ describe('importGpx', () => { `); - const result = svc.importGpx(String(trip.id), gpx, { defaultName: 'morning-hike.gpx' }) as any; + const result = await svc.importGpx(String(trip.id), gpx, { defaultName: 'morning-hike.gpx' }) as any; expect(result.places).toHaveLength(1); expect(result.places[0].name).toBe('morning-hike'); }); @@ -985,11 +985,11 @@ describe('importGpx deduplication', () => { const buf = fs.readFileSync(GPX_FIXTURE); // First import - const first = svc.importGpx(String(trip.id), buf) as any; + const first = await svc.importGpx(String(trip.id), buf) as any; expect(first.count).toBeGreaterThan(0); // Second import — all names already present, nothing new created - const second = svc.importGpx(String(trip.id), buf) as any; + const second = await svc.importGpx(String(trip.id), buf) as any; expect(second.count).toBe(0); expect(second.skipped).toBe(first.count); @@ -1003,12 +1003,12 @@ describe('importGpx deduplication', () => { const trip = createTrip(testDb, user.id); const buf = fs.readFileSync(GPX_FIXTURE); - const first = svc.importGpx(String(trip.id), buf) as any; + const first = await svc.importGpx(String(trip.id), buf) as any; // Manually add a brand-new place so total > first.count createPlace(testDb, trip.id, { name: 'Unique Extra Place', lat: 99, lng: 99 }); // Re-import: the fixture places are skipped, the extra place remains untouched - const second = svc.importGpx(String(trip.id), buf) as any; + const second = await svc.importGpx(String(trip.id), buf) as any; expect(second.count).toBe(0); const total = ((await svc.list(String(trip.id), {})) as any[]).length; @@ -1017,21 +1017,21 @@ describe('importGpx deduplication', () => { }); describe('importKmlPlaces deduplication', () => { - it('PLACE-SVC-035 — skips placemarks already in trip by name', () => { + it('PLACE-SVC-035 — skips placemarks already in trip by name', async () => { const { user } = createUser(testDb); const trip = createTrip(testDb, user.id); const buf = fs.readFileSync(KML_FIXTURE); - const first = svc.importKmlPlaces(String(trip.id), buf); + const first = await svc.importKmlPlaces(String(trip.id), buf); expect(first.count).toBeGreaterThan(0); - const second = svc.importKmlPlaces(String(trip.id), buf); + const second = await svc.importKmlPlaces(String(trip.id), buf); expect(second.count).toBe(0); expect(second.summary.skippedCount).toBeGreaterThanOrEqual(first.count); expect(second.summary.warnings.some((w: string) => w.includes('skipped'))).toBe(true); }); - it('PLACE-SVC-036 — deduplicates within the same file (intra-batch)', () => { + it('PLACE-SVC-036 — deduplicates within the same file (intra-batch)', async () => { const { user } = createUser(testDb); const trip = createTrip(testDb, user.id); // Craft a KML with two placemarks sharing the same name @@ -1041,7 +1041,7 @@ describe('importKmlPlaces deduplication', () => { Dupe Place2.1,48.1,0 `); - const result = svc.importKmlPlaces(String(trip.id), kml); + const result = await svc.importKmlPlaces(String(trip.id), kml); expect(result.count).toBe(1); expect(result.summary.skippedCount).toBe(1); }); @@ -1113,31 +1113,31 @@ describe('custom place image reclaim', () => { }); // ── Collaborative ratings (#1435) ────────────────────────────────────────── - it('PLACE-SVC-050 — rate stores one vote per user, replaces on re-vote, clears with null', () => { + it('PLACE-SVC-050 — rate stores one vote per user, replaces on re-vote, clears with null', async () => { const { user } = createUser(testDb); const trip = createTrip(testDb, user.id); const place = createPlace(testDb, trip.id, { name: 'Rated' }) as { id: number }; - svc.rate(String(trip.id), String(place.id), user.id, 5); + await svc.rate(String(trip.id), String(place.id), user.id, 5); let rows = testDb.prepare('SELECT rating FROM place_ratings WHERE place_id = ? AND user_id = ?').all(place.id, user.id) as { rating: number }[]; expect(rows).toEqual([{ rating: 5 }]); - svc.rate(String(trip.id), String(place.id), user.id, 2); // re-vote replaces via the UNIQUE upsert + await svc.rate(String(trip.id), String(place.id), user.id, 2); // re-vote replaces via the UNIQUE upsert rows = testDb.prepare('SELECT rating FROM place_ratings WHERE place_id = ? AND user_id = ?').all(place.id, user.id) as { rating: number }[]; expect(rows).toEqual([{ rating: 2 }]); - svc.rate(String(trip.id), String(place.id), user.id, null); // clear + await svc.rate(String(trip.id), String(place.id), user.id, null); // clear const count = testDb.prepare('SELECT COUNT(*) AS n FROM place_ratings WHERE place_id = ?').get(place.id) as { n: number }; expect(count.n).toBe(0); }); - it('PLACE-SVC-051 — rate returns null and writes nothing when the place is not in the trip', () => { + it('PLACE-SVC-051 — rate returns null and writes nothing when the place is not in the trip', async () => { const { user } = createUser(testDb); const trip = createTrip(testDb, user.id); const otherTrip = createTrip(testDb, user.id); const place = createPlace(testDb, otherTrip.id, { name: 'Elsewhere' }) as { id: number }; - expect(svc.rate(String(trip.id), String(place.id), user.id, 4)).toBeNull(); + expect(await svc.rate(String(trip.id), String(place.id), user.id, 4)).toBeNull(); const count = testDb.prepare('SELECT COUNT(*) AS n FROM place_ratings WHERE place_id = ?').get(place.id) as { n: number }; expect(count.n).toBe(0); }); @@ -1161,8 +1161,8 @@ const GPX_WITH_TRACKS = ` `; -function importFixture(tripId: number) { - return svc.importGpx(String(tripId), Buffer.from(GPX_WITH_TRACKS), { +async function importFixture(tripId: number) { + return await svc.importGpx(String(tripId), Buffer.from(GPX_WITH_TRACKS), { importWaypoints: true, importRoutes: true, importTracks: true, }); } @@ -1173,10 +1173,10 @@ function tracksOf(tripId: number) { } describe('PlacesService — automatic track colours (#776)', () => { - it('PLACES-SVC-001 — every imported track gets a colour from the shared palette', () => { + it('PLACES-SVC-001 — every imported track gets a colour from the shared palette', async () => { const { user } = createUser(testDb); const trip = createTrip(testDb, user.id); - importFixture(trip.id); + await importFixture(trip.id); const tracks = tracksOf(trip.id); expect(tracks.length).toBeGreaterThan(0); @@ -1185,10 +1185,10 @@ describe('PlacesService — automatic track colours (#776)', () => { } }); - it('PLACES-SVC-002 — the returned places carry the colour, not just the DB rows', () => { + it('PLACES-SVC-002 — the returned places carry the colour, not just the DB rows', async () => { const { user } = createUser(testDb); const trip = createTrip(testDb, user.id); - const result = importFixture(trip.id) as { places: any[] } | null; + const result = await importFixture(trip.id) as { places: any[] } | null; const returnedTracks = (result?.places ?? []).filter(p => p.route_geometry); expect(returnedTracks.length).toBeGreaterThan(0); @@ -1197,10 +1197,10 @@ describe('PlacesService — automatic track colours (#776)', () => { } }); - it('PLACES-SVC-003 — plain waypoints keep no colour at all', () => { + it('PLACES-SVC-003 — plain waypoints keep no colour at all', async () => { const { user } = createUser(testDb); const trip = createTrip(testDb, user.id); - importFixture(trip.id); + await importFixture(trip.id); const waypoints = testDb.prepare( 'SELECT route_color FROM places WHERE trip_id = ? AND route_geometry IS NULL', @@ -1210,10 +1210,10 @@ describe('PlacesService — automatic track colours (#776)', () => { } }); - it('PLACES-SVC-004 — a second import continues the palette instead of repeating it', () => { + it('PLACES-SVC-004 — a second import continues the palette instead of repeating it', async () => { const { user } = createUser(testDb); const trip = createTrip(testDb, user.id); - importFixture(trip.id); + await importFixture(trip.id); const first = tracksOf(trip.id).map(t => t.route_color); // Same fixture again: dedup skips the identical rows, so seed a distinct @@ -1222,7 +1222,7 @@ describe('PlacesService — automatic track colours (#776)', () => { "INSERT INTO places (trip_id, name, lat, lng, route_geometry) VALUES (?, 'Second walk', 1, 1, '[[1,1],[2,2]]')", ).run(trip.id); const seeded = testDb.prepare('SELECT id FROM places WHERE name = ?').get('Second walk') as { id: number }; - (svc as any).colorizeImportedTracks(String(trip.id), { + await (svc as any).colorizeImportedTracks(String(trip.id), { places: [{ id: seeded.id, route_geometry: '[[1,1],[2,2]]', route_color: null }], }); @@ -1231,7 +1231,7 @@ describe('PlacesService — automatic track colours (#776)', () => { expect(first).not.toContain(seededColor); }); - it('PLACES-SVC-006 — a colour already in use by hand is not handed out again', () => { + it('PLACES-SVC-006 — a colour already in use by hand is not handed out again', async () => { const { user } = createUser(testDb); const trip = createTrip(testDb, user.id); // Someone recoloured an existing track to the palette's second entry. A @@ -1240,16 +1240,16 @@ describe('PlacesService — automatic track colours (#776)', () => { "INSERT INTO places (trip_id, name, lat, lng, route_geometry, route_color) VALUES (?, 'Old walk', 1, 1, '[[1,1],[2,2]]', ?)", ).run(trip.id, TRACK_COLORS[1]); - importFixture(trip.id); + await importFixture(trip.id); const colors = tracksOf(trip.id).map(t => t.route_color); expect(new Set(colors).size).toBe(colors.length); }); - it('PLACES-SVC-007 — gaps left by deleted tracks are reused, not skipped past', () => { + it('PLACES-SVC-007 — gaps left by deleted tracks are reused, not skipped past', async () => { const { user } = createUser(testDb); const trip = createTrip(testDb, user.id); - importFixture(trip.id); + await importFixture(trip.id); // Drop the first track; its colour becomes free again. const first = tracksOf(trip.id)[0]; testDb.prepare('DELETE FROM places WHERE id = ?').run(first.id); @@ -1257,7 +1257,7 @@ describe('PlacesService — automatic track colours (#776)', () => { const seeded = testDb.prepare( "INSERT INTO places (trip_id, name, lat, lng, route_geometry) VALUES (?, 'Later walk', 9, 9, '[[9,9],[8,8]]') RETURNING id", ).get(trip.id) as { id: number }; - (svc as any).colorizeImportedTracks(String(trip.id), { + await (svc as any).colorizeImportedTracks(String(trip.id), { places: [{ id: seeded.id, route_geometry: '[[9,9],[8,8]]', route_color: null }], }); @@ -1265,12 +1265,12 @@ describe('PlacesService — automatic track colours (#776)', () => { expect(new Set(colors).size).toBe(colors.length); }); - it('PLACES-SVC-005 — an import that yields nothing colours nothing', () => { + it('PLACES-SVC-005 — an import that yields nothing colours nothing', async () => { const { user } = createUser(testDb); const trip = createTrip(testDb, user.id); - expect(() => (svc as any).colorizeImportedTracks(String(trip.id), null)).not.toThrow(); - expect(() => (svc as any).colorizeImportedTracks(String(trip.id), { places: [] })).not.toThrow(); + await expect((svc as any).colorizeImportedTracks(String(trip.id), null)).resolves.toBeUndefined(); + await expect((svc as any).colorizeImportedTracks(String(trip.id), { places: [] })).resolves.toBeUndefined(); expect(tracksOf(trip.id)).toEqual([]); }); @@ -1394,26 +1394,26 @@ describe('enrichImportedPlaces', () => { // replaced by the default. describe('zero-valued numeric fields', () => { - it('PLACE-SVC-065 — create keeps lat/lng of exactly 0 instead of nulling them', () => { + it('PLACE-SVC-065 — create keeps lat/lng of exactly 0 instead of nulling them', async () => { const { user } = createUser(testDb); const trip = createTrip(testDb, user.id); - const place = svc.create(String(trip.id), { name: 'Null Island', lat: 0, lng: 0 }) as any; + const place = await svc.create(String(trip.id), { name: 'Null Island', lat: 0, lng: 0 }) as any; expect(place.lat).toBe(0); expect(place.lng).toBe(0); // A genuinely absent coordinate still lands as NULL. - const noCoords = svc.create(String(trip.id), { name: 'Unlocated' }) as any; + const noCoords = await svc.create(String(trip.id), { name: 'Unlocated' }) as any; expect(noCoords.lat).toBeNull(); expect(noCoords.lng).toBeNull(); }); - it('PLACE-SVC-066 — create keeps duration_minutes and price of 0', () => { + it('PLACE-SVC-066 — create keeps duration_minutes and price of 0', async () => { const { user } = createUser(testDb); const trip = createTrip(testDb, user.id); - const place = svc.create(String(trip.id), { name: 'Drive-by', duration_minutes: 0, price: 0 }) as any; + const place = await svc.create(String(trip.id), { name: 'Drive-by', duration_minutes: 0, price: 0 }) as any; expect(place.duration_minutes).toBe(0); expect(place.price).toBe(0); // The 60-minute default still applies when the field is absent. - expect((svc.create(String(trip.id), { name: 'Default' }) as any).duration_minutes).toBe(60); + expect((await svc.create(String(trip.id), { name: 'Default' }) as any).duration_minutes).toBe(60); }); it('PLACE-SVC-067 — update can set duration_minutes to 0', async () => { @@ -1473,7 +1473,7 @@ describe('list search escaping', () => { // ── Trip-scoped id filter for the journey delete hook (#1745) ───────────────── describe('scopedIds', () => { - it('PLACE-SVC-070 — returns only the ids that belong to the trip, preserving input order', () => { + it('PLACE-SVC-070 — returns only the ids that belong to the trip, preserving input order', async () => { const { user } = createUser(testDb); const trip = createTrip(testDb, user.id); const other = createTrip(testDb, user.id); @@ -1481,8 +1481,8 @@ describe('scopedIds', () => { const b = createPlace(testDb, trip.id, { name: 'B' }) as any; const foreign = createPlace(testDb, other.id, { name: 'Foreign' }) as any; - expect(svc.scopedIds(String(trip.id), [b.id, foreign.id, a.id, 99999])).toEqual([b.id, a.id]); - expect(svc.scopedIds(String(trip.id), [])).toEqual([]); + expect(await svc.scopedIds(String(trip.id), [b.id, foreign.id, a.id, 99999])).toEqual([b.id, a.id]); + expect(await svc.scopedIds(String(trip.id), [])).toEqual([]); }); }); @@ -1702,26 +1702,26 @@ describe('backfillMissingAddresses', () => { * that this service interprets it faithfully against real SQL. */ describe('findMatchingPlaceId', () => { - it('PLACES-SVC-008 — matches an existing place by name, ignoring case and surrounding space', () => { + it('PLACES-SVC-008 — matches an existing place by name, ignoring case and surrounding space', async () => { const { user } = createUser(testDb); const trip = createTrip(testDb, user.id); const place = createPlace(testDb, trip.id, { name: 'Eiffel Tower' }); - expect(svc.findMatchingPlaceId(String(trip.id), { name: ' eiffel tower ' })).toBe(place.id); + expect(await svc.findMatchingPlaceId(String(trip.id), { name: ' eiffel tower ' })).toBe(place.id); }); - it('PLACES-SVC-009 — matches on a provider id even after the place was renamed (#1550)', () => { + it('PLACES-SVC-009 — matches on a provider id even after the place was renamed (#1550)', async () => { const { user } = createUser(testDb); const trip = createTrip(testDb, user.id); const place = createPlace(testDb, trip.id, { name: 'Original Name' }); testDb.prepare('UPDATE places SET google_place_id = ? WHERE id = ?').run('ChIJ_abc', place.id); expect( - svc.findMatchingPlaceId(String(trip.id), { name: 'Renamed By User', google_place_id: 'ChIJ_abc' }), + await svc.findMatchingPlaceId(String(trip.id), { name: 'Renamed By User', google_place_id: 'ChIJ_abc' }), ).toBe(place.id); }); - it('PLACES-SVC-010 — does NOT match a NAMED candidate to a different place at the same coordinates', () => { + it('PLACES-SVC-010 — does NOT match a NAMED candidate to a different place at the same coordinates', async () => { // The restaurant and the bar in the same building are two places. This is the // rule isPlaceDuplicate has always applied; the SQL copy used to disagree. const { user } = createUser(testDb); @@ -1729,17 +1729,17 @@ describe('findMatchingPlaceId', () => { createPlace(testDb, trip.id, { name: 'Ground Floor Diner', lat: 52.52, lng: 13.405 }); expect( - svc.findMatchingPlaceId(String(trip.id), { name: 'Rooftop Bar', lat: 52.52, lng: 13.405 }), + await svc.findMatchingPlaceId(String(trip.id), { name: 'Rooftop Bar', lat: 52.52, lng: 13.405 }), ).toBeNull(); }); - it('PLACES-SVC-011 — matches an UNNAMED candidate by coordinates within tolerance', () => { + it('PLACES-SVC-011 — matches an UNNAMED candidate by coordinates within tolerance', async () => { const { user } = createUser(testDb); const trip = createTrip(testDb, user.id); const place = createPlace(testDb, trip.id, { name: 'Anything', lat: 48.85, lng: 2.35 }); expect( - svc.findMatchingPlaceId(String(trip.id), { + await svc.findMatchingPlaceId(String(trip.id), { name: null, lat: 48.85 + COORD_DEDUP_TOLERANCE / 2, lng: 2.35, @@ -1747,25 +1747,25 @@ describe('findMatchingPlaceId', () => { ).toBe(place.id); }); - it('PLACES-SVC-012 — returns null when nothing recognises the candidate', () => { + it('PLACES-SVC-012 — returns null when nothing recognises the candidate', async () => { const { user } = createUser(testDb); const trip = createTrip(testDb, user.id); createPlace(testDb, trip.id, { name: 'Somewhere Else' }); - expect(svc.findMatchingPlaceId(String(trip.id), { name: 'Unseen Place' })).toBeNull(); - expect(svc.findMatchingPlaceId(String(trip.id), { name: null, lat: null, lng: null })).toBeNull(); + expect(await svc.findMatchingPlaceId(String(trip.id), { name: 'Unseen Place' })).toBeNull(); + expect(await svc.findMatchingPlaceId(String(trip.id), { name: null, lat: null, lng: null })).toBeNull(); }); - it('PLACES-SVC-013 — never matches a place belonging to another trip', () => { + it('PLACES-SVC-013 — never matches a place belonging to another trip', async () => { const { user } = createUser(testDb); const mine = createTrip(testDb, user.id); const theirs = createTrip(testDb, user.id); createPlace(testDb, theirs.id, { name: 'Shared Name' }); - expect(svc.findMatchingPlaceId(String(mine.id), { name: 'Shared Name' })).toBeNull(); + expect(await svc.findMatchingPlaceId(String(mine.id), { name: 'Shared Name' })).toBeNull(); }); - it('PLACES-SVC-014 — a provider id still wins over a name that points elsewhere', () => { + it('PLACES-SVC-014 — a provider id still wins over a name that points elsewhere', async () => { const { user } = createUser(testDb); const trip = createTrip(testDb, user.id); // The place the user renamed, carrying the id the importer knows it by. @@ -1775,11 +1775,11 @@ describe('findMatchingPlaceId', () => { createPlace(testDb, trip.id, { name: 'Trattoria da Enzo', lat: 41.9, lng: 12.5 }); expect( - svc.findMatchingPlaceId(String(trip.id), { name: 'Trattoria da Enzo', google_ftid: '0x1:0x2' }), + await svc.findMatchingPlaceId(String(trip.id), { name: 'Trattoria da Enzo', google_ftid: '0x1:0x2' }), ).toBe(renamed.id); }); - it('PLACES-SVC-015 — the name comparison is ASCII-only, so an accented capital does not match', () => { + it('PLACES-SVC-015 — the name comparison is ASCII-only, so an accented capital does not match', async () => { // Not a wish, a boundary: `lower(trim(name))` is SQLite's ASCII lowercase, // while normalizePlaceName uses JavaScript's Unicode one. isPlaceDuplicate // does match this pair in memory. Before the shared strategies, the @@ -1789,13 +1789,13 @@ describe('findMatchingPlaceId', () => { const trip = createTrip(testDb, user.id); createPlace(testDb, trip.id, { name: 'CAFÉ CENTRAL', lat: 48.85, lng: 2.35 }); - expect(svc.findMatchingPlaceId(String(trip.id), { name: 'Café Central', lat: 48.85, lng: 2.35 })).toBeNull(); + expect(await svc.findMatchingPlaceId(String(trip.id), { name: 'Café Central', lat: 48.85, lng: 2.35 })).toBeNull(); // The all-ASCII spelling of the same shape does match. createPlace(testDb, trip.id, { name: 'CAFE CENTRAL', lat: 48.86, lng: 2.36 }); - expect(svc.findMatchingPlaceId(String(trip.id), { name: 'Cafe Central' })).not.toBeNull(); + expect(await svc.findMatchingPlaceId(String(trip.id), { name: 'Cafe Central' })).not.toBeNull(); }); - it('PLACES-SVC-016 — an unnamed candidate can match a NAMED row on coordinates', () => { + it('PLACES-SVC-016 — an unnamed candidate can match a NAMED row on coordinates', async () => { // The other place the two halves differ: buildDedupSet collects coordinates // only for unnamed rows, so isPlaceDuplicate would say no here. This is the // answer findMatchingPlaceId wants — a booking with no place name should @@ -1804,10 +1804,10 @@ describe('findMatchingPlaceId', () => { const trip = createTrip(testDb, user.id); const hotel = createPlace(testDb, trip.id, { name: 'Hotel Lutetia', lat: 48.851, lng: 2.326 }); - expect(svc.findMatchingPlaceId(String(trip.id), { name: null, lat: 48.851, lng: 2.326 })).toBe(hotel.id); + expect(await svc.findMatchingPlaceId(String(trip.id), { name: null, lat: 48.851, lng: 2.326 })).toBe(hotel.id); }); - it('PLACES-SVC-017 — the coordinate tolerance is a box roughly 11 m wide, and it closes', () => { + it('PLACES-SVC-017 — the coordinate tolerance is a box roughly 11 m wide, and it closes', async () => { const { user } = createUser(testDb); const trip = createTrip(testDb, user.id); const place = createPlace(testDb, trip.id, { name: null, lat: 48.85, lng: 2.35 }); @@ -1818,12 +1818,12 @@ describe('findMatchingPlaceId', () => { const inside = { name: null, lat: 48.85 + COORD_DEDUP_TOLERANCE * 0.9, lng: 2.35 }; const outside = { name: null, lat: 48.85 + COORD_DEDUP_TOLERANCE * 1.5, lng: 2.35 }; - expect(svc.findMatchingPlaceId(String(trip.id), inside)).toBe(place.id); - expect(svc.findMatchingPlaceId(String(trip.id), outside)).toBeNull(); + expect(await svc.findMatchingPlaceId(String(trip.id), inside)).toBe(place.id); + expect(await svc.findMatchingPlaceId(String(trip.id), outside)).toBeNull(); // Exactly one tolerance away is NOT a match: 48.85 + 0.0001 lands on // 48.850100000000004 in binary floating point, a hair over the bound. The // edge is fuzzy by design of the arithmetic, so nothing should lean on it. const edge = { name: null, lat: 48.85 + COORD_DEDUP_TOLERANCE, lng: 2.35 }; - expect(svc.findMatchingPlaceId(String(trip.id), edge)).toBeNull(); + expect(await svc.findMatchingPlaceId(String(trip.id), edge)).toBeNull(); }); }); diff --git a/server/tests/unit/nest/public-api-scopes.test.ts b/server/tests/unit/nest/public-api-scopes.test.ts index 1b2d55587..ac8a69b08 100644 --- a/server/tests/unit/nest/public-api-scopes.test.ts +++ b/server/tests/unit/nest/public-api-scopes.test.ts @@ -282,8 +282,8 @@ describe('PublicApiController — what a narrowed key reaches', () => { const getTrip = vi.fn().mockReturnValue(TRIP); const ctl = apiController({ listTrips, listBucketList, getTrip }); expect(await ctl.listTrips(req(ALL))).toEqual({ trips: [TRIP] }); - expect(ctl.listBucketList(req(ALL))).toEqual({ items: [] }); - expect(ctl.getTrip(req(ALL), '12', undefined)).toEqual(TRIP); + expect(await ctl.listBucketList(req(ALL))).toEqual({ items: [] }); + expect(await ctl.getTrip(req(ALL), '12', undefined)).toEqual(TRIP); expect(getTrip).toHaveBeenCalledWith(12, 7, [...PUBLIC_API_INCLUDES], [...PUBLIC_API_SCOPES]); }); @@ -292,9 +292,9 @@ describe('PublicApiController — what a narrowed key reaches', () => { expect(await apiController({ listTrips }).listTrips(req(limited('trips')))).toEqual({ trips: [TRIP] }); }); - it('PUBAPI-SCOPE-U022: and is refused the bucket list before the service is ever asked', () => { + it('PUBAPI-SCOPE-U022: and is refused the bucket list before the service is ever asked', async () => { const listBucketList = vi.fn(); - const res = thrown(() => apiController({ listBucketList }).listBucketList(req(limited('trips')))); + const res = await thrownAsync(() => apiController({ listBucketList }).listBucketList(req(limited('trips')))); expect(res.status).toBe(403); expect(res.body).toMatchObject({ code: 'API_SCOPE_FORBIDDEN', required_scope: 'bucket-list' }); // The refusal has to happen before the read, not after it: a 403 carrying @@ -307,35 +307,35 @@ describe('PublicApiController — what a narrowed key reaches', () => { const getTrip = vi.fn(); const ctl = apiController({ listTrips, getTrip }); expect((await thrownAsync(() => ctl.listTrips(req(limited('bucket-list'))))).status).toBe(403); - expect(thrown(() => ctl.getTrip(req(limited('bucket-list')), '12', undefined)).status).toBe(403); + expect((await thrownAsync(() => ctl.getTrip(req(limited('bucket-list')), '12', undefined))).status).toBe(403); expect(listTrips).not.toHaveBeenCalled(); expect(getTrip).not.toHaveBeenCalled(); }); - it('PUBAPI-SCOPE-U024: a trip asked for without include is narrowed, not refused', () => { + it('PUBAPI-SCOPE-U024: a trip asked for without include is narrowed, not refused', async () => { const getTrip = vi.fn().mockReturnValue(TRIP); - apiController({ getTrip }).getTrip(req(limited('trips', 'days', 'notes')), '12', undefined); + await apiController({ getTrip }).getTrip(req(limited('trips', 'days', 'notes')), '12', undefined); // `include` absent means "everything"; refusing a key for wanting sections it // never named would make a narrow key unable to read a trip at all. The grant // rides along so the service can tell an implied day block from a granted one. expect(getTrip).toHaveBeenCalledWith(12, 7, ['days', 'notes'], ['trips', 'days', 'notes']); }); - it('PUBAPI-SCOPE-U025: a trips-only key gets the summary and nothing that hangs off it', () => { + it('PUBAPI-SCOPE-U025: a trips-only key gets the summary and nothing that hangs off it', async () => { const getTrip = vi.fn().mockReturnValue(TRIP); - apiController({ getTrip }).getTrip(req(limited('trips')), '12', undefined); + await apiController({ getTrip }).getTrip(req(limited('trips')), '12', undefined); expect(getTrip).toHaveBeenCalledWith(12, 7, [], ['trips']); }); - it('PUBAPI-SCOPE-U026: an empty include is treated like an absent one, and narrowed too', () => { + it('PUBAPI-SCOPE-U026: an empty include is treated like an absent one, and narrowed too', async () => { const getTrip = vi.fn().mockReturnValue(TRIP); - apiController({ getTrip }).getTrip(req(limited('trips', 'days')), '12', ' '); + await apiController({ getTrip }).getTrip(req(limited('trips', 'days')), '12', ' '); expect(getTrip).toHaveBeenCalledWith(12, 7, ['days'], ['trips', 'days']); }); - it('PUBAPI-SCOPE-U027: a section named explicitly and not granted is a 403, not a silent drop', () => { + it('PUBAPI-SCOPE-U027: a section named explicitly and not granted is a 403, not a silent drop', async () => { const getTrip = vi.fn(); - const res = thrown(() => + const res = await thrownAsync(() => apiController({ getTrip }).getTrip(req(limited('trips', 'days')), '12', 'days,places'), ); expect(res).toEqual({ @@ -349,15 +349,15 @@ describe('PublicApiController — what a narrowed key reaches', () => { expect(getTrip).not.toHaveBeenCalled(); }); - it('PUBAPI-SCOPE-U028: an include the key covers is passed through untouched', () => { + it('PUBAPI-SCOPE-U028: an include the key covers is passed through untouched', async () => { const getTrip = vi.fn().mockReturnValue(TRIP); - apiController({ getTrip }).getTrip(req(limited('trips', 'days', 'notes')), '12', 'days, notes'); + await apiController({ getTrip }).getTrip(req(limited('trips', 'days', 'notes')), '12', 'days, notes'); expect(getTrip).toHaveBeenCalledWith(12, 7, ['days', 'notes'], ['trips', 'days', 'notes']); }); - it('PUBAPI-SCOPE-U029: a bad id is still a 400 — the scope check does not swallow it', () => { + it('PUBAPI-SCOPE-U029: a bad id is still a 400 — the scope check does not swallow it', async () => { const getTrip = vi.fn(); - expect(thrown(() => apiController({ getTrip }).getTrip(req(ALL), 'abc', undefined)).status).toBe(400); + expect((await thrownAsync(() => apiController({ getTrip }).getTrip(req(ALL), 'abc', undefined))).status).toBe(400); expect(getTrip).not.toHaveBeenCalled(); }); }); diff --git a/server/tests/unit/nest/public-api.controller.test.ts b/server/tests/unit/nest/public-api.controller.test.ts index ceeac25de..67311f56c 100644 --- a/server/tests/unit/nest/public-api.controller.test.ts +++ b/server/tests/unit/nest/public-api.controller.test.ts @@ -50,17 +50,6 @@ const req = (userId = 7) => ({ user: { id: userId }, apiToken: FULL_GRANT }) as /** A request the guard left no user on — the shape the controller must refuse. */ const reqWithoutUser = () => ({ apiToken: FULL_GRANT }) as Request; -function thrown(fn: () => unknown): { status: number; body: unknown } { - try { - fn(); - } catch (err) { - expect(err).toBeInstanceOf(HttpException); - const e = err as HttpException; - return { status: e.getStatus(), body: e.getResponse() }; - } - throw new Error('expected the handler to throw'); -} - async function thrownAsync(fn: () => Promise): Promise<{ status: number; body: unknown }> { try { await fn(); @@ -74,24 +63,24 @@ async function thrownAsync(fn: () => Promise): Promise<{ status: number describe('PublicApiController', () => { describe('GET /api/v1/bucket-list', () => { - it('returns the caller’s wishlist and passes the id from the guard, never the query', () => { + it('returns the caller’s wishlist and passes the id from the guard, never the query', async () => { const listBucketList = vi.fn().mockReturnValue([ITEM]); - expect(makeController({ listBucketList }).listBucketList(req(7))).toEqual({ items: [ITEM] }); + expect(await makeController({ listBucketList }).listBucketList(req(7))).toEqual({ items: [ITEM] }); expect(listBucketList).toHaveBeenCalledWith(7); }); - it('401s when the guard left no user behind', () => { + it('401s when the guard left no user behind', async () => { const listBucketList = vi.fn(); - expect(thrown(() => makeController({ listBucketList }).listBucketList(reqWithoutUser()))).toEqual({ + expect(await thrownAsync(() => makeController({ listBucketList }).listBucketList(reqWithoutUser()))).toEqual({ status: 401, body: { error: 'API token required', code: 'API_TOKEN_REQUIRED' }, }); expect(listBucketList).not.toHaveBeenCalled(); }); - it('counts against the same rate budget as everything else', () => { + it('counts against the same rate budget as everything else', async () => { const listBucketList = vi.fn(); - expect(thrown(() => makeController({ listBucketList }, false).listBucketList(req(7))).status).toBe(429); + expect((await thrownAsync(() => makeController({ listBucketList }, false).listBucketList(req(7)))).status).toBe(429); expect(listBucketList).not.toHaveBeenCalled(); }); }); @@ -119,22 +108,22 @@ describe('PublicApiController', () => { }); describe('GET /api/v1/trips/:id', () => { - it('passes the parsed id and user through, and defaults include to every section', () => { + it('passes the parsed id and user through, and defaults include to every section', async () => { const getTrip = vi.fn().mockReturnValue(TRIP); - const res = makeController({ getTrip }).getTrip(req(7), '12', undefined); + const res = await makeController({ getTrip }).getTrip(req(7), '12', undefined); expect(res).toEqual(TRIP); expect(getTrip).toHaveBeenCalledWith(12, 7, [...PUBLIC_API_INCLUDES], expect.arrayContaining(['days'])); }); - it('treats an empty include the same as an absent one', () => { + it('treats an empty include the same as an absent one', async () => { const getTrip = vi.fn().mockReturnValue(TRIP); - makeController({ getTrip }).getTrip(req(7), '12', ' '); + await makeController({ getTrip }).getTrip(req(7), '12', ' '); expect(getTrip).toHaveBeenCalledWith(12, 7, [...PUBLIC_API_INCLUDES], expect.arrayContaining(['days'])); }); - it('narrows to the requested sections and tolerates spacing', () => { + it('narrows to the requested sections and tolerates spacing', async () => { const getTrip = vi.fn().mockReturnValue(TRIP); - makeController({ getTrip }).getTrip(req(7), '12', 'days, notes'); + await makeController({ getTrip }).getTrip(req(7), '12', 'days, notes'); expect(getTrip).toHaveBeenCalledWith(12, 7, ['days', 'notes'], expect.arrayContaining(['days'])); }); @@ -143,9 +132,9 @@ describe('PublicApiController', () => { * trip that was never created. Anything else turns the endpoint into a way to * count another user's trips. */ - it('404s for a trip the caller may not read, with no hint that it exists', () => { + it('404s for a trip the caller may not read, with no hint that it exists', async () => { const getTrip = vi.fn().mockReturnValue(null); - expect(thrown(() => makeController({ getTrip }).getTrip(req(7), '999', undefined))).toEqual({ + expect(await thrownAsync(() => makeController({ getTrip }).getTrip(req(7), '999', undefined))).toEqual({ status: 404, body: { error: 'Trip not found' }, }); @@ -161,9 +150,9 @@ describe('PublicApiController', () => { ['whitespace', ' 12 '], ['a sql fragment', "1 OR 1=1"], ['an id past the safe integer range', '9007199254740993'], - ])('400s on %s without touching the service', (_label, raw) => { + ])('400s on %s without touching the service', async (_label, raw) => { const getTrip = vi.fn(); - expect(thrown(() => makeController({ getTrip }).getTrip(req(7), raw, undefined)).status).toBe(400); + expect((await thrownAsync(() => makeController({ getTrip }).getTrip(req(7), raw, undefined))).status).toBe(400); expect(getTrip).not.toHaveBeenCalled(); }); @@ -171,9 +160,9 @@ describe('PublicApiController', () => { ['an unknown section', 'days,everything'], ['a typo', 'dayz'], ['only commas', ',,,'], - ])('400s on %s rather than silently dropping it', (_label, include) => { + ])('400s on %s rather than silently dropping it', async (_label, include) => { const getTrip = vi.fn(); - const res = thrown(() => makeController({ getTrip }).getTrip(req(7), '12', include)); + const res = await thrownAsync(() => makeController({ getTrip }).getTrip(req(7), '12', include)); expect(res.status).toBe(400); expect(getTrip).not.toHaveBeenCalled(); }); @@ -187,9 +176,9 @@ describe('PublicApiController', () => { expect(listTrips).not.toHaveBeenCalled(); }); - it('429s the detail route the same way', () => { + it('429s the detail route the same way', async () => { const getTrip = vi.fn(); - expect(thrown(() => makeController({ getTrip }, false).getTrip(req(7), '12', undefined)).status).toBe(429); + expect((await thrownAsync(() => makeController({ getTrip }, false).getTrip(req(7), '12', undefined))).status).toBe(429); expect(getTrip).not.toHaveBeenCalled(); }); }); diff --git a/server/tests/unit/nest/reservations.service.test.ts b/server/tests/unit/nest/reservations.service.test.ts index 817a68027..e5da80aa8 100644 --- a/server/tests/unit/nest/reservations.service.test.ts +++ b/server/tests/unit/nest/reservations.service.test.ts @@ -1068,11 +1068,11 @@ describe('ReservationsService — referenced ids stay inside the trip', () => { try { plant(); } finally { testDb.exec('PRAGMA foreign_keys = ON'); } } - it('RESV-SCOPE-006: an id that resolves to nothing is named too, accommodation_id excepted', () => { + it('RESV-SCOPE-006: an id that resolves to nothing is named too, accommodation_id excepted', async () => { const { mine } = twoTrips(); const gone = 999999; - expect(svc.unresolvedReferences(String(mine.id), { + expect(await svc.unresolvedReferences(String(mine.id), { title: 'x', type: 'hotel', day_id: gone, end_day_id: gone, place_id: gone, assignment_id: gone, accommodation_id: gone, create_accommodation: { place_id: gone, start_day_id: gone, end_day_id: gone }, @@ -1082,37 +1082,37 @@ describe('ReservationsService — referenced ids stay inside the trip', () => { ]); }); - it('RESV-SCOPE-007: ids on the trip pass, and a falsy one is not a reference at all', () => { + it('RESV-SCOPE-007: ids on the trip pass, and a falsy one is not a reference at all', async () => { const { mine } = twoTrips(); const place = createPlace(testDb, mine.id); const day = testDb.prepare('SELECT id FROM days WHERE trip_id = ? ORDER BY day_number').get(mine.id) as { id: number }; const assignment = createDayAssignment(testDb, day.id, place.id); - expect(svc.unresolvedReferences(String(mine.id), { + expect(await svc.unresolvedReferences(String(mine.id), { title: 'x', type: 'hotel', day_id: day.id, end_day_id: day.id, place_id: place.id, assignment_id: assignment.id, create_accommodation: { place_id: place.id, start_day_id: day.id, end_day_id: day.id }, })).toEqual([]); // The write paths coerce these to NULL before they reach SQL, so naming // them would 400 a body that stores a null today. - expect(svc.unresolvedReferences(String(mine.id), { title: 'x', day_id: 0, end_day_id: 0, place_id: 0, assignment_id: 0 })).toEqual([]); + expect(await svc.unresolvedReferences(String(mine.id), { title: 'x', day_id: 0, end_day_id: 0, place_id: 0, assignment_id: 0 })).toEqual([]); // Only a hotel booking writes the stay row. - expect(svc.unresolvedReferences(String(mine.id), { + expect(await svc.unresolvedReferences(String(mine.id), { title: 'x', type: 'flight', create_accommodation: { place_id: 999999, start_day_id: 999999, end_day_id: 999999 }, })).toEqual([]); // A stay without a place is a state the booking form writes. - expect(svc.unresolvedReferences(String(mine.id), { + expect(await svc.unresolvedReferences(String(mine.id), { title: 'x', type: 'hotel', create_accommodation: { start_day_id: day.id, end_day_id: day.id }, })).toEqual([]); }); - it('RESV-SCOPE-008: an id from another trip is named by both guards, so the older one answers first', () => { + it('RESV-SCOPE-008: an id from another trip is named by both guards, so the older one answers first', async () => { const { mine, theirs } = twoTrips(); const foreignPlace = createPlace(testDb, theirs.id); const body = { title: 'x', place_id: foreignPlace.id }; expect(svc.referencesOutsideTrip(String(mine.id), body)).toEqual(['place_id']); - expect(svc.unresolvedReferences(String(mine.id), body)).toEqual(['place_id']); + expect(await svc.unresolvedReferences(String(mine.id), body)).toEqual(['place_id']); }); it('RESV-SCOPE-009: an update heals the stored ids whose rows are gone instead of failing on them', async () => { diff --git a/server/tests/unit/services/conflictUpdate.test.ts b/server/tests/unit/services/conflictUpdate.test.ts index 50d18c469..2b807adab 100644 --- a/server/tests/unit/services/conflictUpdate.test.ts +++ b/server/tests/unit/services/conflictUpdate.test.ts @@ -96,8 +96,8 @@ afterAll(() => { testDb.close(); }); -function freshPlace(tripId: number) { - const place = places.create(String(tripId), { name: 'Original' }) as unknown as { id: number; updated_at: string }; +async function freshPlace(tripId: number) { + const place = await places.create(String(tripId), { name: 'Original' }) as unknown as { id: number; updated_at: string }; return place; } @@ -105,7 +105,7 @@ describe('PlacesService.update — optimistic concurrency', () => { it('updates normally when no If-Match token is sent (back-compat)', async () => { const { user } = createUser(testDb); const trip = createTrip(testDb, user.id); - const place = freshPlace(trip.id); + const place = await freshPlace(trip.id); const result = await places.update(String(trip.id), String(place.id), { name: 'Edited' }); expect(isUpdateConflict(result)).toBe(false); @@ -115,7 +115,7 @@ describe('PlacesService.update — optimistic concurrency', () => { it('updates when the If-Match token matches the current updated_at', async () => { const { user } = createUser(testDb); const trip = createTrip(testDb, user.id); - const place = freshPlace(trip.id); + const place = await freshPlace(trip.id); const result = await places.update(String(trip.id), String(place.id), { name: 'Edited' }, place.updated_at); expect(isUpdateConflict(result)).toBe(false); @@ -125,7 +125,7 @@ describe('PlacesService.update — optimistic concurrency', () => { it('returns a conflict (with the server row) when the token is stale', async () => { const { user } = createUser(testDb); const trip = createTrip(testDb, user.id); - const place = freshPlace(trip.id); + const place = await freshPlace(trip.id); const result = await places.update(String(trip.id), String(place.id), { name: 'Mine' }, '1999-01-01 00:00:00'); expect(isUpdateConflict(result)).toBe(true);