From 6d662e81bdc9ea0946cfe7f9d06024c79fc6def6 Mon Sep 17 00:00:00 2001 From: Daniel Roth Date: Thu, 23 Jul 2026 15:34:06 +0000 Subject: [PATCH] feat(ara-projects): edit and show permissions per workstream (#413) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The switch was the wrong control. project_workstream_contractor stores both flags per (workstream, organisation) pair, so one switch stood for n booleans: it could report a per-workstream split but never author one, and clicking it destroyed the split to stay decidable. The schema's central capability was unreachable from the only screen that configures it. Add/edit dialog is now a grid: one row per workstream the *project* covers — not only the ones this contractor holds — with a tick to assign and a tick per permission. Unassigned rows stay visible and greyed, so the shape of the project and its uncovered workstreams are part of the decision. Un-ticking a workstream clears its permissions, so re-ticking never silently restores a grant. Column headers carry an all/none shortcut. The main table no longer shows permissions at all: contractor, workstreams, Actions. Clicking a row (or its disclosure button, which carries aria-expanded/aria-controls for keyboards and screen readers) opens a sub-row listing each assignment with its own two permissions, read-only. Several may be open at once. Actions stay per contractor — Edit and Remove act on the whole set. Wire shape follows: POST/PATCH take assignments[{projectWorkstreamId, updateStagesPermission, uploadDocumentsPermission}]. A workstream named twice is a 400 — two answers to one question. ContractorGroup drops its per-contractor summary and permissionState is gone; nothing needs the flags reduced to one answer, and reducing them is how a contractor allowed on one workstream comes to look allowed everywhere. Updates are grouped by flag pair, so twenty workstreams still cost at most four UPDATEs. ADR-0022 records this; ADR-0021's decision 1 is marked superseded, its other two (removal never cascades; Continue leads to the import) stand. --- ...are-per-assignment-shown-per-contractor.md | 7 +- ...ons-are-edited-and-shown-per-workstream.md | 88 ++++ .../[organisationId]/route.test.ts | 51 ++- .../contractors/[organisationId]/route.ts | 18 +- .../contractors/assignments.test.ts | 122 ++---- .../[projectId]/contractors/assignments.ts | 65 +-- .../[projectId]/contractors/queries.ts | 67 +++- .../[projectId]/contractors/route.test.ts | 99 ++++- .../projects/[projectId]/contractors/route.ts | 15 +- .../[projectId]/contractors/validation.ts | 37 +- .../contractors/ContractorFormDialog.tsx | 363 ++++++++++------- .../ContractorPermissionsTable.tsx | 375 ++++++++++-------- .../setup/contractors/PermissionCell.tsx | 59 +++ .../setup/contractors/PermissionToggle.tsx | 170 -------- 14 files changed, 830 insertions(+), 706 deletions(-) create mode 100644 docs/adr/0022-contractor-permissions-are-edited-and-shown-per-workstream.md create mode 100644 src/app/projects/[projectId]/setup/contractors/PermissionCell.tsx delete mode 100644 src/app/projects/[projectId]/setup/contractors/PermissionToggle.tsx diff --git a/docs/adr/0021-contractor-permissions-are-per-assignment-shown-per-contractor.md b/docs/adr/0021-contractor-permissions-are-per-assignment-shown-per-contractor.md index fcc8e9c6..7ae631e5 100644 --- a/docs/adr/0021-contractor-permissions-are-per-assignment-shown-per-contractor.md +++ b/docs/adr/0021-contractor-permissions-are-per-assignment-shown-per-contractor.md @@ -4,7 +4,12 @@ Date: 2026-07-23 ## Status -Accepted +**Decision 1 superseded by [ADR-0022](./0022-contractor-permissions-are-edited-and-shown-per-workstream.md)** +(2026-07-23): permissions are now edited per workstream in a grid and shown per +workstream in an expandable sub-row, so no per-contractor `mixed` state exists +any more. Decisions 2 (removal never cascades through work orders) and 3 +(Continue leads to the import) stand, and the Context below still explains why +the unit of a permission is the assignment. Numbering note: two ADRs carry 0019 (the UK date format one, and the work-order-import one from #414's branch). 0021 follows 0020. diff --git a/docs/adr/0022-contractor-permissions-are-edited-and-shown-per-workstream.md b/docs/adr/0022-contractor-permissions-are-edited-and-shown-per-workstream.md new file mode 100644 index 00000000..aeb48ee9 --- /dev/null +++ b/docs/adr/0022-contractor-permissions-are-edited-and-shown-per-workstream.md @@ -0,0 +1,88 @@ +# 22. Contractor permissions are edited and shown per workstream + +Date: 2026-07-23 + +## Status + +Accepted. Supersedes decision 1 of +[ADR-0021](./0021-contractor-permissions-are-per-assignment-shown-per-contractor.md); +its other two decisions stand. + +## Context + +ADR-0021 kept the wireframe's shape: one table row per contractor organisation, +with an "Update stages" and an "Upload docs" **switch** on that row. Because +`project_workstream_contractor` stores those flags per (workstream, +organisation) pair, one switch stood for *n* booleans, and a contractor whose +assignments disagreed was reported as `mixed`. + +Building it exposed the flaw. A switch is a control that asserts one state and +sets one state; it cannot honestly represent *n*, so: + +- the `mixed` state needed a chip, a tooltip and a bespoke accessible name + before it was even comprehensible — a lot of explanation for a control that + was still lying about its own shape; +- the only write it could express was "apply one pair of flags to everything", + so the UI could **read** a per-workstream split but never **author** one. The + schema's central capability was unreachable from the only screen that + configures it, and ADR-0021 had to note that no v1 surface produced divergent + flags; +- clicking a `mixed` switch destroyed information the user might not have + looked at, and "resolve upwards" was a rule invented to make an unsuitable + control decidable. + +Meanwhile the actual question — *may this contractor move Windows along, and +may it upload evidence on Roofs?* — is a grid: workstreams down, permissions +across. And on the table, permissions are detail: who delivers what is the +scanning question, and every row spending two columns on flags nobody is +reading crowds it out. + +## Decision + +**Permissions are edited per workstream, in a grid.** The add/edit dialog shows +one row per workstream the *project* covers — not merely the ones this +contractor holds — with a tick to assign it and a tick per permission. +Unassigned rows stay visible and greyed, their permission boxes disabled: the +shape of the project is part of the decision, and a workstream nobody covers is +worth seeing at the moment someone is being assigned. Un-ticking a workstream +clears its permissions, so re-ticking it never silently restores a grant. Each +column header carries an all/none shortcut, because ticking twenty boxes by +hand is how mistakes get made. + +**The main table hides permissions until asked.** A row shows the contractor, +the workstreams it delivers, and its Actions. Clicking the row — or its +disclosure button, which is what carries `aria-expanded`/`aria-controls` for +keyboard and screen-reader users — opens a sub-row listing each assignment with +its two permissions, read-only. Several rows may be open at once; comparing two +contractors is a normal thing to want. Actions stay per contractor, since Edit +and Remove act on the whole assignment set. + +**The wire shape follows.** POST and PATCH take `assignments: [{ +projectWorkstreamId, updateStagesPermission, uploadDocumentsPermission }]` +instead of one id list plus one pair of flags. A workstream may appear only +once per request — two entries for the same workstream are two answers to one +question, and picking one silently is worse than a 400. `ContractorGroup` no +longer carries a per-contractor summary of either permission, and +`permissionState` is gone: nothing needs the flags reduced to one answer, and +reducing them is precisely how a contractor allowed to update stages on one +workstream comes to look allowed everywhere. + +## Consequences + +- The screen can now express what the schema always stored, and what the authz + guards have always enforced per assignment (`canUpdateStage` reads the flag on + the work order's own assignment). UI, storage and enforcement finally agree on + the unit. +- Divergent permissions are no longer an anomaly to be explained but the normal + output of an ordinary edit. The `mixed` vocabulary, its chip and its tooltip + are deleted rather than reworded. +- Setting one permission across many workstreams costs one click per box, or + one on the column header. That is more work than the single switch it + replaces — accepted deliberately: the switch was cheap because it was coarse. +- The table's first read is now "who delivers what", with permissions one click + away. A future need to scan permissions across contractors (an audit view) + would want a different surface, not these columns back. +- Editing permissions on one workstream sends the contractor's whole assignment + set. The removal rules still run over exactly the workstreams a request drops, + so an edit that also un-ticks a workstream with work orders is still refused + (ADR-0021, decision 2). diff --git a/src/app/api/projects/[projectId]/contractors/[organisationId]/route.test.ts b/src/app/api/projects/[projectId]/contractors/[organisationId]/route.test.ts index 14176627..363b0d74 100644 --- a/src/app/api/projects/[projectId]/contractors/[organisationId]/route.test.ts +++ b/src/app/api/projects/[projectId]/contractors/[organisationId]/route.test.ts @@ -104,6 +104,19 @@ function url(confirm = false, organisationId = APEX) { }`; } +/** One grid row: a workstream and the permissions it should carry. */ +function ws( + projectWorkstreamId: string, + updateStagesPermission = false, + uploadDocumentsPermission = false, +) { + return { + projectWorkstreamId, + updateStagesPermission, + uploadDocumentsPermission, + }; +} + function patchRequest(body: unknown, confirm = false) { return new NextRequest(url(confirm), { method: "PATCH", @@ -135,11 +148,7 @@ describe("PATCH", () => { projectExists(); const res = await PATCH( - patchRequest({ - projectWorkstreamIds: ["10"], - updateStagesPermission: false, - uploadDocumentsPermission: true, - }), + patchRequest({ assignments: [ws("10", false, true)] }), params(), ); @@ -147,11 +156,13 @@ describe("PATCH", () => { expect(mockWriteContractorAssignments).toHaveBeenCalledWith( 5n, APEX, - { - projectWorkstreamIds: [10n], - updateStagesPermission: false, - uploadDocumentsPermission: true, - }, + [ + { + projectWorkstreamId: 10n, + updateStagesPermission: false, + uploadDocumentsPermission: true, + }, + ], "replace", ); }); @@ -161,7 +172,7 @@ describe("PATCH", () => { projectExists(); const res = await PATCH( - patchRequest({ projectWorkstreamIds: ["10", "11"] }), + patchRequest({ assignments: [ws("10"), ws("11")] }), params(), ); @@ -175,7 +186,7 @@ describe("PATCH", () => { mockCountOtherContractors.mockResolvedValue({ "11": 0 }); const res = await PATCH( - patchRequest({ projectWorkstreamIds: ["10"] }), + patchRequest({ assignments: [ws("10")] }), params(), ); @@ -192,7 +203,7 @@ describe("PATCH", () => { mockCountOtherContractors.mockResolvedValue({ "11": 0 }); const res = await PATCH( - patchRequest({ projectWorkstreamIds: ["10"] }, true), + patchRequest({ assignments: [ws("10")] }, true), params(), ); @@ -209,7 +220,7 @@ describe("PATCH", () => { ]); const res = await PATCH( - patchRequest({ projectWorkstreamIds: ["10"] }, true), + patchRequest({ assignments: [ws("10")] }, true), params(), ); @@ -226,7 +237,7 @@ describe("PATCH", () => { mockFindContractorAssignments.mockResolvedValue([]); const res = await PATCH( - patchRequest({ projectWorkstreamIds: ["10"] }), + patchRequest({ assignments: [ws("10")] }), params(), ); @@ -238,7 +249,7 @@ describe("PATCH", () => { projectExists(); const res = await PATCH( - patchRequest({ projectWorkstreamIds: ["999"] }), + patchRequest({ assignments: [ws("999")] }), params(), ); @@ -250,7 +261,7 @@ describe("PATCH", () => { signedInAs([CLIENT_ORG]); projectExists(); - const res = await PATCH(patchRequest({ projectWorkstreamIds: [] }), params()); + const res = await PATCH(patchRequest({ assignments: [] }), params()); expect(res.status).toBe(400); }); @@ -260,7 +271,7 @@ describe("PATCH", () => { projectExists(); const res = await PATCH( - patchRequest({ projectWorkstreamIds: ["10"] }), + patchRequest({ assignments: [ws("10")] }), params("apex"), ); @@ -272,7 +283,7 @@ describe("PATCH", () => { projectExists(); const res = await PATCH( - patchRequest({ projectWorkstreamIds: ["10"] }), + patchRequest({ assignments: [ws("10")] }), params(), ); @@ -284,7 +295,7 @@ describe("PATCH", () => { projectExists(); const res = await PATCH( - patchRequest({ projectWorkstreamIds: ["10"] }), + patchRequest({ assignments: [ws("10")] }), params(), ); diff --git a/src/app/api/projects/[projectId]/contractors/[organisationId]/route.ts b/src/app/api/projects/[projectId]/contractors/[organisationId]/route.ts index 54e1a695..6ba1c0c0 100644 --- a/src/app/api/projects/[projectId]/contractors/[organisationId]/route.ts +++ b/src/app/api/projects/[projectId]/contractors/[organisationId]/route.ts @@ -58,8 +58,8 @@ async function refuseRemoval( } /** - * PATCH — set exactly which workstreams this contractor delivers, and with - * what permissions. + * PATCH — set exactly which workstreams this contractor delivers, and the + * permissions each of those carries. * * Workstreams dropped from the set are removed, so the removal rules apply to * them. An empty set would be a removal of the whole contractor; that is what @@ -96,14 +96,14 @@ export async function PATCH(req: NextRequest, props: Params) { (w) => w.projectWorkstreamId, ), ); - if (body.projectWorkstreamIds.some((id) => !owned.has(id))) { + if (body.assignments.some((a) => !owned.has(a.projectWorkstreamId))) { return NextResponse.json( { error: "Workstream not found on this project" }, { status: 404 }, ); } - const wanted = new Set(body.projectWorkstreamIds); + const wanted = new Set(body.assignments.map((a) => a.projectWorkstreamId)); const dropped = current.filter((a) => !wanted.has(a.projectWorkstreamId)); const refusal = await refuseRemoval( auth.projectId, @@ -117,11 +117,11 @@ export async function PATCH(req: NextRequest, props: Params) { const result = await writeContractorAssignments( auth.projectId, organisationId, - { - projectWorkstreamIds: body.projectWorkstreamIds.map(BigInt), - updateStagesPermission: body.updateStagesPermission, - uploadDocumentsPermission: body.uploadDocumentsPermission, - }, + body.assignments.map((a) => ({ + projectWorkstreamId: BigInt(a.projectWorkstreamId), + updateStagesPermission: a.updateStagesPermission, + uploadDocumentsPermission: a.uploadDocumentsPermission, + })), "replace", ); return NextResponse.json({ organisationId, ...result }); diff --git a/src/app/api/projects/[projectId]/contractors/assignments.test.ts b/src/app/api/projects/[projectId]/contractors/assignments.test.ts index 8cf00216..1c3db078 100644 --- a/src/app/api/projects/[projectId]/contractors/assignments.test.ts +++ b/src/app/api/projects/[projectId]/contractors/assignments.test.ts @@ -7,8 +7,6 @@ import { decideRemoval, groupByContractor, listNames, - permissionState, - splitByPermission, type ContractorAssignment, type ContractorAssignmentRow, } from "./assignments"; @@ -46,23 +44,8 @@ function assignment( }; } -describe("permissionState", () => { - it("reads an empty set as off — nothing grants the permission", () => { - expect(permissionState([])).toBe("off"); - }); - - it("reads all-true as on and all-false as off", () => { - expect(permissionState([true, true])).toBe("on"); - expect(permissionState([false, false])).toBe("off"); - }); - - it("reads a disagreement as mixed rather than rounding it", () => { - expect(permissionState([true, false])).toBe("mixed"); - }); -}); - describe("groupByContractor", () => { - it("collapses per-workstream rows into one row per organisation", () => { + it("gathers per-workstream rows under one row per organisation", () => { const groups = groupByContractor([ row({ id: "1", projectWorkstreamId: "10", workstreamName: "Windows" }), row({ id: "2", projectWorkstreamId: "11", workstreamName: "Roofs" }), @@ -99,14 +82,38 @@ describe("groupByContractor", () => { ]); }); - it("reports a permission that disagrees across assignments as mixed", () => { + it("keeps each assignment's own permissions rather than reducing them", () => { const groups = groupByContractor([ - row({ id: "1", updateStagesPermission: true }), - row({ id: "2", projectWorkstreamId: "11", updateStagesPermission: false }), + row({ + id: "1", + workstreamName: "Windows", + updateStagesPermission: true, + uploadDocumentsPermission: false, + }), + row({ + id: "2", + projectWorkstreamId: "11", + workstreamName: "Roofs", + updateStagesPermission: false, + uploadDocumentsPermission: true, + }), ]); - expect(groups[0].updateStages).toBe("mixed"); - expect(groups[0].uploadDocuments).toBe("on"); + expect(groups[0].assignments).toEqual([ + expect.objectContaining({ + workstreamName: "Roofs", + updateStagesPermission: false, + uploadDocumentsPermission: true, + }), + expect.objectContaining({ + workstreamName: "Windows", + updateStagesPermission: true, + uploadDocumentsPermission: false, + }), + ]); + // No per-contractor summary exists to disagree with the detail. + expect(groups[0]).not.toHaveProperty("updateStages"); + expect(groups[0]).not.toHaveProperty("uploadDocuments"); }); it("totals work orders across the contractor's assignments", () => { @@ -123,75 +130,6 @@ describe("groupByContractor", () => { }); }); -describe("splitByPermission", () => { - const assignments = [ - assignment({ - id: "1", - workstreamName: "Windows", - updateStagesPermission: true, - uploadDocumentsPermission: false, - }), - assignment({ - id: "2", - projectWorkstreamId: "11", - workstreamName: "Doors", - updateStagesPermission: false, - uploadDocumentsPermission: false, - }), - assignment({ - id: "3", - projectWorkstreamId: "12", - workstreamName: "Asbestos", - updateStagesPermission: true, - uploadDocumentsPermission: false, - }), - ]; - - it("names the workstreams that have the permission and those that don't", () => { - expect(splitByPermission(assignments, "updateStagesPermission")).toEqual({ - on: ["Asbestos", "Windows"], - off: ["Doors"], - }); - }); - - it("reads each permission independently", () => { - expect(splitByPermission(assignments, "uploadDocumentsPermission")).toEqual({ - on: [], - off: ["Asbestos", "Doors", "Windows"], - }); - }); - - it("orders both lists by workstream name, matching the chips", () => { - const { on } = splitByPermission( - [ - assignment({ id: "1", workstreamName: "Windows" }), - assignment({ id: "2", projectWorkstreamId: "11", workstreamName: "Doors" }), - ], - "updateStagesPermission", - ); - - expect(on).toEqual(["Doors", "Windows"]); - }); - - it("does not mutate the assignments it was given", () => { - const original = [ - assignment({ id: "1", workstreamName: "Windows" }), - assignment({ id: "2", projectWorkstreamId: "11", workstreamName: "Doors" }), - ]; - - splitByPermission(original, "updateStagesPermission"); - - expect(original.map((a) => a.workstreamName)).toEqual(["Windows", "Doors"]); - }); - - it("handles a contractor with no assignments", () => { - expect(splitByPermission([], "updateStagesPermission")).toEqual({ - on: [], - off: [], - }); - }); -}); - describe("listNames", () => { it("joins with commas and a final 'and'", () => { expect(listNames([])).toBe(""); diff --git a/src/app/api/projects/[projectId]/contractors/assignments.ts b/src/app/api/projects/[projectId]/contractors/assignments.ts index 0ff410a9..2937705e 100644 --- a/src/app/api/projects/[projectId]/contractors/assignments.ts +++ b/src/app/api/projects/[projectId]/contractors/assignments.ts @@ -8,12 +8,11 @@ * Two things live here: * * 1. **Grouping.** `project_workstream_contractor` is one row per - * (Project Workstream, organisation) pair, but the permissions table - * (wireframe `06-contractors-permissions.html`) has one row per - * *contractor organisation*, listing the workstreams it delivers. The two - * permission flags belong to the assignment, not to the organisation, so a - * contractor whose assignments disagree is reported as `"mixed"` rather - * than being silently rounded to on or off. + * (Project Workstream, organisation) pair. The table has one row per + * *contractor organisation*, listing the workstreams it delivers, and + * expands to show the permissions each of those assignments carries — so + * grouping rearranges the rows without ever collapsing their permissions + * into a single per-contractor answer (ADR-0022). * * 2. **Removal.** Work orders reference their assignment by a NOT NULL FK * (`work_order.project_workstream_contractor_id`), so an assignment with @@ -26,9 +25,6 @@ * import screen afterwards. */ -/** How a permission reads across all of one contractor's assignments. */ -export type PermissionState = "on" | "off" | "mixed"; - /** One `project_workstream_contractor` row, flattened and serialised. */ export interface ContractorAssignment { /** `project_workstream_contractor.id`. */ @@ -55,28 +51,21 @@ export interface ContractorGroup { organisationName: string; /** This contractor's assignments, ordered by workstream name. */ assignments: ContractorAssignment[]; - updateStages: PermissionState; - uploadDocuments: PermissionState; /** Work orders across every assignment — non-zero means it cannot be removed. */ workOrders: number; } /** - * How one permission reads across a set of assignments. - * - * An empty set reads `"off"`: no assignment grants it, so nothing is permitted. - */ -export function permissionState(values: boolean[]): PermissionState { - if (values.length === 0) return "off"; - if (values.every(Boolean)) return "on"; - if (values.every((v) => !v)) return "off"; - return "mixed"; -} - -/** - * Collapse per-workstream assignment rows into one row per contractor + * Gather per-workstream assignment rows under one row per contractor * organisation, ordered by organisation name then workstream name so the table * is stable between refetches. + * + * The permissions stay on the assignments. There is deliberately no + * per-contractor summary of them: the table's collapsed row shows who delivers + * what, and the expanded sub-row shows each assignment's own permissions + * (ADR-0022). Nothing needs the two flags reduced to one answer, and reducing + * them is how a contractor allowed to update stages on one workstream ends up + * looking allowed everywhere. */ export function groupByContractor( rows: ContractorAssignmentRow[], @@ -90,8 +79,6 @@ export function groupByContractor( organisationId: row.organisationId, organisationName: row.organisationName, assignments: [], - updateStages: "off", - uploadDocuments: "off", workOrders: 0, }; groups.set(row.organisationId, group); @@ -113,12 +100,6 @@ export function groupByContractor( ); return { ...group, - updateStages: permissionState( - group.assignments.map((a) => a.updateStagesPermission), - ), - uploadDocuments: permissionState( - group.assignments.map((a) => a.uploadDocumentsPermission), - ), workOrders: group.assignments.reduce((n, a) => n + a.workOrders, 0), }; }) @@ -130,26 +111,6 @@ export type PermissionKey = | "updateStagesPermission" | "uploadDocumentsPermission"; -/** - * Split a contractor's assignments by whether they carry one permission — - * the detail behind a `"mixed"` switch, so the UI can name the workstreams - * that have it and the ones that don't instead of just saying they differ. - * - * Both lists come back ordered by workstream name, matching the chips. - */ -export function splitByPermission( - assignments: ContractorAssignment[], - permission: PermissionKey, -): { on: string[]; off: string[] } { - const byName = [...assignments].sort((a, b) => - a.workstreamName.localeCompare(b.workstreamName), - ); - return { - on: byName.filter((a) => a[permission]).map((a) => a.workstreamName), - off: byName.filter((a) => !a[permission]).map((a) => a.workstreamName), - }; -} - /** `"Windows, Roofs and Doors"`. */ export function listNames(names: string[]): string { if (names.length === 0) return ""; diff --git a/src/app/api/projects/[projectId]/contractors/queries.ts b/src/app/api/projects/[projectId]/contractors/queries.ts index 8904fe42..20cdccac 100644 --- a/src/app/api/projects/[projectId]/contractors/queries.ts +++ b/src/app/api/projects/[projectId]/contractors/queries.ts @@ -267,13 +267,24 @@ export async function countOtherContractors( ); } -/** What one contractor's assignments should look like after a write. */ +/** + * One workstream and the permissions it should carry — the unit the grid + * edits. Permissions travel per workstream, never as one pair for the whole + * contractor (ADR-0022). + */ export interface DesiredAssignment { - projectWorkstreamIds: bigint[]; + projectWorkstreamId: bigint; updateStagesPermission: boolean; uploadDocumentsPermission: boolean; } +/** `"1-0"` — the flag pair, so rows sharing one can be updated together. */ +function flagKey(desired: DesiredAssignment): string { + return `${desired.updateStagesPermission ? 1 : 0}-${ + desired.uploadDocumentsPermission ? 1 : 0 + }`; +} + /** * Write one contractor's assignments on a project. * @@ -287,13 +298,17 @@ export interface DesiredAssignment { * a transaction — the narrowest guard available without a migration, matching * `selectWorkstream` in the workstream step. * + * Updates are grouped by flag pair rather than issued per row: there are only + * four possible pairs, so a contractor on twenty workstreams still costs at + * most four UPDATEs. + * * `assigned_at` is left to its column default, so a re-permissioned assignment * keeps the timestamp it was first made at. */ export async function writeContractorAssignments( projectId: bigint, organisationId: string, - desired: DesiredAssignment, + desired: DesiredAssignment[], mode: "merge" | "replace", ): Promise<{ created: number; updated: number; removed: number }> { return db.transaction(async (tx) => { @@ -320,37 +335,53 @@ export async function writeContractorAssignments( ), ); - const wanted = new Set(desired.projectWorkstreamIds.map(String)); + const wanted = new Map( + desired.map((row) => [row.projectWorkstreamId.toString(), row]), + ); const held = new Map( existing.map((row) => [row.projectWorkstreamId.toString(), row.id]), ); - const toCreate = desired.projectWorkstreamIds.filter( - (id) => !held.has(id.toString()), + const toCreate = desired.filter( + (row) => !held.has(row.projectWorkstreamId.toString()), ); - const toUpdate = existing - .filter((row) => wanted.has(row.projectWorkstreamId.toString())) - .map((row) => row.id); + + // Existing rows the request still wants, bucketed by the flag pair they + // should end up with. + const updateBuckets = new Map< + string, + { flags: DesiredAssignment; ids: bigint[] } + >(); + let updated = 0; + for (const row of existing) { + const target = wanted.get(row.projectWorkstreamId.toString()); + if (!target) continue; + const key = flagKey(target); + const bucket = updateBuckets.get(key) ?? { flags: target, ids: [] }; + bucket.ids.push(row.id); + updateBuckets.set(key, bucket); + updated += 1; + } if (toCreate.length > 0) { await tx.insert(projectWorkstreamContractor).values( - toCreate.map((projectWorkstreamId) => ({ - projectWorkstreamId, + toCreate.map((row) => ({ + projectWorkstreamId: row.projectWorkstreamId, organisationId, - updateStagesPermission: desired.updateStagesPermission, - uploadDocumentsPermission: desired.uploadDocumentsPermission, + updateStagesPermission: row.updateStagesPermission, + uploadDocumentsPermission: row.uploadDocumentsPermission, })), ); } - if (toUpdate.length > 0) { + for (const { flags, ids } of updateBuckets.values()) { await tx .update(projectWorkstreamContractor) .set({ - updateStagesPermission: desired.updateStagesPermission, - uploadDocumentsPermission: desired.uploadDocumentsPermission, + updateStagesPermission: flags.updateStagesPermission, + uploadDocumentsPermission: flags.uploadDocumentsPermission, }) - .where(inArray(projectWorkstreamContractor.id, toUpdate)); + .where(inArray(projectWorkstreamContractor.id, ids)); } let removed = 0; @@ -367,7 +398,7 @@ export async function writeContractorAssignments( } } - return { created: toCreate.length, updated: toUpdate.length, removed }; + return { created: toCreate.length, updated, removed }; }); } diff --git a/src/app/api/projects/[projectId]/contractors/route.test.ts b/src/app/api/projects/[projectId]/contractors/route.test.ts index 2c0184e4..743a1ea8 100644 --- a/src/app/api/projects/[projectId]/contractors/route.test.ts +++ b/src/app/api/projects/[projectId]/contractors/route.test.ts @@ -82,11 +82,22 @@ function getRequest() { return new NextRequest("http://test/api/projects/5/contractors"); } +/** One grid row: a workstream and the permissions it should carry. */ +function ws( + projectWorkstreamId: string, + updateStagesPermission = false, + uploadDocumentsPermission = false, +) { + return { + projectWorkstreamId, + updateStagesPermission, + uploadDocumentsPermission, + }; +} + const VALID_BODY = { organisationId: APEX, - projectWorkstreamIds: ["10", "11"], - updateStagesPermission: true, - uploadDocumentsPermission: false, + assignments: [ws("10", true, false), ws("11", false, true)], }; beforeEach(() => { @@ -159,11 +170,18 @@ describe("POST /api/projects/[projectId]/contractors", () => { expect(mockWriteContractorAssignments).toHaveBeenCalledWith( 5n, APEX, - { - projectWorkstreamIds: [10n, 11n], - updateStagesPermission: true, - uploadDocumentsPermission: false, - }, + [ + { + projectWorkstreamId: 10n, + updateStagesPermission: true, + uploadDocumentsPermission: false, + }, + { + projectWorkstreamId: 11n, + updateStagesPermission: false, + uploadDocumentsPermission: true, + }, + ], "merge", ); }); @@ -188,17 +206,23 @@ describe("POST /api/projects/[projectId]/contractors", () => { projectExists(); await POST( - postRequest({ organisationId: APEX, projectWorkstreamIds: ["10"] }), + postRequest({ + organisationId: APEX, + assignments: [{ projectWorkstreamId: "10" }], + }), params, ); expect(mockWriteContractorAssignments).toHaveBeenCalledWith( 5n, APEX, - expect.objectContaining({ - updateStagesPermission: false, - uploadDocumentsPermission: false, - }), + [ + { + projectWorkstreamId: 10n, + updateStagesPermission: false, + uploadDocumentsPermission: false, + }, + ], "merge", ); }); @@ -218,7 +242,7 @@ describe("POST /api/projects/[projectId]/contractors", () => { projectExists(); const res = await POST( - postRequest({ organisationId: APEX, projectWorkstreamIds: [] }), + postRequest({ organisationId: APEX, assignments: [] }), params, ); @@ -227,12 +251,55 @@ describe("POST /api/projects/[projectId]/contractors", () => { expect(mockWriteContractorAssignments).not.toHaveBeenCalled(); }); + it("rejects a workstream named twice — two answers to one question", async () => { + signedInAs([CLIENT_ORG]); + projectExists(); + + const res = await POST( + postRequest({ + organisationId: APEX, + assignments: [ws("10", true, true), ws("10", false, false)], + }), + params, + ); + + expect(res.status).toBe(400); + expect(mockWriteContractorAssignments).not.toHaveBeenCalled(); + }); + + it("carries different permissions per workstream through untouched", async () => { + signedInAs([CLIENT_ORG]); + projectExists(); + + await POST( + postRequest({ + organisationId: APEX, + assignments: [ws("10", true, false), ws("11", false, false)], + }), + params, + ); + + const [, , assignments] = mockWriteContractorAssignments.mock.calls[0]; + expect(assignments).toEqual([ + { + projectWorkstreamId: 10n, + updateStagesPermission: true, + uploadDocumentsPermission: false, + }, + { + projectWorkstreamId: 11n, + updateStagesPermission: false, + uploadDocumentsPermission: false, + }, + ]); + }); + it("rejects an organisationId that is not a uuid", async () => { signedInAs([CLIENT_ORG]); projectExists(); const res = await POST( - postRequest({ organisationId: "apex", projectWorkstreamIds: ["10"] }), + postRequest({ organisationId: "apex", assignments: [ws("10")] }), params, ); @@ -258,7 +325,7 @@ describe("POST /api/projects/[projectId]/contractors", () => { projectExists(); const res = await POST( - postRequest({ ...VALID_BODY, projectWorkstreamIds: ["10", "999"] }), + postRequest({ ...VALID_BODY, assignments: [ws("10"), ws("999")] }), params, ); diff --git a/src/app/api/projects/[projectId]/contractors/route.ts b/src/app/api/projects/[projectId]/contractors/route.ts index 6318afd0..00f62520 100644 --- a/src/app/api/projects/[projectId]/contractors/route.ts +++ b/src/app/api/projects/[projectId]/contractors/route.ts @@ -38,7 +38,8 @@ export async function GET(_req: NextRequest, props: Params) { } /** - * POST — assign a contractor to one or more of the project's workstreams. + * POST — assign a contractor to one or more of the project's workstreams, each + * with its own permissions. * * Additive: workstreams the body does not name are left alone, so adding a * second contractor never displaces the first (the schema allows several per @@ -73,7 +74,7 @@ export async function POST(req: NextRequest, props: Params) { (w) => w.projectWorkstreamId, ), ); - if (body.projectWorkstreamIds.some((id) => !owned.has(id))) { + if (body.assignments.some((a) => !owned.has(a.projectWorkstreamId))) { return NextResponse.json( { error: "Workstream not found on this project" }, { status: 404 }, @@ -84,11 +85,11 @@ export async function POST(req: NextRequest, props: Params) { const result = await writeContractorAssignments( auth.projectId, body.organisationId, - { - projectWorkstreamIds: body.projectWorkstreamIds.map(BigInt), - updateStagesPermission: body.updateStagesPermission, - uploadDocumentsPermission: body.uploadDocumentsPermission, - }, + body.assignments.map((a) => ({ + projectWorkstreamId: BigInt(a.projectWorkstreamId), + updateStagesPermission: a.updateStagesPermission, + uploadDocumentsPermission: a.uploadDocumentsPermission, + })), "merge", ); return NextResponse.json( diff --git a/src/app/api/projects/[projectId]/contractors/validation.ts b/src/app/api/projects/[projectId]/contractors/validation.ts index 1f38a390..7520af2c 100644 --- a/src/app/api/projects/[projectId]/contractors/validation.ts +++ b/src/app/api/projects/[projectId]/contractors/validation.ts @@ -8,24 +8,45 @@ import { z } from "zod"; /** - * The assignment a write asks for. + * One (workstream, permissions) triple — the unit the grid edits and the unit + * `project_workstream_contractor` stores. * - * `project_workstream.id` values arrive as strings — they are bigint in the - * database and do not survive JSON. The permission flags default to false, - * matching the column defaults: an assignment grants nothing unless asked. + * `project_workstream.id` arrives as a string: it is bigint in the database and + * does not survive JSON. Both flags default to false, matching the column + * defaults — an assignment grants nothing unless asked. */ -export const assignmentBodySchema = z.object({ - projectWorkstreamIds: z - .array(z.string().regex(/^\d+$/, "workstream ids must be numeric")) - .min(1, "Choose at least one workstream"), +export const assignmentSchema = z.object({ + projectWorkstreamId: z + .string() + .regex(/^\d+$/, "workstream ids must be numeric"), updateStagesPermission: z.boolean().default(false), uploadDocumentsPermission: z.boolean().default(false), }); +/** + * The assignment set a write asks for — permissions per workstream, never one + * pair applied to all of them (ADR-0022). + * + * A workstream may appear only once: two entries for the same workstream carry + * two answers to the same question, and picking one silently is worse than + * refusing. + */ +export const assignmentBodySchema = z.object({ + assignments: z + .array(assignmentSchema) + .min(1, "Choose at least one workstream") + .refine( + (rows) => + new Set(rows.map((r) => r.projectWorkstreamId)).size === rows.length, + { message: "Each workstream may appear only once" }, + ), +}); + /** Adding a contractor also names which organisation is being assigned. */ export const addContractorBodySchema = assignmentBodySchema.extend({ organisationId: z.string().uuid("organisationId must be a uuid"), }); +export type AssignmentInput = z.infer; export type AssignmentBody = z.infer; export type AddContractorBody = z.infer; diff --git a/src/app/projects/[projectId]/setup/contractors/ContractorFormDialog.tsx b/src/app/projects/[projectId]/setup/contractors/ContractorFormDialog.tsx index 8335aa20..f526dcdb 100644 --- a/src/app/projects/[projectId]/setup/contractors/ContractorFormDialog.tsx +++ b/src/app/projects/[projectId]/setup/contractors/ContractorFormDialog.tsx @@ -14,15 +14,15 @@ import { import { Button } from "@/app/shadcn_components/ui/button"; import { Input } from "@/app/shadcn_components/ui/input"; import { Checkbox } from "@/app/shadcn_components/ui/checkbox"; -import { - listNames, - splitByPermission, - type ContractorGroup, +import type { + ContractorGroup, + PermissionKey, } from "@/app/api/projects/[projectId]/contractors/assignments"; import type { OrganisationOption, ProjectWorkstreamOption, } from "@/app/api/projects/[projectId]/contractors/queries"; +import { PERMISSION_COLUMNS } from "./PermissionCell"; /** * Add or edit one contractor (issue #413). @@ -32,6 +32,15 @@ import type { * belongs to the "invite company to Ara" flow, not here. That is why there is * no "create organisation" affordance and no free-text name field. * + * Below it sits **the grid**: one row per workstream the project covers, with + * a tick to assign it and a tick per permission (ADR-0022). Workstreams this + * contractor does not deliver stay in the grid, greyed, rather than being + * hidden — the shape of the project is part of the decision, and a workstream + * nobody covers is worth seeing while assigning someone. Because each row + * carries its own permissions, "Apex may move Windows along but only upload on + * Roofs" is expressible here, which is exactly what the schema has always + * stored. + * * The dialog is mounted only while open and keyed by the contractor it edits, * so its `useState` initialisers *are* the "load the current values" step — * no effect synchronises props into state. @@ -51,6 +60,20 @@ interface SaveOutcome { reason?: string; } +/** One row of the grid: a workstream, whether it is assigned, and its flags. */ +interface GridRow { + projectWorkstreamId: string; + name: string; + /** Contractors already on this workstream — 0 is what blocks import. */ + contractorCount: number; + assigned: boolean; + updateStagesPermission: boolean; + uploadDocumentsPermission: boolean; + /** Work orders under *this contractor's* assignment, if it has one. */ + workOrders: number; +} + + export function ContractorFormDialog({ projectId, mode, @@ -70,19 +93,34 @@ export function ContractorFormDialog({ const [organisation, setOrganisation] = useState<{ id: string; name: string; + alreadyAssigned: boolean; } | null>( editing - ? { id: editing.organisationId, name: editing.organisationName } + ? { + id: editing.organisationId, + name: editing.organisationName, + alreadyAssigned: true, + } : null, ); - const [selected, setSelected] = useState( - editing ? editing.assignments.map((a) => a.projectWorkstreamId) : [], - ); - const [updateStages, setUpdateStages] = useState( - editing ? editing.updateStages === "on" : false, - ); - const [uploadDocuments, setUploadDocuments] = useState( - editing ? editing.uploadDocuments === "on" : false, + // One entry per project workstream — the grid's rows. Unassigned workstreams + // are present but not `assigned`, so they render greyed rather than missing: + // the user sees everything the project covers and what is still uncovered. + const [rows, setRows] = useState(() => + workstreams.map((workstream) => { + const held = editing?.assignments.find( + (a) => a.projectWorkstreamId === workstream.projectWorkstreamId, + ); + return { + projectWorkstreamId: workstream.projectWorkstreamId, + name: workstream.name, + contractorCount: workstream.contractorCount, + assigned: Boolean(held), + updateStagesPermission: held?.updateStagesPermission ?? false, + uploadDocumentsPermission: held?.uploadDocumentsPermission ?? false, + workOrders: held?.workOrders ?? 0, + }; + }), ); const [warning, setWarning] = useState(null); @@ -107,9 +145,13 @@ export function ContractorFormDialog({ if (!organisation) throw new Error("Choose an organisation first"); const payload = { - projectWorkstreamIds: selected, - updateStagesPermission: updateStages, - uploadDocumentsPermission: uploadDocuments, + assignments: rows + .filter((row) => row.assigned) + .map((row) => ({ + projectWorkstreamId: row.projectWorkstreamId, + updateStagesPermission: row.updateStagesPermission, + uploadDocumentsPermission: row.uploadDocumentsPermission, + })), }; const res = editing @@ -152,46 +194,52 @@ export function ContractorFormDialog({ onError: (err: Error) => setWarning({ kind: "blocked", reason: err.message }), }); - function toggleWorkstream(id: string) { + function updateRow(id: string, change: Partial) { setWarning(null); - setSelected((current) => - current.includes(id) - ? current.filter((value) => value !== id) - : [...current, id], + setRows((current) => + current.map((row) => + row.projectWorkstreamId === id ? { ...row, ...change } : row, + ), ); } - const canSave = - Boolean(organisation) && selected.length > 0 && !save.isLoading; + /** + * Un-assigning a workstream drops its permissions too. Keeping them set on a + * row nobody delivers would re-grant them silently if it were re-ticked. + */ + function toggleAssigned(row: GridRow, assigned: boolean) { + updateRow( + row.projectWorkstreamId, + assigned + ? { assigned: true } + : { + assigned: false, + updateStagesPermission: false, + uploadDocumentsPermission: false, + }, + ); + } - // Which permissions this contractor currently holds inconsistently, and how. - // Named rather than merely flagged: the user is about to overwrite the split, - // so they should see what it is before they do. - const mixedPermissions = editing - ? ( - [ - { - label: "Update stages of workstream(s)", - state: editing.updateStages, - key: "updateStagesPermission", - }, - { - label: "Upload docs", - state: editing.uploadDocuments, - key: "uploadDocumentsPermission", - }, - ] as const - ) - .filter((permission) => permission.state === "mixed") - .map(({ label, key }) => ({ - label, - ...splitByPermission(editing.assignments, key), - })) - : []; + /** Set one permission across every assigned row — the column header's job. */ + function setColumn(permission: PermissionKey, value: boolean) { + setWarning(null); + setRows((current) => + current.map((row) => + row.assigned ? { ...row, [permission]: value } : row, + ), + ); + } + + const assignedRows = rows.filter((row) => row.assigned); + const canSave = + Boolean(organisation) && assignedRows.length > 0 && !save.isLoading; + + const columnState = (permission: PermissionKey) => + assignedRows.length > 0 && assignedRows.every((row) => row[permission]); return ( !open && onClose()}> - + {editing ? `Edit ${editing.organisationName}` : "Add contractor"} @@ -212,18 +260,30 @@ export function ContractorFormDialog({ Organisation {organisation ? ( -
- - {organisation.name} - - -
+ <> +
+ + {organisation.name} + + +
+ {organisation.alreadyAssigned && ( +

+ Already delivers work on this project. What you tick below is + added to what it already has — to change existing + permissions, close this and use Edit on its row. +

+ )} + ) : ( <>
@@ -260,7 +320,11 @@ export function ContractorFormDialog({ type="button" className="flex w-full items-center justify-between px-3 py-2 text-left text-sm hover:bg-gray-50" onClick={() => - setOrganisation({ id: option.id, name: option.name }) + setOrganisation({ + id: option.id, + name: option.name, + alreadyAssigned: option.alreadyAssigned, + }) } > @@ -282,91 +346,114 @@ export function ContractorFormDialog({
- Workstreams + Workstreams and permissions - {workstreams.length === 0 ? ( + {rows.length === 0 ? (

This project has no workstreams yet, so there is nothing to assign a contractor to.

) : ( -
    - {workstreams.map((option) => ( -
  • - -
  • - ))} -
- )} -
- -
- - Permissions - - - - {editing && mixedPermissions.length > 0 && ( -
-

- {editing.organisationName} holds these differently on different - workstreams today: -

-
    - {mixedPermissions.map(({ label, on, off }) => ( -
  • - {label} — on for{" "} - {listNames(on)}, off for {listNames(off)} -
  • - ))} -
-

- Saving applies the boxes above to every workstream ticked, which - clears the split. -

+
+ + + + + {PERMISSION_COLUMNS.map((column) => ( + + ))} + + + + {rows.map((row) => ( + + + {PERMISSION_COLUMNS.map((column) => ( + + ))} + + ))} + +
Workstream + {column.header} + +
+ + + + updateRow(row.projectWorkstreamId, { + [column.key]: value === true, + }) + } + aria-label={`${column.header} on ${row.name}`} + data-testid={`grid-${column.key}-${row.projectWorkstreamId}`} + /> +
)} +

+ Update stages moves work orders + along the stage ladder; Upload docs{" "} + attaches evidence against them. Both are set per workstream, so a + contractor can be trusted with one workstream and not another. +

{warning && ( diff --git a/src/app/projects/[projectId]/setup/contractors/ContractorPermissionsTable.tsx b/src/app/projects/[projectId]/setup/contractors/ContractorPermissionsTable.tsx index 5ea29c55..1037541d 100644 --- a/src/app/projects/[projectId]/setup/contractors/ContractorPermissionsTable.tsx +++ b/src/app/projects/[projectId]/setup/contractors/ContractorPermissionsTable.tsx @@ -1,18 +1,23 @@ "use client"; -import { useState } from "react"; +import { Fragment, useState } from "react"; import Link from "next/link"; import { useRouter } from "next/navigation"; import { useMutation, useQuery, useQueryClient } from "@tanstack/react-query"; -import { ArrowLeft, ArrowRight, Info, Pencil, Trash2, UserPlus } from "lucide-react"; +import { + ArrowLeft, + ArrowRight, + ChevronRight, + Info, + Pencil, + Trash2, + UserPlus, +} from "lucide-react"; import { Button } from "@/app/shadcn_components/ui/button"; import { ConfirmDialog } from "@/app/components/ConfirmDialog"; -import { - splitByPermission, - type ContractorGroup, -} from "@/app/api/projects/[projectId]/contractors/assignments"; +import type { ContractorGroup } from "@/app/api/projects/[projectId]/contractors/assignments"; import type { ProjectWorkstreamOption } from "@/app/api/projects/[projectId]/contractors/queries"; -import { PermissionToggle } from "./PermissionToggle"; +import { PERMISSION_COLUMNS, PermissionState } from "./PermissionCell"; import { ContractorFormDialog, type ContractorFormMode, @@ -85,6 +90,9 @@ export function ContractorPermissionsTable({ const [notice, setNotice] = useState(null); const [form, setForm] = useState(null); + // Organisation ids whose permission detail is open. Several may be at once — + // comparing two contractors is a normal thing to want. + const [expanded, setExpanded] = useState([]); const [pendingRemove, setPendingRemove] = useState<{ contractor: ContractorGroup; reason: string; @@ -112,42 +120,6 @@ export function ContractorPermissionsTable({ router.refresh(); }; - const setPermission = useMutation({ - mutationFn: async ({ - contractor, - updateStages, - uploadDocuments, - }: { - contractor: ContractorGroup; - updateStages: boolean; - uploadDocuments: boolean; - }) => { - const res = await fetch( - `/api/projects/${projectId}/contractors/${contractor.organisationId}`, - { - method: "PATCH", - headers: { "content-type": "application/json" }, - // The same workstreams, so no assignment is dropped and the removal - // rules have nothing to say — only the two flags change. - body: JSON.stringify({ - projectWorkstreamIds: contractor.assignments.map( - (a) => a.projectWorkstreamId, - ), - updateStagesPermission: updateStages, - uploadDocumentsPermission: uploadDocuments, - }), - }, - ); - const body = await res.json().catch(() => ({})); - if (!res.ok) throw new Error(body.error ?? "Couldn't save the permission"); - }, - onSuccess: () => { - setNotice(null); - refresh(); - }, - onError: (err: Error) => setNotice({ tone: "error", text: err.message }), - }); - const remove = useMutation({ mutationFn: async ({ contractor, @@ -193,11 +165,16 @@ export function ContractorPermissionsTable({ }, }); - const busyOrganisationId = setPermission.isLoading - ? setPermission.variables?.contractor.organisationId - : remove.isLoading - ? remove.variables?.contractor.organisationId - : undefined; + const busyOrganisationId = remove.isLoading + ? remove.variables?.contractor.organisationId + : undefined; + + const toggleExpanded = (organisationId: string) => + setExpanded((current) => + current.includes(organisationId) + ? current.filter((id) => id !== organisationId) + : [...current, organisationId], + ); const { contractors, workstreams, canManage } = data; const unassigned = workstreams.filter((w) => w.contractorCount === 0); @@ -288,18 +265,15 @@ export function ContractorPermissionsTable({ + - - @@ -307,131 +281,182 @@ export function ContractorPermissionsTable({ {contractors.map((contractor) => { - const busy = - busyOrganisationId === contractor.organisationId; + const busy = busyOrganisationId === contractor.organisationId; const frozen = - !canManage || - (Boolean(busyOrganisationId) && !busy); - - // Which workstreams hold each permission — the detail behind a - // "mixed" switch, so it can say how it is mixed. - const stagesSplit = splitByPermission( - contractor.assignments, - "updateStagesPermission", - ); - const documentsSplit = splitByPermission( - contractor.assignments, - "uploadDocumentsPermission", - ); + !canManage || (Boolean(busyOrganisationId) && !busy); + const isOpen = expanded.includes(contractor.organisationId); + const detailId = `contractor-permissions-${contractor.organisationId}`; return ( - - - toggleExpanded(contractor.organisationId)} + data-testid={`contractor-row-${contractor.organisationId}`} + > + + - - - - + + {contractor.organisationName} + + + {contractor.workOrders > 0 && ( + + {contractor.workOrders}{" "} + {contractor.workOrders === 1 + ? "work order" + : "work orders"}{" "} + issued + + )} + + + + + + {isOpen && ( + + + + )} + ); })} diff --git a/src/app/projects/[projectId]/setup/contractors/PermissionCell.tsx b/src/app/projects/[projectId]/setup/contractors/PermissionCell.tsx new file mode 100644 index 00000000..c3194814 --- /dev/null +++ b/src/app/projects/[projectId]/setup/contractors/PermissionCell.tsx @@ -0,0 +1,59 @@ +"use client"; + +import { Check, Minus } from "lucide-react"; +import type { PermissionKey } from "@/app/api/projects/[projectId]/contractors/assignments"; + +/** + * How one assignment's permissions are named and shown (issue #413). + * + * Both the add/edit grid and the table's expanded detail describe the same two + * columns, in the same order, with the same words — so they live here once + * rather than being retyped either side of the dialog. + */ + +/** The two permission columns, in the order both grids show them. */ +export const PERMISSION_COLUMNS: Array<{ + key: PermissionKey; + header: string; +}> = [ + { key: "updateStagesPermission", header: "Update stages" }, + { key: "uploadDocumentsPermission", header: "Upload docs" }, +]; + +/** + * One permission on one workstream, read-only — what the table's sub-row shows. + * + * Deliberately not a control: the table reports the state, and the grid in the + * Edit dialog is the single place it is changed (ADR-0022). Colour is never the + * only signal — the icon and the word carry it too. + */ +export function PermissionState({ + granted, + label, + workstreamName, +}: { + granted: boolean; + /** The permission's name, e.g. "Update stages". */ + label: string; + workstreamName: string; +}) { + return ( + + {granted ? ( + + ) : ( + + )} + + {label} on {workstreamName}:{" "} + + {granted ? "Allowed" : "Not allowed"} + + ); +} diff --git a/src/app/projects/[projectId]/setup/contractors/PermissionToggle.tsx b/src/app/projects/[projectId]/setup/contractors/PermissionToggle.tsx deleted file mode 100644 index f55b3736..00000000 --- a/src/app/projects/[projectId]/setup/contractors/PermissionToggle.tsx +++ /dev/null @@ -1,170 +0,0 @@ -"use client"; - -import { Check, Info, Minus } from "lucide-react"; -import { - Tooltip, - TooltipContent, - TooltipProvider, - TooltipTrigger, -} from "@/app/shadcn_components/ui/tooltip"; -import { - listNames, - type PermissionState, -} from "@/app/api/projects/[projectId]/contractors/assignments"; - -/** - * One permission switch in the permissions table (issue #413). - * - * The two flags live on the *assignment*, not on the organisation, so a - * contractor delivering three workstreams holds three copies of each. When - * they disagree the switch reads **Mixed** rather than picking a side - * (ADR-0021). - * - * "Mixed" on its own raises more questions than it answers — mixed how, and - * what happens if I click it? — so it is explained three times over, at - * increasing depth: - * - * 1. The chip carries the count: `Mixed · 2 of 3`. No interaction needed. - * 2. Its tooltip names which workstreams have the permission and which do - * not, and states what clicking the switch will do. - * 3. The switch's own accessible name says the same thing in a sentence, so - * a screen-reader user never depends on the hover. - * - * `role="switch"` only admits `aria-checked` true or false (ARIA 1.2 — unlike - * a checkbox, a switch has no mixed value), which is the other reason the - * mixed case has to be announced through the name. - */ -export function PermissionToggle({ - state, - label, - contractorName, - testId, - workstreamsOn, - workstreamsOff, - disabled, - pending, - onChange, -}: { - state: PermissionState; - /** What the permission is, e.g. "Update stages of workstream(s)". */ - label: string; - contractorName: string; - /** Stable hook for tests — not derived from `label`, which is copy. */ - testId: string; - /** Workstream names holding this permission, ordered as the chips are. */ - workstreamsOn: string[]; - /** Workstream names without it. */ - workstreamsOff: string[]; - disabled: boolean; - pending: boolean; - onChange: (next: boolean) => void; -}) { - const on = state === "on"; - const mixed = state === "mixed"; - const total = workstreamsOn.length + workstreamsOff.length; - const plural = total === 1 ? "workstream" : "workstreams"; - - const accessibleName = mixed - ? `${label} for ${contractorName} — on for ${listNames(workstreamsOn)}, off for ${listNames( - workstreamsOff, - )}. Activating turns it on for all ${total} ${plural}.` - : `${label} for ${contractorName}`; - - return ( - - - - {mixed && ( - - - - - - -
-

- {label} -

-

- Set per workstream, and {contractorName}’s differ. -

-
- -
    - {workstreamsOn.map((name) => ( -
  • - - - {name} — allowed - -
  • - ))} - {workstreamsOff.map((name) => ( -
  • - - - {name} — not allowed - -
  • - ))} -
- -

- Turning the switch on grants it on{" "} - - all {total} {plural} - - ; turning it off revokes it from all {total}. This screen always - sets one value across everything a contractor delivers, so - saving here clears the split. -

-
-
-
- )} -
- ); -}
+ Show permissions + Contractor Assigned workstreams - Update stages of workstream(s) - - Upload docs - Actions
-
- - {contractor.organisationName.charAt(0).toUpperCase()} - - - {contractor.organisationName} - -
- {contractor.workOrders > 0 && ( - - {contractor.workOrders}{" "} - {contractor.workOrders === 1 - ? "work order" - : "work orders"}{" "} - issued - - )} -
-
- {contractor.assignments.map((assignment) => ( - - {assignment.workstreamName} + +
+ + +
+ + {contractor.organisationName.charAt(0).toUpperCase()} - ))} -
-
- - setPermission.mutate({ - contractor, - updateStages: next, - uploadDocuments: - contractor.uploadDocuments === "on", - }) - } - /> - - - setPermission.mutate({ - contractor, - updateStages: contractor.updateStages === "on", - uploadDocuments: next, - }) - } - /> - -
- - -
-
+
+ {contractor.assignments.map((assignment) => ( + + {assignment.workstreamName} + + ))} +
+
+
+ + +
+
+ +

+ What {contractor.organisationName} may do +

+ + + + + {PERMISSION_COLUMNS.map((column) => ( + + ))} + + + + {contractor.assignments.map((assignment) => ( + + + {PERMISSION_COLUMNS.map((column) => ( + + ))} + + ))} + +
+ Workstream + + {column.header} +
+ + {assignment.workstreamName} + + {assignment.workOrders > 0 && ( + + {assignment.workOrders}{" "} + {assignment.workOrders === 1 + ? "work order" + : "work orders"} + + )} + + +
+ {canManage && ( +

+ Set per workstream —{" "} + Edit to change + them. +

+ )} +