From 01ea05f01314d191daa468b353cf0795ba69cf28 Mon Sep 17 00:00:00 2001 From: Nathnael Date: Tue, 25 Aug 2026 12:00:43 +0000 Subject: [PATCH 1/4] fix(auth): keep secondary positions in permission checks IAM lets an employee hold several positions, but the vendored JwtGuard collapses employee.positions[] down to a single employee.position and drops the rest. Non-delegate secondary positions vanished entirely, so staff on two posts resolved to one post's permissions and every check on the other rejected them. FreightJwtGuard re-attaches the full list from the same session snapshot the parent guard already read, so nothing extra is fetched per request beyond a cached session lookup. employee.position is left untouched, keeping audit logging and delegation unaffected. collectPermissionKeys and collectPositionTypeKeys now union across every position, and /me returns them all. Verified against a real two-position user (djibouti-gl-director + djibouti-gl-chief) on the local dev database: /me positions 1 -> 2 /me permissionKeys 17 -> 28 GET /api/interchange-documents 403 -> 200 GET /api/trains 403 -> 200 11 permissions recovered, none lost. Six single-position users return byte-identical payloads before and after. --- .../src/common/booking-guards.ts | 10 +- .../src/common/freight-jwt.guard.ts | 101 ++++++++++++++++++ .../common/freight-permission.util.spec.ts | 64 +++++++++++ .../src/common/freight-permission.util.ts | 26 ++++- .../src/common/rule-engine-guards.ts | 12 +-- .../src/modules/auth/account.controller.ts | 4 +- .../src/modules/auth/freight-me.controller.ts | 4 +- .../src/modules/auth/freight-me.service.ts | 95 +++++++++------- .../src/modules/chat/chat.controller.ts | 4 +- .../src/modules/exports/exports.controller.ts | 4 +- .../notification-inbox.controller.ts | 4 +- .../modules/verifayda/verifayda.controller.ts | 4 +- 12 files changed, 265 insertions(+), 67 deletions(-) create mode 100644 apps/edr-freight-api/src/common/freight-jwt.guard.ts diff --git a/apps/edr-freight-api/src/common/booking-guards.ts b/apps/edr-freight-api/src/common/booking-guards.ts index 68bbdcba4..603811178 100644 --- a/apps/edr-freight-api/src/common/booking-guards.ts +++ b/apps/edr-freight-api/src/common/booking-guards.ts @@ -1,5 +1,5 @@ import { applyDecorators, UseGuards } from '@nestjs/common'; -import { JwtGuard } from '@tria-plc/api-common/modules/auth/services/jwt.guard'; +import { FreightJwtGuard } from './freight-jwt.guard'; import { FreightPermissionGuard, @@ -11,7 +11,7 @@ import { FREIGHT_PERMS } from '../seed/freight-permissions.registry'; export const BookingStaff = (permission: string | string[]) => applyDecorators( UseGuards( - JwtGuard, + FreightJwtGuard, FreightPermissionGuard( Array.isArray(permission) ? permission : [permission], ), @@ -26,11 +26,11 @@ export const BookingStaff = (permission: string | string[]) => * BookingStaff() or MixedAudience(); kept for routes not yet swept. */ export const StaffReference = () => - applyDecorators(UseGuards(JwtGuard, FreightPermissionGuard([]))); + applyDecorators(UseGuards(FreightJwtGuard, FreightPermissionGuard([]))); /** Portal routes: customer accounts only; ownership scoping stays in services. */ export const PortalCustomer = () => - applyDecorators(UseGuards(JwtGuard, PortalCustomerGuard)); + applyDecorators(UseGuards(FreightJwtGuard, PortalCustomerGuard)); /** * Routes both audiences call (sign, shared document reads, handover): staff @@ -40,7 +40,7 @@ export const PortalCustomer = () => export const MixedAudience = (permission: string | string[]) => applyDecorators( UseGuards( - JwtGuard, + FreightJwtGuard, MixedAudienceGuard( Array.isArray(permission) ? permission : [permission], ), diff --git a/apps/edr-freight-api/src/common/freight-jwt.guard.ts b/apps/edr-freight-api/src/common/freight-jwt.guard.ts new file mode 100644 index 000000000..68b842217 --- /dev/null +++ b/apps/edr-freight-api/src/common/freight-jwt.guard.ts @@ -0,0 +1,101 @@ +import { CanActivate, ExecutionContext, Injectable } from '@nestjs/common'; +import { Reflector } from '@nestjs/core'; +import { InjectDataSource } from '@nestjs/typeorm'; +import { JwtGuard as IamJwtGuard } from '@tria-plc/api-common/modules/auth/services/jwt.guard'; +import type { TCurrentUser } from '@tria-plc/api-common/modules/auth/types/current-user.type'; +import { DataSource } from 'typeorm'; + +/** One position as the login snapshot stores it (`iam.sessions.userInfo`). */ +type SnapshotPosition = { id?: string; [key: string]: unknown }; + +type SessionUserInfo = { + employee?: { id?: string; positions?: SnapshotPosition[] }[]; +}; + +/** + * Like the IAM JwtGuard, but keeps the caller's SECONDARY positions. + * + * IAM models an employee as holding many positions, and the login snapshot in + * `iam.sessions.userInfo` carries all of them. `JwtGuard.parseToken` then + * collapses that to a single `employee.position` — whichever the request + * headers select, else `positions[0]` — and drops the rest. Non-delegate + * secondary positions vanish entirely, so staff holding two posts resolve to + * only one post's permissions and every check on the other one rejects them. + * + * This re-attaches the full list as `employee.positions`. `employee.position` + * is left exactly as the parent set it, so everything reading the single + * position today (audit log, delegation deadline) is unaffected; only the + * permission utils, which prefer the array, see the difference. + */ +@Injectable() +export class FreightJwtGuard extends IamJwtGuard implements CanActivate { + // ponytail: unbounded-until-TTL map, cleared wholesale when it gets big. + // Sessions are few and the value is small; swap for an LRU if that changes. + private static readonly CACHE_TTL_MS = 30_000; + private static readonly CACHE_MAX_ENTRIES = 5_000; + private readonly cache = new Map< + string, + { positions: SnapshotPosition[]; expiresAt: number } + >(); + + constructor( + reflector: Reflector, + @InjectDataSource() private readonly ds: DataSource, + ) { + super(reflector, ds); + } + + async canActivate(context: ExecutionContext): Promise { + if (!(await super.canActivate(context))) return false; + + const user = context.switchToHttp().getRequest().user as + | TCurrentUser + | undefined; + const employee = user?.employee; + if (!employee || !user?.sessionId) return true; + + const positions = await this.positionsForSession( + user.sessionId, + employee.id, + ); + // Never blank out what the parent resolved: an unreadable session or a + // snapshot without positions must degrade to the single-position + // behaviour, not to no positions at all. + if (positions.length) { + (employee as { positions?: SnapshotPosition[] }).positions = positions; + } + return true; + } + + /** Every position the login snapshot holds for this employee. */ + private async positionsForSession( + sessionId: string, + employeeId: string | undefined, + ): Promise { + const now = Date.now(); + const hit = this.cache.get(sessionId); + if (hit && hit.expiresAt > now) return hit.positions; + + let positions: SnapshotPosition[] = []; + try { + const rows: { userInfo: SessionUserInfo | null }[] = await this.ds.query( + `SELECT "userInfo" FROM iam.sessions WHERE id = $1`, + [sessionId], + ); + const employees = rows[0]?.userInfo?.employee ?? []; + const match = + employees.find((e) => e?.id && e.id === employeeId) ?? employees[0]; + positions = match?.positions ?? []; + } catch { + return []; // iam unreachable — caller keeps the parent's single position + } + + if (this.cache.size >= FreightJwtGuard.CACHE_MAX_ENTRIES) + this.cache.clear(); + this.cache.set(sessionId, { + positions, + expiresAt: now + FreightJwtGuard.CACHE_TTL_MS, + }); + return positions; + } +} 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 f3931f78a..562b21156 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 @@ -2,6 +2,7 @@ import { assertCanApproveContractStep, canEditContractStep, collectPermissionKeys, + collectPositionTypeKeys, hasFreightPermission, setPositionTypePermissionResolver, } from './freight-permission.util'; @@ -121,3 +122,66 @@ describe('collectPermissionKeys — position-type grants', () => { expect(hasFreightPermission(direct, CLEARANCE)).toBe(true); }); }); + +/** + * IAM lets an employee hold several positions, but the vendored `JwtGuard` + * collapses `employee.positions[]` down to a single `employee.position` and + * drops the rest — so staff on two posts resolved to one post's permissions + * and every check on the other rejected them. `FreightJwtGuard` restores the + * full list as `employee.positions`; these cover the union that depends on it. + */ +describe('multiple positions', () => { + // Shaped like the real two-post employee: GL chief AND GL director. + const twoPost = { + employee: { + // What the vendored guard leaves behind — one of the two, arbitrarily. + position: { + positionType: { key: 'djibouti-gl-chief' }, + permissions: [{ key: FREIGHT_PERMS.contracts.view }], + }, + // What FreightJwtGuard puts back. + positions: [ + { + positionType: { key: 'djibouti-gl-chief' }, + permissions: [{ key: FREIGHT_PERMS.contracts.view }], + }, + { + positionType: { key: 'djibouti-gl-director' }, + permissions: [{ key: FREIGHT_PERMS.bookings.view }], + }, + ], + }, + }; + + it('unions permissions across every position', () => { + const keys = collectPermissionKeys(twoPost); + expect(keys).toContain(FREIGHT_PERMS.contracts.view); + expect(keys).toContain(FREIGHT_PERMS.bookings.view); + }); + + it('grants the secondary position’s permission, not just the first', () => { + expect(hasFreightPermission(twoPost, FREIGHT_PERMS.bookings.view)).toBe(true); + }); + + it('answers to both position types', () => { + expect(collectPositionTypeKeys(twoPost)).toEqual( + expect.arrayContaining(['djibouti-gl-chief', 'djibouti-gl-director']), + ); + }); + + it('does not double-count the position the guard also left singular', () => { + const keys = collectPermissionKeys(twoPost); + expect(keys.filter((k) => k === FREIGHT_PERMS.contracts.view)).toHaveLength(1); + }); + + it('still resolves the single position when the array is absent', () => { + // A request that skipped FreightJwtGuard must degrade to the old behaviour, + // not to no permissions at all. + const onePost = { + employee: { + position: { permissions: [{ key: FREIGHT_PERMS.contracts.view }] }, + }, + }; + expect(hasFreightPermission(onePost, FREIGHT_PERMS.contracts.view)).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 bc98a6df4..1078c5ad9 100644 --- a/apps/edr-freight-api/src/common/freight-permission.util.ts +++ b/apps/edr-freight-api/src/common/freight-permission.util.ts @@ -17,6 +17,15 @@ type MeLikeUser = { permissions?: PermissionLike[]; positionType?: PositionTypeLike | null; }; + /** + * Every position the employee holds, restored by `FreightJwtGuard` + * from the login snapshot. The IAM guard only ever sets the singular + * `position` above; without this, a second post's grants are invisible. + */ + positions?: { + permissions?: PermissionLike[]; + positionType?: PositionTypeLike | null; + }[]; delegatedPositions?: { permissions?: PermissionLike[] }[]; } | { @@ -98,10 +107,15 @@ export function collectPermissionKeys(user: MeLikeUser | null | undefined): stri return [...keys]; } - for (const p of employee.position?.permissions ?? []) { - if (p.key) keys.add(p.key); + // `position` is whichever single post the IAM guard selected; `positions` is + // the full set FreightJwtGuard restores. Walk both — the array is absent on + // a session the guard could not re-read, and the two overlap harmlessly. + for (const pos of [employee.position, ...(employee.positions ?? [])]) { + for (const p of pos?.permissions ?? []) { + if (p.key) keys.add(p.key); + } + addTypePermissions(pos?.positionType); } - addTypePermissions(employee.position?.positionType); for (const delegated of employee.delegatedPositions ?? []) { for (const p of delegated.permissions ?? []) { if (p.key) keys.add(p.key); @@ -158,8 +172,10 @@ export function collectPositionTypeKeys( return [...keys]; } - if (employee.position?.positionType?.key) { - keys.add(employee.position.positionType.key); + // Both shapes, same reason as collectPermissionKeys: an employee holding two + // posts answers to both their position types. + for (const pos of [employee.position, ...(employee.positions ?? [])]) { + if (pos?.positionType?.key) keys.add(pos.positionType.key); } return [...keys]; } diff --git a/apps/edr-freight-api/src/common/rule-engine-guards.ts b/apps/edr-freight-api/src/common/rule-engine-guards.ts index ba8096b5b..d7297d3b5 100644 --- a/apps/edr-freight-api/src/common/rule-engine-guards.ts +++ b/apps/edr-freight-api/src/common/rule-engine-guards.ts @@ -1,5 +1,5 @@ import { applyDecorators, UseGuards } from '@nestjs/common'; -import { JwtGuard } from '@tria-plc/api-common/modules/auth/services/jwt.guard'; +import { FreightJwtGuard } from './freight-jwt.guard'; import { FreightPermissionGuard } from './freight-permission.guard'; import { @@ -10,7 +10,7 @@ import { export const RuleEngineView = (slug: RuleEngineResourceSlug) => applyDecorators( - UseGuards(JwtGuard, FreightPermissionGuard([FREIGHT_PERMS.ruleEngine.view(slug)])), + UseGuards(FreightJwtGuard, FreightPermissionGuard([FREIGHT_PERMS.ruleEngine.view(slug)])), ); // Granular CRUD replaces the retired coarse RuleEngineManage. Each write @@ -18,17 +18,17 @@ export const RuleEngineView = (slug: RuleEngineResourceSlug) => // update on PATCH / reorder / move-order, delete on DELETE. export const RuleEngineCreate = (slug: RuleEngineResourceSlug) => applyDecorators( - UseGuards(JwtGuard, FreightPermissionGuard([FREIGHT_PERMS.ruleEngine.create(slug)])), + UseGuards(FreightJwtGuard, FreightPermissionGuard([FREIGHT_PERMS.ruleEngine.create(slug)])), ); export const RuleEngineUpdate = (slug: RuleEngineResourceSlug) => applyDecorators( - UseGuards(JwtGuard, FreightPermissionGuard([FREIGHT_PERMS.ruleEngine.update(slug)])), + UseGuards(FreightJwtGuard, FreightPermissionGuard([FREIGHT_PERMS.ruleEngine.update(slug)])), ); export const RuleEngineDelete = (slug: RuleEngineResourceSlug) => applyDecorators( - UseGuards(JwtGuard, FreightPermissionGuard([FREIGHT_PERMS.ruleEngine.delete(slug)])), + UseGuards(FreightJwtGuard, FreightPermissionGuard([FREIGHT_PERMS.ruleEngine.delete(slug)])), ); /** @@ -38,5 +38,5 @@ export const RuleEngineDelete = (slug: RuleEngineResourceSlug) => */ export const RuleEngineApprove = (slug: RuleEngineApprovableSlug) => applyDecorators( - UseGuards(JwtGuard, FreightPermissionGuard([FREIGHT_PERMS.ruleEngine.approve(slug)])), + UseGuards(FreightJwtGuard, FreightPermissionGuard([FREIGHT_PERMS.ruleEngine.approve(slug)])), ); diff --git a/apps/edr-freight-api/src/modules/auth/account.controller.ts b/apps/edr-freight-api/src/modules/auth/account.controller.ts index d7d7f15c3..96f90e8a3 100644 --- a/apps/edr-freight-api/src/modules/auth/account.controller.ts +++ b/apps/edr-freight-api/src/modules/auth/account.controller.ts @@ -1,7 +1,7 @@ import { Body, Controller, Patch, Post, UseGuards } from "@nestjs/common"; import { ApiBearerAuth, ApiOperation, ApiTags } from "@nestjs/swagger"; import { CurrentUser } from "@tria-plc/api-common/modules/auth/decorators/current-user.decorator"; -import { JwtGuard } from "@tria-plc/api-common/modules/auth/services/jwt.guard"; +import { FreightJwtGuard } from "../../common/freight-jwt.guard"; import type { TCurrentUser } from "@tria-plc/api-common/modules/auth/types/current-user.type"; import { AccountService } from "./account.service"; @@ -19,7 +19,7 @@ import { @ApiTags("auth") @Controller("me") @ApiBearerAuth() -@UseGuards(JwtGuard) +@UseGuards(FreightJwtGuard) export class AccountController { constructor(private readonly accountService: AccountService) {} diff --git a/apps/edr-freight-api/src/modules/auth/freight-me.controller.ts b/apps/edr-freight-api/src/modules/auth/freight-me.controller.ts index b85ecea84..f1c974687 100644 --- a/apps/edr-freight-api/src/modules/auth/freight-me.controller.ts +++ b/apps/edr-freight-api/src/modules/auth/freight-me.controller.ts @@ -1,7 +1,7 @@ import { Controller, Get, UseGuards } from '@nestjs/common'; import { ApiBearerAuth, ApiOperation, ApiTags } from '@nestjs/swagger'; import { CurrentUser } from '@tria-plc/api-common/modules/auth/decorators/current-user.decorator'; -import { JwtGuard } from '@tria-plc/api-common/modules/auth/services/jwt.guard'; +import { FreightJwtGuard } from '../../common/freight-jwt.guard'; import type { TCurrentUser } from '@tria-plc/api-common/modules/auth/types/current-user.type'; import { FreightMeService } from './freight-me.service'; @@ -13,7 +13,7 @@ export class FreightMeController { constructor(private readonly freightMeService: FreightMeService) {} @Get() - @UseGuards(JwtGuard) + @UseGuards(FreightJwtGuard) @ApiOperation({ summary: 'Current user with flat permissionKeys for backoffice gating', }) 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 6bcfd964d..b897c5fef 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 @@ -9,6 +9,9 @@ import { } from '../../common/freight-permission.util'; import { PERMISSIONS_CATALOG } from '../../seed/freight-permissions.registry'; +/** One position as the session snapshot carries it. */ +type TokenPosition = NonNullable['position']; + @Injectable() export class FreightMeService { constructor(@InjectDataSource() private readonly dataSource: DataSource) {} @@ -65,49 +68,63 @@ export class FreightMeService { } async getEnrichedProfile(user: TCurrentUser) { - const positionId = user.employee?.position?.id; - const [positionType, positionTypePermissionKeys] = await Promise.all([ - this.lookupPositionType(positionId), - this.lookupPositionTypePermissions(positionId), - ]); + const employeeRecord = user.employee as + | (typeof user.employee & { positions?: TokenPosition[] }) + | undefined; - // 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), + // `FreightJwtGuard` restores every position the login snapshot holds; the + // stock IAM guard only ever leaves the single `position`. Fall back to it + // so a request that somehow skipped our guard still resolves one post + // rather than none. + const rawPositions: TokenPosition[] = employeeRecord?.positions?.length + ? employeeRecord.positions + : employeeRecord?.position + ? [employeeRecord.position] + : []; + + const enrichedPositions = await Promise.all( + rawPositions.map(async (position) => { + const [positionType, positionTypePermissionKeys] = await Promise.all([ + this.lookupPositionType(position.id), + this.lookupPositionTypePermissions(position.id), + ]); + + // Merge the type-level grants into this position's own permission list + // so BOTH consumers see them: `collectPermissionKeys` below, and the + // backoffice's `getPermissionKeys`, which walks this nested array. + const permissions = [...(position.permissions ?? [])]; + const seen = new Set(permissions.map((p) => p?.key).filter(Boolean)); + for (const key of positionTypePermissionKeys) { + if (!seen.has(key)) { + seen.add(key); + permissions.push({ key } as (typeof permissions)[number]); + } + } + + return { + positionTypePermissionKeys, + position: { + id: position.id, + key: position.key, + employeePositionId: position.employeePositionId, + name: position.name, + isDelegate: position.isDelegate, + parentPositionId: position.parentPositionId, + permissions, + positionType, + }, + }; + }), ); - for (const key of positionTypePermissionKeys) { - if (!seenPermissionKeys.has(key)) { - seenPermissionKeys.add(key); - positionPermissions.push({ key } as (typeof positionPermissions)[number]); - } - } - const employee = user.employee + const employee = employeeRecord ? [ { - id: user.employee.id, - organizationId: user.employee.organizationId, - unitId: user.employee.unitId, - name: user.employee.name, - positions: user.employee.position - ? [ - { - id: user.employee.position.id, - key: user.employee.position.key, - employeePositionId: user.employee.position.employeePositionId, - name: user.employee.position.name, - isDelegate: user.employee.position.isDelegate, - parentPositionId: user.employee.position.parentPositionId, - permissions: positionPermissions, - positionType, - }, - ] - : [], + id: employeeRecord.id, + organizationId: employeeRecord.organizationId, + unitId: employeeRecord.unitId, + name: employeeRecord.name, + positions: enrichedPositions.map((p) => p.position), }, ] : []; @@ -118,7 +135,7 @@ export class FreightMeService { const permissionKeys = [ ...new Set([ ...collectPermissionKeys(user), - ...positionTypePermissionKeys, + ...enrichedPositions.flatMap((p) => p.positionTypePermissionKeys), ]), ]; diff --git a/apps/edr-freight-api/src/modules/chat/chat.controller.ts b/apps/edr-freight-api/src/modules/chat/chat.controller.ts index 0ecca04c0..5e122bdd2 100644 --- a/apps/edr-freight-api/src/modules/chat/chat.controller.ts +++ b/apps/edr-freight-api/src/modules/chat/chat.controller.ts @@ -1,7 +1,7 @@ import { Controller, Get, Post, UseGuards } from '@nestjs/common'; import { ApiBearerAuth, ApiOperation, ApiTags } from '@nestjs/swagger'; import { CurrentUser } from '@tria-plc/api-common/modules/auth/decorators/current-user.decorator'; -import { JwtGuard } from '@tria-plc/api-common/modules/auth/services/jwt.guard'; +import { FreightJwtGuard } from '../../common/freight-jwt.guard'; import type { TCurrentUser } from '@tria-plc/api-common/modules/auth/types/current-user.type'; import { ChatSync } from '../../common/booking-guards'; @@ -18,7 +18,7 @@ export class ChatController { ) {} @Get('sso') - @UseGuards(JwtGuard) + @UseGuards(FreightJwtGuard) @ApiOperation({ summary: 'One-click sign-in link into EDR internal chat' }) getSso(@CurrentUser() user: TCurrentUser) { return this.sso.getSsoUrl(user); diff --git a/apps/edr-freight-api/src/modules/exports/exports.controller.ts b/apps/edr-freight-api/src/modules/exports/exports.controller.ts index 89ca7419a..aea35a9bc 100644 --- a/apps/edr-freight-api/src/modules/exports/exports.controller.ts +++ b/apps/edr-freight-api/src/modules/exports/exports.controller.ts @@ -3,7 +3,7 @@ import { ApiBearerAuth, ApiOperation, ApiTags } from '@nestjs/swagger'; import { InjectDataSource } from '@nestjs/typeorm'; import { DataSource } from 'typeorm'; import { CurrentUser } from '@edr/api-common'; -import { JwtGuard } from '@tria-plc/api-common/modules/auth/services/jwt.guard'; +import { FreightJwtGuard } from '../../common/freight-jwt.guard'; import type { Response } from 'express'; import type { TCurrentUser } from '@tria-plc/api-common/modules/auth/types/current-user.type'; @@ -57,7 +57,7 @@ const toCatalogEntry = (dataset: ExportDataset): ExportCatalogEntry => ({ @ApiTags('Exports') @ApiBearerAuth() @Controller('exports') -@UseGuards(JwtGuard) +@UseGuards(FreightJwtGuard) export class ExportsController { constructor( private readonly runner: ExportRunnerService, diff --git a/apps/edr-freight-api/src/modules/notification-inbox/notification-inbox.controller.ts b/apps/edr-freight-api/src/modules/notification-inbox/notification-inbox.controller.ts index 6fb82c3a0..16026cc96 100644 --- a/apps/edr-freight-api/src/modules/notification-inbox/notification-inbox.controller.ts +++ b/apps/edr-freight-api/src/modules/notification-inbox/notification-inbox.controller.ts @@ -10,7 +10,7 @@ import { UseGuards, } from "@nestjs/common"; import { ApiBearerAuth, ApiOperation, ApiTags } from "@nestjs/swagger"; -import { JwtGuard } from "@tria-plc/api-common/modules/auth/services/jwt.guard"; +import { FreightJwtGuard } from "../../common/freight-jwt.guard"; import { AuthUserPayload, @@ -21,7 +21,7 @@ import { NotificationInboxService } from "./notification-inbox.service"; @ApiTags("notifications") @ApiBearerAuth() -@UseGuards(JwtGuard) +@UseGuards(FreightJwtGuard) @Controller("notifications") export class NotificationInboxController { constructor(private readonly service: NotificationInboxService) {} diff --git a/apps/edr-freight-api/src/modules/verifayda/verifayda.controller.ts b/apps/edr-freight-api/src/modules/verifayda/verifayda.controller.ts index 977e677f8..3a94bb05a 100644 --- a/apps/edr-freight-api/src/modules/verifayda/verifayda.controller.ts +++ b/apps/edr-freight-api/src/modules/verifayda/verifayda.controller.ts @@ -17,7 +17,7 @@ import { } from '@nestjs/swagger'; import type { TCurrentUser } from '@tria-plc/api-common/modules/auth/types/current-user.type'; import { IsPublic } from '@tria-plc/api-common/modules/auth/decorators/public.decorator'; -import { JwtGuard } from '@tria-plc/api-common/modules/auth/services/jwt.guard'; +import { FreightJwtGuard } from '../../common/freight-jwt.guard'; import { OptionalJwtGuard } from './optional-jwt.guard'; import { CompleteVerificationResultDto, @@ -91,7 +91,7 @@ export class VerifaydaController { } @Get('status') - @UseGuards(JwtGuard) + @UseGuards(FreightJwtGuard) @ApiBearerAuth('JWT-auth') @ApiOperation({ summary: "Get the current user's Fayda verification status", From 6d1e4a630e57de24137e2af5efb22d28ceee0acc Mon Sep 17 00:00:00 2001 From: Nathnael Date: Tue, 25 Aug 2026 13:01:47 +0000 Subject: [PATCH 2/4] feat(reports): publish per-stop times when loading & unloading groups by train MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Grouped by Train, a row was a period average over the train's stops, which threw away the one thing that grain is for: when the work actually happened. The row is now the stop itself — logged arrival, departure, unloading and loading times, and that stop's own durations. Grouped by Station it still averages over every train that called there. The two shapes need different columns, so `ReportColumn.hideWhen` names the filter values that hide a column, and the runner resolves sort against the visible set — sorting by an average and then switching to Train falls back to the default sort instead of a 42703 on a column the query no longer selects. `ReportFilterDef.defaultValue` pins the unset grain to `train`, so "no value" is never a third shape. The table, the export field list and the chart toggle all follow the visible set; the chart is station-grain only, since per-stop rows have nothing to bar-chart. Both grains EXPLAIN-validated and run against the dev database. Co-Authored-By: Claude Opus 5 (1M context) --- .../loading-unloading.report.spec.ts | 30 ++ .../definitions/loading-unloading.report.ts | 310 ++++++++++++++---- .../modules/reports/report-runner.service.ts | 27 +- .../src/modules/reports/report.types.ts | 12 + .../components/reports/ReportExportButton.tsx | 20 +- .../src/components/reports/ReportView.tsx | 32 +- .../backoffice/src/types/reports.ts | 11 +- 7 files changed, 364 insertions(+), 78 deletions(-) create mode 100644 apps/edr-freight-api/src/modules/reports/definitions/loading-unloading.report.spec.ts diff --git a/apps/edr-freight-api/src/modules/reports/definitions/loading-unloading.report.spec.ts b/apps/edr-freight-api/src/modules/reports/definitions/loading-unloading.report.spec.ts new file mode 100644 index 000000000..b5b72a06b --- /dev/null +++ b/apps/edr-freight-api/src/modules/reports/definitions/loading-unloading.report.spec.ts @@ -0,0 +1,30 @@ +import { visibleColumns } from '../report-runner.service'; +import { loadingUnloadingReport as def } from './loading-unloading.report'; + +/** + * The two grains select different columns — per train the stop's own times, + * per station the averages over it. A column shown under a grain its query + * doesn't select is a blank column; a column SORTED under one is a 42703. + */ +describe('loading-unloading', () => { + const keys = (grain: string) => visibleColumns(def, { grain }).map((c) => c.key); + + it('shows the stop times per train and the averages per station, never both', () => { + expect(keys('train')).toEqual(expect.arrayContaining(['arrivedAt', 'loadUnloadHours'])); + expect(keys('train')).not.toEqual(expect.arrayContaining(['avgLoadUnloadHours', 'stops'])); + expect(keys('station')).toEqual(expect.arrayContaining(['avgLoadUnloadHours', 'stops'])); + expect(keys('station')).not.toEqual(expect.arrayContaining(['arrivedAt', 'trainNumber'])); + }); + + it('sorts by a column both grains select, so the default sort never 42703s', () => { + for (const grain of ['train', 'station']) { + expect(keys(grain)).toContain(def.defaultSort!.key); + } + }); + + it("defaults the grain, so an unset filter can't show the wrong half", () => { + const grain = def.filters.find((f) => f.key === 'grain')!.defaultValue; + expect(grain).toBe('train'); + expect(keys(grain!)).toEqual(keys('train')); + }); +}); diff --git a/apps/edr-freight-api/src/modules/reports/definitions/loading-unloading.report.ts b/apps/edr-freight-api/src/modules/reports/definitions/loading-unloading.report.ts index 753f71d3a..aa7564b39 100644 --- a/apps/edr-freight-api/src/modules/reports/definitions/loading-unloading.report.ts +++ b/apps/edr-freight-api/src/modules/reports/definitions/loading-unloading.report.ts @@ -6,8 +6,10 @@ import { cycleRateExpr, handlingHours, hoursBetween, + loadingEnd, loadingHours, loadingSource, + loadingStart, otherActivityHours, stationStaysQb, unloadingHours, @@ -18,9 +20,12 @@ import { PERIOD_FILTER, periodExprOn, periodTruncExprOn } from '../revenue-class * Loading and unloading per train — the spec's own report format: train number, * total loading and unloading time, other activity, station staying time. * - * The staying-time report publishes one row per individual stop; this one rolls - * a train's stops up into the chosen period, which is what "for week report, - * calculate average in the week" asks for. The station stays in the grain + * Two shapes, one definition. Per train the row is the stop itself: the logged + * arrival, departure, unloading and loading times and that stop's own + * durations, because the train number is what makes a specific stop worth + * naming. Per station it rolls up into the chosen period — one row per station, + * averaged over every train that called there, which is what "for week report, + * calculate average in the week" asks for. The station stays in both grains * because a train works both ends of the corridor and the standard it is judged * against differs by side (10h Ethiopia, 13h Djibouti) — averaging a train's * Nagad and Gelan stops together would compare that mixture to one standard. @@ -56,12 +61,20 @@ const GRAIN_FILTER: ReportFilterDef = { key: 'grain', label: 'Group by', type: 'select', + defaultValue: 'train', options: [ { value: 'train', label: 'Train' }, { value: 'station', label: 'Station' }, ], }; +/** Per train the rows are stops, so they carry times; per station, averages. */ +const TRAIN_ONLY = { grain: 'station' }; +const STATION_ONLY = { grain: 'train' }; + +/** Same display as the staying-time report, so a stop reads alike in both. */ +const at = (expr: string): string => `to_char(${expr}, 'YYYY-MM-DD HH24:MI')`; + /** Whitelisted here, so the user's value never reaches SQL. */ const byStation = (ctx: ReportContext): boolean => ctx.params.grain === 'station'; @@ -69,9 +82,10 @@ export const loadingUnloadingReport: ReportDefinition = { key: 'loading-unloading', title: 'Loading & Unloading', description: - 'Loading and unloading per train, at the granularity you choose — one row per train per ' + - 'station per period, which at week or month grain is that train’s average over its stops ' + - 'in the period, the way the OCC report publishes it. Total loading and unloading is ' + + 'Loading and unloading, at the granularity you choose. Grouped by Train the row is one ' + + 'stop — its logged arrival, departure, unloading and loading times and that stop’s own ' + + 'durations. Grouped by Station it is one row per station per period, averaged over every ' + + 'train that called there, the way the OCC report publishes it. Total loading and unloading is ' + 'the stop’s handling window, unloading start to loading end, which is the container ' + 'measure; the unloading and loading columns split it for bulk stations that only do ' + 'one of the two (Nagad, BCC and DMP on the Djibouti side; Sebeta, GMP, Adama and Modjo ' + @@ -95,74 +109,241 @@ export const loadingUnloadingReport: ReportDefinition = { ], columns: [ { key: 'period', label: 'Period', type: 'string', sortable: true }, - { key: 'trainNumber', label: 'Train No.', type: 'string', sortable: true }, + { + key: 'trainNumber', + label: 'Train No.', + type: 'string', + sortable: true, + hideWhen: TRAIN_ONLY, + }, { key: 'station', label: 'Station', type: 'string', sortable: true }, { key: 'country', label: 'Country', type: 'string' }, - { key: 'trainType', label: 'Train type', type: 'string' }, - { key: 'stops', label: 'Stops', type: 'number', sortable: true }, - { key: 'handlingMeasured', label: 'Handling measured', type: 'number' }, + // Per station this would be a MAX over whatever mix of trains called there. + { + key: 'trainType', + label: 'Train type', + type: 'string', + hideWhen: TRAIN_ONLY, + }, + { + key: 'stops', + label: 'Stops', + type: 'number', + sortable: true, + hideWhen: STATION_ONLY, + }, + { + key: 'handlingMeasured', + label: 'Handling measured', + type: 'number', + hideWhen: STATION_ONLY, + }, { key: 'loadingSource', label: 'Loading from', type: 'string' }, - { key: 'avgUnloadingHours', label: 'Avg unloading (hrs)', type: 'number', sortable: true }, - { key: 'avgLoadingHours', label: 'Avg loading (hrs)', type: 'number', sortable: true }, + // Per train: this stop's own clock, not a mean of several. + { + key: 'arrivedAt', + label: 'Arrived', + type: 'date', + sortable: true, + hideWhen: TRAIN_ONLY, + }, + { + key: 'departedAt', + label: 'Departed', + type: 'date', + hideWhen: TRAIN_ONLY, + }, + { + key: 'unloadingStartedAt', + label: 'Unloading start', + type: 'date', + hideWhen: TRAIN_ONLY, + }, + { + key: 'unloadingCompletedAt', + label: 'Unloading end', + type: 'date', + hideWhen: TRAIN_ONLY, + }, + { + key: 'loadingStartedAt', + label: 'Loading start', + type: 'date', + hideWhen: TRAIN_ONLY, + }, + { + key: 'loadingCompletedAt', + label: 'Loading end', + type: 'date', + hideWhen: TRAIN_ONLY, + }, + { + key: 'unloadingHours', + label: 'Unloading (hrs)', + type: 'number', + sortable: true, + hideWhen: TRAIN_ONLY, + }, + { + key: 'loadingHours', + label: 'Loading (hrs)', + type: 'number', + sortable: true, + hideWhen: TRAIN_ONLY, + }, + { + key: 'loadUnloadHours', + label: 'Loading + unloading (hrs)', + type: 'number', + sortable: true, + hideWhen: TRAIN_ONLY, + }, + { + key: 'otherActivityHours', + label: 'Other activity (hrs)', + type: 'number', + hideWhen: TRAIN_ONLY, + }, + { + key: 'stayingHours', + label: 'Staying (hrs)', + type: 'number', + sortable: true, + hideWhen: TRAIN_ONLY, + }, + { + key: 'avgUnloadingHours', + label: 'Avg unloading (hrs)', + type: 'number', + sortable: true, + hideWhen: STATION_ONLY, + }, + { + key: 'avgLoadingHours', + label: 'Avg loading (hrs)', + type: 'number', + sortable: true, + hideWhen: STATION_ONLY, + }, { key: 'avgLoadUnloadHours', label: 'Avg loading + unloading (hrs)', type: 'number', sortable: true, + hideWhen: STATION_ONLY, + }, + { + key: 'avgOtherActivityHours', + label: 'Avg other activity (hrs)', + type: 'number', + hideWhen: STATION_ONLY, + }, + { + key: 'avgStayingHours', + label: 'Avg staying (hrs)', + type: 'number', + sortable: true, + hideWhen: STATION_ONLY, + }, + { + key: 'stayStandardHours', + label: 'Staying standard (hrs)', + type: 'number', }, - { key: 'avgOtherActivityHours', label: 'Avg other activity (hrs)', type: 'number' }, - { key: 'avgStayingHours', label: 'Avg staying (hrs)', type: 'number', sortable: true }, - { key: 'stayStandardHours', label: 'Staying standard (hrs)', type: 'number' }, { key: 'stayVerdict', label: 'Staying verdict', type: 'string' }, - { key: 'handlingStandardHours', label: 'Handling standard (hrs)', type: 'number' }, - { key: 'handlingRate', label: 'Handling rate', type: 'percent', sortable: true }, + { + key: 'handlingStandardHours', + label: 'Handling standard (hrs)', + type: 'number', + }, + { + key: 'handlingRate', + label: 'Handling rate', + type: 'percent', + sortable: true, + }, ], defaultSort: { key: 'period', dir: 'DESC' }, - chart: { type: 'bar', x: 'trainNumber', y: ['avgLoadUnloadHours'] }, + // Only plottable at station grain — per train the rows are individual stops, + // and the frontend drops the chart toggle when its columns are hidden. + chart: { type: 'bar', x: 'station', y: ['avgLoadUnloadHours'] }, query(ctx) { const { params } = ctx; + + // Per train the row IS the stop: its own logged times and its own + // durations, since an average of one stop is just the stop with the clock + // thrown away. Averaging starts where the grain stops naming the train. + if (!byStation(ctx)) { + return stationStaysQb(ctx) + .select(periodExprOn('s.arrived_at', params), 'period') + .addSelect(TRAIN_NUMBER, 'trainNumber') + .addSelect('s.station', 'station') + .addSelect('s.country', 'country') + .addSelect('s.train_type', 'trainType') + .addSelect(loadingSource('s'), 'loadingSource') + .addSelect(at('s.arrived_at'), 'arrivedAt') + .addSelect(at('s.departed_at'), 'departedAt') + .addSelect(at('s.unloading_started_at'), 'unloadingStartedAt') + .addSelect(at('s.unloading_completed_at'), 'unloadingCompletedAt') + .addSelect(at(loadingStart('s')), 'loadingStartedAt') + .addSelect(at(loadingEnd('s')), 'loadingCompletedAt') + .addSelect(unloadingHours('s'), 'unloadingHours') + .addSelect(loadingHours('s'), 'loadingHours') + .addSelect(HANDLING_HOURS, 'loadUnloadHours') + .addSelect(OTHER_ACTIVITY_HOURS, 'otherActivityHours') + .addSelect(STAYING_HOURS, 'stayingHours') + .addSelect('s.standard_hours::float8', 'stayStandardHours') + .addSelect( + `CASE WHEN (${STAYING_HOURS})::numeric <= s.standard_hours + THEN 'Encouraging' ELSE 'Needs reason' END`, + 'stayVerdict', + ) + .addSelect('s.handling_standard_hours::float8', 'handlingStandardHours') + .addSelect( + cycleRateExpr(`(${HANDLING_HOURS})::numeric`, 's.handling_standard_hours'), + 'handlingRate', + ); + } + // Reused verbatim in the GROUP BY, per the trap documented on `periodExpr`. const bucket = periodTruncExprOn('s.arrived_at', params); - const perStation = byStation(ctx); - const qb = stationStaysQb(ctx) - .select(periodExprOn('s.arrived_at', params), 'period') - .addSelect(perStation ? "'All trains'" : TRAIN_NUMBER, 'trainNumber') - .addSelect('s.station', 'station') - .addSelect('s.country', 'country') - .addSelect('MAX(s.train_type)', 'trainType') - .addSelect('COUNT(*)::int', 'stops') - .addSelect(`COUNT(${HANDLING_HOURS})::int`, 'handlingMeasured') - // Which side of the COALESCE the loading columns came from. A group that - // mixes both says so rather than claiming either. - .addSelect( - `CASE WHEN COUNT(DISTINCT ${loadingSource('s')}) > 1 THEN 'Mixed' + return ( + stationStaysQb(ctx) + .select(periodExprOn('s.arrived_at', params), 'period') + .addSelect('s.station', 'station') + .addSelect('s.country', 'country') + .addSelect('COUNT(*)::int', 'stops') + .addSelect(`COUNT(${HANDLING_HOURS})::int`, 'handlingMeasured') + // Which side of the COALESCE the loading columns came from. A group that + // mixes both says so rather than claiming either. + .addSelect( + `CASE WHEN COUNT(DISTINCT ${loadingSource('s')}) > 1 THEN 'Mixed' ELSE MAX(${loadingSource('s')}) END`, - 'loadingSource', - ) - .addSelect(avg(unloadingHours('s')), 'avgUnloadingHours') - .addSelect(avg(loadingHours('s')), 'avgLoadingHours') - .addSelect(avg(HANDLING_HOURS), 'avgLoadUnloadHours') - .addSelect(avg(OTHER_ACTIVITY_HOURS), 'avgOtherActivityHours') - .addSelect(avg(STAYING_HOURS), 'avgStayingHours') - .addSelect('MAX(s.standard_hours)::float8', 'stayStandardHours') - .addSelect( - `CASE WHEN AVG((${STAYING_HOURS})::numeric) <= MAX(s.standard_hours) + 'loadingSource', + ) + .addSelect(avg(unloadingHours('s')), 'avgUnloadingHours') + .addSelect(avg(loadingHours('s')), 'avgLoadingHours') + .addSelect(avg(HANDLING_HOURS), 'avgLoadUnloadHours') + .addSelect(avg(OTHER_ACTIVITY_HOURS), 'avgOtherActivityHours') + .addSelect(avg(STAYING_HOURS), 'avgStayingHours') + .addSelect('MAX(s.standard_hours)::float8', 'stayStandardHours') + .addSelect( + `CASE WHEN AVG((${STAYING_HOURS})::numeric) <= MAX(s.standard_hours) THEN 'Encouraging' ELSE 'Needs reason' END`, - 'stayVerdict', - ) - .addSelect(`${HANDLING_STANDARD}::float8`, 'handlingStandardHours') - // Same formula the turnaround cycle publishes, so the two read alike. - // NULL standard in, NULL rate out — nothing to measure against yet. - .addSelect( - cycleRateExpr(`AVG((${HANDLING_HOURS})::numeric)`, HANDLING_STANDARD), - 'handlingRate', - ) - .groupBy(bucket) - .addGroupBy('s.station') - .addGroupBy('s.country'); - if (!perStation) qb.addGroupBy(TRAIN_NUMBER); - return qb; + 'stayVerdict', + ) + .addSelect(`${HANDLING_STANDARD}::float8`, 'handlingStandardHours') + // Same formula the turnaround cycle publishes, so the two read alike. + // NULL standard in, NULL rate out — nothing to measure against yet. + .addSelect( + cycleRateExpr(`AVG((${HANDLING_HOURS})::numeric)`, HANDLING_STANDARD), + 'handlingRate', + ) + .groupBy(bucket) + .addGroupBy('s.station') + .addGroupBy('s.country') + ); }, async summary(ctx) { const row = await stationStaysQb(ctx) @@ -170,13 +351,26 @@ export const loadingUnloadingReport: ReportDefinition = { .addSelect(`COUNT(${HANDLING_HOURS})::int`, 'measured') .addSelect(avg(HANDLING_HOURS), 'avgHandling') .addSelect(avg(OTHER_ACTIVITY_HOURS), 'avgOther') - .getRawOne<{ stops: number; measured: number; avgHandling: number; avgOther: number }>(); + .getRawOne<{ + stops: number; + measured: number; + avgHandling: number; + avgOther: number; + }>(); return [ { label: 'Stops measured', value: Number(row?.stops ?? 0) }, { label: 'Handling measured', value: Number(row?.measured ?? 0) }, - { label: 'Average loading + unloading', value: Number(row?.avgHandling ?? 0), unit: 'h' }, - { label: 'Average other activity', value: Number(row?.avgOther ?? 0), unit: 'h' }, + { + label: 'Average loading + unloading', + value: Number(row?.avgHandling ?? 0), + unit: 'h', + }, + { + label: 'Average other activity', + value: Number(row?.avgOther ?? 0), + unit: 'h', + }, ]; }, }; diff --git a/apps/edr-freight-api/src/modules/reports/report-runner.service.ts b/apps/edr-freight-api/src/modules/reports/report-runner.service.ts index d38a5ba8a..350fdc4ee 100644 --- a/apps/edr-freight-api/src/modules/reports/report-runner.service.ts +++ b/apps/edr-freight-api/src/modules/reports/report-runner.service.ts @@ -7,7 +7,7 @@ import { normalizePagination, } from '../../common/utils/pagination.util'; import { applyBookingRefDirectionScope } from '../user-trade-access/trade-scope.util'; -import { ReportDefinition, ReportRunResult } from './report.types'; +import { ReportColumn, ReportDefinition, ReportRunResult } from './report.types'; const DAY_MS = 24 * 60 * 60 * 1000; @@ -38,7 +38,7 @@ function coerceParams( const items = csv?.split(',').map((s) => s.trim()).filter(Boolean) ?? []; params[filter.key] = items.length ? items : null; } else { - params[filter.key] = raw[filter.key]?.trim() || null; + params[filter.key] = raw[filter.key]?.trim() || filter.defaultValue || null; } } // idKey, when the report declares one, is a plain string param. @@ -57,19 +57,34 @@ function coerceParams( */ const aliasSortExpr = (key: string): string => `"${key.replace(/"/g, '""')}"`; +/** + * Columns the current filter values don't hide. A hidden column is not in the + * SELECT list of the shape those filters produce, so sorting by one would be a + * 42703 — the sort falls back to the default instead. + */ +export const visibleColumns = ( + def: ReportDefinition, + params: Record, +): ReportColumn[] => + def.columns.filter((c) => + Object.entries(c.hideWhen ?? {}).every(([key, value]) => params[key] !== value), + ); + /** Resolve a client-requested sort column against the report's own whitelist. */ function resolveSort( def: ReportDefinition, + params: Record, sortBy?: string, sortOrder?: string, ): { key: string; expr: string; dir: 'ASC' | 'DESC' } | null { const dir = sortOrder?.toUpperCase() === 'DESC' ? 'DESC' : 'ASC'; - const requested = sortBy && def.columns.find((c) => c.key === sortBy && c.sortable); + const columns = visibleColumns(def, params); + const requested = sortBy && columns.find((c) => c.key === sortBy && c.sortable); if (requested) { return { key: requested.key, expr: requested.sortExpr ?? aliasSortExpr(requested.key), dir }; } if (!def.defaultSort) return null; - const fallback = def.columns.find((c) => c.key === def.defaultSort!.key); + const fallback = columns.find((c) => c.key === def.defaultSort!.key); if (!fallback) return null; return { key: fallback.key, @@ -91,7 +106,7 @@ export class ReportRunnerService { const ctx = { ds: this.ds, params, directions }; const qb = def.query(ctx); - const sort = resolveSort(def, raw.sortBy, raw.sortOrder); + const sort = resolveSort(def, params, raw.sortBy, raw.sortOrder); if (sort) qb.orderBy(sort.expr, sort.dir); const { page: pageNum, pageSize, skip, take } = normalizePagination({ @@ -141,7 +156,7 @@ export class ReportRunnerService { const qb = def.query(ctx); // Same sort the on-screen table is using, not always the default — an // export is supposed to match what the user is looking at. - const sort = resolveSort(def, raw.sortBy, raw.sortOrder); + const sort = resolveSort(def, params, raw.sortBy, raw.sortOrder); if (sort) qb.orderBy(sort.expr, sort.dir); const ceiling = limit ?? cap; diff --git a/apps/edr-freight-api/src/modules/reports/report.types.ts b/apps/edr-freight-api/src/modules/reports/report.types.ts index ca7578461..7f1758164 100644 --- a/apps/edr-freight-api/src/modules/reports/report.types.ts +++ b/apps/edr-freight-api/src/modules/reports/report.types.ts @@ -19,6 +19,12 @@ export interface ReportColumn { sortable?: boolean; /** SQL to ORDER BY when this column is sorted, if different from `key`. */ sortExpr?: string; + /** + * Hide the column while a filter holds a given value — how one report serves + * two group-by grains without two column lists. Display only: the value is + * still selected, exported and sortable, it just isn't shown. + */ + hideWhen?: Record; } export type ReportFilterType = 'daterange' | 'date' | 'select' | 'multiselect' | 'text'; @@ -34,6 +40,12 @@ export interface ReportFilterDef { type: ReportFilterType; /** Static option list for select/multiselect. */ options?: ReportFilterOption[]; + /** + * Value the filter takes when the client sends nothing — so a report whose + * shape depends on a filter (see `ReportColumn.hideWhen`) never has to guess + * what "unset" meant. + */ + defaultValue?: string; /** * Resolves the option list from the database instead of declaring it inline — * for filters whose choices are reference data (stations, cargo types). diff --git a/apps/edr-freight-web/backoffice/src/components/reports/ReportExportButton.tsx b/apps/edr-freight-web/backoffice/src/components/reports/ReportExportButton.tsx index 76ff7d628..89cd638a6 100644 --- a/apps/edr-freight-web/backoffice/src/components/reports/ReportExportButton.tsx +++ b/apps/edr-freight-web/backoffice/src/components/reports/ReportExportButton.tsx @@ -1,14 +1,16 @@ import { Button, Checkbox, Group, Modal, Radio, Select, SimpleGrid, Stack, Text } from "@mantine/core"; import { Download, FileSpreadsheet, FileText } from "lucide-react"; -import { useState } from "react"; +import { useEffect, useState } from "react"; import { reportsService } from "@/services/reports.service"; -import type { ReportCatalogEntry, ReportRunParams } from "@/types/reports"; +import type { ReportCatalogEntry, ReportColumn, ReportRunParams } from "@/types/reports"; interface ReportExportButtonProps { def: ReportCatalogEntry; /** Filters + sort currently applied on screen — no key/page/pageSize. */ params: Omit; + /** The columns on screen, which for a report with `hideWhen` is not all of them. */ + columns: ReportColumn[]; } const RECORD_OPTIONS = [ @@ -31,17 +33,21 @@ function saveBlob(blob: Blob, filename: string) { /** One export button: format, which fields, how many records — applies the * filters/sort already on screen. Record count defaults to all (capped * server-side per format). */ -export function ReportExportButton({ def, params }: ReportExportButtonProps) { +export function ReportExportButton({ def, params, columns }: ReportExportButtonProps) { const [opened, setOpened] = useState(false); const [format, setFormat] = useState<"xlsx" | "pdf">("xlsx"); - const [fields, setFields] = useState(def.columns.map((c) => c.key)); + const [fields, setFields] = useState(columns.map((c) => c.key)); const [records, setRecords] = useState("all"); const [exporting, setExporting] = useState(false); - const allSelected = fields.length === def.columns.length; + // A filter change can change which columns exist at all — start over from the + // new set rather than exporting keys the query no longer selects. + useEffect(() => setFields(columns.map((c) => c.key)), [columns]); + + const allSelected = fields.length === columns.length; const toggleField = (key: string) => setFields((prev) => (prev.includes(key) ? prev.filter((k) => k !== key) : [...prev, key])); - const toggleAll = () => setFields(allSelected ? [] : def.columns.map((c) => c.key)); + const toggleAll = () => setFields(allSelected ? [] : columns.map((c) => c.key)); const handleDownload = async () => { setExporting(true); @@ -110,7 +116,7 @@ export function ReportExportButton({ def, params }: ReportExportButtonProps) { - {def.columns.map((col) => ( + {columns.map((col) => ( { + const applied = appliedParams as Record; + const valueOf = (key: string) => applied[key] ?? def?.filters.find((f) => f.key === key)?.defaultValue; + return (def?.columns ?? []).filter((col) => + Object.entries(col.hideWhen ?? {}).every(([key, value]) => valueOf(key) !== value), + ); + }, [def?.columns, def?.filters, appliedParams]); + + /** A chart whose x or y column is hidden has nothing to plot — drop the toggle. */ + const chartDef = useMemo(() => { + if (!def?.chart) return undefined; + const shown = new Set(visibleColumns.map((c) => c.key)); + return shown.has(def.chart.x) && def.chart.y.every((k) => shown.has(k)) ? def.chart : undefined; + }, [def?.chart, visibleColumns]); + const columns: ColumnDef>[] = useMemo( () => - (def?.columns ?? []).map((col) => ({ + visibleColumns.map((col) => ({ id: col.key, accessorKey: col.key, header: col.sortable @@ -188,7 +208,7 @@ export function ReportView({ reportKey, idKeyValue, pageHeader, defaultView }: R ), })), - [def?.columns], + [visibleColumns], ); if (!def) { @@ -197,7 +217,7 @@ export function ReportView({ reportKey, idKeyValue, pageHeader, defaultView }: R ) : null; } - const chartToggle = def.chart ? ( + const chartToggle = chartDef ? ( ); - const exportButton = ; + const exportButton = ; return ( @@ -269,8 +289,8 @@ export function ReportView({ reportKey, idKeyValue, pageHeader, defaultView }: R - {view === "chart" && def.chart ? ( - + {view === "chart" && chartDef ? ( + ) : ( ; } -export type ReportFilterType = "daterange" | "date" | "select" | "multiselect" | "text"; +export type ReportFilterType = + | "daterange" + | "date" + | "select" + | "multiselect" + | "text"; export interface ReportFilterOption { value: string; @@ -25,6 +32,8 @@ export interface ReportFilterDef { label: string; type: ReportFilterType; options?: ReportFilterOption[]; + /** Value the server assumes when the filter is unset. */ + defaultValue?: string; } export interface ReportIdKey { From ac325a6282c327f3be010d62f401fa79309039b6 Mon Sep 17 00:00:00 2001 From: Nathnael Date: Tue, 25 Aug 2026 13:05:18 +0000 Subject: [PATCH 3/4] feat(reports): report charged vs actual volume in revenue categories MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Four changes the business asked for on Charged and Actual Volumes: - Split the Leg column into From and To, both sortable. - Add 20ft and 40ft container counts beside TEU, off the marshalling record's container items. - Classify cargo into the revenue vocabulary rather than the operational one, so a corridor's tonnage and its revenue read in the same buckets. Charge-only buckets (incidental, first/last mile, customs) cannot be emitted — no physical wagon is one. - Carry the empty wagons as rows of their own, the way the marshalling document lists them. `allocationLedgerQb` gains `includeEmptyWagons`, which starts the ledger from the wagon instead of the allocation; the empty and total wagon counts become plain group aggregates, so a departure's wagons now add up down its rows instead of every row repeating the train's total. Vehicle-Km lands on the empty rows and sums across legs. `LOADED_WAGONS_EXPR` gains a FILTER on the allocation being present — a no-op for every allocation-grain report, and the fix at the one place all of them route through. `SCHEDULE_EMPTY_WAGONS` had no callers left and is deleted. Verified: type-check clean, 50 report specs pass (incl. a new one asserting the cargo expression only emits keys the revenue vocabulary offers), and the report SQL EXPLAINs and runs against edr_dev — 60 empty wagons over 4 rows, 53,539 Vehicle-Km, 20ft/40ft counts populating. Co-Authored-By: Claude Opus 5 (1M context) --- .../charged-vs-actual-volume.report.ts | 74 ++++++++++----- .../reports/operations-classification.spec.ts | 16 ++++ .../reports/operations-classification.ts | 92 ++++++++++++++----- 3 files changed, 136 insertions(+), 46 deletions(-) diff --git a/apps/edr-freight-api/src/modules/reports/definitions/charged-vs-actual-volume.report.ts b/apps/edr-freight-api/src/modules/reports/definitions/charged-vs-actual-volume.report.ts index 5fe6ba81b..ffb138c22 100644 --- a/apps/edr-freight-api/src/modules/reports/definitions/charged-vs-actual-volume.report.ts +++ b/apps/edr-freight-api/src/modules/reports/definitions/charged-vs-actual-volume.report.ts @@ -4,19 +4,20 @@ import { Yard } from '../../rule-engine/entities/yard.entity'; import { ReportContext, ReportDefinition } from '../report.types'; import { ACTUAL_TONS_EXPR, - CARGO_CATEGORY_EXPR, - CARGO_CATEGORY_FILTER, - CARGO_CATEGORY_LABEL_EXPR, + ALLOC_CONTAINERS_20, + ALLOC_CONTAINERS_40, CHARGED_TONS_EXPR, LOADED_WAGONS_EXPR, OPERATIONS_FILTERS, - SCHEDULE_EMPTY_WAGONS, + REVENUE_CARGO_CATEGORY_EXPR, + REVENUE_CARGO_FILTER, SCHEDULE_KM_EXPR, TEU_EXPR, allocationLedgerQb, applyCategoryFilter, distanceKmBetween, } from '../operations-classification'; +import { CATEGORY_LABEL_OF } from '../revenue-classification'; /** * A leg is one station-to-station move the train actually made: two consecutive @@ -47,13 +48,34 @@ const LEG_FROM = 'COALESCE(leg.from_yard_id, ts.origin_station_id)'; const LEG_TO = 'COALESCE(leg.to_yard_id, ts.destination_station_id)'; /** - * Distance and empty-wagon count are constant within a group that includes - * `ts.id` and the leg — MAX() satisfies Postgres without dragging a scalar - * subselect through the GROUP BY. + * Distance is constant within a group that includes `ts.id` and the leg — + * MAX() satisfies Postgres without dragging a scalar subselect through the + * GROUP BY. */ const LEG_KM = `MAX(${distanceKmBetween(LEG_FROM, LEG_TO)})`; -const EMPTY_WAGONS = `MAX(${SCHEDULE_EMPTY_WAGONS})`; -const TOTAL_WAGONS = `(COUNT(DISTINCT tsw.id) + ${EMPTY_WAGONS})::int`; + +/** + * The ledger carries the empty wagons as rows of their own, so both counts are + * plain aggregates over the group: an empty-wagon row has no loaded wagons and + * a cargo row has no empty ones. Read down a departure's rows and its wagons + * add up once, instead of every row repeating the train's empty total. + */ +const EMPTY_WAGONS = 'COUNT(DISTINCT tsw.id) FILTER (WHERE wba.id IS NULL)'; +const TOTAL_WAGONS = 'COUNT(DISTINCT tsw.id)::int'; + +/** + * Cargo in the revenue vocabulary, plus the wagon that carried none. + * + * The label is built out of the key expression rather than beside it: Postgres + * only accepts an aggregate-query column inside a GROUP BY expression it can + * match verbatim, so a second `wba.id IS NULL` test of its own would demand + * `wba.id` in the GROUP BY — which would split the grain down to one row per + * allocation. + */ +const CATEGORY_EXPR = `CASE WHEN wba.id IS NULL THEN 'EMPTY_WAGON' + ELSE ${REVENUE_CARGO_CATEGORY_EXPR} END`; +const CATEGORY_LABEL_EXPR = `CASE WHEN (${CATEGORY_EXPR}) = 'EMPTY_WAGON' THEN 'Empty wagon' + ELSE ${CATEGORY_LABEL_OF(CATEGORY_EXPR)} END`; /** * Ton/Km and Vehicle-Km are NULL — not zero — when the yard pair has no @@ -64,8 +86,8 @@ const TON_KM = `ROUND((${CHARGED_TONS_EXPR})::numeric * ${LEG_KM}, 1)::float8`; const VEHICLE_KM = `ROUND(${EMPTY_WAGONS}::numeric * ${LEG_KM}, 1)::float8`; function baseQuery(ctx: ReportContext): SelectQueryBuilder { - const qb = allocationLedgerQb(ctx); - applyCategoryFilter(qb, ctx.params); + const qb = allocationLedgerQb(ctx, { includeEmptyWagons: true }); + applyCategoryFilter(qb, ctx.params, CATEGORY_EXPR); return qb; } @@ -89,20 +111,25 @@ export const chargedVsActualVolumeReport: ReportDefinition = { 'corridor. Charged volume is the standard weight capacity — 20 and 40 tons per laden ' + 'container, 2.24 and 3.88 empty, 70 tons per wagon of steel or fertilizer, 38 for ' + 'perishables — all editable in Operating standards. Actual volume is what the ' + - 'marshalling recorded. Volumes and wagon counts belong to the train, not to the leg, ' + - 'so they repeat on every leg it ran and across its cargo types rather than being split ' + - 'between them — the KPIs above count each train once. Ton/Km and Vehicle-Km are the ' + - 'exception and are the leg’s own, so they add up across legs into the real corridor ' + - 'figure.', + 'marshalling recorded. Cargo types are the revenue categories the money side bills ' + + 'against, so a corridor’s tonnage and its revenue read in the same buckets; wagons ' + + 'that carried nothing are their own “Empty wagon” line. Volumes and wagon counts ' + + 'belong to the train, not to the leg, so they repeat on every leg it ran rather than ' + + 'being split between them — the KPIs above count each train once. Ton/Km and ' + + 'Vehicle-Km are the exception and are the leg’s own, so they add up across legs into ' + + 'the real corridor figure.', group: 'Operations', - filters: [...OPERATIONS_FILTERS, CARGO_CATEGORY_FILTER], + filters: [...OPERATIONS_FILTERS, REVENUE_CARGO_FILTER], columns: [ { key: 'trainNumber', label: 'Train No.', type: 'string', sortable: true, sortExpr: 'ts.train_number' }, { key: 'departedAt', label: 'Departure', type: 'date', sortable: true, sortExpr: 'ts.scheduled_departure_date' }, - { key: 'leg', label: 'Leg', type: 'string' }, - { key: 'category', label: 'Cargo type', type: 'string', sortable: true, sortExpr: CARGO_CATEGORY_EXPR }, + { key: 'legFrom', label: 'From', type: 'string', sortable: true, sortExpr: 'COALESCE(lfy.label, lfy.code)' }, + { key: 'legTo', label: 'To', type: 'string', sortable: true, sortExpr: 'COALESCE(lty.label, lty.code)' }, + { key: 'category', label: 'Cargo type', type: 'string', sortable: true, sortExpr: CATEGORY_EXPR }, { key: 'chargedTons', label: 'Charged volume', type: 'tons', sortable: true }, { key: 'actualTons', label: 'Actual volume', type: 'tons', sortable: true }, + { key: 'containers20', label: '20ft', type: 'number', sortable: true }, + { key: 'containers40', label: '40ft', type: 'number', sortable: true }, { key: 'teu', label: 'TEU', type: 'number' }, { key: 'wagons', label: 'Loaded wagons', type: 'number' }, { key: 'emptyWagons', label: 'Empty wagons', type: 'number' }, @@ -116,10 +143,13 @@ export const chargedVsActualVolumeReport: ReportDefinition = { return legQuery(ctx) .select("COALESCE(ts.train_number, '—')", 'trainNumber') .addSelect(`to_char(COALESCE(ts.actual_departure_at, ts.scheduled_departure_date), 'YYYY-MM-DD HH24:MI')`, 'departedAt') - .addSelect("COALESCE(lfy.label, lfy.code, '?') || ' → ' || COALESCE(lty.label, lty.code, '?')", 'leg') - .addSelect(CARGO_CATEGORY_LABEL_EXPR, 'category') + .addSelect("COALESCE(lfy.label, lfy.code, '?')", 'legFrom') + .addSelect("COALESCE(lty.label, lty.code, '?')", 'legTo') + .addSelect(CATEGORY_LABEL_EXPR, 'category') .addSelect(`ROUND((${CHARGED_TONS_EXPR})::numeric, 2)::float8`, 'chargedTons') .addSelect(`ROUND((${ACTUAL_TONS_EXPR})::numeric, 2)::float8`, 'actualTons') + .addSelect(`COALESCE(SUM(${ALLOC_CONTAINERS_20}), 0)::int`, 'containers20') + .addSelect(`COALESCE(SUM(${ALLOC_CONTAINERS_40}), 0)::int`, 'containers40') .addSelect(TEU_EXPR, 'teu') .addSelect(LOADED_WAGONS_EXPR, 'wagons') .addSelect(`${EMPTY_WAGONS}::int`, 'emptyWagons') @@ -137,7 +167,7 @@ export const chargedVsActualVolumeReport: ReportDefinition = { .addGroupBy('lfy.code') .addGroupBy('lty.label') .addGroupBy('lty.code') - .addGroupBy(CARGO_CATEGORY_EXPR); + .addGroupBy(CATEGORY_EXPR); }, async summary(ctx) { const row = await baseQuery(ctx) diff --git a/apps/edr-freight-api/src/modules/reports/operations-classification.spec.ts b/apps/edr-freight-api/src/modules/reports/operations-classification.spec.ts index 228e24470..9dd31cdfd 100644 --- a/apps/edr-freight-api/src/modules/reports/operations-classification.spec.ts +++ b/apps/edr-freight-api/src/modules/reports/operations-classification.spec.ts @@ -2,6 +2,7 @@ import { CARGO_CATEGORIES, CARGO_CATEGORY_EXPR, CARGO_CATEGORY_LABEL_EXPR, + REVENUE_CARGO_CATEGORY_EXPR, CONTAINER_CLASSES, CONTAINER_CLASS_EXPR, HANDLING_STANDARD_HOURS_EXPR, @@ -13,6 +14,7 @@ import { otherActivityHours, plannedRowsSql, } from './operations-classification'; +import { REVENUE_CATEGORIES } from './revenue-classification'; import { TARGET_DIMENSIONS, TARGET_METRICS } from '../operations-reporting/entities/operations-target.entity'; /** @@ -69,6 +71,20 @@ describe('operations classification', () => { expect(missing).toEqual([]); }); + /** + * The volume report groups tonnage by this expression and the finance reports + * group birr by `REVENUE_CATEGORY_EXPR`. A key only one side can emit is a + * bucket that never reconciles — and it fails silently, as a row that simply + * has no counterpart. + */ + it('classifies cargo into keys the revenue vocabulary offers', () => { + const offered = new Set(REVENUE_CATEGORIES.map((o) => o.value)); + const missing = [...new Set(emittedKeys(REVENUE_CARGO_CATEGORY_EXPR))].filter( + (k) => !offered.has(k), + ); + expect(missing).toEqual([]); + }); + it('offers every container class the expression can emit', () => { const offered = new Set(CONTAINER_CLASSES.map((o) => o.value)); const missing = [...new Set(emittedKeys(CONTAINER_CLASS_EXPR))].filter((k) => !offered.has(k)); diff --git a/apps/edr-freight-api/src/modules/reports/operations-classification.ts b/apps/edr-freight-api/src/modules/reports/operations-classification.ts index f9d65d9b9..3e4bc599d 100644 --- a/apps/edr-freight-api/src/modules/reports/operations-classification.ts +++ b/apps/edr-freight-api/src/modules/reports/operations-classification.ts @@ -11,7 +11,7 @@ import { WagonBookingAllocation } from '../train-schedules/entities/wagon-bookin import { Yard } from '../rule-engine/entities/yard.entity'; import { applyDirectionScope } from '../user-trade-access/trade-scope.util'; import { ReportContext, ReportFilterDef, ReportFilterOption } from './report.types'; -import { resolvePeriod, yardOptions } from './revenue-classification'; +import { REVENUE_CATEGORIES, resolvePeriod, yardOptions } from './revenue-classification'; /** * The shared vocabulary and SQL behind every operations report — turnaround, @@ -115,6 +115,39 @@ export const CARGO_CATEGORY_EXPR = `CASE ELSE 'UNCLASSIFIED' END`; +/** + * The same cargo, classified into the REVENUE vocabulary — the categories + * `revenue-classification.ts` bills against, minus its charge-only buckets + * (incidental, first/last mile, customs), which no physical wagon can be. + * + * Mirrors the cargo arms of `REVENUE_CATEGORY_EXPR` in that expression's own + * order, so a ton and the birr charged for it land in the same bucket: empty + * re-export before domestic, domestic before anything about what is in the box. + * Reports that must reconcile tonnage against revenue group by this one; the + * operational vocabulary above keeps sand and bulk apart, which no invoice does. + */ +export const REVENUE_CARGO_CATEGORY_EXPR = `CASE + WHEN ${IS_EMPTY_CONTAINER} THEN 'EMPTY_CONTAINER_REEXPORT' + WHEN oy.country IS NOT NULL AND oy.country = dy.country THEN 'DOMESTIC' + WHEN ${IS_CONTAINER} AND b.trade_direction = 'EXPORT' THEN 'CONTAINER_EXPORT' + WHEN ${IS_CONTAINER} AND ${IS_MULTIMODAL} THEN 'CONTAINER_IMPORT_MULTIMODAL' + WHEN ${IS_CONTAINER} THEN 'CONTAINER_IMPORT_UNIMODAL' + WHEN ct.code IN (${quote(FERTILIZER_CODES)}) THEN 'FERTILIZER' + WHEN ct.code IN (${quote(BREAK_BULK_CODES)}) THEN 'BREAK_BULK' + WHEN ct.code IN (${quote(RORO_CODES)}) THEN 'RORO' + WHEN b.trade_direction = 'EXPORT' THEN 'OTHER_EXPORT_CARGO' + WHEN b.trade_direction = 'IMPORT' THEN 'OTHER_IMPORT_BULK' + ELSE 'UNCLASSIFIED' +END`; + +/** The revenue vocabulary as a filter, plus the wagon that carries no cargo. */ +export const REVENUE_CARGO_FILTER: ReportFilterDef = { + key: 'categories', + label: 'Cargo type', + type: 'multiselect', + options: [...REVENUE_CATEGORIES, { value: 'EMPTY_WAGON', label: 'Empty wagon' }], +}; + export const CONTAINER_CLASS_EXPR = `CASE WHEN ${IS_EMPTY_CONTAINER} THEN 'EMPTY_CONTAINER_RETURN' WHEN b.trade_direction = 'EXPORT' THEN 'CONTAINER_EXPORT' @@ -330,23 +363,13 @@ export const CHARGED_TONS_EXPR = `( * ${stdAgg('charged_tons_per_wagon_general', 70)} )::float8`; -/** Wagons actually carrying cargo in the grouped set. */ -export const LOADED_WAGONS_EXPR = 'COUNT(DISTINCT tsw.id)::int'; - /** - * Wagons on the departure with nothing allocated to them — the Vehicle-Km base. - * - * A train-level figure: it belongs to the departure, not to any one cargo type - * riding on it, so a report grouped finer than the schedule repeats it rather - * than splitting it. Callers that need a total must de-duplicate by schedule. + * Wagons actually carrying cargo in the grouped set. The FILTER only bites on a + * query built with `includeEmptyWagons` — every row of an allocation-grain + * query has an allocation, so it is a no-op there. */ -export const SCHEDULE_EMPTY_WAGONS = `( - SELECT COUNT(*) FROM freight.train_set_wagons tw - WHERE tw.train_set_id = ts.train_set_id AND tw.deleted_at IS NULL - AND NOT EXISTS ( - SELECT 1 FROM freight.wagon_booking_allocations a - WHERE a.train_set_wagon_id = tw.id AND a.deleted_at IS NULL) -)`; +export const LOADED_WAGONS_EXPR = + 'COUNT(DISTINCT tsw.id) FILTER (WHERE wba.id IS NOT NULL)::int'; /** * Trainsets operated: wagons loaded divided by a full trainset for this cargo. @@ -440,21 +463,41 @@ const DEAD_SCHEDULE_STATUSES = ['DRAFT', 'CANCELLED']; * * The booking is LEFT joined — a wagon can be allocated before its booking data * is complete, and dropping those rows would understate wagon usage. + * + * `includeEmptyWagons` turns the ledger around to start from the wagon instead: + * every wagon of the departure is a row, and one that carried nothing has a + * NULL `wba`. Only the volume report wants that — it reports the empty wagons + * as their own line — and it costs the other reports a row grain they would + * have to filter back out. */ -export function allocationLedgerQb(ctx: ReportContext): SelectQueryBuilder { +export function allocationLedgerQb( + ctx: ReportContext, + opts: { includeEmptyWagons?: boolean } = {}, +): SelectQueryBuilder { const { params, directions } = ctx; - const qb = ctx.ds - .createQueryBuilder() - .from(WagonBookingAllocation, 'wba') - .innerJoin(TrainSetWagon, 'tsw', 'tsw.id = wba.train_set_wagon_id AND tsw.deleted_at IS NULL') - .innerJoin(TrainSchedule, 'ts', 'ts.train_set_id = tsw.train_set_id AND ts.deleted_at IS NULL') + const qb = ctx.ds.createQueryBuilder(); + + if (opts.includeEmptyWagons) { + qb.from(TrainSetWagon, 'tsw') + .leftJoin( + WagonBookingAllocation, + 'wba', + 'wba.train_set_wagon_id = tsw.id AND wba.deleted_at IS NULL', + ) + .where('tsw.deleted_at IS NULL'); + } else { + qb.from(WagonBookingAllocation, 'wba') + .innerJoin(TrainSetWagon, 'tsw', 'tsw.id = wba.train_set_wagon_id AND tsw.deleted_at IS NULL') + .where('wba.deleted_at IS NULL'); + } + + qb.innerJoin(TrainSchedule, 'ts', 'ts.train_set_id = tsw.train_set_id AND ts.deleted_at IS NULL') .leftJoin(Booking, 'b', 'b.id = wba.booking_id AND b.deleted_at IS NULL') .leftJoin(CargoType, 'ct', 'ct.id = b.cargo_type_id') .leftJoin(Yard, 'oy', 'oy.id = ts.origin_station_id') .leftJoin(Yard, 'dy', 'dy.id = ts.destination_station_id') .leftJoin(OperationsStandard, 'std', STANDARDS_JOIN) - .where('wba.deleted_at IS NULL') .andWhere('ts.status NOT IN (:...deadScheduleStatuses)', { deadScheduleStatuses: DEAD_SCHEDULE_STATUSES, }); @@ -715,10 +758,11 @@ export function applyOperationsFilters( export function applyCategoryFilter( qb: SelectQueryBuilder, params: Record, + categoryExpr: string = CARGO_CATEGORY_EXPR, ): void { const categories = params.categories as string[] | null; if (categories?.length) { - qb.andWhere(`${CARGO_CATEGORY_EXPR} IN (:...categories)`, { categories }); + qb.andWhere(`${categoryExpr} IN (:...categories)`, { categories }); } } From 08aba3fb4d76d5090a6d9eebf9f4b768d414af78 Mon Sep 17 00:00:00 2001 From: Nathnael Date: Tue, 25 Aug 2026 13:05:55 +0000 Subject: [PATCH 4/4] fix --- .../components/positionSelection.tsx | 40 ++++++++++++++----- 1 file changed, 31 insertions(+), 9 deletions(-) diff --git a/apps/edr-freight-web/backoffice/src/record-management/components/positionSelection.tsx b/apps/edr-freight-web/backoffice/src/record-management/components/positionSelection.tsx index 889552384..b014425e5 100644 --- a/apps/edr-freight-web/backoffice/src/record-management/components/positionSelection.tsx +++ b/apps/edr-freight-web/backoffice/src/record-management/components/positionSelection.tsx @@ -25,6 +25,22 @@ interface BasePosition { isDelegate: boolean; } +/** + * `x-delegated-position-id` names the DELEGATOR's position and the API matches + * it against `position.id`. This used to be set for every selection, carrying + * `employeePositionId` — so it matched nothing, and because the API resolves + * the delegation header *before* the current-position one, it swallowed the + * whole selection: every request ran as the employee's first position no + * matter what the picker said. Only a genuine delegate carries it. + */ +const applyDelegationCookie = (position?: BasePosition | null) => { + if (position?.isDelegate && position.id) { + Cookies.set("delegatedPositionId", position.id); + return; + } + Cookies.remove("delegatedPositionId"); +}; + export const PositionSelect = () => { const { unFilteredUserDetails, @@ -63,26 +79,32 @@ export const PositionSelect = () => { setSelectedPositionId(currentPosition.employeePositionId); } - if (Cookies.get("current-position-id") !== currentPosition.id) { - Cookies.set("current-position-id", currentPosition.id); - } - + // The API matches x-current-position-id against employeePositionId, NOT + // position.id — writing the latter never matches, so the guard silently + // falls back to the employee's first position and the picker does nothing. + // (useAuthUser already self-heals this cookie for the same reason.) if ( currentPosition.employeePositionId && - Cookies.get("delegatedPositionId") !== currentPosition.employeePositionId + Cookies.get("current-position-id") !== currentPosition.employeePositionId ) { - Cookies.set("delegatedPositionId", currentPosition.employeePositionId); + Cookies.set("current-position-id", currentPosition.employeePositionId); } + + applyDelegationCookie(currentPosition); }, [currentPosition, isLoading, selectedPositionId, setSelectedPositionId]); if (isLoading || selectablePositions.length === 0) return null; const handleChange = (value: string) => { - setSelectedPositionId(value); const selected = selectablePositions.find((pos) => pos.id === value); - Cookies.set("current-position-id", value); - Cookies.set("delegatedPositionId", selected?.employeePositionId || ""); + // Both the picker state and the cookie key off employeePositionId — the + // dropdown's own value is position.id, which the API does not match on. + setSelectedPositionId(selected?.employeePositionId ?? value); + if (selected?.employeePositionId) { + Cookies.set("current-position-id", selected.employeePositionId); + } + applyDelegationCookie(selected); // Invalidate relevant queries [