From 76fae542c8ab54f06f02ba9fe85154077c49e8ec Mon Sep 17 00:00:00 2001 From: wjames111 Date: Mon, 14 Sep 2026 16:53:28 -0400 Subject: [PATCH 1/2] [MPDX-10001] Hide the MPD Goal admin table and finished questionnaires from the HR Tools menu Co-Authored-By: Claude Opus 5 --- .../hrTools/mpdGoalAdmin/index.page.tsx | 4 +- .../[scenarioGoalId]/index.page.test.tsx | 13 +- .../scenario/[scenarioGoalId]/index.page.tsx | 4 +- .../[staffAccountListId]/index.page.test.tsx | 13 +- .../staff/[staffAccountListId]/index.page.tsx | 4 +- .../UserTypeAccess/UserTypeAccess.test.tsx | 29 ++++ .../Shared/UserTypeAccess/UserTypeAccess.tsx | 3 + src/components/User/GetUser.graphql | 1 + src/hooks/NewStaffQuestionnaireStatus.graphql | 6 + src/hooks/useHrToolsNavItems.test.tsx | 147 +++++++++++++++--- src/hooks/useHrToolsNavItems.ts | 28 +++- src/hooks/useIneligibleByGroup.ts | 4 + 12 files changed, 216 insertions(+), 40 deletions(-) create mode 100644 src/hooks/NewStaffQuestionnaireStatus.graphql diff --git a/pages/accountLists/[accountListId]/hrTools/mpdGoalAdmin/index.page.tsx b/pages/accountLists/[accountListId]/hrTools/mpdGoalAdmin/index.page.tsx index f77b0ab732..eb6057df34 100644 --- a/pages/accountLists/[accountListId]/hrTools/mpdGoalAdmin/index.page.tsx +++ b/pages/accountLists/[accountListId]/hrTools/mpdGoalAdmin/index.page.tsx @@ -62,7 +62,9 @@ export const MpdGoalAdminPage: React.FC = () => { )}`} {accountListId ? ( - + diff --git a/pages/accountLists/[accountListId]/hrTools/mpdGoalAdmin/scenario/[scenarioGoalId]/index.page.test.tsx b/pages/accountLists/[accountListId]/hrTools/mpdGoalAdmin/scenario/[scenarioGoalId]/index.page.test.tsx index fc5cffb0ac..62caf78cdb 100644 --- a/pages/accountLists/[accountListId]/hrTools/mpdGoalAdmin/scenario/[scenarioGoalId]/index.page.test.tsx +++ b/pages/accountLists/[accountListId]/hrTools/mpdGoalAdmin/scenario/[scenarioGoalId]/index.page.test.tsx @@ -11,7 +11,6 @@ import { NewStaffGoalCalculationQuery } from 'src/components/HrTools/NsGoalCalcu import { GetUserQuery } from 'src/components/User/GetUser.generated'; import { NewStaffQuestionnaireMaritalStatusEnum, - UsStaffGroupEnum, UserTypeEnum, } from 'src/graphql/types.generated'; import { GoalCalculatorConstantsQuery } from 'src/hooks/goalCalculatorConstants.generated'; @@ -28,12 +27,12 @@ const mockBlockImpersonatingNonDevelopers = >; interface TestComponentProps { - /** Senior Staff is the group the MPD goal tools are open to. */ - usStaffGroup?: UsStaffGroupEnum; + /** The MPD Goals team and MPD coordinators are the only ones let in. */ + canViewNewStaffCohorts?: boolean; } const TestComponent: React.FC = ({ - usStaffGroup = UsStaffGroupEnum.SeniorStaff, + canViewNewStaffCohorts = true, }) => ( = ({ GetUser: { user: { userType: UserTypeEnum.UsStaff, - usStaffGroup, + canViewNewStaffCohorts, staffAccountId: 'staff-account-1', }, }, @@ -91,9 +90,9 @@ describe('Scenario NsGoalCalculator page', () => { ).toBeInTheDocument(); }); - it("denies a user outside the admin table's group", async () => { + it('denies a user without goals-team or coordinator access', async () => { const { findByRole } = render( - , + , ); expect( diff --git a/pages/accountLists/[accountListId]/hrTools/mpdGoalAdmin/scenario/[scenarioGoalId]/index.page.tsx b/pages/accountLists/[accountListId]/hrTools/mpdGoalAdmin/scenario/[scenarioGoalId]/index.page.tsx index 37cc3ef2a3..a22f28da60 100644 --- a/pages/accountLists/[accountListId]/hrTools/mpdGoalAdmin/scenario/[scenarioGoalId]/index.page.tsx +++ b/pages/accountLists/[accountListId]/hrTools/mpdGoalAdmin/scenario/[scenarioGoalId]/index.page.tsx @@ -32,7 +32,9 @@ export const NsScenarioGoalPage: React.FC = () => { {`${appName} | ${t('New Staff Goal Calculator')}`} {scenarioGoalId ? ( - + = ({ - usStaffGroup = UsStaffGroupEnum.SeniorStaff, + canViewNewStaffCohorts = true, staffAccountListId = 'staff-account-list-1', }) => ( = ({ GetUser: { user: { userType: UserTypeEnum.UsStaff, - usStaffGroup, + canViewNewStaffCohorts, staffAccountId: 'staff-account-1', }, }, @@ -140,9 +139,9 @@ describe('Staff Details page', () => { expect(queryByRole('navigation')).not.toBeInTheDocument(); }); - it("denies a user outside the admin table's group", async () => { + it('denies a user without goals-team or coordinator access', async () => { const { findByRole } = render( - , + , ); expect( diff --git a/pages/accountLists/[accountListId]/hrTools/mpdGoalAdmin/staff/[staffAccountListId]/index.page.tsx b/pages/accountLists/[accountListId]/hrTools/mpdGoalAdmin/staff/[staffAccountListId]/index.page.tsx index 24de755188..307f24a0ca 100644 --- a/pages/accountLists/[accountListId]/hrTools/mpdGoalAdmin/staff/[staffAccountListId]/index.page.tsx +++ b/pages/accountLists/[accountListId]/hrTools/mpdGoalAdmin/staff/[staffAccountListId]/index.page.tsx @@ -39,7 +39,9 @@ export const NsStaffDetailsPage: React.FC = () => { )}`} {staffAccountListId ? ( - + = ({ spouseUsStaffGroup = UsStaffGroupEnum.PartTimeFieldStaff, staffAccountId = id, supervisesStaff = true, + canViewNewStaffCohorts = true, requireUserGroups, }) => ( @@ -45,6 +47,7 @@ const TestComponent: React.FC = ({ spouseUsStaffGroup, staffAccountId, supervisesStaff, + canViewNewStaffCohorts, }, }, }} @@ -218,6 +221,32 @@ describe('UserTypeAccess', () => { expect(await findByText('Test Content')).toBeInTheDocument(); }); + it('should render LimitedAccess when the user may not open the admin table', async () => { + const { findByRole } = render( + , + ); + + expect( + await findByRole('heading', { + name: 'Access to this feature is limited.', + }), + ).toBeInTheDocument(); + }); + + it('should render child component for the goals team and coordinators, whatever their group', async () => { + const { findByText } = render( + , + ); + expect(await findByText('Test Content')).toBeInTheDocument(); + }); + it('should render LimitedAccess when staff account is required but not present', async () => { const { findByRole, getByText } = render( , diff --git a/src/components/Shared/UserTypeAccess/UserTypeAccess.tsx b/src/components/Shared/UserTypeAccess/UserTypeAccess.tsx index 7483897838..faafbc1857 100644 --- a/src/components/Shared/UserTypeAccess/UserTypeAccess.tsx +++ b/src/components/Shared/UserTypeAccess/UserTypeAccess.tsx @@ -12,6 +12,7 @@ export enum RequiredUserGroupEnum { NsGoalCalc = 'nsGoalCalc', PdsGoalCalc = 'pdsGoalCalc', MpdSupervisor = 'mpdSupervisor', + NewStaffCohorts = 'newStaffCohorts', } export const isUsStaffLike = (user?: UserTypeEnum) => @@ -42,6 +43,7 @@ export const UserTypeAccess: React.FC = ({ inNsGoalCalcIneligibleGroup, inPdsGoalCalcIneligibleGroup, inMpdSupervisorIneligibleGroup, + canViewNewStaffCohorts, userType, hasNoStaffAccount, userLoading, @@ -57,6 +59,7 @@ export const UserTypeAccess: React.FC = ({ [RequiredUserGroupEnum.NsGoalCalc]: inNsGoalCalcIneligibleGroup, [RequiredUserGroupEnum.PdsGoalCalc]: inPdsGoalCalcIneligibleGroup, [RequiredUserGroupEnum.MpdSupervisor]: inMpdSupervisorIneligibleGroup, + [RequiredUserGroupEnum.NewStaffCohorts]: !canViewNewStaffCohorts, }; const meetsRequiredUserType = diff --git a/src/components/User/GetUser.graphql b/src/components/User/GetUser.graphql index 8fad3da299..a5a7f08d21 100644 --- a/src/components/User/GetUser.graphql +++ b/src/components/User/GetUser.graphql @@ -10,6 +10,7 @@ query GetUser { locale: localeDisplay } staffAccountId + canViewNewStaffCohorts primaryDesignation userType usStaffGroup diff --git a/src/hooks/NewStaffQuestionnaireStatus.graphql b/src/hooks/NewStaffQuestionnaireStatus.graphql new file mode 100644 index 0000000000..b68d411499 --- /dev/null +++ b/src/hooks/NewStaffQuestionnaireStatus.graphql @@ -0,0 +1,6 @@ +query NewStaffQuestionnaireStatus($accountListId: ID!) { + newStaffQuestionnaire(accountListId: $accountListId) { + id + completed + } +} diff --git a/src/hooks/useHrToolsNavItems.test.tsx b/src/hooks/useHrToolsNavItems.test.tsx index 0845630536..e6c99020cf 100644 --- a/src/hooks/useHrToolsNavItems.test.tsx +++ b/src/hooks/useHrToolsNavItems.test.tsx @@ -1,9 +1,12 @@ import { ReactElement } from 'react'; import { renderHook } from '@testing-library/react-hooks'; +import { DeepPartial } from 'ts-essentials'; +import TestRouter from '__tests__/util/TestRouter'; import { GqlMockedProvider } from '__tests__/util/graphqlMocking'; import { mockSession } from '__tests__/util/mockSession'; import { GetUserQuery } from 'src/components/User/GetUser.generated'; import { UsStaffGroupEnum, UserTypeEnum } from 'src/graphql/types.generated'; +import { NewStaffQuestionnaireStatusQuery } from './NewStaffQuestionnaireStatus.generated'; import { UserOptionQuery } from './UserPreference.generated'; import { useHrToolsNavItems } from './useHrToolsNavItems'; @@ -13,6 +16,7 @@ const ineligibleUser = { usStaffGroup: UsStaffGroupEnum.PartTimeFieldStaff, spouseUsStaffGroup: UsStaffGroupEnum.PartTimeFieldStaff, staffAccountId: null, + canViewNewStaffCohorts: false, }; // A Senior Staff user with a staff account, eligible for the MPD Goal tools @@ -21,26 +25,43 @@ const mpdGoalEligibleUser = { usStaffGroup: UsStaffGroupEnum.SeniorStaff, spouseUsStaffGroup: UsStaffGroupEnum.SeniorStaff, staffAccountId: 'staff-account-1', + canViewNewStaffCohorts: true, }; -const Wrapper = ({ children }: { children: ReactElement }) => ( - - mocks={{ GetUser: { user: ineligibleUser } }} - > - {children} - -); - -const MpdGoalEligibleWrapper = ({ children }: { children: ReactElement }) => ( - - mocks={{ - GetUser: { user: mpdGoalEligibleUser }, - UserOption: { userOption: { key: 'user_type_verified', value: 'true' } }, - }} - > - {children} - -); +// A New Staff user, the only group offered the NSO MPD Questionnaire +const newStaffUser = { + userType: UserTypeEnum.UsStaff, + usStaffGroup: UsStaffGroupEnum.NewStaff, + spouseUsStaffGroup: UsStaffGroupEnum.NewStaff, + staffAccountId: 'staff-account-1', + canViewNewStaffCohorts: false, +}; + +type Mocks = { + GetUser: GetUserQuery; + UserOption: UserOptionQuery; + NewStaffQuestionnaireStatus: NewStaffQuestionnaireStatusQuery; +}; + +const verifiedUserOption = { + userOption: { key: 'user_type_verified', value: 'true' }, +}; + +const makeWrapper = (mocks: DeepPartial) => { + const MocksWrapper = ({ children }: { children: ReactElement }) => ( + + mocks={mocks}>{children} + + ); + return MocksWrapper; +}; + +const Wrapper = makeWrapper({ GetUser: { user: ineligibleUser } }); + +const MpdGoalEligibleWrapper = makeWrapper({ + GetUser: { user: mpdGoalEligibleUser }, + UserOption: verifiedUserOption, +}); describe('useHrToolsNavItems', () => { afterEach(() => { @@ -159,4 +180,94 @@ describe('useHrToolsNavItems', () => { expect(result.current.items).toHaveLength(1); expect(result.current.items[0].id).toBe('partnerReminders'); }); + + describe('mpdGoalAdmin', () => { + const renderWithUser = (user: typeof mpdGoalEligibleUser) => + renderHook(() => useHrToolsNavItems(), { + wrapper: makeWrapper({ + GetUser: { user }, + UserOption: verifiedUserOption, + }), + }); + + beforeEach(() => { + mockSession({ developer: false }); + }); + + it('is hidden from a Senior Staff user without goals-team or coordinator access', async () => { + const { result, waitForNextUpdate } = renderWithUser({ + ...mpdGoalEligibleUser, + canViewNewStaffCohorts: false, + }); + await waitForNextUpdate(); + + const ids = result.current.items.map((item) => item.id); + expect(ids).not.toContain('mpdGoalAdmin'); + // The tools that really are group-gated are untouched + expect(ids).toContain('goalCalculator'); + }); + + it('is shown to a coordinator whose own group is ineligible for the MPD Goal tools', async () => { + const { result, waitForNextUpdate } = renderWithUser({ + ...newStaffUser, + canViewNewStaffCohorts: true, + }); + await waitForNextUpdate(); + + const ids = result.current.items.map((item) => item.id); + expect(ids).toContain('mpdGoalAdmin'); + expect(ids).not.toContain('goalCalculator'); + }); + }); + + describe('nsoMpdQuestionnaire', () => { + const renderWithQuestionnaire = ( + newStaffQuestionnaire: NewStaffQuestionnaireStatusQuery['newStaffQuestionnaire'], + ) => + renderHook(() => useHrToolsNavItems(), { + wrapper: makeWrapper({ + GetUser: { user: newStaffUser }, + UserOption: verifiedUserOption, + NewStaffQuestionnaireStatus: { newStaffQuestionnaire }, + }), + }); + + beforeEach(() => { + mockSession({ developer: false }); + }); + + it('is shown to new staff who still have one to fill in', async () => { + const { result, waitForNextUpdate } = renderWithQuestionnaire({ + id: 'questionnaire-1', + completed: false, + }); + await waitForNextUpdate(); + + expect(result.current.items.map((item) => item.id)).toContain( + 'nsoMpdQuestionnaire', + ); + }); + + it('is hidden once new staff have completed it', async () => { + const { result, waitForNextUpdate } = renderWithQuestionnaire({ + id: 'questionnaire-1', + completed: true, + }); + await waitForNextUpdate(); + + const ids = result.current.items.map((item) => item.id); + expect(ids).not.toContain('nsoMpdQuestionnaire'); + // The sibling New Staff tool, gated only by group, stays visible + expect(ids).toContain('nsGoalCalculator'); + }); + + it('is hidden when new staff have no questionnaire at all', async () => { + const { result, waitForNextUpdate } = renderWithQuestionnaire(null); + await waitForNextUpdate(); + + expect(result.current.items.map((item) => item.id)).not.toContain( + 'nsoMpdQuestionnaire', + ); + }); + }); }); diff --git a/src/hooks/useHrToolsNavItems.ts b/src/hooks/useHrToolsNavItems.ts index 9bec359d6f..475e24dad4 100644 --- a/src/hooks/useHrToolsNavItems.ts +++ b/src/hooks/useHrToolsNavItems.ts @@ -1,5 +1,7 @@ import { useMemo } from 'react'; import { useTranslation } from 'react-i18next'; +import { useNewStaffQuestionnaireStatusQuery } from './NewStaffQuestionnaireStatus.generated'; +import { useAccountListId } from './useAccountListId'; import { useDeveloperBypass } from './useDeveloperBypass'; import { useIneligibleByGroup } from './useIneligibleByGroup'; import { NavItems } from './useReportNavItems'; @@ -18,15 +20,28 @@ export function useHrToolsNavItems(): { inNsGoalCalcIneligibleGroup, inPdsGoalCalcIneligibleGroup, inMpdSupervisorIneligibleGroup, + canViewNewStaffCohorts, hasNoStaffAccount, userLoading, } = useIneligibleByGroup(); const developerBypass = useDeveloperBypass(); // Partner Reminders is live in production; every other HR Tool is still disabled const { reportsDisabled } = useReportsDisabled(); + const accountListId = useAccountListId(); + + // Only new staff are ever offered the questionnaire, so nobody else pays for this query. + const { data: questionnaireData, loading: questionnaireLoading } = + useNewStaffQuestionnaireStatusQuery({ + variables: { accountListId }, + skip: userLoading || inNsGoalCalcIneligibleGroup, + }); + const questionnaire = questionnaireData?.newStaffQuestionnaire; + const hasQuestionnaireToFillIn = !!questionnaire && !questionnaire.completed; + + const loading = userLoading || questionnaireLoading; const items = useMemo(() => { - if (userLoading) { + if (loading) { return []; } @@ -55,7 +70,8 @@ export function useHrToolsNavItems(): { hideItem: reportsDisabled || process.env.DISABLE_NS_GOAL_CALCULATOR === 'true' || - inNsGoalCalcIneligibleGroup, + inNsGoalCalcIneligibleGroup || + !hasQuestionnaireToFillIn, }, { id: 'goalCalculator', @@ -68,7 +84,7 @@ export function useHrToolsNavItems(): { hideItem: reportsDisabled || process.env.DISABLE_MPD_GOAL_ADMIN === 'true' || - inMpdGoalCalcIneligibleGroup, + !canViewNewStaffCohorts, }, { id: 'mhaCalculator', @@ -106,11 +122,13 @@ export function useHrToolsNavItems(): { inNsGoalCalcIneligibleGroup, inPdsGoalCalcIneligibleGroup, inMpdSupervisorIneligibleGroup, - userLoading, + canViewNewStaffCohorts, + hasQuestionnaireToFillIn, + loading, hasNoStaffAccount, developerBypass, reportsDisabled, ]); - return { items, loading: userLoading }; + return { items, loading }; } diff --git a/src/hooks/useIneligibleByGroup.ts b/src/hooks/useIneligibleByGroup.ts index ae45f58ac6..9f3efaccf9 100644 --- a/src/hooks/useIneligibleByGroup.ts +++ b/src/hooks/useIneligibleByGroup.ts @@ -33,6 +33,8 @@ export function useIneligibleByGroup() { const userType = data?.user.userType; const hasNoStaffAccount = !data?.user.staffAccountId; const supervisesStaff = data?.user.supervisesStaff; + // A role rather than a group: the MPD Goals team and MPD coordinators only. + const canViewNewStaffCohorts = !!data?.user.canViewNewStaffCohorts; const { SeniorStaff, NewStaff, NationalExpat, PaidWithDesignation } = UsStaffGroupEnum; @@ -71,6 +73,7 @@ export function useIneligibleByGroup() { inNsGoalCalcIneligibleGroup, inPdsGoalCalcIneligibleGroup, inMpdSupervisorIneligibleGroup, + canViewNewStaffCohorts, userType, hasNoStaffAccount, userLoading, @@ -84,6 +87,7 @@ export function useIneligibleByGroup() { inNsGoalCalcIneligibleGroup, inPdsGoalCalcIneligibleGroup, inMpdSupervisorIneligibleGroup, + canViewNewStaffCohorts, userType, hasNoStaffAccount, userLoading, From 828a82933a907530d243831576877f0c862c23b7 Mon Sep 17 00:00:00 2001 From: wjames111 Date: Mon, 14 Sep 2026 17:03:03 -0400 Subject: [PATCH 2/2] fix: address review comments on src/hooks/useHrToolsNavItems.ts Co-Authored-By: Claude Opus 5 --- src/hooks/useHrToolsNavItems.test.tsx | 21 +++++++++++++++++++ src/hooks/useHrToolsNavItems.ts | 29 ++++++++++++++++----------- 2 files changed, 38 insertions(+), 12 deletions(-) diff --git a/src/hooks/useHrToolsNavItems.test.tsx b/src/hooks/useHrToolsNavItems.test.tsx index e6c99020cf..a0ed6dfae2 100644 --- a/src/hooks/useHrToolsNavItems.test.tsx +++ b/src/hooks/useHrToolsNavItems.test.tsx @@ -1,4 +1,5 @@ import { ReactElement } from 'react'; +import { waitFor } from '@testing-library/dom'; import { renderHook } from '@testing-library/react-hooks'; import { DeepPartial } from 'ts-essentials'; import TestRouter from '__tests__/util/TestRouter'; @@ -269,5 +270,25 @@ describe('useHrToolsNavItems', () => { 'nsoMpdQuestionnaire', ); }); + + it('stays visible for new staff when the status query fails', async () => { + const { result } = renderHook(() => useHrToolsNavItems(), { + wrapper: makeWrapper({ + GetUser: { user: newStaffUser }, + UserOption: verifiedUserOption, + NewStaffQuestionnaireStatus: { + newStaffQuestionnaire: () => { + throw new Error('Not authorized'); + }, + }, + } as unknown as DeepPartial), + }); + + await waitFor(() => + expect(result.current.items.map((item) => item.id)).toContain( + 'nsoMpdQuestionnaire', + ), + ); + }); }); }); diff --git a/src/hooks/useHrToolsNavItems.ts b/src/hooks/useHrToolsNavItems.ts index 475e24dad4..21c136314b 100644 --- a/src/hooks/useHrToolsNavItems.ts +++ b/src/hooks/useHrToolsNavItems.ts @@ -30,18 +30,23 @@ export function useHrToolsNavItems(): { const accountListId = useAccountListId(); // Only new staff are ever offered the questionnaire, so nobody else pays for this query. - const { data: questionnaireData, loading: questionnaireLoading } = - useNewStaffQuestionnaireStatusQuery({ - variables: { accountListId }, - skip: userLoading || inNsGoalCalcIneligibleGroup, - }); + const { + data: questionnaireData, + loading: questionnaireLoading, + error: questionnaireError, + } = useNewStaffQuestionnaireStatusQuery({ + variables: { accountListId }, + skip: userLoading || inNsGoalCalcIneligibleGroup, + }); const questionnaire = questionnaireData?.newStaffQuestionnaire; const hasQuestionnaireToFillIn = !!questionnaire && !questionnaire.completed; - - const loading = userLoading || questionnaireLoading; + // A failed query must not hide the path to paperwork new staff still owe + const hideQuestionnaire = questionnaireError + ? false + : questionnaireLoading || !hasQuestionnaireToFillIn; const items = useMemo(() => { - if (loading) { + if (userLoading) { return []; } @@ -71,7 +76,7 @@ export function useHrToolsNavItems(): { reportsDisabled || process.env.DISABLE_NS_GOAL_CALCULATOR === 'true' || inNsGoalCalcIneligibleGroup || - !hasQuestionnaireToFillIn, + hideQuestionnaire, }, { id: 'goalCalculator', @@ -123,12 +128,12 @@ export function useHrToolsNavItems(): { inPdsGoalCalcIneligibleGroup, inMpdSupervisorIneligibleGroup, canViewNewStaffCohorts, - hasQuestionnaireToFillIn, - loading, + hideQuestionnaire, + userLoading, hasNoStaffAccount, developerBypass, reportsDisabled, ]); - return { items, loading }; + return { items, loading: userLoading }; }