WEB-142: Adding roles #88
Conversation
…oles # Conflicts: # components/Breadcrumbs.tsx
WEB-142 No roles
There are no lists of roles or description of what it is or what users or permissions that are assigned Can't add, edit, delete, add permission to role, remove permission from role, add user to role, remove user from role or list all roles |
COMTOP1
changed the base branch from
main
to
web-140-there-is-no-current-admin-ui
|
This is just a draft for now |
app/(authenticated)/admin/roles/[roleID]/AddPermissionsToRoleViews.tsx
Outdated
Show resolved
Hide resolved
app/(authenticated)/admin/roles/[roleID]/AddUserToRoleViews.tsx
Outdated
Show resolved
Hide resolved
| export function DeleteRoleForm(props: { | ||
| action: (data: z.infer<typeof RoleSchema>) => Promise<FormResponse>; | ||
| onSuccess: () => void; | ||
| }) { | ||
| return ( | ||
| <Form | ||
| action={props.action} | ||
| onSuccess={props.onSuccess} | ||
| schema={z.any()} | ||
| submitLabel={"Delete"} | ||
| > | ||
| <h1 className={"mb-2 mt-0 text-4xl font-bold"}>Delete Role</h1> | ||
| <h2 className={"mb-2 mt-0 text-2xl font-bold"}> | ||
| Are you sure you want to delete this? | ||
| <br /> | ||
| Any users with this role may lose access to certain features. | ||
| </h2> | ||
| </Form> | ||
| ); | ||
| } |
There was a problem hiding this comment.
This seems like an ideal use case for a Mantine confirm modal (https://mantine.dev/x/modals/#confirm-modal)
| export function RemovePermissionFromRoleForm(props: { | ||
| action: (data: z.infer<typeof RolePermissionSchema>) => Promise<FormResponse>; | ||
| onSuccess: () => void; | ||
| permission: string; | ||
| role: string; | ||
| }) { | ||
| return ( | ||
| <Form | ||
| action={props.action} | ||
| onSuccess={props.onSuccess} | ||
| schema={z.any()} | ||
| submitLabel={"Remove"} | ||
| > | ||
| <h1 className={"mb-2 mt-0 text-4xl font-bold"}>Remove Permission</h1> | ||
| <h2 className={"mb-2 mt-0 text-2xl font-bold"}> | ||
| Are you sure you want to remove {props.permission} from {props.role}? | ||
| <br /> | ||
| Unintended consequences can occur. | ||
| </h2> | ||
| </Form> | ||
| ); | ||
| } |
There was a problem hiding this comment.
Also could be simplified by using modals.openConfirmModal and yeeting this component
Co-authored-by: Marks Polakovs <[email protected]>
| function sufficientPermissionsFor(perm: Permission): Record<string, boolean> { | ||
| const permMap: Record<string, boolean> = {}; | ||
| permMap[perm] = true; | ||
| // This switch is designed to have multiple fallthrough statements, this is intentional. | ||
| switch (perm) { | ||
| case "ManageMembers.Members.List": | ||
| case "ManageMembers.Members.Add": | ||
| permMap["ManageMembers.Members.Admin"] = true; | ||
| case "ManageMembers.Permissions": | ||
| case "ManageMembers.Groups": | ||
| case "ManageMembers.Members.Admin": | ||
| permMap["ManageMembers.Admin"] = true; | ||
| break; | ||
| case "Calendar.Social.Creator": | ||
| permMap["Calendar.Social.Admin"] = true; | ||
| case "Calendar.Social.Admin": | ||
| permMap["Calendar.Admin"] = true; | ||
| break; | ||
| case "Calendar.Show.Creator": | ||
| permMap["Calendar.Show.Admin"] = true; | ||
| case "Calendar.Show.Admin": | ||
| permMap["Calendar.Admin"] = true; | ||
| break; | ||
| case "Calendar.Meeting.Creator": | ||
| permMap["Calendar.Meeting.Admin"] = true; | ||
| case "Calendar.Meeting.Admin": | ||
| case "CalendarIntegration.Admin": | ||
| permMap["Calendar.Admin"] = true; | ||
| break; |
There was a problem hiding this comment.
This seems error-prone (eg if you forget a break it'll be subtly wrong), meaning you have to be extremely careful when modifying and reviewing it. What do you think about this - verbose, but clearer and less error-prone:
const sufficientPermissionsFor: Record<Permission, Permission[]> = {
"ManageMembers.Members.List": ["ManageMembers.Members.Admin", "ManageMembers.Admin"],
"ManageMembers.Members.Add": ["ManageMembers.Members.Admin", "ManageMembers.Admin"],
};Alternatively some form of explicit tree data structure.
There was a problem hiding this comment.
And honestly I'd argue that this is beyond the scope of this PR - the way I understood it this was (originally at least) about adding a UI to manage roles, not a refactor of the permissions system which this change seems like it'd be a part of. Though happy to be overruled.
|
Superseded by #137 |
This is a draft for now (paid feature on private repo), will add a comment saying ready for review