HS-1639514 - Fix Google Import, MailChimp, Organization, and PrayerLetters redirects

[zweatshirt]

Description

Caused by a minor regression from #1781

• https://secure.helpscout.net/conversation/3341814677/1639514?viewId=8745276

Issue:

useOauthUrl.ts built the OAuth button URLs by reading window.location.origin directly inside getRedirectUrl. The integration accordions call these getters during render to set each button's href, and render also runs on the server.

On a full page load (the OAuth redirect landing, a direct URL, or a refresh), window is undefined on the server, so the read threw ReferenceError: window is not defined and crashed the page render. This was introduced by PR #1781 (Remove SITE_URL), which deleted the env var that had been the SSR-safe branch of process.env.SITE_URL || window.location.origin, leaving only the window read.

Fix: Resolve the origin in a client side only useEffect and hold it in state that starts
as an empty string.

SSR and the first hydration render use the empty value, so no window access happens during render and the server/client markup matches. After mount, the effect sets the real window.location.origin and rerenders,
patching the button hrefs with the user's host. This keeps the redirect correct across all domains without depending on a build-time env var. Fixes all four OAuth flows (Google, MailChimp, DonorHub/Organization, prayerletters.com),
which share getRedirectUrl.

Testing

• Go to MPDX Tools > Import from Google > Select 'Connect Google Account'
  • In production this will cause a server side error, in the preview environment this will resolve correctly.
• You can test also that refreshes in the Settings work when the PrayerLetters and MailChimp accordions are open as well.

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 5a00e95

No significant changes found

[github-actions]

Preview branch generated at https://hs-1639514.d3dytjb8adxkk5.amplifyapp.com

[zweatshirt]

🤖 Multi-Agent Code Review — ✅ CLEAN

4 agents (Security, Architecture, Testing, Standards) · standard mode · Risk: LOW (1/10)

No blockers, no important issues, no standards violations. This is a correct, idiomatic, net-debt-reducing SSR-safety fix. Six low-severity suggestions (all < 2.5/10), all informational.

What the change does

Defers the window.location.origin read out of render and into a useEffect (useState('') default), fixing the SSR ReferenceError: window is not defined crash on the integrations page (introduced when SITE_URL was removed). Server + first hydration render use '' (markup matches → clean hydration), then the effect populates the real origin and React patches the button hrefs.

Consensus theme — transient host-relative redirect_to (all 4 agents)

During SSR / pre-hydration, origin is '', so redirect_to is host-relative until the effect runs. All agents independently concluded this is functionally safe: the href getters are only navigated on user click (post-hydration), and the Organization flows build the URL at click time, so origin is always populated by interaction time. No open-redirect risk (no user input flows into the target). Strictly better than the crash it replaces. Optional belt-and-suspenders (fall back to window.location.origin at call time when origin === '') was considered and not recommended as required.

Notes

• Architecture: the useState('')+useEffect choice is the correct hydration-safe pattern; explicitly don't copy the typeof window lazy-initializer from useLocalStorage.ts (would cause a hydration mismatch here). useEffect([]) deps correct. Net technical debt reduced.
• Standards: all applicable checklist items PASS — named export, hook naming/colocation, no any, @ts-expect-error carries an explanatory comment, exact full-URL test assertions, window read confined to an effect.
• "Fix one, fix all": verified the only other window.location.origin reference is in _document.page.tsx inside a browser-executed <script> string (safe). No remaining instances of this bug — fix is complete.

Suggestions (informational, no /dismiss needed)

• Add getMailChimpOauthUrl / getPrayerlettersOauthUrl to this hook's test (their path segments mailchimp / prayer_letters aren't pinned here — though both are covered at the component level). An it.each over all four getters closes it cheaply.
• Optional: assert the no-session error path (useRequiredSession throws).

Agent	Critical	High	Important	Suggestions	Confidence
Security	0	0	0	1	High
Architecture	0	0	0	2	High
Testing	0	0	0	3	High
Standards	0	0	0	0	High
Total	0	0	0	6	High

_{Suggestions are informational and do not block a CLEAN verdict.}

[zweatshirt]

[Suggestion] During SSR and the first client render `origin` is `''`, so `redirect_to` is host-relative until the post-mount effect fires. This is functionally safe — these getters are only navigated on user click (post-hydration), and the Organization flows build the URL at click time, so `origin` is always populated by then; no open-redirect risk (no user input in the target). Noted for awareness only. If you ever want server-rendered links to be absolute, you could fall back to `window.location.origin` at call time when `origin === ''` — but that adds branching for a near-impossible race and isn't recommended here.

[zweatshirt]

[Suggestion] `getMailChimpOauthUrl` and `getPrayerlettersOauthUrl` aren't asserted here, so their distinct path segments (`/auth/user/mailchimp`, `/auth/user/prayer_letters` — note the underscore) aren't pinned by this unit test. They are covered with full-URL assertions at the component level (`MailchimpAccordion.test.tsx:143`, `PrayerlettersAccordion.test.tsx:112-117`), so this is minor. An `it.each` over all four getters would close the gap cheaply (`getOrganizationOauthUrl` stays separate since it takes an arg).

[github-actions]

AI Review Auto-Approval

Risk Level: LOW (1/10)
Verdict: CLEAN (no issues found)

This PR was auto-approved because:

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

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

[zweatshirt]

@canac Hi Caleb, I had a quick question if that's okay. I assume this wouldn't cause cross-file leakage of process.env.OAUTH_URL? In this case, is it best to add an afterEach anyway?

[dr-bizz]

This is fine

[dr-bizz]

Looks great! thank you for fixing this