From 536d6043c247ab9f59224bd23e9577ee054fb7aa Mon Sep 17 00:00:00 2001 From: Hagernesh Date: Mon, 20 Jul 2026 07:04:18 +0000 Subject: [PATCH] fix(clearance): allow customs risk to be corrected and keep the trail MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit RiskStep returned early to a badge as soon as a risk level existed, so the control was unreachable and a mis-assigned level could never be corrected. Both the server and the sibling AssignRiskCard treat risk as correctable until duty is advised off it — completeWithMetadata has no already-completed guard and overwrites metadata.riskLevel. RiskStep was stricter than either. It now keeps the control mounted alongside the assigned badge, offers "Reassign risk", and locks to badge-only once DUTY_TAXES_ADVISED completes. The control also reads the persisted level (it was hardcoded to GREEN, so unhiding it alone would have misreported the assignment), and the T1 gate is skipped once a level exists, since risk cannot be assigned without a closed T1 and stale T1 data must not hide the badge. Correcting a level previously left no record of the old value, who changed it, or when — thin ground for a customer-visible level that may be disputed. assignRisk now appends each decision to metadata.riskHistory: the level, the level it replaced, the timestamp, the user id, and a display name resolved at assignment time so the trail shows a person rather than a UUID. riskLevel still carries the current value and always equals the last entry, so existing consumers are unchanged. History lives on the existing metadata JSONB column, so no migration is needed, and the logic sits in assignRisk rather than the shared completeWithMetadata that adviseDuty and others also use. Re-picking the level already in force is not recorded — it changed nothing. Co-Authored-By: Claude Opus 4.8 --- .../contracts/booking-clearance.service.ts | 13 +++- .../clearance-milestone.risk.spec.ts | 76 +++++++++++++++++++ .../contracts/clearance-milestone.service.ts | 37 ++++++++- .../contracts/contract-clearance.service.ts | 12 ++- .../modules/contracts/contracts.controller.ts | 6 +- .../entities/clearance-milestone.entity.ts | 23 +++++- .../contracts/PhasedClearanceActionPanel.tsx | 67 +++++++++++++--- packages/types/src/freight/contracts.ts | 20 +++++ packages/types/src/freight/index.ts | 2 + 9 files changed, 241 insertions(+), 15 deletions(-) diff --git a/apps/edr-freight-api/src/modules/contracts/booking-clearance.service.ts b/apps/edr-freight-api/src/modules/contracts/booking-clearance.service.ts index 59dad2248..810232993 100644 --- a/apps/edr-freight-api/src/modules/contracts/booking-clearance.service.ts +++ b/apps/edr-freight-api/src/modules/contracts/booking-clearance.service.ts @@ -13,7 +13,10 @@ import { FilesService } from '../files/files.service'; import { BookingsRepository } from '../bookings/bookings.repository'; import { BookingsService } from '../bookings/bookings.service'; import { BookingLifecycleNotifierService } from '../bookings/booking-lifecycle-notifier.service'; -import { ClearanceMilestone } from './entities/clearance-milestone.entity'; +import { + ClearanceMilestone, + type RiskAssignmentRecord, +} from './entities/clearance-milestone.entity'; import { Booking } from '../bookings/entities/booking.entity'; import { clearanceCodesForBooking } from '../bookings/clearance.util'; import { ClearanceWorkflowService } from './clearance-workflow.service'; @@ -85,6 +88,8 @@ export interface BookingClearanceView { /** Customs risk level assigned by GL ET (import; visible to the customer). */ riskLevel?: string | null; riskAssignedAt?: string | null; + /** Every risk decision, oldest first; the last entry is the current level. */ + riskHistory?: RiskAssignmentRecord[]; /** Post-arrival additional duty/tax round (import). */ secondDuty?: ClearanceSecondDuty | null; importReleaseGranted?: boolean; @@ -282,6 +287,12 @@ export class BookingClearanceService { riskMilestone?.status === 'COMPLETED' && riskMilestone.triggeredAt ? riskMilestone.triggeredAt.toISOString() : null, + // Every risk decision, oldest first. `riskLevel`/`riskAssignedAt` above are + // the current one; this is the trail behind it. + riskHistory: + riskMilestone?.status === 'COMPLETED' + ? (riskMilestone.metadata?.riskHistory ?? []) + : [], secondDuty, importReleaseGranted: bookingMilestone('IMPORT_RELEASE_GRANTED')?.status === 'COMPLETED', diff --git a/apps/edr-freight-api/src/modules/contracts/clearance-milestone.risk.spec.ts b/apps/edr-freight-api/src/modules/contracts/clearance-milestone.risk.spec.ts index eb77533ac..4ad6dab75 100644 --- a/apps/edr-freight-api/src/modules/contracts/clearance-milestone.risk.spec.ts +++ b/apps/edr-freight-api/src/modules/contracts/clearance-milestone.risk.spec.ts @@ -65,4 +65,80 @@ describe('ClearanceMilestoneService.assignRisk', () => { expect(saved.status).toBe('COMPLETED'); expect(saved.metadata?.riskLevel).toBe('YELLOW'); }); + + /** + * The level is customer-visible and stays correctable until duty is advised, + * so a changed level must leave a trail rather than overwrite the last one. + */ + describe('risk history', () => { + it('records the first assignment with no previous level', async () => { + const { service } = makeService('COMPLETED'); + + const saved = await service.assignRisk('b-1', 'RED', 'user-1', 'initial rating', 'Abebe K.'); + + expect(saved.metadata?.riskHistory).toHaveLength(1); + expect(saved.metadata?.riskHistory?.[0]).toMatchObject({ + level: 'RED', + assignedByUserId: 'user-1', + assignedBy: 'Abebe K.', + note: 'initial rating', + }); + expect(saved.metadata?.riskHistory?.[0]).not.toHaveProperty('previousLevel'); + }); + + it('keeps the earlier decision when the level is reassigned', async () => { + const { service } = makeService('COMPLETED'); + + await service.assignRisk('b-1', 'RED', 'user-1', undefined, 'Abebe K.'); + const saved = await service.assignRisk('b-1', 'GREEN', 'user-2', 'downgraded', 'Sara M.'); + + expect(saved.metadata?.riskLevel).toBe('GREEN'); + expect(saved.metadata?.riskHistory).toHaveLength(2); + // The original RED decision survives, with who made it. + expect(saved.metadata?.riskHistory?.[0]).toMatchObject({ + level: 'RED', + assignedBy: 'Abebe K.', + }); + expect(saved.metadata?.riskHistory?.[1]).toMatchObject({ + level: 'GREEN', + previousLevel: 'RED', + assignedByUserId: 'user-2', + assignedBy: 'Sara M.', + note: 'downgraded', + }); + }); + + it('keeps the whole chain across several reassignments, oldest first', async () => { + const { service } = makeService('COMPLETED'); + + await service.assignRisk('b-1', 'GREEN'); + await service.assignRisk('b-1', 'YELLOW'); + const saved = await service.assignRisk('b-1', 'RED'); + + expect(saved.metadata?.riskHistory?.map((e) => e.level)).toEqual([ + 'GREEN', + 'YELLOW', + 'RED', + ]); + }); + + it('does not record a repeat of the level already assigned', async () => { + const { service } = makeService('COMPLETED'); + + await service.assignRisk('b-1', 'GREEN'); + const saved = await service.assignRisk('b-1', 'GREEN'); + + expect(saved.metadata?.riskHistory).toHaveLength(1); + }); + + it('always leaves riskLevel equal to the last history entry', async () => { + const { service } = makeService('COMPLETED'); + + await service.assignRisk('b-1', 'RED'); + const saved = await service.assignRisk('b-1', 'YELLOW'); + + const history = saved.metadata?.riskHistory ?? []; + expect(saved.metadata?.riskLevel).toBe(history[history.length - 1]?.level); + }); + }); }); diff --git a/apps/edr-freight-api/src/modules/contracts/clearance-milestone.service.ts b/apps/edr-freight-api/src/modules/contracts/clearance-milestone.service.ts index ed3597d57..b6d57263a 100644 --- a/apps/edr-freight-api/src/modules/contracts/clearance-milestone.service.ts +++ b/apps/edr-freight-api/src/modules/contracts/clearance-milestone.service.ts @@ -210,15 +210,50 @@ export class ClearanceMilestoneService { * Customs cannot risk-rate cargo still moving under transit: the T1 must be * closed (accepted by GL Ethiopia after the train arrives) first, which is the * catalog order T1_CLOSED → RISK_ASSIGNED. + * + * The level stays correctable until duty is advised off it, so each assignment + * is appended to `riskHistory` instead of silently replacing the last one — a + * customer-visible level that changes needs a trail of who changed it and when. */ async assignRisk( bookingId: string, riskLevel: CustomsRiskLevel, userId?: string, note?: string, + actor?: string, ): Promise { await this.assertT1Closed(bookingId); - return this.completeWithMetadata(bookingId, 'RISK_ASSIGNED', { riskLevel }, userId, note); + + const existing = await this.repo.findOne({ + where: { bookingId, milestoneCode: 'RISK_ASSIGNED' }, + }); + const previousLevel = existing?.metadata?.riskLevel; + const history = existing?.metadata?.riskHistory ?? []; + + // A repeat of the level already assigned is not a decision — recording it + // would pad the trail with entries that changed nothing. + const entries = + previousLevel === riskLevel + ? history + : [ + ...history, + { + level: riskLevel, + ...(previousLevel ? { previousLevel } : {}), + assignedAt: new Date().toISOString(), + assignedByUserId: userId ?? null, + assignedBy: actor ?? null, + note: note ?? null, + }, + ]; + + return this.completeWithMetadata( + bookingId, + 'RISK_ASSIGNED', + { riskLevel, riskHistory: entries }, + userId, + note, + ); } /** Guard: the booking's T1 must be closed before customs risk can be assigned. */ diff --git a/apps/edr-freight-api/src/modules/contracts/contract-clearance.service.ts b/apps/edr-freight-api/src/modules/contracts/contract-clearance.service.ts index d3bbaf098..d752d071a 100644 --- a/apps/edr-freight-api/src/modules/contracts/contract-clearance.service.ts +++ b/apps/edr-freight-api/src/modules/contracts/contract-clearance.service.ts @@ -18,7 +18,10 @@ import { ClearanceWorkflowService } from './clearance-workflow.service'; import { ClearanceMilestoneService } from './clearance-milestone.service'; import { ContractNotifierService } from './contract-notifier.service'; import { GlOperationsService } from './gl-operations.service'; -import { ClearanceMilestone } from './entities/clearance-milestone.entity'; +import { + ClearanceMilestone, + type RiskAssignmentRecord, +} from './entities/clearance-milestone.entity'; import { Contract } from './entities/contract.entity'; import { ContractDocReviewStatus } from './entities/contract-document-review.entity'; import { FilterContractDto } from './dto/filter-contract.dto'; @@ -102,6 +105,8 @@ export interface ContractClearanceView { /** Customs risk level assigned by GL ET (import; visible to the customer). */ riskLevel?: string | null; riskAssignedAt?: string | null; + /** Every risk decision, oldest first; the last entry is the current level. */ + riskHistory?: RiskAssignmentRecord[]; /** Post-arrival additional duty/tax round (import). */ secondDuty?: ClearanceSecondDuty | null; importReleaseGranted?: boolean; @@ -365,6 +370,11 @@ export class ContractClearanceService { riskMilestone?.status === 'COMPLETED' && riskMilestone.triggeredAt ? riskMilestone.triggeredAt.toISOString() : null, + // Every risk decision, oldest first — see booking-clearance.service. + riskHistory: + riskMilestone?.status === 'COMPLETED' + ? (riskMilestone.metadata?.riskHistory ?? []) + : [], secondDuty, importReleaseGranted: bookingMilestone('IMPORT_RELEASE_GRANTED')?.status === 'COMPLETED', diff --git a/apps/edr-freight-api/src/modules/contracts/contracts.controller.ts b/apps/edr-freight-api/src/modules/contracts/contracts.controller.ts index b62b89e32..cb91fbee6 100644 --- a/apps/edr-freight-api/src/modules/contracts/contracts.controller.ts +++ b/apps/edr-freight-api/src/modules/contracts/contracts.controller.ts @@ -31,6 +31,7 @@ import { ApiTags, } from '@nestjs/swagger'; +import { actorLabel } from '../warehouses/current-actor.util'; import { BookingStaff } from '../../common/booking-guards'; import { FREIGHT_PERMS } from '../../seed/freight-permissions.registry'; import { @@ -968,13 +969,16 @@ export class ContractsController { assignRisk( @Param('bookingId', ParseUUIDPipe) bookingId: string, @Body() dto: AssignRiskDto, - @CurrentUser() user: AuthUserPayload, + @CurrentUser() user: TCurrentUser, ) { return this.milestoneService.assignRisk( bookingId, dto.riskLevel, resolveAuthUserId(user), dto.note, + // Risk history is read by people, so resolve the name now — the id alone + // would render as a UUID in the trail. + actorLabel(user), ); } diff --git a/apps/edr-freight-api/src/modules/contracts/entities/clearance-milestone.entity.ts b/apps/edr-freight-api/src/modules/contracts/entities/clearance-milestone.entity.ts index d4676b8cd..14e3b86dd 100644 --- a/apps/edr-freight-api/src/modules/contracts/entities/clearance-milestone.entity.ts +++ b/apps/edr-freight-api/src/modules/contracts/entities/clearance-milestone.entity.ts @@ -12,14 +12,35 @@ export type MilestoneOwnerRegion = (typeof MILESTONE_OWNER_REGIONS)[number]; export const CUSTOMS_RISK_LEVELS = ['GREEN', 'YELLOW', 'RED'] as const; export type CustomsRiskLevel = (typeof CUSTOMS_RISK_LEVELS)[number]; +/** + * One customs risk decision. Risk stays correctable until duty is advised off + * it, and the level is customer-visible, so every assignment is kept rather than + * overwritten — a disputed level needs to show what was set, by whom, and when. + */ +export interface RiskAssignmentRecord { + level: CustomsRiskLevel; + /** The level this replaced; absent on the first assignment. */ + previousLevel?: CustomsRiskLevel; + assignedAt: string; + assignedByUserId?: string | null; + /** Display name resolved at assignment time, so the trail never shows a UUID. */ + assignedBy?: string | null; + note?: string | null; +} + /** * Structured payload some milestones carry beyond a plain note (doc §11.3): - * - RISK_ASSIGNED → `riskLevel` + * - RISK_ASSIGNED → `riskLevel` (current) + `riskHistory` (every assignment) * - DUTY_TAXES_ADVISED → `dutyAmount`, `dutyCurrency`, `declarationSerial` * Stored on the milestone so the timeline can render the value inline. */ export interface MilestoneMetadata { riskLevel?: CustomsRiskLevel; + /** + * Append-only, oldest first. `riskLevel` is the current value and always + * equals the last entry's `level`. + */ + riskHistory?: RiskAssignmentRecord[]; dutyAmount?: number; dutyCurrency?: string; declarationSerial?: string; diff --git a/apps/edr-freight-web/backoffice/src/components/contracts/PhasedClearanceActionPanel.tsx b/apps/edr-freight-web/backoffice/src/components/contracts/PhasedClearanceActionPanel.tsx index 699706654..9d5dfe65e 100644 --- a/apps/edr-freight-web/backoffice/src/components/contracts/PhasedClearanceActionPanel.tsx +++ b/apps/edr-freight-web/backoffice/src/components/contracts/PhasedClearanceActionPanel.tsx @@ -1,4 +1,4 @@ -import { useMemo, useState } from "react"; +import { useEffect, useMemo, useState } from "react"; import { Alert, Badge, @@ -72,6 +72,7 @@ export type ClearanceViewLike = Pick< | "linkedBookingId" | "riskLevel" | "riskAssignedAt" + | "riskHistory" | "secondDuty" | "importReleaseGranted" > & { operationReady?: boolean }; @@ -1040,11 +1041,28 @@ function RiskStep({ done: boolean; onChanged?: () => void; }) { - const [level, setLevel] = useState("GREEN"); + const assigned = done || Boolean(clearance.riskLevel); + // Duty is advised off the risk level, so once that is done the decision is + // final. Until then a mis-assigned level must stay correctable — the server + // overwrites the milestone metadata on reassignment. Mirrors AssignRiskCard. + const locked = isMilestoneDone(clearance.milestones, "DUTY_TAXES_ADVISED"); + + const [level, setLevel] = useState(clearance.riskLevel ?? "GREEN"); const [loading, setLoading] = useState(false); - if (done || clearance.riskLevel) { - return ( + // The clearance view loads (and refetches after a reassignment) after first + // render, so mirror the persisted level onto the control whenever it changes — + // otherwise reopening the step offers GREEN whatever is actually assigned. + useEffect(() => { + if (clearance.riskLevel) setLevel(clearance.riskLevel); + }, [clearance.riskLevel]); + + // Only the decisions before the current one — the badge above already states + // the level in force, so repeating it as a trail entry reads as a duplicate. + const priorDecisions = (clearance.riskHistory ?? []).slice(0, -1); + + const assignedSummary = assigned ? ( + - ); + {priorDecisions.length > 0 ? ( + + + Previously + + {priorDecisions.map((entry, index) => ( + + {entry.level} + {" · "} + {new Date(entry.assignedAt).toLocaleString()} + {entry.assignedBy ? ` · ${entry.assignedBy}` : ""} + {entry.note ? ` · ${entry.note}` : ""} + + ))} + + ) : null} + + ) : null; + + // Assigned and final: the badge is all that is left to show. + if (assigned && (locked || !canAct || !bookingId)) { + return assignedSummary; } // Customs cannot rate cargo still under transit — the server rejects the - // assignment until the T1 is closed, so do not offer the control yet. - if (!clearance.t1?.closed) { + // assignment until the T1 is closed, so do not offer the control yet. Skipped + // once a level exists: risk cannot have been assigned without a closed T1, so + // a still-open T1 here is stale data and must not hide the assigned badge. + if (!assigned && !clearance.t1?.closed) { return ( + {assignedSummary} - The customer sees the assigned risk level. + {assigned + ? "Correctable until duty is advised. The customer sees the assigned risk level." + : "The customer sees the assigned risk level."} diff --git a/packages/types/src/freight/contracts.ts b/packages/types/src/freight/contracts.ts index 362d3c6ef..299c25dc9 100644 --- a/packages/types/src/freight/contracts.ts +++ b/packages/types/src/freight/contracts.ts @@ -407,6 +407,8 @@ export interface ContractClearanceView { /** Customs risk level assigned by GL ET (import; visible to the customer). */ riskLevel?: string | null; riskAssignedAt?: string | null; + /** Every risk decision, oldest first; the last entry is the current level. */ + riskHistory?: RiskAssignmentRecord[]; /** Post-arrival additional duty/tax round (import). */ secondDuty?: ClearanceSecondDuty | null; importReleaseGranted?: boolean; @@ -440,9 +442,27 @@ export type MilestoneStatus = "PENDING" | "COMPLETED" | "SKIPPED"; export const CUSTOMS_RISK_LEVELS = ["GREEN", "YELLOW", "RED"] as const; export type CustomsRiskLevel = (typeof CUSTOMS_RISK_LEVELS)[number]; +/** + * One customs risk decision. The level stays correctable until duty is advised + * off it and is customer-visible, so every assignment is kept rather than + * overwritten. + */ +export interface RiskAssignmentRecord { + level: CustomsRiskLevel; + /** The level this replaced; absent on the first assignment. */ + previousLevel?: CustomsRiskLevel; + assignedAt: string; + assignedByUserId?: string | null; + /** Display name resolved at assignment time, so the trail never shows a UUID. */ + assignedBy?: string | null; + note?: string | null; +} + /** Structured payload carried by RISK_ASSIGNED / DUTY_TAXES_ADVISED milestones. */ export interface MilestoneMetadata { riskLevel?: CustomsRiskLevel; + /** Every risk decision, oldest first; the last entry matches `riskLevel`. */ + riskHistory?: RiskAssignmentRecord[]; dutyAmount?: number; dutyCurrency?: string; declarationSerial?: string; diff --git a/packages/types/src/freight/index.ts b/packages/types/src/freight/index.ts index 8b50fecba..1dabbca19 100644 --- a/packages/types/src/freight/index.ts +++ b/packages/types/src/freight/index.ts @@ -780,6 +780,8 @@ export interface ClearanceView { /** Customs risk level assigned by GL ET (import; visible to the customer). */ riskLevel?: string | null; riskAssignedAt?: string | null; + /** Every risk decision, oldest first; the last entry is the current level. */ + riskHistory?: import("./contracts").RiskAssignmentRecord[]; /** Post-arrival additional duty/tax round (import). */ secondDuty?: import("./contracts").ClearanceSecondDuty | null; importReleaseGranted?: boolean;