Skip to content

Commit e6e7e08

Browse files
committed
fix(webapp): only offer assignable roles in the team role picker
The team page's role dropdown listed every role the org's plan allows, including roles above the viewer's own level. Picking one of those was always rejected, so the dropdown advertised a change that could not happen. Narrow the list to the roles the viewer's own role lets them assign, reusing the same ladder the invite flow already applies. Plan-locked roles are kept in the list and still render as "Name (upgrade)" linking to billing: being unable to assign a role and needing a plan upgrade to assign it are different situations, and collapsing them would offer a viewer an upgrade for a role their own role would still not let them hand out. The member's current role stays listed so the dropdown keeps showing what they actually hold. TeamPresenter now returns `offerableRoleIds` (the ladder) alongside the existing `assignableRoleIds` (the plan), and the invite loader reuses both instead of re-querying — its dropdown has no upgrade affordance, so it keeps intersecting the two. Co-Authored-By: Claude <noreply@anthropic.com>
1 parent ac47902 commit e6e7e08

6 files changed

Lines changed: 150 additions & 54 deletions

File tree

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
---
2+
area: webapp
3+
type: improvement
4+
---
5+
6+
The Team page's role dropdown now only lists roles you are actually allowed to assign, instead of showing higher roles that were rejected when you picked them. Roles that need a plan upgrade still appear, with a link to upgrade.

apps/webapp/app/presenters/TeamPresenter.server.ts

Lines changed: 47 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { getTeamMembersAndInvites } from "~/models/member.server";
22
import { rbac } from "~/services/rbac.server";
33
import { getCurrentPlan, getLimit, getPlans } from "~/services/platform.v3.server";
4+
import { offerableRoleIds as computeOfferableRoleIds } from "~/utils/inviteRoleLadder";
45
import { BasePresenter } from "./v3/basePresenter.server";
56

67
export class TeamPresenter extends BasePresenter {
@@ -14,25 +15,51 @@ export class TeamPresenter extends BasePresenter {
1415
return;
1516
}
1617

17-
const [baseLimit, currentPlan, plans, roles, assignableRoleIds, memberRoleMap] =
18-
await Promise.all([
19-
getLimit(organizationId, "teamMembers", 100_000_000),
20-
getCurrentPlan(organizationId),
21-
getPlans(),
22-
// RBAC role catalogue (system roles + any org-defined custom
23-
// roles). The default fallback returns []; an installed plugin
24-
// may return the seeded system roles plus any custom roles.
25-
rbac.allRoles(organizationId),
26-
// Plan-gated subset — the Teams page disables dropdown options not
27-
// in this set. Server-side enforcement is independent (setUserRole
28-
// rejects a plan-gated assignment regardless of UI state).
29-
rbac.getAssignableRoleIds(organizationId),
30-
// Per-member current role in a single round-trip.
31-
rbac.getUserRoles(
32-
result.members.map((m) => m.user.id),
33-
organizationId
34-
),
35-
]);
18+
const [
19+
baseLimit,
20+
currentPlan,
21+
plans,
22+
roles,
23+
assignableRoleIds,
24+
memberRoleMap,
25+
viewerRole,
26+
systemRoles,
27+
] = await Promise.all([
28+
getLimit(organizationId, "teamMembers", 100_000_000),
29+
getCurrentPlan(organizationId),
30+
getPlans(),
31+
// RBAC role catalogue (system roles + any org-defined custom
32+
// roles). The default fallback returns []; an installed plugin
33+
// may return the seeded system roles plus any custom roles.
34+
rbac.allRoles(organizationId),
35+
// Plan-gated subset — the Teams page disables dropdown options not
36+
// in this set. Server-side enforcement is independent (setUserRole
37+
// rejects a plan-gated assignment regardless of UI state).
38+
rbac.getAssignableRoleIds(organizationId),
39+
// Per-member current role in a single round-trip.
40+
rbac.getUserRoles(
41+
result.members.map((m) => m.user.id),
42+
organizationId
43+
),
44+
// The viewer's own role, plus the system-role ladder it sits on —
45+
// together these say how high this viewer is allowed to assign.
46+
rbac.getUserRole({ userId, organizationId }),
47+
rbac.systemRoles(organizationId),
48+
]);
49+
50+
// Roles this viewer is allowed to hand out: at or below their own level
51+
// on the system-role ladder. Deliberately NOT intersected with
52+
// `assignableRoleIds` — the two answer different questions and the Team
53+
// page renders them differently. A role above the viewer's level is left
54+
// out of the picker altogether, while a role that is merely plan-locked
55+
// still needs to appear with an upgrade link. Merging them would offer a
56+
// viewer "Owner (upgrade)", inviting them to pay for something their own
57+
// role still would not let them assign.
58+
//
59+
// Off-ladder roles (custom org roles, and any role held by a viewer who
60+
// is themselves on a custom role) are refused, the same way the invite
61+
// flow refuses them.
62+
const offerableRoleIds = computeOfferableRoleIds(roles, systemRoles, viewerRole?.id ?? null);
3663

3764
const memberRoles = result.members.map((m) => ({
3865
userId: m.user.id,
@@ -60,6 +87,7 @@ export class TeamPresenter extends BasePresenter {
6087
planSeatLimit,
6188
roles,
6289
assignableRoleIds,
90+
offerableRoleIds,
6391
memberRoles,
6492
};
6593
}

apps/webapp/app/routes/_app.orgs.$organizationSlug.invite/route.tsx

Lines changed: 11 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -77,28 +77,18 @@ export const loader = dashboardLoader(
7777
throw new Response("Not Found", { status: 404 });
7878
}
7979

80-
// Inviter's own role drives the "below their level" filter on the
81-
// dropdown. Plus assignable role IDs already encode the org's plan
82-
// tier — the intersection is what we offer.
83-
const [inviterRole, assignableRoleIds, systemRoles] = await Promise.all([
84-
rbac.getUserRole({ userId, organizationId }),
85-
rbac.getAssignableRoleIds(organizationId),
86-
rbac.systemRoles(organizationId),
87-
]);
88-
8980
// Build the dropdown's offerable set server-side: roles that are
90-
// (a) assignable on the current plan AND (b) at or below the
91-
// inviter's own level. The client just renders these — it doesn't
92-
// need to know about the system-role catalogue or the ladder.
93-
const assignableSet = new Set(assignableRoleIds);
94-
const offerableRoleIds = systemRoles
95-
? result.roles
96-
.filter(
97-
(r) =>
98-
assignableSet.has(r.id) && isAtOrBelow(systemRoles, inviterRole?.id ?? null, r.id)
99-
)
100-
.map((r) => r.id)
101-
: [];
81+
// (a) at or below the inviter's own level AND (b) assignable on the
82+
// current plan. The client just renders these — it doesn't need to know
83+
// about the system-role catalogue or the ladder.
84+
//
85+
// The presenter already applies the ladder; intersecting with the plan is
86+
// right here, because the invite dropdown has no upgrade affordance and a
87+
// plan-locked role is simply not offered. The Team page keeps the two
88+
// sets apart instead, since it still shows plan-locked roles as an
89+
// upgrade link — so it uses the presenter's `offerableRoleIds` unmerged.
90+
const assignableSet = new Set(result.assignableRoleIds);
91+
const offerableRoleIds = result.offerableRoleIds.filter((id) => assignableSet.has(id));
10292

10393
// Buying seats is a billing operation: surface whether this user can, so
10494
// the purchase modal disables its trigger (the team action enforces it).

apps/webapp/app/routes/_app.orgs.$organizationSlug.settings.team/route.tsx

Lines changed: 30 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -328,6 +328,7 @@ export default function Page() {
328328
planSeatLimit,
329329
roles,
330330
assignableRoleIds,
331+
offerableRoleIds,
331332
memberRoles,
332333
canManageMembers,
333334
canManageBilling,
@@ -499,6 +500,7 @@ export default function Page() {
499500
currentRoleId={memberRoleByUserId.get(member.user.id) ?? null}
500501
roles={roles}
501502
assignableRoleIds={assignableRoleIds}
503+
offerableRoleIds={offerableRoleIds}
502504
canManageMembers={canManageMembers}
503505
/>
504506
<div className="justify-self-end">
@@ -688,29 +690,45 @@ function LeaveRemoveButton({
688690
}
689691

690692
// Inline role picker — submits a `_formType=set-role` form via fetcher
691-
// so the change persists without a full page reload. Disabled options
692-
// (and the picker itself) reflect plan gating + manage:members; the
693-
// server's setUserRole enforces both checks again as the source of
694-
// truth, so this is a UI-affordance layer only.
693+
// so the change persists without a full page reload. The picker itself,
694+
// the roles it lists and which of those are selectable all reflect
695+
// manage:members, the viewer's own role and plan gating; the server
696+
// validates the submitted role independently, so this is a
697+
// UI-affordance layer only.
698+
//
699+
// Two different sets narrow the list, and they must stay separate:
700+
// offerableRoleIds — roles the viewer's own role lets them assign.
701+
// Anything else is left out of the list entirely.
702+
// assignableRoleIds — roles the org's plan allows. A role that is
703+
// offerable but not plan-assignable still shows,
704+
// as "Name (upgrade)" linking to billing.
695705
function RolePicker({
696706
memberUserId,
697707
currentRoleId,
698708
roles,
699709
assignableRoleIds,
710+
offerableRoleIds,
700711
canManageMembers,
701712
}: {
702713
memberUserId: string;
703714
currentRoleId: string | null;
704715
roles: Role[];
705716
assignableRoleIds: string[];
717+
offerableRoleIds: string[];
706718
canManageMembers: boolean;
707719
}) {
708720
const organization = useOrganization();
709721
const fetcher = useFetcher<{ ok: boolean; error?: string } | { ok: true }>();
710722
const assignable = new Set(assignableRoleIds);
711-
// With no RBAC plugin installed, the loader returns no roles —
712-
// render nothing rather than an empty dropdown.
713-
if (roles.length === 0) return null;
723+
const offerable = new Set(offerableRoleIds);
724+
// The member's current role stays in the list even when the viewer could
725+
// not assign it, so the controlled `value` below still resolves to a row
726+
// and the dropdown shows the role the member actually holds.
727+
const visibleRoles = roles.filter((r) => offerable.has(r.id) || r.id === currentRoleId);
728+
// With no RBAC plugin installed the loader returns no roles, and a viewer
729+
// with nothing to offer would get an empty dropdown — render nothing
730+
// rather than a dead control.
731+
if (visibleRoles.length === 0) return null;
714732

715733
const isSubmitting = fetcher.state === "submitting";
716734
const error =
@@ -723,13 +741,16 @@ function RolePicker({
723741
// kept the old role; without `value` the UI would show the
724742
// attempted change).
725743
value={currentRoleId ?? ""}
726-
items={roles}
744+
items={visibleRoles}
727745
variant="tertiary/small"
728746
disabled={!canManageMembers || isSubmitting}
729747
dropdownIcon
730-
text={(v) => roles.find((r) => r.id === v)?.name ?? "No role"}
748+
text={(v) => visibleRoles.find((r) => r.id === v)?.name ?? "No role"}
731749
setValue={(next) => {
732750
if (typeof next !== "string" || next === (currentRoleId ?? "")) return;
751+
// The member's current role is listed even when it isn't offerable,
752+
// so re-check before submitting.
753+
if (!offerable.has(next)) return;
733754
// Upgrade-link rows have a value too (Ariakit needs one to
734755
// make the row interactive — without it the Link inside
735756
// doesn't even register the click), but they shouldn't

apps/webapp/app/utils/inviteRoleLadder.ts

Lines changed: 24 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
1-
// An inviter can only assign a role at or below their own. The systemRoles
2-
// array is in canonical order (highest authority first), so array index drives
3-
// the ladder. Custom roles aren't in the table and are refused. Dependency-free
4-
// so the rule can be unit-tested directly.
1+
// A user can only assign a role at or below their own — on invite, and on the
2+
// Team page's role picker. The systemRoles array is in canonical order
3+
// (highest authority first), so array index drives the ladder. Custom roles
4+
// aren't in the table and are refused. Dependency-free so the rule can be
5+
// unit-tested directly.
56

67
export type LadderRole = { id: string };
78

@@ -31,3 +32,22 @@ export function isAtOrBelow(
3132
if (inviter === undefined || invited === undefined) return false;
3233
return invited <= inviter;
3334
}
35+
36+
/**
37+
* The subset of `roles` that a user holding `viewerRoleId` may assign, by the
38+
* ladder above. Knows nothing about plan gating: a role the org's plan does
39+
* not allow is still returned, so a caller that wants to surface it as an
40+
* upgrade affordance can. Callers that have no upgrade affordance intersect
41+
* with their plan-assignable set themselves.
42+
*
43+
* `systemRoles` is null when no RBAC plugin is installed — there is no ladder
44+
* to check against, so nothing is offerable.
45+
*/
46+
export function offerableRoleIds(
47+
roles: ReadonlyArray<LadderRole>,
48+
systemRoles: ReadonlyArray<LadderRole> | null,
49+
viewerRoleId: string | null
50+
): string[] {
51+
if (!systemRoles) return [];
52+
return roles.filter((r) => isAtOrBelow(systemRoles, viewerRoleId, r.id)).map((r) => r.id);
53+
}

apps/webapp/test/inviteRoleLadder.test.ts

Lines changed: 32 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { describe, expect, it } from "vitest";
2-
import { isAtOrBelow } from "../app/utils/inviteRoleLadder.js";
2+
import { isAtOrBelow, offerableRoleIds } from "../app/utils/inviteRoleLadder.js";
33

44
// systemRoles in canonical order: highest authority first.
55
const roles = [{ id: "owner" }, { id: "admin" }, { id: "member" }];
@@ -31,3 +31,34 @@ describe("isAtOrBelow", () => {
3131
expect(isAtOrBelow(roles, "custom-role-id", "member")).toBe(false);
3232
});
3333
});
34+
35+
// Property under test: the picker/dropdown set is the ladder alone. Plan
36+
// gating is a separate concern the caller layers on, so a plan-locked role
37+
// must still come back here — the Team page renders it as an upgrade link.
38+
describe("offerableRoleIds", () => {
39+
const catalogue = [{ id: "owner" }, { id: "admin" }, { id: "member" }, { id: "custom-1" }];
40+
41+
it("offers the viewer's own level and below", () => {
42+
expect(offerableRoleIds(catalogue, roles, "admin")).toEqual(["admin", "member"]);
43+
expect(offerableRoleIds(catalogue, roles, "member")).toEqual(["member"]);
44+
});
45+
46+
it("leaves roles above the viewer out entirely", () => {
47+
expect(offerableRoleIds(catalogue, roles, "admin")).not.toContain("owner");
48+
});
49+
50+
it("does not filter on plan gating — a plan-locked role is still offerable", () => {
51+
// `owner` may be unavailable on the org's plan; that is the caller's
52+
// concern, and it still needs the id back to render the upgrade row.
53+
expect(offerableRoleIds(catalogue, roles, "owner")).toContain("owner");
54+
});
55+
56+
it("drops custom roles, which are not on the ladder", () => {
57+
expect(offerableRoleIds(catalogue, roles, "owner")).not.toContain("custom-1");
58+
});
59+
60+
it("offers nothing to a roleless viewer or with no ladder at all", () => {
61+
expect(offerableRoleIds(catalogue, roles, null)).toEqual([]);
62+
expect(offerableRoleIds(catalogue, null, "owner")).toEqual([]);
63+
});
64+
});

0 commit comments

Comments
 (0)