From cbcc9a02e69d6e5084e528dc5b3421d01c422a8c Mon Sep 17 00:00:00 2001 From: Nathnael Date: Tue, 18 Aug 2026 12:44:02 +0000 Subject: [PATCH] fix: approval window --- .../companies.license-supersede.spec.ts | 187 ++++++++++++++++++ .../companies.profile-approval.spec.ts | 115 +++++++++++ .../modules/companies/companies.service.ts | 59 +++++- .../src/components/customers/badges.tsx | 43 ++-- 4 files changed, 390 insertions(+), 14 deletions(-) create mode 100644 apps/edr-freight-api/src/modules/companies/companies.license-supersede.spec.ts create mode 100644 apps/edr-freight-api/src/modules/companies/companies.profile-approval.spec.ts diff --git a/apps/edr-freight-api/src/modules/companies/companies.license-supersede.spec.ts b/apps/edr-freight-api/src/modules/companies/companies.license-supersede.spec.ts new file mode 100644 index 000000000..5a73b6040 --- /dev/null +++ b/apps/edr-freight-api/src/modules/companies/companies.license-supersede.spec.ts @@ -0,0 +1,187 @@ +import { CompaniesService } from "./companies.service"; +import { CompanyStatus } from "./entities/company.entity"; +import { + ProfileStatus, + ProfileType, +} from "./entities/company-profile.entity"; + +/** + * Uploading a business licence only ever adds a row — nothing overwrites. So a + * customer answering a rejection or a document correction used to end up with + * the refused licence still listed beside the new one, in the portal and in the + * backoffice, with nothing saying which is current. An upload that answers a + * reviewer now retires what it answers; an upload with nothing outstanding is a + * genuine addition and still just adds. + */ +interface StoredFile { + id: string; + name: string; + code: string; + createdAt: Date; + reviewStatus?: string | null; + removed?: boolean; +} + +const T0 = new Date("2026-01-01T00:00:00Z"); +const REJECTED_AT = new Date("2026-02-01T00:00:00Z"); +const T2 = new Date("2026-03-01T00:00:00Z"); + +function makeService( + status: ProfileStatus, + files: StoredFile[], + reviewedAt: Date | null = null, +) { + const stored = [...files]; + const profile = { + id: "profile-1", + companyId: "company-1", + type: ProfileType.importer, + status, + reviewedAt, + }; + const company = { + id: "company-1", + status: + status === ProfileStatus.Active + ? CompanyStatus.Active + : CompanyStatus.Pending, + companyProfiles: [profile], + }; + + const live = () => stored.filter((f) => !f.removed); + const filesService = { + upload: jest.fn(async (input: { code: string; file: { originalname: string } }) => { + const record = { + id: `file-${stored.length + 1}`, + name: input.file.originalname, + code: input.code, + createdAt: T2, + size: 1, + mimeType: "application/pdf", + }; + stored.push(record); + return record; + }), + findByResource: jest.fn(async () => live()), + findWithOpenChangeRequest: jest.fn(async () => + live().filter((f) => f.reviewStatus === "change_requested"), + ), + findById: jest.fn(async (id: string) => ({ + ...stored.find((f) => f.id === id), + resource: "company_profiles", + resourceId: "profile-1", + })), + remove: jest.fn(async (id: string) => { + const found = stored.find((f) => f.id === id); + if (found) found.removed = true; + }), + clearReview: jest.fn(async (id: string) => { + const found = stored.find((f) => f.id === id); + if (found) found.reviewStatus = null; + }), + }; + + const changeRequestRepo = { + findPendingByCompanyId: jest.fn(async () => null), + create: jest.fn(async (row: Record) => ({ id: "cr-1", ...row })), + update: jest.fn(async () => ({ id: "cr-1" })), + findByCompanyId: jest.fn(async () => []), + }; + + const service = new CompaniesService( + { findById: jest.fn(async () => company) } as never, + { findByCompanyId: jest.fn(async () => [profile]) } as never, + changeRequestRepo as never, + {} as never, + { findByCompanyId: jest.fn(async () => []) } as never, + {} as never, + filesService as never, + {} as never, + {} as never, + { changeRequestSubmitted: jest.fn() } as never, + {} as never, + {} as never, + ); + + jest + .spyOn(service, "getCompanyInfoByUserId") + .mockImplementation( + async () => ({ profile: { id: "external-1" }, company }) as never, + ); + + return { service, stored, live, filesService, changeRequestRepo }; +} + +const upload = (service: CompaniesService) => + service.addProfileLicenseFiles("user-1", "profile-1", [ + { originalname: "new-licence.pdf" } as never, + ]); + +describe("a business licence uploaded to answer a reviewer", () => { + it("retires the file the reviewer flagged for correction", async () => { + const { service, live } = makeService(ProfileStatus.Pending, [ + { id: "file-old", name: "old.pdf", code: "business_license", createdAt: T0, reviewStatus: "change_requested" }, + ]); + + await upload(service); + + expect(live().map((f) => f.name)).toEqual(["new-licence.pdf"]); + }); + + it("retires what was on file when the role was rejected, but not the customer's own fix so far", async () => { + // Two uploads answering one rejection (a second page, or a re-pick) must not + // cannibalise each other — only what the reviewer actually refused goes. + const { service, live } = makeService( + ProfileStatus.Rejected, + [ + { id: "file-refused", name: "refused.pdf", code: "business_license", createdAt: T0 }, + { id: "file-fix-1", name: "fix-page-1.pdf", code: "business_license", createdAt: T2 }, + ], + REJECTED_AT, + ); + + await upload(service); + + expect(live().map((f) => f.name)).toEqual([ + "fix-page-1.pdf", + "new-licence.pdf", + ]); + }); + + it("leaves an ordinary addition alone when nothing was asked for", async () => { + const { service, live } = makeService(ProfileStatus.Pending, [ + { id: "file-old", name: "existing.pdf", code: "business_license", createdAt: T0 }, + ]); + + await upload(service); + + expect(live().map((f) => f.name)).toEqual([ + "existing.pdf", + "new-licence.pdf", + ]); + }); + + it("stages the swap for review on an approved role instead of deleting", async () => { + // A live role's licence is not the customer's to remove unilaterally: the + // old file stays until a reviewer approves the swap. + const { service, live, changeRequestRepo } = makeService( + ProfileStatus.Active, + [ + { id: "file-old", name: "old.pdf", code: "business_license", createdAt: T0, reviewStatus: "change_requested" }, + ], + ); + + await upload(service); + + expect(live().map((f) => f.name)).toEqual(["old.pdf", "new-licence.pdf"]); + const intents = changeRequestRepo.create.mock.calls.flatMap( + ([row]) => (row as any).documents.licenseChanges, + ); + expect(intents).toEqual( + expect.arrayContaining([ + expect.objectContaining({ op: "add", fileId: "file-2" }), + expect.objectContaining({ op: "remove", fileId: "file-old" }), + ]), + ); + }); +}); diff --git a/apps/edr-freight-api/src/modules/companies/companies.profile-approval.spec.ts b/apps/edr-freight-api/src/modules/companies/companies.profile-approval.spec.ts new file mode 100644 index 000000000..79fbdd479 --- /dev/null +++ b/apps/edr-freight-api/src/modules/companies/companies.profile-approval.spec.ts @@ -0,0 +1,115 @@ +import { BadRequestException } from "@nestjs/common"; + +import { CompaniesService } from "./companies.service"; +import { Company, CompanyStatus } from "./entities/company.entity"; +import { + CompanyProfile, + ProfileStatus, + ProfileType, +} from "./entities/company-profile.entity"; + +/** + * A rejection hands the role back to the customer: they fix what was flagged + * and resubmit (`reapplyCompanyProfile` → Pending). The reviewer used to be + * able to skip that entirely and approve straight out of Rejected — granting + * the role over the documents that were just refused, while the customer's + * "please fix this" note was still on their screen. + */ +function makeService(status: ProfileStatus) { + const profile: Partial = { + id: "profile-1", + companyId: "company-1", + type: ProfileType.importer, + status, + reference: null, + reviewNote: status === ProfileStatus.Rejected ? "Licence expired" : null, + }; + const company = { + id: "company-1", + status: CompanyStatus.Pending, + attributes: {}, + }; + + const written: Partial[] = []; + const profileRepo = { + update: jest.fn(async (_id: string, patch: Partial) => { + written.push(patch); + Object.assign(profile, patch); + return null; + }), + findOne: jest.fn(async () => profile), + }; + const companyRepo = { findOne: jest.fn(async () => company), update: jest.fn() }; + + const companyProfilesRepo = { + findById: jest.fn(async () => profile), + generateReference: jest.fn(async () => "IM-A00001"), + }; + const profilesRepo = { + // Onboarding submitted — the other gate in this method is not what these + // tests are about. + findByCompanyId: jest.fn(async () => [{ onboardingCompleted: true }]), + }; + const filesService = { findWithOpenChangeRequest: jest.fn(async () => []) }; + const dataSource = { + transaction: jest.fn(async (cb: (m: unknown) => Promise) => + cb({ + findOne: jest.fn(async () => company), + getRepository: (entity: unknown) => + entity === Company ? companyRepo : profileRepo, + }), + ), + }; + const companyNotifier = { profileStatusChanged: jest.fn(), companyApproved: jest.fn() }; + + const service = new CompaniesService( + {} as never, + companyProfilesRepo as never, + {} as never, + {} as never, + profilesRepo as never, + {} as never, + filesService as never, + {} as never, + {} as never, + companyNotifier as never, + dataSource as never, + {} as never, + ); + + return { service, profile, written, companyProfilesRepo }; +} + +describe("approving an operational role", () => { + it("refuses to approve a role the customer has not resubmitted", async () => { + const { service, companyProfilesRepo } = makeService(ProfileStatus.Rejected); + + await expect( + service.setCompanyProfileStatus("profile-1", ProfileStatus.Active), + ).rejects.toBeInstanceOf(BadRequestException); + // Refused before any reference could be minted against the rejected role. + expect(companyProfilesRepo.generateReference).not.toHaveBeenCalled(); + }); + + it("lets a reviewer undo their own rejection, and drops the note with it", async () => { + const { service, written } = makeService(ProfileStatus.Rejected); + + await service.setCompanyProfileStatus("profile-1", ProfileStatus.Pending); + + expect(written[0]).toMatchObject({ + status: ProfileStatus.Pending, + reviewNote: null, + }); + }); + + it("still approves a role that is awaiting its first decision", async () => { + const { service, written } = makeService(ProfileStatus.Pending); + + await service.setCompanyProfileStatus("profile-1", ProfileStatus.Active); + + expect(written[0]).toMatchObject({ + status: ProfileStatus.Active, + reference: "IM-A00001", + }); + }); +}); 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 1eb611c24..d7151f200 100644 --- a/apps/edr-freight-api/src/modules/companies/companies.service.ts +++ b/apps/edr-freight-api/src/modules/companies/companies.service.ts @@ -1830,6 +1830,24 @@ export class CompaniesService { ); } + // A rejected role is waiting on the customer, not on the reviewer: nothing + // has been resubmitted, and the note telling them what to fix is still on + // their screen. Approving straight out of Rejected grants the very role that + // was refused, over the documents that were refused with it. The way back is + // the customer's own resubmission (`reapplyCompanyProfile` → Pending); a + // rejection made in error is undone by moving the role back to pending + // review first — the same shape as "withdraw the change request first" on + // the document gate below. + if ( + status === ProfileStatus.Active && + existing.status === ProfileStatus.Rejected + ) { + throw new BadRequestException( + "This role was rejected — the customer has to fix what was flagged and resubmit it before it can be approved. " + + "If the rejection was a mistake, move the role back to pending review first.", + ); + } + // A self-registered company is only reviewable once its owner submits the // onboarding wizard (markOnboardingComplete) — until then its profiles are // half-filled drafts and approving one would mint a reference against an @@ -1958,7 +1976,14 @@ export class CompaniesService { status === ProfileStatus.Suspended ) { patch.reviewNote = note ?? null; - } else if (status === ProfileStatus.Active) { + } else if ( + status === ProfileStatus.Active || + status === ProfileStatus.Pending + ) { + // Pending only reaches here when a reviewer withdraws their own rejection + // (the customer's resubmission clears the note in `reapplyCompanyProfile`), + // so the reason they gave goes with it — leaving it would keep telling the + // customer to fix something nobody is waiting on any more. patch.reviewNote = null; } if (status !== ProfileStatus.Pending) { @@ -2675,6 +2700,9 @@ export class CompaniesService { * with the role itself. Only for an already-approved role are they staged under * the pending code and recorded as `add` intents on a pending change request — * a licence swap on a live role is a change; a licence on a new role is not. + * + * An upload that answers a reviewer also retires the licence it answers (see + * below), so a correction never leaves both copies on file. */ async addProfileLicenseFiles( userId: string, @@ -2686,6 +2714,28 @@ export class CompaniesService { const gated = profile.status === ProfileStatus.Active; const code = gated ? LICENSE_PENDING_CODE : LICENSE_CODE; + // An upload that answers the reviewer replaces what they refused; it does + // not sit next to it. Uploading only ever adds a row, so without this the + // refused licence stays listed in the portal and the backoffice beside the + // new one and nothing says which is current. Two things count as refused: + // the file the reviewer flagged for correction, and — when the whole role + // came back rejected — every licence that was already on file when they + // rejected it. Anything the customer uploaded *since* that decision is part + // of the same fix (a second page, a re-pick), so it survives, and an upload + // with nothing outstanding is a genuine addition and is left alone. + const rejectedAt = + profile.status === ProfileStatus.Rejected + ? (profile.reviewedAt ?? null) + : null; + const superseded = rejectedAt + ? ( + await this.filesService.findByResource(profileId, LICENSE_RESOURCE) + ).filter((f) => f.createdAt < rejectedAt) + : await this.filesService.findWithOpenChangeRequest( + [profileId], + LICENSE_RESOURCE, + ); + const uploaded = await Promise.all( files.map((file) => this.filesService.upload({ @@ -2710,6 +2760,13 @@ export class CompaniesService { ); } + // Retire what the upload supersedes, through the normal removal path so an + // approved role stages a `remove` intent (reviewed as a swap) while an + // unapproved one just drops the file. + for (const stale of superseded) { + await this.removeProfileLicenseFile(userId, profileId, stale.id); + } + // A fresh licence upload answers any correction the reviewer asked for on the // previous one, so the old row must stop blocking approval. await this.resolveDocumentChangeRequests( diff --git a/apps/edr-freight-web/backoffice/src/components/customers/badges.tsx b/apps/edr-freight-web/backoffice/src/components/customers/badges.tsx index 22d3e66cf..3127a39f8 100644 --- a/apps/edr-freight-web/backoffice/src/components/customers/badges.tsx +++ b/apps/edr-freight-web/backoffice/src/components/customers/badges.tsx @@ -338,9 +338,11 @@ export function InvoiceStatusBadge({ /** * Inline approval action buttons for a profile row. - * Transitions: pending → approve / reject-with-note | rejected → approve (override) | + * Transitions: pending → approve / reject-with-note | rejected → undo-rejection (→ pending) | * active → suspend | suspended → reactivate/blacklist | blacklisted → reinstate. - * Rejecting captures a note the customer sees so they can fix and reapply. + * Rejecting captures a note the customer sees so they can fix and reapply — a + * rejected role is theirs to resubmit, so it cannot be approved from here until + * they do (the API refuses it); undoing the rejection is the only way back. * * `locked` (customer hasn't submitted onboarding) withholds the review decision * only — there's no application to judge yet, and the API rejects the call @@ -526,18 +528,33 @@ export function ProfileApprovalActions({ } if (status === "rejected") { - if (!canSet("active")) return null; + // No Approve here: the role is waiting on the customer to fix what was + // flagged and resubmit it, and the API refuses rejected → active outright. + // All that's left is undoing a rejection that shouldn't have happened, + // which puts the role back in the queue rather than into service. + if (!canSet("pending")) return null; return ( - + + + Awaiting customer resubmission + + + + + ); }