From 3d53f671563a00801cac3710ca7e5688b95844e7 Mon Sep 17 00:00:00 2001 From: Nathnael Date: Mon, 10 Aug 2026 06:46:25 +0000 Subject: [PATCH 1/2] fix: gm and poa email requirement --- .../booking-lifecycle-notifier.service.ts | 13 +- .../companies.fayda-identity.spec.ts | 164 +++++++++++++++++- .../modules/companies/companies.service.ts | 36 ++-- .../companies/company-notifier.service.ts | 11 +- .../dto/complete-identity-verification.dto.ts | 8 +- .../contracts/contract-notifier.service.ts | 11 +- .../notifications/notify-company.util.ts | 4 +- .../resolve-company-phone.util.ts | 58 +++++-- .../booking-notifier.service.ts | 11 +- .../booking-window.service.ts | 5 +- .../src/pages/accounts/CompanyProfileForm.tsx | 110 ++++++++++-- .../companyProfileForm/schema.test.ts | 52 +++++- .../accounts/companyProfileForm/schema.ts | 43 ++++- .../steps/PersonnelStep.tsx | 35 ++++ .../companyProfileForm/steps/PoaStep.tsx | 33 ++-- .../src/pages/settings/TabGeneralManager.tsx | 50 +++++- 16 files changed, 556 insertions(+), 88 deletions(-) diff --git a/apps/edr-freight-api/src/modules/bookings/booking-lifecycle-notifier.service.ts b/apps/edr-freight-api/src/modules/bookings/booking-lifecycle-notifier.service.ts index 90b020472..eff15ec3d 100644 --- a/apps/edr-freight-api/src/modules/bookings/booking-lifecycle-notifier.service.ts +++ b/apps/edr-freight-api/src/modules/bookings/booking-lifecycle-notifier.service.ts @@ -10,7 +10,7 @@ import { import { Booking } from './entities/booking.entity'; import { NotificationsService } from '../notifications/notifications.service'; import { NotificationInboxService } from '../notification-inbox/notification-inbox.service'; -import { resolveCompanyNotifyPhone } from '../notifications/resolve-company-phone.util'; +import { resolveCompanyNotifyContact } from '../notifications/resolve-company-phone.util'; import { FREIGHT_PERMS } from '../../seed/freight-permissions.registry'; /** @@ -55,10 +55,13 @@ export class BookingLifecycleNotifierService { logLabel: string, ): Promise { this.logger.log(`${logLabel} — ${this.ref(b)}`); - const phone = b.companyId - ? await resolveCompanyNotifyPhone(this.dataSource, b.companyId) - : null; - const email = b.company?.email ?? b.company?.generalManagerEmail ?? null; + // Both channels come from the same resolver: the company row's own columns + // are only half the story (see companyNotifyEmailExpr), and reading them off + // the loaded entity silently dropped every mail to a company whose address + // lives in `attributes`. + const { phone, email } = b.companyId + ? await resolveCompanyNotifyContact(this.dataSource, b.companyId) + : { phone: null, email: null }; if (phone) { try { 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 599187b22..4a53915ec 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 @@ -147,6 +147,10 @@ function makeService(overrides: Partial = {}) { verifayda: { completeVerification: jest.fn(async () => ctx.verification), }, + // Onboarding requirements resolve the nationality's document set through + // this; no setting means no company documents, which keeps those cases + // focused on the identity/PoA half of the list. + fileUploadSettings: { getByCode: jest.fn(async () => null) }, }; const service = new CompaniesService( @@ -157,7 +161,7 @@ function makeService(overrides: Partial = {}) { deps.profilesRepo as never, {} as never, deps.filesService as never, - {} as never, + deps.fileUploadSettings as never, {} as never, deps.companyNotifier as never, {} as never, @@ -763,6 +767,73 @@ describe("Ethiopian companies verify with Fayda; foreign companies verify identi expect(state.gm.email).toBe("legacy@example.com"); }); + // The mirror image, and the reason the fallback above is gated on the GM + // being unverified: a manager Fayda proved but supplied no email for types + // one instead, and the portal decides whether to render that input by asking + // whether the identity holds one. Reading the typed column back as part of + // the verified identity would answer "yes" the moment it was saved — the + // input would vanish and a typo could never be corrected. + it("keeps a verified GM's typed email out of the verified identity", async () => { + const { service, company } = makeService({ + attributes: { + ...OWNER_VERIFIED, + gmFaydaSub: "gm-sub", + gmFaydaVerifiedAt: "2026-07-03T00:00:00.000Z", + gmName: "Derartu Tulu", + generalManagerName: "Derartu Tulu", + generalManagerEmail: "typed@example.com", + }, + }); + + const state = service.getCompanyIdentityState(company() as never); + + expect(state.gm.verified).toBe(true); + expect(state.gm.name).toBe("Derartu Tulu"); + expect(state.gm.email).toBeNull(); + }); + + it("never stands the account in for a GM Fayda gave no email", async () => { + // Deliberate: the account is the person onboarding, not necessarily the + // manager. The portal asks for the email instead. + const { service, ctx } = makeService({ + attributes: { ...OWNER_VERIFIED }, + verification: { + purpose: "VERIFY", + verified: true, + sub: "gm-sub", + fullName: "Derartu Tulu", + phoneNumber: "+251911222333", + }, + }); + + const state = await service.completeIdentityVerification( + "user-1", + { subject: "gm", code: "c", state: "s" }, + { email: "account@example.com", phoneNumber: "+251911777777" }, + ); + + expect(ctx.attributes.gmEmail).toBeUndefined(); + expect(state.gm.verified).toBe(true); + expect(state.gm.email).toBeNull(); + }); + + it("accepts the email typed for a GM whose verification carried none", async () => { + const { service, ctx } = makeService({ + attributes: { + ...OWNER_VERIFIED, + gmFaydaSub: "gm-sub", + gmName: "Derartu Tulu", + generalManagerName: "Derartu Tulu", + }, + }); + + await service.updateProfile("user-1", { + generalManagerEmail: "gm@example.com", + } as never); + + expect(ctx.attributes.generalManagerEmail).toBe("gm@example.com"); + }); + it("does not let an unproven GM block the company from trading", async () => { // The GM names who to talk to, not what the company may do. Capturing it // through Fayda changed how it is collected, not whether it gates. @@ -775,3 +846,94 @@ describe("Ethiopian companies verify with Fayda; foreign companies verify identi ).resolves.toBeDefined(); }); }); + +/** + * Fayda's email and phone claims are optional, so a *verified* representative + * can still be missing the details `REQUIRED_POA_FIELDS` demands. The PoA step + * renders an input for whatever the verification did not supply — so onboarding + * has to report them outstanding, rather than letting a freight forwarder + * submit an incomplete representative and be refused its next PoA edit for it. + */ +describe("onboarding requirements name the PoA details Fayda did not supply", () => { + const POA_VERIFIED_NO_CONTACTS = { + poaFaydaSub: "poa-sub", + poaFaydaVerifiedAt: "2026-07-02T00:00:00.000Z", + poaName: "Tirunesh Dibaba", + }; + + it("reports the missing email and phone for a freight forwarder", async () => { + const { service } = makeService({ + attributes: { ...OWNER_VERIFIED, ...POA_VERIFIED_NO_CONTACTS }, + profileTypes: [ProfileType.freightForwarder], + files: [paper()], + }); + + const req = await service.getOnboardingRequirements("user-1"); + + expect(req.poa.missingFields.map((f) => f.key)).toEqual([ + "poaEmail", + "poaPhone", + ]); + expect(req.poa.complete).toBe(false); + expect(req.outstanding).toEqual( + expect.arrayContaining(["Add your poa email", "Add your poa phone"]), + ); + }); + + it("clears once they are typed", async () => { + const { service } = makeService({ + attributes: { ...OWNER_VERIFIED, ...POA_VERIFIED }, + profileTypes: [ProfileType.freightForwarder], + files: [paper()], + }); + + const req = await service.getOnboardingRequirements("user-1"); + + expect(req.poa.missingFields).toEqual([]); + expect(req.poa.complete).toBe(true); + expect(req.outstanding).not.toContain("Add your poa email"); + }); + + // Fayda's email claim is optional and the GM's verification has no account to + // fall back on, so demanding one blocked a manager the government had already + // proved. `companyNotifyEmailExpr` resolves the address from the contact + // person or the registering account instead, so nothing needs this filled. + it("does not hold a company back for a general manager with no email", async () => { + const { service } = makeService({ + attributes: { + ...OWNER_VERIFIED, + gmFaydaSub: "gm-sub", + generalManagerName: "Derartu Tulu", + generalManagerPhone: "+251911222333", + }, + }); + + const req = await service.getOnboardingRequirements("user-1"); + + expect(req.companyInfo.missingFields.map((f) => f.key)).not.toContain( + "generalManagerEmail", + ); + expect(req.outstanding).not.toContain("Add your general manager email"); + }); + + it("still holds it back for the manager's name and phone", async () => { + const { service } = makeService({ attributes: { ...OWNER_VERIFIED } }); + + const req = await service.getOnboardingRequirements("user-1"); + + expect(req.companyInfo.missingFields.map((f) => f.key)).toEqual( + expect.arrayContaining(["generalManagerName", "generalManagerPhone"]), + ); + }); + + // An importer that never named a representative owes nothing here — the step + // is one it may walk straight past. + it("asks nothing of a company with no PoA at all", async () => { + const { service } = makeService({ attributes: { ...OWNER_VERIFIED } }); + + const req = await service.getOnboardingRequirements("user-1"); + + expect(req.poa.missingFields).toEqual([]); + expect(req.poa.complete).toBe(true); + }); +}); 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 2cddb2974..72dcfdded 100644 --- a/apps/edr-freight-api/src/modules/companies/companies.service.ts +++ b/apps/edr-freight-api/src/modules/companies/companies.service.ts @@ -253,11 +253,14 @@ export class CompaniesService { label: "General manager name", get: (c) => c.attributes?.generalManagerName, }, - { - key: "generalManagerEmail", - label: "General manager email", - get: (c) => c.attributes?.generalManagerEmail, - }, + // The manager's EMAIL is deliberately absent. It was demanded because the + // notifiers were believed to mail it, and Fayda's email claim is optional + // — so a manager the government proved without one blocked the whole + // submission over an address nothing could produce. `companyNotifyEmailExpr` + // now resolves the address itself and falls through to the contact + // person's, then to the registering account's (which signup guarantees), + // so nothing depends on this being filled. It is still collected and still + // preferred when present; it just no longer holds the company hostage. { key: "generalManagerPhone", label: "General manager phone", @@ -2042,13 +2045,21 @@ export class CompaniesService { const poaProvided = POA_ATTRIBUTES.some((k) => (company.attributes?.[k] as string | undefined)?.trim(), ); - // No company types its PoA details — they arrive from the Fayda - // verification whatever the nationality — so reporting them as missing - // fields would ask for something no form offers. The identity block below - // reports "verify your PoA" instead. - const missingPoaFields: typeof REQUIRED_POA_FIELDS = []; const delegation = await this.getPoaDelegationState(company.id); const delegationDue = poaRequired || poaProvided; + // The representative's details normally arrive from their Fayda + // verification — but Fayda's email and phone claims are optional and + // routinely come back empty, and the PoA step renders an input for whatever + // the verification did not supply. So these are askable after all, and are + // reported outstanding once a PoA is required or provided; reporting + // nothing here let a freight forwarder finish onboarding with a + // representative the API's own `REQUIRED_POA_FIELDS` calls incomplete, then + // 400'd their next PoA edit for it. + const missingPoaFields = delegationDue + ? REQUIRED_POA_FIELDS.filter( + (f) => !(company.attributes?.[f.key] as string | undefined)?.trim(), + ) + : []; const missingDelegation = delegationDue && !delegation.onFile; // A paper the reviewer sent back is not evidence — the customer has to // replace it before the application counts as complete. @@ -2101,7 +2112,10 @@ export class CompaniesService { // fields, required documents, one license per operational profile, and the // PoA details/paper whenever those are mandatory. const requiredDocCount = documents.filter((d) => d.isRequired).length; - const poaItemCount = delegationDue ? 1 : 0; + // The delegation paper plus the representative's own required details — + // `completed` below subtracts every one of those it is still missing, so + // leaving them out of the total would make the bar understate progress. + const poaItemCount = delegationDue ? 1 + REQUIRED_POA_FIELDS.length : 0; // One item per identity credential the company has to prove: the owner // always (Fayda for Ethiopian, passport for foreign), plus the PoA once // there is one — Fayda for an Ethiopian company, a named representative diff --git a/apps/edr-freight-api/src/modules/companies/company-notifier.service.ts b/apps/edr-freight-api/src/modules/companies/company-notifier.service.ts index 178ace492..6bcd21f08 100644 --- a/apps/edr-freight-api/src/modules/companies/company-notifier.service.ts +++ b/apps/edr-freight-api/src/modules/companies/company-notifier.service.ts @@ -10,7 +10,7 @@ import { import { Company, CompanyStatus } from "./entities/company.entity"; import { NotificationsService } from "../notifications/notifications.service"; import { NotificationInboxService } from "../notification-inbox/notification-inbox.service"; -import { resolveCompanyNotifyPhone } from "../notifications/resolve-company-phone.util"; +import { resolveCompanyNotifyContact } from "../notifications/resolve-company-phone.util"; import { FREIGHT_PERMS } from "../../seed/freight-permissions.registry"; /** Account statuses that lock the customer out and therefore must be told to them. */ @@ -38,8 +38,13 @@ export class CompanyNotifierService { /** Send SMS + email to the company contact; log-only on failure. */ private async notifyContact(company: Company, message: string): Promise { - const phone = await resolveCompanyNotifyPhone(this.dataSource, company.id); - const email = company.email ?? company.generalManagerEmail ?? null; + // One resolver for both channels — the company row's own email column is + // only set for a Fayda-verified owner (see companyNotifyEmailExpr), which + // left an approval/suspension notice unsent to everyone else. + const { phone, email } = await resolveCompanyNotifyContact( + this.dataSource, + company.id, + ); if (phone) { try { diff --git a/apps/edr-freight-api/src/modules/companies/dto/complete-identity-verification.dto.ts b/apps/edr-freight-api/src/modules/companies/dto/complete-identity-verification.dto.ts index d41e61e8e..ae4a73a77 100644 --- a/apps/edr-freight-api/src/modules/companies/dto/complete-identity-verification.dto.ts +++ b/apps/edr-freight-api/src/modules/companies/dto/complete-identity-verification.dto.ts @@ -143,12 +143,18 @@ function stateFor( birthdate: read(`${p}Birthdate`), gender: read(`${p}Gender`), }; - if (subject !== "gm") return state; + if (subject !== "gm" || state.verified) return state; // Companies onboarded before the GM was verifiable have typed details and no // `gm*` attributes at all. Report those rather than a blank card — they are // still what the notifiers mail — leaving `verified` false so the portal // offers the upgrade instead of pretending the identity is proven. + // + // Only for such an unverified GM, which is the whole population this exists + // for. Merging the typed columns into a *verified* manager's state would read + // back the email the portal asked them to type when Fayda supplied none, and + // the input offering it — keyed on that value being absent — would vanish the + // moment it was saved, leaving a typo uncorrectable. return { ...state, name: state.name ?? read(GM_TYPED_KEYS.name), diff --git a/apps/edr-freight-api/src/modules/contracts/contract-notifier.service.ts b/apps/edr-freight-api/src/modules/contracts/contract-notifier.service.ts index e3c79a742..477724839 100644 --- a/apps/edr-freight-api/src/modules/contracts/contract-notifier.service.ts +++ b/apps/edr-freight-api/src/modules/contracts/contract-notifier.service.ts @@ -10,7 +10,7 @@ import { import { Contract } from './entities/contract.entity'; import { NotificationsService } from '../notifications/notifications.service'; import { NotificationInboxService } from '../notification-inbox/notification-inbox.service'; -import { resolveCompanyNotifyPhone } from '../notifications/resolve-company-phone.util'; +import { resolveCompanyNotifyContact } from '../notifications/resolve-company-phone.util'; import { FREIGHT_PERMS } from '../../seed/freight-permissions.registry'; /** @@ -52,10 +52,11 @@ export class ContractNotifierService { logLabel: string, ): Promise { this.logger.log(`${logLabel} — ${this.ref(c)}`); - const phone = c.companyId - ? await resolveCompanyNotifyPhone(this.dataSource, c.companyId) - : null; - const email = c.company?.email ?? c.company?.generalManagerEmail ?? null; + // One resolver for both channels — the company row's own email column is + // only set for a Fayda-verified owner (see companyNotifyEmailExpr). + const { phone, email } = c.companyId + ? await resolveCompanyNotifyContact(this.dataSource, c.companyId) + : { phone: null, email: null }; if (phone) { try { diff --git a/apps/edr-freight-api/src/modules/notifications/notify-company.util.ts b/apps/edr-freight-api/src/modules/notifications/notify-company.util.ts index 9f121e18a..e3b2fd80a 100644 --- a/apps/edr-freight-api/src/modules/notifications/notify-company.util.ts +++ b/apps/edr-freight-api/src/modules/notifications/notify-company.util.ts @@ -2,6 +2,7 @@ import { DataSource } from 'typeorm'; import { NotificationsService } from './notifications.service'; import { + companyNotifyEmailExpr, companyNotifyPhoneExpr, primaryContactUserJoin, } from './resolve-company-phone.util'; @@ -19,7 +20,8 @@ export async function sendCompanyChannels( ): Promise { const [contact]: Array<{ phone: string | null; email: string | null }> = await dataSource.query( - `SELECT ${companyNotifyPhoneExpr('co')} AS phone, co.email + `SELECT ${companyNotifyPhoneExpr('co')} AS phone, + ${companyNotifyEmailExpr('co')} AS email FROM freight.companies co ${primaryContactUserJoin('co')} WHERE co.id = $1 AND co.deleted_at IS NULL`, diff --git a/apps/edr-freight-api/src/modules/notifications/resolve-company-phone.util.ts b/apps/edr-freight-api/src/modules/notifications/resolve-company-phone.util.ts index 511f3cf8c..886aa8b85 100644 --- a/apps/edr-freight-api/src/modules/notifications/resolve-company-phone.util.ts +++ b/apps/edr-freight-api/src/modules/notifications/resolve-company-phone.util.ts @@ -1,7 +1,7 @@ import { DataSource, EntityManager } from "typeorm"; /** - * Where a customer-facing SMS actually goes. + * Where a customer-facing SMS or email actually goes. * * The person who signs up, logs in, and receives OTPs is an IAM user, and * `iam.users.phone_number` is the number they control and can change themselves @@ -12,11 +12,13 @@ import { DataSource, EntityManager } from "typeorm"; * `companies.contact_person_phone` is deliberately NOT consulted: the live write * path stores that value in the `attributes` jsonb and has never populated the * column, so every reader of it was silently falling through to `phone` anyway. + * `companies.general_manager_email` is the same trap on the email side — see + * {@link companyNotifyEmailExpr}. */ /** * LEFT JOIN a company alias to its primary contact's IAM user, exposing - * `pc.phone_number`. + * `pc.phone_number` and `pc.email`. * * LATERAL + LIMIT 1 rather than a plain join: nothing in the schema stops a * company having two `is_primary_contact` rows, and a plain join would then @@ -28,7 +30,7 @@ import { DataSource, EntityManager } from "typeorm"; export function primaryContactUserJoin(alias: string): string { return ` LEFT JOIN LATERAL ( - SELECT u.phone_number + SELECT u.phone_number, u.email FROM freight.external_profiles ep JOIN iam.users u ON u.id = ep.user_id AND u.is_active = true WHERE ep.company_id = ${alias}.id @@ -44,17 +46,45 @@ export function companyNotifyPhoneExpr(alias: string): string { return `COALESCE(pc.phone_number, ${alias}.phone)`; } -/** The SMS number for one company, or null when neither source has one. */ -export async function resolveCompanyNotifyPhone( +/** + * SQL expression for the company's notification address, given the joined `pc` + * alias. + * + * `companies.email` alone is not enough: it is written from ONE place — a + * Fayda-verified owner's email claim — so a foreign company, whose owner proves + * identity by passport instead, never gets one. Readers papered over that with + * `COALESCE(email, general_manager_email)`, but that column has the same problem + * `contact_person_phone` has above: onboarding writes the value into the + * `attributes` jsonb and nothing has ever populated the column, so the fallback + * could not fire and the mail was dropped in silence. + * + * So: the company address, then the two the customer actually filled in during + * onboarding, then the account that registered them — which always has one, + * signup requires it. `NULLIF` because a blank jsonb key is not an address and + * `COALESCE` would happily stop on it. + */ +export function companyNotifyEmailExpr(alias: string): string { + return `COALESCE( + NULLIF(${alias}.email, ''), + NULLIF(${alias}.attributes->>'generalManagerEmail', ''), + NULLIF(${alias}.attributes->>'contactPersonEmail', ''), + NULLIF(pc.email, '') + )`; +} + +/** Both channels for one company; either side is null when nothing has one. */ +export async function resolveCompanyNotifyContact( db: DataSource | EntityManager, companyId: string, -): Promise { - const rows: Array<{ phone: string | null }> = await db.query( - `SELECT ${companyNotifyPhoneExpr("co")} AS phone - FROM freight.companies co - ${primaryContactUserJoin("co")} - WHERE co.id = $1 AND co.deleted_at IS NULL`, - [companyId], - ); - return rows[0]?.phone ?? null; +): Promise<{ phone: string | null; email: string | null }> { + const rows: Array<{ phone: string | null; email: string | null }> = + await db.query( + `SELECT ${companyNotifyPhoneExpr("co")} AS phone, + ${companyNotifyEmailExpr("co")} AS email + FROM freight.companies co + ${primaryContactUserJoin("co")} + WHERE co.id = $1 AND co.deleted_at IS NULL`, + [companyId], + ); + return { phone: rows[0]?.phone ?? null, email: rows[0]?.email ?? null }; } diff --git a/apps/edr-freight-api/src/modules/train-scheduling/booking-notifier.service.ts b/apps/edr-freight-api/src/modules/train-scheduling/booking-notifier.service.ts index efb540066..9468387c5 100644 --- a/apps/edr-freight-api/src/modules/train-scheduling/booking-notifier.service.ts +++ b/apps/edr-freight-api/src/modules/train-scheduling/booking-notifier.service.ts @@ -11,7 +11,7 @@ import { import { Booking } from '../bookings/entities/booking.entity'; import { NotificationsService } from '../notifications/notifications.service'; import { NotificationInboxService } from '../notification-inbox/notification-inbox.service'; -import { resolveCompanyNotifyPhone } from '../notifications/resolve-company-phone.util'; +import { resolveCompanyNotifyContact } from '../notifications/resolve-company-phone.util'; import { TrainSchedulesRepository } from '../train-schedules/train-schedules.repository'; import { BATCH_TIMEZONE } from './booking-batch.constants'; @@ -65,10 +65,11 @@ export class BookingNotifierService { logLabel: string, ): Promise { this.logger.log(`${logLabel} — ${this.ref(b)}`); - const phone = b.companyId - ? await resolveCompanyNotifyPhone(this.dataSource, b.companyId) - : null; - const email = b.company?.email ?? b.company?.generalManagerEmail ?? null; + // One resolver for both channels — the company row's own email column is + // only set for a Fayda-verified owner (see companyNotifyEmailExpr). + const { phone, email } = b.companyId + ? await resolveCompanyNotifyContact(this.dataSource, b.companyId) + : { phone: null, email: null }; if (phone) { try { diff --git a/apps/edr-freight-api/src/modules/train-scheduling/booking-window.service.ts b/apps/edr-freight-api/src/modules/train-scheduling/booking-window.service.ts index 5036b01c1..47d0a0d21 100644 --- a/apps/edr-freight-api/src/modules/train-scheduling/booking-window.service.ts +++ b/apps/edr-freight-api/src/modules/train-scheduling/booking-window.service.ts @@ -14,6 +14,7 @@ import { TrainSchedulesRepository } from '../train-schedules/train-schedules.rep import { NotificationsService } from '../notifications/notifications.service'; import { NotificationInboxService } from '../notification-inbox/notification-inbox.service'; import { + companyNotifyEmailExpr, companyNotifyPhoneExpr, primaryContactUserJoin, } from '../notifications/resolve-company-phone.util'; @@ -649,7 +650,7 @@ export class BookingWindowService implements OnModuleInit { `SELECT DISTINCT c.company_id, ${companyNotifyPhoneExpr('co')} AS phone, - COALESCE(co.email, co.general_manager_email) AS email + ${companyNotifyEmailExpr('co')} AS email FROM freight.contract_routes cr JOIN freight.contracts c ON c.id = cr.contract_id @@ -761,7 +762,7 @@ export class BookingWindowService implements OnModuleInit { b.id AS "bookingId", b.company_id AS "companyId", ${companyNotifyPhoneExpr('co')} AS phone, - COALESCE(co.email, co.general_manager_email) AS email, + ${companyNotifyEmailExpr('co')} AS email, COALESCE(oy.label, oy.code) || ' to ' || COALESCE(dy.label, dy.code) AS corridor FROM freight.bookings b diff --git a/apps/edr-freight-web/portal/src/pages/accounts/CompanyProfileForm.tsx b/apps/edr-freight-web/portal/src/pages/accounts/CompanyProfileForm.tsx index 2405dbe5e..757c7508a 100644 --- a/apps/edr-freight-web/portal/src/pages/accounts/CompanyProfileForm.tsx +++ b/apps/edr-freight-web/portal/src/pages/accounts/CompanyProfileForm.tsx @@ -184,10 +184,20 @@ export default function CompanyProfileForm({ // forms (plus a mandatory owner passport number). const verifiedIdentity = identity?.faydaRequired === true; + // Which fields the current step renders an input for and therefore requires. + // Filled in further down (it depends on values this form owns), and read at + // validation time rather than at render time — the resolver below runs on + // events, always after the render that assigned it. + const requiredKeysRef = useRef<(keyof FormData)[]>([]); + const form = useForm({ - resolver: zodResolver( - buildOnboardingSchema(identity?.passportRequired === true), - ), + resolver: (values, context, options) => + zodResolver( + buildOnboardingSchema( + identity?.passportRequired === true, + requiredKeysRef.current, + ), + )(values, context, options), // `values` below re-seeds the form whenever the profile is refetched — and // an in-page identity action (ticking "same as owner") refetches it. Without // this, that reset silently throws away whatever the customer was part-way @@ -432,15 +442,19 @@ export default function CompanyProfileForm({ // Where the GM's details come from depends on how they were established: a // Fayda verification (or a "same as owner" declaration) owns them outright, // and only a company that may still type them falls back to form state. + // A verified GM's identity wins, but Fayda's email and phone claims are + // optional: what the verification did not supply is typed on this step, and + // the API deliberately does not read those back onto the identity (they are + // not proven), so the form value is the only place they exist. const gmVerified = identity?.gm.verified ?? false; const gmName = gmVerified - ? (identity?.gm.name ?? "") + ? firstPresent(identity?.gm.name, watch("generalManagerName")) : watch("generalManagerName"); const gmEmail = gmVerified - ? (identity?.gm.email ?? "") + ? firstPresent(identity?.gm.email, watch("generalManagerEmail")) : watch("generalManagerEmail"); const gmPhone = gmVerified - ? (identity?.gm.phone ?? "") + ? firstPresent(identity?.gm.phone, watch("generalManagerPhone")) : watch("generalManagerPhone"); /** @@ -448,9 +462,11 @@ export default function CompanyProfileForm({ * "same as owner" declaration, or (only where Fayda is optional) by typing. * Fayda is an Ethiopian national ID, so a foreign company's GM may hold none. */ + // Matches what the API actually demands (`REQUIRED_COMPANY_INFO`): a name and + // a phone. The email is collected but optional — a manager proved through + // Fayda may have no email claim, and the notify resolver no longer needs one. const gmTyped = Boolean( watch("generalManagerName")?.trim() && - watch("generalManagerEmail")?.trim() && watch("generalManagerPhone")?.trim(), ); const gmEstablished = @@ -623,12 +639,23 @@ export default function CompanyProfileForm({ // delegated, so it's required the moment a PoA exists. The API enforces the // same rule on save, so skipping it here only costs the customer a // round-trip. - // A PoA exists exactly when one has been verified — the details are the - // verification's output, so there is nothing else that could stand for one. - // Until then the upload is hidden: there is no representative for the paper - // to authorise, and a freight forwarder is held on the verification gate - // below rather than on a file field it cannot yet fill. - const poaProvided = (identity?.poa.verified ?? false) || poaTyped; + // + // "Exists" is the API's own test (`POA_ATTRIBUTES.some(...)`): ANY detail, + // verified or typed. Requiring a complete typed representative here instead + // hid the upload from a customer who had entered only a name — for whom the + // API still demands the paper, and whose resume would then be clamped back to + // this step with nothing on it to fill. + const poaAnyDetail = [ + identity?.poa.name, + identity?.poa.email, + identity?.poa.phone, + identity?.poa.address, + watch("poaName"), + watch("poaEmail"), + watch("poaPhone"), + watch("poaLocation"), + ].some((v) => v?.trim()); + const poaProvided = (identity?.poa.verified ?? false) || poaAnyDetail; // A freight forwarder owes the paper whether or not its representative could // verify with Fayda — the API demands it at completion either way. Keying // this on the verification alone hid the upload from a foreign forwarder and @@ -641,6 +668,60 @@ export default function CompanyProfileForm({ return Array.isArray(v) ? v.length > 0 : v != null; })(); + /** + * What a Fayda verification did NOT supply, per person. + * + * Fayda's email, phone and address claims are optional and routinely come + * back empty, so a *verified* person can still be missing details the API + * demands (`REQUIRED_POA_FIELDS`, `REQUIRED_COMPANY_INFO`). Those gaps are + * typed instead — the API keeps exactly the keys a claim left empty typeable, + * since a claim that returned nothing owns no value to protect. + * + * Computed here, once, and handed to both the step (which renders an input + * per gap) and the schema (which requires exactly those) — a field is + * required if and only if there is an input on screen to fix it in. + */ + // Where Fayda is mandatory an unverified representative must verify rather + // than be typed, so nothing is offered until the verification lands. + const poaTypedAllowed = + !identity || identity.poa.verified || !identity.faydaRequired; + // A representative the verification never proved holds only typed details — + // the API leaves those unlocked, so the inputs stay on screen and stay + // editable. Only a *verified* PoA hides the fields their claim did fill, + // which is also the only case the API refuses to let anyone overwrite. + const poaGap = (v?: string | null) => + poaTypedAllowed && (!identity?.poa.verified || !v?.trim()); + const poaGaps = { + name: poaGap(identity?.poa.name), + email: poaGap(identity?.poa.email), + phone: poaGap(identity?.poa.phone), + address: poaGap(identity?.poa.address), + }; + // The GM's own verification never falls back to the signed-in account — that + // account is the person onboarding, not necessarily the manager — so a GM + // verified with no email claim has nowhere else for one to come from. + // "Same as owner" is exempt: the API copies the owner's (account-backed) + // contact details across, so there is no gap and no input. + const gmGaps = { + email: !gmSameAsOwner && gmVerified && !identity?.gm.email?.trim(), + phone: !gmSameAsOwner && gmVerified && !identity?.gm.phone?.trim(), + }; + + const requiredKeys: (keyof FormData)[] = []; + if (step === "personnel") { + // The manager's email is offered but not demanded: the API dropped it from + // `REQUIRED_COMPANY_INFO` once the notify resolver stopped depending on it. + // The phone is still required there, so it is still required here. + if (gmGaps.phone) requiredKeys.push("generalManagerPhone"); + } else if (step === "poa" && delegationRequired) { + // Only once a PoA is required or provided: an untouched optional PoA is + // still a step the customer may walk straight past. + if (poaGaps.name) requiredKeys.push("poaName"); + if (poaGaps.email) requiredKeys.push("poaEmail"); + if (poaGaps.phone) requiredKeys.push("poaPhone"); + } + requiredKeysRef.current = requiredKeys; + /** * Collect the messages for a set of fields into one sentence. * @@ -656,6 +737,7 @@ export default function CompanyProfileForm({ // yet, so the closure would still be holding the previous attempt's state. const parsed = buildOnboardingSchema( identity?.passportRequired === true, + requiredKeys, ).safeParse(getValues()); const wanted = new Set(fields as string[]); const messages = parsed.success @@ -875,6 +957,7 @@ export default function CompanyProfileForm({ onToggleGmSameAsOwner={toggleGmSameAsOwner} gmLinkPending={gmLinkPending} gmVerified={gmVerified} + gaps={gmGaps} /> )} @@ -892,6 +975,7 @@ export default function CompanyProfileForm({ form={form} identity={identity} requirePoa={requirePoa} + gaps={poaGaps} delegationRequired={delegationRequired} poaDocumentSetting={poaDocumentSetting} documentFiles={documentFiles} diff --git a/apps/edr-freight-web/portal/src/pages/accounts/companyProfileForm/schema.test.ts b/apps/edr-freight-web/portal/src/pages/accounts/companyProfileForm/schema.test.ts index 6e459a6fb..f38d54432 100644 --- a/apps/edr-freight-web/portal/src/pages/accounts/companyProfileForm/schema.test.ts +++ b/apps/edr-freight-web/portal/src/pages/accounts/companyProfileForm/schema.test.ts @@ -1,6 +1,6 @@ import { describe, expect, it } from "vitest"; -import { onboardingSchema, stepFields } from "./schema"; +import { buildOnboardingSchema, onboardingSchema, stepFields } from "./schema"; import { firstPresent, firstValidEmail, @@ -101,6 +101,56 @@ describe("stepFields", () => { }); }); +describe("buildOnboardingSchema (conditionally required fields)", () => { + const issuesFor = ( + data: FormData, + required: (keyof FormData)[], + ): (keyof FormData)[] => { + const parsed = buildOnboardingSchema(false, required).safeParse(data); + return parsed.success + ? [] + : (parsed.error.issues.map((i) => i.path[0]) as (keyof FormData)[]); + }; + + // Fayda's email/phone claims are optional: the step renders an input for what + // the verification did not supply, and requires exactly those. Nothing else — + // a field with no input on screen must never fail Continue. + it("requires only the keys it is handed", () => { + const issues = issuesFor(values(), [ + "generalManagerEmail", + "generalManagerPhone", + ]); + expect(issues).toEqual(["generalManagerEmail", "generalManagerPhone"]); + }); + + it("passes once those keys are filled", () => { + expect( + issuesFor( + values({ + generalManagerEmail: "gm@example.com", + generalManagerPhone: "+251911223344", + }), + ["generalManagerEmail", "generalManagerPhone"], + ), + ).toEqual([]); + }); + + it("leaves the same blank fields alone when nothing is required", () => { + expect(issuesFor(values(), [])).toEqual([]); + }); + + it("names the field in the message, so it reads under its own input", () => { + const parsed = buildOnboardingSchema(false, ["poaEmail"]).safeParse( + values(), + ); + expect(parsed.success).toBe(false); + if (parsed.success) return; + expect(parsed.error.issues[0]?.message).toBe( + "Representative's email is required", + ); + }); +}); + describe("stepPayload (company)", () => { it("omits the eTrade bundle when nothing was re-verified", () => { const payload = stepPayload("company", values(), {}); diff --git a/apps/edr-freight-web/portal/src/pages/accounts/companyProfileForm/schema.ts b/apps/edr-freight-web/portal/src/pages/accounts/companyProfileForm/schema.ts index 82b925f7f..37f92e05e 100644 --- a/apps/edr-freight-web/portal/src/pages/accounts/companyProfileForm/schema.ts +++ b/apps/edr-freight-web/portal/src/pages/accounts/companyProfileForm/schema.ts @@ -105,27 +105,54 @@ export type FormData = z.infer; export const POA_DELEGATION_FILE_KEY = "poa_delegation_letter"; /** - * The PoA's identifying fields are never typed — they come from the Fayda - * verification, whatever the company's nationality — so nothing here requires - * them. A freight forwarder's mandatory PoA is gated on the verification - * itself, and its delegation letter alongside it, both in CompanyProfileForm - * (files live outside form state). + * Labels for the fields a Fayda verification may or may not have supplied. Which + * of them are mandatory is decided per render (see `requiredKeys` below), so the + * message has to be built here rather than attached to the base schema. + */ +const CONDITIONAL_LABELS: Partial> = { + poaName: "Representative's name", + poaEmail: "Representative's email", + poaPhone: "Representative's phone", + generalManagerName: "General manager's name", + generalManagerEmail: "General manager's email", + generalManagerPhone: "General manager's phone", +}; + +/** + * The PoA's and GM's identifying fields normally come from their Fayda + * verification, so nothing in the base schema requires them. But Fayda's email + * and phone claims are optional and routinely come back empty, and the steps + * render an input for whatever the verification did not supply — so those + * fields become mandatory exactly then. * - * That leaves the owner's passport number as the only conditional field. + * `requiredKeys` is that decision, made by CompanyProfileForm from the same + * state that drives the rendering: a field is required iff an input exists for + * it. Passing it in (rather than deriving it here) is what keeps the two from + * drifting into a Continue button that fails on a field nobody can see. */ export function buildOnboardingSchema( /** True for a foreign company: the owner's passport number is mandatory. */ passportRequired = false, + /** Fields the current step renders an input for and must not leave blank. */ + requiredKeys: readonly (keyof FormData)[] = [], ) { - if (!passportRequired) return onboardingSchema; + if (!passportRequired && requiredKeys.length === 0) return onboardingSchema; return onboardingSchema.superRefine((d, ctx) => { - if (!d.ownerPassportNumber?.trim()) { + if (passportRequired && !d.ownerPassportNumber?.trim()) { ctx.addIssue({ code: z.ZodIssueCode.custom, path: ["ownerPassportNumber"], message: "The owner's passport number is required", }); } + for (const key of requiredKeys) { + if (d[key]?.trim()) continue; + ctx.addIssue({ + code: z.ZodIssueCode.custom, + path: [key], + message: `${CONDITIONAL_LABELS[key] ?? key} is required`, + }); + } }); } diff --git a/apps/edr-freight-web/portal/src/pages/accounts/companyProfileForm/steps/PersonnelStep.tsx b/apps/edr-freight-web/portal/src/pages/accounts/companyProfileForm/steps/PersonnelStep.tsx index 0c1eaa049..023cdb933 100644 --- a/apps/edr-freight-web/portal/src/pages/accounts/companyProfileForm/steps/PersonnelStep.tsx +++ b/apps/edr-freight-web/portal/src/pages/accounts/companyProfileForm/steps/PersonnelStep.tsx @@ -18,6 +18,12 @@ export interface PersonnelStepProps { /** A server-side "same as owner" declaration is in flight. */ gmLinkPending: boolean; gmVerified: boolean; + /** + * Which of the manager's contact details their Fayda verification did not + * supply, and are therefore typed here. Computed by CompanyProfileForm, which + * requires exactly these in the schema. + */ + gaps: { email: boolean; phone: boolean }; } export default function PersonnelStep({ @@ -28,6 +34,7 @@ export default function PersonnelStep({ onToggleGmSameAsOwner, gmLinkPending, gmVerified, + gaps, }: PersonnelStepProps) { const { register, @@ -74,6 +81,34 @@ export default function PersonnelStep({ /> )} + {/* Fayda's email and phone claims are optional, and the manager's own + verification has no account to fall back on the way the owner's does + — the person onboarding is not necessarily the manager. Whatever the + verification left empty is typed here, and required: without it the + submit fails on "Add your general manager email" with no field + anywhere to satisfy it. */} + {(gaps.email || gaps.phone) && ( + + {gaps.email && ( + + )} + {gaps.phone && ( + + )} + + )} + {/* Typed details survive only where Fayda cannot be required — a foreign company's manager may hold no Fayda ID. Once verified the API owns these fields, so they go away. */} diff --git a/apps/edr-freight-web/portal/src/pages/accounts/companyProfileForm/steps/PoaStep.tsx b/apps/edr-freight-web/portal/src/pages/accounts/companyProfileForm/steps/PoaStep.tsx index bed6a6068..d3782909d 100644 --- a/apps/edr-freight-web/portal/src/pages/accounts/companyProfileForm/steps/PoaStep.tsx +++ b/apps/edr-freight-web/portal/src/pages/accounts/companyProfileForm/steps/PoaStep.tsx @@ -14,6 +14,13 @@ export interface PoaStepProps { identity?: CompanyIdentityState; /** This company holds a freight-forwarder profile, so the PoA is mandatory. */ requirePoa: boolean; + /** + * Which of the representative's details the Fayda verification did not + * supply, and are therefore typed here. Computed by CompanyProfileForm, which + * requires exactly these in the schema — so every input rendered below is one + * the customer is actually asked to fill. + */ + gaps: { name: boolean; email: boolean; phone: boolean; address: boolean }; /** The DARS delegation paper is owed (a PoA exists, or the company forwards). */ delegationRequired: boolean; /** Single-field upload setting carrying just the delegation letter. */ @@ -28,6 +35,7 @@ export default function PoaStep({ form, identity, requirePoa, + gaps, delegationRequired, poaDocumentSetting, documentFiles, @@ -45,24 +53,21 @@ export default function PoaStep({ // empty, so a *verified* representative can still be missing the email and // phone the API demands from a freight forwarder (`REQUIRED_POA_FIELDS`) — // and the panel above renders no input for them, which dead-ends the step on - // "Add the poa email first". Offer an input for whatever the verification - // did not supply: the API keeps exactly those keys typeable, since a claim - // that returned nothing owns no value to protect (`faydaOwnedKeys`). - const poa = identity?.poa; - // Where Fayda is mandatory an unverified representative must verify rather - // than be typed, so nothing is offered until the verification lands. - const typedAllowed = !identity || poa!.verified || !identity.faydaRequired; - const missing = (value: string | null | undefined) => - typedAllowed && !value?.trim(); - const needsEmail = missing(poa?.email); - const needsPhone = missing(poa?.phone); + // "Add the poa email first". `gaps` is exactly what the verification did not + // supply: the API keeps those keys typeable, since a claim that returned + // nothing owns no value to protect (`faydaOwnedKeys`). + const needsEmail = gaps.email; + const needsPhone = gaps.phone; return ( <> {requirePoa ? "As a freight forwarder you act on other companies' behalf, so Power of Attorney details and the DARS delegation paper are required." - : "Power of Attorney details are optional. Fill them in if you have them, or skip to continue. If you do enter a representative, upload the delegation paper authenticated by DARS."} + : "Power of Attorney details are optional. Fill them in if you have them, or skip to continue. If you do enter a representative, upload the delegation paper authenticated by DARS."}{" "} + {/* The API refuses an owner who delegates to themselves — say so here, + or the customer only finds out after being sent to Fayda and back. */} + The representative must be someone other than the company's owner. {/* A representative acts for the company inside Ethiopia whoever owns it, so the PoA is proven with Fayda regardless of @@ -78,7 +83,7 @@ export default function PoaStep({ )} {/* Whatever the Fayda claim did carry is shown on the panel above and is never typed here — the verification owns it. */} - {missing(poa?.name) && ( + {gaps.name && ( )} - {missing(poa?.address) && ( + {gaps.address && ( api.companies.updateProfile.call({ - generalManagerName: data.generalManagerName, - generalManagerEmail: data.generalManagerEmail, - generalManagerPhone: data.generalManagerPhone, + // `|| undefined`, never "": the DTO's `@IsOptional()` only skips null + // and undefined, so an empty string is validated and 400s with + // "generalManagerEmail must be an email". A verified manager legitimately + // leaves the fields Fayda did supply blank here. + generalManagerName: data.generalManagerName || undefined, + generalManagerEmail: data.generalManagerEmail || undefined, + generalManagerPhone: data.generalManagerPhone || undefined, }), onSuccess: () => { queryClient.invalidateQueries({ queryKey: api.companies.getProfile.queryKey() }); @@ -144,6 +148,16 @@ export default function TabGeneralManager({ profile, mode = "edit", onContinue } // posting empty strings over a proven identity. const typedFieldsInUse = !gmSameAsOwner && !gm?.verified && !faydaRequired; + // Except for what the verification never supplied. Fayda's email and phone + // claims are optional, and a manager verified without them has no account to + // fall back on the way the owner does — so those stay typed, here as well as + // in onboarding, or a wrong address could never be corrected. + const gmGaps = { + email: !gmSameAsOwner && Boolean(gm?.verified) && !gm?.email?.trim(), + phone: !gmSameAsOwner && Boolean(gm?.verified) && !gm?.phone?.trim(), + }; + const savable = typedFieldsInUse || gmGaps.email || gmGaps.phone; + return ( @@ -187,6 +201,34 @@ export default function TabGeneralManager({ profile, mode = "edit", onContinue } /> )} + {/* Whatever the verification did not supply is typed instead — the + API keeps exactly those keys writable. */} + {(gmGaps.email || gmGaps.phone) && ( + + {gmGaps.email && ( + + + + )} + {gmGaps.phone && ( + + + + )} + + )} + {/* Typed details survive only where Fayda cannot be required — a foreign company's manager may hold no Fayda ID. Once verified the API owns these fields and refuses edits, so they go away. */} @@ -258,7 +300,7 @@ export default function TabGeneralManager({ profile, mode = "edit", onContinue } empty strings at an identity the API owns and refuses to overwrite. Onboarding still needs a way forward, so the button becomes a plain Continue rather than disappearing. */} - {typedFieldsInUse ? ( + {savable ? (