From d65d1f045a138d7328d1f8525afd5842a086ae0c Mon Sep 17 00:00:00 2001 From: hikmat67-code <263225385+hikmat67-code@users.noreply.github.com> Date: Sat, 17 Oct 2026 12:59:43 +0100 Subject: [PATCH] feat: paginate manual-review queue, runtime websocket CORS, recon metrics and transition logging Closes #1944 Closes #1943 Closes #1942 Closes #1941 --- backend/.env.example | 5 +- backend/src/common/metrics.service.ts | 36 +++ .../reconciliation-mismatch-metric.util.ts | 15 -- backend/src/common/ws-cors.io-adapter.spec.ts | 76 ++++++ backend/src/common/ws-cors.io-adapter.ts | 53 ++++ backend/src/database/data-source.ts | 2 - backend/src/main.ts | 9 + backend/src/payments/README.md | 22 +- .../dto/list-manual-review-query.dto.ts | 37 +++ .../dto/manual-review-page-response.dto.ts | 18 ++ .../payment-confirmation.service.spec.ts | 1 + .../payments/payment-confirmation.service.ts | 21 +- .../payments/payment-state-machine.spec.ts | 61 +++++ backend/src/payments/payment-state-machine.ts | 70 ++++- .../payments-admin-pagination.util.ts | 4 +- .../src/payments/payments-admin.controller.ts | 22 +- backend/src/payments/payments-admin.spec.ts | 251 +++++++++++++----- backend/src/payments/payments.gateway.ts | 10 +- backend/src/payments/payments.service.ts | 18 +- .../payments/reconciliation.service.spec.ts | 146 +++++++++- .../src/payments/reconciliation.service.ts | 67 ++++- .../payments/state-transition-logger.util.ts | 19 -- frontend/app/admin/payments/page.tsx | 48 +++- frontend/app/admin/refund/page.tsx | 4 +- frontend/lib/payments-api.ts | 18 +- 25 files changed, 882 insertions(+), 151 deletions(-) delete mode 100644 backend/src/common/reconciliation-mismatch-metric.util.ts create mode 100644 backend/src/common/ws-cors.io-adapter.spec.ts create mode 100644 backend/src/common/ws-cors.io-adapter.ts create mode 100644 backend/src/payments/dto/list-manual-review-query.dto.ts create mode 100644 backend/src/payments/dto/manual-review-page-response.dto.ts delete mode 100644 backend/src/payments/state-transition-logger.util.ts diff --git a/backend/.env.example b/backend/.env.example index 9a2cedab..3374eb0f 100644 --- a/backend/.env.example +++ b/backend/.env.example @@ -95,8 +95,9 @@ MAILTRAP_USER=your_mailtrap_user MAILTRAP_PASS=your_mailtrap_pass # Company and frontend information -# COMPANY_NAME and SUPPORT_EMAIL are read by the reconciliation alert email; -# FRONTEND_URL is the separate Socket.IO gateway origin in payments.gateway.ts. +# COMPANY_NAME and SUPPORT_EMAIL are read by the reconciliation alert email. +# The Socket.IO gateway shares the CORS_ORIGINS allowlist above (issue #1943); +# FRONTEND_URL is retained for compatibility but no longer gates sockets. COMPANY_NAME=ManageHub SUPPORT_EMAIL=support@yourdomain.com FRONTEND_URL=http://localhost:3000 diff --git a/backend/src/common/metrics.service.ts b/backend/src/common/metrics.service.ts index 7126c406..b3dc11ce 100644 --- a/backend/src/common/metrics.service.ts +++ b/backend/src/common/metrics.service.ts @@ -50,6 +50,42 @@ export class MetricsService { this.setGauge('managehub_manual_review_queue_depth', {}, depth); } + /** + * Gauge for the number of ledger/provider-state mismatches a single + * reconciliation run detected (issue #1942). Reset on every run so a + * scrape always reflects the most recent pass; the companion counter + * below accumulates the running total for rate-based alerting. + */ + setReconciliationMismatchCount(count: number): void { + this.setGauge('managehub_reconciliation_mismatch_count', {}, count); + } + + /** Running total of mismatches detected across reconciliation runs (issue #1942). */ + recordReconciliationMismatches(count: number): void { + this.incrementCounter( + 'managehub_reconciliation_mismatches_total', + {}, + count, + ); + } + + /** + * Counts a payment state transition that the state machine refused to + * apply (issue #1941), labelled by the states and the origin of the + * attempt, so an incident can be reconstructed from `/metrics` alone. + */ + recordRejectedPaymentTransition( + from: string, + to: string, + source: string, + ): void { + this.incrementCounter('managehub_payment_rejected_transitions_total', { + from, + to, + source, + }); + } + recordSettlementPayoutAttempt(result: 'submitted' | 'confirmed' | 'failed'): void { this.incrementCounter('managehub_settlement_payout_attempts_total', { result, diff --git a/backend/src/common/reconciliation-mismatch-metric.util.ts b/backend/src/common/reconciliation-mismatch-metric.util.ts deleted file mode 100644 index c03daf92..00000000 --- a/backend/src/common/reconciliation-mismatch-metric.util.ts +++ /dev/null @@ -1,15 +0,0 @@ -// Emits a metric for mismatches found per reconciliation.service.ts run, -// so a growing drift has an alerting signal via common/metrics.service.ts. -export interface MetricsGaugeSink { - setGauge(name: string, value: number, labels?: Record): void; -} - -const METRIC_NAME = 'reconciliation_mismatch_count'; - -export function recordReconciliationMismatchCount( - sink: MetricsGaugeSink, - mismatchCount: number, - runId: string, -): void { - sink.setGauge(METRIC_NAME, mismatchCount, { runId }); -} diff --git a/backend/src/common/ws-cors.io-adapter.spec.ts b/backend/src/common/ws-cors.io-adapter.spec.ts new file mode 100644 index 00000000..5b92e17d --- /dev/null +++ b/backend/src/common/ws-cors.io-adapter.spec.ts @@ -0,0 +1,76 @@ +import { IoAdapter } from '@nestjs/platform-socket.io'; +import { WsCorsIoAdapter } from './ws-cors.io-adapter'; + +function makeConfig(values: Record) { + return { + get: jest.fn((key: string) => values[key]), + }; +} + +describe('WsCorsIoAdapter (issue #1943)', () => { + afterEach(() => { + jest.restoreAllMocks(); + }); + + function buildCors(values: Record) { + const config = makeConfig(values); + const adapter = new WsCorsIoAdapter({} as any, config as any); + + // Capture the options the adapter hands to the underlying socket.io + // server instead of opening a real port. + const superCreate = jest + .spyOn(IoAdapter.prototype, 'createIOServer') + .mockReturnValue({} as any); + + adapter.createIOServer(0, { namespace: '/payments' } as any); + const passedOptions = superCreate.mock.calls[0][1] as any; + return passedOptions.cors as { + credentials: boolean; + origin: ( + origin: string | undefined, + callback: (err: Error | null, allow?: boolean) => void, + ) => void; + }; + } + + it('allows an origin present in CORS_ORIGINS and rejects one outside it', () => { + const cors = buildCors({ + CORS_ORIGINS: 'https://app.example.com,https://admin.example.com', + NODE_ENV: 'production', + }); + + expect(cors.credentials).toBe(true); + + const allowed = jest.fn(); + cors.origin('https://admin.example.com', allowed); + expect(allowed).toHaveBeenCalledWith(null, true); + + const rejected = jest.fn(); + cors.origin('https://evil.example.com', rejected); + expect(rejected.mock.calls[0][0]).toBeInstanceOf(Error); + expect(rejected.mock.calls[0][1]).toBe(false); + }); + + it('reads the allowlist at runtime, so .env-only configuration works', () => { + const cors = buildCors({ + CORS_ORIGINS: 'https://from-dotenv.example.com', + NODE_ENV: 'production', + }); + + const allowed = jest.fn(); + cors.origin('https://from-dotenv.example.com', allowed); + expect(allowed).toHaveBeenCalledWith(null, true); + }); + + it('falls back to the development defaults when NODE_ENV is development', () => { + const cors = buildCors({ NODE_ENV: 'development' }); + + const allowed = jest.fn(); + cors.origin('http://localhost:3000', allowed); + expect(allowed).toHaveBeenCalledWith(null, true); + + const rejected = jest.fn(); + cors.origin('https://evil.example.com', rejected); + expect(rejected.mock.calls[0][1]).toBe(false); + }); +}); diff --git a/backend/src/common/ws-cors.io-adapter.ts b/backend/src/common/ws-cors.io-adapter.ts new file mode 100644 index 00000000..80bd9511 --- /dev/null +++ b/backend/src/common/ws-cors.io-adapter.ts @@ -0,0 +1,53 @@ +import { INestApplicationContext, Logger } from '@nestjs/common'; +import { ConfigService } from '@nestjs/config'; +import { IoAdapter } from '@nestjs/platform-socket.io'; +import type { ServerOptions } from 'socket.io'; +import { isOriginAllowed, resolveAllowedOrigins } from './cors'; + +/** + * Resolves the Socket.IO allowlist at runtime instead of at import time + * (issue #1943). + * + * `@WebSocketGateway({ cors: { origin: process.env.FRONTEND_URL ... } })` is + * evaluated when the gateway file is imported — before `ConfigModule.forRoot` + * has loaded `.env` — so an `.env`-only `FRONTEND_URL` was silently ignored + * and a single hard-coded origin was used. This adapter reads the same + * `CORS_ORIGINS` allowlist the HTTP API uses (via `resolveAllowedOrigins` / + * `isOriginAllowed`), so both surfaces share one policy and multiple origins + * are supported. It is installed during application bootstrap, after config + * has loaded and before gateways are discovered. + */ +export class WsCorsIoAdapter extends IoAdapter { + private readonly logger = new Logger(WsCorsIoAdapter.name); + private readonly allowedOrigins: readonly string[]; + + constructor(app: INestApplicationContext, config: ConfigService) { + super(app); + this.allowedOrigins = resolveAllowedOrigins({ + CORS_ORIGINS: config.get('CORS_ORIGINS'), + NODE_ENV: config.get('NODE_ENV'), + }); + } + + createIOServer(port: number, options?: ServerOptions) { + return super.createIOServer(port, { + ...options, + cors: { + credentials: true, + origin: ( + origin: string | undefined, + callback: (err: Error | null, allow?: boolean) => void, + ) => { + if (isOriginAllowed(origin, this.allowedOrigins)) { + callback(null, true); + return; + } + + const rejection = `WebSocket CORS origin "${origin}" is not allowlisted`; + this.logger.warn(rejection); + callback(new Error(rejection), false); + }, + }, + }); + } +} diff --git a/backend/src/database/data-source.ts b/backend/src/database/data-source.ts index b1a40bf0..275224bd 100644 --- a/backend/src/database/data-source.ts +++ b/backend/src/database/data-source.ts @@ -29,5 +29,3 @@ export const AppDataSource = new DataSource({ max: DB_POOL_MAX, }, }); - -export default AppDataSource; diff --git a/backend/src/main.ts b/backend/src/main.ts index 1ce4e20b..3c68e135 100644 --- a/backend/src/main.ts +++ b/backend/src/main.ts @@ -1,10 +1,12 @@ import { NestFactory } from '@nestjs/core'; import { Logger, ValidationPipe } from '@nestjs/common'; +import { ConfigService } from '@nestjs/config'; import { NestExpressApplication } from '@nestjs/platform-express'; import { SwaggerModule, DocumentBuilder } from '@nestjs/swagger'; import { AppModule } from './app.module'; import { AllExceptionsFilter } from './common/http-exception.filter'; import { isOriginAllowed, resolveAllowedOrigins } from './common/cors'; +import { WsCorsIoAdapter } from './common/ws-cors.io-adapter'; import { GracefulShutdownServer, installGracefulShutdown, @@ -44,6 +46,13 @@ async function bootstrap() { app.use((req, res, next) => requestLogger.use(req, res, next)); } + // The websocket gateway must share the HTTP API's CORS_ORIGINS allowlist + // (issue #1943). The IoAdapter is installed here, after ConfigModule has + // loaded `.env` but before app.init() discovers gateways — setting it in + // an onModuleInit hook would be too late because Nest connects gateways in + // registerWsModule(), which runs before lifecycle hooks. + app.useWebSocketAdapter(new WsCorsIoAdapter(app, app.get(ConfigService))); + const requestBodyLimit = process.env.REQUEST_BODY_LIMIT ?? '1mb'; const captureRawBody = ( req: Request, diff --git a/backend/src/payments/README.md b/backend/src/payments/README.md index 58f86483..f63557c9 100644 --- a/backend/src/payments/README.md +++ b/backend/src/payments/README.md @@ -57,7 +57,9 @@ mass-flag every in-flight payment. ### Admin recovery actions (`PaymentsAdminController`, `ADMIN` role only) -- `GET /payments/admin/manual-review` — the review queue, oldest first. +- `GET /payments/admin/manual-review` — the review queue, oldest first, + paginated via `limit` (1–100, default 25) and `offset` (default 0); the + response is `{ items, total }`. - `GET /payments/admin/metrics` — manual-review queue depth + alert status. - `POST /payments/admin/:id/force-reconcile` — reconcile one payment now, bypassing the backoff schedule. @@ -99,6 +101,24 @@ scheduled job logs a `WARN`-level alert when it does. This is a log-based signal by design — this module doesn't assume any particular metrics/observability backend is wired up yet. +Each reconciliation run also records ledger/provider drift on `/metrics` +(issue #1942) as: + +- `managehub_reconciliation_mismatch_count` (gauge) — mismatches detected by + the **most recent** run. A mismatch is any payment whose stored status the + run found out of line with the authoritative source: one the provider had + already settled (resolved), one past its TTL (expired-swept), or one + escalated to `MANUAL_REVIEW`. +- `managehub_reconciliation_mismatches_total` (counter) — cumulative + mismatches across all runs. + +**Alert threshold:** page when `managehub_reconciliation_mismatch_count` is +non-zero on consecutive runs, or when +`rate(managehub_reconciliation_mismatches_total[1h])` climbs — a single +non-zero sample can be a normal webhook-vs-poll race, but sustained drift +means the webhook/confirmation path is failing and payments are only being +settled by the reconciler. + ## Webhook payload contract Every payment rail adapter maps its provider-specific confirmation event diff --git a/backend/src/payments/dto/list-manual-review-query.dto.ts b/backend/src/payments/dto/list-manual-review-query.dto.ts new file mode 100644 index 00000000..9b7690a9 --- /dev/null +++ b/backend/src/payments/dto/list-manual-review-query.dto.ts @@ -0,0 +1,37 @@ +import { ApiPropertyOptional } from '@nestjs/swagger'; +import { Type } from 'class-transformer'; +import { IsInt, Max, Min } from 'class-validator'; +import { + DEFAULT_PAGE_SIZE, + MAX_PAGE_SIZE, +} from '../payments-admin-pagination.util'; + +/** + * Offset pagination for `GET /payments/admin/manual-review` (issue #1944). + * Bounds the manual-review queue response so a provider outage that floods + * the queue can't produce an unbounded, unusable payload. Defaults and + * maximums are shared with `payments-admin-pagination.util.ts`. + */ +export class ListManualReviewQueryDto { + @ApiPropertyOptional({ + description: 'Page size (1–100)', + default: DEFAULT_PAGE_SIZE, + minimum: 1, + maximum: MAX_PAGE_SIZE, + }) + @Type(() => Number) + @IsInt() + @Min(1) + @Max(MAX_PAGE_SIZE) + limit: number = DEFAULT_PAGE_SIZE; + + @ApiPropertyOptional({ + description: 'Rows to skip from the start of the oldest-first queue', + default: 0, + minimum: 0, + }) + @Type(() => Number) + @IsInt() + @Min(0) + offset: number = 0; +} diff --git a/backend/src/payments/dto/manual-review-page-response.dto.ts b/backend/src/payments/dto/manual-review-page-response.dto.ts new file mode 100644 index 00000000..0b191e5c --- /dev/null +++ b/backend/src/payments/dto/manual-review-page-response.dto.ts @@ -0,0 +1,18 @@ +import { ApiProperty } from '@nestjs/swagger'; +import { PaymentResponseDto } from './payment-response.dto'; + +/** + * Paginated manual-review queue response (issue #1944). `total` is the + * full count of MANUAL_REVIEW payments, independent of the current page, + * so a client can compute the last page and render a pager. + */ +export class ManualReviewPageResponseDto { + @ApiProperty({ type: [PaymentResponseDto] }) + items: PaymentResponseDto[]; + + @ApiProperty({ + description: 'Total MANUAL_REVIEW payments across all pages', + type: Number, + }) + total: number; +} diff --git a/backend/src/payments/payment-confirmation.service.spec.ts b/backend/src/payments/payment-confirmation.service.spec.ts index 588107e9..54d7f3a4 100644 --- a/backend/src/payments/payment-confirmation.service.spec.ts +++ b/backend/src/payments/payment-confirmation.service.spec.ts @@ -114,6 +114,7 @@ describe('PaymentConfirmationService', () => { expect(paymentsService.transitionStatus).toHaveBeenCalledWith( payment, PaymentStatus.CONFIRMED, + 'webhook', ); expect(gateway.emitPaymentUpdate).toHaveBeenCalledWith( payment.id, diff --git a/backend/src/payments/payment-confirmation.service.ts b/backend/src/payments/payment-confirmation.service.ts index 903ef8b4..c2c55b25 100644 --- a/backend/src/payments/payment-confirmation.service.ts +++ b/backend/src/payments/payment-confirmation.service.ts @@ -11,11 +11,26 @@ import { PaymentVerificationOutcome } from './interfaces/payment-rail-adapter.in import { PaymentsService } from './payments.service'; import { PaymentsGateway } from './payments.gateway'; import { PaymentRailRegistry } from './payment-rail-registry'; +import { PaymentTransitionSource } from './payment-state-machine'; import { withTimeout } from './utils/with-timeout'; import { withRequestId } from '../common/request-context'; const DEFAULT_VERIFY_TIMEOUT_MS = 3000; +/** Maps a ConfirmationSource to the transition-source label used when a refused transition is logged (issue #1941). */ +function transitionSourceFor( + source: ConfirmationSource, +): PaymentTransitionSource { + switch (source) { + case ConfirmationSource.WEBHOOK: + return 'webhook'; + case ConfirmationSource.RECONCILIATION: + return 'reconciliation'; + default: + return 'verify_return'; + } +} + const TERMINAL_STATUSES: ReadonlySet = new Set([ PaymentStatus.CONFIRMED, PaymentStatus.FAILED, @@ -125,7 +140,11 @@ export class PaymentConfirmationService { const nextStatus = outcome === 'confirmed' ? PaymentStatus.CONFIRMED : PaymentStatus.FAILED; - this.paymentsService.transitionStatus(payment, nextStatus); + this.paymentsService.transitionStatus( + payment, + nextStatus, + transitionSourceFor(source), + ); const saved = await this.paymentRepository.save(payment); await this.logEvent({ diff --git a/backend/src/payments/payment-state-machine.spec.ts b/backend/src/payments/payment-state-machine.spec.ts index 11cf7894..0c5ed0fe 100644 --- a/backend/src/payments/payment-state-machine.spec.ts +++ b/backend/src/payments/payment-state-machine.spec.ts @@ -1,6 +1,7 @@ import { UnprocessableEntityException } from '@nestjs/common'; import { assertValidTransition, canTransition } from './payment-state-machine'; import { PaymentStatus } from './enums/payment-status.enum'; +import { runWithRequestContext } from '../common/request-context'; describe('payment state machine', () => { const ALL_STATUSES = Object.values(PaymentStatus); @@ -59,4 +60,64 @@ describe('payment state machine', () => { } } }); + + describe('rejected-transition incident record (issue #1941)', () => { + it('logs one structured warning and increments the metric with the states and source', () => { + const logger = { warn: jest.fn() }; + const metrics = { recordRejectedPaymentTransition: jest.fn() }; + + expect(() => + assertValidTransition(PaymentStatus.FAILED, PaymentStatus.CONFIRMED, { + paymentId: 'payment-1', + source: 'webhook', + logger, + metrics, + }), + ).toThrow(UnprocessableEntityException); + + expect(metrics.recordRejectedPaymentTransition).toHaveBeenCalledTimes(1); + expect(metrics.recordRejectedPaymentTransition).toHaveBeenCalledWith( + PaymentStatus.FAILED, + PaymentStatus.CONFIRMED, + 'webhook', + ); + + expect(logger.warn).toHaveBeenCalledTimes(1); + expect(logger.warn).toHaveBeenCalledWith( + expect.stringContaining('Rejected illegal payment state transition'), + expect.objectContaining({ + paymentId: 'payment-1', + from: PaymentStatus.FAILED, + to: PaymentStatus.CONFIRMED, + source: 'webhook', + }), + ); + }); + + it('includes the active request id from RequestContext', () => { + const logger = { warn: jest.fn() }; + const metrics = { recordRejectedPaymentTransition: jest.fn() }; + + runWithRequestContext({ requestId: 'request-abc' }, () => { + expect(() => + assertValidTransition( + PaymentStatus.EXPIRED, + PaymentStatus.CONFIRMED, + { paymentId: 'payment-2', source: 'reconciliation', logger, metrics }, + ), + ).toThrow(UnprocessableEntityException); + }); + + expect(logger.warn).toHaveBeenCalledWith( + expect.any(String), + expect.objectContaining({ requestId: 'request-abc' }), + ); + }); + + it('still throws without a logger or metrics sink', () => { + expect(() => + assertValidTransition(PaymentStatus.VOIDED, PaymentStatus.CONFIRMED), + ).toThrow(UnprocessableEntityException); + }); + }); }); diff --git a/backend/src/payments/payment-state-machine.ts b/backend/src/payments/payment-state-machine.ts index 16db1781..38706f4e 100644 --- a/backend/src/payments/payment-state-machine.ts +++ b/backend/src/payments/payment-state-machine.ts @@ -1,6 +1,44 @@ import { UnprocessableEntityException } from '@nestjs/common'; +import { currentRequestId } from '../common/request-context'; import { PaymentStatus } from './enums/payment-status.enum'; +/** + * Where a rejected state transition originated. Used only for the incident + * record (issue #1941) — see `assertValidTransition`. + */ +export type PaymentTransitionSource = + | 'webhook' + | 'reconciliation' + | 'admin' + | 'refund' + | 'verify_return' + | 'api'; + +/** + * Minimal structural types so this pure module never needs to import the + * concrete Nest Logger/MetricsService (keeps the state machine unit-testable + * and free of framework wiring). Both are optional: callers that have no + * logger/metrics simply get the throw with no incident record. + */ +interface RejectedTransitionLogger { + warn(message: string, context?: Record): void; +} + +interface RejectedTransitionMetrics { + recordRejectedPaymentTransition( + from: string, + to: string, + source: string, + ): void; +} + +export interface RejectedTransitionContext { + paymentId?: string; + source?: PaymentTransitionSource; + logger?: RejectedTransitionLogger; + metrics?: RejectedTransitionMetrics; +} + /** * The single source of truth for legal Payment status transitions. * No code outside PaymentsService.transitionStatus() may assign @@ -48,13 +86,37 @@ export function canTransition(from: PaymentStatus, to: PaymentStatus): boolean { return ALLOWED_TRANSITIONS[from]?.includes(to) ?? false; } +/** + * Throws when `from -> to` is illegal, recording one structured warning and + * incrementing a metric first (issue #1941) so a refused transition (e.g. a + * webhook retry hitting a terminal payment) leaves an incident trail with the + * payment id, both states, the origin of the attempt, and the active request + * id. The record is emitted only when a logger/metrics sink is supplied — the + * transition guard itself stays a pure, dependency-free check. + */ export function assertValidTransition( from: PaymentStatus, to: PaymentStatus, + context: RejectedTransitionContext = {}, ): void { - if (!canTransition(from, to)) { - throw new UnprocessableEntityException( - `Illegal payment status transition: ${from} -> ${to}`, - ); + if (canTransition(from, to)) { + return; } + const requestId = currentRequestId(); + const { paymentId, source, logger, metrics } = context; + metrics?.recordRejectedPaymentTransition( + from, + to, + source ?? 'api', + ); + logger?.warn(`Rejected illegal payment state transition: ${from} -> ${to}`, { + paymentId, + from, + to, + source: source ?? 'api', + requestId, + }); + throw new UnprocessableEntityException( + `Illegal payment status transition: ${from} -> ${to}`, + ); } diff --git a/backend/src/payments/payments-admin-pagination.util.ts b/backend/src/payments/payments-admin-pagination.util.ts index 01d0acb0..db1c2db6 100644 --- a/backend/src/payments/payments-admin-pagination.util.ts +++ b/backend/src/payments/payments-admin-pagination.util.ts @@ -1,7 +1,7 @@ // Offset pagination helper for payments-admin.controller.ts list // endpoints, with a sane default page size to bound unbounded queries. -const DEFAULT_PAGE_SIZE = 25; -const MAX_PAGE_SIZE = 100; +export const DEFAULT_PAGE_SIZE = 25; +export const MAX_PAGE_SIZE = 100; export interface PaginationParams { offset: number; diff --git a/backend/src/payments/payments-admin.controller.ts b/backend/src/payments/payments-admin.controller.ts index a9967253..5b43e013 100644 --- a/backend/src/payments/payments-admin.controller.ts +++ b/backend/src/payments/payments-admin.controller.ts @@ -27,6 +27,8 @@ import { RefundsService } from './refunds.service'; import { AdminActionLogService } from '../admin-audit/admin-action-log.service'; import { AdminActionType } from '../admin-audit/admin-action-type.enum'; import { PaymentResponseDto } from './dto/payment-response.dto'; +import { ListManualReviewQueryDto } from './dto/list-manual-review-query.dto'; +import { ManualReviewPageResponseDto } from './dto/manual-review-page-response.dto'; import { ReconciliationMetricsResponseDto } from './dto/reconciliation-metrics-response.dto'; import { ReconciliationRunResponseDto } from './dto/reconciliation-run-response.dto'; import { ResolvePaymentManuallyDto } from './dto/resolve-payment-manually.dto'; @@ -57,12 +59,22 @@ export class PaymentsAdminController { @Get('manual-review') @ApiOperation({ - summary: 'List payments awaiting manual review, oldest first', + summary: 'List payments awaiting manual review, oldest first (paginated)', + description: + 'Returns one page of the MANUAL_REVIEW queue. `limit` is clamped to 1–100 and defaults to 25; `offset` defaults to 0. The response carries the full queue `total` so a client can page through it.', }) - @ApiResponse({ status: 200, type: [PaymentResponseDto] }) - async listManualReview(): Promise { - const payments = await this.reconciliationService.listManualReview(); - return payments.map((payment) => PaymentResponseDto.fromEntity(payment)); + @ApiResponse({ status: 200, type: ManualReviewPageResponseDto }) + async listManualReview( + @Query() query: ListManualReviewQueryDto, + ): Promise { + const { items, total } = await this.reconciliationService.listManualReview( + query.limit, + query.offset, + ); + return { + items: items.map((payment) => PaymentResponseDto.fromEntity(payment)), + total, + }; } @Get('metrics') diff --git a/backend/src/payments/payments-admin.spec.ts b/backend/src/payments/payments-admin.spec.ts index 8fd91efc..90aec834 100644 --- a/backend/src/payments/payments-admin.spec.ts +++ b/backend/src/payments/payments-admin.spec.ts @@ -1,84 +1,199 @@ -// backend/src/payments/payments-admin.spec.ts -import { Test, TestingModule } from '@nestjs/testing'; +import { plainToInstance } from 'class-transformer'; +import { validate } from 'class-validator'; import { PaymentsAdminController } from './payments-admin.controller'; -import { PaymentsAdminService } from './payments-admin.service'; -import { JwtAuthGuard } from '../../common/guards/jwt-auth.guard'; -import { RolesGuard } from '../../common/guards/roles.guard'; -import { ExecutionContext } from '@nestjs/common'; -import { UserRole } from '../../users/entities/user.entity'; +import { ReconciliationService } from './reconciliation.service'; +import { ListManualReviewQueryDto } from './dto/list-manual-review-query.dto'; +import { Payment } from './entities/payment.entity'; +import { PaymentRail } from './enums/payment-rail.enum'; +import { PaymentStatus } from './enums/payment-status.enum'; -describe('PaymentsAdminController', () => { - let controller: PaymentsAdminController; - let paymentsAdminService: PaymentsAdminService; - - const mockPaymentsAdminService = { - getManualReviewList: jest.fn().mockResolvedValue([]), - getMetrics: jest.fn().mockResolvedValue({ totalVolume: 1000 }), - forceReconcile: jest.fn().mockResolvedValue({ reconciled: true }), - resolveManually: jest.fn().mockResolvedValue({ resolved: true }), - voidPayment: jest.fn().mockResolvedValue({ voided: true }), - refundPayment: jest.fn().mockResolvedValue({ refunded: true }), +function makeManualReviewPayment(index: number): Payment { + const base = new Date('2025-01-01T00:00:00.000Z').getTime(); + return { + id: `p${String(index).padStart(3, '0')}`, + bookingId: `booking-${index}`, + userId: 'user-1', + amount: 1_000, + currency: 'USD', + rail: PaymentRail.FIAT, + provider: null, + providerReference: `ref-${index}`, + status: PaymentStatus.MANUAL_REVIEW, + idempotencyKey: `key-${index}`, + metadata: null, + expiresAt: null, + failureReason: null, + reconciliationAttempts: 1, + providerErrorStreak: 0, + lastReconciledAt: null, + manualReviewReason: `reason-${index}`, + createdAt: new Date(base + index * 1_000), + updatedAt: new Date(base + index * 1_000), + } as Payment; +} + +/** Minimal Repository fake supporting the paginated findAndCount query. */ +function makePaymentRepository(seed: Payment[]) { + return { + findAndCount: jest.fn( + async (options: { + where: { status: PaymentStatus }; + order?: { updatedAt?: 'ASC' | 'DESC'; id?: 'ASC' | 'DESC' }; + take?: number; + skip?: number; + }) => { + const filtered = seed.filter( + (payment) => payment.status === options.where.status, + ); + filtered.sort((left, right) => { + const byUpdatedAt = + left.updatedAt.getTime() - right.updatedAt.getTime(); + return byUpdatedAt !== 0 + ? byUpdatedAt + : left.id.localeCompare(right.id); + }); + const total = filtered.length; + const skip = options.skip ?? 0; + const take = options.take ?? filtered.length; + return [ + filtered.slice(skip, skip + take).map((payment) => ({ ...payment })), + total, + ]; + }, + ), + count: jest.fn(async () => seed.length), + findOne: jest.fn(), }; +} + +describe('PaymentsAdminController — manual-review pagination (issue #1944)', () => { + let controller: PaymentsAdminController; + let reconciliationService: ReconciliationService; + let refundsService: { requestRefund: jest.Mock }; + let audit: { record: jest.Mock }; + + function build(seed: Payment[]) { + const paymentRepository = makePaymentRepository(seed); + reconciliationService = new ReconciliationService( + paymentRepository as any, + {} as any, + {} as any, + {} as any, + {} as any, + { get: (_key: string, fallback?: unknown) => fallback } as any, + { + setReconciliationMismatchCount: jest.fn(), + recordReconciliationMismatches: jest.fn(), + recordRejectedPaymentTransition: jest.fn(), + } as any, + ); + refundsService = { requestRefund: jest.fn() }; + audit = { record: jest.fn() }; + controller = new PaymentsAdminController( + reconciliationService, + refundsService as any, + audit as any, + ); + return controller; + } + + const seedReview = (count: number): Payment[] => + Array.from({ length: count }, (_, i) => makeManualReviewPayment(i)); + + const query = (limit: number, offset: number): ListManualReviewQueryDto => + plainToInstance(ListManualReviewQueryDto, { limit, offset }); + + it('returns the first page with the full total', async () => { + build(seedReview(60)); - beforeEach(async () => { - const module: TestingModule = await Test.createTestingModule({ - controllers: [PaymentsAdminController], - providers: [ - { provide: PaymentsAdminService, useValue: mockPaymentsAdminService }, - ], - }) - .overrideGuard(JwtAuthGuard) - .useValue({ - canActivate: (context: ExecutionContext) => { - const req = context.switchToHttp().getRequest(); - req.user = { id: 'admin-1', role: UserRole.ADMIN }; - return true; - }, - }) - .overrideGuard(RolesGuard) - .useValue({ - canActivate: (context: ExecutionContext) => { - const req = context.switchToHttp().getRequest(); - return req.user?.role === UserRole.ADMIN; - }, - }) - .compile(); - - controller = module.get(PaymentsAdminController); - paymentsAdminService = module.get(PaymentsAdminService); + const page = await controller.listManualReview(query(25, 0)); + + expect(page.total).toBe(60); + expect(page.items).toHaveLength(25); + expect(page.items[0].id).toBe('p000'); + expect(page.items[24].id).toBe('p024'); }); - it('should be defined', () => { - expect(controller).toBeDefined(); + it('returns a short last page', async () => { + build(seedReview(60)); + + const page = await controller.listManualReview(query(25, 50)); + + expect(page.total).toBe(60); + expect(page.items).toHaveLength(10); + expect(page.items[0].id).toBe('p050'); }); - it('should retrieve manual review listing and metrics', async () => { - const reviews = await controller.getManualReviewList(); - expect(paymentsAdminService.getManualReviewList).toHaveBeenCalled(); - expect(reviews).toEqual([]); + it('returns an empty page for an out-of-range offset', async () => { + build(seedReview(60)); + + const page = await controller.listManualReview(query(25, 500)); - const metrics = await controller.getMetrics(); - expect(paymentsAdminService.getMetrics).toHaveBeenCalled(); - expect(metrics).toEqual({ totalVolume: 1000 }); + expect(page.total).toBe(60); + expect(page.items).toEqual([]); }); - it('should execute force-reconcile and resolve-manually actions', async () => { - const reconcileRes = await controller.forceReconcile('tx-1'); - expect(paymentsAdminService.forceReconcile).toHaveBeenCalledWith('tx-1'); - expect(reconcileRes).toEqual({ reconciled: true }); + it('applies the default page size and rejects an over-maximum limit', async () => { + const defaults = plainToInstance(ListManualReviewQueryDto, {}); + expect(defaults.limit).toBe(25); + expect(defaults.offset).toBe(0); - const resolveRes = await controller.resolveManually('tx-1', { note: 'Resolved' } as any); - expect(paymentsAdminService.resolveManually).toHaveBeenCalledWith('tx-1', expect.any(Object)); - expect(resolveRes).toEqual({ resolved: true }); + const tooLarge = plainToInstance(ListManualReviewQueryDto, { limit: 101 }); + const errors = await validate(tooLarge); + expect(errors.some((error) => error.property === 'limit')).toBe(true); + + const negativeOffset = plainToInstance(ListManualReviewQueryDto, { + offset: -1, + }); + const offsetErrors = await validate(negativeOffset); + expect(offsetErrors.some((error) => error.property === 'offset')).toBe( + true, + ); }); - it('should handle void and refund routes', async () => { - const voidRes = await controller.voidPayment('tx-1'); - expect(paymentsAdminService.voidPayment).toHaveBeenCalledWith('tx-1'); - expect(voidRes).toEqual({ voided: true }); + it('delegates the remaining admin actions through the reconciliation service', async () => { + build(seedReview(1)); + const payment = makeManualReviewPayment(0); + jest + .spyOn(reconciliationService, 'forceReconcileNow') + .mockResolvedValue(payment); + jest + .spyOn(reconciliationService, 'resolveManually') + .mockResolvedValue(payment); + jest.spyOn(reconciliationService, 'void').mockResolvedValue(payment); + refundsService.requestRefund.mockResolvedValue({ + payment, + refund: { id: 'refund-1' }, + }); + + const reconciled = await controller.forceReconcile(payment.id); + expect(reconciled.id).toBe(payment.id); + + await controller.resolveManually( + payment.id, + { resolution: PaymentStatus.CONFIRMED, reason: 'checked' } as any, + { id: 'admin-1', role: 'ADMIN' } as any, + ); + expect(audit.record).toHaveBeenCalledTimes(1); + + await controller.void( + payment.id, + { reason: 'cancelled' } as any, + { id: 'admin-1', role: 'ADMIN' } as any, + ); + expect(audit.record).toHaveBeenCalledTimes(2); - const refundRes = await controller.refundPayment('tx-1', { amount: 50 } as any); - expect(paymentsAdminService.refundPayment).toHaveBeenCalledWith('tx-1', expect.any(Object)); - expect(refundRes).toEqual({ refunded: true }); + const refund = await controller.requestRefund( + payment.id, + { amount: 500, reason: 'partial' } as any, + { id: 'admin-1', role: 'ADMIN' } as any, + ); + expect(refundsService.requestRefund).toHaveBeenCalledWith( + payment.id, + 500, + 'partial', + 'admin-1', + ); + expect(refund.refund.id).toBe('refund-1'); }); -}); \ No newline at end of file +}); diff --git a/backend/src/payments/payments.gateway.ts b/backend/src/payments/payments.gateway.ts index e8cf17df..1c578a29 100644 --- a/backend/src/payments/payments.gateway.ts +++ b/backend/src/payments/payments.gateway.ts @@ -40,13 +40,15 @@ function paymentRoom(paymentId: string): string { * the payment reaches a terminal status. The socket is a latency * optimization, never the outcome's delivery guarantee; a dropped socket is * not a signal that the payment changed. See `payments/README.md`. + * + * CORS is deliberately NOT configured here: decorator arguments are + * evaluated at import time (before ConfigModule loads `.env`) and only + * accept a fixed origin. The `WsCorsIoAdapter` installed at bootstrap owns + * the origin allowlist and reads the same `CORS_ORIGINS` the HTTP API uses + * (issue #1943). */ @WebSocketGateway({ namespace: '/payments', - cors: { - origin: process.env.FRONTEND_URL || 'http://localhost:3000', - credentials: true, - }, }) export class PaymentsGateway implements OnGatewayConnection, OnGatewayDisconnect diff --git a/backend/src/payments/payments.service.ts b/backend/src/payments/payments.service.ts index 99aa45c0..d666322e 100644 --- a/backend/src/payments/payments.service.ts +++ b/backend/src/payments/payments.service.ts @@ -20,7 +20,7 @@ import { PaymentStatus, } from './enums/payment-status.enum'; import { PaymentRail } from './enums/payment-rail.enum'; -import { assertValidTransition } from './payment-state-machine'; +import { assertValidTransition, PaymentTransitionSource } from './payment-state-machine'; import { PaymentRailRegistry, PaymentRailResolution, @@ -118,10 +118,20 @@ export class PaymentsService { /** * The only sanctioned way to change Payment#status. Every write path in * this service must route through here so illegal transitions throw - * instead of silently corrupting state. + * instead of silently corrupting state. A refused transition is logged + * and counted (issue #1941) with the origin of the attempt. */ - transitionStatus(payment: Payment, next: PaymentStatus): Payment { - assertValidTransition(payment.status, next); + transitionStatus( + payment: Payment, + next: PaymentStatus, + source: PaymentTransitionSource = 'api', + ): Payment { + assertValidTransition(payment.status, next, { + paymentId: payment.id, + source, + logger: this.logger, + metrics: this.metrics, + }); this.metrics.recordPaymentTransition(payment.status, next); payment.status = next; return payment; diff --git a/backend/src/payments/reconciliation.service.spec.ts b/backend/src/payments/reconciliation.service.spec.ts index f11ba9d2..bac524d0 100644 --- a/backend/src/payments/reconciliation.service.spec.ts +++ b/backend/src/payments/reconciliation.service.spec.ts @@ -110,6 +110,44 @@ function makePaymentRepository(seed: Payment[]) { if (!options?.where) return rows.length; return rows.filter((p) => matchesWhere(p, options.where!)).length; }), + findAndCount: jest.fn( + async (options?: { + where?: Record | Record[]; + order?: Record; + take?: number; + skip?: number; + }) => { + let result = rows; + if (options?.where) { + const clauses = Array.isArray(options.where) + ? options.where + : [options.where]; + result = rows.filter((p) => + clauses.some((clause) => matchesWhere(p, clause)), + ); + } + if (options?.order) { + result = [...result].sort((left, right) => { + for (const [key, direction] of Object.entries(options.order!)) { + const a = (left as unknown as Record)[key]; + const b = (right as unknown as Record)[key]; + if (a === b) continue; + const comparison = + a instanceof Date && b instanceof Date + ? a.getTime() - b.getTime() + : String(a).localeCompare(String(b)); + return direction === 'DESC' ? -comparison : comparison; + } + return 0; + }); + } + const total = result.length; + const skip = options?.skip ?? 0; + const take = options?.take ?? result.length; + result = result.slice(skip, skip + take); + return [result.map((p) => ({ ...p })), total]; + }, + ), _rows: rows, }; } @@ -186,6 +224,9 @@ describe('ReconciliationService', () => { let metrics: { recordReconciliationPass: jest.Mock; setManualReviewDepth: jest.Mock; + setReconciliationMismatchCount: jest.Mock; + recordReconciliationMismatches: jest.Mock; + recordRejectedPaymentTransition: jest.Mock; }; beforeEach(() => { @@ -196,6 +237,9 @@ describe('ReconciliationService', () => { metrics = { recordReconciliationPass: jest.fn(), setManualReviewDepth: jest.fn(), + setReconciliationMismatchCount: jest.fn(), + recordReconciliationMismatches: jest.fn(), + recordRejectedPaymentTransition: jest.fn(), }; }); @@ -380,7 +424,9 @@ describe('ReconciliationService', () => { }); const summary = await service.reconcileDueBatch(now); - expect(summary.providerErrors).toBe(1); + // The cap-tripping attempt is reported as an escalation rather than a + // plain provider error — a single outcome per candidate. + expect(summary.providerErrors).toBe(0); expect(summary.escalatedToManualReview).toBe(1); expect(summary.providerErrorStreakCapEscalations).toBe(1); const updated = await paymentRepository.findOne({ @@ -604,10 +650,15 @@ describe('ReconciliationService', () => { const now = new Date(); const payment = makePayment({ createdAt: minutesAgo(10, now) }); let releaseVerify!: (outcome: { outcome: 'confirmed' }) => void; + let notifyVerifyStarted!: () => void; + const verifyStarted = new Promise((resolve) => { + notifyVerifyStarted = resolve; + }); railAdapter.verifyByReference.mockImplementation( () => new Promise((resolve) => { releaseVerify = resolve; + notifyVerifyStarted(); }), ); confirmationService.apply.mockResolvedValue({ @@ -617,10 +668,9 @@ describe('ReconciliationService', () => { const { service } = build([payment]); const firstPass = service.handleCron(); - // Give the first pass's microtasks a chance to reach the pending - // provider call before the "overlapping" second tick fires. - await Promise.resolve(); - await Promise.resolve(); + // Wait deterministically until the first pass is blocked on the + // provider call before firing the "overlapping" second tick. + await verifyStarted; await service.handleCron(); @@ -787,7 +837,8 @@ describe('ReconciliationService', () => { const { service } = build([inReview, confirmed]); const result = await service.listManualReview(); - expect(result.map((p) => p.id)).toEqual(['p1']); + expect(result.items.map((p) => p.id)).toEqual(['p1']); + expect(result.total).toBe(1); }); it('lists recent reconciliation runs newest first with a bounded limit', async () => { @@ -829,4 +880,87 @@ describe('ReconciliationService', () => { expect(metrics.confirmationMaxProviderErrorStreak).toBe(10); }); }); + + describe('listManualReview pagination (issue #1944)', () => { + function seedReview(count: number): Payment[] { + const base = new Date('2025-01-01T00:00:00.000Z').getTime(); + return Array.from({ length: count }, (_, i) => + makePayment({ + id: `p${String(i).padStart(3, '0')}`, + status: PaymentStatus.MANUAL_REVIEW, + updatedAt: new Date(base + i * 1_000), + }), + ); + } + + it('returns the first page with the full total and oldest-first ordering', async () => { + const { service } = build(seedReview(60)); + const result = await service.listManualReview(25, 0); + + expect(result.total).toBe(60); + expect(result.items).toHaveLength(25); + expect(result.items[0].id).toBe('p000'); + expect(result.items[24].id).toBe('p024'); + }); + + it('returns a short last page', async () => { + const { service } = build(seedReview(60)); + const result = await service.listManualReview(25, 50); + + expect(result.total).toBe(60); + expect(result.items).toHaveLength(10); + expect(result.items[0].id).toBe('p050'); + }); + + it('returns an empty page for an out-of-range offset without changing the total', async () => { + const { service } = build(seedReview(60)); + const result = await service.listManualReview(25, 1_000); + + expect(result.total).toBe(60); + expect(result.items).toEqual([]); + }); + }); + + describe('reconciliation mismatch metric (issue #1942)', () => { + it('records zero mismatches for a clean run where every candidate is still pending', async () => { + const now = new Date(); + const payment = makePayment({ createdAt: minutesAgo(10, now) }); + railAdapter.verifyByReference.mockResolvedValue({ outcome: 'pending' }); + + const { service } = build([payment]); + const summary = await service.reconcileDueBatch(now); + + expect(summary.mismatches).toBe(0); + expect(metrics.setReconciliationMismatchCount).toHaveBeenCalledWith(0); + expect(metrics.recordReconciliationMismatches).toHaveBeenCalledWith(0); + }); + + it('counts resolved and expired-swept payments as mismatches', async () => { + const now = new Date(); + const drifted = makePayment({ + id: 'drifted', + createdAt: minutesAgo(10, now), + updatedAt: minutesAgo(10, now), + }); + const expired = makePayment({ + id: 'stale', + status: PaymentStatus.INITIATED, + expiresAt: minutesAgo(1, now), + }); + confirmationService.apply.mockResolvedValue({ + ...drifted, + status: PaymentStatus.CONFIRMED, + }); + railAdapter.verifyByReference.mockResolvedValue({ outcome: 'confirmed' }); + + const { service } = build([drifted, expired]); + const summary = await service.reconcileDueBatch(now); + + expect(summary.resolved).toBe(1); + expect(summary.expiredSwept).toBe(1); + expect(summary.mismatches).toBe(2); + expect(metrics.setReconciliationMismatchCount).toHaveBeenCalledWith(2); + expect(metrics.recordReconciliationMismatches).toHaveBeenCalledWith(2); + }); + }); }); diff --git a/backend/src/payments/reconciliation.service.ts b/backend/src/payments/reconciliation.service.ts index 15cea098..66638063 100644 --- a/backend/src/payments/reconciliation.service.ts +++ b/backend/src/payments/reconciliation.service.ts @@ -15,6 +15,7 @@ import { PaymentStatus } from './enums/payment-status.enum'; import { PaymentFailureReason } from './enums/payment-failure-reason.enum'; import { ConfirmationSource } from './enums/confirmation-source.enum'; import { assertValidTransition } from './payment-state-machine'; +import { DEFAULT_PAGE_SIZE } from './payments-admin-pagination.util'; import { PaymentConfirmationService } from './payment-confirmation.service'; import { PaymentsGateway } from './payments.gateway'; import { PaymentRailRegistry } from './payment-rail-registry'; @@ -38,6 +39,13 @@ export interface ReconciliationSummary { attemptCapEscalations: number; /** Payments escalated specifically because the provider-error streak cap tripped. */ providerErrorStreakCapEscalations: number; + /** + * Stored-state mismatches this run detected against the authoritative + * source (issue #1942) — every payment that was resolved, expired-swept, or + * escalated because the ledger/provider/TTL truth disagreed with the stored + * status. Feeds the `managehub_reconciliation_mismatch_count` gauge. + */ + mismatches: number; } export interface ReconciliationMetrics { @@ -180,6 +188,7 @@ export class ReconciliationService { expiredSwept, attemptCapEscalations: 0, providerErrorStreakCapEscalations: 0, + mismatches: 0, }; for (const payment of awaiting) { @@ -229,6 +238,15 @@ export class ReconciliationService { } } + // A "mismatch" is any payment whose stored status this run found to be + // out of line with the authoritative source (issue #1942): one the + // provider had already settled (resolved), one past its TTL + // (expired-swept), or one stuck long enough to need a human (escalated). + summary.mismatches = + summary.resolved + summary.escalatedToManualReview + summary.expiredSwept; + this.metrics.setReconciliationMismatchCount(summary.mismatches); + this.metrics.recordReconciliationMismatches(summary.mismatches); + this.lastRunCapState = { attemptCapEscalations: summary.attemptCapEscalations, providerErrorStreakCapEscalations: @@ -262,7 +280,12 @@ export class ReconciliationService { ): Promise { this.assertReason(reason, 'resolve a payment manually'); const payment = await this.getPaymentOrThrow(paymentId); - assertValidTransition(payment.status, resolution); + assertValidTransition(payment.status, resolution, { + paymentId, + source: 'admin', + logger: this.logger, + metrics: this.metrics, + }); payment.status = resolution; payment.manualReviewReason = reason; @@ -281,7 +304,12 @@ export class ReconciliationService { async void(paymentId: string, reason: string): Promise { this.assertReason(reason, 'void a payment'); const payment = await this.getPaymentOrThrow(paymentId); - assertValidTransition(payment.status, PaymentStatus.VOIDED); + assertValidTransition(payment.status, PaymentStatus.VOIDED, { + paymentId, + source: 'admin', + logger: this.logger, + metrics: this.metrics, + }); payment.status = PaymentStatus.VOIDED; payment.manualReviewReason = reason; @@ -291,11 +319,19 @@ export class ReconciliationService { return saved; } - async listManualReview(): Promise { - return this.paymentRepository.find({ + async listManualReview( + limit: number = DEFAULT_PAGE_SIZE, + offset: number = 0, + ): Promise<{ items: Payment[]; total: number }> { + const [items, total] = await this.paymentRepository.findAndCount({ where: { status: PaymentStatus.MANUAL_REVIEW }, - order: { updatedAt: 'ASC' }, + // Oldest-first keeps the queue stable across pages; the id tiebreaker + // makes the ordering deterministic when two rows share an updatedAt. + order: { updatedAt: 'ASC', id: 'ASC' }, + take: limit, + skip: offset, }); + return { items, total }; } /** Returns persisted reconciliation history, newest first, with a bounded take. */ @@ -531,7 +567,12 @@ export class ReconciliationService { payment.status === PaymentStatus.INITIATED ? PaymentFailureReason.ABANDONED : PaymentFailureReason.EXPIRED; - assertValidTransition(payment.status, PaymentStatus.EXPIRED); + assertValidTransition(payment.status, PaymentStatus.EXPIRED, { + paymentId: payment.id, + source: 'reconciliation', + logger: this.logger, + metrics: this.metrics, + }); payment.status = PaymentStatus.EXPIRED; payment.failureReason = reason; await this.paymentRepository.save(payment); @@ -631,7 +672,12 @@ export class ReconciliationService { if (fresh.status !== PaymentStatus.AWAITING_CONFIRMATION) { return false; } - assertValidTransition(fresh.status, PaymentStatus.MANUAL_REVIEW); + assertValidTransition(fresh.status, PaymentStatus.MANUAL_REVIEW, { + paymentId: fresh.id, + source: 'reconciliation', + logger: this.logger, + metrics: this.metrics, + }); const maxAttempts = this.getConfirmationMaxAttempts(); const maxProviderErrorStreak = this.getConfirmationMaxProviderErrorStreak(); @@ -729,7 +775,12 @@ export class ReconciliationService { } const fresh = await this.getPaymentOrThrow(payment.id); - assertValidTransition(fresh.status, PaymentStatus.MANUAL_REVIEW); + assertValidTransition(fresh.status, PaymentStatus.MANUAL_REVIEW, { + paymentId: fresh.id, + source: 'reconciliation', + logger: this.logger, + metrics: this.metrics, + }); fresh.status = PaymentStatus.MANUAL_REVIEW; if (caps.length > 0) { const capDescriptions: string[] = []; diff --git a/backend/src/payments/state-transition-logger.util.ts b/backend/src/payments/state-transition-logger.util.ts deleted file mode 100644 index 51cd6de6..00000000 --- a/backend/src/payments/state-transition-logger.util.ts +++ /dev/null @@ -1,19 +0,0 @@ -// Logs rejected transitions from payment-state-machine.ts at warn level -// with both states and the payment id, so incidents like a webhook retry -// trying to re-release an already-released payment are reconstructable. -export interface RejectedTransitionLogger { - warn(message: string, context: Record): void; -} - -export function logRejectedTransition( - logger: RejectedTransitionLogger, - paymentId: string, - fromState: string, - toState: string, -): void { - logger.warn('Rejected illegal payment state transition', { - paymentId, - fromState, - toState, - }); -} diff --git a/frontend/app/admin/payments/page.tsx b/frontend/app/admin/payments/page.tsx index c523dfe5..2255a467 100644 --- a/frontend/app/admin/payments/page.tsx +++ b/frontend/app/admin/payments/page.tsx @@ -1,5 +1,6 @@ "use client"; +import { useState } from "react"; import { useQuery } from "@tanstack/react-query"; import Link from "next/link"; import { ClipboardList, ExternalLink } from "lucide-react"; @@ -13,6 +14,8 @@ import { } from "@/components/admin/ui"; import { RequireAdmin, useAdminToken } from "@/components/admin/require-admin"; +const PAGE_SIZE = 25; + export default function AdminManualReviewPage() { const token = useAdminToken(); return ( @@ -23,12 +26,18 @@ export default function AdminManualReviewPage() { } function ManualReviewInner({ token }: { token: string }) { - const { data: payments, isLoading, error } = useQuery({ - queryKey: ["manual-review-payments"], - queryFn: () => listManualReviewPayments(token), + const [offset, setOffset] = useState(0); + const { data, isLoading, error } = useQuery({ + queryKey: ["manual-review-payments", offset], + queryFn: () => listManualReviewPayments(token, PAGE_SIZE, offset), refetchInterval: 30_000, }); + const payments = data?.items ?? []; + const total = data?.total ?? 0; + const hasPrevious = offset > 0; + const hasNext = offset + PAGE_SIZE < total; + return (
@@ -46,7 +55,7 @@ function ManualReviewInner({ token }: { token: string }) { @@ -70,7 +79,7 @@ function ManualReviewInner({ token }: { token: string }) { - {payments?.map((p: Payment) => ( + {payments.map((p: Payment) => ( ))} - {!isLoading && (payments?.length ?? 0) === 0 && ( + {!isLoading && payments.length === 0 && (
)} + + {total > 0 && ( +
+ + Showing {total === 0 ? 0 : offset + 1}– + {Math.min(offset + PAGE_SIZE, total)} of {total} + +
+ + +
+
+ )}
); diff --git a/frontend/app/admin/refund/page.tsx b/frontend/app/admin/refund/page.tsx index fcd7f07b..39c985b2 100644 --- a/frontend/app/admin/refund/page.tsx +++ b/frontend/app/admin/refund/page.tsx @@ -113,12 +113,12 @@ export default function AdminRefundPage() {

Loading…

) : error ? (

{(error as Error).message}

- ) : data && data.length === 0 ? ( + ) : data && data.items.length === 0 ? (

No payments found in manual review.

) : ( - (data ?? []).map((payment) => ( + (data?.items ?? []).map((payment) => ( )) )} diff --git a/frontend/lib/payments-api.ts b/frontend/lib/payments-api.ts index 77ff5f23..46251804 100644 --- a/frontend/lib/payments-api.ts +++ b/frontend/lib/payments-api.ts @@ -86,6 +86,11 @@ export interface RequestRefundResponse { refund: Refund; } +export interface ManualReviewPage { + items: Payment[]; + total: number; +} + const API_BASE_URL = process.env.NEXT_PUBLIC_API_URL ?? ""; const TRANSIENT_RETRY_DELAY_MS = 250; const RETRYABLE_STATUS_CODES = new Set([408, 425, 429, 500, 502, 503, 504]); @@ -212,8 +217,17 @@ export function listMyUsage(accessToken: string): Promise { export function listManualReviewPayments( accessToken: string, -): Promise { - return apiFetch("/payments/admin/manual-review", accessToken); + limit = 25, + offset = 0, +): Promise { + const params = new URLSearchParams({ + limit: String(limit), + offset: String(offset), + }); + return apiFetch( + `/payments/admin/manual-review?${params.toString()}`, + accessToken, + ); } // ── admin payment actions (FE-104 / FE-106) ──────────────────────────────