From 79eb10306474acea351459b48f0092c04dc99241 Mon Sep 17 00:00:00 2001 From: Alec M Date: Thu, 26 Dec 2024 16:34:37 -0500 Subject: [PATCH 01/21] init: Permission Based Access Control for User Management --- src/components/PermissionPanel/index.tsx | 310 +++++++++++++++++ src/content/users/ProfileView.tsx | 402 ++++++++++++----------- src/graphql/editUser.ts | 6 +- src/graphql/getMyUser.ts | 1 + src/graphql/getUser.ts | 2 + src/graphql/index.ts | 6 + src/graphql/retrievePBACDefaults.ts | 44 +++ src/hooks/useProfileFields.test.ts | 25 ++ src/hooks/useProfileFields.ts | 13 +- src/types/Auth.d.ts | 30 +- src/types/PBAC.d.ts | 88 +++++ 11 files changed, 701 insertions(+), 226 deletions(-) create mode 100644 src/components/PermissionPanel/index.tsx create mode 100644 src/graphql/retrievePBACDefaults.ts create mode 100644 src/types/PBAC.d.ts diff --git a/src/components/PermissionPanel/index.tsx b/src/components/PermissionPanel/index.tsx new file mode 100644 index 000000000..8d097b3b2 --- /dev/null +++ b/src/components/PermissionPanel/index.tsx @@ -0,0 +1,310 @@ +import { useQuery } from "@apollo/client"; +import { ArrowDropDown } from "@mui/icons-material"; +import { + Accordion, + AccordionDetails, + AccordionSummary, + Checkbox, + FormControlLabel, + FormGroup, + styled, + Typography, +} from "@mui/material"; +import { FC, memo, useEffect, useMemo } from "react"; +import { useFormContext } from "react-hook-form"; +import Grid2 from "@mui/material/Unstable_Grid2"; +import { + EditUserInput, + RetrievePBACDefaultsResp, + RetrievePBACDefaultsInput, + RETRIEVE_PBAC_DEFAULTS, +} from "../../graphql"; + +const StyledAccordion = styled(Accordion)({ + width: "957px", // TODO: Need to fix the page layout +}); + +const StyledAccordionSummary = styled(AccordionSummary)({ + borderBottom: "1px solid #6B7294", + minHeight: "unset !important", + "& .MuiAccordionSummary-content": { + margin: "9px 0", + }, + "& .MuiAccordionSummary-content.Mui-expanded": { + margin: "9px 0", + }, + "& .MuiAccordionSummary-expandIcon": { + color: "#356AAD", + }, +}); + +const StyledAccordionHeader = styled(Typography)<{ component: React.ElementType }>({ + fontWeight: 700, + fontSize: "16px", + lineHeight: "19px", + color: "#356AAD", +}); + +const StyledGroupTitle = styled(Typography)({ + fontWeight: 600, + fontSize: "13px", + lineHeight: "20px", + color: "#187A90", + textTransform: "uppercase", + marginBottom: "7.5px", + marginTop: "13.5px", + userSelect: "none", +}); + +const StyledFormControlLabel = styled(FormControlLabel)({ + whiteSpace: "nowrap", + userSelect: "none", + "& .MuiTypography-root": { + fontWeight: 400, + fontSize: "16px", + color: "#083A50 !important", + }, + "& .MuiCheckbox-root": { + paddingTop: "5.5px", + paddingBottom: "5.5px", + color: "#005EA2 !important", + }, + "& .MuiTypography-root.Mui-disabled, & .MuiCheckbox-root.Mui-disabled": { + opacity: 0.6, + cursor: "not-allowed !important", + }, +}); + +type PermissionPanelProps = { + /** + * The original/stored role of the user. + * + * This is used to determine if the role has changed and to update the default permissions. + */ + role: UserRole; +}; + +const mockPerms: PBACDefault[] = [ + { + _id: "submission_request:view", + group: "Submission Request", + name: "View", + checked: false, + disabled: false, + }, + { + _id: "submission_request:create", + group: "Submission Request", + name: "Create", + checked: false, + disabled: false, + }, + { + _id: "data_submission:view", + group: "Data Submission", + name: "View", + checked: true, + disabled: true, + }, + { + _id: "data_submission:create", + group: "Data Submission", + name: "Create", + checked: false, + disabled: false, + }, + { + _id: "data_submission:confirm", + group: "Data Submission", + name: "Confirm", + checked: false, + disabled: false, + }, + { + _id: "program:manage", + group: "Admin", + name: "Manage Programs", + checked: false, + disabled: false, + }, + { + _id: "study:manage", + group: "Admin", + name: "Manage Studies", + checked: false, + disabled: false, + }, + { + _id: "access:request", + group: "Miscellaneous", + name: "Request Access", + checked: false, + disabled: true, + }, +]; + +/** + * Provides a panel for managing permissions and notifications for a user role. + * + * @returns The PermissionPanel component. + */ +const PermissionPanel: FC = ({ role }) => { + const { setValue, watch } = useFormContext(); + + // eslint-disable-next-line @typescript-eslint/no-unused-vars + const { data: pbacData, loading } = useQuery( + RETRIEVE_PBAC_DEFAULTS, + { + variables: { roles: ["All"] }, + context: { clientName: "backend" }, + fetchPolicy: "cache-first", + } + ); + + const data = mockPerms; // TODO: remove this + + const selectedRole = watch("role"); + const permissionsValue = watch("permissions"); + const notificationsValue = watch("notifications"); + + const permissionColumns = useMemo< + Array[] }>> + >(() => { + const updatedPermissions: PBACDefault[] = data.map((perm) => ({ + ...perm, + checked: permissionsValue.includes(perm._id), + })); + + const groupedPermissions: Record[]> = + updatedPermissions.reduce((acc, perm) => { + if (!acc[perm.group]) { + acc[perm.group] = []; + } + acc[perm.group].push(perm); + return acc; + }, {}); + + const columns: Array[] }>> = [ + [], + [], + [], + ]; + + Object.entries(groupedPermissions).forEach(([name, permissions], index) => { + const placement = index > 1 ? 2 : index; + columns[placement].push({ name, permissions }); + }); + + return columns; + }, [data, permissionsValue]); + + const notificationColumns = useMemo< + Array[] }>> + >(() => [], []); + + const handlePermissionChange = (_id: AuthPermissions) => { + if (permissionsValue.includes(_id)) { + setValue( + "permissions", + permissionsValue.filter((p) => p !== _id) + ); + } else { + setValue("permissions", [...permissionsValue, _id]); + } + }; + + const handleNotificationChange = (_id: AuthNotifications) => { + if (notificationsValue.includes(_id)) { + setValue( + "notifications", + notificationsValue.filter((n) => n !== _id) + ); + } else { + setValue("notifications", [...notificationsValue, _id]); + } + }; + + const handleRoleChange = (selectedRole: UserRole) => { + if (selectedRole === role) { + return; + } + + // TODO: This is a mock implementation. Refactor it to use the actual data based on the role. + setValue( + "permissions", + data.filter((perm) => perm.checked).map((perm) => perm._id) + ); + setValue("notifications", []); // TODO: Need default notifications + }; + + useEffect(() => { + handleRoleChange(selectedRole); + }, [selectedRole]); + + return ( + <> + + }> + Permissions + + + + {permissionColumns.map((column, index) => ( + // eslint-disable-next-line react/no-array-index-key + + {column.map(({ name, permissions }) => ( +
+ {name} + + {permissions.map(({ _id, checked, disabled, name }) => ( + handlePermissionChange(_id)} + control={} + data-testid={`permission-${_id}`} + /> + ))} + +
+ ))} + + ))} + + + + + }> + Email Notifications + + + + {notificationColumns.map((column, index) => ( + // eslint-disable-next-line react/no-array-index-key + + {column.map(({ name, notifications }) => ( +
+ {name} + + {notifications.map(({ _id, checked, disabled, name }) => ( + handleNotificationChange(_id)} + control={} + data-testid={`notification-${_id}`} + /> + ))} + +
+ ))} + + ))} + + + + + ); +}; + +export default memo(PermissionPanel); diff --git a/src/content/users/ProfileView.tsx b/src/content/users/ProfileView.tsx index 4ce4bfca7..c9c3121f3 100644 --- a/src/content/users/ProfileView.tsx +++ b/src/content/users/ProfileView.tsx @@ -2,7 +2,13 @@ import { FC, useEffect, useMemo, useRef, useState } from "react"; import { useLazyQuery, useMutation, useQuery } from "@apollo/client"; import { LoadingButton } from "@mui/lab"; import { Box, Container, MenuItem, Stack, TextField, Typography, styled } from "@mui/material"; -import { Controller, ControllerRenderProps, SubmitHandler, useForm } from "react-hook-form"; +import { + Controller, + ControllerRenderProps, + FormProvider, + SubmitHandler, + useForm, +} from "react-hook-form"; import { useNavigate } from "react-router-dom"; import { useSnackbar } from "notistack"; import bannerSvg from "../../assets/banner/profile_banner.png"; @@ -33,13 +39,14 @@ import BaseOutlinedInput from "../../components/StyledFormComponents/StyledOutli import BaseAutocomplete from "../../components/StyledFormComponents/StyledAutocomplete"; import useProfileFields, { VisibleFieldState } from "../../hooks/useProfileFields"; import AccessRequest from "../../components/AccessRequest"; +import PermissionPanel from "../../components/PermissionPanel"; type Props = { _id: User["_id"]; viewType: "users" | "profile"; }; -type FormInput = UpdateMyUserInput["userInfo"] | EditUserInput; +export type FormInput = UpdateMyUserInput["userInfo"] | EditUserInput; const StyledContainer = styled(Container)({ marginBottom: "90px", @@ -158,7 +165,7 @@ const ProfileView: FC = ({ _id, viewType }: Props) => { const { enqueueSnackbar } = useSnackbar(); const { user: currentUser, setData, logout, status: authStatus } = useAuthContext(); const { lastSearchParams } = useSearchParamsContext(); - const { handleSubmit, register, reset, watch, setValue, control } = useForm(); + const methods = useForm(); const ALL_STUDIES_OPTION = "All"; const manageUsersPageUrl = `/users${lastSearchParams?.["/users"] ?? ""}`; @@ -169,6 +176,7 @@ const ProfileView: FC = ({ _id, viewType }: Props) => { const [saving, setSaving] = useState(false); const [studyOptions, setStudyOptions] = useState([]); + const { handleSubmit, register, reset, watch, setValue, control } = methods; const roleField = watch("role"); const prevRoleRef = useRef(roleField); const studiesField = watch("studies"); @@ -258,6 +266,8 @@ const ProfileView: FC = ({ _id, viewType }: Props) => { userStatus: data.userStatus, studies: fieldset.studies !== "HIDDEN" ? data.studies : null, dataCommons: fieldset.dataCommons !== "HIDDEN" ? data.dataCommons : null, + permissions: data.permissions, + notifications: data.notifications, }, }).catch((e) => ({ errors: e?.message, data: null })); setSaving(false); @@ -401,197 +411,201 @@ const ProfileView: FC = ({ _id, viewType }: Props) => { {user.email} - -
- - Account Type - {formatIDP(user.IDP)} - - - Email - {user.email} - - - First name - {VisibleFieldState.includes(fieldset.firstName) ? ( - v?.trim(), - })} - inputProps={{ "aria-labelledby": "firstNameLabel", maxLength: 30 }} - size="small" - required - /> - ) : ( - user.firstName - )} - - - Last name - {VisibleFieldState.includes(fieldset.lastName) ? ( - v?.trim(), - })} - inputProps={{ "aria-labelledby": "lastNameLabel", maxLength: 30 }} - size="small" - required - /> - ) : ( - user.lastName - )} - - - Role - {VisibleFieldState.includes(fieldset.role) ? ( - ( - handleRoleChange(field, e?.target?.value as UserRole)} - MenuProps={{ disablePortal: true }} - inputProps={{ "aria-labelledby": "userRoleLabel" }} - > - {Roles.map((role) => ( - - {role} - - ))} - - )} - /> - ) : ( - <> - {user?.role} - {canRequestRole && } - - )} - - - Studies - {VisibleFieldState.includes(fieldset.studies) ? ( - ( - ( - 0 ? undefined : "Select studies"} - inputProps={{ "aria-labelledby": "userStudies", ...inputProps }} - onBlur={sortStudyOptions} - /> - )} - renderTags={(value: string[], _, state) => { - if (value?.length === 0 || state.focused) { - return null; - } - - return ( - - {formatStudySelectionValue(value, formattedStudyMap)} - - ); - }} - options={studyOptions} - getOptionLabel={(option: string) => formattedStudyMap[option]} - onChange={(_, data: string[]) => handleStudyChange(field, data)} - disabled={fieldset.studies === "DISABLED"} - loading={approvedStudiesLoading} - disableCloseOnSelect - multiple - /> - )} - /> - ) : null} - - - Account Status - {VisibleFieldState.includes(fieldset.userStatus) ? ( - ( - - Active - Inactive - - )} - /> - ) : ( - user.userStatus - )} - - - Data Commons - {VisibleFieldState.includes(fieldset.dataCommons) ? ( - ( - - {DataCommons.map((dc) => ( - - {dc.name} - - ))} - - )} - /> - ) : ( - user.dataCommons?.join(", ") - )} - - - - {Object.values(fieldset).some((fieldState) => fieldState === "UNLOCKED") && ( - - Save - - )} - {viewType === "users" && ( - navigate(manageUsersPageUrl)} - txt="#666666" - border="#828282" - > - Cancel - - )} - -
+ +
+ + Account Type + {formatIDP(user.IDP)} + + + Email + {user.email} + + + First name + {VisibleFieldState.includes(fieldset.firstName) ? ( + v?.trim(), + })} + inputProps={{ "aria-labelledby": "firstNameLabel", maxLength: 30 }} + size="small" + required + /> + ) : ( + user.firstName + )} + + + Last name + {VisibleFieldState.includes(fieldset.lastName) ? ( + v?.trim(), + })} + inputProps={{ "aria-labelledby": "lastNameLabel", maxLength: 30 }} + size="small" + required + /> + ) : ( + user.lastName + )} + + + Role + {VisibleFieldState.includes(fieldset.role) ? ( + ( + handleRoleChange(field, e?.target?.value as UserRole)} + MenuProps={{ disablePortal: true }} + inputProps={{ "aria-labelledby": "userRoleLabel" }} + > + {Roles.map((role) => ( + + {role} + + ))} + + )} + /> + ) : ( + <> + {user?.role} + {canRequestRole && } + + )} + + + Studies + {VisibleFieldState.includes(fieldset.studies) ? ( + ( + ( + 0 ? undefined : "Select studies"} + inputProps={{ "aria-labelledby": "userStudies", ...inputProps }} + onBlur={sortStudyOptions} + /> + )} + renderTags={(value: string[], _, state) => { + if (value?.length === 0 || state.focused) { + return null; + } + + return ( + + {formatStudySelectionValue(value, formattedStudyMap)} + + ); + }} + options={studyOptions} + getOptionLabel={(option: string) => formattedStudyMap[option]} + onChange={(_, data: string[]) => handleStudyChange(field, data)} + disabled={fieldset.studies === "DISABLED"} + loading={approvedStudiesLoading} + disableCloseOnSelect + multiple + /> + )} + /> + ) : null} + + + Account Status + {VisibleFieldState.includes(fieldset.userStatus) ? ( + ( + + Active + Inactive + + )} + /> + ) : ( + user.userStatus + )} + + + Data Commons + {VisibleFieldState.includes(fieldset.dataCommons) ? ( + ( + + {DataCommons.map((dc) => ( + + {dc.name} + + ))} + + )} + /> + ) : ( + user.dataCommons?.join(", ") + )} + + {VisibleFieldState.includes(fieldset.permissions) && + VisibleFieldState.includes(fieldset.notifications) && ( + + )} + + {Object.values(fieldset).some((fieldState) => fieldState === "UNLOCKED") && ( + + Save + + )} + {viewType === "users" && ( + navigate(manageUsersPageUrl)} + txt="#666666" + border="#828282" + > + Cancel + + )} + + +
diff --git a/src/graphql/editUser.ts b/src/graphql/editUser.ts index bb5d1ae8a..f6006c19c 100644 --- a/src/graphql/editUser.ts +++ b/src/graphql/editUser.ts @@ -25,6 +25,8 @@ export const mutation = gql` dbGaPID controlledAccess } + permissions + notifications } } `; @@ -38,10 +40,10 @@ export type Input = { * An array of studyIDs to assign to the user */ studies: string[]; -} & Pick; +} & Pick; export type Response = { - editUser: Pick & { + editUser: Pick & { studies: Pick< ApprovedStudy, "_id" | "studyName" | "studyAbbreviation" | "dbGaPID" | "controlledAccess" diff --git a/src/graphql/getMyUser.ts b/src/graphql/getMyUser.ts index 1f4283ea2..9a737ca20 100644 --- a/src/graphql/getMyUser.ts +++ b/src/graphql/getMyUser.ts @@ -19,6 +19,7 @@ export const query = gql` controlledAccess } permissions + notifications createdAt updateAt } diff --git a/src/graphql/getUser.ts b/src/graphql/getUser.ts index 12347d5da..8146ac8d3 100644 --- a/src/graphql/getUser.ts +++ b/src/graphql/getUser.ts @@ -18,6 +18,8 @@ export const query = gql` studyName studyAbbreviation } + permissions + notifications } } `; diff --git a/src/graphql/index.ts b/src/graphql/index.ts index ae3f10207..13f1cbf49 100644 --- a/src/graphql/index.ts +++ b/src/graphql/index.ts @@ -149,6 +149,12 @@ export type { Input as EditUserInput, Response as EditUserResp } from "./editUse export { mutation as REQUEST_ACCESS } from "./requestAccess"; export type { Input as RequestAccessInput, Response as RequestAccessResp } from "./requestAccess"; +export { query as RETRIEVE_PBAC_DEFAULTS } from "./retrievePBACDefaults"; +export type { + Input as RetrievePBACDefaultsInput, + Response as RetrievePBACDefaultsResp, +} from "./retrievePBACDefaults"; + // Organizations export { query as LIST_ORGS } from "./listOrganizations"; export type { Response as ListOrgsResp } from "./listOrganizations"; diff --git a/src/graphql/retrievePBACDefaults.ts b/src/graphql/retrievePBACDefaults.ts new file mode 100644 index 000000000..f50b821ac --- /dev/null +++ b/src/graphql/retrievePBACDefaults.ts @@ -0,0 +1,44 @@ +import gql from "graphql-tag"; + +export const query = gql` + query retrievePBACDefaults($roles: [String!]!) { + retrievePBACDefaults(roles: $roles) { + role + permissions { + _id + group + name + checked + disabled + } + notifications { + _id + group + name + checked + disabled + } + } + } +`; + +export type Input = { + roles: Array; +}; + +export type Response = { + retrievePBACDefaults: Array<{ + /** + * The role that the defaults apply to. + */ + role: UserRole; + /** + * The default permissions for the role. + */ + permissions: PBACDefault[]; + /** + * The default notifications for the role. + */ + notifications: PBACDefault[]; + }>; +}; diff --git a/src/hooks/useProfileFields.test.ts b/src/hooks/useProfileFields.test.ts index eb3874a1b..af37195c4 100644 --- a/src/hooks/useProfileFields.test.ts +++ b/src/hooks/useProfileFields.test.ts @@ -17,6 +17,8 @@ describe("Users View", () => { expect(result.current.role).toBe("UNLOCKED"); expect(result.current.userStatus).toBe("UNLOCKED"); + expect(result.current.permissions).toBe("UNLOCKED"); + expect(result.current.notifications).toBe("UNLOCKED"); }); it("should return READ_ONLY for all standard fields when a Submitter views the page", () => { @@ -130,6 +132,27 @@ describe("Profile View", () => { expect(result.current.studies).toBe("HIDDEN"); }); + it.each([ + "User", + "Submitter", + "Federal Lead", + "Data Commons Personnel", + "fake role" as UserRole, + ])( + "should return HIDDEN for the permissions and notifications panel on the profile page for role %s", + (role) => { + const user = { _id: "User-A", role } as User; + const profileOf: Pick = { _id: "User-A", role }; + + jest.spyOn(Auth, "useAuthContext").mockReturnValue({ user } as Auth.ContextState); + + const { result } = renderHook(() => useProfileFields(profileOf, "profile")); + + expect(result.current.permissions).toBe("HIDDEN"); + expect(result.current.notifications).toBe("HIDDEN"); + } + ); + it.each<[state: FieldState, role: UserRole]>([ ["HIDDEN", "User"], ["HIDDEN", "Submitter"], @@ -172,6 +195,8 @@ describe("Profile View", () => { expect(result.current.userStatus).toBe("READ_ONLY"); expect(result.current.dataCommons).toBe("HIDDEN"); expect(result.current.studies).toBe("HIDDEN"); + expect(result.current.permissions).toBe("HIDDEN"); + expect(result.current.notifications).toBe("HIDDEN"); } ); diff --git a/src/hooks/useProfileFields.ts b/src/hooks/useProfileFields.ts index a48029fdb..84d98d32d 100644 --- a/src/hooks/useProfileFields.ts +++ b/src/hooks/useProfileFields.ts @@ -5,7 +5,14 @@ import { RequiresStudiesAssigned } from "../config/AuthRoles"; */ type EditableFields = Extends< keyof User, - "firstName" | "lastName" | "role" | "userStatus" | "studies" | "dataCommons" + | "firstName" + | "lastName" + | "role" + | "userStatus" + | "studies" + | "dataCommons" + | "permissions" + | "notifications" >; /** @@ -47,6 +54,8 @@ const useProfileFields = ( userStatus: "READ_ONLY", dataCommons: "HIDDEN", studies: "HIDDEN", + permissions: "HIDDEN", + notifications: "HIDDEN", }; const isSelf: boolean = user?._id === profileOf?._id; @@ -60,6 +69,8 @@ const useProfileFields = ( if (user?.role === "Admin" && viewType === "users") { fields.role = "UNLOCKED"; fields.userStatus = "UNLOCKED"; + fields.permissions = "UNLOCKED"; + fields.notifications = "UNLOCKED"; // Editable for Admin viewing certain roles, otherwise hidden (even for a user viewing their own profile) fields.studies = RequiresStudiesAssigned.includes(profileOf?.role) ? "UNLOCKED" : "HIDDEN"; diff --git a/src/types/Auth.d.ts b/src/types/Auth.d.ts index 28d8aca62..b369b930c 100644 --- a/src/types/Auth.d.ts +++ b/src/types/Auth.d.ts @@ -54,7 +54,7 @@ type User = { /** * The list of notifications the user will receive */ - notifications: string[]; + notifications: AuthNotifications[]; /** * The last update date of the user object * @@ -71,34 +71,6 @@ type User = { type UserRole = "User" | "Admin" | "Data Commons Personnel" | "Federal Lead" | "Submitter"; -type SubmissionRequestPermissions = - | "submission_request:view" - | "submission_request:create" - | "submission_request:review" - | "submission_request:submit"; - -type DataSubmissionPermissions = - | "data_submission:view" - | "data_submission:create" - | "data_submission:review" - | "data_submission:admin_submit" - | "data_submission:confirm"; - -type DashboardPermissions = "dashboard:view"; -type AccessPermissions = "access:request"; -type UserPermissions = "user:manage"; -type ProgramPermissions = "program:manage"; -type StudyPermissions = "study:manage"; - -type AuthPermissions = - | SubmissionRequestPermissions - | DataSubmissionPermissions - | DashboardPermissions - | AccessPermissions - | UserPermissions - | ProgramPermissions - | StudyPermissions; - type OrgInfo = { orgID: string; orgName: string; diff --git a/src/types/PBAC.d.ts b/src/types/PBAC.d.ts new file mode 100644 index 000000000..850e384a1 --- /dev/null +++ b/src/types/PBAC.d.ts @@ -0,0 +1,88 @@ +type SubmissionRequestPermissions = + | "submission_request:view" + | "submission_request:create" + | "submission_request:review" + | "submission_request:submit"; + +type DataSubmissionPermissions = + | "data_submission:view" + | "data_submission:create" + | "data_submission:review" + | "data_submission:admin_submit" + | "data_submission:confirm"; + +type DashboardPermissions = "dashboard:view"; +type AccessPermissions = "access:request"; +type UserPermissions = "user:manage"; +type ProgramPermissions = "program:manage"; +type StudyPermissions = "study:manage"; + +type AuthPermissions = + | SubmissionRequestPermissions + | DataSubmissionPermissions + | DashboardPermissions + | AccessPermissions + | UserPermissions + | ProgramPermissions + | StudyPermissions; + +type SubmissionRequestNotifications = + | "submission_request:submitted" + | "submission_request:to_be_reviewed" + | "submission_request:reviewed" + | "submission_request:deleted" + | "submission_request:expiring"; + +type DataSubmissionNotifications = + | "data_submission:submitted" + | "data_submission:cancelled" + | "data_submission:withdrawn" + | "data_submission:released" + | "data_submission:completed" + | "data_submission:deleted" + | "data_submission:expiring"; + +type MiscNotifications = + | "access:requested" + | "account:inactivated" + | "account:users_inactivated" + | "account:disabled"; + +type AuthNotifications = + | SubmissionRequestNotifications + | DataSubmissionNotifications + | MiscNotifications; + +/** + * Defines the default structure of a PBAC object. + * + * e.g. Permission or Notification + */ +type PBACDefault = { + /** + * The unique identifier of the PBAC object. + * + * @example "manage:users" + */ + _id: T; + /** + * The group the PBAC object belongs to. + * + * @example "User Management" + */ + group: string; + /** + * The name of the PBAC object. + * + * @example "Manage Users" + */ + name: string; + /** + * Whether the PBAC object is checked for the role. + */ + checked: boolean; + /** + * Whether the PBAC object is disabled for the role. + */ + disabled: boolean; +}; From 010fcf6f62de7f78eaa9a936805dc10511397da0 Mon Sep 17 00:00:00 2001 From: Alec M Date: Thu, 26 Dec 2024 16:43:47 -0500 Subject: [PATCH 02/21] Remove mock data --- src/components/PermissionPanel/index.tsx | 128 +++++++++-------------- 1 file changed, 51 insertions(+), 77 deletions(-) diff --git a/src/components/PermissionPanel/index.tsx b/src/components/PermissionPanel/index.tsx index 8d097b3b2..d1541701c 100644 --- a/src/components/PermissionPanel/index.tsx +++ b/src/components/PermissionPanel/index.tsx @@ -19,6 +19,7 @@ import { RetrievePBACDefaultsInput, RETRIEVE_PBAC_DEFAULTS, } from "../../graphql"; +import { Logger } from "../../utils"; const StyledAccordion = styled(Accordion)({ width: "957px", // TODO: Need to fix the page layout @@ -84,65 +85,6 @@ type PermissionPanelProps = { role: UserRole; }; -const mockPerms: PBACDefault[] = [ - { - _id: "submission_request:view", - group: "Submission Request", - name: "View", - checked: false, - disabled: false, - }, - { - _id: "submission_request:create", - group: "Submission Request", - name: "Create", - checked: false, - disabled: false, - }, - { - _id: "data_submission:view", - group: "Data Submission", - name: "View", - checked: true, - disabled: true, - }, - { - _id: "data_submission:create", - group: "Data Submission", - name: "Create", - checked: false, - disabled: false, - }, - { - _id: "data_submission:confirm", - group: "Data Submission", - name: "Confirm", - checked: false, - disabled: false, - }, - { - _id: "program:manage", - group: "Admin", - name: "Manage Programs", - checked: false, - disabled: false, - }, - { - _id: "study:manage", - group: "Admin", - name: "Manage Studies", - checked: false, - disabled: false, - }, - { - _id: "access:request", - group: "Miscellaneous", - name: "Request Access", - checked: false, - disabled: true, - }, -]; - /** * Provides a panel for managing permissions and notifications for a user role. * @@ -151,8 +93,7 @@ const mockPerms: PBACDefault[] = [ const PermissionPanel: FC = ({ role }) => { const { setValue, watch } = useFormContext(); - // eslint-disable-next-line @typescript-eslint/no-unused-vars - const { data: pbacData, loading } = useQuery( + const { data } = useQuery( RETRIEVE_PBAC_DEFAULTS, { variables: { roles: ["All"] }, @@ -161,8 +102,6 @@ const PermissionPanel: FC = ({ role }) => { } ); - const data = mockPerms; // TODO: remove this - const selectedRole = watch("role"); const permissionsValue = watch("permissions"); const notificationsValue = watch("notifications"); @@ -170,17 +109,23 @@ const PermissionPanel: FC = ({ role }) => { const permissionColumns = useMemo< Array[] }>> >(() => { - const updatedPermissions: PBACDefault[] = data.map((perm) => ({ - ...perm, - checked: permissionsValue.includes(perm._id), + const defaults = data?.retrievePBACDefaults?.find((pbac) => pbac.role === selectedRole); + if (!defaults || !defaults?.permissions) { + Logger.error("Role not found in PBAC defaults", { role: selectedRole }); + return []; + } + + const updatedPermissions: PBACDefault[] = defaults?.permissions.map((p) => ({ + ...p, + checked: permissionsValue.includes(p._id), })); const groupedPermissions: Record[]> = - updatedPermissions.reduce((acc, perm) => { - if (!acc[perm.group]) { - acc[perm.group] = []; + updatedPermissions.reduce((acc, p) => { + if (!acc[p.group]) { + acc[p.group] = []; } - acc[perm.group].push(perm); + acc[p.group].push(p); return acc; }, {}); @@ -200,7 +145,39 @@ const PermissionPanel: FC = ({ role }) => { const notificationColumns = useMemo< Array[] }>> - >(() => [], []); + >(() => { + const defaults = data?.retrievePBACDefaults?.find((pbac) => pbac.role === selectedRole); + if (!defaults || !defaults?.notifications) { + Logger.error("Role not found in PBAC defaults", { role: selectedRole }); + return []; + } + + const updatedNotifications: PBACDefault[] = defaults?.notifications.map( + (n) => ({ + ...n, + checked: notificationsValue.includes(n._id), + }) + ); + + const groupedNotifications: Record[]> = + updatedNotifications.reduce((acc, n) => { + if (!acc[n.group]) { + acc[n.group] = []; + } + acc[n.group].push(n); + return acc; + }, {}); + + const columns: Array[] }>> = + [[], [], []]; + + Object.entries(groupedNotifications).forEach(([name, notifications], index) => { + const placement = index > 1 ? 2 : index; + columns[placement].push({ name, notifications }); + }); + + return columns; + }, []); const handlePermissionChange = (_id: AuthPermissions) => { if (permissionsValue.includes(_id)) { @@ -229,12 +206,9 @@ const PermissionPanel: FC = ({ role }) => { return; } - // TODO: This is a mock implementation. Refactor it to use the actual data based on the role. - setValue( - "permissions", - data.filter((perm) => perm.checked).map((perm) => perm._id) - ); - setValue("notifications", []); // TODO: Need default notifications + const defaults = data?.retrievePBACDefaults?.find((pbac) => pbac.role === selectedRole); + setValue("permissions", defaults?.permissions?.filter((p) => p.checked).map((p) => p._id)); + setValue("notifications", defaults?.notifications?.filter((n) => n.checked).map((n) => n._id)); }; useEffect(() => { From cb58bb2c8b9d86201f4770077087e552ca712d21 Mon Sep 17 00:00:00 2001 From: Alec M Date: Thu, 26 Dec 2024 16:51:23 -0500 Subject: [PATCH 03/21] init: Unit tests for PermissionsPanel --- src/components/PermissionPanel/index.test.tsx | 303 ++++++++++++++++++ 1 file changed, 303 insertions(+) create mode 100644 src/components/PermissionPanel/index.test.tsx diff --git a/src/components/PermissionPanel/index.test.tsx b/src/components/PermissionPanel/index.test.tsx new file mode 100644 index 000000000..14c4ae62e --- /dev/null +++ b/src/components/PermissionPanel/index.test.tsx @@ -0,0 +1,303 @@ +import { MockedProvider, MockedResponse } from "@apollo/client/testing"; +import { act, render, within } from "@testing-library/react"; +import { FormProvider, FormProviderProps } from "react-hook-form"; +import { axe } from "jest-axe"; +import { FC } from "react"; +import PermissionPanel from "./index"; +import { + RETRIEVE_PBAC_DEFAULTS, + RetrievePBACDefaultsInput, + RetrievePBACDefaultsResp, +} from "../../graphql"; + +type MockParentProps = { + children: React.ReactNode; + methods?: FormProviderProps; + mocks?: MockedResponse[]; +}; + +const MockParent: FC = ({ children, methods, mocks = [] }) => ( + + []) as FormProviderProps["watch"]} + setValue={jest.fn()} + {...methods} + > + {children} + + +); + +describe("Accessibility", () => { + it("should not have any accessibility violations (empty)", async () => { + const mock: MockedResponse = { + request: { + query: RETRIEVE_PBAC_DEFAULTS, + variables: { roles: ["All"] }, + }, + result: { + data: { + retrievePBACDefaults: [ + { + role: "Submitter", + permissions: [], + notifications: [], + }, + ], + }, + }, + }; + + const { container } = render(, { + wrapper: ({ children }) => {children}, + }); + + let result; + await act(async () => { + result = await axe(container); + }); + expect(result).toHaveNoViolations(); + }); + + it("should not have any accessibility violations (populated)", async () => { + const mock: MockedResponse = { + request: { + query: RETRIEVE_PBAC_DEFAULTS, + variables: { roles: ["All"] }, + }, + result: { + data: { + retrievePBACDefaults: [ + { + role: "Submitter", + permissions: [ + { + _id: "submission_request:create", + group: "Submission Request", + name: "Create", + checked: false, + disabled: false, + }, + { + _id: "data_submission:view", + group: "Data Submission", + name: "View", + checked: true, + disabled: true, + }, + ], + notifications: [ + { + _id: "data_submission:cancelled", + group: "Data Submissions", + name: "Cancelled", + checked: false, + disabled: false, + }, + { + _id: "account:disabled", + group: "Account", + name: "Disabled", + checked: false, + disabled: false, + }, + ], + }, + ], + }, + }, + }; + + const { container } = render(, { + wrapper: ({ children }) => {children}, + }); + + let result; + await act(async () => { + result = await axe(container); + }); + expect(result).toHaveNoViolations(); + }); +}); + +describe("Basic Functionality", () => { + it("should not crash when rendered", async () => { + const mock: MockedResponse = { + request: { + query: RETRIEVE_PBAC_DEFAULTS, + variables: { roles: ["All"] }, + }, + result: { + data: { + retrievePBACDefaults: [ + { + role: "Submitter", + permissions: [], + notifications: [], + }, + ], + }, + }, + }; + + render(, { + wrapper: ({ children }) => {children}, + }); + }); + + it("should cache the default PBAC data by default", async () => { + const mockMatcher = jest.fn().mockImplementation(() => true); + const mock: MockedResponse = { + request: { + query: RETRIEVE_PBAC_DEFAULTS, + }, + variableMatcher: mockMatcher, + result: { + data: { + retrievePBACDefaults: [ + { + role: "Submitter", + permissions: [], + notifications: [], + }, + ], + }, + }, + maxUsageCount: 999, + }; + + const { rerender } = render(, { + wrapper: ({ children }) => {children}, + }); + + expect(mockMatcher).toHaveBeenCalledTimes(1); + + rerender(); + + expect(mockMatcher).toHaveBeenCalledTimes(1); + }); + + it("should group the permissions by their pre-defined groups", async () => { + const mock: MockedResponse = { + request: { + query: RETRIEVE_PBAC_DEFAULTS, + variables: { roles: ["All"] }, + }, + result: { + data: { + retrievePBACDefaults: [ + { + role: "Submitter", + permissions: [ + { + _id: "submission_request:create", + group: "Submission Request", + name: "Create", + checked: false, + disabled: false, + }, + { + _id: "data_submission:view", + group: "Data Submission", + name: "View", + checked: true, + disabled: true, + }, + { + _id: "program:manage", + group: "Admin", + name: "Manage Programs", + checked: false, + disabled: false, + }, + ], + notifications: [ + { + _id: "data_submission:cancelled", + group: "Data Submissions", + name: "Cancelled", + checked: false, + disabled: false, + }, + { + _id: "account:disabled", + group: "Account", + name: "Disabled", + checked: false, + disabled: false, + }, + ], + }, + ], + }, + }, + }; + + const mockWatcher = jest.fn().mockImplementation((field) => { + if (field === "role") return "Admin"; + return []; + }); + + const { getByTestId } = render(, { + wrapper: ({ children }) => ( + + {children} + + ), + }); + + const srGroup = getByTestId("permissions-group-Submission Request"); + expect(srGroup).toBeInTheDocument(); + expect(within(srGroup).getByTestId("permission-submission_request:create")).toBeInTheDocument(); + const dsGroup = getByTestId("permissions-group-Data Submission"); + + expect(dsGroup).toBeInTheDocument(); + expect(within(dsGroup).getByTestId("permission-data_submission:view")).toBeInTheDocument(); + + const adminGroup = getByTestId("permissions-group-Admin"); + expect(adminGroup).toBeInTheDocument(); + expect(within(adminGroup).getByTestId("permission-program:manage")).toBeInTheDocument(); + + const dsNotifGroup = getByTestId("notifications-group-Data Submissions"); + expect(dsNotifGroup).toBeInTheDocument(); + expect( + within(dsNotifGroup).getByTestId("notification-data_submission:cancelled") + ).toBeInTheDocument(); + + const accountGroup = getByTestId("notifications-group-Account"); + expect(accountGroup).toBeInTheDocument(); + expect(within(accountGroup).getByTestId("notification-account:disabled")).toBeInTheDocument(); + }); + + it.todo("should utilize a maximum of 3 columns for the permissions"); + + it.todo("should utilize a maximum of 3 columns for the notifications"); + + it.todo( + "should utilize the current permissions to determine the checked state of each permission" + ); + + it.todo("should log an error when unable to retrieve the default PBAC details (GraphQL)"); + + it.todo("should log an error when unable to retrieve the default PBAC details (Network)"); + + it.todo("should log an error when unable to retrieve the default PBAC details (API)"); + + it.todo("should log an error when a permission is assigned but not provided in the defaults"); + + it.todo("should log an error when a notification is assigned but not provided in the defaults"); + + it.todo("should render a loading state when fetching the default PBAC details"); +}); + +describe("Implementation Requirements", () => { + it.todo("should reset the permissions to their default values when the role changes"); + + it.todo("should allow disabled permissions to be checked by default"); + + it.todo("should be rendered as collapsed by default"); + + it.todo("should propagate the permissions selections to the parent form"); + + it.todo("should propagate the notification selections to the parent form"); +}); From ac673796573d24842d57f2470e86f42844b65c1c Mon Sep 17 00:00:00 2001 From: Alec M Date: Fri, 27 Dec 2024 11:22:04 -0500 Subject: [PATCH 04/21] fix: Missing useMemo dependencies --- src/components/PermissionPanel/index.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/PermissionPanel/index.tsx b/src/components/PermissionPanel/index.tsx index d1541701c..60d466968 100644 --- a/src/components/PermissionPanel/index.tsx +++ b/src/components/PermissionPanel/index.tsx @@ -177,7 +177,7 @@ const PermissionPanel: FC = ({ role }) => { }); return columns; - }, []); + }, [data, notificationsValue]); const handlePermissionChange = (_id: AuthPermissions) => { if (permissionsValue.includes(_id)) { From 386e90b973a60bbf640f6512c1437170e6b0d2c8 Mon Sep 17 00:00:00 2001 From: Alec M Date: Fri, 27 Dec 2024 11:34:27 -0500 Subject: [PATCH 05/21] feat: Implement a No Options fallback placeholder --- src/components/PermissionPanel/index.test.tsx | 28 +++++++++++++++++++ src/components/PermissionPanel/index.tsx | 22 +++++++++++++-- 2 files changed, 48 insertions(+), 2 deletions(-) diff --git a/src/components/PermissionPanel/index.test.tsx b/src/components/PermissionPanel/index.test.tsx index 14c4ae62e..46f4bcfdf 100644 --- a/src/components/PermissionPanel/index.test.tsx +++ b/src/components/PermissionPanel/index.test.tsx @@ -300,4 +300,32 @@ describe("Implementation Requirements", () => { it.todo("should propagate the permissions selections to the parent form"); it.todo("should propagate the notification selections to the parent form"); + + it("should render a notice when there are no default PBAC details for a role", async () => { + const mock: MockedResponse = { + request: { + query: RETRIEVE_PBAC_DEFAULTS, + variables: { roles: ["All"] }, + }, + result: { + data: { + retrievePBACDefaults: [], + }, + }, + }; + + const { getByTestId } = render(, { + wrapper: ({ children }) => {children}, + }); + + expect(getByTestId("no-permissions-notice")).toBeInTheDocument(); + expect(getByTestId("no-permissions-notice")).toHaveTextContent( + /No permission options found for this role./i + ); + + expect(getByTestId("no-notifications-notice")).toBeInTheDocument(); + expect(getByTestId("no-notifications-notice")).toHaveTextContent( + /No notification options found for this role./i + ); + }); }); diff --git a/src/components/PermissionPanel/index.tsx b/src/components/PermissionPanel/index.tsx index 60d466968..c9cbf7f37 100644 --- a/src/components/PermissionPanel/index.tsx +++ b/src/components/PermissionPanel/index.tsx @@ -76,6 +76,14 @@ const StyledFormControlLabel = styled(FormControlLabel)({ }, }); +const StyledNotice = styled(Typography)({ + marginTop: "29.5px", + textAlign: "center", + width: "100%", + color: "#6B7294", + userSelect: "none", +}); + type PermissionPanelProps = { /** * The original/stored role of the user. @@ -111,7 +119,7 @@ const PermissionPanel: FC = ({ role }) => { >(() => { const defaults = data?.retrievePBACDefaults?.find((pbac) => pbac.role === selectedRole); if (!defaults || !defaults?.permissions) { - Logger.error("Role not found in PBAC defaults", { role: selectedRole }); + Logger.error("Role not found in PBAC defaults", { role: selectedRole, data }); return []; } @@ -148,7 +156,7 @@ const PermissionPanel: FC = ({ role }) => { >(() => { const defaults = data?.retrievePBACDefaults?.find((pbac) => pbac.role === selectedRole); if (!defaults || !defaults?.notifications) { - Logger.error("Role not found in PBAC defaults", { role: selectedRole }); + Logger.error("Role not found in PBAC defaults", { role: selectedRole, data }); return []; } @@ -244,6 +252,11 @@ const PermissionPanel: FC = ({ role }) => { ))} ))} + {permissionColumns.length === 0 && ( + + No permission options found for this role. + + )} @@ -274,6 +287,11 @@ const PermissionPanel: FC = ({ role }) => { ))} ))} + {notificationColumns.length === 0 && ( + + No notification options found for this role. + + )} From f776d1f17bcf17ca51553ef9f8b7a5b69f7e6e70 Mon Sep 17 00:00:00 2001 From: Alec M Date: Fri, 27 Dec 2024 11:48:57 -0500 Subject: [PATCH 06/21] Fix failing base test --- src/components/PermissionPanel/index.test.tsx | 28 +++++++++++++------ 1 file changed, 20 insertions(+), 8 deletions(-) diff --git a/src/components/PermissionPanel/index.test.tsx b/src/components/PermissionPanel/index.test.tsx index 46f4bcfdf..63985ea74 100644 --- a/src/components/PermissionPanel/index.test.tsx +++ b/src/components/PermissionPanel/index.test.tsx @@ -1,5 +1,5 @@ import { MockedProvider, MockedResponse } from "@apollo/client/testing"; -import { act, render, within } from "@testing-library/react"; +import { act, render, waitFor, within } from "@testing-library/react"; import { FormProvider, FormProviderProps } from "react-hook-form"; import { axe } from "jest-axe"; import { FC } from "react"; @@ -193,7 +193,7 @@ describe("Basic Functionality", () => { _id: "submission_request:create", group: "Submission Request", name: "Create", - checked: false, + checked: true, disabled: false, }, { @@ -234,7 +234,12 @@ describe("Basic Functionality", () => { }; const mockWatcher = jest.fn().mockImplementation((field) => { - if (field === "role") return "Admin"; + // Return the selected role (e.g. watch("role")) + if (field === "role") { + return "Submitter"; + } + + // Return the selected permissions (e.g. watch("permissions")) return []; }); @@ -246,11 +251,14 @@ describe("Basic Functionality", () => { ), }); + await waitFor(() => { + expect(getByTestId("permissions-group-Submission Request")).toBeInTheDocument(); + }); + const srGroup = getByTestId("permissions-group-Submission Request"); - expect(srGroup).toBeInTheDocument(); expect(within(srGroup).getByTestId("permission-submission_request:create")).toBeInTheDocument(); - const dsGroup = getByTestId("permissions-group-Data Submission"); + const dsGroup = getByTestId("permissions-group-Data Submission"); expect(dsGroup).toBeInTheDocument(); expect(within(dsGroup).getByTestId("permission-data_submission:view")).toBeInTheDocument(); @@ -258,10 +266,10 @@ describe("Basic Functionality", () => { expect(adminGroup).toBeInTheDocument(); expect(within(adminGroup).getByTestId("permission-program:manage")).toBeInTheDocument(); - const dsNotifGroup = getByTestId("notifications-group-Data Submissions"); - expect(dsNotifGroup).toBeInTheDocument(); + const dsEmailGroup = getByTestId("notifications-group-Data Submissions"); + expect(dsEmailGroup).toBeInTheDocument(); expect( - within(dsNotifGroup).getByTestId("notification-data_submission:cancelled") + within(dsEmailGroup).getByTestId("notification-data_submission:cancelled") ).toBeInTheDocument(); const accountGroup = getByTestId("notifications-group-Account"); @@ -297,6 +305,10 @@ describe("Implementation Requirements", () => { it.todo("should be rendered as collapsed by default"); + it.todo( + "should sort the permission groups in the following order: Submission Request, Data Submission, Admin, Miscellaneous" + ); + it.todo("should propagate the permissions selections to the parent form"); it.todo("should propagate the notification selections to the parent form"); From bd8c7b0cc1429a93408b3fd9f0b27422054a1beb Mon Sep 17 00:00:00 2001 From: Alec M Date: Fri, 27 Dec 2024 12:29:41 -0500 Subject: [PATCH 07/21] fix: Error logged during initial PBAC loading --- src/components/PermissionPanel/index.tsx | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/components/PermissionPanel/index.tsx b/src/components/PermissionPanel/index.tsx index c9cbf7f37..31d86b2b5 100644 --- a/src/components/PermissionPanel/index.tsx +++ b/src/components/PermissionPanel/index.tsx @@ -101,7 +101,7 @@ type PermissionPanelProps = { const PermissionPanel: FC = ({ role }) => { const { setValue, watch } = useFormContext(); - const { data } = useQuery( + const { data, loading } = useQuery( RETRIEVE_PBAC_DEFAULTS, { variables: { roles: ["All"] }, @@ -117,6 +117,10 @@ const PermissionPanel: FC = ({ role }) => { const permissionColumns = useMemo< Array[] }>> >(() => { + if (!data?.retrievePBACDefaults && loading) { + return []; + } + const defaults = data?.retrievePBACDefaults?.find((pbac) => pbac.role === selectedRole); if (!defaults || !defaults?.permissions) { Logger.error("Role not found in PBAC defaults", { role: selectedRole, data }); @@ -154,6 +158,10 @@ const PermissionPanel: FC = ({ role }) => { const notificationColumns = useMemo< Array[] }>> >(() => { + if (!data?.retrievePBACDefaults && loading) { + return []; + } + const defaults = data?.retrievePBACDefaults?.find((pbac) => pbac.role === selectedRole); if (!defaults || !defaults?.notifications) { Logger.error("Role not found in PBAC defaults", { role: selectedRole, data }); From 08e9a11d2491d4a66b8b1337f7b8f043129e7863 Mon Sep 17 00:00:00 2001 From: Alec M Date: Fri, 27 Dec 2024 13:25:11 -0500 Subject: [PATCH 08/21] Implement component behavior tests --- src/components/PermissionPanel/index.test.tsx | 269 +++++++++++++++++- src/components/PermissionPanel/index.tsx | 6 + 2 files changed, 264 insertions(+), 11 deletions(-) diff --git a/src/components/PermissionPanel/index.test.tsx b/src/components/PermissionPanel/index.test.tsx index 63985ea74..c8652eb87 100644 --- a/src/components/PermissionPanel/index.test.tsx +++ b/src/components/PermissionPanel/index.test.tsx @@ -3,6 +3,7 @@ import { act, render, waitFor, within } from "@testing-library/react"; import { FormProvider, FormProviderProps } from "react-hook-form"; import { axe } from "jest-axe"; import { FC } from "react"; +import { GraphQLError } from "graphql"; import PermissionPanel from "./index"; import { RETRIEVE_PBAC_DEFAULTS, @@ -277,28 +278,274 @@ describe("Basic Functionality", () => { expect(within(accountGroup).getByTestId("notification-account:disabled")).toBeInTheDocument(); }); - it.todo("should utilize a maximum of 3 columns for the permissions"); + it("should utilize a maximum of 3 columns for the permissions", async () => { + const mock: MockedResponse = { + request: { + query: RETRIEVE_PBAC_DEFAULTS, + variables: { roles: ["All"] }, + }, + result: { + data: { + retrievePBACDefaults: [ + { + role: "Submitter", + permissions: [ + { + _id: "submission_request:create", + group: "Group1", + name: "Create", + checked: true, + disabled: false, + }, + { + _id: "submission_request:review", + group: "Group2", + name: "Create", + checked: true, + disabled: false, + }, + { + _id: "submission_request:submit", + group: "Group3", + name: "Create", + checked: true, + disabled: false, + }, + { + _id: "submission_request:view", + group: "Group4", + name: "Create", + checked: true, + disabled: false, + }, + { + _id: "data_submission:create", + group: "Group5", + name: "Create", + checked: true, + disabled: false, + }, + { + _id: "study:manage", + group: "Group6", + name: "Create", + checked: true, + disabled: false, + }, + ], + notifications: [], + }, + ], + }, + }, + }; - it.todo("should utilize a maximum of 3 columns for the notifications"); + const mockWatcher = jest.fn().mockImplementation((field) => { + if (field === "role") { + return "Submitter"; + } - it.todo( - "should utilize the current permissions to determine the checked state of each permission" - ); + return []; + }); - it.todo("should log an error when unable to retrieve the default PBAC details (GraphQL)"); + const { getByTestId, queryByTestId } = render(, { + wrapper: ({ children }) => ( + + {children} + + ), + }); - it.todo("should log an error when unable to retrieve the default PBAC details (Network)"); + await waitFor(() => { + expect(getByTestId("permissions-column-0")).toBeInTheDocument(); + expect(getByTestId("permissions-column-1")).toBeInTheDocument(); + expect(getByTestId("permissions-column-2")).toBeInTheDocument(); + }); - it.todo("should log an error when unable to retrieve the default PBAC details (API)"); + // Column 0-1 (has 1 group) + expect( + within(getByTestId("permissions-column-0")).getByTestId("permissions-group-Group1") + ).toBeInTheDocument(); + expect( + within(getByTestId("permissions-column-1")).getByTestId("permissions-group-Group2") + ).toBeInTheDocument(); - it.todo("should log an error when a permission is assigned but not provided in the defaults"); + // Column 2 (has remaining groups) + expect( + within(getByTestId("permissions-column-2")).getByTestId("permissions-group-Group3") + ).toBeInTheDocument(); + expect( + within(getByTestId("permissions-column-2")).getByTestId("permissions-group-Group4") + ).toBeInTheDocument(); + expect( + within(getByTestId("permissions-column-2")).getByTestId("permissions-group-Group5") + ).toBeInTheDocument(); + expect( + within(getByTestId("permissions-column-2")).getByTestId("permissions-group-Group6") + ).toBeInTheDocument(); - it.todo("should log an error when a notification is assigned but not provided in the defaults"); + // Sanity check + expect(queryByTestId("permissions-column-3")).not.toBeInTheDocument(); + }); - it.todo("should render a loading state when fetching the default PBAC details"); + it("should utilize a maximum of 3 columns for the notifications", async () => { + const mock: MockedResponse = { + request: { + query: RETRIEVE_PBAC_DEFAULTS, + variables: { roles: ["All"] }, + }, + result: { + data: { + retrievePBACDefaults: [ + { + role: "Submitter", + permissions: [], + notifications: [ + { + _id: "access:requested", + group: "Group1", + name: "Notification 1", + checked: true, + disabled: false, + }, + { + _id: "account:disabled", + group: "Group2", + name: "Notification 2", + checked: true, + disabled: false, + }, + { + _id: "data_submission:cancelled", + group: "Group3", + name: "Notification 3", + checked: true, + disabled: false, + }, + { + _id: "submission_request:to_be_reviewed", + group: "Group4", + name: "Notification 4", + checked: true, + disabled: false, + }, + { + _id: "data_submission:withdrawn", + group: "Group5", + name: "Notification 5", + checked: true, + disabled: false, + }, + { + _id: "data_submission:deleted", + group: "Group6", + name: "Notification 6", + checked: true, + disabled: false, + }, + ], + }, + ], + }, + }, + }; + + const mockWatcher = jest.fn().mockImplementation((field) => { + if (field === "role") { + return "Submitter"; + } + + return []; + }); + + const { getByTestId, queryByTestId } = render(, { + wrapper: ({ children }) => ( + + {children} + + ), + }); + + await waitFor(() => { + expect(getByTestId("notifications-column-0")).toBeInTheDocument(); + expect(getByTestId("notifications-column-1")).toBeInTheDocument(); + expect(getByTestId("notifications-column-2")).toBeInTheDocument(); + }); + + // Column 0-1 (has 1 group) + expect( + within(getByTestId("notifications-column-0")).getByTestId("notifications-group-Group1") + ).toBeInTheDocument(); + expect( + within(getByTestId("notifications-column-1")).getByTestId("notifications-group-Group2") + ).toBeInTheDocument(); + + // Column 2 (has remaining groups) + expect( + within(getByTestId("notifications-column-2")).getByTestId("notifications-group-Group3") + ).toBeInTheDocument(); + expect( + within(getByTestId("notifications-column-2")).getByTestId("notifications-group-Group4") + ).toBeInTheDocument(); + expect( + within(getByTestId("notifications-column-2")).getByTestId("notifications-group-Group5") + ).toBeInTheDocument(); + expect( + within(getByTestId("notifications-column-2")).getByTestId("notifications-group-Group6") + ).toBeInTheDocument(); + + // Sanity check + expect(queryByTestId("notifications-column-3")).not.toBeInTheDocument(); + }); + + it("should show an error when unable to retrieve the default PBAC details (GraphQL)", async () => { + const mock: MockedResponse = { + request: { + query: RETRIEVE_PBAC_DEFAULTS, + variables: { roles: ["All"] }, + }, + result: { + errors: [new GraphQLError("Mock Error")], + }, + }; + + render(, { + wrapper: ({ children }) => {children}, + }); + + await waitFor(() => { + expect(global.mockEnqueue).toHaveBeenCalledWith(expect.any(String), { + variant: "error", + }); + }); + }); + + it("should show an error when unable to retrieve the default PBAC details (Network)", async () => { + const mock: MockedResponse = { + request: { + query: RETRIEVE_PBAC_DEFAULTS, + variables: { roles: ["All"] }, + }, + error: new Error("Network error"), + }; + + render(, { + wrapper: ({ children }) => {children}, + }); + + await waitFor(() => { + expect(global.mockEnqueue).toHaveBeenCalledWith(expect.any(String), { + variant: "error", + }); + }); + }); }); describe("Implementation Requirements", () => { + it.todo( + "should utilize the current permissions to determine the checked state of each permission" + ); + it.todo("should reset the permissions to their default values when the role changes"); it.todo("should allow disabled permissions to be checked by default"); diff --git a/src/components/PermissionPanel/index.tsx b/src/components/PermissionPanel/index.tsx index 31d86b2b5..60e5b9efb 100644 --- a/src/components/PermissionPanel/index.tsx +++ b/src/components/PermissionPanel/index.tsx @@ -13,6 +13,7 @@ import { import { FC, memo, useEffect, useMemo } from "react"; import { useFormContext } from "react-hook-form"; import Grid2 from "@mui/material/Unstable_Grid2"; +import { useSnackbar } from "notistack"; import { EditUserInput, RetrievePBACDefaultsResp, @@ -99,6 +100,7 @@ type PermissionPanelProps = { * @returns The PermissionPanel component. */ const PermissionPanel: FC = ({ role }) => { + const { enqueueSnackbar } = useSnackbar(); const { setValue, watch } = useFormContext(); const { data, loading } = useQuery( @@ -107,6 +109,10 @@ const PermissionPanel: FC = ({ role }) => { variables: { roles: ["All"] }, context: { clientName: "backend" }, fetchPolicy: "cache-first", + onError: (error) => { + Logger.error("Failed to retrieve PBAC defaults", { error }); + enqueueSnackbar("Failed to retrieve PBAC defaults", { variant: "error" }); + }, } ); From 8f4453fdf4ee0aa8bc1b06e0fa8d311b6b23b1b9 Mon Sep 17 00:00:00 2001 From: Alec M Date: Fri, 27 Dec 2024 15:04:58 -0500 Subject: [PATCH 09/21] Add coverage for requirements --- src/components/PermissionPanel/index.test.tsx | 510 +++++++++++++++++- 1 file changed, 502 insertions(+), 8 deletions(-) diff --git a/src/components/PermissionPanel/index.test.tsx b/src/components/PermissionPanel/index.test.tsx index c8652eb87..6ee1c9de8 100644 --- a/src/components/PermissionPanel/index.test.tsx +++ b/src/components/PermissionPanel/index.test.tsx @@ -4,6 +4,7 @@ import { FormProvider, FormProviderProps } from "react-hook-form"; import { axe } from "jest-axe"; import { FC } from "react"; import { GraphQLError } from "graphql"; +import userEvent from "@testing-library/user-event"; import PermissionPanel from "./index"; import { RETRIEVE_PBAC_DEFAULTS, @@ -542,23 +543,516 @@ describe("Basic Functionality", () => { }); describe("Implementation Requirements", () => { - it.todo( - "should utilize the current permissions to determine the checked state of each permission" - ); + it("should utilize the initial form values to determine the checked state of each permission", async () => { + const mock: MockedResponse = { + request: { + query: RETRIEVE_PBAC_DEFAULTS, + variables: { roles: ["All"] }, + }, + result: { + data: { + retrievePBACDefaults: [ + { + role: "Submitter", + permissions: [ + { + _id: "submission_request:create", + group: "Submission Request", + name: "Create", + checked: false, + disabled: false, + }, + { + _id: "data_submission:view", + group: "Data Submission", + name: "View", + checked: false, + disabled: false, + }, + { + _id: "program:manage", + group: "Admin", + name: "Manage Programs", + checked: false, + disabled: false, + }, + ], + notifications: [ + { + _id: "data_submission:cancelled", + group: "Data Submissions", + name: "Cancelled", + checked: false, + disabled: false, + }, + { + _id: "account:disabled", + group: "Account", + name: "Disabled", + checked: false, + disabled: false, + }, + ], + }, + ], + }, + }, + }; + + const mockWatcher = jest.fn().mockImplementation((field) => { + if (field === "role") { + return "Submitter"; + } + + if (field === "permissions") { + return ["submission_request:create", "program:manage"]; + } + + if (field === "notifications") { + return ["data_submission:cancelled"]; + } + + return []; + }); + + const { getByTestId } = render(, { + wrapper: ({ children }) => ( + + {children} + + ), + }); + + await waitFor(() => { + expect(getByTestId("permission-submission_request:create")).toBeInTheDocument(); + }); + + // Checked permissions by default based on the initial form values + expect( + within(getByTestId("permission-submission_request:create")).getByRole("checkbox", { + hidden: true, + }) + ).toBeChecked(); + expect( + within(getByTestId("permission-program:manage")).getByRole("checkbox", { + hidden: true, + }) + ).toBeChecked(); + + // Unchecked permissions sanity check + expect( + within(getByTestId("permission-data_submission:view")).getByRole("checkbox", { + hidden: true, + }) + ).not.toBeChecked(); + + // Checked notifications by default based on the initial form values + expect( + within(getByTestId("notification-data_submission:cancelled")).getByRole("checkbox", { + hidden: true, + }) + ).toBeChecked(); + + // Unchecked notifications sanity check + expect( + within(getByTestId("notification-account:disabled")).getByRole("checkbox", { + hidden: true, + }) + ).not.toBeChecked(); + }); + + it("should reset the permissions to their default values when the role changes", async () => { + const mock: MockedResponse = { + request: { + query: RETRIEVE_PBAC_DEFAULTS, + variables: { roles: ["All"] }, + }, + result: { + data: { + retrievePBACDefaults: [ + { + role: "Submitter", + permissions: [ + { + _id: "submission_request:create", + group: "Submission Request", + name: "Create", + checked: false, + disabled: false, + }, + { + _id: "data_submission:view", + group: "Data Submission", + name: "View", + checked: false, + disabled: false, + }, + ], + notifications: [ + { + _id: "data_submission:cancelled", + group: "Data Submissions", + name: "Cancelled", + checked: false, + disabled: false, + }, + { + _id: "account:disabled", + group: "Account", + name: "Disabled", + checked: false, + disabled: false, + }, + ], + }, + { + role: "Federal Lead", + permissions: [ + { + _id: "submission_request:create", + group: "Submission Request", + name: "Create", + checked: false, // Original submitter had this checked + disabled: false, + }, + { + _id: "data_submission:view", + group: "Data Submission", + name: "View", + checked: true, + disabled: false, + }, + ], + notifications: [ + { + _id: "data_submission:cancelled", + group: "Data Submissions", + name: "Cancelled", + checked: false, // Original submitter had this checked + disabled: false, + }, + { + _id: "account:disabled", + group: "Account", + name: "Disabled", + checked: true, + disabled: false, + }, + ], + }, + ], + }, + }, + }; + + const formValues = { + role: "Submitter", + permissions: ["submission_request:create"], + notifications: ["data_submission:cancelled"], + }; + + const mockWatcher = jest.fn().mockImplementation((field) => formValues[field] ?? ""); + + const mockSetValue = jest.fn().mockImplementation((field, value) => { + formValues[field] = value; + }); - it.todo("should reset the permissions to their default values when the role changes"); + const { getByTestId, rerender } = render(, { + wrapper: ({ children }) => ( + + {children} + + ), + }); + + await waitFor(() => { + expect(getByTestId("permission-submission_request:create")).toBeInTheDocument(); + }); + + // Checked permissions by default based on the initial form values + expect( + within(getByTestId("permission-submission_request:create")).getByRole("checkbox", { + hidden: true, + }) + ).toBeChecked(); + + // Unchecked permissions sanity check + expect( + within(getByTestId("permission-data_submission:view")).getByRole("checkbox", { + hidden: true, + }) + ).not.toBeChecked(); + + // Checked notifications by default based on the initial form values + expect( + within(getByTestId("notification-data_submission:cancelled")).getByRole("checkbox", { + hidden: true, + }) + ).toBeChecked(); + + // Unchecked notifications sanity check + expect( + within(getByTestId("notification-account:disabled")).getByRole("checkbox", { + hidden: true, + }) + ).not.toBeChecked(); + + // Change the role + formValues.role = "Federal Lead"; + + rerender(); // The original role is "Submitter", nothing should change + rerender(); // This is a work-around to trigger the UI update + + // Checked permissions by default based on the NEW role + expect( + within(getByTestId("permission-data_submission:view")).getByRole("checkbox", { + hidden: true, + }) + ).toBeChecked(); + + // Unchecked permissions sanity check + expect( + within(getByTestId("permission-submission_request:create")).getByRole("checkbox", { + hidden: true, + }) + ).not.toBeChecked(); + + // Checked notifications by default based on the NEW role + expect( + within(getByTestId("notification-account:disabled")).getByRole("checkbox", { + hidden: true, + }) + ).toBeChecked(); + + // Unchecked notifications sanity check + expect( + within(getByTestId("notification-data_submission:cancelled")).getByRole("checkbox", { + hidden: true, + }) + ).not.toBeChecked(); + }); + + it("should allow disabled PBAC options to be checked by default", async () => { + const mock: MockedResponse = { + request: { + query: RETRIEVE_PBAC_DEFAULTS, + variables: { roles: ["All"] }, + }, + result: { + data: { + retrievePBACDefaults: [ + { + role: "Submitter", + permissions: [ + { + _id: "submission_request:create", + group: "Submission Request", + name: "Create", + checked: true, + disabled: false, + }, + { + _id: "data_submission:view", + group: "Data Submission", + name: "View", + checked: true, + disabled: false, + }, + ], + notifications: [ + { + _id: "data_submission:cancelled", + group: "Data Submissions", + name: "Cancelled", + checked: true, + disabled: true, + }, + { + _id: "account:disabled", + group: "Account", + name: "Disabled", + checked: true, + disabled: false, + }, + ], + }, + ], + }, + }, + }; + + const formValues = { + role: "Federal Lead", + permissions: [], + notifications: [], + }; + + const mockWatcher = jest.fn().mockImplementation((field) => formValues[field] ?? ""); + + const mockSetValue = jest.fn().mockImplementation((field, value) => { + formValues[field] = value; + }); + + const { getByTestId, rerender } = render(, { + wrapper: ({ children }) => ( + + {children} + + ), + }); + + // Trigger role change + formValues.role = "Submitter"; - it.todo("should allow disabled permissions to be checked by default"); + rerender(); + rerender(); - it.todo("should be rendered as collapsed by default"); + await waitFor(() => { + expect(getByTestId("permission-submission_request:create")).toBeInTheDocument(); + }); + + expect( + within(getByTestId("permission-submission_request:create")).getByRole("checkbox", { + hidden: true, + }) + ).toBeChecked(); + expect( + within(getByTestId("permission-submission_request:create")).getByRole("checkbox", { + hidden: true, + }) + ).toBeDisabled(); + }); + + it("should be rendered as collapsed by default", async () => { + const mock: MockedResponse = { + request: { + query: RETRIEVE_PBAC_DEFAULTS, + variables: { roles: ["All"] }, + }, + result: { + data: { + retrievePBACDefaults: [], + }, + }, + }; + + const { getByTestId } = render(, { + wrapper: ({ children }) => {children}, + }); + + expect(within(getByTestId("permissions-accordion")).getByRole("button")).toHaveAttribute( + "aria-expanded", + "false" + ); + + userEvent.click(within(getByTestId("permissions-accordion")).getByRole("button")); + + expect(within(getByTestId("permissions-accordion")).getByRole("button")).toHaveAttribute( + "aria-expanded", + "true" + ); + + expect(within(getByTestId("notifications-accordion")).getByRole("button")).toHaveAttribute( + "aria-expanded", + "false" + ); + + userEvent.click(within(getByTestId("notifications-accordion")).getByRole("button")); + + expect(within(getByTestId("notifications-accordion")).getByRole("button")).toHaveAttribute( + "aria-expanded", + "true" + ); + }); it.todo( "should sort the permission groups in the following order: Submission Request, Data Submission, Admin, Miscellaneous" ); - it.todo("should propagate the permissions selections to the parent form"); + it("should propagate the permission and notification selections to the parent form", async () => { + const mock: MockedResponse = { + request: { + query: RETRIEVE_PBAC_DEFAULTS, + variables: { roles: ["All"] }, + }, + result: { + data: { + retrievePBACDefaults: [ + { + role: "Submitter", + permissions: [ + { + _id: "submission_request:create", + group: "Submission Request", + name: "Create", + checked: false, + disabled: false, + }, + ], + notifications: [ + { + _id: "data_submission:cancelled", + group: "Data Submissions", + name: "Cancelled", + checked: false, + disabled: false, + }, + ], + }, + ], + }, + }, + }; - it.todo("should propagate the notification selections to the parent form"); + const formValues = { + role: "Submitter", + permissions: [], + notifications: [], + }; + + const mockWatcher = jest.fn().mockImplementation((field) => formValues[field] ?? ""); + + const mockSetValue = jest.fn().mockImplementation((field, value) => { + formValues[field] = value; + }); + + const { getByTestId } = render(, { + wrapper: ({ children }) => ( + + {children} + + ), + }); + + await waitFor(() => { + expect(getByTestId("permission-submission_request:create")).toBeInTheDocument(); + }); + + userEvent.click( + within(getByTestId("permission-submission_request:create")).getByRole("checkbox", { + hidden: true, + }) + ); + + expect(mockSetValue).toHaveBeenCalledWith("permissions", ["submission_request:create"]); + + userEvent.click( + within(getByTestId("notification-data_submission:cancelled")).getByRole("checkbox", { + hidden: true, + }) + ); + + expect(mockSetValue).toHaveBeenCalledWith("notifications", ["data_submission:cancelled"]); + }); it("should render a notice when there are no default PBAC details for a role", async () => { const mock: MockedResponse = { From fb598ac5519846353aa034aebd256de31a524b9c Mon Sep 17 00:00:00 2001 From: Alec M Date: Fri, 27 Dec 2024 15:59:59 -0500 Subject: [PATCH 10/21] Cleanup PBAC panel implementation --- src/components/PermissionPanel/index.tsx | 52 +++++++++++------------- src/types/PBAC.d.ts | 2 +- 2 files changed, 24 insertions(+), 30 deletions(-) diff --git a/src/components/PermissionPanel/index.tsx b/src/components/PermissionPanel/index.tsx index 60e5b9efb..fb49b550a 100644 --- a/src/components/PermissionPanel/index.tsx +++ b/src/components/PermissionPanel/index.tsx @@ -10,10 +10,11 @@ import { styled, Typography, } from "@mui/material"; -import { FC, memo, useEffect, useMemo } from "react"; +import { FC, memo, useEffect, useMemo, useRef } from "react"; import { useFormContext } from "react-hook-form"; import Grid2 from "@mui/material/Unstable_Grid2"; import { useSnackbar } from "notistack"; +import { cloneDeep } from "lodash"; import { EditUserInput, RetrievePBACDefaultsResp, @@ -90,6 +91,8 @@ type PermissionPanelProps = { * The original/stored role of the user. * * This is used to determine if the role has changed and to update the default permissions. + * + * @deprecated This prop is no longer used. Remove it. */ role: UserRole; }; @@ -119,9 +122,10 @@ const PermissionPanel: FC = ({ role }) => { const selectedRole = watch("role"); const permissionsValue = watch("permissions"); const notificationsValue = watch("notifications"); + const roleRef = useRef(selectedRole); const permissionColumns = useMemo< - Array[] }>> + { name: string; data: PBACDefault[] }[][] >(() => { if (!data?.retrievePBACDefaults && loading) { return []; @@ -133,10 +137,9 @@ const PermissionPanel: FC = ({ role }) => { return []; } - const updatedPermissions: PBACDefault[] = defaults?.permissions.map((p) => ({ - ...p, - checked: permissionsValue.includes(p._id), - })); + const updatedPermissions: PBACDefault[] = cloneDeep(defaults.permissions).map( + (p) => ({ ...p, checked: permissionsValue.includes(p._id) }) + ); const groupedPermissions: Record[]> = updatedPermissions.reduce((acc, p) => { @@ -147,22 +150,17 @@ const PermissionPanel: FC = ({ role }) => { return acc; }, {}); - const columns: Array[] }>> = [ - [], - [], - [], - ]; - + const columns: { name: string; data: PBACDefault[] }[][] = [[], [], []]; Object.entries(groupedPermissions).forEach(([name, permissions], index) => { const placement = index > 1 ? 2 : index; - columns[placement].push({ name, permissions }); + columns[placement].push({ name, data: permissions }); }); return columns; }, [data, permissionsValue]); const notificationColumns = useMemo< - Array[] }>> + { name: string; data: PBACDefault[] }[][] >(() => { if (!data?.retrievePBACDefaults && loading) { return []; @@ -174,12 +172,9 @@ const PermissionPanel: FC = ({ role }) => { return []; } - const updatedNotifications: PBACDefault[] = defaults?.notifications.map( - (n) => ({ - ...n, - checked: notificationsValue.includes(n._id), - }) - ); + const updatedNotifications: PBACDefault[] = cloneDeep( + defaults.notifications + ).map((n) => ({ ...n, checked: notificationsValue.includes(n._id) })); const groupedNotifications: Record[]> = updatedNotifications.reduce((acc, n) => { @@ -190,12 +185,10 @@ const PermissionPanel: FC = ({ role }) => { return acc; }, {}); - const columns: Array[] }>> = - [[], [], []]; - + const columns: { name: string; data: PBACDefault[] }[][] = [[], [], []]; Object.entries(groupedNotifications).forEach(([name, notifications], index) => { const placement = index > 1 ? 2 : index; - columns[placement].push({ name, notifications }); + columns[placement].push({ name, data: notifications }); }); return columns; @@ -224,13 +217,14 @@ const PermissionPanel: FC = ({ role }) => { }; const handleRoleChange = (selectedRole: UserRole) => { - if (selectedRole === role) { + if (selectedRole === roleRef.current) { return; } const defaults = data?.retrievePBACDefaults?.find((pbac) => pbac.role === selectedRole); setValue("permissions", defaults?.permissions?.filter((p) => p.checked).map((p) => p._id)); setValue("notifications", defaults?.notifications?.filter((n) => n.checked).map((n) => n._id)); + roleRef.current = selectedRole; }; useEffect(() => { @@ -248,11 +242,11 @@ const PermissionPanel: FC = ({ role }) => { {permissionColumns.map((column, index) => ( // eslint-disable-next-line react/no-array-index-key - {column.map(({ name, permissions }) => ( + {column.map(({ name, data }) => (
{name} - {permissions.map(({ _id, checked, disabled, name }) => ( + {data.map(({ _id, checked, disabled, name }) => ( = ({ role }) => { {notificationColumns.map((column, index) => ( // eslint-disable-next-line react/no-array-index-key - {column.map(({ name, notifications }) => ( + {column.map(({ name, data }) => (
{name} - {notifications.map(({ _id, checked, disabled, name }) => ( + {data.map(({ _id, checked, disabled, name }) => ( = { */ group: string; /** - * The name of the PBAC object. + * The name of the individual PBAC setting. * * @example "Manage Users" */ From d670ff716767d0aef87c8ab61bb0bb6ca6278b47 Mon Sep 17 00:00:00 2001 From: Alec M Date: Fri, 27 Dec 2024 16:14:33 -0500 Subject: [PATCH 11/21] fix: Apollo normalizing permissions incorrectly --- src/client.ts | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/client.ts b/src/client.ts index 66b9b72db..209ae78ba 100644 --- a/src/client.ts +++ b/src/client.ts @@ -31,6 +31,15 @@ const cache = new InMemoryCache({ Collaborator: { keyFields: ["collaboratorID"], }, + PBACDefaults: { + keyFields: ["role"], + }, + Permission: { + keyFields: false, + }, + Notification: { + keyFields: false, + }, }, }); From 636d00dc0baa2113df5bf6695700b02b19ef11b2 Mon Sep 17 00:00:00 2001 From: Alec M Date: Fri, 27 Dec 2024 16:20:10 -0500 Subject: [PATCH 12/21] fix: PBAC data not sent to API --- src/graphql/editUser.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/graphql/editUser.ts b/src/graphql/editUser.ts index f6006c19c..0ca66f8f7 100644 --- a/src/graphql/editUser.ts +++ b/src/graphql/editUser.ts @@ -7,6 +7,8 @@ export const mutation = gql` $role: String $studies: [String] $dataCommons: [String] + $permissions: [String] + $notifications: [String] ) { editUser( userID: $userID @@ -14,6 +16,8 @@ export const mutation = gql` role: $role studies: $studies dataCommons: $dataCommons + permissions: $permissions + notifications: $notifications ) { userStatus role From e30fbe2141f3c16caf9569924ef6cf964630e8fe Mon Sep 17 00:00:00 2001 From: Alec M Date: Mon, 30 Dec 2024 09:11:54 -0500 Subject: [PATCH 13/21] Remove unused `role` prop --- src/components/PermissionPanel/index.test.tsx | 40 +++++++++---------- src/components/PermissionPanel/index.tsx | 13 +----- src/content/users/ProfileView.tsx | 4 +- 3 files changed, 22 insertions(+), 35 deletions(-) diff --git a/src/components/PermissionPanel/index.test.tsx b/src/components/PermissionPanel/index.test.tsx index 6ee1c9de8..6f6045f04 100644 --- a/src/components/PermissionPanel/index.test.tsx +++ b/src/components/PermissionPanel/index.test.tsx @@ -50,7 +50,7 @@ describe("Accessibility", () => { }, }; - const { container } = render(, { + const { container } = render(, { wrapper: ({ children }) => {children}, }); @@ -110,7 +110,7 @@ describe("Accessibility", () => { }, }; - const { container } = render(, { + const { container } = render(, { wrapper: ({ children }) => {children}, }); @@ -142,7 +142,7 @@ describe("Basic Functionality", () => { }, }; - render(, { + render(, { wrapper: ({ children }) => {children}, }); }); @@ -168,13 +168,13 @@ describe("Basic Functionality", () => { maxUsageCount: 999, }; - const { rerender } = render(, { + const { rerender } = render(, { wrapper: ({ children }) => {children}, }); expect(mockMatcher).toHaveBeenCalledTimes(1); - rerender(); + rerender(); expect(mockMatcher).toHaveBeenCalledTimes(1); }); @@ -245,7 +245,7 @@ describe("Basic Functionality", () => { return []; }); - const { getByTestId } = render(, { + const { getByTestId } = render(, { wrapper: ({ children }) => ( {children} @@ -349,7 +349,7 @@ describe("Basic Functionality", () => { return []; }); - const { getByTestId, queryByTestId } = render(, { + const { getByTestId, queryByTestId } = render(, { wrapper: ({ children }) => ( {children} @@ -459,7 +459,7 @@ describe("Basic Functionality", () => { return []; }); - const { getByTestId, queryByTestId } = render(, { + const { getByTestId, queryByTestId } = render(, { wrapper: ({ children }) => ( {children} @@ -510,7 +510,7 @@ describe("Basic Functionality", () => { }, }; - render(, { + render(, { wrapper: ({ children }) => {children}, }); @@ -530,7 +530,7 @@ describe("Basic Functionality", () => { error: new Error("Network error"), }; - render(, { + render(, { wrapper: ({ children }) => {children}, }); @@ -615,7 +615,7 @@ describe("Implementation Requirements", () => { return []; }); - const { getByTestId } = render(, { + const { getByTestId } = render(, { wrapper: ({ children }) => ( {children} @@ -757,7 +757,7 @@ describe("Implementation Requirements", () => { formValues[field] = value; }); - const { getByTestId, rerender } = render(, { + const { getByTestId, rerender } = render(, { wrapper: ({ children }) => ( { // Change the role formValues.role = "Federal Lead"; - rerender(); // The original role is "Submitter", nothing should change - rerender(); // This is a work-around to trigger the UI update + rerender(); // The original role is "Submitter", nothing should change + rerender(); // This is a work-around to trigger the UI update // Checked permissions by default based on the NEW role expect( @@ -896,7 +896,7 @@ describe("Implementation Requirements", () => { formValues[field] = value; }); - const { getByTestId, rerender } = render(, { + const { getByTestId, rerender } = render(, { wrapper: ({ children }) => ( { // Trigger role change formValues.role = "Submitter"; - rerender(); - rerender(); + rerender(); + rerender(); await waitFor(() => { expect(getByTestId("permission-submission_request:create")).toBeInTheDocument(); @@ -942,7 +942,7 @@ describe("Implementation Requirements", () => { }, }; - const { getByTestId } = render(, { + const { getByTestId } = render(, { wrapper: ({ children }) => {children}, }); @@ -1022,7 +1022,7 @@ describe("Implementation Requirements", () => { formValues[field] = value; }); - const { getByTestId } = render(, { + const { getByTestId } = render(, { wrapper: ({ children }) => ( { }, }; - const { getByTestId } = render(, { + const { getByTestId } = render(, { wrapper: ({ children }) => {children}, }); diff --git a/src/components/PermissionPanel/index.tsx b/src/components/PermissionPanel/index.tsx index fb49b550a..c5a8c19cf 100644 --- a/src/components/PermissionPanel/index.tsx +++ b/src/components/PermissionPanel/index.tsx @@ -86,23 +86,12 @@ const StyledNotice = styled(Typography)({ userSelect: "none", }); -type PermissionPanelProps = { - /** - * The original/stored role of the user. - * - * This is used to determine if the role has changed and to update the default permissions. - * - * @deprecated This prop is no longer used. Remove it. - */ - role: UserRole; -}; - /** * Provides a panel for managing permissions and notifications for a user role. * * @returns The PermissionPanel component. */ -const PermissionPanel: FC = ({ role }) => { +const PermissionPanel: FC = () => { const { enqueueSnackbar } = useSnackbar(); const { setValue, watch } = useFormContext(); diff --git a/src/content/users/ProfileView.tsx b/src/content/users/ProfileView.tsx index c9c3121f3..0b217dc01 100644 --- a/src/content/users/ProfileView.tsx +++ b/src/content/users/ProfileView.tsx @@ -579,9 +579,7 @@ const ProfileView: FC = ({ _id, viewType }: Props) => { )} {VisibleFieldState.includes(fieldset.permissions) && - VisibleFieldState.includes(fieldset.notifications) && ( - - )} + VisibleFieldState.includes(fieldset.notifications) && } Date: Mon, 30 Dec 2024 09:43:33 -0500 Subject: [PATCH 14/21] Finish coverage --- src/components/PermissionPanel/index.test.tsx | 45 ++++++++++++++++--- 1 file changed, 40 insertions(+), 5 deletions(-) diff --git a/src/components/PermissionPanel/index.test.tsx b/src/components/PermissionPanel/index.test.tsx index 6f6045f04..2e4085f23 100644 --- a/src/components/PermissionPanel/index.test.tsx +++ b/src/components/PermissionPanel/index.test.tsx @@ -298,6 +298,13 @@ describe("Basic Functionality", () => { checked: true, disabled: false, }, + { + _id: "submission_request:submit", + group: "Group1", + name: "Create", + checked: true, + disabled: false, + }, { _id: "submission_request:review", group: "Group2", @@ -409,6 +416,13 @@ describe("Basic Functionality", () => { checked: true, disabled: false, }, + { + _id: "data_submission:deleted", + group: "Group1", + name: "Notification 1-2", + checked: true, + disabled: false, + }, { _id: "account:disabled", group: "Group2", @@ -852,7 +866,7 @@ describe("Implementation Requirements", () => { group: "Submission Request", name: "Create", checked: true, - disabled: false, + disabled: true, }, { _id: "data_submission:view", @@ -882,6 +896,7 @@ describe("Implementation Requirements", () => { ], }, }, + maxUsageCount: 999, }; const formValues = { @@ -910,7 +925,6 @@ describe("Implementation Requirements", () => { // Trigger role change formValues.role = "Submitter"; - rerender(); rerender(); await waitFor(() => { @@ -921,9 +935,10 @@ describe("Implementation Requirements", () => { within(getByTestId("permission-submission_request:create")).getByRole("checkbox", { hidden: true, }) - ).toBeChecked(); + ).toBeDisabled(); + expect( - within(getByTestId("permission-submission_request:create")).getByRole("checkbox", { + within(getByTestId("notification-data_submission:cancelled")).getByRole("checkbox", { hidden: true, }) ).toBeDisabled(); @@ -1022,7 +1037,7 @@ describe("Implementation Requirements", () => { formValues[field] = value; }); - const { getByTestId } = render(, { + const { getByTestId, rerender } = render(, { wrapper: ({ children }) => ( { expect(mockSetValue).toHaveBeenCalledWith("permissions", ["submission_request:create"]); + rerender(); // Force the watch() to be called again and update the form values + + userEvent.click( + within(getByTestId("permission-submission_request:create")).getByRole("checkbox", { + hidden: true, + }) + ); + + expect(mockSetValue).toHaveBeenCalledWith("permissions", []); + userEvent.click( within(getByTestId("notification-data_submission:cancelled")).getByRole("checkbox", { hidden: true, @@ -1052,6 +1077,16 @@ describe("Implementation Requirements", () => { ); expect(mockSetValue).toHaveBeenCalledWith("notifications", ["data_submission:cancelled"]); + + rerender(); // Force the watch() to be called again and update the form values + + userEvent.click( + within(getByTestId("notification-data_submission:cancelled")).getByRole("checkbox", { + hidden: true, + }) + ); + + expect(mockSetValue).toHaveBeenCalledWith("notifications", []); }); it("should render a notice when there are no default PBAC details for a role", async () => { From 3f04f0eb2a8a20251da96f758e91213e36033355 Mon Sep 17 00:00:00 2001 From: Alec M Date: Mon, 30 Dec 2024 10:54:32 -0500 Subject: [PATCH 15/21] Deduplicate PBAC grouping logic --- src/components/PermissionPanel/index.tsx | 48 ++------- src/types/PBAC.d.ts | 2 +- src/utils/profileUtils.test.ts | 124 +++++++++++++++++++++++ src/utils/profileUtils.ts | 61 +++++++++++ 4 files changed, 193 insertions(+), 42 deletions(-) diff --git a/src/components/PermissionPanel/index.tsx b/src/components/PermissionPanel/index.tsx index c5a8c19cf..1caf1a984 100644 --- a/src/components/PermissionPanel/index.tsx +++ b/src/components/PermissionPanel/index.tsx @@ -21,7 +21,7 @@ import { RetrievePBACDefaultsInput, RETRIEVE_PBAC_DEFAULTS, } from "../../graphql"; -import { Logger } from "../../utils"; +import { ColumnizedPBACGroups, columnizePBACGroups, Logger } from "../../utils"; const StyledAccordion = styled(Accordion)({ width: "957px", // TODO: Need to fix the page layout @@ -113,9 +113,7 @@ const PermissionPanel: FC = () => { const notificationsValue = watch("notifications"); const roleRef = useRef(selectedRole); - const permissionColumns = useMemo< - { name: string; data: PBACDefault[] }[][] - >(() => { + const permissionColumns = useMemo>(() => { if (!data?.retrievePBACDefaults && loading) { return []; } @@ -126,31 +124,14 @@ const PermissionPanel: FC = () => { return []; } - const updatedPermissions: PBACDefault[] = cloneDeep(defaults.permissions).map( + const remappedPermissions: PBACDefault[] = cloneDeep(defaults.permissions).map( (p) => ({ ...p, checked: permissionsValue.includes(p._id) }) ); - const groupedPermissions: Record[]> = - updatedPermissions.reduce((acc, p) => { - if (!acc[p.group]) { - acc[p.group] = []; - } - acc[p.group].push(p); - return acc; - }, {}); - - const columns: { name: string; data: PBACDefault[] }[][] = [[], [], []]; - Object.entries(groupedPermissions).forEach(([name, permissions], index) => { - const placement = index > 1 ? 2 : index; - columns[placement].push({ name, data: permissions }); - }); - - return columns; + return columnizePBACGroups(remappedPermissions, 3); }, [data, permissionsValue]); - const notificationColumns = useMemo< - { name: string; data: PBACDefault[] }[][] - >(() => { + const notificationColumns = useMemo>(() => { if (!data?.retrievePBACDefaults && loading) { return []; } @@ -161,26 +142,11 @@ const PermissionPanel: FC = () => { return []; } - const updatedNotifications: PBACDefault[] = cloneDeep( + const remappedNotifications: PBACDefault[] = cloneDeep( defaults.notifications ).map((n) => ({ ...n, checked: notificationsValue.includes(n._id) })); - const groupedNotifications: Record[]> = - updatedNotifications.reduce((acc, n) => { - if (!acc[n.group]) { - acc[n.group] = []; - } - acc[n.group].push(n); - return acc; - }, {}); - - const columns: { name: string; data: PBACDefault[] }[][] = [[], [], []]; - Object.entries(groupedNotifications).forEach(([name, notifications], index) => { - const placement = index > 1 ? 2 : index; - columns[placement].push({ name, data: notifications }); - }); - - return columns; + return columnizePBACGroups(remappedNotifications, 3); }, [data, notificationsValue]); const handlePermissionChange = (_id: AuthPermissions) => { diff --git a/src/types/PBAC.d.ts b/src/types/PBAC.d.ts index b12fcf8b3..98640a2cf 100644 --- a/src/types/PBAC.d.ts +++ b/src/types/PBAC.d.ts @@ -58,7 +58,7 @@ type AuthNotifications = * * e.g. Permission or Notification */ -type PBACDefault = { +type PBACDefault = { /** * The unique identifier of the PBAC object. * diff --git a/src/utils/profileUtils.test.ts b/src/utils/profileUtils.test.ts index 8958c3050..0b83f2f77 100644 --- a/src/utils/profileUtils.test.ts +++ b/src/utils/profileUtils.test.ts @@ -259,3 +259,127 @@ describe("userToCollaborator cases", () => { expect(mockFormatName).toHaveBeenCalledWith("John", "Doe"); }); }); + +describe("columnizePBACGroups cases", () => { + const baseDefault: PBACDefault = { + _id: "access:request", // This is not actually used by the util + name: "", + group: "", + checked: false, + disabled: false, + }; + + it("should return empty array for invalid input", () => { + expect(utils.columnizePBACGroups([])).toEqual([]); + expect(utils.columnizePBACGroups(null)).toEqual([]); + expect(utils.columnizePBACGroups(undefined)).toEqual([]); + }); + + it("should group PBACDefaults into columns using the default colCount", () => { + const pbacDefaults: PBACDefault[] = [ + { ...baseDefault, name: "1", group: "A" }, + { ...baseDefault, name: "2", group: "A" }, + { ...baseDefault, name: "3", group: "B" }, + { ...baseDefault, name: "4", group: "B" }, + { ...baseDefault, name: "5", group: "C" }, + { ...baseDefault, name: "6", group: "C" }, + ]; + + const columnized = utils.columnizePBACGroups(pbacDefaults); + + expect(columnized).toHaveLength(3); + expect(columnized[0]).toHaveLength(1); + expect(columnized[1]).toHaveLength(1); + expect(columnized[2]).toHaveLength(1); + + expect(columnized[0][0].data).toEqual([ + { ...baseDefault, name: "1", group: "A" }, + { ...baseDefault, name: "2", group: "A" }, + ]); + + expect(columnized[1][0].data).toEqual([ + { ...baseDefault, name: "3", group: "B" }, + { ...baseDefault, name: "4", group: "B" }, + ]); + + expect(columnized[2][0].data).toEqual([ + { ...baseDefault, name: "5", group: "C" }, + { ...baseDefault, name: "6", group: "C" }, + ]); + }); + + it("should group PBACDefaults into columns using a custom colCount", () => { + const pbacDefaults: PBACDefault[] = [ + { ...baseDefault, name: "1", group: "A" }, + { ...baseDefault, name: "2", group: "B" }, + { ...baseDefault, name: "3", group: "C" }, + { ...baseDefault, name: "4", group: "D" }, + { ...baseDefault, name: "5", group: "E" }, + { ...baseDefault, name: "6", group: "F" }, + ]; + + const columnized = utils.columnizePBACGroups(pbacDefaults, 2); + + expect(columnized).toHaveLength(2); + expect(columnized[0]).toHaveLength(1); + expect(columnized[1]).toHaveLength(5); + }); + + it("should handle a higher colCount than the number of groups", () => { + const pbacDefaults: PBACDefault[] = [ + { ...baseDefault, name: "1", group: "A" }, + { ...baseDefault, name: "2", group: "B" }, + { ...baseDefault, name: "3", group: "C" }, + ]; + + const columnized = utils.columnizePBACGroups(pbacDefaults, 10); + + expect(columnized).toHaveLength(3); + expect(columnized[0]).toHaveLength(1); + expect(columnized[1]).toHaveLength(1); + expect(columnized[2]).toHaveLength(1); + }); + + it("should handle PBACDefaults with no group", () => { + const pbacDefaults: PBACDefault[] = [ + { ...baseDefault, name: "1", group: "A" }, + { ...baseDefault, name: "2", group: "B" }, + { ...baseDefault, name: "3", group: "" }, + ]; + + const columnized = utils.columnizePBACGroups(pbacDefaults); + + expect(columnized).toHaveLength(3); + expect(columnized[0]).toHaveLength(1); + expect(columnized[1]).toHaveLength(1); + expect(columnized[2]).toHaveLength(1); + + expect(columnized[2][0].data).toEqual([{ ...baseDefault, name: "3", group: "" }]); + }); + + it("should fallback to an empty group name if the PBACDefault has an invalid group name", () => { + const pbacDefaults: PBACDefault[] = [ + { ...baseDefault, name: "1", group: "valid" }, + { ...baseDefault, name: "2", group: undefined }, + { ...baseDefault, name: "3", group: null }, + { ...baseDefault, name: "4", group: 3 as unknown as string }, + { ...baseDefault, name: "5", group: { Obj: "yes" } as unknown as string }, + ]; + + const columnized = utils.columnizePBACGroups(pbacDefaults, 10); // Set to 10 to ensure all groups COULD go to their own column + + expect(columnized).toHaveLength(2); + expect(columnized[0]).toHaveLength(1); + expect(columnized[1]).toHaveLength(1); // 1 Group for all invalid + + expect(columnized[0][0].data).toEqual([{ ...baseDefault, name: "1", group: "valid" }]); + expect(columnized[1][0].name).toBe(""); + expect(columnized[1][0].data).toHaveLength(4); // All invalid groups are together + expect(columnized[1][0].data).toEqual([ + pbacDefaults[1], + pbacDefaults[2], + pbacDefaults[3], + pbacDefaults[4], + ]); + }); +}); diff --git a/src/utils/profileUtils.ts b/src/utils/profileUtils.ts index e4ffb981b..eef72cf26 100644 --- a/src/utils/profileUtils.ts +++ b/src/utils/profileUtils.ts @@ -40,3 +40,64 @@ export const userToCollaborator = ( orgName: user?.organization?.orgName, }, }); + +/** + * The data structure for a columized PBAC group + */ +export type ColumnizedPBACGroups = { + /** + * The name of the group of PBACDefaults + * + * All PBACDefaults in the group will have the same group name + */ + name: string; + /** + * The PBACDefaults for the group + */ + data: PBACDefault[]; +}[][]; + +/** + * A utility function to group an array of PBACDefaults into columns + * based on the group name. + * + * If colCount is greater than the number of groups, the last column will + * aggregate the remaining groups. + * + * Data Structure: Array of Columns -> Array of Groups -> PBAC Defaults for the group + * + * @see {@link ColumnizedPBACGroups} + * @param data The array of PBACDefaults to columnize + * @param colCount The number of columns to create + * @returns An array of columns containing an array of PBACDefaults + */ +export const columnizePBACGroups = ( + data: PBACDefault[], + colCount = 3 +): ColumnizedPBACGroups => { + if (!data || !Array.isArray(data) || data.length === 0) { + return []; + } + + const groupedData: Record[]> = {}; + data.forEach((item) => { + const groupName = typeof item?.group === "string" ? item.group : ""; + if (!groupedData[groupName]) { + groupedData[groupName] = []; + } + + groupedData[groupName].push(item); + }); + + const columns: ColumnizedPBACGroups = []; + Object.entries(groupedData).forEach(([group, data], index) => { + const groupIndex = index > colCount - 1 ? colCount - 1 : index; + if (!columns[groupIndex]) { + columns[groupIndex] = []; + } + + columns[groupIndex].push({ name: group, data }); + }); + + return columns; +}; From 69abfde39f7c145775b1d5caf1e8055cb0952e65 Mon Sep 17 00:00:00 2001 From: Alec M Date: Mon, 30 Dec 2024 11:35:41 -0500 Subject: [PATCH 16/21] feat: Support partial sorting of PBAC groups --- src/utils/profileUtils.test.ts | 28 +++++++++++++++++++++++ src/utils/profileUtils.ts | 41 +++++++++++++++++++++++++++++++++- 2 files changed, 68 insertions(+), 1 deletion(-) diff --git a/src/utils/profileUtils.test.ts b/src/utils/profileUtils.test.ts index 0b83f2f77..883a02843 100644 --- a/src/utils/profileUtils.test.ts +++ b/src/utils/profileUtils.test.ts @@ -382,4 +382,32 @@ describe("columnizePBACGroups cases", () => { pbacDefaults[4], ]); }); + + it("should sort the groups in the order: Submission Request, Data Submission, Admin, Miscellaneous", () => { + const pbacDefaults: PBACDefault[] = [ + { ...baseDefault, name: "6", group: "Random Group 1" }, // 5 + { ...baseDefault, name: "1", group: "Data Submission" }, // 2 + { ...baseDefault, name: "3", group: "Miscellaneous" }, // 4 + { ...baseDefault, name: "2", group: "Admin" }, // 3 + { ...baseDefault, name: "4", group: "Submission Request" }, // 1 + ]; + + const columnized = utils.columnizePBACGroups(pbacDefaults, 4); + + expect(columnized).toHaveLength(4); + expect(columnized[0]).toHaveLength(1); + expect(columnized[1]).toHaveLength(1); + expect(columnized[2]).toHaveLength(1); + expect(columnized[3]).toHaveLength(2); + + expect(columnized[0][0].data).toEqual([ + { ...baseDefault, name: "4", group: "Submission Request" }, + ]); + expect(columnized[1][0].data).toEqual([ + { ...baseDefault, name: "1", group: "Data Submission" }, + ]); + expect(columnized[2][0].data).toEqual([{ ...baseDefault, name: "2", group: "Admin" }]); + expect(columnized[3][0].data).toEqual([{ ...baseDefault, name: "3", group: "Miscellaneous" }]); + expect(columnized[3][1].data).toEqual([{ ...baseDefault, name: "6", group: "Random Group 1" }]); + }); }); diff --git a/src/utils/profileUtils.ts b/src/utils/profileUtils.ts index eef72cf26..859a29b66 100644 --- a/src/utils/profileUtils.ts +++ b/src/utils/profileUtils.ts @@ -89,8 +89,11 @@ export const columnizePBACGroups = ( groupedData[groupName].push(item); }); + const sortedGroups = Object.entries(groupedData); + sortedGroups.sort(([a], [b]) => orderPBACGroups(a, b)); + const columns: ColumnizedPBACGroups = []; - Object.entries(groupedData).forEach(([group, data], index) => { + sortedGroups.forEach(([group, data], index) => { const groupIndex = index > colCount - 1 ? colCount - 1 : index; if (!columns[groupIndex]) { columns[groupIndex] = []; @@ -101,3 +104,39 @@ export const columnizePBACGroups = ( return columns; }; + +/** + * A utility function to order PBAC Groups by their partial group name in the following order: + * + * 1. Submission Request + * 2. Data Submission + * 3. Admin + * 4. Miscellaneous + * 5. All other groups + * + * If the name does not contain any of the above groups, it will be pushed to the end, + * but will not be sorted against other unlisted groups. + * + * @param groups The groups to order + * @returns The ordered groups in the format of Record + */ +export const orderPBACGroups = (a: string, b: string): number => { + const SORT_PRIORITY = ["Submission Request", "Data Submission", "Admin", "Miscellaneous"]; + + const aIndex = SORT_PRIORITY.findIndex((group) => a.includes(group)); + const bIndex = SORT_PRIORITY.findIndex((group) => b.includes(group)); + + if (aIndex === -1 && bIndex === -1) { + return 0; + } + + if (aIndex === -1) { + return 1; + } + + if (bIndex === -1) { + return -1; + } + + return aIndex - bIndex; +}; From 8febde5da3a3ed3e7d3bf7963e27a98464dce315 Mon Sep 17 00:00:00 2001 From: Alec M Date: Mon, 30 Dec 2024 11:47:56 -0500 Subject: [PATCH 17/21] Standardize ProfileView buttons --- src/content/users/ProfileView.tsx | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/src/content/users/ProfileView.tsx b/src/content/users/ProfileView.tsx index 0b217dc01..781c87084 100644 --- a/src/content/users/ProfileView.tsx +++ b/src/content/users/ProfileView.tsx @@ -125,17 +125,12 @@ const StyledButtonStack = styled(Stack)({ marginTop: "50px", }); -const StyledButton = styled(LoadingButton)(({ txt, border }: { txt: string; border: string }) => ({ - borderRadius: "8px", - border: `2px solid ${border}`, - color: `${txt} !important`, - width: "101px", - height: "51px", - textTransform: "none", - fontWeight: 700, - fontSize: "17px", - padding: "6px 8px", -})); +const StyledButton = styled(LoadingButton)({ + minWidth: "120px", + fontSize: "16px", + padding: "10px", + lineHeight: "24px", +}); const StyledContentStack = styled(Stack)({ marginLeft: "2px !important", @@ -587,7 +582,12 @@ const ProfileView: FC = ({ _id, viewType }: Props) => { spacing={1} > {Object.values(fieldset).some((fieldState) => fieldState === "UNLOCKED") && ( - + Save )} @@ -595,8 +595,8 @@ const ProfileView: FC = ({ _id, viewType }: Props) => { navigate(manageUsersPageUrl)} - txt="#666666" - border="#828282" + variant="contained" + color="info" > Cancel From fc4b98ab14f97a958c05f03ccd2440e5243867e3 Mon Sep 17 00:00:00 2001 From: Alec M Date: Mon, 30 Dec 2024 12:03:15 -0500 Subject: [PATCH 18/21] fix: Center PBAC Panel and adjust margins --- src/components/PermissionPanel/index.tsx | 14 +++++++++++--- src/content/users/ProfileView.tsx | 8 ++++++-- 2 files changed, 17 insertions(+), 5 deletions(-) diff --git a/src/components/PermissionPanel/index.tsx b/src/components/PermissionPanel/index.tsx index 1caf1a984..f2c481353 100644 --- a/src/components/PermissionPanel/index.tsx +++ b/src/components/PermissionPanel/index.tsx @@ -4,6 +4,7 @@ import { Accordion, AccordionDetails, AccordionSummary, + Box, Checkbox, FormControlLabel, FormGroup, @@ -23,8 +24,15 @@ import { } from "../../graphql"; import { ColumnizedPBACGroups, columnizePBACGroups, Logger } from "../../utils"; +const StyledBox = styled(Box)({ + width: "957px", + transform: "translateX(-50%)", + marginLeft: "50%", + marginTop: "63px", +}); + const StyledAccordion = styled(Accordion)({ - width: "957px", // TODO: Need to fix the page layout + width: "100%", }); const StyledAccordionSummary = styled(AccordionSummary)({ @@ -187,7 +195,7 @@ const PermissionPanel: FC = () => { }, [selectedRole]); return ( - <> + }> Permissions @@ -258,7 +266,7 @@ const PermissionPanel: FC = () => { - + ); }; diff --git a/src/content/users/ProfileView.tsx b/src/content/users/ProfileView.tsx index 781c87084..7dafafb2d 100644 --- a/src/content/users/ProfileView.tsx +++ b/src/content/users/ProfileView.tsx @@ -95,6 +95,10 @@ const StyledHeaderText = styled(Typography)({ fontWeight: 700, }); +const StyledForm = styled("form")({ + width: "532px", +}); + const StyledField = styled("div", { shouldForwardProp: (p) => p !== "visible" })<{ visible?: boolean; }>(({ visible = true }) => ({ @@ -407,7 +411,7 @@ const ProfileView: FC = ({ _id, viewType }: Props) => { {user.email} -
+ Account Type {formatIDP(user.IDP)} @@ -602,7 +606,7 @@ const ProfileView: FC = ({ _id, viewType }: Props) => { )} - +
From bbe6497e58816efd47beab77d8749336a11ec7e1 Mon Sep 17 00:00:00 2001 From: Alec M Date: Mon, 30 Dec 2024 12:50:27 -0500 Subject: [PATCH 19/21] Remove redundant test --- src/components/PermissionPanel/index.test.tsx | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/components/PermissionPanel/index.test.tsx b/src/components/PermissionPanel/index.test.tsx index 2e4085f23..6c41f7de8 100644 --- a/src/components/PermissionPanel/index.test.tsx +++ b/src/components/PermissionPanel/index.test.tsx @@ -986,10 +986,6 @@ describe("Implementation Requirements", () => { ); }); - it.todo( - "should sort the permission groups in the following order: Submission Request, Data Submission, Admin, Miscellaneous" - ); - it("should propagate the permission and notification selections to the parent form", async () => { const mock: MockedResponse = { request: { From 99a161ff19572b35a4e9a8e342008854fcf41bf3 Mon Sep 17 00:00:00 2001 From: Alec M Date: Tue, 31 Dec 2024 14:14:22 -0500 Subject: [PATCH 20/21] Update incorrect JSDoc --- src/utils/profileUtils.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/utils/profileUtils.ts b/src/utils/profileUtils.ts index 859a29b66..831ed92f2 100644 --- a/src/utils/profileUtils.ts +++ b/src/utils/profileUtils.ts @@ -61,8 +61,8 @@ export type ColumnizedPBACGroups = { * A utility function to group an array of PBACDefaults into columns * based on the group name. * - * If colCount is greater than the number of groups, the last column will - * aggregate the remaining groups. + * If the number of unique groups exceeds `colCount`, the function will + * aggregate the remaining groups into the last column. * * Data Structure: Array of Columns -> Array of Groups -> PBAC Defaults for the group * From 4553245ec4ad948066af5dd3c55d535161504882 Mon Sep 17 00:00:00 2001 From: Alec M Date: Tue, 31 Dec 2024 14:28:12 -0500 Subject: [PATCH 21/21] fix: Change expand icon --- src/components/PermissionPanel/index.tsx | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/components/PermissionPanel/index.tsx b/src/components/PermissionPanel/index.tsx index f2c481353..97b5c408b 100644 --- a/src/components/PermissionPanel/index.tsx +++ b/src/components/PermissionPanel/index.tsx @@ -1,5 +1,5 @@ import { useQuery } from "@apollo/client"; -import { ArrowDropDown } from "@mui/icons-material"; +import ExpandMoreIcon from "@mui/icons-material/ExpandMore"; import { Accordion, AccordionDetails, @@ -10,10 +10,10 @@ import { FormGroup, styled, Typography, + Unstable_Grid2 as Grid2, } from "@mui/material"; import { FC, memo, useEffect, useMemo, useRef } from "react"; import { useFormContext } from "react-hook-form"; -import Grid2 from "@mui/material/Unstable_Grid2"; import { useSnackbar } from "notistack"; import { cloneDeep } from "lodash"; import { @@ -44,7 +44,7 @@ const StyledAccordionSummary = styled(AccordionSummary)({ "& .MuiAccordionSummary-content.Mui-expanded": { margin: "9px 0", }, - "& .MuiAccordionSummary-expandIcon": { + "& .MuiAccordionSummary-expandIconWrapper": { color: "#356AAD", }, }); @@ -197,7 +197,7 @@ const PermissionPanel: FC = () => { return ( - }> + }> Permissions @@ -232,7 +232,7 @@ const PermissionPanel: FC = () => { - }> + }> Email Notifications