Skip to content

feat: DAH-0000 Read the signed-in user from one session provider - #3058

Draft
fwextensions wants to merge 1 commit into
mainfrom
DAH-0000/session-facade
Draft

feat: DAH-0000 Read the signed-in user from one session provider#3058
fwextensions wants to merge 1 commit into
mainfrom
DAH-0000/session-facade

Conversation

@fwextensions

@fwextensions fwextensions commented Sep 4, 2026

Copy link
Copy Markdown
Collaborator

Description

Replaces the per-call-site "is the Clerk flag on?" branching with one SessionProvider that owns the answer, exposing session state as a discriminated union so each consumer handles every case or fails to compile. Migrating the existing consumers onto it fixes three defects: the apply button's empty href while loading (DAH-4340), an idle timeout that never ended a Clerk session, and Layout's duplicated sign-out.

What's here

New in app/javascript/authentication/session/:

File Role
types.ts Session union: Loading / SignedOut / SignedInWithoutProfile / SignedIn, with each variant carrying its own payload
SessionProvider.tsx Three source components — Clerk, Devise, unresolved — with one provider choosing between them
useSession.ts, SessionContext.ts, index.ts Consumer surface

Migrated: withAuthentication, Layout, IdleTimeout, ListingDetailsApply.

Why a provider rather than a hook. useAuth() throws outside a ClerkProvider, and withAppSetup mounts ClerkProvider only when the flag is on. So any hook calling useAuth() unconditionally crashes the Devise path. useAuth() is confined to ClerkSessionSource, which is only ever mounted inside ClerkProvider.

Why hasCredentials exists alongside the union. The header needs a synchronous answer or signed-in Devise users see "Sign In" for one paint and watch it flip. It's documented as chrome-only — please push back if you see it gating access or data anywhere.

One deliberate provider check remains. Under Devise the Angular flow still accepts anonymous applications, so the apply button links straight to the form; under Clerk it routes signed-out users to sign in first. That's product behaviour, not plumbing, and it's commented as such.

Review instructions

Applies to review/staging, and to local dev. Everything below is on the listing details page and the account area; there is no new UI.

The Clerk path needs ?featureFlag[temp.webapp.auth.clerk]=true on the URL; omit it for the Devise path.

Accounts to use. For the Devise path (section 3), review apps preload test@test.com / abcd1234 via rake preload:user on postdeploy (app.json:35) — already confirmed, with a Salesforce Contact attached. That account does not work on the Clerk path, because it exists only in the Rails users table and Clerk sign-in never consults it. Sections 1 and 2 therefore need a Clerk identity: either a test account created directly in the Clerk instance, or one created through /create-account on the review app.

If you create one through the sign-up flow, a delayed verification email can trap you: clicking "Send again" invalidates the earlier code, so if the first email lands after you have requested a second, entering the first code fails. The on-screen error does say "Sent more than 1 code? Use the newest one" — trust it and use the most recent email. This is Clerk's behaviour and predates this branch; it is noted because it will bite anyone walking these steps. A newly created account also lands on /add-profile first, so complete that before returning to a listing in section 1.

1. Apply button no longer dead-clicks (DAH-4340)

  1. Signed out, with Clerk on, open any open listing's details page.
  2. Click Apply Online → goes to the sign-in page. Before this change, catching the page mid-load could produce a button that just reloaded the listing.
  3. Throttle the network (DevTools → Network → Slow 3G) and reload. While loading, the Apply Online button is now disabled rather than a link to nowhere.
  4. Sign in, return to the listing, click Apply Online → goes to the application.

2. Idle timeout actually signs Clerk users out

  1. With Clerk on, sign in and land on /account.
  2. Wait 30 minutes without interacting → "Stay logged in?" prompt.
  3. Let it expire (60 more seconds).
  4. You land on the sign-in page and are signed out — previously you were bounced straight back to /account because only the Devise sign-out ran.
  5. Confirm the sign-in page still shows the inactivity message.

3. Devise path unchanged (regression check)

  1. Without the flag param, sign in with a Devise account.
  2. Header shows the account menu, not "Sign In" — watch the first paint on a hard reload, there should be no flicker between the two.
  3. Sign out from the header menu → returns to the sign-in page, signed out.
  4. Open a listing and click Apply Online → still goes straight to the application.

Not yet performed end-to-end by me — I ran the unit tests and typechecker, not the browser flows. The 30-minute idle wait in particular wants someone with a real environment. Flagging that rather than checking the box below.

Notes for reviewers

  • accountUtils.setupUserContext now also stubs SessionContext; without it, migrated components see the Loading default and render null. That one change repaired eight suites. It takes a clerkEnabled param defaulting to Clerk, matching what the suites using it already mock.
  • Full Jest run has some userEvent 5s timeouts under parallel load. At --testTimeout=30000 only two suites fail, both also fail on main, and both pass in isolation on this branch. tsc --noEmit and ESLint are clean. Leaving the "all checks pass" box unchecked for CI to confirm.
  • Follow-up not in scope: remaining ad-hoc flag call sites, ambient credentials in the API layer, and the housing-counselor delegate state.

Before requesting eng review

Version Control

  • branch name begins with angular if it contains updates to Angular code
  • branch name contains the Jira ticket number
  • PR name follows type: TICKET-NUMBER Description format, use DAH-000 if it does not need a ticket
  • PR name follows urgent: Description format if it is urgent and does not need a ticket

Code quality

  • the set of changes is small
  • all automated code checks pass (linting, tests, coverage, etc.) — lint and typecheck clean locally; see notes on Jest flakiness, awaiting CI
  • if the PR is a bugfix, there are tests and logs around the bug

Code conventions

  • web pages are formatted with .scss stylesheets and ui-seeds tokens, rather than inline styles or Tailwind

Review instructions

  • instructions specify which environment(s) it applies to
  • instructions work for PA testers
  • instructions have already been performed at least once — unit-tested only, browser flows not yet walked

Request eng review

  • PR has needs review label
  • Use Housing Eng group to automatically assign reviewers, and/or assign specific engineers
  • If time sensitive, notify engineers in Slack

Before merging

Request product acceptance (PA) testing

  • PA tested in the review environment (use needs product acceptance label)
  • if PA testing cannot be done, changes are behind a feature flag — the Clerk path rides the existing temp.webapp.auth.clerk flag; the Devise path changes are not flagged, so section 3 above is the regression check that matters

The React app asked "who is signed in" in several different ways: some call
sites read the Clerk flag then branched to useAuth or UserContext, others
checked isTokenValid directly. Each site re-derived the same answer from
booleans, and the combinations that were never covered turned into bugs.

Introduce a single SessionProvider that owns that decision, and a Session
discriminated union that call sites consume with an exhaustive switch, so an
unhandled state is a type error instead of a silent fallthrough. useAuth is
confined to the Clerk source component, which is only mounted inside the
ClerkProvider that withAppSetup adds under the flag.

Migrating the existing call sites onto it fixes three defects:

- ListingDetailsApply rendered an apply button with an empty href while the
  session was still resolving; it is now disabled until the session is known.
- IdleTimeout called Devise's timeOut even under Clerk, so a timed-out user
  landed on the sign-in page still signed in. It now ends the session through
  the facade, and passes the alert reason as the redirect path so the query
  param survives BaseIdleTimeout's own navigation.
- Layout forked its sign-out between the two backends; it now has one path.

Header chrome reads hasCredentials rather than the async union, so signed-in
Devise users no longer see a "Sign In" flash on first paint.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants