diff --git a/apps/edr-freight-api/src/modules/companies/companies.fayda-identity.spec.ts b/apps/edr-freight-api/src/modules/companies/companies.fayda-identity.spec.ts index 32513df4f..51dd1d61f 100644 --- a/apps/edr-freight-api/src/modules/companies/companies.fayda-identity.spec.ts +++ b/apps/edr-freight-api/src/modules/companies/companies.fayda-identity.spec.ts @@ -280,15 +280,104 @@ describe("Fayda identity verification binds a person to the company", () => { expect(ctx.attributes.poaFaydaSub).toBe("new-sub"); }); - it("refuses to rename a verified person by hand", async () => { - const { service } = makeService({ + it("stages nothing for a verified field an approved company resubmits", async () => { + // Approving it could not move the live row — the verified value is written + // back over it — so it must never reach a reviewer as a pending change. + const { service, deps } = makeService({ + status: CompanyStatus.Active, + attributes: { ...OWNER_VERIFIED, ownerEmail: "abebe@example.com" }, + }); + + await expect( + service.updateProfile("user-1", { + companyEmail: "someone-else@example.com", + } as never), + ).resolves.toBeDefined(); + expect(deps.changeRequestRepo.create).not.toHaveBeenCalled(); + expect(deps.changeRequestRepo.update).not.toHaveBeenCalled(); + expect(deps.companiesRepo.update).not.toHaveBeenCalled(); + }); + + // The verified value wins, and it wins by overwriting rather than by + // rejecting: nobody types these fields, so a submission that disagrees is a + // stale form echoing itself back, not an edit. Failing it would block a save + // the customer never made — and leave them no way through, since re-verifying + // returns the same value they are being 400'd for. + it("overwrites a hand-renamed verified person with the verified name", async () => { + const { service, ctx } = makeService({ attributes: { ...OWNER_VERIFIED, ...POA_VERIFIED }, files: [paper()], }); await expect( service.updateProfile("user-1", { poaName: "Someone Else" } as never), - ).rejects.toBeInstanceOf(BadRequestException); + ).resolves.toBeDefined(); + expect(ctx.attributes.poaName).toBe(POA_VERIFIED.poaName); + }); + + // Fayda's email and phone claims are optional — a verification can prove the + // person and return neither. Holding the company mirrors to "the owner is + // verified" rather than to "the verification supplied this value" would + // clobber the fallbacks the portal is built to send (account email, eTrade's + // registered phone) with nothing at all. OWNER_VERIFIED is exactly that + // shape: a sub, no contact details. + it("keeps company contact details a Fayda verification never supplied", async () => { + const { service, deps } = makeService({ + attributes: { ...OWNER_VERIFIED }, + }); + + await expect( + service.updateProfile("user-1", { + companyEmail: "account@example.com", + companyPhone: "+251911777777", + } as never), + ).resolves.toBeDefined(); + const [, patch] = deps.companiesRepo.update.mock.calls.at(-1)!; + expect(patch.email).toBe("account@example.com"); + expect(patch.phone).toBe("+251911777777"); + }); + + it("overwrites company contact details the verification did supply", async () => { + const { service, deps } = makeService({ + attributes: { + ...OWNER_VERIFIED, + ownerEmail: "abebe@example.com", + ownerPhone: "+251911000000", + }, + }); + + await expect( + service.updateProfile("user-1", { + companyEmail: "someone-else@example.com", + companyPhone: "+251911999999", + } as never), + ).resolves.toBeDefined(); + const [, patch] = deps.companiesRepo.update.mock.calls.at(-1)!; + expect(patch.email).toBe("abebe@example.com"); + expect(patch.phone).toBe("+251911000000"); + }); + + // "Same as owner" copies `ownerEmail ?? null` onto the GM while setting + // `gmFaydaSub`. Locking that null made generalManagerEmail required by + // onboarding, hidden by the portal's link card and unwritable at once. + it("lets the GM's details be typed when the copied owner identity carried none", async () => { + const { service } = makeService({ + attributes: { + ...OWNER_VERIFIED, + gmSameAsOwner: true, + gmFaydaSub: "owner-sub", + generalManagerName: "Abebe Bikila", + generalManagerEmail: null, + generalManagerPhone: null, + }, + }); + + await expect( + service.updateProfile("user-1", { + generalManagerEmail: "gm@example.com", + generalManagerPhone: "+251911888888", + } as never), + ).resolves.toBeDefined(); }); it("never locks or gates the general manager — it is not the verified subject", async () => { diff --git a/apps/edr-freight-api/src/modules/companies/companies.service.ts b/apps/edr-freight-api/src/modules/companies/companies.service.ts index f21263c7f..b5c953cac 100644 --- a/apps/edr-freight-api/src/modules/companies/companies.service.ts +++ b/apps/edr-freight-api/src/modules/companies/companies.service.ts @@ -739,6 +739,39 @@ export class CompaniesService { return out; } + /** + * `UpdateProfileDto` keys this company's completed verifications own — the + * ones `mapProfileDtoToCompanyUpdates` overwrites with the verified value + * whatever a request submits for them. + * + * A key only lands here once there is a verified value to hold it to: Fayda's + * email and phone claims are optional, and a verification that returned + * neither owns nothing to overwrite with. + * + * The map is the enforcement; this is the list used to keep those keys out of + * a change request in the first place. If the two ever drift the map still + * wins — the cost is a staged field that approving turns out not to move. + */ + private faydaOwnedKeys(company: Company): string[] { + const attrs = company.attributes ?? {}; + const held = (key: string) => { + const v = attrs[key]; + return v !== null && v !== undefined && v !== ""; + }; + + const keys: string[] = []; + if (attrs.ownerFaydaSub) { + // The Company-column mirrors of the owner's verified contact details. + if (held("ownerEmail")) keys.push("companyEmail"); + if (held("ownerPhone")) keys.push("companyPhone"); + } + for (const subject of IDENTITY_SUBJECTS) { + if (!attrs[`${IDENTITY_PREFIX[subject]}FaydaSub`]) continue; + keys.push(...IDENTITY_OWNED_FIELDS[subject].filter(held)); + } + return keys; + } + /** * Translate an UpdateProfileDto (or a staged change-request snapshot) into a * `Company` patch: scalar columns plus a merged `attributes` blob (contact/GM/ @@ -829,49 +862,46 @@ export class CompaniesService { // lets the customer type them once verified) — lock them the same way // ownerEmail/ownerPhone themselves are locked below, once there is a // verified owner to lock them to. + // + // Keyed on the verified VALUE, not on `ownerFaydaSub`: Fayda's email and + // phone claims are optional, so a verification can prove the person while + // supplying neither (see completeIdentityVerification's conditional + // spreads). The portal falls back to the account email / eTrade's + // registered phone in exactly that case and submits it on every save of + // the company step — locking against an absent value would 400 that + // forever, and re-verifying could never clear it because Fayda still has + // nothing to return. if (attrUpdates.ownerFaydaSub) { - if ( - dto.companyEmail !== undefined && - dto.companyEmail !== attrUpdates.ownerEmail - ) { - throw new BadRequestException( - "companyEmail is set by the owner's Fayda verification and cannot be edited. Re-verify to change it.", - ); - } - if ( - dto.companyPhone !== undefined && - normalizeE164(dto.companyPhone) !== - normalizeE164(String(attrUpdates.ownerPhone ?? "")) - ) { - throw new BadRequestException( - "companyPhone is set by the owner's Fayda verification and cannot be edited. Re-verify to change it.", - ); - } + if (attrUpdates.ownerEmail && dto.companyEmail !== undefined) + companyUpdates.email = attrUpdates.ownerEmail; + if (attrUpdates.ownerPhone && dto.companyPhone !== undefined) + companyUpdates.phone = normalizeE164(String(attrUpdates.ownerPhone)); } // Renaming a Fayda-verified person by hand would launder the guarantee - // away, so the fields the verification owns are refused once it exists. + // away, so the verification keeps these fields: a submission that disagrees + // is overwritten with the verified value rather than rejected — the same + // doctrine `applyEtradeSourcedFields` uses for eTrade's fields, and for the + // same reason. The customer never types these (the portal derives them, and + // a stale form or a re-render can echo back something else entirely), so a + // 400 punishes a save they never made while an overwrite lands the truth. for (const subject of IDENTITY_SUBJECTS) { if (!attrUpdates[`${IDENTITY_PREFIX[subject]}FaydaSub`]) continue; for (const field of IDENTITY_OWNED_FIELDS[subject]) { - const incoming = (dto as Record)[field]; - if (incoming === undefined) continue; - // The verification itself is allowed to write them; anything else is - // compared against what is already stored, not against the value this - // same call just copied into the patch. Phones are compared normalized: - // a form that re-renders +251911000000 as 0911000000 is echoing the - // stored value back, not trying to change it. + if ((dto as Record)[field] === undefined) continue; + // The verification itself is what writes them; it must not be undone by + // the value this same call just copied into the patch. if (dto.faydaIdentity && field in dto.faydaIdentity) continue; const stored = company.attributes?.[field]; - const same = field.endsWith("Phone") - ? normalizeE164(String(incoming)) === - normalizeE164(String(stored ?? "")) - : incoming === stored; - if (!same) { - throw new BadRequestException( - `${field} is set by the Fayda verification of this company's ${IDENTITY_LABEL[subject]} and cannot be edited. Re-verify to change it.`, - ); - } + // A verification that supplied nothing for this field left no guarantee + // to protect, so it stays typeable. Matters most for the GM — + // `setGmSameAsOwner` copies `ownerEmail ?? null` onto + // `generalManagerEmail` while setting `gmFaydaSub`, and + // REQUIRED_COMPANY_INFO still demands that email, so holding a null + // here makes it required, hidden by the portal's "same as owner" card, + // and unwritable all at once. + if (stored === null || stored === undefined || stored === "") continue; + attrUpdates[field] = stored; } } @@ -977,6 +1007,11 @@ export class CompaniesService { // for review with the live row left intact. await this.assertTinAvailable(company, dto.tin); const fields = this.pickDefined(dto); + // Drop what the verifications own before anything is staged. Approving one + // of these could not change the live row — mapProfileDtoToCompanyUpdates + // writes the verified value back over it — so showing it to a reviewer + // asks them to rule on a change that does not exist. + for (const key of this.faydaOwnedKeys(company)) delete fields[key]; const selfService: Record = {}; const staged: Record = {}; for (const [key, value] of Object.entries(fields)) { diff --git a/apps/edr-freight-web/backoffice/.env.example b/apps/edr-freight-web/backoffice/.env.example index c840d18a1..454817139 100644 --- a/apps/edr-freight-web/backoffice/.env.example +++ b/apps/edr-freight-web/backoffice/.env.example @@ -1,3 +1,6 @@ +# Dev server port. Default: 5283. +PORT=5283 + VITE_API_URL=http://localhost:3001 VITE_BASE_API_URL=http://localhost:3001 diff --git a/apps/edr-freight-web/backoffice/package.json b/apps/edr-freight-web/backoffice/package.json index 675eced1a..68fdb2a8f 100644 --- a/apps/edr-freight-web/backoffice/package.json +++ b/apps/edr-freight-web/backoffice/package.json @@ -4,7 +4,7 @@ "private": true, "type": "module", "scripts": { - "dev": "vite --port 5183 --clearScreen false", + "dev": "vite --clearScreen false", "prebuild": "node -e \"const fs=require('fs'); fs.rmSync('dist',{recursive:true,force:true});\"", "build": "vite build", "preview": "vite preview --port 5183", diff --git a/apps/edr-freight-web/backoffice/vite.config.ts b/apps/edr-freight-web/backoffice/vite.config.ts index 6e7326189..63735d72e 100644 --- a/apps/edr-freight-web/backoffice/vite.config.ts +++ b/apps/edr-freight-web/backoffice/vite.config.ts @@ -2,6 +2,7 @@ import path from "node:path"; import { fileURLToPath } from "node:url"; import { createRequire } from "node:module"; +import { loadEnv } from "vite"; import { defineConfig } from "vitest/config"; import react from "@vitejs/plugin-react"; import tailwindcss from "@tailwindcss/vite"; @@ -10,7 +11,9 @@ const __dirname = path.dirname(fileURLToPath(import.meta.url)); const require = createRequire(import.meta.url); const streamBrowserifyPath = require.resolve("stream-browserify"); -export default defineConfig(() => { +export default defineConfig(({ mode }) => { + const env = loadEnv(mode, __dirname, ""); + return { plugins: [react(), tailwindcss()], resolve: { @@ -31,7 +34,7 @@ export default defineConfig(() => { dedupe: ["react", "react-dom", "@mantine/core", "@mantine/hooks"], }, server: { - port: 5183, + port: Number(env.PORT) || 5283, host: "0.0.0.0", }, test: { diff --git a/apps/edr-freight-web/portal/package.json b/apps/edr-freight-web/portal/package.json index f3b493aca..4bb96ee4f 100644 --- a/apps/edr-freight-web/portal/package.json +++ b/apps/edr-freight-web/portal/package.json @@ -4,7 +4,7 @@ "private": true, "type": "module", "scripts": { - "dev": "vite --port 3000 --clearScreen false", + "dev": "vite --clearScreen false", "build": "tsc -b && vite build", "preview": "vite preview --port 5173", "lint": "eslint src", diff --git a/apps/edr-freight-web/portal/vite.config.ts b/apps/edr-freight-web/portal/vite.config.ts index 99e2a41d5..89483c7dd 100644 --- a/apps/edr-freight-web/portal/vite.config.ts +++ b/apps/edr-freight-web/portal/vite.config.ts @@ -1,6 +1,7 @@ import path from "node:path"; import { fileURLToPath } from "node:url"; +import { loadEnv } from "vite"; import { defineConfig } from "vitest/config"; import react from "@vitejs/plugin-react"; import tailwindcss from "@tailwindcss/vite"; @@ -13,26 +14,30 @@ const __dirname = path.dirname(fileURLToPath(import.meta.url)); const mantineCore = path.resolve(__dirname, "node_modules/@mantine/core"); const mantineHooks = path.resolve(__dirname, "node_modules/@mantine/hooks"); -export default defineConfig({ - plugins: [react(), tailwindcss()], - resolve: { - alias: { - "@": path.resolve(__dirname, "./src"), - // Resolve from TS source so Vite gets ESM named exports (dist is CommonJS). - "@edr/types": path.resolve(__dirname, "../../../packages/types/src/index.ts"), - "@mantine/core": mantineCore, - "@mantine/hooks": mantineHooks, +export default defineConfig(({ mode }) => { + const env = loadEnv(mode, __dirname, ""); + + return { + plugins: [react(), tailwindcss()], + resolve: { + alias: { + "@": path.resolve(__dirname, "./src"), + // Resolve from TS source so Vite gets ESM named exports (dist is CommonJS). + "@edr/types": path.resolve(__dirname, "../../../packages/types/src/index.ts"), + "@mantine/core": mantineCore, + "@mantine/hooks": mantineHooks, + }, + dedupe: ["react", "react-dom", "@mantine/core", "@mantine/hooks"], }, - dedupe: ["react", "react-dom", "@mantine/core", "@mantine/hooks"], - }, - optimizeDeps: { - include: ["@mantine/core", "@mantine/hooks", "@edr/ui-common"], - }, - server: { - port: 5173, - host: "0.0.0.0", - }, - test: { - environment: "node", - }, + optimizeDeps: { + include: ["@mantine/core", "@mantine/hooks", "@edr/ui-common"], + }, + server: { + port: Number(env.PORT) || 5273, + host: "0.0.0.0", + }, + test: { + environment: "node", + }, + }; });