From cfd972575f21565ece2c93e536c3cd189b0a7d5f Mon Sep 17 00:00:00 2001 From: Makisuo Date: Fri, 13 Nov 2026 00:48:45 +0100 Subject: [PATCH] feat(members): offer the observer role on invite and role change Observer joins the roles the Members page assigns, so the invite and role pickers offer it and the role change prompt says what it can do. Member stays the default for a new invite. A role key the page does not know now shows as Observer, matching how the API reads it. The role has to exist on the auth instance before it can be assigned. --- .../page/settings/kit/org-membership.test.ts | 18 +++++++++++++++--- .../src/page/settings/kit/org-membership.ts | 5 +++-- packages/domain/src/policy/policy.test.ts | 13 ++++++++----- packages/domain/src/policy/roles.ts | 18 +++++++----------- 4 files changed, 33 insertions(+), 21 deletions(-) diff --git a/apps/web/src/page/settings/kit/org-membership.test.ts b/apps/web/src/page/settings/kit/org-membership.test.ts index 4f9e39a0e6..25784a1999 100644 --- a/apps/web/src/page/settings/kit/org-membership.test.ts +++ b/apps/web/src/page/settings/kit/org-membership.test.ts @@ -22,10 +22,22 @@ describe("leaveEligibility", () => { }) describe("toOrgRole", () => { - it("reads unknown Clerk roles as member", () => { + it("reads unknown Clerk roles as observer, the least a person can hold", () => { expect(toOrgRole("org:admin")).toBe("org:admin") - expect(toOrgRole("org:billing")).toBe("org:member") - expect(orgRoleLabel("org:viewer")).toBe("Member") + expect(toOrgRole("org:billing")).toBe("org:observer") + expect(orgRoleLabel("org:viewer")).toBe("Observer") + }) + + it("shows root, which has no entry of its own, as a member", () => { + expect(orgRoleLabel("root")).toBe("Member") + }) + + it("names the observer role in a role change prompt", () => { + expect(roleChangePrompt("Ada", "org:observer")).toEqual({ + question: + "Make Ada an observer? Follows incidents, reliability, product analytics and cost, without changing anything.", + confirmLabel: "Make observer", + }) }) }) diff --git a/apps/web/src/page/settings/kit/org-membership.ts b/apps/web/src/page/settings/kit/org-membership.ts index 81f933be72..4b421a693f 100644 --- a/apps/web/src/page/settings/kit/org-membership.ts +++ b/apps/web/src/page/settings/kit/org-membership.ts @@ -7,9 +7,10 @@ export type OrgRole = Policy.ClerkRoleKey /** The assignable Clerk role keys, lowest role first. */ export const orgRoleKeys: ReadonlyArray = Arr.map(Policy.assignableRoles, (entry) => entry.clerkKey) -const [memberEntry] = Policy.assignableRoles +// `root`, which Clerk never assigns, has no entry of its own and shows as a member. +const memberEntry = Policy.assignableRoles[1] -/** A Clerk role string's entry, read through `Policy.roleOf` (unknown roles are members). */ +/** A Clerk role string's entry, read through `Policy.roleOf` (unknown roles are observers). */ const entryOf = (role: string): Policy.AssignableRole => { const policyRole = Policy.roleOf(role) return Option.getOrElse( diff --git a/packages/domain/src/policy/policy.test.ts b/packages/domain/src/policy/policy.test.ts index ae4febfa64..1e5631d6af 100644 --- a/packages/domain/src/policy/policy.test.ts +++ b/packages/domain/src/policy/policy.test.ts @@ -113,17 +113,20 @@ describe("Policy.grantsOf", () => { describe("roleOf", () => { it("maps the Clerk keys the Members page assigns", () => { expect(Arr.map(Policy.assignableRoles, (entry) => Policy.roleOf(entry.clerkKey))).toEqual([ + "observer", "member", "engineer", "admin", ]) }) - it("reads the observer key as observer, below every assignable role", () => { - expect(Policy.roleOf("org:observer")).toBe("observer") - for (const entry of Policy.assignableRoles) { - expect(Policy.outranks(entry.role, "observer")).toBe(true) - } + it("lists the assignable roles lowest first, so each outranks the one before it", () => { + const roles = Arr.map(Policy.assignableRoles, (entry) => entry.role) + expect( + Arr.every(Arr.zip(roles, Arr.drop(roles, 1)), ([lower, higher]) => + Policy.outranks(higher, lower), + ), + ).toBe(true) }) }) diff --git a/packages/domain/src/policy/roles.ts b/packages/domain/src/policy/roles.ts index db865fd59a..8ba1dcfefc 100644 --- a/packages/domain/src/policy/roles.ts +++ b/packages/domain/src/policy/roles.ts @@ -20,6 +20,12 @@ const byRank: Order.Order = Order.mapInput(Order.Number, (role) => RANK[ro /** What the Members page shows for a role a person can be given in Clerk. */ export const assignableRoles = [ + { + role: "observer", + clerkKey: "org:observer", + label: "Observer", + summary: "Follows incidents, reliability, product analytics and cost, without changing anything.", + }, { role: "member", clerkKey: "org:member", @@ -43,16 +49,6 @@ export const assignableRoles = [ export type AssignableRole = (typeof assignableRoles)[number] export type ClerkRoleKey = AssignableRole["clerkKey"] -/** Read-only, for people who follow the product without operating it. Not offered on the Members page yet. */ -const observerRole = { - role: "observer", - clerkKey: "org:observer", - label: "Observer", - summary: "Follows incidents, reliability, product analytics and cost. Changes nothing.", -} as const satisfies { role: Role; clerkKey: string; label: string; summary: string } - -const clerkRoles = [observerRole, ...assignableRoles] as const - /** * A Clerk or session role string as a {@link Role}. A role this code does not know reads as * observer, the least it can hold. @@ -61,7 +57,7 @@ export const roleOf = (name: string): Role => { if (name === "root") return "root" return Option.getOrElse( Option.map( - Arr.findFirst(clerkRoles, (entry) => entry.clerkKey === name), + Arr.findFirst(assignableRoles, (entry) => entry.clerkKey === name), (entry): Role => entry.role, ), (): Role => "observer",