refactor: convert sequence data (fetchSequence) to React Query by brian-smith-tcril · Pull Request #2061 · openedx/frontend-app-learning

brian-smith-tcril · 2026-09-15T03:34:53Z

Summary

Convert the fetchSequence thunk to a useSequenceMetadata React Query hook plus a transitional useSequenceStatusBridge, mirroring the pattern #2023 established for the courseware metadata/outline fetches. No user-facing change — the bridge keeps the Redux sequences/units model store and the sequenceStatus / sequenceId / sequenceMightBeUnit slice fields written, so the not-yet-converted readers (the container's redirect helpers, Sequence, breadcrumbs, sequence-navigation, sequence-alerts, the outline sidebar) behave exactly as before.

Part of the Redux → React Query migration (#1946, Stage 1), stacked above #2060 (the Course.test.jsx waitFor fix this branch depends on). Resolves #2011.

What changed

useSequenceMetadata (apiHooks.ts) — the RQ query; meta.models mirrors the sequence + its units into the model store via the QueryCache onSuccess bridge. retry: false so the expected 422 unit-detection fails fast (the parent-sequence redirect only fires once sequenceStatus === 'failed'; the default ~7s backoff would stall it).
silent logger tier (queryClient.ts) — a new no-op log level, so the expected 422 is tagged logStatusAs: { 422: 'silent' } and stays unlogged, matching the old thunk (which logged nothing on 422). Reusable for any future "surface as error to callers, but don't log" status.
useSequenceStatusBridge (statusBridge.ts) — a transitional effect hook that writes the still-Redux slice status fields from the query state.
CoursewareContainer — drops the checkFetchSequence guard and calls the bridge; the redirect helpers/selectors are untouched.
isPreview in the query key — the preview view can return different data for the same sequence, so keeping it in the key faithfully reproduces the old refetch-on-mode-change (omitting it would serve stale other-mode data).
Cleanup — the fetchSequence thunk and its re-export are deleted; the test seed moves to seedSequenceModels in setupTest.js.

The decision log covers the full rationale — the silent tier, retry: false, the non-sequential throw, the isPreview key, why there's no denied state, the seed split, and the test-altitude choices.

Testing

npm run types (0 errors), npm run lint (clean), and the full suite (112 suites, 1103 passed, 3 pre-existing skips, 0 failures) all green. Manual smoke on tutor local (DemoX) confirmed the two #2011-unique behaviors — the expected 422 fails fast (no stall) and logs nothing, and the sequence fetch goes out with preview=1 in preview mode. Two unrelated pre-existing issues surfaced and were ruled out (filed #2059; an LMS problem-render error reproduces on the pre-RQ baseline).

Decisions

Full decision log

Decisions — #2011 (convert sequence data to React Query)

Working log; folded into the PR's Full decision log at push time.

Two-part bridge pattern (reused from #2023)

The sequence fetch converts the same way metadata/outline did:

useSequenceMetadata (apiHooks.ts) — the RQ query. Its meta.models mirrors the
result into the model store via the bridgeToModelStore QueryCache onSuccess:
sequence → sequences (updateModel, merge) and units → units (updateModels,
merge). These are exactly the two dispatches the old fetchSequence thunk made.
useSequenceStatusBridge (statusBridge.ts) — a transitional effect hook that
writes the still-Redux slice fields (sequenceId / sequenceStatus /
sequenceMightBeUnit) from the query state, so the many not-yet-converted readers (the
container redirect helpers, Sequence.jsx, breadcrumbs, sequence-navigation,
sequence-alerts, the outline sidebar) keep working unchanged.

CoursewareContainer drops its checkFetchSequence memoize-guard + dispatch(fetchSequence)
and instead calls useSequenceStatusBridge(routeSequenceId, isPreview) next to
useCourseStatusBridge. The redirect helpers/selectors are untouched.

Why checkFetchSequence is fully removed but checkFetchCourse survives. Same
status-bridge mechanism, different conversion completeness. fetchSequence had no
responsibility beyond the fetch + model mirroring + status fields, all of which the bridge +
meta.models now cover — so the thunk and its container guard are deleted outright. fetchCourse,
by contrast, was only thinned by #2023: useCourseStatusBridge took over its status derivation,
but the thunk still owns one un-converted responsibility — the sidebar-toggles fetch
(getCoursewareOutlineSidebarToggles → setCoursewareOutlineSidebarToggles) — so
checkFetchCourse still dispatches the thinned thunk alongside the bridge. That residue is #2013's
job; when it lands, checkFetchCourse disappears and the two cases look identical. So the "same
pattern" claim is about the bridge, not about the thunk being gone — fetchSequence is simply a
complete one-step conversion, whereas fetchCourse is mid-conversion.

The sequence bridge has no `denied` state (faithful, not an omission)

useSequenceStatusBridge dispatches only fetchSequenceRequest / Success / Failure — no
denied, unlike useCourseStatusBridge (which has fetchCourseDenied → DENIED). This is faithful:
the slice has no fetchSequenceDenied action, the old fetchSequence thunk never dispatched a denied,
and no reader checks for a denied sequenceStatus. Adding one would invent a status nothing produces
or consumes.

The asymmetry is intentional and comes from where each kind of access-gating lives:

Course access denial is a fetch status — derived from courseHomeMeta.courseAccess.hasAccess
(fetchCourseDenied → DENIED); whether you may enter the course is known at metadata time.
Sequence access-gating is data, not status — a sequence fetch either loads (a sequential
block → success) or fails (422 / error → failure); gated/locked content and prerequisites ride
inside the payload (gatedContent and friends) and are rendered by the Sequence component, so
there's no denied fetch state for the bridge to map.

Sequence query key includes `isPreview`

coursewareQueryKeys.sequence(sequenceId, isPreview) puts isPreview in the key because it changes
the request: the queryFn calls getSequenceMetadata(sequenceId, { preview: isPreview ? '1' : '0' }),
and the preview view can return different sequence/unit data for the same sequenceId. (The
metadata/outline keys have no isPreview because those requests never took a preview flag — only
the sequence fetch did.)

Including isPreview is the faithful choice, not an enhancement — omitting it would be the
regression. Redux had no query-key cache: fetchSequence(sequenceId, isPreview) used isPreview
only to shape the request and wrote the result to models.sequences[sequenceId] via updateModel,
which keys purely by the sequence's id — a single slot per sequenceId, refetched-and-overwritten
on every dispatch (so a mode change gave you the new mode's data). To reproduce that in RQ, the mode
must be part of the key: with it, flipping preview changes the key and refetches; without it, the
key wouldn't change on a mode flip and RQ would serve the cached other-mode data (e.g. the preview
route showing non-preview content) until it went stale — a bug Redux never had.

No behavior change now, none post-bridge. Today the model-store bridge (meta.models) mirrors
both variants back into the same models.sequences[sequenceId] slot (keyed by id), so current
model-store readers see the old single-slot behavior. Post-#1977, when readers consume
useSequenceMetadata().data directly, a correct reader always reads its current mode's entry —
exactly what Redux's slot held for the current mode; the other-mode entry is inert. The only genuinely
new thing is that such a reader must supply isPreview to select an entry (a Redux reader got the
single slot for free) — but that's explicit, not silent: isPreview is a required parameter of
the hook, and every sequence reader derives preview from the same source (the route prefix) the
container already uses. In practice it's moot day-to-day anyway — preview is a whole-session mode, not
something toggled for one sequence mid-session.

`retry: false` on the sequence query

The old thunk was a single axios call with no retry. The app's production query client
(src/queryClient.ts) sets no retry, so RQ's default (3× exponential backoff, ~7s) would
otherwise apply. Two reasons this must be false:

Faithfulness — match the old no-retry behavior.
The expected 422 must fail fast. The container detects "this URL segment is
actually a unit" from the sequence fetch returning 422 (sequenceMightBeUnit); the
parent-sequence redirect only fires once sequenceStatus === 'failed'. Under the default
retry, that status would stay pending for ~7s → a visible navigation stall. retry: false
is the one setting whose absence would regress behavior.

Interaction with #2024 (smart retry): when that lands (retry 5xx/network, skip 4xx globally),
a per-query retry: false still wins. Revisit then whether sequence should opt into 5xx
retry; for now no-retry is the safe, faithful default.

Expected 422 telemetry: kept silent via a new `silent` tier

The old thunk logged nothing on the expected 422 (if (!sequenceMightBeUnit) logError(error)
skipped it) — because it's routine control flow, not a failure: the container requests a maybe-unit
as a sequence and reads the 422 as sequenceMightBeUnit. That happens on a normal path (bare-unit /
section+unit redirect resolution), so it must not enter error telemetry.

In the RQ model the query genuinely ends in error state (it must — the container reads
sequenceStatus === 'failed' to drive the redirect), so onError fires for it. Originally loggers
in queryClient.ts offered only error / info, so the only way to keep 422 out of error logging
was info — a real behavior change (silent → info) accepted only because no silent level existed.

Rather than accept that change for a faithful port, added a third silent level to loggers (a
typed no-op) and tagged the 422 meta.logStatusAs: { 422: 'silent' }. Net: the 422 is truly
silent again, matching the thunk exactly — no telemetry change. The tier is reusable for any future
"surface as error to callers, but don't log" status. Covered by a queryClient.test.ts case.

Not to be confused with the outline 403. The 403 was already logged at info in the pre-#2023
thunk (explicit logInfo on 403, logError otherwise), so #2023's logStatusAs: { 403: 'info' }
faithfully preserved it — the 403 is the precedent for the info-via-logStatusAs mechanism, not for
a silent→info change. Only the 422 was silent, which is why it (and not the 403) needed the new tier.

Non-sequential block → throw (not a separate failure dispatch)

The sequence API can return a non-sequential block (e.g. a chapter); the old thunk
logError'd and dispatched fetchSequenceFailure (no mightBeUnit). In the hook the
queryFn throws instead, so: (a) the QueryCache onSuccess bridge does not mirror a
non-sequential block, (b) the query goes to error state → the status bridge sets
sequenceStatus: 'failed' with sequenceMightBeUnit: false (thrown Error has no 422), and
(c) the single onError path logs it once via logError (thrown Error has no status → default
error level).

The error message text is a verbatim port of the thunk's logError string
(Requested sequence '…' has block type '…'; expected block type 'sequential'.) — character-identical.
The old thunk split it across two concatenated template literals to satisfy max-len; the hook writes
it as one template literal (it fits) and hands it to throw new Error(...) instead of logError(...).
So the same text still reaches logError, just routed through onError rather than called inline.
Nothing about the wording was invented for the conversion.

Error handling: throw and let `onError` log, no inline `logError`/try-catch

General pattern behind the two cases above. The old thunk logged imperatively inside a try/catch:
its catch did if (!sequenceMightBeUnit) logError(error) — i.e. logError for any non-422
failure, silent for the expected 422 — then dispatched fetchSequenceFailure. The hook has no
try/catch and no inline logError: the queryFn either returns { sequence, units } or throws
(its own non-sequential Error, or the axios error propagating), and all logging is centralized in
the query client's single onError, which picks the level from meta.logStatusAs. The mapping is
faithful:

non-sequential block → thrown Error, no status → onError default error level (was inline
logError(message)).
generic/network failure → axios error propagates, no matching logStatusAs → error level (was
inline logError(error)).
expected 422 → logStatusAs: { 422: 'silent' } → not logged (matches the old silent behavior; see
the telemetry note above for the silent tier).

Why: it removes hand-rolled error plumbing from each hook and makes logging a declarative property of
the query (meta.logStatusAs), consistent with useCoursewareOutline's 403 handling. With the
silent tier added, all three paths are faithful to the thunk — no telemetry change.

`isPreview` dropped from `CoursewareContainer`'s `latest` ref

latest.current.isPreview was read only by the deleted checkFetchSequence; removed as dead.
The isPreview variable is still used (redirect helpers + the new bridge call).

Full cleanup: `fetchSequence` thunk deleted, test seed migrated

The thunk had no production caller after the container change, so it's deleted (from
thunks.js and the index.js re-export) along with its now-unused imports
(getSequenceMetadata, fetchSequence{Request,Success,Failure}). The slice actions stay —
now dispatched by useSequenceStatusBridge, and Sequence.test.jsx still uses
fetchSequenceFailure directly.

Seed helper, not folded into seedCoursewareModels. The test harness previously seeded
sequences suite-wide via executeThunk(fetchSequence(...)). I added a separate
seedSequenceModels(store, sequenceIds) to setupTest.js (dispatches updateModel sequence

updateModels units + fetchSequenceSuccess, the same shape the bridge produces) rather
than folding it into seedCoursewareModels. Reason: the two seeds are deliberately distinct —
seedCoursewareModels populates the outline's partial sequence data (no gatedContent /
activeUnitIndex / units); the sequence seed adds the full metadata. Keeping them separate
preserves that two-phase distinction (and matches how the runtime has an outline query and a
separate sequence query).

The setupTest.js import delta is just "inline the thunk's body": seedSequenceModels dispatches
the same three actions fetchSequence did on success, so the four things the thunk used to
encapsulate are now imported directly — getSequenceMetadata (the fetch), updateModel /
updateModels (the model-store writes), and fetchSequenceSuccess (the status dispatch) — while
fetchSequence itself is dropped from the imports. (Each seed also mirrors its query's meta.models
action variants: courseware's map-shaped payloads use *ModelsMap, the sequence's object+array uses
updateModel + updateModels.)

Tests

redux.test.js — removed the Test fetchSequence describe (networkError / non-sequential /
normalize+mirror) and the consts only it used (learningSequencesUrlRegExp, courseUrl,
courseHomeMetadataUrl, courseHomeMetadata, the seedCoursewareModels/buildOutlineFromBlocks
imports). Reseated the "Thunks that require fetched sequences" beforeEach onto
seedSequenceModels.
statusBridge.test.ts — added a useSequenceStatusBridge describe (mocked hook +
useDispatch, same pattern as the course bridge): no-id → nothing, pending → request,
success → success, 422 → failure with sequenceMightBeUnit: true, non-422 → failure with
false.
apiHooks.test.tsx — added a useSequenceMetadata describe that tests the kept hook's own
contract: success asserts result.current.data deep-equals the normalized
{ sequence, units }; a non-sequential block asserts the query errors with the thrown
"expected block type 'sequential'" message; a 422 asserts the query errors and
getResponseStatus(error) === 422 (which also makes it distinct from the non-sequential case);
and a retrying client (retry: 3, retryDelay: 0) still issues exactly one GET —
proving the hook's own retry: false (the harness's createTestQueryClient disables retry
globally, so a dedicated client is needed to observe the hook-level setting).

Every removed `fetchSequence` test has an equivalent (split by responsibility)

The deleted thunk did two jobs at once — fetch/mirror and status dispatch — so each old test's
assertions were verified against the new split coverage, not dropped. The mapping:

Removed case (`redux.test.js`)	Fetch / error surfaced	Status field	Logging
networkError → `logError` + `failed`	error surfaced (covered by the 422 / non-seq `isError` tests)	`statusBridge.test.ts` "non-422 failure" → `fetchSequenceFailure` sets `failed`	`queryClient.test.ts` "reports query errors through onError" (generic error → `logError`)
non-sequential → `logError` + `failed`	`apiHooks.test.tsx` "throws for a non-sequential block type" (isError + message)	`statusBridge.test.ts` "non-422 failure" (`mightBeUnit: false`)	same `onError` test (thrown Error → `error` level)
success → mirror seq+units + `loaded`/id	`apiHooks.test.tsx` "fetches and normalizes…" (`data` = full normalized; stronger than the old `objectContaining`)	`statusBridge.test.ts` "succeeds when the query resolves" → `fetchSequenceSuccess`	n/a

One assertion changed altitude — flag for review. The old success test seeded the outline first
(partial sequence in the store), then fetched, and asserted the partial got enriched
(objectContaining gatedContent/activeUnitIndex) — implicitly testing that updateModel merges
rather than replaces. The new apiHooks test renders the hook cold and asserts full normalized
data; it does not re-test merge-into-partial. That merge is now covered by composition —
modelStoreBridge.test.ts (bridge → updateModel) plus the model store's own updateModel merge
semantics — rather than by a single sequence-specific before/after integration test. Consistent with
the altitude split below, but it's the one behavior no longer asserted end-to-end in a sequence test.

Test altitude: the hook tests assert the hook, not the bridge

The first draft of the apiHooks.test.tsx tests rendered useSequenceMetadata through the bridged
query client and asserted the model-store result (models.sequences / models.units). That
coupled the tests for a hook we intend to keep to the model-store bridge we intend to remove
(#1977): once the bridge goes, those assertions lose meaning and the kept hook would be left with
almost no coverage. It also duplicated src/data/modelStoreBridge.test.ts, which already covers the
mirroring mechanism generically.

Reworked so each layer is tested at its own altitude:

The kept hook (apiHooks.test.tsx) asserts only its durable output — result.current.data,
the thrown-error message, the surfaced 422 status, and retry: false. These survive the bridge's
removal untouched.
The mirroring (model-store side effect) is left to modelStoreBridge.test.ts; no per-hook
mirror assertions.
The status-field derivation (sequenceStatus / sequenceMightBeUnit / logError) stays in
statusBridge.test.ts, which is correctly a transitional bridge test — it lives and dies with
useSequenceStatusBridge.

This maps cleanly onto the responsibility split: the old fetchSequence thunk did fetch+mirror
and status dispatch at once; the conversion separates those, and the tests follow — durable
behavior tested at the durable layer, transitional behavior at the transitional layer.

`Course.test.jsx` — fixed four latent un-awaited `waitFor` leaks (surfaced, not caused, by this change)

Split out as the bottom stack layer. Because this fix is unrelated to sequences (test hygiene
the conversion merely surfaced), it lives in its own PR below #2011 in the stack, so the #2011
diff stays sequence-only. It's a prerequisite: this branch's suite is red without it. Kept
documented here for context; the details below explain the bug and fix it carries.

The full suite went red on my branch (green on base) in a test unrelated to sequences:
Course › displays learner tools. Root cause: four
fire-and-forget waitFor(...) calls (no await) elsewhere in Course.test.jsx whose
background polls throw after their own test ends, so Jest misattributes the rejection to
whichever test runs next.

line 351 (passes handlers to the sequence) — un-awaited waitFor → leaked a
"cannot find /previous/i" error.
lines 380 / 414 / 448 (Sequence alerts display) — worse: waitFor(() => expect(screen.findByText(X)).toBeInTheDocument()). findByText returns a Promise, so
the matcher ran against a promise (received value {}) — and un-awaited, so these three
tests asserted nothing and passed vacuously.

These are pre-existing bugs (the file already documents this class — see its it.skip
comments about "improper waitFor use", #1669); base only passed because the leaks' timing
happened not to collide. This conversion shifts seed timing (async getSequenceMetadata
instead of a thunk), which makes them collide reliably.

Fix: await the line-351 waitFor; rewrite the three alert assertions as
expect(await screen.findByText(X)).toBeInTheDocument(). All four now genuinely assert and
pass in isolation, and the full suite is green twice consecutively. No production
sequence-nav regression — verified: with the line-351 waitFor properly awaited, "passes
handlers to the sequence" passes (Previous/Next links render, handlers fire as expected).

Left untouched: three un-awaited waitFors inside it.skip blocks (they don't run).

Verification

Pre-split all-in-one WIP: npm run types ✓ · npm run lint ✓ · full suite green ×2 (111
suites, 936 passed, 3 pre-existing skips). Base full-suite run (changes stashed) also green,
confirming the only behavioral delta is the intended conversion.
After the review-driven forward edits — the apiHooks.test.tsx hook-altitude rework, and the
silent tier (queryClient.ts + apiHooks.ts logStatusAs: { 422: 'silent' } + a
queryClient.test.ts case): npm run types ✓ · npm run lint ✓.
Full suite (final, post-rework + post-split): npm run types ✓ · npm run lint ✓ · full suite
green — 112 suites, 1103 passed, 3 pre-existing skips, 0 failures.
Manual smoke (tutor local, DemoX, user tutorsuper — see the manual-testing notes): the two
Convert sequence data to React Query #2011-unique behaviors confirmed in the browser — the expected 422 fails fast (no ~7s stall on
the unit-detection redirects) and logs nothing (the silent tier), and the sequence fetch goes
out with preview=1 in preview mode. The non-sequential (chapter) case logs at error as
expected/faithful. Two unrelated issues surfaced and ruled out: a pre-existing course-block info
log (normalizeOutlineBlocks, from [FC-0056] Course outline sidebar #1375 — filed as Spurious info log for the root course block in normalizeOutlineBlocks #2059), and an LMS problem-render error that
reproduces on the pre-RQ baseline db2134c (env/content, not the migration).

Manual testing

Manual testing — sequence data (`fetchSequence`) → React Query (#2011)

In-browser verification for the top-of-stack PR, run against a live backend (tutor
local). This conversion claims no user-facing change: one sequence's metadata + its
units now load via useSequenceMetadata (React Query) instead of the fetchSequence
thunk, are mirrored into the Redux models.sequences / models.units store through the
bridge, and useSequenceStatusBridge keeps writing state.courseware.sequenceStatus /
sequenceId / sequenceMightBeUnit so the still-Redux readers (the container's redirect
helpers, Sequence, breadcrumbs, sequence-navigation, sequence-alerts, the outline
sidebar) behave exactly as before.

Two things are unique to this conversion and can't be fully exercised by jest — verify
them with real redirects and real telemetry:

The expected 422 must fail fast. The container detects "this URL segment is
actually a unit, not a sequence" from the sequence fetch returning 422
(sequenceMightBeUnit), and the parent-sequence redirect only fires once
sequenceStatus === 'failed'. The hook sets retry: false, so the 422 resolves
immediately; without it React Query's default backoff (~7s) would stall the redirect.
The expected 422 must log nothing. It's routine control flow, so it's tagged
meta.logStatusAs: { 422: 'silent' } (a no-op logger) — matching the old thunk, which
logged nothing on 422. Confirm no error and no info page-action fires for it.

Routes involved (DECODE_ROUTES):

courseware: /course/:courseId/:sequenceId/:unitId (+ shorter forms + /preview/...)

Getting real IDs (DemoX on tutor local)

Course id: course-v1:OpenedX+DemoX+DemoCourse. The URLs below are app-relative —
prefix with your learning MFE origin (whatever host is in the address bar, e.g.
http://apps.local.openedx.io/learning). The : and + in the course id go in the URL
literally.

sequenceId + unitId — navigate to any unit; the address bar reads
/course/course-v1:OpenedX+DemoX+DemoCourse/<SEQ>/<UNIT>. Copy the two block ids
(…type@sequential+block@… = sequence, …type@vertical+block@… = unit).
sectionId (chapter) — never shows up in normal nav. Open the course, DevTools →
Network, filter course_outline; in the response JSON each outline.sections[] has
.id (the …type@chapter+block@… section id) and .sequence_ids — grab a section
.id and note its .sequence_ids[0] (where it should redirect).

Verify by hand

Highest-risk (the two #2011-unique behaviors) first. Each item has a click-ready URL for
this instance (base http://apps.local.openedx.io:2000/learning, course
course-v1:OpenedX+DemoX+DemoCourse). Redirects use replace, so after each one the
address bar shows the normalized URL — to retest, paste the source URL fresh instead of
using the back button.

Left to the automated suite (not re-done by hand)

useSequenceMetadata fetch/normalize/throw/retry: false — apiHooks.test.tsx.
useSequenceStatusBridge state derivation (request/success/422-failure/non-422-failure)
— statusBridge.test.ts.
The silent logger tier + QueryCache onError level selection — queryClient.test.ts.
The full redirect matrix incl. the 422 unit-detection case — CoursewareContainer.test.jsx.
The reseated sequence seed (seedSequenceModels) and the surviving thunk tests —
redux.test.js.
Model-store mirroring of sequences/units (meta.models) — modelStoreBridge.test.ts.

Results

Env: tutor local, course-v1:OpenedX+DemoX+DemoCourse, user tutorsuper.

(fill in items as checked — note anything surprising, especially any stall on the 422 path
or any log emitted for the expected 422)

🤖 Generated with Claude Code

codecov · 2026-09-15T03:39:55Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.67%. Comparing base (65dbeb8) to head (4e6f76b).

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #2061   +/-   ##
=======================================
  Coverage   93.66%   93.67%           
=======================================
  Files         368      368           
  Lines        6015     6019    +4     
  Branches     1417     1420    +3     
=======================================
+ Hits         5634     5638    +4     
  Misses        364      364           
  Partials       17       17

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Convert the fetchSequence thunk to a useSequenceMetadata query plus a transitional useSequenceStatusBridge, mirroring the pattern established for the course metadata/outline fetches. - useSequenceMetadata (apiHooks.ts): retry:false so the expected 422 unit-detection fails fast; meta.models mirrors the sequence and its units into the model store; the expected 422 is tagged logStatusAs: { 422: 'silent' } (a new no-op logger tier in queryClient.ts) so it stays unlogged, matching the old thunk. - useSequenceStatusBridge (statusBridge.ts): writes sequenceId / sequenceStatus / sequenceMightBeUnit from the query state so the still-Redux readers keep working. - CoursewareContainer: drop the checkFetchSequence guard and call the bridge. - Delete the fetchSequence thunk and its re-export; migrate the test seed to seedSequenceModels in setupTest.js. Part of #1946 (Convert Learning from Redux to Context + React Query). Resolves #2011. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

arbrandes

👍🏼

brian-smith-tcril added this pull request to stack #2062 September 15, 2026 03:34

brian-smith-tcril marked this pull request as ready for review September 15, 2026 05:01

This was referenced Sep 15, 2026

Peel: convert checkBlockCompletion to a React Query mutation #2012

Closed

refactor: convert checkBlockCompletion to a React Query mutation #2063

Merged

brian-smith-tcril requested a review from arbrandes September 15, 2026 17:17

Base automatically changed from bsmith/course-test-waitfor-hygiene to master September 18, 2026 14:58

arbrandes force-pushed the bsmith/react-query-sequence-data branch from 42377a3 to 4e6f76b Compare September 18, 2026 14:58

arbrandes approved these changes Sep 18, 2026

View reviewed changes

arbrandes merged commit c587f2a into master Sep 18, 2026
7 checks passed

arbrandes deleted the bsmith/react-query-sequence-data branch September 18, 2026 16:13

Navigation Menu

refactor: convert sequence data (fetchSequence) to React Query - #2061

refactor: convert sequence data (fetchSequence) to React Query#2061
arbrandes merged 1 commit into
masterfrom
bsmith/react-query-sequence-data

brian-smith-tcril commented Sep 15, 2026 •

edited

Loading

Uh oh!

codecov Bot commented Sep 15, 2026 •

edited

Loading

Uh oh!

arbrandes left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

brian-smith-tcril commented Sep 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

What changed

Testing

Decisions

Decisions — #2011 (convert sequence data to React Query)

Two-part bridge pattern (reused from #2023)

The sequence bridge has no denied state (faithful, not an omission)

Sequence query key includes isPreview

retry: false on the sequence query

Expected 422 telemetry: kept silent via a new silent tier

Non-sequential block → throw (not a separate failure dispatch)

Error handling: throw and let onError log, no inline logError/try-catch

isPreview dropped from CoursewareContainer's latest ref

Full cleanup: fetchSequence thunk deleted, test seed migrated

Tests

Every removed fetchSequence test has an equivalent (split by responsibility)

Test altitude: the hook tests assert the hook, not the bridge

Course.test.jsx — fixed four latent un-awaited waitFor leaks (surfaced, not caused, by this change)

Verification

Manual testing — sequence data (fetchSequence) → React Query (#2011)

Getting real IDs (DemoX on tutor local)

Verify by hand

Left to the automated suite (not re-done by hand)

Results

Uh oh!

codecov Bot commented Sep 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

arbrandes left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

brian-smith-tcril commented Sep 15, 2026 •

edited

Loading

The sequence bridge has no `denied` state (faithful, not an omission)

Sequence query key includes `isPreview`

`retry: false` on the sequence query

Expected 422 telemetry: kept silent via a new `silent` tier

Error handling: throw and let `onError` log, no inline `logError`/try-catch

`isPreview` dropped from `CoursewareContainer`'s `latest` ref

Full cleanup: `fetchSequence` thunk deleted, test seed migrated

Every removed `fetchSequence` test has an equivalent (split by responsibility)

`Course.test.jsx` — fixed four latent un-awaited `waitFor` leaks (surfaced, not caused, by this change)

Manual testing — sequence data (`fetchSequence`) → React Query (#2011)

codecov Bot commented Sep 15, 2026 •

edited

Loading