MPDX-9919 Add geographic location to account preferences

[kegrimes]

Description

We want to add geographic location to account preferences so the MPD supervisor report can grab these locations easily. Anytime a calculator submits and changes to a different geographic location, we will update the account preference to reflect the update.

Solution:

• Add geographic location to user's account settings under monthly goal
• Update geographic location preference whenever MPD goal calculator, salary calculator, PDS calculator, or NSO calculator is submitted with a new location
• Add information alert boxes to salary calculator and NSO questionnaire on submit modals
• Add information alert boxes under geographic location select/autocompletes on MPD and PDS since there is no modal that appears when applying goal to MPDX
• Put "apply goal to mpdx" logic into a shared hook for PDS and MPD goal calculators

Double check this is okay to go into production before the reports go-live

Note: We need to make the change for the NS goal calculator, however, admins/coaches are not allowed to update account list settings for new staff members. I need to update this server-side and then make changes in a follow-up PR

Testing

Account settings test: ✅

• Impersonate: anyone
• Go to /settings/preferences
• Open geographic location accordion and choose a location
• Check that it was saved successfully

Salary calculator test: ✅

• Impersonate: anyone eligible for salary calculator
• Go to /hrTools/salaryCalculator
• Set geographic location on "Your Information" step (the SAME location as preferences)
• Click "Submit" for modal to appear
• Check no disabled check box is present in the modal
• Go back and select a different geographic location & click "Submit"
• Check that an info alert box appears with message to update account preferences
• Submit salary calculator and check update was made in account preferences

MPD goal calculator test: ✅

• Impersonate: anyone eligible for goal calculator
• Go to /hrTools/goalCalculator
• Create a goal and set different geographical location
• Ensure there is an info alert box present under the select
• Click through steps and go to "Presenting Your Goal"
• Click "Apply Goal to MPDX"
• Check that location was updated in account preferences

PDS goal calculator test: ✅

• Impersonate: kathryn.hougham@cru.org
• Go to /hrTools/pdsGoalCalculator
• Create a goal and set different geographical location
• Ensure there is an info alert box present under the select
• Click through steps and click "Apply Goal to MPDX"
• Check that location was updated in account preferences

NSO questionnaire test: ✅

• Impersonate: timothy.ethington@cru.org
• Go to /hrTools/nsoMpdQuestionnaire
• Set SAME ministry location as location in preferences
• Click through steps & click "Submit"
• Check that no information about updating preferences in present
• Go back and set a different ministry location
• Click through steps & click "Submit"
• Check that an information alert appears telling user account settings will update
• Submit and ensure account preference was updated

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 /quality:agent-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]

Preview branch generated at https://mpdx-9919-add-geographic-location-to-preferences.d3dytjb8adxkk5.amplifyapp.com

[github-actions]

Bundle sizes [mpdx-react]

Compared against e02be0d

Route	Size (gzipped)	Diff
/accountLists/[accountListId]/hrTools/nsoMpdQuestionnaire	156.82 KB	+1.29 KB
/accountLists/[accountListId]/hrTools/salaryCalculator/[calculationId]	174.05 KB	+1.28 KB

[kegrimes]

Multi-Agent Code Review

7 specialized agents (Security, Architecture, Data Integrity, Testing, UX, Standards, Financial Reporting) + a dependency-impact analysis reviewed this PR in parallel, standard mode.

Verdict: Approved with suggestions

No hard blockers by the numeric formula, but three things genuinely warrant attention before merge:

1. Confirm server-side authorization on the one new write path that can target an account that isn't the current user's own (GoalSettingsForm.tsx, reachable from the coaching route) — this is exactly the kind of thing the PR description itself flagged ("double check this is okay to go into production").
2. Fix the mutation-sequencing bug in StepNavigation.tsx and Summary.tsx — confirmed independently by 5 agents — where a failed secondary write can make a successful primary submission look like it failed.
3. Two of the five tools (MPD GoalCalculator, PDS) silently change the location preference with no disclosure at all, while the other three show one — worth deciding deliberately rather than leaving as an accident of independent implementation.

Line comments below cover the concrete, single-location findings. A few structural findings don't anchor to one line and are summarized here instead:

• 3× duplicated read-compare-write logic across GoalSettingsForm.tsx, Summary.tsx, StepNavigation.tsx (Architecture, severity 6.5) — and this duplication is why the sequencing bug was reproduced independently in two places rather than fixed once. The repo already has the right precedent: src/hooks/useUserPreference.ts pairs exactly this read-compare-write-skip-if-unchanged pattern in one hook.
• Three HrTools domains now run the full page-scoped GetAccountPreferences query (Architecture 6.0, Data Integrity 4.0, Security 4.0) just to read one scalar — it also drags in an unpaginated accountLists { nodes } connection. A narrow AccountListGeographicLocation query would normalize into the same cache entry without the extra weight.
• 4 new translation strings were never extracted to public/locales/en/translation.json (Standards, severity 5.5) — this repo commits extracted keys as part of the feature PR (checked 4 prior examples). Run yarn extract.
• Coaching-route authorization (see the line comment on GoalSettingsForm.tsx) — Security found a concrete signal that writing into a coachee's account is normally avoided: CoachingLink.tsx exists specifically to hide a link into a coachee's preferences page. Whether the new write here is fine depends entirely on updateAccountList's server-side authorization, which isn't visible from this repo.

Investigated and disproven

Testing raised three "Important" (6.5-7.5) findings claiming the negative-mutation-assertion tests in StepNavigation.test.tsx, Summary.test.tsx, and GoalSettingsForm.test.tsx could pass even if their underlying guards were removed (a race between waitFor's resolution and the async mutation chain). I verified this directly rather than relying on reasoning: I temporarily removed each of the three guards in turn and re-ran the exact named tests.

• StepNavigation.test.tsx — guard removed → test failed (caught UpdateAccountPreferences with geographicLocation: null).
• GoalSettingsForm.test.tsx — equality check removed → test failed (caught the spurious sync).
• Summary.test.tsx — equality check removed → test failed (caught the disclosure Alert appearing).

All three tests are sound; waitFor's polling interval gives ample time for the guarded await to resolve before the synchronous negative assertion runs. These three findings are dismissed with concrete counter-evidence and don't count toward the verdict. All files were reverted to their original diff state afterward and the full suite re-passes.

Dependency impact

• Confirmation.tsx — 20 consumers app-wide. The only behavioral change (component="div") is confirmed fixing, not introducing, a pre-existing invalid-HTML risk.
• AccordionEnum.ts (PreferenceAccordion) — 13 consumers, purely additive enum member, no exhaustive switch anywhere. No breaking change.
• SubmitModal.tsx — 3 consumers, new prop is optional and collision-free.
• StyledDialogContentText.ts — 2 consumers, new type annotation is additive, not narrowing.
• No breaking changes found anywhere.

Review summary

Agent	Important	Medium	Suggestions	Confidence
Security	2	0	3	Medium-High
Architecture	4	3	4	High
Data Integrity	2	0	5	High
Testing	1 (3 disproven)	0	6	High (static); disproven on 3 dynamic-timing claims
UX	4	2	4	High
Standards	0	2	many (checklist mostly PASS)	High
Financial Reporting	2	0	3	High
Dependency Impact	0	0	0 (all confirmed safe)	High

[kegrimes]

❗️BLOCKED❗️

Waiting for a couple of issues to be fixed before I can finish up this PR:

• "None" option exists in the UI
• Discussion on whether geographic location should be optional or required
• UAT bug fix with PDS defaulting to "None" and causing an extra update mutation call

[canac]

Looks great! It will be nice for users to not have to keep putting in their geographic location.

[canac]

What do you think about "updated to"?

Suggested change

	'Your geographic location will be updated as {{geographicLocation}} in your account settings.',
	'Your geographic location will be updated to {{geographicLocation}} in your account settings.',

[canac]

If you decide to change the other one.

Suggested change

	'Your geographic location will be updated as {{geographicLocation}} in your account settings.',
	'Your geographic location will be updated to {{geographicLocation}} in your account settings.',

[canac]

I know you're still working out how to handle the None option, but I assume you'll want a fallback value here and in the other alerts and notifications.

Suggested change

	{ geographicLocation: questionnaire?.geographicLocation },
	{ geographicLocation: questionnaire?.geographicLocation ?? "None" },

[canac]

I don't know a practical scenario of how senior staff would use MPD and salary calculator together but not sure if we want one over the other to be a source of truth.

I think "last write wins" is mostly reasonable! The one caveat I just realized is that staff can use the MPD goal calculator to answer questions about future realities, i.e. "What will my goal be when I move to NYC?" In that case, their location in the goal might not be their actual location. But MPDX doesn't change the location until they click Apply, so I think it's still OK. Users probably won't apply hypothetical goals for cities they aren't at least planning to move to.