From 3a2f1a46d6edb4db7736d9143cfb63fe25116a2d Mon Sep 17 00:00:00 2001 From: Marshal Date: Thu, 6 Aug 2026 13:24:42 +0000 Subject: [PATCH 1/2] fix(auth): resolve position-type permissions so GL staff can open clearance pages --- apps/edr-freight-api/src/app.module.ts | 4 + .../common/freight-permission.util.spec.ts | 72 +++++++++++ .../src/common/freight-permission.util.ts | 33 ++++- .../common/position-type-permissions.cache.ts | 83 +++++++++++++ .../src/modules/auth/freight-me.service.ts | 63 +++++++++- .../bookings/booking-invoice.service.ts | 15 +++ .../contract-booking.resubmit-cargo.spec.ts | 115 ++++++++++++++++++ .../contracts/contract-booking.service.ts | 21 +++- .../src/seed/freight-permissions.registry.ts | 15 ++- .../backoffice/src/auth/types.ts | 8 +- .../BookingChangesRequestedAlert.tsx | 18 ++- .../backoffice/src/lib/permissions.ts | 8 ++ .../contracts/ContractClearanceDetailPage.tsx | 1 + .../ChangesRequestedView.tsx | 21 +++- 14 files changed, 466 insertions(+), 11 deletions(-) create mode 100644 apps/edr-freight-api/src/common/position-type-permissions.cache.ts create mode 100644 apps/edr-freight-api/src/modules/contracts/contract-booking.resubmit-cargo.spec.ts diff --git a/apps/edr-freight-api/src/app.module.ts b/apps/edr-freight-api/src/app.module.ts index 73e19709e..f44d40d97 100644 --- a/apps/edr-freight-api/src/app.module.ts +++ b/apps/edr-freight-api/src/app.module.ts @@ -107,6 +107,7 @@ import { ImportOperationsModule } from "./modules/import-operations/import-opera import { AiModule } from "./modules/ai/ai.module"; import { LoggerMiddleware } from "./logger.middleware"; import { LoginAudienceMiddleware } from "./modules/auth/login-audience.middleware"; +import { PositionTypePermissionsCache } from "./common/position-type-permissions.cache"; @Module({ imports: [ @@ -251,6 +252,9 @@ import { LoginAudienceMiddleware } from "./modules/auth/login-audience.middlewar ApprovedFirstLastMileDemoBookingsSeeder, PaidImportExportMileDemoSeeder, LoginAudienceMiddleware, + // Feeds position-TYPE grants to the synchronous permission checks — without + // it, staff whose permissions live on their position type resolve to none. + PositionTypePermissionsCache, ], }) export class AppModule implements OnApplicationBootstrap { diff --git a/apps/edr-freight-api/src/common/freight-permission.util.spec.ts b/apps/edr-freight-api/src/common/freight-permission.util.spec.ts index d0d01535a..f3931f78a 100644 --- a/apps/edr-freight-api/src/common/freight-permission.util.spec.ts +++ b/apps/edr-freight-api/src/common/freight-permission.util.spec.ts @@ -1,6 +1,9 @@ import { assertCanApproveContractStep, canEditContractStep, + collectPermissionKeys, + hasFreightPermission, + setPositionTypePermissionResolver, } from './freight-permission.util'; import { FREIGHT_PERMS } from '../seed/freight-permissions.registry'; @@ -49,3 +52,72 @@ describe('canEditContractStep (strict per-step edit gate)', () => { ); }); }); + +/** + * The GL lockout regression: positions created through the admin UI keep their + * grants on the position TYPE, and the JWT only ever snapshots DIRECT position + * permissions. Without the type resolver those staff resolved to zero + * permissions, so every gated route rejected them — which is what kept GL + * officers out of their own clearance detail pages. + */ +describe('collectPermissionKeys — position-type grants', () => { + const CLEARANCE = FREIGHT_PERMS.contracts.clearanceReview; + + afterEach(() => { + setPositionTypePermissionResolver(() => []); + }); + + const glOfficer = { + roles: [], + permissions: [], + employee: { + position: { + permissions: [], // admin-created position carries NO direct grants + positionType: { key: 'commercial-global-logistics-(et)-officer' }, + }, + }, + }; + + it('resolves permissions carried by the position type', () => { + setPositionTypePermissionResolver((key) => + key === 'commercial-global-logistics-(et)-officer' ? [CLEARANCE] : [], + ); + + expect(collectPermissionKeys(glOfficer)).toContain(CLEARANCE); + expect(hasFreightPermission(glOfficer, CLEARANCE)).toBe(true); + }); + + it('handles the array-shaped employee payload too', () => { + setPositionTypePermissionResolver(() => [CLEARANCE]); + + const arrayShaped = { + roles: [], + permissions: [], + employee: [ + { + positions: [ + { permissions: [], positionType: { key: 'djibouti-gl-officer' } }, + ], + }, + ], + }; + + expect(hasFreightPermission(arrayShaped, CLEARANCE)).toBe(true); + }); + + it('still rejects when neither the position nor its type grants it', () => { + setPositionTypePermissionResolver(() => []); + + expect(hasFreightPermission(glOfficer, CLEARANCE)).toBe(false); + }); + + it('keeps direct position permissions working with no resolver installed', () => { + const direct = { + roles: [], + permissions: [], + employee: { position: { permissions: [{ key: CLEARANCE }] } }, + }; + + expect(hasFreightPermission(direct, CLEARANCE)).toBe(true); + }); +}); diff --git a/apps/edr-freight-api/src/common/freight-permission.util.ts b/apps/edr-freight-api/src/common/freight-permission.util.ts index 56c0e77c2..bc98a6df4 100644 --- a/apps/edr-freight-api/src/common/freight-permission.util.ts +++ b/apps/edr-freight-api/src/common/freight-permission.util.ts @@ -42,12 +42,41 @@ export function isFreightApprovalAdmin(user: MeLikeUser | null | undefined): boo return isSuperAdmin(user) || isOrganizationAdmin(user); } -/** Flat permission keys from JWT / session user (roles + position permissions). */ +/** + * Permissions carried by a position TYPE rather than the position itself. + * + * The JWT snapshots only DIRECT position permissions, so type-level grants — + * which is where admin-created positions keep theirs — are absent from the + * token entirely. This resolver is installed at startup + * (see `PositionTypePermissionsCache`) so the synchronous permission checks + * below can still see them. Left as a no-op resolver until then, which + * degrades to the old position-only behaviour rather than throwing. + */ +let positionTypePermissionResolver: (positionTypeKey: string) => string[] = () => + []; + +export function setPositionTypePermissionResolver( + resolver: (positionTypeKey: string) => string[], +): void { + positionTypePermissionResolver = resolver; +} + +/** + * Flat permission keys from JWT / session user: roles, position permissions, + * and the grants held by each position's TYPE. + */ export function collectPermissionKeys(user: MeLikeUser | null | undefined): string[] { if (!user) return []; const keys = new Set(); + const addTypePermissions = (positionType: PositionTypeLike | null | undefined) => { + if (!positionType?.key) return; + for (const key of positionTypePermissionResolver(positionType.key)) { + keys.add(key); + } + }; + for (const p of user.permissions ?? []) { if (p.key) keys.add(p.key); } @@ -63,6 +92,7 @@ export function collectPermissionKeys(user: MeLikeUser | null | undefined): stri for (const p of pos.permissions ?? []) { if (p.key) keys.add(p.key); } + addTypePermissions(pos.positionType); } } return [...keys]; @@ -71,6 +101,7 @@ export function collectPermissionKeys(user: MeLikeUser | null | undefined): stri for (const p of employee.position?.permissions ?? []) { if (p.key) keys.add(p.key); } + addTypePermissions(employee.position?.positionType); for (const delegated of employee.delegatedPositions ?? []) { for (const p of delegated.permissions ?? []) { if (p.key) keys.add(p.key); diff --git a/apps/edr-freight-api/src/common/position-type-permissions.cache.ts b/apps/edr-freight-api/src/common/position-type-permissions.cache.ts new file mode 100644 index 000000000..2848ed0b2 --- /dev/null +++ b/apps/edr-freight-api/src/common/position-type-permissions.cache.ts @@ -0,0 +1,83 @@ +import { Injectable, Logger, OnModuleInit } from '@nestjs/common'; +import { InjectDataSource } from '@nestjs/typeorm'; +import { DataSource } from 'typeorm'; + +import { setPositionTypePermissionResolver } from './freight-permission.util'; + +/** + * Permissions granted to a position TYPE (`iam.position_type_permissions`). + * + * A position type is the platform's notion of a role, and positions created + * through the admin UI carry their grants there rather than on the position + * itself. The JWT only ever snapshots DIRECT position permissions, so those + * grants are invisible to `collectPermissionKeys` — staff on such a position + * resolve to zero permissions and every permission-gated route rejects them. + * + * The permission checks (`hasFreightPermission`, `FreightPermissionGuard`) are + * synchronous and sit on the request path, so the mapping is held in memory and + * refreshed periodically rather than queried per request. The dataset is tiny + * (tens of types, a few hundred rows), so a full reload is cheaper than any + * incremental scheme. + */ +@Injectable() +export class PositionTypePermissionsCache implements OnModuleInit { + private readonly logger = new Logger(PositionTypePermissionsCache.name); + + /** position_type key → permission keys. Empty until the first load lands. */ + private byPositionTypeKey = new Map(); + + // ponytail: fixed 5-min refresh, no invalidation hook. A permission granted + // in the admin UI takes up to one interval to reach the guards. Wire the + // grant mutation to call `refresh()` if that lag ever matters. + private static readonly REFRESH_INTERVAL_MS = 5 * 60 * 1000; + + constructor(@InjectDataSource() private readonly dataSource: DataSource) {} + + async onModuleInit(): Promise { + await this.refresh(); + // Hand the lookup to the permission utils, whose checks are synchronous and + // therefore cannot query IAM themselves. + setPositionTypePermissionResolver((positionTypeKey) => + this.get(positionTypeKey), + ); + const timer = setInterval(() => { + void this.refresh(); + }, PositionTypePermissionsCache.REFRESH_INTERVAL_MS); + // Never hold the process open for a cache refresh. + timer.unref?.(); + } + + /** Permission keys for a position-type key ([] when unknown/not loaded). */ + get(positionTypeKey: string | undefined | null): string[] { + if (!positionTypeKey) return []; + return this.byPositionTypeKey.get(positionTypeKey) ?? []; + } + + /** Reload the whole mapping. Failures keep the previous snapshot in place. */ + async refresh(): Promise { + try { + const rows: { position_type_key: string; permission_key: string }[] = + await this.dataSource.query( + `SELECT pt.key AS position_type_key, perm.key AS permission_key + FROM iam.position_type_permissions ptp + JOIN iam.position_types pt ON pt.id = ptp.position_type_id + JOIN iam.permissions perm ON perm.id = ptp.permission_id`, + ); + + const next = new Map(); + for (const row of rows) { + if (!row.position_type_key || !row.permission_key) continue; + const keys = next.get(row.position_type_key); + if (keys) keys.push(row.permission_key); + else next.set(row.position_type_key, [row.permission_key]); + } + this.byPositionTypeKey = next; + } catch (err) { + // iam schema unreachable — keep serving the previous snapshot rather than + // dropping every type-derived permission and locking staff out. + this.logger.warn( + `Position-type permission refresh failed: ${(err as Error).message}`, + ); + } + } +} diff --git a/apps/edr-freight-api/src/modules/auth/freight-me.service.ts b/apps/edr-freight-api/src/modules/auth/freight-me.service.ts index 006b482f4..6bcfd964d 100644 --- a/apps/edr-freight-api/src/modules/auth/freight-me.service.ts +++ b/apps/edr-freight-api/src/modules/auth/freight-me.service.ts @@ -35,10 +35,57 @@ export class FreightMeService { } } + /** + * Permissions granted to the position's TYPE (`iam.position_type_permissions`). + * A position type is the platform's notion of a role, and admin-created + * positions carry their grants there rather than on the position itself — but + * the JWT only ever snapshots direct position permissions. Without this, staff + * on such a position resolve to zero permissions and every permission-gated + * route rejects them (this is what locked GL officers out of their clearance + * detail pages). Resolved live from IAM, same as the position type above. + */ + private async lookupPositionTypePermissions( + positionId: string | undefined, + ): Promise { + if (!positionId) return []; + try { + const rows: { key: string }[] = await this.dataSource.query( + `SELECT DISTINCT perm.key + FROM iam.positions p + JOIN iam.position_type_permissions ptp + ON ptp.position_type_id = p.position_type_id + JOIN iam.permissions perm ON perm.id = ptp.permission_id + WHERE p.id = $1`, + [positionId], + ); + return rows.map((r) => r.key).filter(Boolean); + } catch { + return []; // iam schema unreachable — degrade to position-only permissions + } + } + async getEnrichedProfile(user: TCurrentUser) { - const positionType = await this.lookupPositionType( - user.employee?.position?.id, + const positionId = user.employee?.position?.id; + const [positionType, positionTypePermissionKeys] = await Promise.all([ + this.lookupPositionType(positionId), + this.lookupPositionTypePermissions(positionId), + ]); + + // Merge the type-level grants into the position's own permission list so + // BOTH consumers see them: `collectPermissionKeys` below, and the + // backoffice's `getPermissionKeys`, which walks this same nested array. + const positionPermissions = [ + ...(user.employee?.position?.permissions ?? []), + ]; + const seenPermissionKeys = new Set( + positionPermissions.map((p) => p?.key).filter(Boolean), ); + for (const key of positionTypePermissionKeys) { + if (!seenPermissionKeys.has(key)) { + seenPermissionKeys.add(key); + positionPermissions.push({ key } as (typeof positionPermissions)[number]); + } + } const employee = user.employee ? [ @@ -56,7 +103,7 @@ export class FreightMeService { name: user.employee.position.name, isDelegate: user.employee.position.isDelegate, parentPositionId: user.employee.position.parentPositionId, - permissions: user.employee.position.permissions ?? [], + permissions: positionPermissions, positionType, }, ] @@ -65,7 +112,15 @@ export class FreightMeService { ] : []; - const permissionKeys = collectPermissionKeys(user); + // `collectPermissionKeys` reads the raw token (position-level only), so + // union the type-level grants in — the backoffice prefers this flat list + // over the nested array and would otherwise still see none of them. + const permissionKeys = [ + ...new Set([ + ...collectPermissionKeys(user), + ...positionTypePermissionKeys, + ]), + ]; return { id: user.id, diff --git a/apps/edr-freight-api/src/modules/bookings/booking-invoice.service.ts b/apps/edr-freight-api/src/modules/bookings/booking-invoice.service.ts index cd803d719..faeab8cee 100644 --- a/apps/edr-freight-api/src/modules/bookings/booking-invoice.service.ts +++ b/apps/edr-freight-api/src/modules/bookings/booking-invoice.service.ts @@ -90,6 +90,21 @@ export class BookingInvoiceService { return this.billing.generateInvoice(input); } + /** + * Cancel the booking's open PREPAID invoice, if any — used when a + * changes-requested resubmit restates the cargo, so the re-priced booking can + * be re-invoiced. Throws when the invoice already has payments recorded + * (cargo must not change out from under recorded money). + */ + async cancelUnpaidInvoiceForBooking(bookingId: string): Promise { + const existing = await this.billing.findPayable( + Freight.InvoiceSource.Booking, + bookingId, + "PREPAID", + ); + if (existing) await this.billing.cancelInvoice(existing.id); + } + /** * React to a booking invoice being paid — the settlement branch point. Per-type * reactions live here (not in the payment process): each invoice type advances diff --git a/apps/edr-freight-api/src/modules/contracts/contract-booking.resubmit-cargo.spec.ts b/apps/edr-freight-api/src/modules/contracts/contract-booking.resubmit-cargo.spec.ts new file mode 100644 index 000000000..d5d15d73d --- /dev/null +++ b/apps/edr-freight-api/src/modules/contracts/contract-booking.resubmit-cargo.spec.ts @@ -0,0 +1,115 @@ +import { ContractBookingService } from './contract-booking.service'; + +/** + * OPERATION_CHANGES_REQUESTED resubmit with restated cargo. Operations can ask + * for the cargo itself to change, so a completion payload that restates + * containers must cancel the unpaid invoice, wipe the persisted cargo and + * re-run the fresh-completion path (re-persist, re-price, re-invoice). A + * payload without cargo keeps the day-only resubmit behavior. + */ +describe('ContractBookingService — changes-requested resubmit restating cargo', () => { + const CONTRACT = { + id: 'c-1', + reference: 'CTR-1', + contractKind: 'GENERAL', + freightType: 'CONTAINER', + tradeDirection: 'IMPORT', + customsClearingEnabled: false, + contractValidUntil: null, + cargoScope: [], + }; + + const bookingWithCargo = () => ({ + id: 'b-1', + contractId: 'c-1', + reference: 'BKG-1', + status: 'OPERATION_CHANGES_REQUESTED', + bookingContainers: [{ containerSize: '20FT', quantity: 4 }], + cargoTotalWeightVgm: 80, + originYardId: 'y-o', + destinationYardId: 'y-d', + }); + + function makeService() { + const bookingsRepository = { + findByIdWithFiles: jest.fn().mockResolvedValue(bookingWithCargo()), + deleteContainers: jest.fn().mockResolvedValue(undefined), + update: jest.fn().mockResolvedValue(undefined), + }; + const invoiceService = { + cancelUnpaidInvoiceForBooking: jest.fn().mockResolvedValue(undefined), + }; + const trainSchedulingService = { + assertBookingWindowOpen: jest.fn().mockResolvedValue(undefined), + }; + const contractsRepository = { + findByIdWithRelations: jest.fn().mockResolvedValue(CONTRACT), + }; + const service = new ContractBookingService( + contractsRepository as never, + bookingsRepository as never, + {} as never, // bookingPricingService + {} as never, // consolidationService + {} as never, // containerTypesService + {} as never, // ruleEngineService + {} as never, // milestoneService + invoiceService as never, + {} as never, // bookingNotifier + {} as never, // dataSource + trainSchedulingService as never, + {} as never, // bookingBatchService + {} as never, // bookingTransitionService + ); + return { service, bookingsRepository, invoiceService }; + } + + // Both paths dead-end into a downstream private assert we replace with a + // sentinel — which path threw tells us which branch the resubmit took. + const SENTINEL = new Error('reached-branch'); + + it('restated cargo cancels the invoice, wipes cargo and re-runs fresh completion', async () => { + const { service, bookingsRepository, invoiceService } = makeService(); + // First gate inside the fresh-completion (!hasCargo) path. + jest + .spyOn( + service as never as { assertWithinQuantityCap: () => Promise }, + 'assertWithinQuantityCap', + ) + .mockRejectedValue(SENTINEL); + + await expect( + service.completeUnderContract('c-1', 'b-1', { + scheduledDate: new Date().toISOString(), + containers: [{ containerSize: '20FT', quantity: 2 }], + } as never), + ).rejects.toBe(SENTINEL); + + expect(invoiceService.cancelUnpaidInvoiceForBooking).toHaveBeenCalledWith('b-1'); + expect(bookingsRepository.deleteContainers).toHaveBeenCalledWith('b-1'); + expect(bookingsRepository.update).toHaveBeenCalledWith('b-1', { + cargoTotalWeightVgm: 0, + }); + }); + + it('a day-only resubmit keeps the persisted cargo and invoice untouched', async () => { + const { service, bookingsRepository, invoiceService } = makeService(); + // First call inside the day-only (hasCargo) resubmit path. + jest + .spyOn( + service as never as { + assertPersistedContainersAvailable: () => Promise; + }, + 'assertPersistedContainersAvailable', + ) + .mockRejectedValue(SENTINEL); + + await expect( + service.completeUnderContract('c-1', 'b-1', { + scheduledDate: new Date().toISOString(), + } as never), + ).rejects.toBe(SENTINEL); + + expect(invoiceService.cancelUnpaidInvoiceForBooking).not.toHaveBeenCalled(); + expect(bookingsRepository.deleteContainers).not.toHaveBeenCalled(); + }); +}); diff --git a/apps/edr-freight-api/src/modules/contracts/contract-booking.service.ts b/apps/edr-freight-api/src/modules/contracts/contract-booking.service.ts index 7e0a23e3f..1c152d795 100644 --- a/apps/edr-freight-api/src/modules/contracts/contract-booking.service.ts +++ b/apps/edr-freight-api/src/modules/contracts/contract-booking.service.ts @@ -692,11 +692,30 @@ export class ContractBookingService { }); const freightType = contract.freightType; - const hasCargo = + let hasCargo = (booking.bookingContainers?.length ?? 0) > 0 || Number(booking.cargoTotalWeightVgm) > 0; const warnings: string[] = []; + // Operations may return a booking asking for the CARGO to change (fewer or + // more containers), not just the day. A resubmit whose payload restates the + // cargo therefore starts the completion over: cancel the unpaid invoice + // first (it throws if money is already recorded — cargo must not change + // under a paid invoice), then wipe the persisted cargo so the fresh- + // completion path below re-persists, re-prices and re-invoices from the + // payload. A resubmit without cargo keeps today's day-only behavior. + const restatesCargo = Boolean( + dto.containers?.length || dto.bulkLines?.length, + ); + if (hasCargo && restatesCargo) { + await this.invoiceService.cancelUnpaidInvoiceForBooking(booking.id); + await this.bookingsRepository.deleteContainers(booking.id); + await this.bookingsRepository.update(booking.id, { + cargoTotalWeightVgm: 0, + } as never); + hasCargo = false; + } + // EXPORT rides whole or not at all (no split concept): the chosen day must // have a single open train that carries the whole booking. First completion // sizes from the dto's cargo; a changes-requested resubmit (cargo already diff --git a/apps/edr-freight-api/src/seed/freight-permissions.registry.ts b/apps/edr-freight-api/src/seed/freight-permissions.registry.ts index c30a2ebda..4f3b171bb 100644 --- a/apps/edr-freight-api/src/seed/freight-permissions.registry.ts +++ b/apps/edr-freight-api/src/seed/freight-permissions.registry.ts @@ -947,7 +947,20 @@ export const POSITION_PERMISSION_PRESETS = { FREIGHT_PERMS.customers.verify, FREIGHT_PERMS.customers.deactivate, ]), - director: dedupe([...ROLE_PERMISSION_PRESETS.director]), + // Director additionally manages train scheduling + rail fleet (same block the + // operation officer/chief hold), on top of the approval-chain role preset. + director: dedupe([ + ...ROLE_PERMISSION_PRESETS.director, + FREIGHT_PERMS.trainScheduling.view, + FREIGHT_PERMS.trainScheduling.create, + FREIGHT_PERMS.trainScheduling.update, + FREIGHT_PERMS.trainScheduling.cancel, + FREIGHT_PERMS.trainScheduling.reschedule, + FREIGHT_PERMS.trainScheduling.rulesManage, + FREIGHT_PERMS.fleet.view, + FREIGHT_PERMS.fleet.manage, + ...FLEET_GRANULAR_KEYS, + ]), ceo: dedupe([...ROLE_PERMISSION_PRESETS.ceo]), ethiopianGl: dedupe([...ROLE_PERMISSION_PRESETS.glEthiopia]), djiboutiGl: dedupe([...ROLE_PERMISSION_PRESETS.glDjibouti]), diff --git a/apps/edr-freight-web/backoffice/src/auth/types.ts b/apps/edr-freight-web/backoffice/src/auth/types.ts index a361c2abc..44fa4e595 100644 --- a/apps/edr-freight-web/backoffice/src/auth/types.ts +++ b/apps/edr-freight-web/backoffice/src/auth/types.ts @@ -23,7 +23,13 @@ interface AuthEmployeePosition { permissions?: AuthPermission[]; /** Some IAM payloads nest the position record instead of flattening its key. */ position?: { id?: string; key?: string; name?: LocaleText }; - positionType?: { id?: string; key?: string; name?: LocaleText } | null; + positionType?: { + id?: string; + key?: string; + name?: LocaleText; + /** Grants held by the TYPE — where admin-created positions keep theirs. */ + permissions?: AuthPermission[]; + } | null; } interface AuthEmployeeRecord { diff --git a/apps/edr-freight-web/backoffice/src/components/contracts/BookingChangesRequestedAlert.tsx b/apps/edr-freight-web/backoffice/src/components/contracts/BookingChangesRequestedAlert.tsx index 16b1ab602..f3b56a464 100644 --- a/apps/edr-freight-web/backoffice/src/components/contracts/BookingChangesRequestedAlert.tsx +++ b/apps/edr-freight-web/backoffice/src/components/contracts/BookingChangesRequestedAlert.tsx @@ -1,6 +1,6 @@ import { Alert, Button, Group, Paper, Stack, Text } from "@mantine/core"; import { DateInput } from "@mantine/dates"; -import { AlertTriangle, Send } from "lucide-react"; +import { AlertTriangle, Pencil, Send } from "lucide-react"; import { useState } from "react"; import { Link } from "react-router-dom"; import toast from "react-hot-toast"; @@ -16,6 +16,9 @@ export interface BookingChangesRequestedAlertProps { scheduledDate?: string | null; /** GL Ethiopia owns customs bookings, so only they get the resubmit control. */ canResubmit: boolean; + /** Completion-form route for editing the cargo before resubmitting — + * rendered only for resubmit-capable users when provided. */ + editHref?: string; onResubmitted?: () => void; } @@ -33,6 +36,7 @@ export function BookingChangesRequestedAlert({ note, scheduledDate, canResubmit, + editHref, onResubmitted, }: BookingChangesRequestedAlertProps) { const [day, setDay] = useState( @@ -122,6 +126,18 @@ export function BookingChangesRequestedAlert({ > Resubmit to Operations + {editHref ? ( + + ) : null} ) : null} diff --git a/apps/edr-freight-web/backoffice/src/lib/permissions.ts b/apps/edr-freight-web/backoffice/src/lib/permissions.ts index bc70c95a2..9c98fe4ab 100644 --- a/apps/edr-freight-web/backoffice/src/lib/permissions.ts +++ b/apps/edr-freight-web/backoffice/src/lib/permissions.ts @@ -342,6 +342,14 @@ export function getPermissionKeys(user: AuthUser | null | undefined): string[] { for (const p of pos.permissions ?? []) { if (p.key) keys.add(p.key); } + // Positions created through the admin UI keep their grants on the + // position TYPE, not the position — miss these and such staff resolve to + // zero permissions and every gated route rejects them. `/api/me` folds + // them into the position's permission list, but older payloads may still + // carry them separately. + for (const p of pos.positionType?.permissions ?? []) { + if (p.key) keys.add(p.key); + } } } return [...keys]; diff --git a/apps/edr-freight-web/backoffice/src/pages/contracts/ContractClearanceDetailPage.tsx b/apps/edr-freight-web/backoffice/src/pages/contracts/ContractClearanceDetailPage.tsx index 3b4d2edd9..b49eb419c 100644 --- a/apps/edr-freight-web/backoffice/src/pages/contracts/ContractClearanceDetailPage.tsx +++ b/apps/edr-freight-web/backoffice/src/pages/contracts/ContractClearanceDetailPage.tsx @@ -311,6 +311,7 @@ export default function ContractClearanceDetailPage() { note={clearance.linkedBookingReviewNote} scheduledDate={clearance.linkedBookingScheduledDate} canResubmit={canResubmitBooking} + editHref={`/dashboard/contracts/${id}/bookings/${linkedBookingId}/complete?copyFrom=${linkedBookingId}`} onResubmitted={() => { void refetch(); void refetchContract(); diff --git a/apps/edr-freight-web/portal/src/pages/bookings/BookingDetailPage/ChangesRequestedView.tsx b/apps/edr-freight-web/portal/src/pages/bookings/BookingDetailPage/ChangesRequestedView.tsx index 6b4c8927d..3cb3feb9e 100644 --- a/apps/edr-freight-web/portal/src/pages/bookings/BookingDetailPage/ChangesRequestedView.tsx +++ b/apps/edr-freight-web/portal/src/pages/bookings/BookingDetailPage/ChangesRequestedView.tsx @@ -8,7 +8,7 @@ import { TextInput, } from "@mantine/core"; import { useMutation, useQuery } from "@tanstack/react-query"; -import { AlertCircle, Send, XCircle } from "lucide-react"; +import { AlertCircle, Pencil, Send, XCircle } from "lucide-react"; import { useState } from "react"; import { useNavigate } from "react-router-dom"; @@ -86,7 +86,7 @@ export function ChangesRequestedView({ {booking.latestChangeRequestNote ? ( - + {booking.latestChangeRequestNote} ) : undefined} @@ -108,8 +108,25 @@ export function ChangesRequestedView({ Update the documents for this booking, then resubmit for review. Replace any that changed and attach any that are still required. + Need to change the cargo itself — containers, route, schedule or + other details? Edit the booking first, then come back and + resubmit. + + {flow.validationError && ( From 533de3cc93591a757cad846a0d6b94af2e925b1d Mon Sep 17 00:00:00 2001 From: Marshal Date: Thu, 6 Aug 2026 17:50:27 +0000 Subject: [PATCH 2/2] fix(cbe): update response code from 3 to 2 for business failures in CBE integration --- .../TrainScheduleV2DetailPage.tsx | 50 +++++++++++++++++-- .../modules/cbe-bill/cbe-bill.controller.ts | 2 +- .../src/modules/cbe-bill/cbe-bill.service.ts | 18 ++++++- .../modules/cbe-bill/cbe-exception.filter.ts | 4 +- .../cbe-bill/mappers/cbe-error.mapper.ts | 4 +- .../cbe-bill/mappers/cbe-payment.mapper.ts | 2 +- .../cbe-bill/mappers/cbe-query.mapper.ts | 2 +- integration/src/cbe-bill.it.ts | 35 ++++++++++--- 8 files changed, 98 insertions(+), 19 deletions(-) diff --git a/apps/edr-freight-web/backoffice/src/pages/trainScheduling/TrainScheduleV2DetailPage.tsx b/apps/edr-freight-web/backoffice/src/pages/trainScheduling/TrainScheduleV2DetailPage.tsx index ce6fe5353..d20ed83ef 100644 --- a/apps/edr-freight-web/backoffice/src/pages/trainScheduling/TrainScheduleV2DetailPage.tsx +++ b/apps/edr-freight-web/backoffice/src/pages/trainScheduling/TrainScheduleV2DetailPage.tsx @@ -916,17 +916,57 @@ export default function TrainScheduleV2DetailPage() { {schedule.route?.name ?? "Train schedule"} - {schedule.trainNumber ? ( - - {schedule.trainNumber} - - ) : null} {schedule.train ? ( Train {schedule.train.code} ) : null} + + {/* Voyage (train) number and trade direction — the two things + operations identify a run by, so they read at a glance + rather than as small badges among the rest. */} + + {schedule.trainNumber ? ( + + + Voyage No. + + + {schedule.trainNumber} + + + ) : null} + {schedule.direction ? ( + + + Direction + + + {schedule.direction} + + + ) : null} + {(schedule.stops?.length ?? 0) >= 3 || (schedule.bookings ?? []).some( (b) => b.tradeDirection === "DOMESTIC", diff --git a/apps/edr-payment-api/src/modules/cbe-bill/cbe-bill.controller.ts b/apps/edr-payment-api/src/modules/cbe-bill/cbe-bill.controller.ts index 46b8f9009..86f8f1475 100644 --- a/apps/edr-payment-api/src/modules/cbe-bill/cbe-bill.controller.ts +++ b/apps/edr-payment-api/src/modules/cbe-bill/cbe-bill.controller.ts @@ -22,7 +22,7 @@ import { CbePaymentResponseDto } from "./dto/cbe-payment-response.dto"; * CBE Unified Bill Payment — the INBOUND surface CBE core banking calls (docs/cbe/). We are * the biller: CBE authenticates against /cbe/oauth/token with credentials we issued, then * presents the bearer token on /cbe/query and /cbe/payment. Business failures answer HTTP 200 - * with Response_Code "3"; only authentication answers 401 (plan D6/D7). + * with Response_Code "2"; only authentication answers 401 (plan D6/D7). */ @ApiTags("CBE Unified Bill (inbound)") @Controller("cbe") diff --git a/apps/edr-payment-api/src/modules/cbe-bill/cbe-bill.service.ts b/apps/edr-payment-api/src/modules/cbe-bill/cbe-bill.service.ts index 478f60d0c..c56622da0 100644 --- a/apps/edr-payment-api/src/modules/cbe-bill/cbe-bill.service.ts +++ b/apps/edr-payment-api/src/modules/cbe-bill/cbe-bill.service.ts @@ -59,7 +59,7 @@ function localReason(intent: PaymentIntent): BillNotPayableReason { /** * Orchestration for CBE's three inbound calls (docs/cbe/CBE_IMPLEMENTATION_PLAN.md Phase 3). - * Business failures return HTTP 200 + Response_Code "3" envelopes (never throw past the + * Business failures return HTTP 200 + Response_Code "2" envelopes (never throw past the * controller); the exception filter only catches auth, validation, and the unexpected. */ @Injectable() @@ -184,6 +184,22 @@ export class CbeBillService { ); if (prior) { if (prior.tradeStatus === "SUCCESS") { + // A replay must be the SAME attempt. A reused id with different money details is + // not a retry — echoing the stored success would fake a settlement that never ran. + const orig = prior.requestPayload as unknown as + | CbePaymentRequestDto + | undefined; + if ( + orig && + (orig.Bill_Id !== dto.Bill_Id || + orig.Cbe_Txn_Ref !== dto.Cbe_Txn_Ref || + Number(orig.Amount) !== Number(dto.Amount)) + ) { + return mapPaymentFailure( + dto, + `End_To_End_Txn_Id ${dto.End_To_End_Txn_Id} was already used by a different payment.`, + ); + } // Replay the stored body verbatim. Never re-settle. return prior.responsePayload as unknown as CbePaymentResponseDto; } diff --git a/apps/edr-payment-api/src/modules/cbe-bill/cbe-exception.filter.ts b/apps/edr-payment-api/src/modules/cbe-bill/cbe-exception.filter.ts index dcbb5d82a..a5a7055b5 100644 --- a/apps/edr-payment-api/src/modules/cbe-bill/cbe-exception.filter.ts +++ b/apps/edr-payment-api/src/modules/cbe-bill/cbe-exception.filter.ts @@ -54,7 +54,7 @@ export class CbeExceptionFilter implements ExceptionFilter { : exception.message; response.status(HttpStatus.OK).json({ Status: "FAILED", - Response_Code: "3", + Response_Code: "2", Response_Description: message || "Invalid request", }); return; @@ -65,7 +65,7 @@ export class CbeExceptionFilter implements ExceptionFilter { ); response.status(HttpStatus.OK).json({ Status: "FAILED", - Response_Code: "3", + Response_Code: "2", Response_Description: "Internal server error.", }); } diff --git a/apps/edr-payment-api/src/modules/cbe-bill/mappers/cbe-error.mapper.ts b/apps/edr-payment-api/src/modules/cbe-bill/mappers/cbe-error.mapper.ts index 7cda33117..98b5e8825 100644 --- a/apps/edr-payment-api/src/modules/cbe-bill/mappers/cbe-error.mapper.ts +++ b/apps/edr-payment-api/src/modules/cbe-bill/mappers/cbe-error.mapper.ts @@ -16,8 +16,8 @@ export class CbeBillError extends Error { } /** - * Every failure maps to Response_Code "3" — the AAFDA spec (§2.10, §3.10) defines only - * 0 (success), 1 (auth), 3 (business); only the description is specific (plan §6.6). + * Every failure maps to Response_Code "2" (per current CBE integration requirement; the + * original AAFDA plan used 3); only the description is specific (plan §6.6). */ export function toCbeFailure(err: unknown): { description: string; diff --git a/apps/edr-payment-api/src/modules/cbe-bill/mappers/cbe-payment.mapper.ts b/apps/edr-payment-api/src/modules/cbe-bill/mappers/cbe-payment.mapper.ts index d90d94c91..8e6259930 100644 --- a/apps/edr-payment-api/src/modules/cbe-bill/mappers/cbe-payment.mapper.ts +++ b/apps/edr-payment-api/src/modules/cbe-bill/mappers/cbe-payment.mapper.ts @@ -27,7 +27,7 @@ export function mapPaymentFailure( Cbe_Txn_Ref: request.Cbe_Txn_Ref, Destination_Txn_Ref: "", Status: "FAILED", - Response_Code: "3", + Response_Code: "2", Response_Description: description, Additional_Fields: [], }; diff --git a/apps/edr-payment-api/src/modules/cbe-bill/mappers/cbe-query.mapper.ts b/apps/edr-payment-api/src/modules/cbe-bill/mappers/cbe-query.mapper.ts index 95caadc75..d2153b6a5 100644 --- a/apps/edr-payment-api/src/modules/cbe-bill/mappers/cbe-query.mapper.ts +++ b/apps/edr-payment-api/src/modules/cbe-bill/mappers/cbe-query.mapper.ts @@ -48,7 +48,7 @@ export function mapQueryFailure( Transaction_Type: "", Timestamp: new Date().toISOString(), Status: "FAILED", - Response_Code: "3", + Response_Code: "2", Response_Description: description, Additional_Fields: [], }; diff --git a/integration/src/cbe-bill.it.ts b/integration/src/cbe-bill.it.ts index 974d00b07..a75e897a4 100644 --- a/integration/src/cbe-bill.it.ts +++ b/integration/src/cbe-bill.it.ts @@ -122,15 +122,18 @@ describe("CBE Unified Bill (payment service as biller)", () => { End_To_End_Txn_Id: txnId("q404"), Bill_Id: "000000000000", }); - // Business failures are HTTP 200 + Response_Code "3" — CBE treats a non-200 + // Business failures are HTTP 200 + Response_Code "2" — CBE treats a non-200 // as a channel fault and retries. expect(res.status).toBe(200); - expect(res.body.Response_Code).toBe("3"); + expect(res.body.Response_Code).toBe("2"); }); + let settleBody: Record; + let settledTxnRef: string; + it("settles the freight invoice when CBE reports the debit", async () => { const invoice = await currentInvoice(invoiceId); - const res = await cbe(token, "/cbe/payment", { + settleBody = { Destination_Api_Name: API_NAME, End_To_End_Txn_Id: txnId("p1"), Cbe_Txn_Ref: `CBE${Date.now()}`, @@ -140,9 +143,11 @@ describe("CBE Unified Bill (payment service as biller)", () => { Currency: "ETB", Full_Name: "IT Payer", Phone_No: "+251911000001", - }); + }; + const res = await cbe(token, "/cbe/payment", settleBody); expect(res.status).toBe(200); expect(res.body.Response_Code).toBe("0"); + settledTxnRef = res.body.Destination_Txn_Ref; const paid = await poll<{ status: string }>( "invoice PAID via CBE bill", @@ -154,6 +159,24 @@ describe("CBE Unified Bill (payment service as biller)", () => { expect(paid.status).toBe("PAID"); }); + it("replays the stored success when CBE retries the same attempt verbatim", async () => { + const res = await cbe(token, "/cbe/payment", settleBody); + expect(res.status).toBe(200); + expect(res.body.Response_Code).toBe("0"); + // The stored body, not a re-settlement — same order id as the first answer. + expect(res.body.Destination_Txn_Ref).toBe(settledTxnRef); + }); + + it("rejects a settled End_To_End_Txn_Id reused with a different amount", async () => { + const res = await cbe(token, "/cbe/payment", { + ...settleBody, + Amount: "1.00", + }); + expect(res.status).toBe(200); + expect(res.body.Response_Code).toBe("2"); + expect(res.body.Response_Description).toContain("already used by a different payment"); + }); + it("rejects a second debit on the same bill", async () => { const res = await cbe(token, "/cbe/payment", { Destination_Api_Name: API_NAME, @@ -165,7 +188,7 @@ describe("CBE Unified Bill (payment service as biller)", () => { Currency: "ETB", }); expect(res.status).toBe(200); - expect(res.body.Response_Code).toBe("3"); + expect(res.body.Response_Code).toBe("2"); }); it("reports an already-paid bill on a later query", async () => { @@ -175,6 +198,6 @@ describe("CBE Unified Bill (payment service as biller)", () => { Bill_Id: billId, }); expect(res.status).toBe(200); - expect(res.body.Response_Code).toBe("3"); + expect(res.body.Response_Code).toBe("2"); }); });