MPDX-9613 Gate MPD and PDS Goal Calculators by peopleGroupSupportType

[wjames111]

Description

• Restrict the MPD Goal Calculator to US Staff whose peopleGroupSupportType is SupportedRmo (senior staff).
• Restrict the PDS Goal Calculator to US Staff whose peopleGroupSupportType is Designation.
• Add MpdGoalCalc and PdsGoalCalc values to RequiredUserGroupEnum and extend useUsStaffGroups with matching inMpdGoalCalcIneligibleGroup / inPdsGoalCalcIneligibleGroup flags.
• Wrap both Goal Calculator pages in UserTypeAccess so ineligible users see the "Access to this feature is limited." page.
• Hide each Goal Calculator entry from the HR Tools nav for ineligible users.
• Eligibility for Goal Calculators is evaluated against the logged-in user (not spouse), consistent with the Salary Calculator check.

Jira: MPDX-9613

Testing

• Go to HR Tools as a US Staff user whose peopleGroupSupportType is SupportedRmo
• Check that the MPD Goal Calculator nav item is visible and the page loads normally
• Check that the Paid with Designation Support Goal Calculator nav item is hidden, and visiting its URL directly shows the limited-access page
• Sign in as a US Staff user whose peopleGroupSupportType is Designation
• Check that the Paid with Designation Support Goal Calculator nav item is visible and the page loads normally
• Check that the MPD Goal Calculator nav item is hidden, and visiting its URL directly shows the limited-access page
• Sign in as a Non-Cru user
• Check that both Goal Calculator pages show the limited-access page

Checklist:

• I have given my PR a title with the format "MPDX-(JIRA#) (summary sentence max 80 chars)"
• I have applied the appropriate labels (Add the label "Preview" to automatically create a preview environment)
• I have run the Claude Code /pr-review command locally and fixed any relevant suggestions
• I have requested a review from another person on the project
• I have tested my changes in preview or in staging
• I have cleaned up my commit history

[github-actions]

Bundle sizes [mpdx-react]

Compared against 01f7baf

Route	Size (gzipped)	Diff
/accountLists/[accountListId]/hrTools/pdsGoalCalculator	162.89 KB	+2.19 KB
/accountLists/[accountListId]/hrTools/pdsGoalCalculator/[pdsGoalId]	336.21 KB	+4.49 KB

[wjames111]

Multi-Agent Code Review

Verdict: APPROVED WITH SUGGESTIONS — no merge blockers. Several Important and Medium findings recommended for follow-up.

Risk: HIGH (7/10) — modifies a shared gating component (UserTypeAccess, 15 dependents) and a hook consumed by multiple HR Tools pages.

Agents: Security · Architecture · Testing · UX · Standards (debated, with rebuttals)

---

Top 3 Findings

1. PDS detail page is not gated at all (severity 7.5, files NOT in this PR diff — see "Related Files" section)
2. Null vs undefined peopleGroupSupportType creates fail-open / asymmetric behavior (severity 7.0, useUsStaffGroups.ts:58)
3. MPD/PDS goal-calc nav items flash visible during HCM load (severity 6.5, useHrToolsNavItems.ts)

These are not blockers — the PR's stated objective (gating list pages by support type) is correctly implemented and tested. Findings are improvements to make the gating consistent and robust.

---

Cross-Cutting Consistency Check — Detail-Page Gating Gap

The PR title commits to gating MPD and PDS Goal Calculators by peopleGroupSupportType. Verified the gate is applied at the list page level only:

Page	Auth	Support-type gate
goalCalculator/index.page.tsx (list)	UsStaff	✅ MpdGoalCalc
pdsGoalCalculator/index.page.tsx (list)	UsStaff	✅ PdsGoalCalc
goalCalculator/[goalCalculationId].page.tsx (detail)	UsStaff (no requireUserGroups)	❌ MISSING
pdsGoalCalculator/[pdsGoalId].page.tsx (detail)	❌ NO UserTypeAccess wrapper	❌ MISSING

For comparison, salaryCalculator/[calculationId]/index.page.tsx:62 and mhaCalculator/[requestId].page.tsx:69 both pass requireUserGroups={...} to their detail page wrappers — that pattern is established in this repo. A user with an ineligible peopleGroupSupportType who has a valid goalCalculationId / pdsGoalId (bookmark, shared link, browser history) can deep-link directly to the detail page and bypass the new gate. Server-side authorization on the upstream MPDX API should be the ultimate defense, but UI parity with the existing pattern is the highest-leverage UI fix.

---

Findings on Related Files (Not in This PR)

These files were identified as having gating-gap issues directly caused by this PR's design choices, but the files themselves are not modified by this PR. They cannot be posted as line comments.

[Important] pages/accountLists/[accountListId]/hrTools/pdsGoalCalculator/[pdsGoalId].page.tsx — No UserTypeAccess wrapper at all

• Severity: 7.5/10
• Flagged by: Architecture, Security
• Problem: The PDS detail page has no UserTypeAccess wrapper. After this PR ships, an ineligible US Staff user (or even a non-US-Staff user) can deep-link a pdsGoalId and reach the detail page directly.
• Recommended Action: Wrap with <UserTypeAccess requireUserGroups={RequiredUserGroupEnum.PdsGoalCalc}> to match the new list-page gating.

[Medium] pages/accountLists/[accountListId]/hrTools/goalCalculator/[goalCalculationId].page.tsx:142 — UserTypeAccess is missing requireUserGroups

• Severity: 6.5/10
• Flagged by: Architecture, Security
• Problem: The MPD detail page wraps in <UserTypeAccess> with no requireUserGroups — so it only checks userType === UsStaff, not the support-type gate. Pre-existing inconsistency, but in scope to fix given this PR's stated goal.
• Recommended Action: Change to <UserTypeAccess requireUserGroups={RequiredUserGroupEnum.MpdGoalCalc}>.

---

Other Notes (Not Inline)

[Suggestion] Server-side enforcement of peopleGroupSupportType is unverified (severity 6.5, Security)

• UserTypeAccess is a UI-only gate. Per .claude/rules/code-review.md Security Focus Areas: "every Yup rule, every disabled button, every required field must have a server-side equivalent." The upstream MPDX GraphQL API for goalCalculations/designationSupportCalculations and their mutations should enforce peopleGroupSupportType independently. Not visible in this repo — verify with the API team. Same posture already exists for ASR/Salary/MHA, so this is not a regression.

[Suggestion] LimitedAccess copy is misleading when peopleGroupSupportType is null/None (severity 4.5, UX, pre-existing)

• A US Staff user whose peopleGroupSupportType is genuinely None or null sees "Our records show that you are not part of the user group… contact support@mpdx.org to change your user group" (src/components/Shared/LimitedAccess/getLimitedText.tsx:42-51). The actionable fix is with HR/HCM data ownership, not MPDX support. Pre-existing copy.

[Suggestion] Senior staff landing on PDS page has no signposting to MPD calc (severity 5.5, UX)

• A SupportedRmo user navigating to PDS (or Designation user to MPD) hits a generic "limited access" wall with only a "Back to Dashboard" CTA. The nav menu correctly hides the wrong calculator, so they have a way forward — but the page itself doesn't signpost. Out of scope for this PR.

[Pre-existing] PdsGoalCalculatorPage uses export default (severity 2.0, Standards)

• Next.js Pages Router requires default export. This is a project-wide carve-out from CLAUDE.md's "named exports only." Not introduced by this PR.

---

Standards Compliance — All Pass

• Named exports for new code, file naming, i18n via t(), generated GraphQL types used, typed GqlMockedProvider, findBy* for async assertions — all PASS.
• yarn lint:ci and yarn lint:ts clean on changed files.
• No any, no debug output, no new Date(), no commented code.

---

To dismiss a non-blocking suggestion (severity < 7), reply /dismiss: <reason> on the comment.

[wjames111]

[Important] **Missing tests for `peopleGroupSupportType: null` and `undefined`.**

The new Goal Calculator eligibility describe block covers SupportedRmo, Designation, SupportedNonRmo, None, and a spouse case — but not the two nullish cases that drive the asymmetric fail-open behavior (see comment on useUsStaffGroups.ts:58):

• peopleGroupSupportType: undefined (field omitted) — currently treated as eligible for both calcs (fail-open)
• peopleGroupSupportType: null (explicit null from server) — currently treated as ineligible for both calcs (fail-closed)

Whichever contract the team intends, add tests to lock it in. UX recommends the test also assert the rendered body copy of LimitedAccess so the misleading "contact support@mpdx.org" message doesn't silently survive a refactor.

Suggested additions:

it('marks user as ineligible for both goal calcs when peopleGroupSupportType is null', async () => {
  const { result } = renderUseUsStaffGroups(
    buildHcmMock({
      peopleGroupSupportType: null as unknown as PeopleGroupSupportTypeEnum,
    }),
  );
  await waitFor(() => {
    expect(result.current.inMpdGoalCalcIneligibleGroup).toBe(true);
    expect(result.current.inPdsGoalCalcIneligibleGroup).toBe(true);
  });
});

it('treats peopleGroupSupportType undefined as <chosen behavior>', async () => {
  const { result } = renderUseUsStaffGroups({
    hcm: [{ asrEit: { asrEligibility: true }, salaryRequestEligible: true, mhaEit: { mhaEligibility: true }, staffInfo: { /* peopleGroupSupportType intentionally omitted */ } }],
  } as HcmQuery);
  await waitFor(() => {
    // Pin to the chosen behavior
  });
});

Note: Do not write tests against the current fail-open behavior if you plan to also adopt the architectural fix on useUsStaffGroups.ts:58 — write tests against the fixed behavior.

[wjames111]

Not sure what this should default to.

[github-actions]

Preview branch generated at https://MPDX-9613.d3dytjb8adxkk5.amplifyapp.com

[kegrimes]

Thank you for doing this Will, it looks amazing! I did have a few suggestions and found a couple bugs.

Also just to let you know, I have already updated the backend to cache all possible US staff groups (senior staff, new staff, PTFS, paid with designation, and interns) into the User record. This will eliminate the extra call we are making to HCM so, a lot of this work will be refactored! But, thank you for adding MPD and PDS calculators to the list! That is one less thing I need to do.

Would you mind adding MPD and PDS goal calculators to this document with the groups who are eligible for them? This is just so we can keep track of who is eligible for what!

https://docs.google.com/document/d/1OWkY1iDCE-DGlVuYMYpLcvtPucmjD-j1wQt9AsQxxIs/edit?tab=t.0

[kegrimes]

If senior staff are the only ones who are eligible for MPD goal calculator, I don't think this check is correct. The status codes for senior staff are...

• SC - Staff Full time
  Employee - Staff, Supported - RMO, Active - Payroll Eligible, Full-time regular
• SCO - Staff Full time only
  Employee - Staff non-RMO Spouse, Supported - RMO, Active - Payroll Eligible, Full-time regular

...but an intern can also be Supported - RMO (if you impersonate nataley.guetherman@cru.org from this excel sheet, you will be able to see MPD)

For MHA, ASR, and Salary calc, I have the eligibility validators checking the fields instead of doing it like this on the frontend. Is there an eligibility field for MPD (and even PDS)? If there is, I would use that field instead and verify that it is for a senior staff only (you can check Salary calc in the backend for reference - senior staff only).

But that might need to be another PR for the backend if those eligibility checks are not correct for MPD and PDS. If that is the case, then I would keep these checks on the frontend but fix senior staff to only be those status codes listed above. Senior staff is the one group that needs all 4 fields to check. You can look at the status codes here to verify)

[kegrimes]

@wjames111 Now that I am thinking about it, I don't think the PDS calculator requires a staff account id because paid with designation staff members do not have one at all.

[wjames111]

@kegrimes Okay so I made the changes you requested. Also inMpdGoalCalcIneligibleGroup now uses salaryRequestEligible and I hardcoded inPdsGoalCalcIneligibleGroup as true, until we can get a backend PR up that adds a PDS Goal Calculator eligibility validator. However in order to create that backend work we'll need to resolve them needing a staff account Id. Does that sounds right?

[kegrimes]

We are almost there! Just a couple things. I want to be able to test the HR Tools tab not showing up when the user does not have access to any of those reports! The new Alithya change affected that, so we need to change that.

[kegrimes]

I also reached out to Daniel to see if he could merge his PR to unblock us for the backend work!

[Update]: It has been merged to production!

[kegrimes]

Great work Will!

I have one more bug fix for you, but other than that, this PR is good to go so I am going to approve. Thank you for all your hard work! It will be easier for me to update the frontend customized work with MPD and PDS set up!

[kegrimes]

I actually think we need to separate these errors. I was impersonating myself and it was still allowing me to see the MPD goal calculator even though I did not have any HCM data. I think we need to do something like this:

  const hasNoStaffAccount =
    error?.graphQLErrors.some(
      (graphQLError) => graphQLError.extensions?.code === 'NO_STAFF_ACCOUNT',
    ) ?? false;

  const hcmPersonNotFound =
    error?.graphQLErrors.some(
      (graphQLError) =>
        graphQLError.extensions?.code === 'HCM_PERSON_NOT_FOUND',
    ) ?? false;

  const hasNoHcmData = hasNoStaffAccount || hcmPersonNotFound;

  const inAsrIneligibleGroup = hasNoHcmData || allAsrIneligible;
  const inSalaryCalcIneligibleGroup =
    hasNoHcmData || user?.salaryRequestEligible === false;
  const inMhaIneligibleGroup = hasNoHcmData || allMhaIneligible;
  const inMpdGoalCalcIneligibleGroup =
    hcmPersonNotFound || user?.salaryRequestEligible === false;
  const inPdsGoalCalcIneligibleGroup =
    hcmPersonNotFound || allPdsGoalCalcIneligible;

I already tested all 5 scenarios and they should all be working with this logic. MPD and PDS calculators don't require a staff account id, so even without one HCM still gets called (we have a person number) and can come back with HCM_PERSON_NOT_FOUND. We only want to check that error against these 2 calculators. NO_STAFF_ACCOUNT just means no account id exists, and since these reports don't require one, we don't want that error to hide them. ASR, Salary, and MHA do require a staff account id, so they hide on either error (hasNoHcmData).

So the distinction is:

• NO_STAFF_ACCOUNT = no id exists (raised before HCM hit)
• HCM_PERSON_NOT_FOUND = HCM data was reached, but there is no data

[kegrimes]

If you are wanting to test these for yourself, kathryn.hougham@cru.org is the only account that will work for paid with designation users. She has her person number set up correctly in staging!