Show "Super Admin" rather than the raw enum value - #450
Conversation
The Roles: line rendered SuperAdmin verbatim, the last place a raw enum reached a user after Role.User started showing as "Member". Replaces the inline ternary with a ROLE_DISPLAY_NAMES lookup so the two mappings sit together and a third costs one line. Role.Admin and the relationship roles are absent and fall through unchanged.
Greptile SummaryThe PR maps
Confidence Score: 4/5The PR appears safe to merge, with only non-blocking type-strength, test-duplication, and comment-concision concerns. The current role values render correctly and the changed tests exercise the new label; the remaining concerns affect maintainability and compile-time protection rather than current behavior. Files Needing Attention: src/lib/utils/user-roles.ts and src/lib/utils/tests/user-roles.test.ts
|
| Filename | Overview |
|---|---|
| src/lib/utils/user-roles.ts | Correctly adds the SuperAdmin display label, but uses an unnecessarily permissive key type and overlong mapping documentation. |
| src/lib/utils/tests/user-roles.test.ts | Assertions match the new label, though the added test substantially duplicates an existing SuperAdmin case. |
| docs/test-plans/member_role_change_manual_testing.md | Accurately documents the Member and Super Admin recipient-facing labels. |
Reviews (1): Last reviewed commit: "fix(members): show "Super Admin" rather ..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| * "Member" matches the add-member dialog and the row's actions menu; rendering | ||
| * the raw "User" here taught a second name for the same role. | ||
| */ | ||
| const ROLE_DISPLAY_NAMES: Record<string, string> = { |
There was a problem hiding this comment.
Record<string, string> accepts invalid role keys without a type error, allowing a typo to silently fall through and render the raw enum value. Typing this partial mapping with DisplayRole preserves the intentional fallback while rejecting unsupported keys.
| const ROLE_DISPLAY_NAMES: Record<string, string> = { | |
| const ROLE_DISPLAY_NAMES: Partial<Record<DisplayRole, string>> = { |
Context Used: Use refactor-platform-fe's coding standards file f... (source)
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| it('renders Role.SuperAdmin as "Super Admin", not the raw enum value', () => { | ||
| const user = createUser([{ role: Role.SuperAdmin, organization_id: null }]); | ||
| const roles = getUserDisplayRoles(user, organizationId, []); | ||
|
|
||
| expect(roles).toEqual(['Super Admin']); | ||
| expect(roles).not.toContain('SuperAdmin'); | ||
| }); |
There was a problem hiding this comment.
This test repeats the existing SuperAdmin setup and exact ['Super Admin'] assertion, so future label changes require maintaining two equivalent cases. Consolidate the raw-value assertion into the existing test to retain the distinct check without duplicating the behavior.
Context Used: I want you to look at existing code patterns and e... (source)
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| /** | ||
| * Recipient-facing names for roles whose enum value is not what we want to show | ||
| * a user. `Role.Admin` and the relationship roles already read correctly, so | ||
| * they are absent and fall through unchanged. | ||
| * | ||
| * "Member" matches the add-member dialog and the row's actions menu; rendering | ||
| * the raw "User" here taught a second name for the same role. | ||
| */ |
There was a problem hiding this comment.
This comment restates the visible mappings and fall-through implementation across two paragraphs, increasing documentation maintenance whenever role handling changes. Keep only the non-obvious rationale, or remove the comment where the names are already self-explanatory.
| /** | |
| * Recipient-facing names for roles whose enum value is not what we want to show | |
| * a user. `Role.Admin` and the relationship roles already read correctly, so | |
| * they are absent and fall through unchanged. | |
| * | |
| * "Member" matches the add-member dialog and the row's actions menu; rendering | |
| * the raw "User" here taught a second name for the same role. | |
| */ |
Context Used: Use refactor-platform-fe's coding standards file f... (source)
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
1 participant
Description
The member row's
Roles:line renderedSuperAdminverbatim — the last place a raw enum valuestill reached a user after #449 started showing
Role.Useras "Member".Changes
SuperAdminnow displays as "Super Admin" on theRoles:line.getUserDisplayRoleswith aROLE_DISPLAY_NAMESlookup, so thetwo mappings live together and a third costs one line.
Role.Adminand the relationship roles(Coach/Coachee) are deliberately absent and fall through unchanged.
Screenshots / Videos Showing UI Changes
Helpful to attach: a members list viewed as a global super admin, showing the
Roles:line.Testing Strategy
npm run test:run(163 files / 1746 tests),npx tsc --noEmit,npm run lint, and the chromiumPlaywright suite all pass.
Two existing assertions were updated from the raw enum, and a dedicated case was added asserting
Role.SuperAdminrenders as "Super Admin" and never as "SuperAdmin". Verified the tests have teeth:removing the mapping fails three of them.
getUserDisplayRolesis the only place a raw role value is shown to a user — everywhere else usesthe
isAdminOrSuperAdmin/isSuperAdminpredicates — so this completes the sweep.Concerns
None. Display-only; no API, permission or behaviour change.