From 0685b88a9a1dc27505faee5753224aea06c150c4 Mon Sep 17 00:00:00 2001 From: Alec M Date: Wed, 27 Nov 2024 15:57:13 -0500 Subject: [PATCH 1/8] init: Removal of Organization assignment --- src/components/APITokenDialog/index.test.tsx | 1 - src/components/Contexts/AuthContext.tsx | 8 +- .../CreateDataSubmissionDialog.test.tsx | 81 +---------------- .../CreateDataSubmissionDialog.tsx | 48 ++-------- .../CrossValidationButton.test.tsx | 1 - .../DataSubmissionSummary.test.tsx | 1 - .../DataSubmissions/DataUpload.test.tsx | 1 - .../DeleteNodeDataButton.test.tsx | 1 - .../DataSubmissions/MetadataUpload.test.tsx | 1 - .../ValidationControls.test.tsx | 1 - src/config/AuthRoles.ts | 19 ---- .../OperationDashboard/Controller.test.tsx | 1 - .../OperationDashboard/DashboardView.test.tsx | 1 - .../OperationDashboard/DashboardView.tsx | 5 +- .../dataSubmissions/SubmittedData.test.tsx | 1 - src/content/questionnaire/ListView.test.tsx | 1 - src/content/studies/Controller.test.tsx | 1 - src/content/users/Controller.tsx | 23 +---- src/content/users/ListView.tsx | 90 ++----------------- src/content/users/ProfileView.tsx | 82 ++--------------- src/graphql/editUser.ts | 21 +++-- src/graphql/getMyUser.ts | 8 +- src/graphql/getUser.ts | 7 +- src/graphql/listUsers.ts | 9 +- src/graphql/updateMyUser.ts | 9 +- src/hooks/useProfileFields.test.ts | 54 ----------- src/hooks/useProfileFields.ts | 16 +--- src/types/Auth.d.ts | 8 +- src/utils/formModeUtils.test.ts | 11 +-- 29 files changed, 55 insertions(+), 456 deletions(-) diff --git a/src/components/APITokenDialog/index.test.tsx b/src/components/APITokenDialog/index.test.tsx index 910bd9f43..785fe9483 100644 --- a/src/components/APITokenDialog/index.test.tsx +++ b/src/components/APITokenDialog/index.test.tsx @@ -34,7 +34,6 @@ const baseUser: User = { role: null, IDP: "nih", email: "", - organization: null, studies: null, dataCommons: [], createdAt: "", diff --git a/src/components/Contexts/AuthContext.tsx b/src/components/Contexts/AuthContext.tsx index 0f95b4c7e..386798ea6 100644 --- a/src/components/Contexts/AuthContext.tsx +++ b/src/components/Contexts/AuthContext.tsx @@ -140,13 +140,7 @@ export const AuthProvider: FC = ({ children }: ProviderProps) => const setData = (data: Partial): void => { if (!state.isLoggedIn) return; - // Remove any nested objects that are null - const newUser = { ...state.user, ...data }; - if (!data?.organization) { - delete newUser.organization; - } - - setState((prev) => ({ ...prev, user: newUser })); + setState((prev) => ({ ...prev, user: { ...state.user, ...data } })); }; useEffect(() => { diff --git a/src/components/DataSubmissions/CreateDataSubmissionDialog.test.tsx b/src/components/DataSubmissions/CreateDataSubmissionDialog.test.tsx index d1d2b46db..f583d3245 100644 --- a/src/components/DataSubmissions/CreateDataSubmissionDialog.test.tsx +++ b/src/components/DataSubmissions/CreateDataSubmissionDialog.test.tsx @@ -1,6 +1,6 @@ import { FC } from "react"; import { MemoryRouter } from "react-router-dom"; -import { fireEvent, render, waitFor, within } from "@testing-library/react"; +import { render, waitFor, within } from "@testing-library/react"; import userEvent from "@testing-library/user-event"; import { MockedProvider, MockedResponse } from "@apollo/client/testing"; import { GraphQLError } from "graphql"; @@ -120,13 +120,6 @@ const baseUser: Omit = { userStatus: "Active", IDP: "nih", email: "", - organization: { - orgID: "some-org-1", - orgName: "org1", - status: "Active", - createdAt: "2023-10-06T19:19:04.183Z", - updateAt: "2024-07-03T19:09:29.513Z", - }, studies: null, dataCommons: [], createdAt: "", @@ -188,7 +181,6 @@ describe("Basic Functionality", () => { ).toBeInTheDocument(); expect(getByText("Submission Type")).toBeInTheDocument(); expect(getByText("Data Type")).toBeInTheDocument(); - expect(getByText("Organization")).toBeInTheDocument(); expect(getByText("Data Commons")).toBeInTheDocument(); expect(getByText("Study")).toBeInTheDocument(); expect(getByText("dbGaP ID")).toBeInTheDocument(); @@ -254,77 +246,6 @@ describe("Basic Functionality", () => { expect(handleCreate).toHaveBeenCalledTimes(1); }); - it("should disable open dialog button and show tooltip if user has 'Inactive' org", async () => { - const { getByTestId, getByRole } = render( - - - - ); - - const openDialogButton = getByRole("button", { name: "Create a Data Submission" }); - expect(openDialogButton).toBeInTheDocument(); - expect(openDialogButton).toBeDisabled(); - - userEvent.click(openDialogButton); - - await waitFor(() => { - expect(() => getByTestId("create-submission-dialog")).toThrow(); - }); - - fireEvent.mouseOver(openDialogButton); - - await waitFor(() => { - const tooltip = getByRole("tooltip"); - expect(tooltip).toBeInTheDocument(); - expect(tooltip).toHaveTextContent( - "Your associated organization is inactive. You cannot create a data submission at this time." - ); - }); - }); - - it("should not disable open dialog button or show tooltip if user has 'Active' org", async () => { - const { getByTestId, getByRole } = render( - - - - ); - - const openDialogButton = getByRole("button", { name: "Create a Data Submission" }); - expect(openDialogButton).toBeInTheDocument(); - - await waitFor(() => expect(openDialogButton).toBeEnabled()); - - fireEvent.mouseOver(openDialogButton); - - await waitFor(() => { - expect(() => getByRole("tooltip")).toThrow(); - }); - - userEvent.click(openDialogButton); - - await waitFor(() => { - expect(getByTestId("create-submission-dialog")).toBeInTheDocument(); - }); - }); - it("should only show the dbGaP ID if study is controlled access", async () => { const { getByText, getByRole, getByTestId } = render( diff --git a/src/components/DataSubmissions/CreateDataSubmissionDialog.tsx b/src/components/DataSubmissions/CreateDataSubmissionDialog.tsx index d9e204d00..6e0a739c3 100644 --- a/src/components/DataSubmissions/CreateDataSubmissionDialog.tsx +++ b/src/components/DataSubmissions/CreateDataSubmissionDialog.tsx @@ -1,4 +1,4 @@ -import { FC, useEffect, useMemo, useState } from "react"; +import { FC, useEffect, useState } from "react"; import { Button, Dialog, @@ -170,10 +170,6 @@ const StyledField = styled("div")({ position: "relative", }); -const StyledOrganizationField = styled(StyledField)({ - marginBottom: "25px", -}); - const StyledHelperText = styled(BaseStyledHelperText)({ marginTop: "5px", }); @@ -241,13 +237,7 @@ const CreateDataSubmissionDialog: FC = ({ onCreate }) => { }); const orgOwnerOrSubmitter = user?.role === "Organization Owner" || user?.role === "Submitter"; - const hasOrganizationAssigned = user?.organization !== null && user?.organization?.orgID !== null; const intention = watch("intention"); - const userHasInactiveOrg = useMemo( - () => user?.organization?.status === "Inactive", - [user?.organization?.status] - ); - const submissionTypeOptions: RadioOption[] = [ { label: "New/Update", @@ -415,14 +405,6 @@ const CreateDataSubmissionDialog: FC = ({ onCreate }) => { {errors?.intention?.message} - - Organization - - Data Commons @@ -572,28 +554,14 @@ const CreateDataSubmissionDialog: FC = ({ onCreate }) => { {orgOwnerOrSubmitter && ( - - - - Create a Data Submission - - - + Create a Data Submission + )} diff --git a/src/components/DataSubmissions/CrossValidationButton.test.tsx b/src/components/DataSubmissions/CrossValidationButton.test.tsx index c7d83cdbd..a615186e1 100644 --- a/src/components/DataSubmissions/CrossValidationButton.test.tsx +++ b/src/components/DataSubmissions/CrossValidationButton.test.tsx @@ -80,7 +80,6 @@ const baseUser: Omit = { userStatus: "Active", IDP: "nih", email: "", - organization: null, studies: null, dataCommons: [], createdAt: "", diff --git a/src/components/DataSubmissions/DataSubmissionSummary.test.tsx b/src/components/DataSubmissions/DataSubmissionSummary.test.tsx index d36f072d3..6f7aea26a 100644 --- a/src/components/DataSubmissions/DataSubmissionSummary.test.tsx +++ b/src/components/DataSubmissions/DataSubmissionSummary.test.tsx @@ -32,7 +32,6 @@ const baseUser: User = { role: null, IDP: "nih", email: "", - organization: null, studies: null, dataCommons: [], createdAt: "", diff --git a/src/components/DataSubmissions/DataUpload.test.tsx b/src/components/DataSubmissions/DataUpload.test.tsx index 437701061..b7f7efa82 100644 --- a/src/components/DataSubmissions/DataUpload.test.tsx +++ b/src/components/DataSubmissions/DataUpload.test.tsx @@ -67,7 +67,6 @@ const baseUser: User = { role: "Submitter", // NOTE: This base role allows for all actions IDP: "nih", email: "", - organization: null, studies: null, dataCommons: null, createdAt: "", diff --git a/src/components/DataSubmissions/DeleteNodeDataButton.test.tsx b/src/components/DataSubmissions/DeleteNodeDataButton.test.tsx index 9e9904d34..c36915c06 100644 --- a/src/components/DataSubmissions/DeleteNodeDataButton.test.tsx +++ b/src/components/DataSubmissions/DeleteNodeDataButton.test.tsx @@ -67,7 +67,6 @@ const baseUser: User = { role: "Submitter", // NOTE: This role has access to the delete button by default IDP: "nih", email: "", - organization: null, studies: null, dataCommons: [], createdAt: "", diff --git a/src/components/DataSubmissions/MetadataUpload.test.tsx b/src/components/DataSubmissions/MetadataUpload.test.tsx index 861416210..635dd3b52 100644 --- a/src/components/DataSubmissions/MetadataUpload.test.tsx +++ b/src/components/DataSubmissions/MetadataUpload.test.tsx @@ -57,7 +57,6 @@ const baseUser: Omit = { userStatus: "Active", IDP: "nih", email: "", - organization: null, studies: null, dataCommons: [], createdAt: "", diff --git a/src/components/DataSubmissions/ValidationControls.test.tsx b/src/components/DataSubmissions/ValidationControls.test.tsx index 04cdeb589..cc1b7c9c1 100644 --- a/src/components/DataSubmissions/ValidationControls.test.tsx +++ b/src/components/DataSubmissions/ValidationControls.test.tsx @@ -80,7 +80,6 @@ const baseUser: Omit = { userStatus: "Active", IDP: "nih", email: "", - organization: null, studies: null, dataCommons: [], createdAt: "", diff --git a/src/config/AuthRoles.ts b/src/config/AuthRoles.ts index 58628805d..62f3c5e3b 100644 --- a/src/config/AuthRoles.ts +++ b/src/config/AuthRoles.ts @@ -14,25 +14,6 @@ export const Roles: UserRole[] = [ "Admin", ]; -/** - * Defines a list of roles that are required to have an - * organization assigned to them. This unlocks the Organization assignment dropdown. - */ -export const OrgRequiredRoles: UserRole[] = ["Submitter", "Organization Owner", "Data Commons POC"]; - -/** - * A map of the roles that are required to be pre-assigned - * to a specific organization in the database. This locks the Organization assignment dropdown. - * - * @note This requires that an organization with the specified name exists in the database. - */ -type RoleSubset = Extends; -export const OrgAssignmentMap: Record = { - Admin: "FNL", - "Federal Lead": "NCI", - "Federal Monitor": "NCI", -}; - /** * Defines a list of roles that are allowed to interact with regular Validation. */ diff --git a/src/content/OperationDashboard/Controller.test.tsx b/src/content/OperationDashboard/Controller.test.tsx index a9cc379d7..c95d6259c 100644 --- a/src/content/OperationDashboard/Controller.test.tsx +++ b/src/content/OperationDashboard/Controller.test.tsx @@ -26,7 +26,6 @@ const baseUser: Omit = { userStatus: "Active", IDP: "nih", email: "", - organization: null, dataCommons: [], createdAt: "", updateAt: "", diff --git a/src/content/OperationDashboard/DashboardView.test.tsx b/src/content/OperationDashboard/DashboardView.test.tsx index 04018a1a6..fc2197b39 100644 --- a/src/content/OperationDashboard/DashboardView.test.tsx +++ b/src/content/OperationDashboard/DashboardView.test.tsx @@ -16,7 +16,6 @@ const baseUser: Omit = { userStatus: "Active", IDP: "nih", email: "", - organization: null, dataCommons: [], createdAt: "", updateAt: "", diff --git a/src/content/OperationDashboard/DashboardView.tsx b/src/content/OperationDashboard/DashboardView.tsx index 6ec1174f3..5cbfde313 100644 --- a/src/content/OperationDashboard/DashboardView.tsx +++ b/src/content/OperationDashboard/DashboardView.tsx @@ -91,7 +91,10 @@ const DashboardView: FC = ({ const params: DashboardContentOptions["parameters"] = []; if (role === "Federal Monitor" && Array.isArray(studies) && studies.length > 0) { - params.push({ Name: "studiesParameter", Values: studies }); + params.push({ + Name: "studiesParameter", + Values: studies?.map((study: ApprovedStudy) => study?._id), + }); } else if (role === "Federal Monitor") { Logger.error("This role requires studies to be set but none were found.", studies); params.push({ Name: "studiesParameter", Values: ["NO-CONTENT"] }); diff --git a/src/content/dataSubmissions/SubmittedData.test.tsx b/src/content/dataSubmissions/SubmittedData.test.tsx index 006840af4..d6e815c9d 100644 --- a/src/content/dataSubmissions/SubmittedData.test.tsx +++ b/src/content/dataSubmissions/SubmittedData.test.tsx @@ -34,7 +34,6 @@ const baseUser: User = { role: "Submitter", // NOTE: This role has access to everything nested here by default IDP: "nih", email: "", - organization: null, studies: null, dataCommons: [], createdAt: "", diff --git a/src/content/questionnaire/ListView.test.tsx b/src/content/questionnaire/ListView.test.tsx index 85ab25894..8913f2301 100644 --- a/src/content/questionnaire/ListView.test.tsx +++ b/src/content/questionnaire/ListView.test.tsx @@ -40,7 +40,6 @@ const baseUser: Omit = { userStatus: "Active", IDP: "nih", email: "", - organization: null, dataCommons: [], createdAt: "", updateAt: "", diff --git a/src/content/studies/Controller.test.tsx b/src/content/studies/Controller.test.tsx index 91b50555c..dd09182f6 100644 --- a/src/content/studies/Controller.test.tsx +++ b/src/content/studies/Controller.test.tsx @@ -26,7 +26,6 @@ const baseUser: Omit = { userStatus: "Active", IDP: "nih", email: "", - organization: null, dataCommons: [], createdAt: "", updateAt: "", diff --git a/src/content/users/Controller.tsx b/src/content/users/Controller.tsx index 82b45cc61..9ef0fd803 100644 --- a/src/content/users/Controller.tsx +++ b/src/content/users/Controller.tsx @@ -1,7 +1,6 @@ -import React, { memo } from "react"; +import React from "react"; import { Navigate, useParams } from "react-router-dom"; import { useAuthContext } from "../../components/Contexts/AuthContext"; -import { OrganizationProvider } from "../../components/Contexts/OrganizationListContext"; import ListView from "./ListView"; import ProfileView from "./ProfileView"; import { CanManageUsers } from "../../config/AuthRoles"; @@ -10,14 +9,6 @@ type Props = { type: "users" | "profile"; }; -/** - * A memoized version of OrganizationProvider - * which caches data between ListView and ProfileView - * - * @see OrganizationProvider - */ -const MemorizedProvider = memo(OrganizationProvider); - /** * Renders the correct view based on the URL and permissions-tier * @@ -54,19 +45,11 @@ const UserController = ({ type }: Props) => { // Show list of users to Admin or Org Owner if (!userId && isAdministrative) { - return ( - - - - ); + return ; } // Admin or Org Owner viewing a user's "Edit User" page or their own "Edit User" page - return ( - - - - ); + return ; }; export default UserController; diff --git a/src/content/users/ListView.tsx b/src/content/users/ListView.tsx index 126079b92..f7e527824 100644 --- a/src/content/users/ListView.tsx +++ b/src/content/users/ListView.tsx @@ -15,10 +15,6 @@ import { } from "@mui/material"; import { Link, useLocation } from "react-router-dom"; import { Controller, useForm } from "react-hook-form"; -import { - Status as OrgStatus, - useOrganizationListContext, -} from "../../components/Contexts/OrganizationListContext"; import PageBanner from "../../components/PageBanner"; import { Roles } from "../../config/AuthRoles"; import { LIST_USERS, ListUsersResp } from "../../graphql"; @@ -28,13 +24,9 @@ import usePageTitle from "../../hooks/usePageTitle"; import GenericTable, { Column } from "../../components/GenericTable"; import { useSearchParamsContext } from "../../components/Contexts/SearchParamsContext"; -type T = User; +type T = ListUsersResp["listUsers"][number]; type FilterForm = { - /** - * @see Organization["_id"] | "All" - */ - organization: string; role: User["role"] | "All"; status: User["userStatus"] | "All"; }; @@ -142,7 +134,6 @@ const StyledActionButton = styled(Button)( type TouchedState = { [K in keyof FilterForm]: boolean }; const initialTouchedFields: TouchedState = { - organization: false, role: false, status: false, }; @@ -178,15 +169,6 @@ const columns: Column[] = [ comparator: (a, b) => a.email.localeCompare(b.email), field: "email", }, - { - label: "Organization", - renderValue: (a) => a.organization?.orgName || "", - comparator: (a, b) => compareStrings(a?.organization?.orgName, b?.organization?.orgName), - sx: { - width: "11%", - }, - fieldKey: "orgName", - }, { label: "Status", renderValue: (a) => a.userStatus, @@ -233,9 +215,8 @@ const columns: Column[] = [ const ListingView: FC = () => { usePageTitle("Manage Users"); - const { user, status: authStatus } = useAuthContext(); + const { status: authStatus } = useAuthContext(); const { state } = useLocation(); - const { data: orgData, activeOrganizations, status: orgStatus } = useOrganizationListContext(); const { searchParams, setSearchParams } = useSearchParamsContext(); const [dataset, setDataset] = useState([]); const [count, setCount] = useState(0); @@ -243,12 +224,11 @@ const ListingView: FC = () => { const { watch, setValue, control } = useForm({ defaultValues: { - organization: "All", role: "All", status: "All", }, }); - const orgFilter = watch("organization"); + const roleFilter = watch("role"); const statusFilter = watch("status"); const tableRef = useRef(null); @@ -269,7 +249,6 @@ const ListingView: FC = () => { } const filters: FilterFunction[] = [ - (u: T) => (orgFilter && orgFilter !== "All" ? u.organization?.orgID === orgFilter : true), (u: T) => (roleFilter && roleFilter !== "All" ? u.role === roleFilter : true), (u: T) => (statusFilter && statusFilter !== "All" ? u.userStatus === statusFilter : true), ]; @@ -282,33 +261,15 @@ const ListingView: FC = () => { setDataset(paginatedData); }; - useEffect(() => { - if (user?.role !== "Organization Owner") { - return; - } - - const orgID = orgData?.find((org: Organization) => org._id === user.organization?.orgID)?._id; - setValue("organization", orgID || "All"); - }, [user, orgData]); - - const isValidOrg = (orgId: string) => !!activeOrganizations?.find((org) => org._id === orgId); const isRoleFilterOption = (role: string): role is FilterForm["role"] => ["All", ...Roles].includes(role); const isStatusFilterOption = (status: string): status is FilterForm["status"] => ["All", "Inactive", "Active"].includes(status); useEffect(() => { - if (!activeOrganizations?.length) { - return; - } - - const organizationId = searchParams.get("organization"); const role = searchParams.get("role"); const status = searchParams.get("status"); - if (isValidOrg(organizationId) && organizationId !== orgFilter) { - setValue("organization", organizationId); - } if (isRoleFilterOption(role) && role !== roleFilter) { setValue("role", role); } @@ -317,25 +278,15 @@ const ListingView: FC = () => { } setTablePage(0); - }, [ - activeOrganizations, - searchParams.get("organization"), - searchParams.get("role"), - searchParams.get("status"), - ]); + }, [searchParams.get("role"), searchParams.get("status")]); useEffect(() => { - if (!touchedFilters.organization && !touchedFilters.role && !touchedFilters.status) { + if (!touchedFilters.role && !touchedFilters.status) { return; } const newSearchParams = new URLSearchParams(searchParams); - if (orgFilter && orgFilter !== "All") { - newSearchParams.set("organization", orgFilter); - } else if (orgFilter === "All") { - newSearchParams.delete("organization"); - } if (roleFilter && roleFilter !== "All") { newSearchParams.set("role", roleFilter); } else if (roleFilter === "All") { @@ -350,7 +301,7 @@ const ListingView: FC = () => { if (newSearchParams?.toString() !== searchParams?.toString()) { setSearchParams(newSearchParams); } - }, [orgFilter, roleFilter, statusFilter, touchedFilters]); + }, [roleFilter, statusFilter, touchedFilters]); const setTablePage = (page: number) => { tableRef.current?.setPage(page, true); @@ -372,33 +323,6 @@ const ListingView: FC = () => { )} - Organization - - ( - { - field.onChange(e); - handleFilterChange("organization"); - }} - > - All - {activeOrganizations?.map((org: Organization) => ( - - {org.name} - - ))} - - )} - /> - Role { columns={columns} data={dataset || []} total={count || 0} - loading={loading || orgStatus === OrgStatus.LOADING || authStatus === AuthStatus.LOADING} + loading={loading || authStatus === AuthStatus.LOADING} disableUrlParams={false} defaultRowsPerPage={20} defaultOrder="asc" diff --git a/src/content/users/ProfileView.tsx b/src/content/users/ProfileView.tsx index 5593d7e22..6bd4b1cc8 100644 --- a/src/content/users/ProfileView.tsx +++ b/src/content/users/ProfileView.tsx @@ -8,12 +8,8 @@ import { useSnackbar } from "notistack"; import bannerSvg from "../../assets/banner/profile_banner.png"; import profileIcon from "../../assets/icons/profile_icon.svg"; import { useAuthContext, Status as AuthStatus } from "../../components/Contexts/AuthContext"; -import { - Status as OrgStatus, - useOrganizationListContext, -} from "../../components/Contexts/OrganizationListContext"; import SuspenseLoader from "../../components/SuspenseLoader"; -import { OrgAssignmentMap, Roles } from "../../config/AuthRoles"; +import { Roles } from "../../config/AuthRoles"; import { EDIT_USER, EditUserInput, @@ -164,25 +160,21 @@ const ProfileView: FC = ({ _id, viewType }: Props) => { const navigate = useNavigate(); const { enqueueSnackbar } = useSnackbar(); - const { data: orgData, activeOrganizations, status: orgStatus } = useOrganizationListContext(); const { user: currentUser, setData, logout, status: authStatus } = useAuthContext(); const { lastSearchParams } = useSearchParamsContext(); - const { handleSubmit, register, reset, watch, setValue, control, formState } = - useForm(); + const { handleSubmit, register, reset, watch, control } = useForm(); const isSelf = _id === currentUser._id; const [user, setUser] = useState( isSelf && viewType === "profile" ? { ...currentUser } : null ); const [saving, setSaving] = useState(false); - const [orgList, setOrgList] = useState[]>(undefined); const roleField = watch("role"); const studiesField = watch("studies"); const fieldset = useProfileFields({ _id: user?._id, role: roleField }, viewType); const visibleFieldState: FieldState[] = ["UNLOCKED", "DISABLED"]; - const userOrg = orgData?.find((org) => org._id === user?.organization?.orgID); const manageUsersPageUrl = `/users${lastSearchParams?.["/users"] ?? ""}`; const canRequestRole: boolean = useMemo(() => { @@ -251,7 +243,6 @@ const ProfileView: FC = ({ _id, viewType }: Props) => { const { data: d, errors } = await editUser({ variables: { userID: _id, - organization: data.organization, role: data.role, userStatus: data.userStatus, studies: fieldset.studies !== "HIDDEN" ? data.studies : null, @@ -285,7 +276,7 @@ const ProfileView: FC = ({ _id, viewType }: Props) => { setUser({ ...currentUser }); reset({ ...currentUser, - organization: currentUser.organization?.orgID || "", + studies: currentUser.studies?.map((s: ApprovedStudy) => s?._id) || [], }); return; } @@ -303,46 +294,12 @@ const ProfileView: FC = ({ _id, viewType }: Props) => { setUser({ ...data?.getUser }); reset({ ...data?.getUser, - organization: data?.getUser.organization?.orgID || "", + studies: data?.getUser?.studies?.map((s: ApprovedStudy) => s?._id) || [], }); })(); }, [_id]); - useEffect(() => { - if (fieldset.organization !== "DISABLED" || !OrgAssignmentMap[roleField]) { - return; - } - - const expectedOrg = orgData?.find((org) => org.name === OrgAssignmentMap[roleField])?._id; - setValue("organization", expectedOrg || ""); - }, [fieldset.organization === "DISABLED", roleField, user, orgData]); - - useEffect(() => { - if (!user || orgStatus === OrgStatus.LOADING) { - return; - } - if (userOrg?.status === "Inactive") { - setOrgList( - [...activeOrganizations, userOrg].sort((a, b) => a.name?.localeCompare(b.name || "")) - ); - return; - } - - setOrgList(activeOrganizations || []); - }, [activeOrganizations, userOrg, user, orgStatus]); - - useEffect(() => { - if (roleField === "User" && "role" in formState.dirtyFields && formState.dirtyFields.role) { - setValue("organization", ""); - } - }, [roleField]); - - if ( - !user || - orgStatus === OrgStatus.LOADING || - authStatus === AuthStatus.LOADING || - orgList === undefined - ) { + if (!user || authStatus === AuthStatus.LOADING) { return ; } @@ -465,35 +422,6 @@ const ProfileView: FC = ({ _id, viewType }: Props) => { user.userStatus )} - - Organization - {visibleFieldState.includes(fieldset.organization) ? ( - ( - - {""} - {orgList?.map((org) => ( - - {org.name} - - ))} - - )} - /> - ) : ( - user?.organization?.orgName - )} - Studies {visibleFieldState.includes(fieldset.studies) ? ( diff --git a/src/graphql/editUser.ts b/src/graphql/editUser.ts index 104b4547e..970fed702 100644 --- a/src/graphql/editUser.ts +++ b/src/graphql/editUser.ts @@ -3,7 +3,6 @@ import gql from "graphql-tag"; export const mutation = gql` mutation editUser( $userID: ID! - $organization: String $userStatus: String $role: String $studies: [String] @@ -11,7 +10,6 @@ export const mutation = gql` ) { editUser( userID: $userID - organization: $organization status: $userStatus role: $role studies: $studies @@ -20,22 +18,23 @@ export const mutation = gql` userStatus role dataCommons + # TODO: Request the study fields from the server studies - organization { - orgID - orgName - createdAt - updateAt - } } } `; export type Input = { + /** + * The UUIDv4 identifier of the user account + */ userID: User["_id"]; - organization: string; -} & Pick; + /** + * An array of studyIDs to assign to the user + */ + studies: string[]; +} & Pick; export type Response = { - editUser: Pick; + editUser: Pick; }; diff --git a/src/graphql/getMyUser.ts b/src/graphql/getMyUser.ts index c31fc3dbb..7c027e78a 100644 --- a/src/graphql/getMyUser.ts +++ b/src/graphql/getMyUser.ts @@ -11,14 +11,8 @@ export const query = gql` IDP email dataCommons + # TODO: Request the study fields from the server studies - organization { - orgID - orgName - status - createdAt - updateAt - } createdAt updateAt } diff --git a/src/graphql/getUser.ts b/src/graphql/getUser.ts index 5c2b67d4d..8844de49a 100644 --- a/src/graphql/getUser.ts +++ b/src/graphql/getUser.ts @@ -13,13 +13,8 @@ export const query = gql` createdAt updateAt dataCommons + # TODO: Request the study fields from the server studies - organization { - orgID - orgName - createdAt - updateAt - } } } `; diff --git a/src/graphql/listUsers.ts b/src/graphql/listUsers.ts index 512334628..0ed5ab370 100644 --- a/src/graphql/listUsers.ts +++ b/src/graphql/listUsers.ts @@ -8,10 +8,6 @@ export const query = gql` lastName IDP email - organization { - orgID - orgName - } userStatus role } @@ -19,5 +15,8 @@ export const query = gql` `; export type Response = { - listUsers: User[]; + listUsers: Pick< + User, + "_id" | "firstName" | "lastName" | "IDP" | "email" | "userStatus" | "role" + >[]; }; diff --git a/src/graphql/updateMyUser.ts b/src/graphql/updateMyUser.ts index dd12fda8a..edff2d1ed 100644 --- a/src/graphql/updateMyUser.ts +++ b/src/graphql/updateMyUser.ts @@ -7,13 +7,8 @@ export const mutation = gql` lastName userStatus role + # TODO: query for subfields studies - organization { - orgID - orgName - createdAt - updateAt - } } } `; @@ -26,5 +21,5 @@ export type Input = { }; export type Response = { - updateMyUser: Pick; + updateMyUser: Pick; }; diff --git a/src/hooks/useProfileFields.test.ts b/src/hooks/useProfileFields.test.ts index 5dccb4a68..a7fbc05a0 100644 --- a/src/hooks/useProfileFields.test.ts +++ b/src/hooks/useProfileFields.test.ts @@ -19,35 +19,6 @@ describe("Users View", () => { expect(result.current.userStatus).toBe("UNLOCKED"); }); - it.each(["User", "Submitter", "Organization Owner", "Data Commons POC"])( - "should return UNLOCKED for organization when viewing the role %s", - (role) => { - const user = { _id: "User-A", role: "Admin" } as User; - const profileOf: Pick = { _id: "I-Am-User-B", role }; - - jest.spyOn(Auth, "useAuthContext").mockReturnValue({ user } as Auth.ContextState); - - const { result } = renderHook(() => useProfileFields(profileOf, "users")); - - expect(result.current.organization).toBe("UNLOCKED"); - } - ); - - // NOTE: This list is derived from the OrgAssignmentMap in src/config/AuthRoles.ts - it.each(["Admin", "Federal Lead", "Federal Monitor"])( - "should return DISABLED for organization when viewing the role %s", - (role) => { - const user = { _id: "User-A", role: "Admin" } as User; - const profileOf: Pick = { _id: "I-Am-User-B", role }; - - jest.spyOn(Auth, "useAuthContext").mockReturnValue({ user } as Auth.ContextState); - - const { result } = renderHook(() => useProfileFields(profileOf, "users")); - - expect(result.current.organization).toBe("DISABLED"); - } - ); - it("should return READ_ONLY for all standard fields when a Organization Owner views the page", () => { const user = { _id: "User-A", role: "Organization Owner" } as User; const profileOf: Pick = { _id: "I-Am-User-B", role: "Submitter" }; @@ -60,7 +31,6 @@ describe("Users View", () => { expect(result.current.lastName).toBe("READ_ONLY"); expect(result.current.role).toBe("READ_ONLY"); expect(result.current.userStatus).toBe("READ_ONLY"); - expect(result.current.organization).toBe("READ_ONLY"); }); it.each<[FieldState, UserRole]>([ @@ -105,17 +75,6 @@ describe("Users View", () => { expect(result.current.dataCommons).toBe(state); }); - it("should return HIDDEN organization field for an Admin viewing a Data Curator profile", () => { - const user = { _id: "User-A", role: "Admin" } as User; - const profileOf: Pick = { _id: "Not-User-a", role: "Data Curator" }; - - jest.spyOn(Auth, "useAuthContext").mockReturnValue({ user } as Auth.ContextState); - - const { result } = renderHook(() => useProfileFields(profileOf, "users")); - - expect(result.current.organization).toBe("HIDDEN"); - }); - it("should always return READ_ONLY for the firstName and lastName fields on the users page", () => { const user = { _id: "User-A", role: "Admin" } as User; const profileOf: Pick = { _id: "I-Am-User-B", role: "Submitter" }; @@ -156,7 +115,6 @@ describe("Profile View", () => { expect(result.current.role).toBe("READ_ONLY"); expect(result.current.userStatus).toBe("READ_ONLY"); - expect(result.current.organization).toBe("READ_ONLY"); }); it.each([ @@ -222,7 +180,6 @@ describe("Profile View", () => { expect(result.current.lastName).toBe("READ_ONLY"); expect(result.current.role).toBe("READ_ONLY"); expect(result.current.userStatus).toBe("READ_ONLY"); - expect(result.current.organization).toBe("READ_ONLY"); expect(result.current.dataCommons).toBe("HIDDEN"); expect(result.current.studies).toBe("HIDDEN"); } @@ -240,15 +197,4 @@ describe("Profile View", () => { expect(result.current.dataCommons).toBe("READ_ONLY"); }); - - it("should return HIDDEN organization field for a Data Curator viewing their own profile", () => { - const user = { _id: "User-A", role: "Data Curator" } as User; - const profileOf: Pick = { ...user }; - - jest.spyOn(Auth, "useAuthContext").mockReturnValue({ user } as Auth.ContextState); - - const { result } = renderHook(() => useProfileFields(profileOf, "profile")); - - expect(result.current.organization).toBe("HIDDEN"); - }); }); diff --git a/src/hooks/useProfileFields.ts b/src/hooks/useProfileFields.ts index 20c772f90..2da4f4592 100644 --- a/src/hooks/useProfileFields.ts +++ b/src/hooks/useProfileFields.ts @@ -1,12 +1,10 @@ import { useAuthContext } from "../components/Contexts/AuthContext"; -import { OrgRequiredRoles } from "../config/AuthRoles"; - /** * Constrains the fields that this hook supports generating states for */ type EditableFields = Extends< keyof User, - "firstName" | "lastName" | "role" | "userStatus" | "organization" | "studies" | "dataCommons" + "firstName" | "lastName" | "role" | "userStatus" | "studies" | "dataCommons" >; /** @@ -41,7 +39,6 @@ const useProfileFields = ( lastName: "READ_ONLY", role: "READ_ONLY", userStatus: "READ_ONLY", - organization: "READ_ONLY", dataCommons: "HIDDEN", studies: "HIDDEN", }; @@ -57,12 +54,6 @@ const useProfileFields = ( if (user?.role === "Admin" && viewType === "users") { fields.role = "UNLOCKED"; fields.userStatus = "UNLOCKED"; - - // Disable for roles with a pre-assigned organization requirement - fields.organization = - !OrgRequiredRoles.includes(profileOf?.role) && profileOf?.role !== "User" - ? "DISABLED" - : "UNLOCKED"; } // Editable for Admin viewing Federal Monitor otherwise hidden @@ -78,11 +69,6 @@ const useProfileFields = ( fields.dataCommons = "HIDDEN"; } - // Only applies to Data Curator - if (profileOf?.role === "Data Curator") { - fields.organization = "HIDDEN"; - } - return fields; }; diff --git a/src/types/Auth.d.ts b/src/types/Auth.d.ts index ba7e2c626..b966cbf15 100644 --- a/src/types/Auth.d.ts +++ b/src/types/Auth.d.ts @@ -25,18 +25,20 @@ type User = { * The user's organization if assigned, null otherwise * * @see {@link OrgInfo} + * @deprecated This field is deprecated and NOT populated by all APIs. Remove ASAP. */ - organization: OrgInfo | null; + organization?: OrgInfo | null; /** * List of data commons that the user has access to */ dataCommons: string[]; /** - * A list of studyIDs that the user is assigned to + * List of ApprovedStudies that the user has access to * + * @note Not all APIs populate this field fully. * @see {@link ApprovedStudy} */ - studies: string[] | null; + studies: ApprovedStudy[] | Pick[]; /** * The SSO IDP used to login */ diff --git a/src/utils/formModeUtils.test.ts b/src/utils/formModeUtils.test.ts index 5cdfee47c..9be6be935 100644 --- a/src/utils/formModeUtils.test.ts +++ b/src/utils/formModeUtils.test.ts @@ -11,13 +11,6 @@ describe("getFormMode tests based on provided requirements", () => { IDP: "nih", createdAt: "2023-05-01T09:23:30Z", updateAt: "2023-05-02T09:23:30Z", - organization: { - orgID: "org1", - orgName: "TestOrg", - status: "Active", - createdAt: "2023-05-01T09:23:30Z", - updateAt: "2023-05-02T09:23:30Z", - }, studies: null, dataCommons: [], }; @@ -29,8 +22,8 @@ describe("getFormMode tests based on provided requirements", () => { questionnaireData: InitialQuestionnaire, status: "New", organization: { - _id: baseUser.organization.orgID, - name: baseUser.organization.orgName, + _id: null, + name: null, }, applicant: { applicantID: baseUser._id, From 25c16bdad238cc011f974327f57c8827149de171 Mon Sep 17 00:00:00 2001 From: Alec M Date: Mon, 2 Dec 2024 10:48:44 -0500 Subject: [PATCH 2/8] feat: Show studies dropdown for Submitters --- src/content/users/ProfileView.tsx | 46 +++++++++++++++--------------- src/hooks/useProfileFields.test.ts | 4 +-- src/hooks/useProfileFields.ts | 11 +++---- 3 files changed, 31 insertions(+), 30 deletions(-) diff --git a/src/content/users/ProfileView.tsx b/src/content/users/ProfileView.tsx index 6bd4b1cc8..526824c22 100644 --- a/src/content/users/ProfileView.tsx +++ b/src/content/users/ProfileView.tsx @@ -399,29 +399,6 @@ const ProfileView: FC = ({ _id, viewType }: Props) => { )} - - Account Status - {visibleFieldState.includes(fieldset.userStatus) ? ( - ( - - Active - Inactive - - )} - /> - ) : ( - user.userStatus - )} - Studies {visibleFieldState.includes(fieldset.studies) ? ( @@ -464,6 +441,29 @@ const ProfileView: FC = ({ _id, viewType }: Props) => { ) : null} + + Account Status + {visibleFieldState.includes(fieldset.userStatus) ? ( + ( + + Active + Inactive + + )} + /> + ) : ( + user.userStatus + )} + Data Commons {visibleFieldState.includes(fieldset.dataCommons) ? ( diff --git a/src/hooks/useProfileFields.test.ts b/src/hooks/useProfileFields.test.ts index a7fbc05a0..748ecc1ea 100644 --- a/src/hooks/useProfileFields.test.ts +++ b/src/hooks/useProfileFields.test.ts @@ -35,14 +35,14 @@ describe("Users View", () => { it.each<[FieldState, UserRole]>([ ["HIDDEN", "User"], - ["HIDDEN", "Submitter"], ["HIDDEN", "Organization Owner"], ["HIDDEN", "Federal Lead"], ["HIDDEN", "Data Curator"], ["HIDDEN", "Data Commons POC"], ["HIDDEN", "Admin"], ["HIDDEN", "fake role" as UserRole], - ["UNLOCKED", "Federal Monitor"], // NOTE: Only this role accepts studies + ["UNLOCKED", "Submitter"], // NOTE: This role accepts studies + ["UNLOCKED", "Federal Monitor"], // NOTE: This role accepts studies ])("should return %s for the studies field on the users page for role %s", (state, role) => { const user = { _id: "User-A", role: "Admin" } as User; const profileOf: Pick = { _id: "I-Am-User-B", role }; diff --git a/src/hooks/useProfileFields.ts b/src/hooks/useProfileFields.ts index 2da4f4592..fc585a2a1 100644 --- a/src/hooks/useProfileFields.ts +++ b/src/hooks/useProfileFields.ts @@ -54,12 +54,13 @@ const useProfileFields = ( if (user?.role === "Admin" && viewType === "users") { fields.role = "UNLOCKED"; fields.userStatus = "UNLOCKED"; - } - // Editable for Admin viewing Federal Monitor otherwise hidden - // even for a user viewing their own profile - if (user?.role === "Admin" && viewType === "users" && profileOf?.role === "Federal Monitor") { - fields.studies = "UNLOCKED"; + // Editable for Admin viewing Federal Monitor or Submitter, otherwise hidden + // even for a user viewing their own profile + fields.studies = + profileOf?.role === "Submitter" || profileOf?.role === "Federal Monitor" + ? "UNLOCKED" + : "HIDDEN"; } // Only applies to Data Commons POC From 164c486dd7a4547d3a67296b08a19807c70376dc Mon Sep 17 00:00:00 2001 From: Alec M Date: Mon, 2 Dec 2024 10:53:51 -0500 Subject: [PATCH 3/8] Remove study selection count --- src/content/users/ProfileView.tsx | 77 ++++++++++++------------------- 1 file changed, 29 insertions(+), 48 deletions(-) diff --git a/src/content/users/ProfileView.tsx b/src/content/users/ProfileView.tsx index 526824c22..7faf1992a 100644 --- a/src/content/users/ProfileView.tsx +++ b/src/content/users/ProfileView.tsx @@ -138,17 +138,6 @@ const StyledTitleBox = styled(Box)({ width: "100%", }); -const StyledSelectionCount = styled(Typography)({ - fontSize: "16px", - fontWeight: 600, - color: "#666666", - width: "200px", - position: "absolute", - left: "373px", - transform: "translateY(-50%)", - top: "50%", -}); - /** * User Profile View Component * @@ -171,7 +160,6 @@ const ProfileView: FC = ({ _id, viewType }: Props) => { const [saving, setSaving] = useState(false); const roleField = watch("role"); - const studiesField = watch("studies"); const fieldset = useProfileFields({ _id: user?._id, role: roleField }, viewType); const visibleFieldState: FieldState[] = ["UNLOCKED", "DISABLED"]; @@ -402,43 +390,36 @@ const ProfileView: FC = ({ _id, viewType }: Props) => { Studies {visibleFieldState.includes(fieldset.studies) ? ( -
- ( - - formatStudySelectionValue( - selected, - approvedStudies?.listApprovedStudies?.studies - ) - } - multiple - > - {approvedStudies?.listApprovedStudies?.studies?.map( - ({ _id, studyName, studyAbbreviation }) => ( - - {formatFullStudyName(studyName, studyAbbreviation)} - - ) - )} - - )} - /> - {studiesField?.length > 1 && ( - - * {studiesField?.length} Studies selected - + ( + + formatStudySelectionValue( + selected, + approvedStudies?.listApprovedStudies?.studies + ) + } + multiple + > + {approvedStudies?.listApprovedStudies?.studies?.map( + ({ _id, studyName, studyAbbreviation }) => ( + + {formatFullStudyName(studyName, studyAbbreviation)} + + ) + )} + )} -
+ /> ) : null} From 954f14ef8ee5d7d306d079cba5ec766b6be98dad Mon Sep 17 00:00:00 2001 From: Alec M Date: Tue, 3 Dec 2024 11:43:10 -0500 Subject: [PATCH 4/8] feat: Migrate studies dropdown to Autocomplete --- src/content/users/ProfileView.tsx | 91 ++++++++++++++++------ src/utils/formUtils.test.ts | 120 ++++++++++++++++++------------ src/utils/formUtils.ts | 31 ++++---- 3 files changed, 159 insertions(+), 83 deletions(-) diff --git a/src/content/users/ProfileView.tsx b/src/content/users/ProfileView.tsx index 7faf1992a..8b58f4932 100644 --- a/src/content/users/ProfileView.tsx +++ b/src/content/users/ProfileView.tsx @@ -1,7 +1,7 @@ import { FC, useEffect, useMemo, useState } from "react"; import { useLazyQuery, useMutation, useQuery } from "@apollo/client"; import { LoadingButton } from "@mui/lab"; -import { Box, Container, MenuItem, Stack, Typography, styled } from "@mui/material"; +import { Box, Container, MenuItem, Stack, TextField, Typography, styled } from "@mui/material"; import { Controller, useForm } from "react-hook-form"; import { useNavigate } from "react-router-dom"; import { useSnackbar } from "notistack"; @@ -30,6 +30,7 @@ import usePageTitle from "../../hooks/usePageTitle"; import { useSearchParamsContext } from "../../components/Contexts/SearchParamsContext"; import BaseSelect from "../../components/StyledFormComponents/StyledSelect"; import BaseOutlinedInput from "../../components/StyledFormComponents/StyledOutlinedInput"; +import BaseAutocomplete from "../../components/StyledFormComponents/StyledAutocomplete"; import useProfileFields, { FieldState } from "../../hooks/useProfileFields"; import AccessRequest from "../../components/AccessRequest"; @@ -109,6 +110,7 @@ const BaseInputStyling = { width: "363px", }; +const StyledAutocomplete = styled(BaseAutocomplete)(BaseInputStyling); const StyledTextField = styled(BaseOutlinedInput)(BaseInputStyling); const StyledSelect = styled(BaseSelect)(BaseInputStyling); @@ -138,6 +140,11 @@ const StyledTitleBox = styled(Box)({ width: "100%", }); +const StyledTag = styled("div")({ + position: "absolute", + paddingLeft: "12px", +}); + /** * User Profile View Component * @@ -158,6 +165,7 @@ const ProfileView: FC = ({ _id, viewType }: Props) => { isSelf && viewType === "profile" ? { ...currentUser } : null ); const [saving, setSaving] = useState(false); + const [studyOptions, setStudyOptions] = useState([]); const roleField = watch("role"); const fieldset = useProfileFields({ _id: user?._id, role: roleField }, viewType); @@ -205,6 +213,20 @@ const ProfileView: FC = ({ _id, viewType }: Props) => { } ); + const formattedStudyMap = useMemo>(() => { + if (!approvedStudies?.listApprovedStudies?.studies) { + return {}; + } + + return approvedStudies.listApprovedStudies.studies.reduce( + (acc, { _id, studyName, studyAbbreviation }) => ({ + ...acc, + [_id]: formatFullStudyName(studyName, studyAbbreviation), + }), + {} + ); + }, [approvedStudies?.listApprovedStudies?.studies]); + const onSubmit = async (data: FormInput) => { setSaving(true); @@ -258,6 +280,20 @@ const ProfileView: FC = ({ _id, viewType }: Props) => { } }; + const sortStudyOptions = () => { + const val = watch("studies"); + const options = Object.keys(formattedStudyMap); + + const selectedOptions = val + .filter((v) => options.includes(v)) + .sort((a, b) => formattedStudyMap[a]?.localeCompare(formattedStudyMap?.[b])); + const unselectedOptions = options + .filter((o) => !selectedOptions.includes(o)) + .sort((a, b) => formattedStudyMap[a]?.localeCompare(formattedStudyMap?.[b])); + + setStudyOptions([...selectedOptions, ...unselectedOptions]); + }; + useEffect(() => { // No action needed if viewing own profile, using cached data if (isSelf && viewType === "profile") { @@ -287,6 +323,12 @@ const ProfileView: FC = ({ _id, viewType }: Props) => { })(); }, [_id]); + useEffect(() => { + if (fieldset.studies === "UNLOCKED") { + sortStudyOptions(); + } + }, [formattedStudyMap]); + if (!user || authStatus === AuthStatus.LOADING) { return ; } @@ -395,29 +437,36 @@ const ProfileView: FC = ({ _id, viewType }: Props) => { control={control} rules={{ required: false }} render={({ field }) => ( - ( + 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[]) => field.onChange(data)} disabled={fieldset.studies === "DISABLED"} - MenuProps={{ disablePortal: true }} - inputProps={{ "aria-labelledby": "userStudies" }} - renderValue={(selected: string[]) => - formatStudySelectionValue( - selected, - approvedStudies?.listApprovedStudies?.studies - ) - } + disableCloseOnSelect multiple - > - {approvedStudies?.listApprovedStudies?.studies?.map( - ({ _id, studyName, studyAbbreviation }) => ( - - {formatFullStudyName(studyName, studyAbbreviation)} - - ) - )} - + /> )} /> ) : null} diff --git a/src/utils/formUtils.test.ts b/src/utils/formUtils.test.ts index b7fda526f..a8681e6f7 100644 --- a/src/utils/formUtils.test.ts +++ b/src/utils/formUtils.test.ts @@ -449,73 +449,97 @@ describe("formatORCIDInput cases", () => { }); describe("renderStudySelectionValue cases", () => { - const baseStudy: ApprovedStudy = { - _id: "", - studyName: "", - studyAbbreviation: "", - dbGaPID: "", - controlledAccess: false, - originalOrg: "", - openAccess: false, - PI: "", - ORCID: "", - createdAt: "", - }; - - it("should return the fallback value if studyIds is not an array", () => { - const result = utils.formatStudySelectionValue(null, [baseStudy], "fallback"); - expect(result).toBe("fallback"); + it("should filter out studies that are not in the studyMap", () => { + const studies = { + a: "Study A", + b: "Study B", + }; + + const result = utils.formatStudySelectionValue(["a", "c"], studies); + expect(result).toBe("Study A"); }); - it("should return the fallback value if approvedStudies is not an array", () => { - const result = utils.formatStudySelectionValue(["1"], null, "fallback"); - expect(result).toBe("fallback"); + it("should append an ellipsis if the combined result is longer than 30 char", () => { + const studies = { + a: "Study A", + b: "Study B", + longStudy: "X".repeat(30), + }; + + const result = utils.formatStudySelectionValue(["a", "b"], studies); + expect(result).toContain("Study A, Study B"); + + const longResult = utils.formatStudySelectionValue(["a", "longStudy"], studies); + expect(longResult).toContain("Study A, XXXXXXXXXXXXXXXXXXXXX..."); }); - it("should return the fallback value if studyIds is empty", () => { - const result = utils.formatStudySelectionValue([], [baseStudy], "fallback"); - expect(result).toBe("fallback"); + it("should sort the selection alphabetically ignoring selection order", () => { + const studies = { + a: "AA", + b: "BB", + c: "CC", + y: "YY", + }; + + const result = utils.formatStudySelectionValue(["c", "a", "y"], studies); + expect(result).toContain("AA, CC, YY"); + + const result2 = utils.formatStudySelectionValue(["b", "y", "c", "a"], studies); + expect(result2).toContain("AA, BB, CC, YY"); }); - it("should return the fallback value if approvedStudies is empty", () => { - const result = utils.formatStudySelectionValue(["1"], [], "fallback"); - expect(result).toBe("fallback"); + it("should contain the number of studies selected if more than one", () => { + const studies = { + a: "Study A", + b: "Study B", + c: "Study C", + long: "X".repeat(30), + }; + + const noCount = utils.formatStudySelectionValue(["a"], studies); + expect(noCount).toBe("Study A"); + + const withCount = utils.formatStudySelectionValue(["a", "b", "c"], studies); + expect(withCount).not.toContain("..."); + expect(withCount).toContain(" (3)"); + + const withCountAndEllipsis = utils.formatStudySelectionValue(["a", "b", "c", "long"], studies); + expect(withCountAndEllipsis).toContain("..."); + expect(withCountAndEllipsis).toContain(" (4)"); }); - it("should return the fallback value if no matching study is found", () => { - const studies = [ - { _id: "1", studyName: "Study 1", studyAbbreviation: "S1" }, - { _id: "2", studyName: "Study 2", studyAbbreviation: "S2" }, - ] as ApprovedStudy[]; + it("should return the fallback value if selectedIds is not an array", () => { + const result = utils.formatStudySelectionValue(null, { b: "xyz" }, "fallback"); + expect(result).toBe("fallback"); + }); - const result = utils.formatStudySelectionValue(["3"], studies, "fallback"); + it("should return the fallback value if studyMap is not a map", () => { + const result = utils.formatStudySelectionValue(["1"], null, "fallback"); expect(result).toBe("fallback"); }); - it("should sort the approved studies by name and return the first element", () => { - const studies = [ - { _id: "3", studyName: "Study C", studyAbbreviation: "SA" }, // actual 3 - { _id: "2", studyName: "Study A", studyAbbreviation: "SA" }, // actual 1 - { _id: "1", studyName: "Study B", studyAbbreviation: "SB" }, // actual 2 - ] as ApprovedStudy[]; + it("should return the fallback value if selectedIds is empty", () => { + const result = utils.formatStudySelectionValue([], { b: "xyz" }, "fallback"); + expect(result).toBe("fallback"); + }); - const result = utils.formatStudySelectionValue(["3", "2"], studies, "fallback"); - expect(result).toBe("Study A (SA)"); + it("should return the fallback value if studyMap is empty", () => { + const result = utils.formatStudySelectionValue(["1"], {}, "fallback"); + expect(result).toBe("fallback"); }); - it("should filter out studies with formatted names", () => { - const studies = [ - { _id: "1", studyName: "", studyAbbreviation: "" }, - { _id: "2", studyName: "Study 2", studyAbbreviation: "S2" }, - { _id: "3", studyName: "Study 3", studyAbbreviation: "S3" }, - ] as ApprovedStudy[]; + it("should return the fallback value if no matching study is found", () => { + const studies = { + a: "Study A", + b: "Study B", + }; - const result = utils.formatStudySelectionValue(["1", "2", "3"], studies, "fallback"); - expect(result).toBe("Study 2 (S2)"); + const result = utils.formatStudySelectionValue(["x"], studies, "fallback"); + expect(result).toBe("fallback"); }); it("should use the default fallback value if none is provided", () => { - const result = utils.formatStudySelectionValue([], []); + const result = utils.formatStudySelectionValue([], {}); expect(result).toBe(""); }); }); diff --git a/src/utils/formUtils.ts b/src/utils/formUtils.ts index c54d11234..733c959c8 100644 --- a/src/utils/formUtils.ts +++ b/src/utils/formUtils.ts @@ -222,33 +222,36 @@ export const formatORCIDInput = (input: string): string => { * Given an array of Study IDs, return the first Formatted Study Name from the list of approved studies, sorted by the Study Name. * * @note MUI shows the first SELECTED item by default, this will show the first SORTED item - * @see {@link formatFullStudyName} for the formatting implementation - * @param studyIds Array of Study IDs - * @param approvedStudies List of approved studies, ideally containing the studies in studyIds + * @param selectedIds Array of Study IDs + * @param studyMap A map of the _ID: Study Name and Abbreviation * @returns The first formatted study name from the list of approved studies */ export const formatStudySelectionValue = ( - studyIds: string[], - approvedStudies: ApprovedStudy[], + selectedIds: string[], + studyMap: Record, fallback = "" ): string => { - if (!Array.isArray(studyIds) || !Array.isArray(approvedStudies)) { + if (!Array.isArray(selectedIds) || selectedIds.length === 0) { return fallback; } - if (studyIds.length === 0 || approvedStudies.length === 0) { + if (!studyMap || Object.keys(studyMap).length === 0) { return fallback; } - const mappedStudies: string[] = studyIds - .map((studyID) => { - const study: ApprovedStudy = approvedStudies?.find((s) => s?._id === studyID); - - return formatFullStudyName(study?.studyName, study?.studyAbbreviation); - }) + const sortedStudies: string[] = selectedIds + .map((studyID) => studyMap?.[studyID]) .filter((study) => typeof study === "string" && study.length > 0) .sort((a: string, b: string) => a.localeCompare(b)); - return mappedStudies.length > 0 ? mappedStudies[0] : fallback; + if (sortedStudies.length === 0) { + return fallback; + } + + const joinedStudies = sortedStudies.join(", "); + const trimmedJoin = + joinedStudies.length > 30 ? `${joinedStudies.substring(0, 30)}...` : joinedStudies; + + return `${trimmedJoin}${sortedStudies.length > 1 ? ` (${sortedStudies.length})` : ""}`; }; /** From f5605e3c64f5d8105cc858709f72249b12b84514 Mon Sep 17 00:00:00 2001 From: Alec M Date: Wed, 4 Dec 2024 16:59:45 -0500 Subject: [PATCH 5/8] Query subfields on ApprovedStudy type --- src/graphql/editUser.ts | 16 +++++++++++++--- src/graphql/getMyUser.ts | 16 +++++++++++++--- src/graphql/getUser.ts | 11 ++++++++--- src/graphql/updateMyUser.ts | 16 +++++++++++++--- 4 files changed, 47 insertions(+), 12 deletions(-) diff --git a/src/graphql/editUser.ts b/src/graphql/editUser.ts index 970fed702..bb5d1ae8a 100644 --- a/src/graphql/editUser.ts +++ b/src/graphql/editUser.ts @@ -18,8 +18,13 @@ export const mutation = gql` userStatus role dataCommons - # TODO: Request the study fields from the server - studies + studies { + _id + studyName + studyAbbreviation + dbGaPID + controlledAccess + } } } `; @@ -36,5 +41,10 @@ export type Input = { } & 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 7c027e78a..1691eed40 100644 --- a/src/graphql/getMyUser.ts +++ b/src/graphql/getMyUser.ts @@ -11,8 +11,13 @@ export const query = gql` IDP email dataCommons - # TODO: Request the study fields from the server - studies + studies { + _id + studyName + studyAbbreviation + dbGaPID + controlledAccess + } createdAt updateAt } @@ -20,5 +25,10 @@ export const query = gql` `; export type Response = { - getMyUser: User; + getMyUser: User & { + studies: Pick< + ApprovedStudy, + "_id" | "studyName" | "studyAbbreviation" | "dbGaPID" | "controlledAccess" + >[]; + }; }; diff --git a/src/graphql/getUser.ts b/src/graphql/getUser.ts index 8844de49a..12347d5da 100644 --- a/src/graphql/getUser.ts +++ b/src/graphql/getUser.ts @@ -13,8 +13,11 @@ export const query = gql` createdAt updateAt dataCommons - # TODO: Request the study fields from the server - studies + studies { + _id + studyName + studyAbbreviation + } } } `; @@ -24,5 +27,7 @@ export type Input = { }; export type Response = { - getUser: User; + getUser: User & { + studies: Pick[]; + }; }; diff --git a/src/graphql/updateMyUser.ts b/src/graphql/updateMyUser.ts index edff2d1ed..16c9423f0 100644 --- a/src/graphql/updateMyUser.ts +++ b/src/graphql/updateMyUser.ts @@ -7,8 +7,13 @@ export const mutation = gql` lastName userStatus role - # TODO: query for subfields - studies + studies { + _id + studyName + studyAbbreviation + dbGaPID + controlledAccess + } } } `; @@ -21,5 +26,10 @@ export type Input = { }; export type Response = { - updateMyUser: Pick; + updateMyUser: Pick & { + studies: Pick< + ApprovedStudy, + "_id" | "studyName" | "studyAbbreviation" | "dbGaPID" | "controlledAccess" + >[]; + }; }; From 58d6248278282a3cfdbf60147bcd937e347dabab Mon Sep 17 00:00:00 2001 From: Alec M Date: Wed, 4 Dec 2024 17:14:16 -0500 Subject: [PATCH 6/8] Track studies loading state --- src/content/users/ProfileView.tsx | 33 ++++++++++++++++--------------- 1 file changed, 17 insertions(+), 16 deletions(-) diff --git a/src/content/users/ProfileView.tsx b/src/content/users/ProfileView.tsx index 8b58f4932..1f72203d1 100644 --- a/src/content/users/ProfileView.tsx +++ b/src/content/users/ProfileView.tsx @@ -196,22 +196,22 @@ const ProfileView: FC = ({ _id, viewType }: Props) => { fetchPolicy: "no-cache", }); - const { data: approvedStudies } = useQuery( - LIST_APPROVED_STUDIES, - { - variables: { - // show all access types - controlledAccess: "All", - first: -1, - offset: 0, - orderBy: "studyName", - sortDirection: "asc", - }, - context: { clientName: "backend" }, - fetchPolicy: "cache-and-network", - skip: fieldset.studies !== "UNLOCKED", - } - ); + const { data: approvedStudies, loading: approvedStudiesLoading } = useQuery< + ListApprovedStudiesResp, + ListApprovedStudiesInput + >(LIST_APPROVED_STUDIES, { + variables: { + // show all access types + controlledAccess: "All", + first: -1, + offset: 0, + orderBy: "studyName", + sortDirection: "asc", + }, + context: { clientName: "backend" }, + fetchPolicy: "cache-and-network", + skip: fieldset.studies !== "UNLOCKED", + }); const formattedStudyMap = useMemo>(() => { if (!approvedStudies?.listApprovedStudies?.studies) { @@ -464,6 +464,7 @@ const ProfileView: FC = ({ _id, viewType }: Props) => { getOptionLabel={(option: string) => formattedStudyMap[option]} onChange={(_, data: string[]) => field.onChange(data)} disabled={fieldset.studies === "DISABLED"} + loading={approvedStudiesLoading} disableCloseOnSelect multiple /> From 7b2a835970ee68fd5dae0d25ff6afb6656b1b7bc Mon Sep 17 00:00:00 2001 From: Alec M Date: Wed, 4 Dec 2024 17:25:06 -0500 Subject: [PATCH 7/8] fix: Enforce field required logic on FE --- src/content/users/ProfileView.tsx | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/content/users/ProfileView.tsx b/src/content/users/ProfileView.tsx index 1f72203d1..d8c74edbf 100644 --- a/src/content/users/ProfileView.tsx +++ b/src/content/users/ProfileView.tsx @@ -435,7 +435,7 @@ const ProfileView: FC = ({ _id, viewType }: Props) => { ( = ({ _id, viewType }: Props) => { } inputProps={{ "aria-labelledby": "userStudies", ...inputProps }} onBlur={sortStudyOptions} + required /> )} renderTags={(value: string[], _, state) => { From 11ea590f4722f7bf8c41a3538a6608b969a4749f Mon Sep 17 00:00:00 2001 From: Alec M Date: Thu, 5 Dec 2024 10:32:50 -0500 Subject: [PATCH 8/8] fix: Form is unable to be submitted --- src/content/users/ProfileView.tsx | 1 - 1 file changed, 1 deletion(-) diff --git a/src/content/users/ProfileView.tsx b/src/content/users/ProfileView.tsx index d8c74edbf..de194a54b 100644 --- a/src/content/users/ProfileView.tsx +++ b/src/content/users/ProfileView.tsx @@ -447,7 +447,6 @@ const ProfileView: FC = ({ _id, viewType }: Props) => { } inputProps={{ "aria-labelledby": "userStudies", ...inputProps }} onBlur={sortStudyOptions} - required /> )} renderTags={(value: string[], _, state) => {