MPDX-9813 - NSO MPD Questionnaire Spouse Phone Number input by zweatshirt · Pull Request #1879 · CruGlobal/mpdx-react

zweatshirt · 2026-07-02T14:29:35Z

Description

Adds a single row table for collecting Staff and Spouse phone numbers in ContactInformation
Makes the repeated hasSpouse logic a shared function and variable in the context.

Testing

Go to NSO Questionnaire
Test that the Contact Information section works as expected and that both staff and spouse phone numbers are required if married
Check that Continue is blocked until both inputs are filled
Check that error handling works correctly
Check that this step works for single, married, SOSA, and senior spouse

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 · 2026-07-02T14:33:15Z

Bundle sizes [mpdx-react]

Compared against 837d6e6

No significant changes found

github-actions · 2026-07-02T14:33:59Z

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

zweatshirt

Multi-Agent Code Review — Verdict: CLEAN

6 specialized agents (Architecture, Testing, Standards, UX, Data Integrity, Financial Reporting) plus a focused delta pass on the new test. 0 blockers. The change reduces technical debt (centralized getHasSpouse, extracted a reusable PhoneNumberField, and converted a pre-existing sx-callback to useTheme()).

The one Medium finding from the initial pass — the untested single-staff stepCompletion branch — is resolved by stepCompletion.test.ts (verified: genuinely typed fixture, exercises both single→100% and married 75%→100%).

Everything remaining is an optional suggestion (severity < 5), posted inline. None block merge and none require /dismiss.

Risk: MEDIUM. The repo's flat per-file scoring mechanically rated this higher, but this is a shallow, fully-tested, single-feature addition with no auth / data-model / money / migration surface; the GraphQL change is a single scalar added to an existing fragment (id intact).

Severity	Count
Blockers (>= 8)	0
Important (7–7.9)	0
Medium (5–6.9)	0
Suggestions (< 5)	8

zweatshirt · 2026-07-02T17:02:50Z

-    schema,
-  });
+  const userName = firstName ?? t('You');
+  const spouseColumnName = spouseFirstName ?? t('Spouse');


[Suggestion] (Testing) The `You` / `Spouse` / `your spouse` fallbacks (when `firstName`/`spouseFirstName` are null) are never exercised — the shared mock always sets names. Consider a `firstName: null, spouseFirstName: null` test asserting the fallback column/aria-label names.

zweatshirt · 2026-07-02T17:02:50Z

+          ? t('Please provide a cell phone number for each person below.')
+          : t('Please provide your cell phone number.')}
+      </Typography>
+      <Table size="small" sx={{ maxWidth: theme.spacing(90) }}>


[Suggestion] (UX) Fixed-width table (`maxWidth ~720px`) with two person columns, each holding an input capped at ~320px, has no explicit small-screen handling. Matches the SalaryCalculator reference (consistent, not a regression) — worth a manual check at `xs`.

zweatshirt · 2026-07-02T17:02:50Z

+            <TableCell scope="col" sx={{ color: theme.palette.mpdxBlue.main }}>
+              {userName}
+            </TableCell>
+            {hasSpouse && (


[Suggestion] (UX) Before the questionnaire loads, `hasSpouse` is false, so a married user briefly sees a single column before the spouse column appears. Consistent with existing load/seed behavior; acceptable.

zweatshirt · 2026-07-02T17:02:50Z

                  label: t('Spouse name'),
-                  value: formatText(spouseFirstName),
+                  value:
+                    [spouseFirstName, lastName].filter(Boolean).join(' ') ||


[Suggestion] (Data Integrity / Testing) The Spouse name row now renders `[spouseFirstName, lastName]`, assuming the spouse shares the staff member's last name — a product assumption, not a bug. Also, no test asserts the rendered spouse full-name value (`Jane Doe`); existing tests only check row presence/absence. Consider a `getByRole('cell', { name: 'Jane Doe' })` assertion.

github-actions

AI Review Auto-Approval

Risk Level: MEDIUM (5/10)
Verdict: CLEAN (no issues found)

This PR was auto-approved because:

The multi-agent AI review determined it is medium risk
No blocking issues were found

If you believe this PR needs human review, dismiss this approval and request a review manually.

Extract the marital-status check into a shared getHasSpouse helper and expose hasSpouse from NsoMpdQuestionnaireContext, so consumers stop recomputing it. StaffInformation now reads hasSpouse from context.

Add spousePhoneNumber to the questionnaire fragment and a reusable PhoneNumberField, and render a per-person phone column in ContactInformation (user + spouse) with person-scoped labels for screen readers. Require the spouse phone for step completion when married, and show it in the Summary.

zweatshirt

Multi-Agent Code Review — Verdict: CLEAN

Re-review after the per-person-label change. Fresh UX, Testing, and Standards agents. 0 blockers.

This change resolves both Medium findings from the prior review:

Duplicate accessible name → resolved. Each phone input now has a distinct, person-scoped accessible name equal to its visible label ("John's cell phone number" / "Jane's cell phone number"). WCAG 2.5.3 satisfied (no aria-label override), matches the SalaryCalculator per-person-label convention, and the required asterisk is preserved.
Vacuous test → resolved. Tests now query each field by its distinct name (getByRole/findByRole), which throw if a field is missing — the "two fields exist" guarantee is genuinely restored.

Standards: i18n interpolation keys are correct (variable form, static keys, double-quote-wrapped possessives — extraction-verified against existing en-locale precedent); no any, named exports, test conventions upheld.

Severity	Count
Blockers (>= 8)	0
Important (7–7.9)	0
Medium (5–6.9)	0
Suggestions (< 5)	2

Both remaining items are optional suggestions (severity < 5), posted inline — neither blocks merge.

Risk: MEDIUM — a shallow, fully-tested, single-feature change with no auth / data-model / money / migration surface.

zweatshirt · 2026-07-02T18:29:38Z

-    fieldName: 'phoneNumber',
-    schema,
-  });
+  const userLabel = firstName


[Suggestion] (Testing) The null-name fallback branches introduced here are untested: `userLabel` falls back to "Your cell phone number" when `firstName` is null, and the spouse label falls back to "your spouse's cell phone number" (`spouseName = spouseFirstName ?? t('your spouse')`). Consider a `firstName: null, spouseFirstName: null` test case to cover them. Low priority — `firstName` is HCM-prepopulated, so realistically always present.

zweatshirt · 2026-07-02T18:29:38Z

+              {t('Cell Phone Number')}
+            </TableCell>
+            <TableCell>
+              <PhoneNumberField fieldName="phoneNumber" label={userLabel} />


[Suggestion] (UX) The person's name now appears in both the column header and the field label. This is intentional and correct for accessibility — the column header isn't programmatically associated with the input, so the per-person label is what disambiguates the two fields for screen readers in forms-navigation mode. Keep as-is; noted only for awareness.

github-actions

AI Review Auto-Approval

Risk Level: MEDIUM (5/10)
Verdict: CLEAN (no issues found)

This PR was auto-approved because:

The multi-agent AI review determined it is medium risk
No blocking issues were found

If you believe this PR needs human review, dismiss this approval and request a review manually.

zweatshirt self-assigned this Jul 2, 2026

zweatshirt added the Preview Environment Add this label to create an Amplify Preview label Jul 2, 2026

zweatshirt marked this pull request as ready for review July 2, 2026 14:33

zweatshirt commented Jul 2, 2026

View reviewed changes

github-actions Bot approved these changes Jul 2, 2026

View reviewed changes

zweatshirt added 2 commits July 2, 2026 12:57

Centralize hasSpouse derivation in the NSO questionnaire context

bb08ac2

Extract the marital-status check into a shared getHasSpouse helper and expose hasSpouse from NsoMpdQuestionnaireContext, so consumers stop recomputing it. StaffInformation now reads hasSpouse from context.

zweatshirt force-pushed the MPDX-9813 branch from e650681 to 9d33ee2 Compare July 2, 2026 17:58

zweatshirt commented Jul 2, 2026

View reviewed changes

github-actions Bot approved these changes Jul 2, 2026

View reviewed changes

zweatshirt merged commit 5e57129 into main Jul 2, 2026
23 of 24 checks passed

zweatshirt deleted the MPDX-9813 branch July 2, 2026 18:34

Uh oh!

Conversation

zweatshirt commented Jul 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Testing

Checklist:

Uh oh!

github-actions Bot commented Jul 2, 2026

Bundle sizes [mpdx-react]

Uh oh!

github-actions Bot commented Jul 2, 2026

Uh oh!

zweatshirt left a comment

Choose a reason for hiding this comment

Multi-Agent Code Review — Verdict: CLEAN

Uh oh!

Uh oh!

Uh oh!

zweatshirt Jul 2, 2026

Choose a reason for hiding this comment

Uh oh!

zweatshirt Jul 2, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

zweatshirt Jul 2, 2026

Choose a reason for hiding this comment

Uh oh!

zweatshirt Jul 2, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

github-actions Bot left a comment

Choose a reason for hiding this comment

AI Review Auto-Approval

Uh oh!

zweatshirt left a comment

Choose a reason for hiding this comment

Multi-Agent Code Review — Verdict: CLEAN

Uh oh!

zweatshirt Jul 2, 2026

Choose a reason for hiding this comment

Uh oh!

zweatshirt Jul 2, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot left a comment

Choose a reason for hiding this comment

AI Review Auto-Approval

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

zweatshirt commented Jul 2, 2026 •

edited

Loading