-
Notifications
You must be signed in to change notification settings - Fork 4
Show "Super Admin" rather than the raw enum value #450
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -11,6 +11,19 @@ import { type Option, Some, None } from "@/types/option"; | |||||||||||||||||
|
|
||||||||||||||||||
| export type DisplayRole = Role | RelationshipRole | ||||||||||||||||||
|
|
||||||||||||||||||
| /** | ||||||||||||||||||
| * 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. | ||||||||||||||||||
| */ | ||||||||||||||||||
|
Comment on lines
+14
to
+21
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
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.
Suggested change
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! |
||||||||||||||||||
| const ROLE_DISPLAY_NAMES: Record<string, string> = { | ||||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
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! |
||||||||||||||||||
| [Role.User]: "Member", | ||||||||||||||||||
| [Role.SuperAdmin]: "Super Admin", | ||||||||||||||||||
| }; | ||||||||||||||||||
|
|
||||||||||||||||||
| /** | ||||||||||||||||||
| * Gets display roles for a user combining organization roles and coaching relationship roles | ||||||||||||||||||
| * @param user - The user to get roles for | ||||||||||||||||||
|
|
@@ -47,11 +60,8 @@ export function getUserDisplayRoles( | |||||||||||||||||
| roles.add(RelationshipRole.Coachee); | ||||||||||||||||||
| } | ||||||||||||||||||
|
|
||||||||||||||||||
| // "Member" is the recipient-facing word for Role.User, matching the add-member | ||||||||||||||||||
| // dialog and the row's actions menu. Showing the raw "User" here taught a | ||||||||||||||||||
| // different name for the same role. | ||||||||||||||||||
| return Array.from(roles) | ||||||||||||||||||
| .map(role => (role === Role.User ? "Member" : (role as string))) | ||||||||||||||||||
| .map(role => ROLE_DISPLAY_NAMES[role] ?? (role as string)) | ||||||||||||||||||
| .sort(); | ||||||||||||||||||
| } | ||||||||||||||||||
|
|
||||||||||||||||||
|
|
||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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!