diff --git a/apps/edr-passenger-api/src/common/utils/payment-deadline.utils.ts b/apps/edr-passenger-api/src/common/utils/payment-deadline.utils.ts new file mode 100644 index 000000000..1d4c286be --- /dev/null +++ b/apps/edr-passenger-api/src/common/utils/payment-deadline.utils.ts @@ -0,0 +1,22 @@ +/** + * Single source of truth for how long a PENDING_PAYMENT booking has to be paid for, + * shared by TasksService (which auto-cancels bookings past this deadline) and + * SeatsService (which extends the seat hold to cover exactly this window when a + * booking/PNR is created — without this, the seat hold reverted to its original + * short seat-selection TTL and could expire mid-payment, letting a second customer + * grab the same seat). + */ + +/** Maximum time (hours) a passenger has to pay after booking. */ +export const MAX_PAYMENT_HOURS = 2; +/** Minutes before departure: cutoff for new bookings and payment deadline. */ +export const CUTOFF_MINUTES = 30; + +/** + * payment_deadline = MIN(booking_time + 2h, departure_time - 30min) + */ +export function computePaymentDeadline(createdAt: Date, departureAt: Date): Date { + const maxDeadline = new Date(createdAt.getTime() + MAX_PAYMENT_HOURS * 60 * 60 * 1000); + const cutoffDeadline = new Date(departureAt.getTime() - CUTOFF_MINUTES * 60 * 1000); + return maxDeadline < cutoffDeadline ? maxDeadline : cutoffDeadline; +} diff --git a/apps/edr-passenger-api/src/modules/currency/currency.service.ts b/apps/edr-passenger-api/src/modules/currency/currency.service.ts index 198571572..95ecc2cd7 100644 --- a/apps/edr-passenger-api/src/modules/currency/currency.service.ts +++ b/apps/edr-passenger-api/src/modules/currency/currency.service.ts @@ -51,26 +51,31 @@ export class CurrencyService { } /** - * Converts an ETB minor-unit amount to the charge major-unit amount sent to the - * payment provider. Applies the exchange rate for foreign currencies then divides - * by 100 to yield major units (e.g. 300000 ETB minor → 3000.00 ETB major). + * Converts a booking's stored minor-unit amount (in its own `fromCurrency`) to the charge + * major-unit amount sent to the payment provider in `targetCurrency`. When the two currencies + * match, no exchange rate is applied — the stored amount is charged as-is. Otherwise the + * fromCurrency→targetCurrency rate is applied. In both cases the result is divided by 100 to + * yield major units and rounded to the target currency's precision + * (e.g. 300000 ETB minor → 3000.00 ETB major; DJF rounds to whole francs). */ - async convertEtbMinorToChargeMajor( - amountMinorEtb: number, + async convertMinorToChargeMajor( + amountMinor: number, + fromCurrency: string, targetCurrency: string, ): Promise { + const from = fromCurrency.toUpperCase(); const target = targetCurrency.toUpperCase(); if (CHARGE_CURRENCY_DECIMALS[target] === undefined) { throw new BadRequestException(`Unsupported charge currency: ${targetCurrency}`); } const decimals = CHARGE_CURRENCY_DECIMALS[target]; - if (target === Currency.ETB) { - return this.roundTo(amountMinorEtb / 100, decimals); + if (from === target) { + return this.roundTo(amountMinor / 100, decimals); } - const rate = await this.getRateOrThrow(Currency.ETB, target as Currency); - return this.roundTo((amountMinorEtb * rate) / 100, decimals); + const rate = await this.getRateOrThrow(from as Currency, target as Currency); + return this.roundTo((amountMinor * rate) / 100, decimals); } async getRateOrThrow( diff --git a/apps/edr-passenger-api/src/modules/seats/seats.service.ts b/apps/edr-passenger-api/src/modules/seats/seats.service.ts index ad92660a4..b2bff14b6 100644 --- a/apps/edr-passenger-api/src/modules/seats/seats.service.ts +++ b/apps/edr-passenger-api/src/modules/seats/seats.service.ts @@ -1,13 +1,16 @@ -import { Injectable, ConflictException, NotFoundException, BadRequestException } from '@nestjs/common'; +import { Injectable, ConflictException, NotFoundException, BadRequestException, Logger } from '@nestjs/common'; import { PrismaService } from '../../common/prisma.service'; import { HoldSeatsDto, JourneyDirection } from './seats.dto'; import { Cron, CronExpression } from '@nestjs/schedule'; import { SegmentsService } from '../segments/segments.service'; import { SystemConfigService, CONFIG_KEYS } from '../system-config/system-config.service'; import { AuditService } from '../../common/audit.service'; +import { computePaymentDeadline } from '../../common/utils/payment-deadline.utils'; @Injectable() export class SeatsService { + private readonly logger = new Logger(SeatsService.name); + constructor( private prisma: PrismaService, private segmentsService: SegmentsService, @@ -623,7 +626,50 @@ export class SeatsService { return { released: true, holdId }; } - async confirmSeats(_seatIds: string[]) {} + // Called right after a booking (PNR) is created, and again on successful payment. + // Extends the SeatHold(s) covering these seats to the booking's actual payment + // deadline — the same MIN(createdAt + 2h, departureAt - 30min) window TasksService + // uses to auto-cancel unpaid bookings — instead of leaving them on the original + // short seat-selection hold (5 min by default). Without this, the hold could expire + // while the customer was still on the payment page, and a second customer could + // hold/book the exact same seat out from under them. + async confirmSeats(seatIds: string[], now: Date = new Date()): Promise { + if (seatIds.length === 0) return; + + const holds = await this.prisma.seatHold.findMany({ + where: { seatIds: { hasSome: seatIds } }, + select: { id: true, scheduleId: true, expiresAt: true }, + }); + if (holds.length === 0) return; + + const scheduleIds = Array.from(new Set(holds.map(h => h.scheduleId))); + const schedules = await this.prisma.trainSchedule.findMany({ + where: { id: { in: scheduleIds } }, + select: { id: true, departureAt: true }, + }); + const departureById = new Map(schedules.map(s => [s.id, s.departureAt])); + + let extended = 0; + await Promise.all( + holds.map(async (hold) => { + const departureAt = departureById.get(hold.scheduleId); + if (!departureAt) return; + const deadline = computePaymentDeadline(now, departureAt); + // Only ever extend forward — never shorten a hold that's already valid longer + // than the payment deadline would give it (e.g. a second confirmSeats call on + // the same booking, or a hold that was already extended). + if (deadline <= hold.expiresAt) return; + await this.prisma.seatHold.update({ where: { id: hold.id }, data: { expiresAt: deadline } }); + extended++; + }), + ); + + if (extended > 0) { + this.logger.log( + `Extended ${extended} seat hold(s) covering ${seatIds.length} seat(s) to their booking's payment deadline`, + ); + } + } // Delete the Journey (and its JourneySegments) scoped to this booking. async releaseSeats(bookingId: string) { @@ -893,30 +939,83 @@ export class SeatsService { ); } + // Runs every minute, but is also safe to call on-demand (e.g. right after a hold's + // TTL is read back to the client) — expiresAt/now are both absolute UTC instants + // (Date objects, not wall-clock strings), so this is correct regardless of the + // server's or a client's local timezone; there's no wall-clock parsing involved. @Cron(CronExpression.EVERY_MINUTE) async expireHolds() { + try { + const result = await this.expireHoldsCore(); + if (result.expiredHolds > 0) { + this.logger.log( + `Expired ${result.expiredHolds} hold(s): released ${result.releasedSeatIds.length} seat(s), ` + + `skipped ${result.skippedSeatIds.length} still held by another active hold on the same schedule`, + ); + } + } catch (error) { + // A failed run must not crash the process or silently go unnoticed — the next + // scheduled run one minute later will retry the same (still-expired) holds, + // since nothing here is deleted/updated until the queries above succeed. + this.logger.error('Failed to expire seat holds', error instanceof Error ? error.stack : error); + } + } + + async expireHoldsCore(now: Date = new Date()): Promise<{ + expiredHolds: number; + releasedSeatIds: string[]; + skippedSeatIds: string[]; + }> { const expired = await this.prisma.seatHold.findMany({ - where: { expiresAt: { lt: new Date() } }, - select: { id: true, seatIds: true }, + where: { expiresAt: { lt: now } }, + select: { id: true, scheduleId: true, seatIds: true }, }); - if (expired.length === 0) return; + if (expired.length === 0) { + return { expiredHolds: 0, releasedSeatIds: [], skippedSeatIds: [] }; + } - const expiredSeatIds = expired.flatMap(h => h.seatIds as string[]); - - // Only reset seats that have no remaining active holds - const stillHeld = await this.prisma.seatHold.findMany({ - where: { expiresAt: { gte: new Date() }, seatIds: { hasSome: expiredSeatIds } }, - select: { seatIds: true }, + // Still-active holds — scoped per (scheduleId, seatId), not just seatId. The same + // physical Seat row is reused across every recurring date a coach runs, so the + // same seatId legitimately appears in unrelated holds for other schedules; without + // this scoping, an unrelated active hold on a DIFFERENT schedule would wrongly + // block release of a seat whose hold expired on THIS schedule, leaving it stuck at + // status 'HELD' indefinitely. + const activeHolds = await this.prisma.seatHold.findMany({ + where: { expiresAt: { gte: now } }, + select: { scheduleId: true, seatIds: true }, }); - const stillHeldIds = new Set(stillHeld.flatMap(h => h.seatIds as string[])); - const toRelease = expiredSeatIds.filter(id => !stillHeldIds.has(id)); + const stillHeldKeys = new Set( + activeHolds.flatMap(h => (h.seatIds as string[]).map(seatId => `${h.scheduleId}:${seatId}`)), + ); - if (toRelease.length > 0) { + const releasedSeatIds = new Set(); + const skippedSeatIds = new Set(); + for (const hold of expired) { + for (const seatId of hold.seatIds as string[]) { + if (stillHeldKeys.has(`${hold.scheduleId}:${seatId}`)) { + skippedSeatIds.add(seatId); + } else { + releasedSeatIds.add(seatId); + } + } + } + + if (releasedSeatIds.size > 0) { await this.prisma.seat.updateMany({ - where: { id: { in: toRelease }, status: 'HELD' }, - data: { status: 'AVAILABLE' }, + where: { id: { in: Array.from(releasedSeatIds) }, status: 'HELD' }, + // heldUntil is cleared alongside status — leaving a stale (past) heldUntil on an + // AVAILABLE seat is stale data that any future code reading heldUntil directly + // (instead of re-deriving availability live) would misinterpret. + data: { status: 'AVAILABLE', heldUntil: null }, }); } - await this.prisma.seatHold.deleteMany({ where: { expiresAt: { lt: new Date() } } }); + + await this.prisma.seatHold.deleteMany({ where: { expiresAt: { lt: now } } }); + + return { + expiredHolds: expired.length, + releasedSeatIds: Array.from(releasedSeatIds), + skippedSeatIds: Array.from(skippedSeatIds), + }; } } diff --git a/apps/edr-passenger-api/src/modules/segments/enhanced-seats.service.ts b/apps/edr-passenger-api/src/modules/segments/enhanced-seats.service.ts index 406c9e61b..bd99f1ceb 100644 --- a/apps/edr-passenger-api/src/modules/segments/enhanced-seats.service.ts +++ b/apps/edr-passenger-api/src/modules/segments/enhanced-seats.service.ts @@ -1,4 +1,4 @@ -import { Injectable, BadRequestException, ConflictException } from '@nestjs/common'; +import { Injectable, BadRequestException, ConflictException, Logger } from '@nestjs/common'; import { PrismaService } from '../../common/prisma.service'; import { SegmentsService, Segment } from '../segments/segments.service'; import { EventEmitter2 } from '@nestjs/event-emitter'; @@ -18,6 +18,8 @@ export interface BookingConfirmRequest { @Injectable() export class EnhancedSeatsService { + private readonly logger = new Logger(EnhancedSeatsService.name); + constructor( private prisma: PrismaService, private segmentsService: SegmentsService, @@ -57,6 +59,15 @@ export class EnhancedSeatsService { }, }); + // Mirrors SeatsService.holdSeats() — without this, a seat held through this path + // reads back as status 'AVAILABLE' in the DB despite being actively held, which is + // wrong for any consumer that trusts `status` directly instead of re-deriving + // availability live from SeatHold. + await tx.seat.updateMany({ + where: { id: { in: request.seatIds } }, + data: { status: 'HELD', heldUntil: expiresAt }, + }); + this.eventEmitter.emit('seats.held', { holdId: seatHold.id, scheduleId: request.scheduleId, seatIds: request.seatIds, segments }); return { holdId: seatHold.id, expiresAt, segments, seats: request.seatIds }; }); @@ -157,19 +168,58 @@ export class EnhancedSeatsService { }); } - async expireHolds() { - return this.prisma.$transaction(async (tx) => { - const expiredHolds = await tx.seatHold.findMany({ where: { expiresAt: { lt: new Date() } } }); - const expiredSeatIds = expiredHolds.flatMap(h => h.seatIds); + // now/expiresAt are absolute UTC instants (Date objects), not wall-clock strings, so + // this comparison is correct regardless of the server's local timezone. + async expireHolds(now: Date = new Date()) { + try { + const result = await this.prisma.$transaction(async (tx) => { + const expiredHolds = await tx.seatHold.findMany({ where: { expiresAt: { lt: now } } }); + if (expiredHolds.length === 0) { + return { expiredHolds: 0, releasedSeats: [] as string[] }; + } - if (expiredSeatIds.length > 0) { - await tx.seat.updateMany({ where: { id: { in: expiredSeatIds } }, data: { status: 'AVAILABLE', heldUntil: null } }); - await tx.seatHold.deleteMany({ where: { expiresAt: { lt: new Date() } } }); - this.eventEmitter.emit('holds.expired', { expiredHolds: expiredHolds.length, releasedSeats: expiredSeatIds }); + // Still-active holds — scoped per (scheduleId, seatId). The same physical Seat + // row is reused across every recurring date a coach runs, so the same seatId can + // legitimately appear in an unrelated hold for a different schedule; without this + // scoping, that unrelated hold would wrongly be treated as covering THIS + // schedule's seat too, and a seat still genuinely held (same schedule, a newer + // non-expired hold) could be released out from under it. + const activeHolds = await tx.seatHold.findMany({ where: { expiresAt: { gte: now } } }); + const stillHeldKeys = new Set( + activeHolds.flatMap(h => h.seatIds.map(seatId => `${h.scheduleId}:${seatId}`)), + ); + + const releasedSeatIds = new Set(); + for (const hold of expiredHolds) { + for (const seatId of hold.seatIds) { + if (!stillHeldKeys.has(`${hold.scheduleId}:${seatId}`)) releasedSeatIds.add(seatId); + } + } + + if (releasedSeatIds.size > 0) { + await tx.seat.updateMany({ + where: { id: { in: Array.from(releasedSeatIds) } }, + data: { status: 'AVAILABLE', heldUntil: null }, + }); + } + await tx.seatHold.deleteMany({ where: { expiresAt: { lt: now } } }); + + return { expiredHolds: expiredHolds.length, releasedSeats: Array.from(releasedSeatIds) }; + }); + + if (result.expiredHolds > 0) { + this.logger.log(`Expired ${result.expiredHolds} hold(s), released ${result.releasedSeats.length} seat(s)`); + this.eventEmitter.emit('holds.expired', { expiredHolds: result.expiredHolds, releasedSeats: result.releasedSeats }); } - return { expiredHolds: expiredHolds.length, releasedSeats: expiredSeatIds }; - }); + return result; + } catch (error) { + // A failed run must not go unnoticed — nothing is deleted/updated until the + // transaction commits, so the next caller/scheduled run simply retries the same + // still-expired holds. + this.logger.error('Failed to expire seat holds', error instanceof Error ? error.stack : error); + return { expiredHolds: 0, releasedSeats: [] as string[] }; + } } async getSeatAvailability(scheduleId: string, originStationId: string, destinationStationId: string) { diff --git a/apps/edr-passenger-api/src/modules/tasks/tasks.service.ts b/apps/edr-passenger-api/src/modules/tasks/tasks.service.ts index fb9957b92..4fb3a4f0f 100644 --- a/apps/edr-passenger-api/src/modules/tasks/tasks.service.ts +++ b/apps/edr-passenger-api/src/modules/tasks/tasks.service.ts @@ -3,11 +3,7 @@ import { Cron } from '@nestjs/schedule'; import { PrismaService } from '../../common/prisma.service'; import { SmsClientService } from '../notifications/sms-client.service'; import { CurrencyService } from '../currency/currency.service'; - -/** Maximum time (hours) a passenger has to pay after booking. */ -const MAX_PAYMENT_HOURS = 2; -/** Minutes before departure: cutoff for new bookings and payment deadline. */ -const CUTOFF_MINUTES = 30; +import { MAX_PAYMENT_HOURS, CUTOFF_MINUTES, computePaymentDeadline } from '../../common/utils/payment-deadline.utils'; // Retention windows const OTP_RETENTION_HOURS = 1; @@ -16,15 +12,6 @@ const AUDIT_LOG_RETENTION_DAYS = 365; const WEBHOOK_EVENT_RETENTION_DAYS = 90; const GATE_LOG_RETENTION_DAYS = 180; -/** - * payment_deadline = MIN(booking_time + 2h, departure_time - 30min) - */ -function computePaymentDeadline(createdAt: Date, departureAt: Date): Date { - const maxDeadline = new Date(createdAt.getTime() + MAX_PAYMENT_HOURS * 60 * 60 * 1000); - const cutoffDeadline = new Date(departureAt.getTime() - CUTOFF_MINUTES * 60 * 1000); - return maxDeadline < cutoffDeadline ? maxDeadline : cutoffDeadline; -} - function fmtTime(d: Date): string { return d.toLocaleTimeString('en-GB', { hour: '2-digit', @@ -192,6 +179,7 @@ export class TasksService { }, }, paymentIntent: { select: { method: true } }, + seats: { select: { seatId: true } }, }, }); @@ -205,9 +193,21 @@ export class TasksService { const paymentDeadline = computePaymentDeadline(createdAt, dep); if (now < paymentDeadline) continue; - // 1. Release held seats (Journey rows are the occupancy source of truth) + // 1a. Release held seats (Journey rows are the occupancy source of truth once paid) await this.prisma.journey.deleteMany({ where: { bookingId: booking.id } as any }); + // 1b. Also release the SeatHold(s) covering this booking's seats — SeatsService + // extends these to the payment deadline when the booking is created, so without + // this they'd otherwise keep the seat locked for up to MAX_PAYMENT_HOURS even + // though the booking is now cancelled. Scoped to this booking's own schedule, + // since the same physical Seat row is reused across other recurring dates. + const seatIds = booking.seats.map(s => s.seatId); + if (seatIds.length > 0) { + await this.prisma.seatHold.deleteMany({ + where: { scheduleId: booking.scheduleId, seatIds: { hasSome: seatIds } }, + }); + } + // 2. Audit record (no refund — payment was never completed) await this.prisma.bookingCancellation.create({ data: {