From 72164b0b8e55851d85867849a60e531f130bf0ff Mon Sep 17 00:00:00 2001 From: Nathnael Date: Tue, 11 Aug 2026 12:54:38 +0000 Subject: [PATCH] fix(freight-portal): stop owner details vanishing between wizard steps MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit SourcedField rendered read-only whenever a value existed, so the input a customer had just typed into turned into a badge as soon as the step saved and they navigated back — and dropped out of requiredKeys at the same time. It now locks on ownership instead: a Fayda verification owns what its claims filled, everything else stays an editable, prefilled input. The representation step follows the same rule and asks in the right order: - The power-of-attorney question collapses to its answer once given, with a button back to it (none for a freight forwarder, whose answer is forced). - A foreign company picks how to prove the person outright — Fayda or a passport — rather than being shown both at once. - The representative's own fields appear only once the person is established, and only for what the verification did not supply; what it did supply is already on the panel above and is no longer repeated beneath it. - Dropped the freight-forwarder lecture and the DARS blurb; the badge and the upload field's own help text already say both. VAT numbers accept any non-blank value. A foreign tax authority's carries letters and dashes and a co-operative's follows neither pattern, so the 10-11 digit rule only ever rejected numbers we had no business judging. --- .../companies/dto/update-profile.dto.ts | 12 +- .../companyProfileForm/SourcedField.tsx | 34 +- .../companyProfileForm/steps/OwnerStep.tsx | 67 +-- .../steps/RepresentationStep.tsx | 442 +++++++++++------- .../portal/src/pages/settings/TabOwner.tsx | 30 +- .../src/pages/settings/TabPowerOfAttorney.tsx | 8 +- 6 files changed, 363 insertions(+), 230 deletions(-) diff --git a/apps/edr-freight-api/src/modules/companies/dto/update-profile.dto.ts b/apps/edr-freight-api/src/modules/companies/dto/update-profile.dto.ts index b7b814da1..2273bf79a 100644 --- a/apps/edr-freight-api/src/modules/companies/dto/update-profile.dto.ts +++ b/apps/edr-freight-api/src/modules/companies/dto/update-profile.dto.ts @@ -5,7 +5,6 @@ import { MaxLength, IsEnum, IsIn, - Matches, } from "class-validator"; import { ETHIOPIAN_REGIONS, type EthiopianRegion } from "@edr/types"; import { CompanyNationality } from "../entities/company.entity"; @@ -36,13 +35,14 @@ export class UpdateProfileDto { @IsTin({ message: "TIN must be exactly 10 digits" }) tin?: string; - // Ethiopian VAT registration numbers are 10 digits (the same shape as the - // TIN), but some are issued with an 11th. Both portal forms enforce the same - // range; without it here the API happily stored whatever a stale client sent, - // and the two layers disagreed about what the column may hold. + // No shape check. Ethiopian VAT numbers are usually 10 or 11 digits, but a + // foreign company's is whatever its own tax authority issues — letters, + // dashes and any length — and a co-operative's registration numbering does + // not follow the trade-licence pattern either. The field is required (the + // portal enforces non-blank) but its content is not ours to police. @IsOptional() @IsString() - @Matches(/^\d{10,11}$/, { message: "VAT number must be 10 or 11 digits" }) + @MaxLength(64) vatNumber?: string; // `fanNumber` is deliberately absent: the FAN is a Fayda number, so it would diff --git a/apps/edr-freight-web/portal/src/pages/accounts/companyProfileForm/SourcedField.tsx b/apps/edr-freight-web/portal/src/pages/accounts/companyProfileForm/SourcedField.tsx index 480606db8..3b3601711 100644 --- a/apps/edr-freight-web/portal/src/pages/accounts/companyProfileForm/SourcedField.tsx +++ b/apps/edr-freight-web/portal/src/pages/accounts/companyProfileForm/SourcedField.tsx @@ -1,7 +1,7 @@ import { Badge, Group, Stack, Text } from "@mantine/core"; import type { ReactNode } from "react"; -/** Where a prefilled value came from, shown as a badge next to it. */ +/** Where a locked value came from, shown as a badge next to it. */ export type FieldSource = "eTrade" | "Fayda"; const SOURCE_NOTE: Record = { @@ -10,35 +10,37 @@ const SOURCE_NOTE: Record = { }; /** - * One person-detail field that may already be answered for us. + * One person-detail field that a verification may have taken ownership of. * - * The onboarding wizard fills what it can from the eTrade lookup and the Fayda - * verification, and asks the customer only for what neither supplied. Both - * sources are patchy in practice — eTrade returns no email at all and often no - * manager name; Fayda's email and phone claims are optional and routinely come - * back empty — so "what is missing" varies per company and cannot be decided - * once at build time. + * `locked` — not "does a value exist" — decides which side renders. That + * distinction is the whole point: `ownerName` holds a value the moment the + * eTrade lookup prefills it or the customer types it and the step saves, and + * keying off presence meant the input a customer had just filled in turned into + * a read-only badge as soon as they navigated away and back, with no way to + * correct it. Only a Fayda verification actually owns a field — the API refuses + * to overwrite those — so only those lock. * - * This is the single place that decision is rendered: a supplied value shows - * read-only with its provenance, a gap shows the input. It pairs with - * `requiredKeys` in CompanyProfileForm, which requires exactly the fields that - * fall through to `children` — the invariant being that **a field is required - * if and only if there is an input on screen to satisfy it**. + * It pairs with `requiredKeys` in CompanyProfileForm, which requires exactly + * the fields that fall through to `children`: **a field is required if and only + * if there is an input on screen to satisfy it.** */ export default function SourcedField({ label, value, source, + locked, children, }: { label: string; - /** The value a source supplied. Blank/absent means "ask the customer". */ + /** The value to display when locked. */ value?: string | null; source: FieldSource; - /** The input rendered when no source supplied a value. */ + /** A verification owns this field: show it read-only instead of an input. */ + locked: boolean; + /** The input rendered whenever the field is still the customer's to fill. */ children: ReactNode; }) { - if (!value?.trim()) return <>{children}; + if (!locked || !value?.trim()) return <>{children}; return ( diff --git a/apps/edr-freight-web/portal/src/pages/accounts/companyProfileForm/steps/OwnerStep.tsx b/apps/edr-freight-web/portal/src/pages/accounts/companyProfileForm/steps/OwnerStep.tsx index 7c7c05fec..7861f4818 100644 --- a/apps/edr-freight-web/portal/src/pages/accounts/companyProfileForm/steps/OwnerStep.tsx +++ b/apps/edr-freight-web/portal/src/pages/accounts/companyProfileForm/steps/OwnerStep.tsx @@ -14,12 +14,15 @@ export interface OwnerStepProps { /** eTrade's registered manager, once a TIN lookup has succeeded. */ etradeOwner: { name: string; phone: string } | null; /** - * Which of the owner's details neither eTrade nor Fayda supplied, and are - * therefore typed here. Computed by CompanyProfileForm, which requires - * exactly these in the schema — so every input below is one the customer is - * actually asked to fill, and nothing is required that has no input. + * Which of the owner's details a Fayda verification owns. A locked field is + * shown read-only; every other one is an editable input, prefilled from + * eTrade or from what was saved earlier. CompanyProfileForm computes this and + * requires exactly the unlocked fields, so every input on screen is one the + * customer is actually asked to fill and nothing is required that has none. */ - gaps: { name: boolean; email: boolean; phone: boolean }; + locked: { name: boolean; email: boolean; phone: boolean }; + /** A co-operative union or farm: no licence, so no eTrade record to match. */ + cooperative?: boolean; } /** @@ -32,12 +35,17 @@ export interface OwnerStepProps { * onboarding is often not the person on the licence, and stamping their name, * email and phone onto the owner turned three required fields into a guess * wearing the licence's authority. + * + * A co-operative union or farm has no licence, so there is nobody named on one + * — the owner is simply the person who runs it, typed in full and compared + * against nothing. */ export default function OwnerStep({ form, identity, etradeOwner, - gaps, + locked, + cooperative = false, }: OwnerStepProps) { const { register, @@ -45,16 +53,7 @@ export default function OwnerStep({ formState: { errors }, } = form; - // A verified owner's own claims outrank eTrade's record for display: the - // government IdP is the higher-trust source, and the API locks those fields - // to it. eTrade still supplies the name and phone when there is no - // verification — which is the case for every company represented by a PoA. const ownerVerified = identity?.owner.verified ?? false; - const nameValue = identity?.owner.name || etradeOwner?.name || ""; - const phoneValue = identity?.owner.phone || etradeOwner?.phone || ""; - const emailValue = identity?.owner.email || ""; - const nameSource = identity?.owner.name ? "Fayda" : "eTrade"; - const phoneSource = identity?.owner.phone ? "Fayda" : "eTrade"; // A Fayda verification that names someone other than the person on the // licence is the one thing this step exists to catch. Advisory here — the two @@ -65,12 +64,12 @@ export default function OwnerStep({ return ( - These are the details of the person registered on your eTrade licence. - We fill in whatever eTrade and Fayda gave us; anything they left blank - we need from you. + {cooperative + ? "The person who runs the co-operative union or farm. We have no licence record to fill these in from, so we need all of them from you." + : "These are the details of the person registered on your eTrade licence. We fill in whatever eTrade and Fayda gave us; anything they left blank we need from you."} - {!etradeOwner && !ownerVerified && ( + {!cooperative && !etradeOwner && !ownerVerified && ( }> Your eTrade licence didn't list a manager, so there's nothing for us to prefill. Enter the details of the person registered on it. @@ -84,15 +83,19 @@ export default function OwnerStep({ icon={} title="This doesn't match your eTrade licence" > - Your licence lists{" "} - {identity?.etradeManagerName}, but the name here is{" "} - {nameValue}. You can continue, but our team will - check this before approving your account — so make sure it's the - person the licence actually names. + Your licence lists {identity?.etradeManagerName}, but + the name here is {identity?.owner.name}. You can + continue, but our team will check this before approving your account — + so make sure it's the person the licence actually names. )} - + - + {/* eTrade never returns an email for the manager and Fayda's email claim is optional, so this is the field most companies actually type — it is required either way (`REQUIRED_COMPANY_INFO`). */} - + ; identity?: CompanyIdentityState; @@ -32,11 +36,26 @@ export interface RepresentationStepProps { /** A declaration change is in flight. */ declarePending: boolean; /** - * Which of the representative's details the Fayda verification did not - * supply. Same contract as OwnerStep's `gaps`: an input is rendered for - * exactly these, and the schema requires exactly these. + * The answer is not the company's to change — it operates as a freight + * forwarder, which the API forces to "yes". No alert says so: the summary + * simply offers no way back, which is the same information without the + * lecture. */ - gaps: { name: boolean; email: boolean; phone: boolean; address: boolean }; + declarationLocked: boolean; + /** + * Which of the representative's details the Fayda verification owns. Same + * contract as OwnerStep's `locked`: a locked field is read-only, every other + * one is an input, and the schema requires exactly the unlocked ones. + */ + locked: { name: boolean; email: boolean; phone: boolean; address: boolean }; + /** + * How a foreign company is proving the subject. Null until it picks — the + * either/or is a fork, not a fallback, so nothing below it renders until one + * side is chosen. Always "fayda" for an Ethiopian company, which has no + * choice to make. + */ + method: IdentityMethod | null; + onMethodChange: (method: IdentityMethod) => void; /** Single-field upload setting carrying just the DARS delegation letter. */ poaDocumentSetting?: FileUploadSetting; documentFiles: Record; @@ -64,7 +83,10 @@ export default function RepresentationStep({ identity, onDeclare, declarePending, - gaps, + declarationLocked, + locked, + method, + onMethodChange, poaDocumentSetting, documentFiles, uploadedDocumentKeys, @@ -86,184 +108,220 @@ export default function RepresentationStep({ } const declared = identity.poaDeclared; - const locked = identity.poaDeclared === "yes" && identity.subject === "poa"; - // Only the API knows whether the lock is the freight-forwarder rule; it - // reports the answer as "yes" for them no matter what is stored, so a company - // that cannot switch to "no" is one the API will refuse. Rather than - // duplicating the role check here, the "no" card simply reports the refusal. const passportAccepted = identity.passportAccepted; + const subject = declared === "yes" ? identity.poa : identity.owner; + const verified = subject.verified; + + // Nothing below the fork renders until the person is actually established: + // a Fayda claim that came back, or the passport path deliberately chosen. + // Asking for a name before the verification runs is asking for a value the + // verification is about to overwrite. + const established = verified || method === "passport"; + + const who = declared === "yes" ? "Representative" : "Owner"; + const passportField = + declared === "yes" ? "poaPassportNumber" : "ownerPassportNumber"; return ( - - - Does anyone hold power of attorney for this company? - - - Your answer decides whose identity we verify — the representative's, - or the owner's. - - - - - } - selected={declared === "yes"} - onClick={ - declarePending || declared === "yes" - ? undefined - : () => onDeclare("yes") - } - /> - } - selected={declared === "no"} - onClick={ - declarePending || declared === "no" - ? undefined - : () => onDeclare("no") - } - /> - - - {locked && ( - }> - As a freight forwarder you act on other companies' behalf, so a Power - of Attorney is required — this can't be set to "no" while you hold the - freight forwarder role. - - )} - - {declared === null && ( - - Pick one to continue. - - )} - - {/* ---------------------------------------------------------------- */} - {/* No representative → the owner is the one who verifies. */} - {/* ---------------------------------------------------------------- */} - {declared === "no" && ( + {/* ------------------------------------------------------------------ */} + {/* The question. Once answered it collapses to its answer, so the step */} + {/* is about the person rather than re-presenting a settled choice. */} + {/* ------------------------------------------------------------------ */} + {declared === null ? ( <> - - - {/* Fayda is an Ethiopian national ID, so a foreign company's owner - may hold none — a passport number proves them instead. Offered - alongside, not after: either one satisfies the gate. */} - {passportAccepted && !identity.owner.verified && ( - + + Does anyone hold power of attorney for this company? + + + Your answer decides whose identity we verify — the + representative's, or the owner's. + + + + + } + selected={false} + onClick={declarePending ? undefined : () => onDeclare("yes")} /> - )} - - )} - - {/* ---------------------------------------------------------------- */} - {/* A representative → they verify, and the delegation is evidenced. */} - {/* ---------------------------------------------------------------- */} - {declared === "yes" && ( - <> - - - {passportAccepted && !identity.poa.verified && ( - } + selected={false} + onClick={declarePending ? undefined : () => onDeclare("no")} /> - )} - - {/* Whatever the Fayda claim carried is shown on the panel above and - never typed here — the verification owns it. The rest is asked - for outright, because the API demands name/email/phone from any - declared representative (`REQUIRED_POA_FIELDS`). */} - - - - - - - - - - - - + + ) : ( + : } + label={ + declared === "yes" + ? "A representative holds power of attorney" + : "The owner acts for the company" + } + detail={ + declared === "yes" + ? "We'll verify their identity and ask for the DARS delegation paper." + : "Nobody holds power of attorney, so we verify the owner." + } + onChange={ + declarePending || declarationLocked + ? undefined + : () => onDeclare(declared === "yes" ? "no" : "yes") + } + changeLabel={declared === "yes" ? "We have no representative" : "We have a representative"} + /> + )} - {gaps.address && ( - + {declared !== null && ( + <> + + + {/* -------------------------------------------------------------- */} + {/* How the person is proved. Ethiopian: Fayda, no choice. Foreign: */} + {/* Fayda or a passport — one or the other, picked outright. */} + {/* -------------------------------------------------------------- */} + {passportAccepted && !verified && method === null ? ( + + + How would you like to prove {who.toLowerCase()}'s identity? + + + Either one is enough — you don't need both. + + + } + selected={false} + onClick={() => onMethodChange("fayda")} + /> + } + selected={false} + onClick={() => onMethodChange("passport")} + /> + + + ) : ( + <> + {(method === "fayda" || !passportAccepted || verified) && ( + + )} + + {passportAccepted && !verified && method === "passport" && ( + + )} + + {passportAccepted && !verified && method !== null && ( + + )} + )} - {poaDocumentSetting && ( + {/* -------------------------------------------------------------- */} + {/* The representative's own details — only once the person exists, */} + {/* and only the parts the verification did not already carry. What */} + {/* Fayda supplied is shown on the panel above and never repeated. */} + {/* -------------------------------------------------------------- */} + {declared === "yes" && established && ( <> - - - Upload the delegation paper authenticated by DARS. It is what - evidences that this person was actually delegated. - - + + + + + + + + + + + + + + + {!locked.address && ( + + )} + + {poaDocumentSetting && ( + <> + + + + )} )} @@ -271,3 +329,41 @@ export default function RepresentationStep({ ); } + +/** A settled choice, shown as its answer with a way back to the question. */ +function ChoiceSummary({ + icon, + label, + detail, + onChange, + changeLabel, +}: { + icon: React.ReactNode; + label: string; + detail: string; + onChange?: () => void; + changeLabel: string; +}) { + return ( + + + + {icon} + + + {label} + + + {detail} + + + + {onChange && ( + + )} + + + ); +} diff --git a/apps/edr-freight-web/portal/src/pages/settings/TabOwner.tsx b/apps/edr-freight-web/portal/src/pages/settings/TabOwner.tsx index 2c03aaed3..9399c5efc 100644 --- a/apps/edr-freight-web/portal/src/pages/settings/TabOwner.tsx +++ b/apps/edr-freight-web/portal/src/pages/settings/TabOwner.tsx @@ -69,6 +69,15 @@ export default function TabOwner({ // Only the person the declaration points at carries the verification, so the // panel is offered here only when that person is the owner. const ownerIsSubject = identity?.subject === "owner"; + // A Fayda verification owns what its claims filled — the API refuses to + // overwrite those, so they show read-only. Anything it left blank stays + // editable here, whatever value is currently stored. + const ownerVerified = owner?.verified ?? false; + const ownerLocked = { + name: ownerVerified && Boolean(owner?.name?.trim()), + email: ownerVerified && Boolean(owner?.email?.trim()), + phone: ownerVerified && Boolean(owner?.phone?.trim()), + }; const { register, @@ -147,7 +156,12 @@ export default function TabOwner({ /> )} - + - + - + - {requirePoa - ? "As a freight forwarder you act on other companies' behalf, so a Power of Attorney is required." - : "Tell us whether anyone is authorised to act for the company — your answer decides whose identity we verify."}{" "} + {/* The "Required for freight forwarder" badge above already says why + a forwarder has no choice here; repeating it in prose was a + lecture, not information. */} + {!requirePoa && + "Tell us whether anyone is authorised to act for the company — your answer decides whose identity we verify. "} {declared === "no" ? "The owner acts for the company, so no delegation paper is needed." : "A representative must be identified, and the DARS delegation paper authorising them uploaded."}