From 0a582a8dfd82a3eaadaa516d45a9263919411868 Mon Sep 17 00:00:00 2001 From: Logan Rasmussen Date: Thu, 24 Sep 2026 17:49:58 -0600 Subject: [PATCH 1/7] fix(connecting): do not treat firefly's pre-job CONNECTED update as the finished job Firefly sets an OAuth member to CONNECTED on the redirect before any job exists. Over websockets that update can arrive after the widget has started its own job; runJobSchedule$ took it as "done" and, unable to load a job, assumed the job it had started finished, showing Success! while the real job ended IMPEDED (CT-2332). The update names the job the member had before runJob was called: null for a first job, the previous job's guid for a returning member. runJobSchedule$ now records that guid when it calls runJob and keeps observing while a CONNECTED idle update still names it. `undefined` is left alone because hosts are not required to send the field. After a 409 the widget started nothing, so only the null case is detectable there. Verified in SAND with websockets on: 50/50 NoDDA runs pass (was ~30% failing). Co-Authored-By: Claude Fable 5.1 --- docs/APIDOCUMENTATION.md | 2 + .../__tests__/runJobSchedule-test.js | 153 ++++++++++++++++++ src/utilities/runJobSchedule.js | 49 ++++-- .../__tests__/ConnectingOAuthJobs-test.tsx | 151 +++++++++++++++-- 4 files changed, 331 insertions(+), 24 deletions(-) create mode 100644 src/utilities/__tests__/runJobSchedule-test.js diff --git a/docs/APIDOCUMENTATION.md b/docs/APIDOCUMENTATION.md index e51f98fab9..c22d830f5d 100644 --- a/docs/APIDOCUMENTATION.md +++ b/docs/APIDOCUMENTATION.md @@ -135,6 +135,8 @@ ##### Notes > This callback is also used during OAuth flows to synchronize member data when the backend returns a different `inbound_member_guid` than the one used to start the flow (e.g., during non-OAuth to OAuth migrations). When this happens, the widget will fetch the new member record and update its internal state to use the new GUID. +> +> If your backend tracks jobs, `most_recent_job_guid` must change when a new job starts and be `null` before the first one. A `CONNECTED` member whose value is `null`, or unchanged since the widget called `runJob`, keeps the Connecting step waiting. Omit the field entirely if your backend does not track jobs. ##### Responses diff --git a/src/utilities/__tests__/runJobSchedule-test.js b/src/utilities/__tests__/runJobSchedule-test.js new file mode 100644 index 0000000000..028211f712 --- /dev/null +++ b/src/utilities/__tests__/runJobSchedule-test.js @@ -0,0 +1,153 @@ +import { Subject } from 'rxjs' + +import { runJobSchedule$ } from 'src/utilities/runJobSchedule' +import { JOB_STATUSES, JOB_TYPES } from 'src/const/consts' +import { ReadableStatuses } from 'src/const/Statuses' + +// CT-2332 pre-job update rules, driven through runJobSchedule$'s injected api and pollMember. + +const MEMBER_GUID = 'MBR-1' +const OUR_JOB_GUID = 'JOB-1' + +const verificationSchedule = () => ({ + isInitialized: true, + jobs: [{ type: JOB_TYPES.VERIFICATION, status: JOB_STATUSES.ACTIVE }], +}) + +const member = (connectionStatus, extra = {}) => ({ + guid: MEMBER_GUID, + connection_status: connectionStatus, + is_being_aggregated: false, + ...extra, +}) + +// Over websockets the polled job carries only the guid, never a job_type. +const doneState = (polledMember) => ({ + pollingIsDone: true, + currentResponse: { + member: polledMember, + job: { guid: polledMember.most_recent_job_guid ?? null, async_account_data_ready: false }, + }, +}) + +const verificationJob = (guid) => ({ guid, job_type: JOB_TYPES.VERIFICATION }) + +const flush = () => new Promise((resolve) => setTimeout(resolve, 0)) + +const run = ({ api, member: startingMember }) => { + const pollingStates$ = new Subject() + const emissions = [] + + const subscription = runJobSchedule$({ + api, + pollMember: () => pollingStates$, + member: startingMember, + schedule: verificationSchedule(), + config: {}, + }).subscribe({ next: (emission) => emissions.push(emission) }) + + return { pollingStates$, emissions, subscription } +} + +describe('runJobSchedule$ pre-job updates', () => { + it('does not treat a CONNECTED member with most_recent_job_guid null as the finished job', async () => { + const api = { + runJob: vi.fn().mockResolvedValue({}), + loadJob: vi.fn(async (guid) => verificationJob(guid)), + } + const { pollingStates$, emissions, subscription } = run({ + api, + member: member(ReadableStatuses.PENDING, { most_recent_job_guid: null }), + }) + await flush() + + // Firefly's pre-job CONNECTED update, delivered late. + pollingStates$.next( + doneState(member(ReadableStatuses.CONNECTED, { most_recent_job_guid: null })), + ) + await flush() + + expect(emissions).toHaveLength(0) + expect(api.loadJob).not.toHaveBeenCalled() + + const impeded = member(ReadableStatuses.IMPEDED, { most_recent_job_guid: OUR_JOB_GUID }) + pollingStates$.next(doneState(impeded)) + await flush() + + expect(emissions).toHaveLength(1) + expect(emissions[0]).toMatchObject({ member: impeded, job: verificationJob(OUR_JOB_GUID) }) + + subscription.unsubscribe() + }) + + it('does not treat an idle CONNECTED member still naming its previous job as the finished job', async () => { + const returningMember = member(ReadableStatuses.CONNECTED, { + most_recent_job_guid: 'JOB-old', + }) + const api = { + runJob: vi.fn().mockResolvedValue({}), + loadJob: vi.fn(async (guid) => verificationJob(guid)), + } + const { pollingStates$, emissions, subscription } = run({ api, member: returningMember }) + await flush() + + // The same late update for a returning member names the old job. + pollingStates$.next(doneState(returningMember)) + await flush() + + expect(emissions).toHaveLength(0) + expect(api.loadJob).not.toHaveBeenCalled() + + const impeded = member(ReadableStatuses.IMPEDED, { most_recent_job_guid: OUR_JOB_GUID }) + pollingStates$.next(doneState(impeded)) + await flush() + + expect(emissions).toHaveLength(1) + expect(emissions[0]).toMatchObject({ member: impeded, job: verificationJob(OUR_JOB_GUID) }) + + subscription.unsubscribe() + }) + + it('still accepts the member’s current job when runJob was rejected with a 409', async () => { + const memberWithFireflyJob = member(ReadableStatuses.CONNECTED, { + most_recent_job_guid: 'JOB-firefly', + }) + const conflict = Object.assign(new Error('conflict'), { response: { status: 409 } }) + const api = { + runJob: vi.fn().mockRejectedValue(conflict), + loadJob: vi.fn(async (guid) => verificationJob(guid)), + } + const { pollingStates$, emissions, subscription } = run({ api, member: memberWithFireflyJob }) + await flush() + + pollingStates$.next(doneState(memberWithFireflyJob)) + await flush() + + expect(emissions).toHaveLength(1) + expect(emissions[0].job.job_type).toBe(JOB_TYPES.VERIFICATION) + + subscription.unsubscribe() + }) + + it('still treats an undefined most_recent_job_guid as a finished job for hosts that do not send it', async () => { + const api = { + runJob: vi.fn().mockResolvedValue({}), + loadJob: vi.fn(), + } + const { pollingStates$, emissions, subscription } = run({ + api, + member: member(ReadableStatuses.PENDING), + }) + await flush() + + // Field absent entirely: the documented member response does not include it. + pollingStates$.next(doneState(member(ReadableStatuses.CONNECTED))) + await flush() + + expect(api.loadJob).not.toHaveBeenCalled() + expect(emissions).toHaveLength(1) + expect(emissions[0].job.job_type).toBe(JOB_TYPES.VERIFICATION) + + subscription.unsubscribe() + }) +}) diff --git a/src/utilities/runJobSchedule.js b/src/utilities/runJobSchedule.js index 9b83d5a385..6816500576 100644 --- a/src/utilities/runJobSchedule.js +++ b/src/utilities/runJobSchedule.js @@ -30,6 +30,18 @@ const isSafeConflictError = (error) => error?.response?.status === 409 const isConnectedWithoutError = (member) => member?.connection_status === ReadableStatuses.CONNECTED && !member?.error?.error_code +const NOT_STARTED_BY_US = { type: null, previousJobGuid: null } + +// Firefly sets an OAuth member CONNECTED on the redirect before any job exists, and over +// websockets that update can arrive after we started ours (CT-2332). It names the job the +// member had before runJob: null for a first job, the previous job for a returning member. +// `undefined` passes because hosts are not required to send the field. +const isPreJobUpdate = (member, started) => { + const guid = member?.most_recent_job_guid + + return guid === null || guid === started.previousJobGuid +} + /** * Work out which job just finished, in order of trust: * - the job we loaded fresh off the polled member @@ -102,17 +114,26 @@ export const runJobSchedule$ = ({ * else scheduled after it, otherwise we keep polling until the member is idle * so the next job can be started. */ - const observeRunningJob = (memberGuid, currentSchedule, startedType) => + const observeRunningJob = (memberGuid, currentSchedule, started) => pollMember(memberGuid).pipe( + // onPoll runs before the gate on purpose: it is where the Connecting timeout lives. tap(onPoll), - filter((pollingState) => pollingState.pollingIsDone), + // Error and MFA states route on the member alone; only CONNECTED needs a real finished job. + filter((pollingState) => { + const polledMember = pollingState.currentResponse?.member + + return ( + pollingState.pollingIsDone && + !(isConnectedWithoutError(polledMember) && isPreJobUpdate(polledMember, started)) + ) + }), take(1), map((pollingState) => pollingState.currentResponse), mergeMap((polledResponse) => loadJob(polledResponse.member).pipe( map((job) => ({ member: polledResponse.member, - job: resolveFinishedJob(job, polledResponse.job, startedType), + job: resolveFinishedJob(job, polledResponse.job, started.type), })), ), ), @@ -135,8 +156,8 @@ export const runJobSchedule$ = ({ }), ) - const observeThenContinue = (memberGuid, currentSchedule, iteration, startedType) => - observeRunningJob(memberGuid, currentSchedule, startedType).pipe( + const observeThenContinue = (memberGuid, currentSchedule, iteration, started) => + observeRunningJob(memberGuid, currentSchedule, started).pipe( mergeMap(({ member: observedMember, job }) => { const emitted = of({ member: observedMember, job }) @@ -157,22 +178,30 @@ export const runJobSchedule$ = ({ } if (currentMember.is_being_aggregated !== false) { - return observeThenContinue(currentMember.guid, currentSchedule, iteration, null) + return observeThenContinue( + currentMember.guid, + currentSchedule, + iteration, + NOT_STARTED_BY_US, + ) } const activeJob = JobSchedule.getActiveJob(currentSchedule) return defer(() => api.runJob(activeJob.type, currentMember.guid, config, true)).pipe( - map(() => activeJob.type), + map(() => ({ + type: activeJob.type, + previousJobGuid: currentMember.most_recent_job_guid ?? null, + })), catchError((error) => { // 409 is usually the job Firefly created on the OAuth redirect. // It gets observed and reconciled like any other running job. - if (isSafeConflictError(error)) return of(null) + if (isSafeConflictError(error)) return of(NOT_STARTED_BY_US) return throwError(() => error) }), - mergeMap((startedType) => - observeThenContinue(currentMember.guid, currentSchedule, iteration, startedType), + mergeMap((started) => + observeThenContinue(currentMember.guid, currentSchedule, iteration, started), ), ) }) diff --git a/src/views/connecting/__tests__/ConnectingOAuthJobs-test.tsx b/src/views/connecting/__tests__/ConnectingOAuthJobs-test.tsx index 7589204f9b..0dd9ec67a2 100644 --- a/src/views/connecting/__tests__/ConnectingOAuthJobs-test.tsx +++ b/src/views/connecting/__tests__/ConnectingOAuthJobs-test.tsx @@ -1,14 +1,20 @@ import React from 'react' +import { Subject } from 'rxjs' import { createTestReduxStore, render, waitFor } from 'src/utilities/testingLibrary' import { Connecting } from 'src/views/connecting/Connecting' import { PostMessageContext } from 'src/ConnectWidget' import { ApiContextTypes, ApiProvider } from 'src/context/ApiContext' +import { WebSocketConnection, WebSocketProvider } from 'src/context/WebSocketContext' import { POST_MESSAGES } from 'src/const/postMessages' import { ReadableStatuses } from 'src/const/Statuses' import { JOB_TYPES } from 'src/const/consts' -import { VERIFY_MODE } from 'src/const/Connect' +import { STEPS, VERIFY_MODE } from 'src/const/Connect' +import { ACTIONABLE_ERROR_CODES } from 'src/views/actionableError/consts' import { EXTRA_ITERATIONS_ALLOWED } from 'src/utilities/runJobSchedule' +// fadeOut (Velocity) never resolves in jsdom; Connecting's error path dispatches inside its .then. +vi.mock('src/utilities/Animation', () => ({ fadeOut: vi.fn(() => Promise.resolve()) })) + /** * CT-2495: after OAuth the widget lands on Connecting holding the member it * created before the user left for the institution. That copy is PENDING, is not @@ -29,6 +35,7 @@ type Member = { is_being_aggregated: boolean most_recent_job_guid: string | null is_oauth: boolean + error?: { error_code: number } | null } type Job = { guid: string; job_type: number; async_account_data_ready?: boolean } @@ -59,26 +66,31 @@ const connectedMemberRunning = (jobGuid: string): Member => ({ most_recent_job_guid: jobGuid, }) -const createStore = () => +const createStore = ({ member = staleOAuthMember, useWebSockets = false } = {}) => createTestReduxStore({ connect: { currentMemberGuid: MEMBER_GUID, - members: [staleOAuthMember], + members: [member], jobSchedule: { isInitialized: false, jobs: [] }, location: [], selectedInstitution: {}, }, experimentalFeatures: { - memberPollingMilliseconds: 10, + // With websockets on, frames drive the observation and polling is effectively off. + memberPollingMilliseconds: useWebSockets ? 60_000 : 10, optOutOfEarlyUserRelease: false, unavailableInstitutions: [], - useWebSockets: false, + useWebSockets, }, }) -const createFakeBackend = ({ pollsUntilDone = 2, earlyDataRelease = false } = {}) => { +const createFakeBackend = ({ + pollsUntilDone = 2, + earlyDataRelease = false, + member = staleOAuthMember, +} = {}) => { const backend = { - member: { ...staleOAuthMember } as Member, + member: { ...member } as Member, jobs: {} as Record, pollsWhileRunning: 0, @@ -153,6 +165,7 @@ class TestErrorBoundary extends React.Component< const renderConnecting = ( backend: ReturnType, connectConfig: Record, + { webSocket, member }: { webSocket?: WebSocketConnection; member?: Member } = {}, ) => { const onPostMessage = vi.fn() const onError = vi.fn() @@ -161,19 +174,28 @@ const renderConnecting = ( loadJob: backend.loadJob, runJob: backend.runJob, } as unknown as ApiContextTypes + const store = createStore({ member, useWebSockets: !!webSocket }) + + const connecting = ( + + + + + + ) render( - - - - - + {webSocket ? ( + {connecting} + ) : ( + connecting + )} , - { store: createStore() }, + { store }, ) - return { onPostMessage, onError } + return { onPostMessage, onError, store } } const expectMemberConnected = (onPostMessage: ReturnType) => @@ -364,3 +386,104 @@ describe(' after OAuth', () => { await expectMemberConnected(onPostMessage) }) }) + +/** + * CT-2332: firefly sets the member CONNECTED on the OAuth redirect before any job exists, + * and over websockets that frame can arrive after the widget has started its job. + */ +describe(' after OAuth over websockets', () => { + const createWebSocket = () => { + // Plain Subject: like brokaw, no replay for late subscribers. + const messages$ = new Subject<{ event: string; payload: Member }>() + const connection: WebSocketConnection = { + isConnected: () => true, + webSocketMessages$: messages$.asObservable(), + } + + return { messages$, connection } + } + + const memberUpdated = (payload: Member) => ({ event: 'members/updated', payload }) + + const impededMember = (jobGuid: string): Member => ({ + ...staleOAuthMember, + connection_status: ReadableStatuses.IMPEDED, + most_recent_job_guid: jobGuid, + error: { error_code: ACTIONABLE_ERROR_CODES.NO_ELIGIBLE_ACCOUNTS }, + }) + + // Lets runJob settle so the schedule has subscribed to the socket before frames are sent. + const settle = () => new Promise((resolve) => setTimeout(resolve, 0)) + + const expectActionableErrorInsteadOfSuccess = async ( + store: ReturnType, + onPostMessage: ReturnType, + ) => { + const lastStep = () => { + const { location } = store.getState().connect + return location[location.length - 1]?.step + } + + // Wait for any step, then assert which: before the fix this routes to CONNECTED at once. + await waitFor(() => expect(lastStep()).toBeDefined(), { timeout: 4000 }) + expect(lastStep()).toBe(STEPS.ACTIONABLE_ERROR) + expect(onPostMessage).not.toHaveBeenCalledWith( + POST_MESSAGES.MEMBER_CONNECTED, + expect.anything(), + ) + } + + afterEach(() => { + vi.restoreAllMocks() + }) + + it('ignores a late CONNECTED update with no job and lands on the actionable error when the job we started is impeded', async () => { + const backend = createFakeBackend() + const { messages$, connection } = createWebSocket() + + const { onPostMessage, store } = renderConnecting( + backend, + { mode: VERIFY_MODE }, + { webSocket: connection }, + ) + + await waitFor(() => expect(backend.runJob).toHaveBeenCalled()) + await settle() + + messages$.next( + memberUpdated({ ...staleOAuthMember, connection_status: ReadableStatuses.CONNECTED }), + ) + await settle() + messages$.next(memberUpdated(impededMember(`JOB-${JOB_TYPES.VERIFICATION}`))) + + await expectActionableErrorInsteadOfSuccess(store, onPostMessage) + }) + + it('ignores a late CONNECTED update that still names a returning member’s previous job', async () => { + const PREVIOUS_JOB_GUID = 'JOB-old' + const returningMember: Member = { + ...staleOAuthMember, + connection_status: ReadableStatuses.CONNECTED, + most_recent_job_guid: PREVIOUS_JOB_GUID, + } + const backend = createFakeBackend({ member: returningMember }) + // The previous job was also a verification, so attributing it by type would end the schedule. + backend.jobs[PREVIOUS_JOB_GUID] = { guid: PREVIOUS_JOB_GUID, job_type: JOB_TYPES.VERIFICATION } + const { messages$, connection } = createWebSocket() + + const { onPostMessage, store } = renderConnecting( + backend, + { mode: VERIFY_MODE }, + { webSocket: connection, member: returningMember }, + ) + + await waitFor(() => expect(backend.runJob).toHaveBeenCalled()) + await settle() + + messages$.next(memberUpdated(returningMember)) + await settle() + messages$.next(memberUpdated(impededMember(`JOB-${JOB_TYPES.VERIFICATION}`))) + + await expectActionableErrorInsteadOfSuccess(store, onPostMessage) + }) +}) From 36c0b65ced140a2dbd0ea7af1a33534db12a48ef Mon Sep 17 00:00:00 2001 From: Logan Rasmussen Date: Thu, 24 Sep 2026 18:59:06 -0600 Subject: [PATCH 2/7] test(connecting): spell out that firefly's assigned job is observed after a 409 Co-Authored-By: Claude Fable 5.1 --- src/utilities/__tests__/runJobSchedule-test.js | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/src/utilities/__tests__/runJobSchedule-test.js b/src/utilities/__tests__/runJobSchedule-test.js index 028211f712..30bfe29615 100644 --- a/src/utilities/__tests__/runJobSchedule-test.js +++ b/src/utilities/__tests__/runJobSchedule-test.js @@ -108,9 +108,12 @@ describe('runJobSchedule$ pre-job updates', () => { subscription.unsubscribe() }) - it('still accepts the member’s current job when runJob was rejected with a 409', async () => { + it('observes the job firefly assigned when runJob is rejected with a 409, instead of treating it as the previous job', async () => { + // Firefly started this job on the OAuth redirect. It is the job that matters: the + // widget's own runJob is a duplicate and firefly rejects it with a 409. + const FIREFLY_JOB_GUID = 'JOB-firefly' const memberWithFireflyJob = member(ReadableStatuses.CONNECTED, { - most_recent_job_guid: 'JOB-firefly', + most_recent_job_guid: FIREFLY_JOB_GUID, }) const conflict = Object.assign(new Error('conflict'), { response: { status: 409 } }) const api = { @@ -120,11 +123,16 @@ describe('runJobSchedule$ pre-job updates', () => { const { pollingStates$, emissions, subscription } = run({ api, member: memberWithFireflyJob }) await flush() + expect(api.runJob).toHaveBeenCalledTimes(1) + + // The member still names firefly's job when it finishes. Had the 409 path recorded that + // guid as "the previous job", this update would be ignored and Connecting would hang. pollingStates$.next(doneState(memberWithFireflyJob)) await flush() + expect(api.loadJob).toHaveBeenCalledWith(FIREFLY_JOB_GUID) expect(emissions).toHaveLength(1) - expect(emissions[0].job.job_type).toBe(JOB_TYPES.VERIFICATION) + expect(emissions[0].job).toEqual(verificationJob(FIREFLY_JOB_GUID)) subscription.unsubscribe() }) From 9e3b11b927893df91e582cdcc0d21ff2ba927631 Mon Sep 17 00:00:00 2001 From: Logan Rasmussen Date: Thu, 24 Sep 2026 19:08:17 -0600 Subject: [PATCH 3/7] fix(connecting): assume most_recent_job_guid is always sent The repo is moving internal and the backend always sends the field (a guid or null), so the gate no longer distinguishes undefined from null. Drops the test that pinned that distinction and the doc sentence that told hosts they could omit the field. Co-Authored-By: Claude Fable 5.1 --- docs/APIDOCUMENTATION.md | 2 +- .../__tests__/runJobSchedule-test.js | 22 ------------------- src/utilities/runJobSchedule.js | 10 +++------ 3 files changed, 4 insertions(+), 30 deletions(-) diff --git a/docs/APIDOCUMENTATION.md b/docs/APIDOCUMENTATION.md index c22d830f5d..5beef83710 100644 --- a/docs/APIDOCUMENTATION.md +++ b/docs/APIDOCUMENTATION.md @@ -136,7 +136,7 @@ > This callback is also used during OAuth flows to synchronize member data when the backend returns a different `inbound_member_guid` than the one used to start the flow (e.g., during non-OAuth to OAuth migrations). When this happens, the widget will fetch the new member record and update its internal state to use the new GUID. > -> If your backend tracks jobs, `most_recent_job_guid` must change when a new job starts and be `null` before the first one. A `CONNECTED` member whose value is `null`, or unchanged since the widget called `runJob`, keeps the Connecting step waiting. Omit the field entirely if your backend does not track jobs. +> `most_recent_job_guid` must change when a new job starts and be `null` before the first one. A `CONNECTED` member whose value is `null`, or unchanged since the widget called `runJob`, keeps the Connecting step waiting. ##### Responses diff --git a/src/utilities/__tests__/runJobSchedule-test.js b/src/utilities/__tests__/runJobSchedule-test.js index 30bfe29615..f7686fae2e 100644 --- a/src/utilities/__tests__/runJobSchedule-test.js +++ b/src/utilities/__tests__/runJobSchedule-test.js @@ -136,26 +136,4 @@ describe('runJobSchedule$ pre-job updates', () => { subscription.unsubscribe() }) - - it('still treats an undefined most_recent_job_guid as a finished job for hosts that do not send it', async () => { - const api = { - runJob: vi.fn().mockResolvedValue({}), - loadJob: vi.fn(), - } - const { pollingStates$, emissions, subscription } = run({ - api, - member: member(ReadableStatuses.PENDING), - }) - await flush() - - // Field absent entirely: the documented member response does not include it. - pollingStates$.next(doneState(member(ReadableStatuses.CONNECTED))) - await flush() - - expect(api.loadJob).not.toHaveBeenCalled() - expect(emissions).toHaveLength(1) - expect(emissions[0].job.job_type).toBe(JOB_TYPES.VERIFICATION) - - subscription.unsubscribe() - }) }) diff --git a/src/utilities/runJobSchedule.js b/src/utilities/runJobSchedule.js index 6816500576..e42ea56116 100644 --- a/src/utilities/runJobSchedule.js +++ b/src/utilities/runJobSchedule.js @@ -34,12 +34,11 @@ const NOT_STARTED_BY_US = { type: null, previousJobGuid: null } // Firefly sets an OAuth member CONNECTED on the redirect before any job exists, and over // websockets that update can arrive after we started ours (CT-2332). It names the job the -// member had before runJob: null for a first job, the previous job for a returning member. -// `undefined` passes because hosts are not required to send the field. +// member had before runJob: none for a first job, the previous job for a returning member. const isPreJobUpdate = (member, started) => { const guid = member?.most_recent_job_guid - return guid === null || guid === started.previousJobGuid + return !guid || guid === started.previousJobGuid } /** @@ -189,10 +188,7 @@ export const runJobSchedule$ = ({ const activeJob = JobSchedule.getActiveJob(currentSchedule) return defer(() => api.runJob(activeJob.type, currentMember.guid, config, true)).pipe( - map(() => ({ - type: activeJob.type, - previousJobGuid: currentMember.most_recent_job_guid ?? null, - })), + map(() => ({ type: activeJob.type, previousJobGuid: currentMember.most_recent_job_guid })), catchError((error) => { // 409 is usually the job Firefly created on the OAuth redirect. // It gets observed and reconciled like any other running job. From f27654ae7f82c5b48603e6263d89735f58d34fbe Mon Sep 17 00:00:00 2001 From: Logan Rasmussen Date: Fri, 25 Sep 2026 10:32:23 -0600 Subject: [PATCH 4/7] test(connecting): cover the 409 path through Connecting and drop the runJobSchedule unit tests The unit file fed hand-built polling states straight into runJobSchedule$, so it asserted assumptions about what the poller emits rather than what the rendered widget does. Every case now runs through Connecting with the real store, hook and transport, driven by the fake API and a Subject in place of brokaw: the first-time member, the returning member, and now the 409 path where firefly's redirect job is the one observed. All three fail against the previous runner. Co-Authored-By: Claude Fable 5.1 --- .../__tests__/runJobSchedule-test.js | 139 ------------------ .../__tests__/ConnectingOAuthJobs-test.tsx | 36 +++++ 2 files changed, 36 insertions(+), 139 deletions(-) delete mode 100644 src/utilities/__tests__/runJobSchedule-test.js diff --git a/src/utilities/__tests__/runJobSchedule-test.js b/src/utilities/__tests__/runJobSchedule-test.js deleted file mode 100644 index f7686fae2e..0000000000 --- a/src/utilities/__tests__/runJobSchedule-test.js +++ /dev/null @@ -1,139 +0,0 @@ -import { Subject } from 'rxjs' - -import { runJobSchedule$ } from 'src/utilities/runJobSchedule' -import { JOB_STATUSES, JOB_TYPES } from 'src/const/consts' -import { ReadableStatuses } from 'src/const/Statuses' - -// CT-2332 pre-job update rules, driven through runJobSchedule$'s injected api and pollMember. - -const MEMBER_GUID = 'MBR-1' -const OUR_JOB_GUID = 'JOB-1' - -const verificationSchedule = () => ({ - isInitialized: true, - jobs: [{ type: JOB_TYPES.VERIFICATION, status: JOB_STATUSES.ACTIVE }], -}) - -const member = (connectionStatus, extra = {}) => ({ - guid: MEMBER_GUID, - connection_status: connectionStatus, - is_being_aggregated: false, - ...extra, -}) - -// Over websockets the polled job carries only the guid, never a job_type. -const doneState = (polledMember) => ({ - pollingIsDone: true, - currentResponse: { - member: polledMember, - job: { guid: polledMember.most_recent_job_guid ?? null, async_account_data_ready: false }, - }, -}) - -const verificationJob = (guid) => ({ guid, job_type: JOB_TYPES.VERIFICATION }) - -const flush = () => new Promise((resolve) => setTimeout(resolve, 0)) - -const run = ({ api, member: startingMember }) => { - const pollingStates$ = new Subject() - const emissions = [] - - const subscription = runJobSchedule$({ - api, - pollMember: () => pollingStates$, - member: startingMember, - schedule: verificationSchedule(), - config: {}, - }).subscribe({ next: (emission) => emissions.push(emission) }) - - return { pollingStates$, emissions, subscription } -} - -describe('runJobSchedule$ pre-job updates', () => { - it('does not treat a CONNECTED member with most_recent_job_guid null as the finished job', async () => { - const api = { - runJob: vi.fn().mockResolvedValue({}), - loadJob: vi.fn(async (guid) => verificationJob(guid)), - } - const { pollingStates$, emissions, subscription } = run({ - api, - member: member(ReadableStatuses.PENDING, { most_recent_job_guid: null }), - }) - await flush() - - // Firefly's pre-job CONNECTED update, delivered late. - pollingStates$.next( - doneState(member(ReadableStatuses.CONNECTED, { most_recent_job_guid: null })), - ) - await flush() - - expect(emissions).toHaveLength(0) - expect(api.loadJob).not.toHaveBeenCalled() - - const impeded = member(ReadableStatuses.IMPEDED, { most_recent_job_guid: OUR_JOB_GUID }) - pollingStates$.next(doneState(impeded)) - await flush() - - expect(emissions).toHaveLength(1) - expect(emissions[0]).toMatchObject({ member: impeded, job: verificationJob(OUR_JOB_GUID) }) - - subscription.unsubscribe() - }) - - it('does not treat an idle CONNECTED member still naming its previous job as the finished job', async () => { - const returningMember = member(ReadableStatuses.CONNECTED, { - most_recent_job_guid: 'JOB-old', - }) - const api = { - runJob: vi.fn().mockResolvedValue({}), - loadJob: vi.fn(async (guid) => verificationJob(guid)), - } - const { pollingStates$, emissions, subscription } = run({ api, member: returningMember }) - await flush() - - // The same late update for a returning member names the old job. - pollingStates$.next(doneState(returningMember)) - await flush() - - expect(emissions).toHaveLength(0) - expect(api.loadJob).not.toHaveBeenCalled() - - const impeded = member(ReadableStatuses.IMPEDED, { most_recent_job_guid: OUR_JOB_GUID }) - pollingStates$.next(doneState(impeded)) - await flush() - - expect(emissions).toHaveLength(1) - expect(emissions[0]).toMatchObject({ member: impeded, job: verificationJob(OUR_JOB_GUID) }) - - subscription.unsubscribe() - }) - - it('observes the job firefly assigned when runJob is rejected with a 409, instead of treating it as the previous job', async () => { - // Firefly started this job on the OAuth redirect. It is the job that matters: the - // widget's own runJob is a duplicate and firefly rejects it with a 409. - const FIREFLY_JOB_GUID = 'JOB-firefly' - const memberWithFireflyJob = member(ReadableStatuses.CONNECTED, { - most_recent_job_guid: FIREFLY_JOB_GUID, - }) - const conflict = Object.assign(new Error('conflict'), { response: { status: 409 } }) - const api = { - runJob: vi.fn().mockRejectedValue(conflict), - loadJob: vi.fn(async (guid) => verificationJob(guid)), - } - const { pollingStates$, emissions, subscription } = run({ api, member: memberWithFireflyJob }) - await flush() - - expect(api.runJob).toHaveBeenCalledTimes(1) - - // The member still names firefly's job when it finishes. Had the 409 path recorded that - // guid as "the previous job", this update would be ignored and Connecting would hang. - pollingStates$.next(doneState(memberWithFireflyJob)) - await flush() - - expect(api.loadJob).toHaveBeenCalledWith(FIREFLY_JOB_GUID) - expect(emissions).toHaveLength(1) - expect(emissions[0].job).toEqual(verificationJob(FIREFLY_JOB_GUID)) - - subscription.unsubscribe() - }) -}) diff --git a/src/views/connecting/__tests__/ConnectingOAuthJobs-test.tsx b/src/views/connecting/__tests__/ConnectingOAuthJobs-test.tsx index 0dd9ec67a2..7209f3421a 100644 --- a/src/views/connecting/__tests__/ConnectingOAuthJobs-test.tsx +++ b/src/views/connecting/__tests__/ConnectingOAuthJobs-test.tsx @@ -486,4 +486,40 @@ describe(' after OAuth over websockets', () => { await expectActionableErrorInsteadOfSuccess(store, onPostMessage) }) + + it('observes the job firefly assigned when its own runJob is rejected with a 409', async () => { + const backend = createFakeBackend() + const { messages$, connection } = createWebSocket() + + // disable_background_agg clients: firefly started the job on the redirect and rejects the + // widget's duplicate. Firefly's job is the one that matters from here on. + backend.runJob.mockImplementationOnce(async () => { + backend.startJob(REDIRECT_JOB_GUID, JOB_TYPES.VERIFICATION) + throw new HttpError(409) + }) + + const { onPostMessage } = renderConnecting( + backend, + { mode: VERIFY_MODE }, + { webSocket: connection }, + ) + + await waitFor(() => expect(backend.runJob).toHaveBeenCalled()) + await settle() + + // The late pre-job update is still ignored on this path. + messages$.next( + memberUpdated({ ...staleOAuthMember, connection_status: ReadableStatuses.CONNECTED }), + ) + await settle() + messages$.next(memberUpdated(connectedMemberRunning(REDIRECT_JOB_GUID))) + await settle() + messages$.next( + memberUpdated({ ...connectedMemberRunning(REDIRECT_JOB_GUID), is_being_aggregated: false }), + ) + + await expectMemberConnected(onPostMessage) + expect(backend.runJob).toHaveBeenCalledTimes(1) + expect(backend.loadJob).toHaveBeenCalledWith(REDIRECT_JOB_GUID) + }) }) From 07864dcef57b3e05c9527ccee24f33c1dd48399c Mon Sep 17 00:00:00 2001 From: Logan Rasmussen Date: Fri, 25 Sep 2026 13:47:04 -0600 Subject: [PATCH 5/7] test(connecting): move the CT-2332 integration tests next to runJobSchedule The three websocket scenarios (first-time member, returning member, 409 path) now live in src/utilities/__tests__/runJobSchedule-test.tsx so the coverage sits beside the code it proves. They still render the real with the real store, hook and transport; only the API and brokaw are faked. The fake backend and render harness they share with ConnectingOAuthJobs-test move to src/utilities/test/connectingOAuthHarness.tsx. ConnectingOAuthJobs-test keeps its original eight tests unchanged and no longer needs the fadeOut mock. Co-Authored-By: Claude Fable 5.1 --- .../__tests__/runJobSchedule-test.tsx | 162 +++++++++ src/utilities/test/connectingOAuthHarness.tsx | 200 ++++++++++ .../__tests__/ConnectingOAuthJobs-test.tsx | 343 +----------------- 3 files changed, 372 insertions(+), 333 deletions(-) create mode 100644 src/utilities/__tests__/runJobSchedule-test.tsx create mode 100644 src/utilities/test/connectingOAuthHarness.tsx diff --git a/src/utilities/__tests__/runJobSchedule-test.tsx b/src/utilities/__tests__/runJobSchedule-test.tsx new file mode 100644 index 0000000000..7011eecb04 --- /dev/null +++ b/src/utilities/__tests__/runJobSchedule-test.tsx @@ -0,0 +1,162 @@ +import { Subject } from 'rxjs' +import { waitFor } from 'src/utilities/testingLibrary' +import { WebSocketConnection } from 'src/context/WebSocketContext' +import { POST_MESSAGES } from 'src/const/postMessages' +import { ReadableStatuses } from 'src/const/Statuses' +import { JOB_TYPES } from 'src/const/consts' +import { STEPS, VERIFY_MODE } from 'src/const/Connect' +import { ACTIONABLE_ERROR_CODES } from 'src/views/actionableError/consts' +import { + connectedMemberRunning, + createFakeBackend, + createStore, + expectMemberConnected, + HttpError, + Member, + REDIRECT_JOB_GUID, + renderConnecting, + staleOAuthMember, +} from 'src/utilities/test/connectingOAuthHarness' + +// fadeOut (Velocity) never resolves in jsdom; Connecting's error path dispatches inside its .then. +vi.mock('src/utilities/Animation', () => ({ fadeOut: vi.fn(() => Promise.resolve()) })) + +/** + * runJobSchedule$ drives the Connecting step's job schedule. These tests exercise it through the + * real (real store, hook and transport) with the network and brokaw faked. + * + * CT-2332: firefly sets the member CONNECTED on the OAuth redirect before any job exists, and + * over websockets that frame can arrive after the widget has started its job. + */ +describe('runJobSchedule$ through over websockets', () => { + const createWebSocket = () => { + // Plain Subject: like brokaw, no replay for late subscribers. + const messages$ = new Subject<{ event: string; payload: Member }>() + const connection: WebSocketConnection = { + isConnected: () => true, + webSocketMessages$: messages$.asObservable(), + } + + return { messages$, connection } + } + + const memberUpdated = (payload: Member) => ({ event: 'members/updated', payload }) + + const impededMember = (jobGuid: string): Member => ({ + ...staleOAuthMember, + connection_status: ReadableStatuses.IMPEDED, + most_recent_job_guid: jobGuid, + error: { error_code: ACTIONABLE_ERROR_CODES.NO_ELIGIBLE_ACCOUNTS }, + }) + + // Lets runJob settle so the schedule has subscribed to the socket before frames are sent. + const settle = () => new Promise((resolve) => setTimeout(resolve, 0)) + + const expectActionableErrorInsteadOfSuccess = async ( + store: ReturnType, + onPostMessage: ReturnType, + ) => { + const lastStep = () => { + const { location } = store.getState().connect + return location[location.length - 1]?.step + } + + // Wait for any step, then assert which: before the fix this routes to CONNECTED at once. + await waitFor(() => expect(lastStep()).toBeDefined(), { timeout: 4000 }) + expect(lastStep()).toBe(STEPS.ACTIONABLE_ERROR) + expect(onPostMessage).not.toHaveBeenCalledWith( + POST_MESSAGES.MEMBER_CONNECTED, + expect.anything(), + ) + } + + afterEach(() => { + vi.restoreAllMocks() + }) + + it('ignores a late CONNECTED update with no job and lands on the actionable error when the job we started is impeded', async () => { + const backend = createFakeBackend() + const { messages$, connection } = createWebSocket() + + const { onPostMessage, store } = renderConnecting( + backend, + { mode: VERIFY_MODE }, + { webSocket: connection }, + ) + + await waitFor(() => expect(backend.runJob).toHaveBeenCalled()) + await settle() + + messages$.next( + memberUpdated({ ...staleOAuthMember, connection_status: ReadableStatuses.CONNECTED }), + ) + await settle() + messages$.next(memberUpdated(impededMember(`JOB-${JOB_TYPES.VERIFICATION}`))) + + await expectActionableErrorInsteadOfSuccess(store, onPostMessage) + }) + + it('ignores a late CONNECTED update that still names a returning member’s previous job', async () => { + const PREVIOUS_JOB_GUID = 'JOB-old' + const returningMember: Member = { + ...staleOAuthMember, + connection_status: ReadableStatuses.CONNECTED, + most_recent_job_guid: PREVIOUS_JOB_GUID, + } + const backend = createFakeBackend({ member: returningMember }) + // The previous job was also a verification, so attributing it by type would end the schedule. + backend.jobs[PREVIOUS_JOB_GUID] = { guid: PREVIOUS_JOB_GUID, job_type: JOB_TYPES.VERIFICATION } + const { messages$, connection } = createWebSocket() + + const { onPostMessage, store } = renderConnecting( + backend, + { mode: VERIFY_MODE }, + { webSocket: connection, member: returningMember }, + ) + + await waitFor(() => expect(backend.runJob).toHaveBeenCalled()) + await settle() + + messages$.next(memberUpdated(returningMember)) + await settle() + messages$.next(memberUpdated(impededMember(`JOB-${JOB_TYPES.VERIFICATION}`))) + + await expectActionableErrorInsteadOfSuccess(store, onPostMessage) + }) + + it('observes the job firefly assigned when its own runJob is rejected with a 409', async () => { + const backend = createFakeBackend() + const { messages$, connection } = createWebSocket() + + // disable_background_agg clients: firefly started the job on the redirect and rejects the + // widget's duplicate. Firefly's job is the one that matters from here on. + backend.runJob.mockImplementationOnce(async () => { + backend.startJob(REDIRECT_JOB_GUID, JOB_TYPES.VERIFICATION) + throw new HttpError(409) + }) + + const { onPostMessage } = renderConnecting( + backend, + { mode: VERIFY_MODE }, + { webSocket: connection }, + ) + + await waitFor(() => expect(backend.runJob).toHaveBeenCalled()) + await settle() + + // The late pre-job update is still ignored on this path. + messages$.next( + memberUpdated({ ...staleOAuthMember, connection_status: ReadableStatuses.CONNECTED }), + ) + await settle() + messages$.next(memberUpdated(connectedMemberRunning(REDIRECT_JOB_GUID))) + await settle() + messages$.next( + memberUpdated({ ...connectedMemberRunning(REDIRECT_JOB_GUID), is_being_aggregated: false }), + ) + + await expectMemberConnected(onPostMessage) + expect(backend.runJob).toHaveBeenCalledTimes(1) + expect(backend.loadJob).toHaveBeenCalledWith(REDIRECT_JOB_GUID) + }) +}) diff --git a/src/utilities/test/connectingOAuthHarness.tsx b/src/utilities/test/connectingOAuthHarness.tsx new file mode 100644 index 0000000000..7f51736f09 --- /dev/null +++ b/src/utilities/test/connectingOAuthHarness.tsx @@ -0,0 +1,200 @@ +import React from 'react' +import { createTestReduxStore, render, waitFor } from 'src/utilities/testingLibrary' +import { Connecting } from 'src/views/connecting/Connecting' +import { PostMessageContext } from 'src/ConnectWidget' +import { ApiContextTypes, ApiProvider } from 'src/context/ApiContext' +import { WebSocketConnection, WebSocketProvider } from 'src/context/WebSocketContext' +import { POST_MESSAGES } from 'src/const/postMessages' +import { ReadableStatuses } from 'src/const/Statuses' + +/** + * Shared harness for driving the real after OAuth against an in-memory + * backend: a stale PENDING member, jobs firefly or the widget start, and 409s when a job is + * already running. Used by ConnectingOAuthJobs-test and runJobSchedule-test. + */ + +export const MEMBER_GUID = 'MBR-oauth' +export const USER_GUID = 'USR-1' +export const REDIRECT_JOB_GUID = 'JOB-redirect' + +export type Member = { + guid: string + user_guid: string + connection_status: number + is_being_aggregated: boolean + most_recent_job_guid: string | null + is_oauth: boolean + error?: { error_code: number } | null +} + +export type Job = { guid: string; job_type: number; async_account_data_ready?: boolean } + +export class HttpError extends Error { + response: { status: number } + + constructor(status: number, message = 'Request failed') { + super(message) + this.name = 'HttpError' + this.response = { status } + } +} + +export const staleOAuthMember: Member = { + guid: MEMBER_GUID, + user_guid: USER_GUID, + connection_status: ReadableStatuses.PENDING, + is_being_aggregated: false, + most_recent_job_guid: null, + is_oauth: true, +} + +export const connectedMemberRunning = (jobGuid: string): Member => ({ + ...staleOAuthMember, + connection_status: ReadableStatuses.CONNECTED, + is_being_aggregated: true, + most_recent_job_guid: jobGuid, +}) + +export const createStore = ({ member = staleOAuthMember, useWebSockets = false } = {}) => + createTestReduxStore({ + connect: { + currentMemberGuid: MEMBER_GUID, + members: [member], + jobSchedule: { isInitialized: false, jobs: [] }, + location: [], + selectedInstitution: {}, + }, + experimentalFeatures: { + // With websockets on, frames drive the observation and polling is effectively off. + memberPollingMilliseconds: useWebSockets ? 60_000 : 10, + optOutOfEarlyUserRelease: false, + unavailableInstitutions: [], + useWebSockets, + }, + }) + +export const createFakeBackend = ({ + pollsUntilDone = 2, + earlyDataRelease = false, + member = staleOAuthMember, +} = {}) => { + const backend = { + member: { ...member } as Member, + jobs: {} as Record, + pollsWhileRunning: 0, + + startJob(guid: string, jobType: number) { + // With early data release the job reports its data as ready while it is + // still running, which makes member polling stop before the job finishes. + backend.jobs[guid] = { guid, job_type: jobType, async_account_data_ready: earlyDataRelease } + backend.member = connectedMemberRunning(guid) + }, + + loadMemberByGuid: vi.fn(async (): Promise => { + if (backend.member.is_being_aggregated) { + backend.pollsWhileRunning += 1 + + if (backend.pollsWhileRunning >= pollsUntilDone) { + backend.pollsWhileRunning = 0 + backend.member = { ...backend.member, is_being_aggregated: false } + } + } + + return backend.member + }), + + loadJob: vi.fn(async (guid: string): Promise => { + const job = backend.jobs[guid] + + if (!job) { + throw new HttpError(404) + } + + return job + }), + + runJob: vi.fn(async (jobType: number): Promise> => { + if (backend.member.is_being_aggregated) { + // Firefly returns a 409 when the member already has a running job. + throw new HttpError(409) + } + + backend.startJob(`JOB-${jobType}`, jobType) + + return {} + }), + } + + return backend +} + +export type FakeBackend = ReturnType + +/** + * Connecting throws `connectingError` during render so the host's error + * boundary can take over. Tests need a boundary of their own to observe that. + */ +class TestErrorBoundary extends React.Component< + { onError: (error: Error) => void; children: React.ReactNode }, + { hasError: boolean } +> { + state = { hasError: false } + + componentDidCatch(error: Error) { + this.props.onError(error) + } + + static getDerivedStateFromError() { + return { hasError: true } + } + + render() { + return this.state.hasError ?
: this.props.children + } +} + +export const renderConnecting = ( + backend: FakeBackend, + connectConfig: Record, + { webSocket, member }: { webSocket?: WebSocketConnection; member?: Member } = {}, +) => { + const onPostMessage = vi.fn() + const onError = vi.fn() + const api = { + loadMemberByGuid: backend.loadMemberByGuid, + loadJob: backend.loadJob, + runJob: backend.runJob, + } as unknown as ApiContextTypes + const store = createStore({ member, useWebSockets: !!webSocket }) + + const connecting = ( + + + + + + ) + + render( + + {webSocket ? ( + {connecting} + ) : ( + connecting + )} + , + { store }, + ) + + return { onPostMessage, onError, store } +} + +export const expectMemberConnected = (onPostMessage: ReturnType) => + waitFor( + () => + expect(onPostMessage).toHaveBeenCalledWith(POST_MESSAGES.MEMBER_CONNECTED, { + user_guid: USER_GUID, + member_guid: MEMBER_GUID, + }), + { timeout: 5000 }, + ) diff --git a/src/views/connecting/__tests__/ConnectingOAuthJobs-test.tsx b/src/views/connecting/__tests__/ConnectingOAuthJobs-test.tsx index 7209f3421a..85b1f6f359 100644 --- a/src/views/connecting/__tests__/ConnectingOAuthJobs-test.tsx +++ b/src/views/connecting/__tests__/ConnectingOAuthJobs-test.tsx @@ -1,19 +1,17 @@ -import React from 'react' -import { Subject } from 'rxjs' -import { createTestReduxStore, render, waitFor } from 'src/utilities/testingLibrary' -import { Connecting } from 'src/views/connecting/Connecting' -import { PostMessageContext } from 'src/ConnectWidget' -import { ApiContextTypes, ApiProvider } from 'src/context/ApiContext' -import { WebSocketConnection, WebSocketProvider } from 'src/context/WebSocketContext' +import { waitFor } from 'src/utilities/testingLibrary' import { POST_MESSAGES } from 'src/const/postMessages' import { ReadableStatuses } from 'src/const/Statuses' import { JOB_TYPES } from 'src/const/consts' -import { STEPS, VERIFY_MODE } from 'src/const/Connect' -import { ACTIONABLE_ERROR_CODES } from 'src/views/actionableError/consts' +import { VERIFY_MODE } from 'src/const/Connect' import { EXTRA_ITERATIONS_ALLOWED } from 'src/utilities/runJobSchedule' - -// fadeOut (Velocity) never resolves in jsdom; Connecting's error path dispatches inside its .then. -vi.mock('src/utilities/Animation', () => ({ fadeOut: vi.fn(() => Promise.resolve()) })) +import { + createFakeBackend, + expectMemberConnected, + HttpError, + REDIRECT_JOB_GUID, + renderConnecting, + staleOAuthMember, +} from 'src/utilities/test/connectingOAuthHarness' /** * CT-2495: after OAuth the widget lands on Connecting holding the member it @@ -24,190 +22,6 @@ vi.mock('src/utilities/Animation', () => ({ fadeOut: vi.fn(() => Promise.resolve * that used to leave the widget on this screen forever. */ -const MEMBER_GUID = 'MBR-oauth' -const USER_GUID = 'USR-1' -const REDIRECT_JOB_GUID = 'JOB-redirect' - -type Member = { - guid: string - user_guid: string - connection_status: number - is_being_aggregated: boolean - most_recent_job_guid: string | null - is_oauth: boolean - error?: { error_code: number } | null -} - -type Job = { guid: string; job_type: number; async_account_data_ready?: boolean } - -class HttpError extends Error { - response: { status: number } - - constructor(status: number, message = 'Request failed') { - super(message) - this.name = 'HttpError' - this.response = { status } - } -} - -const staleOAuthMember: Member = { - guid: MEMBER_GUID, - user_guid: USER_GUID, - connection_status: ReadableStatuses.PENDING, - is_being_aggregated: false, - most_recent_job_guid: null, - is_oauth: true, -} - -const connectedMemberRunning = (jobGuid: string): Member => ({ - ...staleOAuthMember, - connection_status: ReadableStatuses.CONNECTED, - is_being_aggregated: true, - most_recent_job_guid: jobGuid, -}) - -const createStore = ({ member = staleOAuthMember, useWebSockets = false } = {}) => - createTestReduxStore({ - connect: { - currentMemberGuid: MEMBER_GUID, - members: [member], - jobSchedule: { isInitialized: false, jobs: [] }, - location: [], - selectedInstitution: {}, - }, - experimentalFeatures: { - // With websockets on, frames drive the observation and polling is effectively off. - memberPollingMilliseconds: useWebSockets ? 60_000 : 10, - optOutOfEarlyUserRelease: false, - unavailableInstitutions: [], - useWebSockets, - }, - }) - -const createFakeBackend = ({ - pollsUntilDone = 2, - earlyDataRelease = false, - member = staleOAuthMember, -} = {}) => { - const backend = { - member: { ...member } as Member, - jobs: {} as Record, - pollsWhileRunning: 0, - - startJob(guid: string, jobType: number) { - // With early data release the job reports its data as ready while it is - // still running, which makes member polling stop before the job finishes. - backend.jobs[guid] = { guid, job_type: jobType, async_account_data_ready: earlyDataRelease } - backend.member = connectedMemberRunning(guid) - }, - - loadMemberByGuid: vi.fn(async (): Promise => { - if (backend.member.is_being_aggregated) { - backend.pollsWhileRunning += 1 - - if (backend.pollsWhileRunning >= pollsUntilDone) { - backend.pollsWhileRunning = 0 - backend.member = { ...backend.member, is_being_aggregated: false } - } - } - - return backend.member - }), - - loadJob: vi.fn(async (guid: string): Promise => { - const job = backend.jobs[guid] - - if (!job) { - throw new HttpError(404) - } - - return job - }), - - runJob: vi.fn(async (jobType: number): Promise> => { - if (backend.member.is_being_aggregated) { - // Firefly returns a 409 when the member already has a running job. - throw new HttpError(409) - } - - backend.startJob(`JOB-${jobType}`, jobType) - - return {} - }), - } - - return backend -} - -/** - * Connecting throws `connectingError` during render so the host's error - * boundary can take over. Tests need a boundary of their own to observe that. - */ -class TestErrorBoundary extends React.Component< - { onError: (error: Error) => void; children: React.ReactNode }, - { hasError: boolean } -> { - state = { hasError: false } - - componentDidCatch(error: Error) { - this.props.onError(error) - } - - static getDerivedStateFromError() { - return { hasError: true } - } - - render() { - return this.state.hasError ?
: this.props.children - } -} - -const renderConnecting = ( - backend: ReturnType, - connectConfig: Record, - { webSocket, member }: { webSocket?: WebSocketConnection; member?: Member } = {}, -) => { - const onPostMessage = vi.fn() - const onError = vi.fn() - const api = { - loadMemberByGuid: backend.loadMemberByGuid, - loadJob: backend.loadJob, - runJob: backend.runJob, - } as unknown as ApiContextTypes - const store = createStore({ member, useWebSockets: !!webSocket }) - - const connecting = ( - - - - - - ) - - render( - - {webSocket ? ( - {connecting} - ) : ( - connecting - )} - , - { store }, - ) - - return { onPostMessage, onError, store } -} - -const expectMemberConnected = (onPostMessage: ReturnType) => - waitFor( - () => - expect(onPostMessage).toHaveBeenCalledWith(POST_MESSAGES.MEMBER_CONNECTED, { - user_guid: USER_GUID, - member_guid: MEMBER_GUID, - }), - { timeout: 5000 }, - ) - describe(' after OAuth', () => { afterEach(() => { vi.restoreAllMocks() @@ -386,140 +200,3 @@ describe(' after OAuth', () => { await expectMemberConnected(onPostMessage) }) }) - -/** - * CT-2332: firefly sets the member CONNECTED on the OAuth redirect before any job exists, - * and over websockets that frame can arrive after the widget has started its job. - */ -describe(' after OAuth over websockets', () => { - const createWebSocket = () => { - // Plain Subject: like brokaw, no replay for late subscribers. - const messages$ = new Subject<{ event: string; payload: Member }>() - const connection: WebSocketConnection = { - isConnected: () => true, - webSocketMessages$: messages$.asObservable(), - } - - return { messages$, connection } - } - - const memberUpdated = (payload: Member) => ({ event: 'members/updated', payload }) - - const impededMember = (jobGuid: string): Member => ({ - ...staleOAuthMember, - connection_status: ReadableStatuses.IMPEDED, - most_recent_job_guid: jobGuid, - error: { error_code: ACTIONABLE_ERROR_CODES.NO_ELIGIBLE_ACCOUNTS }, - }) - - // Lets runJob settle so the schedule has subscribed to the socket before frames are sent. - const settle = () => new Promise((resolve) => setTimeout(resolve, 0)) - - const expectActionableErrorInsteadOfSuccess = async ( - store: ReturnType, - onPostMessage: ReturnType, - ) => { - const lastStep = () => { - const { location } = store.getState().connect - return location[location.length - 1]?.step - } - - // Wait for any step, then assert which: before the fix this routes to CONNECTED at once. - await waitFor(() => expect(lastStep()).toBeDefined(), { timeout: 4000 }) - expect(lastStep()).toBe(STEPS.ACTIONABLE_ERROR) - expect(onPostMessage).not.toHaveBeenCalledWith( - POST_MESSAGES.MEMBER_CONNECTED, - expect.anything(), - ) - } - - afterEach(() => { - vi.restoreAllMocks() - }) - - it('ignores a late CONNECTED update with no job and lands on the actionable error when the job we started is impeded', async () => { - const backend = createFakeBackend() - const { messages$, connection } = createWebSocket() - - const { onPostMessage, store } = renderConnecting( - backend, - { mode: VERIFY_MODE }, - { webSocket: connection }, - ) - - await waitFor(() => expect(backend.runJob).toHaveBeenCalled()) - await settle() - - messages$.next( - memberUpdated({ ...staleOAuthMember, connection_status: ReadableStatuses.CONNECTED }), - ) - await settle() - messages$.next(memberUpdated(impededMember(`JOB-${JOB_TYPES.VERIFICATION}`))) - - await expectActionableErrorInsteadOfSuccess(store, onPostMessage) - }) - - it('ignores a late CONNECTED update that still names a returning member’s previous job', async () => { - const PREVIOUS_JOB_GUID = 'JOB-old' - const returningMember: Member = { - ...staleOAuthMember, - connection_status: ReadableStatuses.CONNECTED, - most_recent_job_guid: PREVIOUS_JOB_GUID, - } - const backend = createFakeBackend({ member: returningMember }) - // The previous job was also a verification, so attributing it by type would end the schedule. - backend.jobs[PREVIOUS_JOB_GUID] = { guid: PREVIOUS_JOB_GUID, job_type: JOB_TYPES.VERIFICATION } - const { messages$, connection } = createWebSocket() - - const { onPostMessage, store } = renderConnecting( - backend, - { mode: VERIFY_MODE }, - { webSocket: connection, member: returningMember }, - ) - - await waitFor(() => expect(backend.runJob).toHaveBeenCalled()) - await settle() - - messages$.next(memberUpdated(returningMember)) - await settle() - messages$.next(memberUpdated(impededMember(`JOB-${JOB_TYPES.VERIFICATION}`))) - - await expectActionableErrorInsteadOfSuccess(store, onPostMessage) - }) - - it('observes the job firefly assigned when its own runJob is rejected with a 409', async () => { - const backend = createFakeBackend() - const { messages$, connection } = createWebSocket() - - // disable_background_agg clients: firefly started the job on the redirect and rejects the - // widget's duplicate. Firefly's job is the one that matters from here on. - backend.runJob.mockImplementationOnce(async () => { - backend.startJob(REDIRECT_JOB_GUID, JOB_TYPES.VERIFICATION) - throw new HttpError(409) - }) - - const { onPostMessage } = renderConnecting( - backend, - { mode: VERIFY_MODE }, - { webSocket: connection }, - ) - - await waitFor(() => expect(backend.runJob).toHaveBeenCalled()) - await settle() - - // The late pre-job update is still ignored on this path. - messages$.next( - memberUpdated({ ...staleOAuthMember, connection_status: ReadableStatuses.CONNECTED }), - ) - await settle() - messages$.next(memberUpdated(connectedMemberRunning(REDIRECT_JOB_GUID))) - await settle() - messages$.next( - memberUpdated({ ...connectedMemberRunning(REDIRECT_JOB_GUID), is_being_aggregated: false }), - ) - - await expectMemberConnected(onPostMessage) - expect(backend.runJob).toHaveBeenCalledTimes(1) - expect(backend.loadJob).toHaveBeenCalledWith(REDIRECT_JOB_GUID) - }) -}) From bc1ce76c51f32ee0c89210d217ed49fe01e081fe Mon Sep 17 00:00:00 2001 From: Logan Rasmussen Date: Fri, 25 Sep 2026 13:50:14 -0600 Subject: [PATCH 6/7] test(connecting): make the runJobSchedule scenarios read as given/when/then The Subject plumbing and settle calls moved into the harness behind a fake brokaw (`createFakeBrokaw().memberUpdated(member)`) and a `runJobCalled()` step on the rendered widget. Member fixtures are named for what they represent (connectedWithNoJob, runningJob, finishedJob, impededWithNoEligibleAccounts) and each test narrates its steps. Co-Authored-By: Claude Fable 5.1 --- .../__tests__/runJobSchedule-test.tsx | 187 ++++++++---------- src/utilities/test/connectingOAuthHarness.tsx | 38 +++- 2 files changed, 123 insertions(+), 102 deletions(-) diff --git a/src/utilities/__tests__/runJobSchedule-test.tsx b/src/utilities/__tests__/runJobSchedule-test.tsx index 7011eecb04..6b4ee75856 100644 --- a/src/utilities/__tests__/runJobSchedule-test.tsx +++ b/src/utilities/__tests__/runJobSchedule-test.tsx @@ -1,15 +1,12 @@ -import { Subject } from 'rxjs' import { waitFor } from 'src/utilities/testingLibrary' -import { WebSocketConnection } from 'src/context/WebSocketContext' import { POST_MESSAGES } from 'src/const/postMessages' import { ReadableStatuses } from 'src/const/Statuses' import { JOB_TYPES } from 'src/const/consts' import { STEPS, VERIFY_MODE } from 'src/const/Connect' import { ACTIONABLE_ERROR_CODES } from 'src/views/actionableError/consts' import { - connectedMemberRunning, createFakeBackend, - createStore, + createFakeBrokaw, expectMemberConnected, HttpError, Member, @@ -22,140 +19,128 @@ import { vi.mock('src/utilities/Animation', () => ({ fadeOut: vi.fn(() => Promise.resolve()) })) /** - * runJobSchedule$ drives the Connecting step's job schedule. These tests exercise it through the - * real (real store, hook and transport) with the network and brokaw faked. + * runJobSchedule$ drives the Connecting step's job schedule. These tests run it through the + * real (real store, hook and transport); only the API and brokaw are faked. * - * CT-2332: firefly sets the member CONNECTED on the OAuth redirect before any job exists, and - * over websockets that frame can arrive after the widget has started its job. + * CT-2332: firefly sets an OAuth member CONNECTED on the redirect before any job exists. + * Over websockets, a copy of that member update can reach the widget *after* it has started + * its own job. It looks finished (CONNECTED, not aggregating) but names no job, or the job the + * member had before. The widget must not mistake it for its job finishing. */ -describe('runJobSchedule$ through over websockets', () => { - const createWebSocket = () => { - // Plain Subject: like brokaw, no replay for late subscribers. - const messages$ = new Subject<{ event: string; payload: Member }>() - const connection: WebSocketConnection = { - isConnected: () => true, - webSocketMessages$: messages$.asObservable(), - } - - return { messages$, connection } - } - - const memberUpdated = (payload: Member) => ({ event: 'members/updated', payload }) - - const impededMember = (jobGuid: string): Member => ({ - ...staleOAuthMember, - connection_status: ReadableStatuses.IMPEDED, - most_recent_job_guid: jobGuid, - error: { error_code: ACTIONABLE_ERROR_CODES.NO_ELIGIBLE_ACCOUNTS }, - }) - // Lets runJob settle so the schedule has subscribed to the socket before frames are sent. - const settle = () => new Promise((resolve) => setTimeout(resolve, 0)) - - const expectActionableErrorInsteadOfSuccess = async ( - store: ReturnType, - onPostMessage: ReturnType, - ) => { - const lastStep = () => { - const { location } = store.getState().connect - return location[location.length - 1]?.step - } - - // Wait for any step, then assert which: before the fix this routes to CONNECTED at once. - await waitFor(() => expect(lastStep()).toBeDefined(), { timeout: 4000 }) - expect(lastStep()).toBe(STEPS.ACTIONABLE_ERROR) - expect(onPostMessage).not.toHaveBeenCalledWith( - POST_MESSAGES.MEMBER_CONNECTED, - expect.anything(), - ) - } +const OUR_JOB_GUID = `JOB-${JOB_TYPES.VERIFICATION}` + +// The member as firefly leaves it on the OAuth redirect: CONNECTED, idle, no job yet. +const connectedWithNoJob: Member = { + ...staleOAuthMember, + connection_status: ReadableStatuses.CONNECTED, +} + +const runningJob = (jobGuid: string): Member => ({ + ...connectedWithNoJob, + is_being_aggregated: true, + most_recent_job_guid: jobGuid, +}) + +const finishedJob = (jobGuid: string): Member => ({ + ...connectedWithNoJob, + most_recent_job_guid: jobGuid, +}) + +const impededWithNoEligibleAccounts = (jobGuid: string): Member => ({ + ...staleOAuthMember, + connection_status: ReadableStatuses.IMPEDED, + most_recent_job_guid: jobGuid, + error: { error_code: ACTIONABLE_ERROR_CODES.NO_ELIGIBLE_ACCOUNTS }, +}) + +const expectNoEligibleAccountsScreen = async (widget: ReturnType) => { + // Wait for Connecting to route anywhere, then check where. Before the fix it routed to + // CONNECTED as soon as the stale update arrived. + await waitFor(() => expect(widget.currentStep()).toBeDefined(), { timeout: 4000 }) + expect(widget.currentStep()).toBe(STEPS.ACTIONABLE_ERROR) + expect(widget.onPostMessage).not.toHaveBeenCalledWith( + POST_MESSAGES.MEMBER_CONNECTED, + expect.anything(), + ) +} +describe('runJobSchedule$ through over websockets', () => { afterEach(() => { vi.restoreAllMocks() }) - it('ignores a late CONNECTED update with no job and lands on the actionable error when the job we started is impeded', async () => { + it('ignores the late CONNECTED update with no job and shows the real outcome of the job it started', async () => { + // Given a first-time OAuth member: no job has ever run on it. const backend = createFakeBackend() - const { messages$, connection } = createWebSocket() - - const { onPostMessage, store } = renderConnecting( + const brokaw = createFakeBrokaw() + const widget = renderConnecting( backend, { mode: VERIFY_MODE }, - { webSocket: connection }, + { webSocket: brokaw.connection }, ) - await waitFor(() => expect(backend.runJob).toHaveBeenCalled()) - await settle() - - messages$.next( - memberUpdated({ ...staleOAuthMember, connection_status: ReadableStatuses.CONNECTED }), - ) - await settle() - messages$.next(memberUpdated(impededMember(`JOB-${JOB_TYPES.VERIFICATION}`))) + // When the widget starts its verification job... + await widget.runJobCalled() + // ...and firefly's pre-job update arrives late, looking finished but naming no job... + await brokaw.memberUpdated(connectedWithNoJob) + // ...then the widget's job actually finishes with no eligible accounts. + await brokaw.memberUpdated(impededWithNoEligibleAccounts(OUR_JOB_GUID)) - await expectActionableErrorInsteadOfSuccess(store, onPostMessage) + // Then the widget shows the error, never a success. + await expectNoEligibleAccountsScreen(widget) }) - it('ignores a late CONNECTED update that still names a returning member’s previous job', async () => { + it('ignores the late CONNECTED update that still names a returning member’s previous job', async () => { + // Given a returning member whose previous job was also a verification. Attributing that + // old job by type would wrongly complete the schedule. const PREVIOUS_JOB_GUID = 'JOB-old' - const returningMember: Member = { - ...staleOAuthMember, - connection_status: ReadableStatuses.CONNECTED, - most_recent_job_guid: PREVIOUS_JOB_GUID, - } + const returningMember = finishedJob(PREVIOUS_JOB_GUID) const backend = createFakeBackend({ member: returningMember }) - // The previous job was also a verification, so attributing it by type would end the schedule. backend.jobs[PREVIOUS_JOB_GUID] = { guid: PREVIOUS_JOB_GUID, job_type: JOB_TYPES.VERIFICATION } - const { messages$, connection } = createWebSocket() - - const { onPostMessage, store } = renderConnecting( + const brokaw = createFakeBrokaw() + const widget = renderConnecting( backend, { mode: VERIFY_MODE }, - { webSocket: connection, member: returningMember }, + { webSocket: brokaw.connection, member: returningMember }, ) - await waitFor(() => expect(backend.runJob).toHaveBeenCalled()) - await settle() - - messages$.next(memberUpdated(returningMember)) - await settle() - messages$.next(memberUpdated(impededMember(`JOB-${JOB_TYPES.VERIFICATION}`))) + // When the widget starts a new verification job... + await widget.runJobCalled() + // ...and firefly's pre-job update arrives late, still naming the previous job... + await brokaw.memberUpdated(returningMember) + // ...then the new job finishes with no eligible accounts. + await brokaw.memberUpdated(impededWithNoEligibleAccounts(OUR_JOB_GUID)) - await expectActionableErrorInsteadOfSuccess(store, onPostMessage) + // Then the widget shows the error, never a success. + await expectNoEligibleAccountsScreen(widget) }) it('observes the job firefly assigned when its own runJob is rejected with a 409', async () => { + // Given firefly already started the verification job on the redirect + // (disable_background_agg clients), so the widget's own runJob is a duplicate. const backend = createFakeBackend() - const { messages$, connection } = createWebSocket() - - // disable_background_agg clients: firefly started the job on the redirect and rejects the - // widget's duplicate. Firefly's job is the one that matters from here on. backend.runJob.mockImplementationOnce(async () => { backend.startJob(REDIRECT_JOB_GUID, JOB_TYPES.VERIFICATION) throw new HttpError(409) }) - - const { onPostMessage } = renderConnecting( + const brokaw = createFakeBrokaw() + const widget = renderConnecting( backend, { mode: VERIFY_MODE }, - { webSocket: connection }, + { webSocket: brokaw.connection }, ) - await waitFor(() => expect(backend.runJob).toHaveBeenCalled()) - await settle() - - // The late pre-job update is still ignored on this path. - messages$.next( - memberUpdated({ ...staleOAuthMember, connection_status: ReadableStatuses.CONNECTED }), - ) - await settle() - messages$.next(memberUpdated(connectedMemberRunning(REDIRECT_JOB_GUID))) - await settle() - messages$.next( - memberUpdated({ ...connectedMemberRunning(REDIRECT_JOB_GUID), is_being_aggregated: false }), - ) + // When the widget's runJob is rejected... + await widget.runJobCalled() + // ...the late pre-job update is still ignored... + await brokaw.memberUpdated(connectedWithNoJob) + // ...and firefly's job is seen running, then finishing. + await brokaw.memberUpdated(runningJob(REDIRECT_JOB_GUID)) + await brokaw.memberUpdated(finishedJob(REDIRECT_JOB_GUID)) - await expectMemberConnected(onPostMessage) + // Then the widget completes against firefly's job without starting another. + await expectMemberConnected(widget.onPostMessage) expect(backend.runJob).toHaveBeenCalledTimes(1) expect(backend.loadJob).toHaveBeenCalledWith(REDIRECT_JOB_GUID) }) diff --git a/src/utilities/test/connectingOAuthHarness.tsx b/src/utilities/test/connectingOAuthHarness.tsx index 7f51736f09..11f1cd875f 100644 --- a/src/utilities/test/connectingOAuthHarness.tsx +++ b/src/utilities/test/connectingOAuthHarness.tsx @@ -1,4 +1,5 @@ import React from 'react' +import { Subject } from 'rxjs' import { createTestReduxStore, render, waitFor } from 'src/utilities/testingLibrary' import { Connecting } from 'src/views/connecting/Connecting' import { PostMessageContext } from 'src/ConnectWidget' @@ -130,6 +131,30 @@ export const createFakeBackend = ({ export type FakeBackend = ReturnType +// Yields one macrotask so the widget's pending promises and subscriptions settle. +const settle = () => new Promise((resolve) => setTimeout(resolve, 0)) + +/** + * Stands in for brokaw, the websocket server. `memberUpdated(member)` delivers a + * `members/updated` frame to the widget and waits for it to be processed. Like brokaw, + * nothing is replayed to late subscribers, so send frames only once the widget is observing. + */ +export const createFakeBrokaw = () => { + const frames$ = new Subject<{ event: string; payload: Member }>() + + const connection: WebSocketConnection = { + isConnected: () => true, + webSocketMessages$: frames$.asObservable(), + } + + const memberUpdated = async (member: Member) => { + frames$.next({ event: 'members/updated', payload: member }) + await settle() + } + + return { connection, memberUpdated } +} + /** * Connecting throws `connectingError` during render so the host's error * boundary can take over. Tests need a boundary of their own to observe that. @@ -186,7 +211,18 @@ export const renderConnecting = ( { store }, ) - return { onPostMessage, onError, store } + // Resolves once the widget has asked the backend to run a job and is observing the result. + const runJobCalled = async () => { + await waitFor(() => expect(backend.runJob).toHaveBeenCalled()) + await settle() + } + + const currentStep = () => { + const { location } = store.getState().connect + return location[location.length - 1]?.step + } + + return { onPostMessage, onError, store, runJobCalled, currentStep } } export const expectMemberConnected = (onPostMessage: ReturnType) => From 6dd26a56cf120274015d360b8bd2d4e9304eac6d Mon Sep 17 00:00:00 2001 From: Logan Rasmussen Date: Fri, 25 Sep 2026 14:02:37 -0600 Subject: [PATCH 7/7] test(connecting): let the shared render helper own the store and ApiProvider in the harness The harness no longer hand-builds a redux store or nests its own ApiProvider; it passes preloadedState and apiValue to src/utilities/testingLibrary's render, like the rest of the suite. Exports are trimmed to what the two test files use. Test behavior is unchanged. Co-Authored-By: Claude Fable 5.1 --- src/utilities/test/connectingOAuthHarness.tsx | 120 +++++++++--------- 1 file changed, 60 insertions(+), 60 deletions(-) diff --git a/src/utilities/test/connectingOAuthHarness.tsx b/src/utilities/test/connectingOAuthHarness.tsx index 11f1cd875f..c1d15ac41e 100644 --- a/src/utilities/test/connectingOAuthHarness.tsx +++ b/src/utilities/test/connectingOAuthHarness.tsx @@ -1,21 +1,20 @@ import React from 'react' import { Subject } from 'rxjs' -import { createTestReduxStore, render, waitFor } from 'src/utilities/testingLibrary' +import { render, waitFor } from 'src/utilities/testingLibrary' import { Connecting } from 'src/views/connecting/Connecting' import { PostMessageContext } from 'src/ConnectWidget' -import { ApiContextTypes, ApiProvider } from 'src/context/ApiContext' +import { ApiContextTypes } from 'src/context/ApiContext' import { WebSocketConnection, WebSocketProvider } from 'src/context/WebSocketContext' import { POST_MESSAGES } from 'src/const/postMessages' import { ReadableStatuses } from 'src/const/Statuses' /** - * Shared harness for driving the real after OAuth against an in-memory - * backend: a stale PENDING member, jobs firefly or the widget start, and 409s when a job is - * already running. Used by ConnectingOAuthJobs-test and runJobSchedule-test. + * Drives the real after OAuth against fakes for the two things a test cannot + * use for real: the backend (firefly/persona) and brokaw (websockets). */ -export const MEMBER_GUID = 'MBR-oauth' -export const USER_GUID = 'USR-1' +const MEMBER_GUID = 'MBR-oauth' +const USER_GUID = 'USR-1' export const REDIRECT_JOB_GUID = 'JOB-redirect' export type Member = { @@ -28,7 +27,7 @@ export type Member = { error?: { error_code: number } | null } -export type Job = { guid: string; job_type: number; async_account_data_ready?: boolean } +type Job = { guid: string; job_type: number; async_account_data_ready?: boolean } export class HttpError extends Error { response: { status: number } @@ -40,6 +39,8 @@ export class HttpError extends Error { } } +// The member the widget holds when it lands on Connecting: created before the user left for +// the institution, so PENDING and without a job. export const staleOAuthMember: Member = { guid: MEMBER_GUID, user_guid: USER_GUID, @@ -49,31 +50,11 @@ export const staleOAuthMember: Member = { is_oauth: true, } -export const connectedMemberRunning = (jobGuid: string): Member => ({ - ...staleOAuthMember, - connection_status: ReadableStatuses.CONNECTED, - is_being_aggregated: true, - most_recent_job_guid: jobGuid, -}) - -export const createStore = ({ member = staleOAuthMember, useWebSockets = false } = {}) => - createTestReduxStore({ - connect: { - currentMemberGuid: MEMBER_GUID, - members: [member], - jobSchedule: { isInitialized: false, jobs: [] }, - location: [], - selectedInstitution: {}, - }, - experimentalFeatures: { - // With websockets on, frames drive the observation and polling is effectively off. - memberPollingMilliseconds: useWebSockets ? 60_000 : 10, - optOutOfEarlyUserRelease: false, - unavailableInstitutions: [], - useWebSockets, - }, - }) - +/** + * In-memory firefly/persona. `startJob` puts the member into aggregation for that job, + * `loadMemberByGuid` lets it go idle after `pollsUntilDone` polls, and `runJob` answers 409 + * while a job is already running, as firefly does. + */ export const createFakeBackend = ({ pollsUntilDone = 2, earlyDataRelease = false, @@ -88,7 +69,12 @@ export const createFakeBackend = ({ // With early data release the job reports its data as ready while it is // still running, which makes member polling stop before the job finishes. backend.jobs[guid] = { guid, job_type: jobType, async_account_data_ready: earlyDataRelease } - backend.member = connectedMemberRunning(guid) + backend.member = { + ...backend.member, + connection_status: ReadableStatuses.CONNECTED, + is_being_aggregated: true, + most_recent_job_guid: guid, + } }, loadMemberByGuid: vi.fn(async (): Promise => { @@ -116,7 +102,6 @@ export const createFakeBackend = ({ runJob: vi.fn(async (jobType: number): Promise> => { if (backend.member.is_being_aggregated) { - // Firefly returns a 409 when the member already has a running job. throw new HttpError(409) } @@ -129,15 +114,13 @@ export const createFakeBackend = ({ return backend } -export type FakeBackend = ReturnType - // Yields one macrotask so the widget's pending promises and subscriptions settle. const settle = () => new Promise((resolve) => setTimeout(resolve, 0)) /** - * Stands in for brokaw, the websocket server. `memberUpdated(member)` delivers a - * `members/updated` frame to the widget and waits for it to be processed. Like brokaw, - * nothing is replayed to late subscribers, so send frames only once the widget is observing. + * Stands in for brokaw. `memberUpdated(member)` delivers a `members/updated` frame and waits + * for the widget to process it. Like brokaw, nothing is replayed to late subscribers, so send + * frames only once the widget is observing. */ export const createFakeBrokaw = () => { const frames$ = new Subject<{ event: string; payload: Member }>() @@ -155,10 +138,8 @@ export const createFakeBrokaw = () => { return { connection, memberUpdated } } -/** - * Connecting throws `connectingError` during render so the host's error - * boundary can take over. Tests need a boundary of their own to observe that. - */ +// Connecting throws `connectingError` during render so the host's error boundary can take +// over. Tests need a boundary of their own to observe that. class TestErrorBoundary extends React.Component< { onError: (error: Error) => void; children: React.ReactNode }, { hasError: boolean } @@ -179,28 +160,25 @@ class TestErrorBoundary extends React.Component< } export const renderConnecting = ( - backend: FakeBackend, + backend: ReturnType, connectConfig: Record, - { webSocket, member }: { webSocket?: WebSocketConnection; member?: Member } = {}, + { + webSocket, + member = staleOAuthMember, + }: { webSocket?: WebSocketConnection; member?: Member } = {}, ) => { const onPostMessage = vi.fn() const onError = vi.fn() - const api = { - loadMemberByGuid: backend.loadMemberByGuid, - loadJob: backend.loadJob, - runJob: backend.runJob, - } as unknown as ApiContextTypes - const store = createStore({ member, useWebSockets: !!webSocket }) + // The shared render helper hard-codes a no-op onPostMessage and has no websocket context, + // so those two are provided here. const connecting = ( - - - - - + + + ) - render( + const { store } = render( {webSocket ? ( {connecting} @@ -208,7 +186,29 @@ export const renderConnecting = ( connecting )} , - { store }, + { + apiValue: { + loadMemberByGuid: backend.loadMemberByGuid, + loadJob: backend.loadJob, + runJob: backend.runJob, + } as unknown as ApiContextTypes, + preloadedState: { + connect: { + currentMemberGuid: MEMBER_GUID, + members: [member], + jobSchedule: { isInitialized: false, jobs: [] }, + location: [], + selectedInstitution: {}, + }, + experimentalFeatures: { + // With websockets on, frames drive the observation and polling is effectively off. + memberPollingMilliseconds: webSocket ? 60_000 : 10, + optOutOfEarlyUserRelease: false, + unavailableInstitutions: [], + useWebSockets: !!webSocket, + }, + }, + }, ) // Resolves once the widget has asked the backend to run a job and is observing the result. @@ -222,7 +222,7 @@ export const renderConnecting = ( return location[location.length - 1]?.step } - return { onPostMessage, onError, store, runJobCalled, currentStep } + return { onPostMessage, onError, runJobCalled, currentStep } } export const expectMemberConnected = (onPostMessage: ReturnType) =>