refactor: convert the courseware metadata fetch to React Query - #2023
Draft
brian-smith-tcril wants to merge 2 commits into
Draft
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## bsmith/react-query-courseware-container-typescript #2023 +/- ##
======================================================================================
- Coverage 93.53% 93.49% -0.04%
======================================================================================
Files 363 367 +4
Lines 5905 5946 +41
Branches 1367 1374 +7
======================================================================================
+ Hits 5523 5559 +36
- Misses 367 371 +4
- Partials 15 16 +1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
brian-smith-tcril
force-pushed
the
bsmith/react-query-courseware-metadata
branch
2 times, most recently
from
August 21, 2026 14:48
54b53a9 to
b9eb81e
Compare
brian-smith-tcril
force-pushed
the
bsmith/react-query-courseware-metadata
branch
2 times, most recently
from
August 21, 2026 18:24
27fc4d9 to
bd4c718
Compare
brian-smith-tcril
force-pushed
the
bsmith/react-query-courseware-metadata
branch
from
August 21, 2026 18:32
bd4c718 to
11de7ef
Compare
brian-smith-tcril
force-pushed
the
bsmith/react-query-courseware-metadata
branch
from
August 21, 2026 18:34
11de7ef to
e145ef9
Compare
brian-smith-tcril
force-pushed
the
bsmith/react-query-courseware-metadata
branch
from
August 21, 2026 18:40
e145ef9 to
b72083e
Compare
brian-smith-tcril
force-pushed
the
bsmith/react-query-courseware-metadata
branch
from
August 21, 2026 18:55
b72083e to
fb407a8
Compare
brian-smith-tcril
force-pushed
the
bsmith/react-query-courseware-metadata
branch
from
August 21, 2026 19:05
fb407a8 to
8d9a956
Compare
brian-smith-tcril
force-pushed
the
bsmith/react-query-courseware-metadata
branch
from
August 21, 2026 19:15
8d9a956 to
e861457
Compare
brian-smith-tcril
force-pushed
the
bsmith/react-query-courseware-metadata
branch
from
August 21, 2026 19:23
e861457 to
29b1274
Compare
brian-smith-tcril
force-pushed
the
bsmith/react-query-courseware-metadata
branch
from
August 21, 2026 21:19
29b1274 to
462f871
Compare
Extend the transitional QueryCache bridge so a query can mirror collection
results (and several model targets) into the models store, not just a single
addModel. Adds a `meta.models` list — each entry runs a model-store action
(addModel/updateModel/addModelsMap/updateModelsMap/updateModels) with an optional
`source` key into the result — while keeping the existing `{ modelType, courseId }`
single form. No runtime effect until a query opts in (wired up in the courseware
metadata conversion that follows in this PR).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Convert fetchCourse's metadata/outline/courseHomeMeta fetches to React Query, mirroring results into the model store via the bridge so the existing useModel readers keep working. - Query hooks: useCoursewareMetadata + useCoursewareOutline (courseware/data/apiHooks + queryKeys); reuse useCourseHomeMeta (now typed — enabled guard + courseAccess). courseId is string|undefined from useParams with an `enabled` guard; key factories stay strict string. - Status: CoursewareContainer and CourseExit call transitional bridges (courseware/data/statusBridge) that mirror the combined query state into state.courseware.courseStatus/courseId, so the redirect helpers/selectors, TabPage, and the exit-page children keep working until they move to React Query. - fetchCourse is thinned to just the sidebar-toggles fetch (its full conversion is #2013); its model writes move to the model-store bridge and its status derivation to the status bridges. - Query error logging: the app QueryCache onError logs failures; a query can override the level per HTTP status via meta.logStatusAs (the outline's expected 403 -> logInfo). meta is typed globally via a Register.queryMeta augmentation, so onError and the bridge read it cast-free. - useIFrameBehavior's post-event refetch invalidates the courseware queries instead of dispatching fetchCourse. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
brian-smith-tcril
force-pushed
the
bsmith/react-query-courseware-metadata
branch
from
August 22, 2026 07:05
462f871 to
a2eb96d
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Convert the courseware metadata fetch (
fetchCourse) to React Query, and extend the model-store bridge to mirror collection results. Part of the Redux → React Query migration (#1946), the #1976 courseware decomposition (Target 1); stacked on theCoursewareContainerde-class (#2020) and TypeScript (#2021) layers as the new top of the stack. Two commits — #2009 (bridge → collections) and #2010 (courseware metadata). Closes #2009, #2010.fetchCoursefetched four things (course metadata, the learning-sequences outline, course-home metadata, sidebar toggles) and derivedcourseStatus. This moves the three data fetches to query hooks (mirrored into the model store via the bridge so the existinguseModelreaders keep working), moves the status derivation into transitional bridge hooks, and thinsfetchCourseto just the un-converted sidebar-toggles fetch.What changed
courseware/data/apiHooks.ts/queryKeys.ts(new) —useCoursewareMetadata(getCourseMetadata) anduseCoursewareOutline(getLearningSequencesOutline), tagged withmeta.modelsso the bridge fans each result into the right model(s).courseIdisstring | undefined(fromuseParams) with anenabledguard; key factories stay strictstring.course-home/data/modelStoreBridge.ts(Peel: extend the model-store bridge to collection dispatches #2009) — the bridge'smetagains amodels: [{ modelType, strategy, source? }]list form, so one query result can fan out to several model targets with the right add-vs-merge semantics (the outline writes three:coursewareMeta/sections/sequences).courseware/data/statusBridge.ts(new) —useCourseStatusBridge(container) anduseCourseExitStatusBridge(CourseExit) run the queries and mirror their combined state intostate.courseware.courseStatus/courseId, so the still-Redux readers (redirect helpers, selectors,TabPage's string status, the exit-page children) keep working. Transitional — removed when those readers move to React Query.CoursewareContainer.tsx— callsuseCourseStatusBridge(routeCourseId); the memoizedcheckFetchCourseguard is byte-identical to base (it now dispatches the thinnedfetchCourse).CourseExit.jsx— self-wrapping on the query hooks viauseCourseExitStatusBridge, renderingTabPageitself.courseware/data/thunks.js—fetchCoursethinned to just the sidebar-toggles fetch (its full conversion is Convert the courseware outline sidebar to React Query + context #2013); its model writes move to the bridge and its status derivation to the status bridges.queryClient.ts/data/http-error.ts— the appQueryCache'sonErrorlogs query failures; a query overrides the level per HTTP status viameta.logStatusAs: { <status>: <level> }(the outline's expected 403 →logInfo).metais typed globally via aRegister.queryMetaaugmentation, soonErrorand the bridge read it cast-free. Implements the last piece of Restore dropped query error logging via a global QueryCache.onError #2022.useIFrameBehavior.ts— the post-event refetch invalidates the courseware queries instead of dispatchingfetchCourse.index.jsx— the courseware route drops the<TabContainer fetch={fetchCourse}>wrapper for a bare<CourseExit />.Behavior
No user-facing change. Metadata/outline/courseHomeMeta now load via React Query and populate the model store through the bridge;
courseStatus/courseIdare still written (transitionally, by the status bridges) so the redirects, gating, andTabPagebehave as before. The sidebar toggles still load via the thinnedfetchCourse. Query error logging is preserved, including the outline's403 → logInfo(a logged-out learner's expected redirect stays a New Relic page action, not a noticed error).Testing
npm run types,npm run lint, and the fullnpm testsuite pass (109 suites, 912 passing, 3 pre-existing skips). NewqueryClient.test.tscoversonError(defaultlogError,logStatusAsoverride) and the model-store bridge;modelStoreBridge.test.tscovers the list-form fan-out; the container / CourseExit / useIFrameBehavior / ProductTours tests render through the bridged query client;setupTest'sseedCoursewareModelsreplaces theexecuteThunk(fetchCourse)seed. Manual browser verification is still pending — keeping this PR in draft until that's done.Decisions
Full decision log
Decisions — courseware metadata → React Query (+ bridge collections) (#2009 + #2010)
Working notes for this PR (part of the wider Redux → React Query migration,
#1946). Not checked in — referenced when opening the PR. Part of the #1976
courseware decomposition (Target 1). Stacked on the de-class (#2020) and the TS
conversion (#2021).
Why #2009 and #2010 are one PR (two commits). #2009 (the bridge extension) has
no runtime effect on its own — nothing uses the new
metaform until #2010 wiresa query to it — and we expect to tweak the bridge while doing #2010. Landing them
together ships a "real" chunk (courseware metadata actually on React Query) instead of
a dormant infra PR followed by its only consumer. Two commits keep the concerns
legible: commit 1 = bridge, commit 2 = the metadata conversion.
Part 1 — extend the model-store bridge to collections (#2009, commit 1)
Extend the transitional bridge (
src/course-home/data/modelStoreBridge.ts) so a querycan mirror collection results (and several model targets) into the
modelsstore,not just a single
addModel.Why: the courseware producers write collections, and one fetch → many models
fetchCoursewrites four model types with mixed strategies; the outline endpoint alonewrites three from one response:
getCourseMetadatacoursewareMetaaddModel(data has its own id)getCourseHomeCourseMetadatacourseHomeMetaaddModelkeyed by courseIdgetLearningSequencesOutline→.coursescoursewareMetaupdateModelsMap(merge sectionIds)getLearningSequencesOutline→.sectionssectionsaddModelsMapgetLearningSequencesOutline→.sequencessequencesupdateModelsMap(merge)The bridge runs in the QueryCache
onSuccess, which receives the query's rawresult, so a query whose result is
{ courses, sections, sequences }must fan that oneresult out to three mirrors.
Contract: keep the single form, add a
modelslist{ modelType, courseId }(unchanged) — mirror the whole result as one model keyedby courseId. The course-home tabs use this; byte-compatible.
{ models: [{ modelType, strategy, source? }] }(new) — one or more mirrors.strategyis a model-store action (addModel/updateModel/addModelsMap/updateModelsMap/updateModels);sourceselects a key of the result (omitted =the whole result). Lets one query populate several targets with the right add-vs-merge
semantics;
sourceis what avoids splitting the outline into 3 fetches.Alternatives considered
collectionflag. Rejected: the bridge sees the raw result andmetais one-target-per-query, so the outline's 3-in-1 shape couldn't be expressedwithout 3 separate fetches (3× network) or a bespoke
onSuccess.North star
The bridge is throwaway scaffolding, deleted with the model store in #1977. After that,
readers stop calling
useModel(...)and read from the RQ hooks directly. A couple ofmodels are assembled from two endpoints (
sequences= outline shallow +getSequenceMetadatafull;coursewareMeta=getCourseMetadata+ outlinesectionIds), so those readers combine the relevant hooks — a #2011/#2013/#1977concern, not this PR.
Tests
modelStoreBridge.test.tsdrives real queries throughcreateModelStoreQueryCache(store)and asserts the resulting
modelsstate: single form, list-form fan-out viasource,updateModelsMapmerging (not clobbering),updateModelsover an array, and the no-opwhen meta is absent.
Part 2 — convert courseware metadata to React Query (#2010, commit 2)
Convert the courseware metadata fetch (
fetchCourse) to React Query, mirroring into themodel store via the Part-1 bridge so the ~14
useModelreaders keep working.Scope: convert the metadata/outline/courseHomeMeta fetches; thin
fetchCoursein placefetchCoursedid four fetches (metadata, outline, courseHomeMeta, sidebar toggles) + setcourseStatus. Its consumers:CoursewareContainer(player), the CourseExit route(
<TabContainer fetch={fetchCourse}>),useIFrameBehavior(refetch on an iframe event), andsetupTest'sinitializeTestStore. Decision: move the three data fetches to RQ hooks andthe status derivation into the container, then thin
fetchCoursein place so it does onlythe un-converted remainder (the sidebar toggles) — rather than deleting it and adding a new
fetchCoursewareOutlineSidebarTogglesthunk.deletes
fetchCoursethen. Renaming/replacing it now is churn for a transitional step —CoursewareContainer's guard (checkFetchCourse→dispatch(fetchCourse(id))) staysbyte-identical to the base, and
fetchCoursevisibly shrinks (4 fetches → 1) acrosslayers until it's gone. Transitional cost:
fetchCourseis briefly a misnomer (it onlyfetches toggles now).
useIFrameBehaviordouble-populating models on the player.The query hooks (new
courseware/data/apiHooks.ts+queryKeys.ts)useCoursewareMetadata(courseId)→getCourseMetadata,meta: { models: [{ modelType: 'coursewareMeta', strategy: 'addModel' }] }(result has its own id).{ modelType, courseId }form (asuseCourseHomeMetauses)? The single form (bridge line 44) doesaddModel({ model: { id: courseId, ...data } })— its purpose is to injectcourseIdas the id, which
courseHomeMetaneeds because its payload has no id of its own.coursewareMeta's payload has its own id, andfetchCoursestored it withaddModel({ model: metadata })(no injection). The array form +strategy: 'addModel'(no
source) maps exactly to that. The two happen to produce the same stored value here(in
{ id: courseId, ...data }the spread wins, anddata.id === courseIdon thisroute), so the single form wouldn't break — but the array form (a) expresses the real
intent ("store by the model's own id") instead of relying on the spread-override
coincidence, (b) is the byte-analog of
fetchCourse's call, and (c) keeps both coursewarehooks on one contract (the outline hook must use the array form to fan out to 3 targets).
useCoursewareOutline(courseId)→getLearningSequencesOutline,meta: { models: [ courses→coursewareMeta/updateModelsMap, sections→addModelsMap, sequences→updateModelsMap ] }.useCourseHomeMetaforcourseHomeMeta— don't re-fetch it.useCourseHomeMetafetches the'outline'rootSlug variant;fetchCourseused'courseware'.rootSlugonly renames the courseware tab'sslugin the normalized
tabs;courseAccess(what the gating reads) is identical. Soreuse is faithful for gating; the only difference is that tab's slug.
courseIdtyping: guarded hooks, strict keys (a migration-wide convention)useParams()types every route param asstring | undefined— React Router can't provewhich route a component renders under — even though
:courseIdis always present on thecourseware/course-exit routes. This bit only surfaces now because #2010 adds the first
typed (
.tsx) caller of these hooks (CoursewareContainer); the existing course-homecallers are all
.jsx, so the argument was never type-checked. Every future.tsxconversion hits the same thing, so we picked one convention:
string | undefined+enabled: !!courseId. The hook honestly toleratesthe
useParamstype by not firing when the id is absent (the standard React Queryidiom for "param may not be ready"). Call sites pass
useParamsstraight through — nocasts or guards proliferating across the migration. No-op for the 6 existing
.jsxcallers, since courseId is never actually undefined there.
string(neverundefined). A query key is a real identity; akey on
undefinedis meaningless. SoqueryKeys.tsstays strict.!at the key call inside the hook —coursewareQueryKeys.metadata(courseId!).This is deliberate, not sloppy:
queryKeyis evaluated eagerly (React Query computesit every render regardless of
enabled), so the factory is still called when the queryis disabled. The
!says "keys are built from real ids"; the adjacentenabled: !!courseIdis what actually makes the never-happens undefined case safe (no fetch). Runtime-wise the
!is purely type-level.Alternative considered — narrow
courseIdtostringonce at the.tsxboundary(then keys and hooks are
string, noenabled, no!). Rejected: it relocates theuseParamsundefined into a guard/assertion at every typed boundary — thecast-in-the-wrong-place friction from #2019 — instead of handling it once, idiomatically,
in the hook. (This also reverts a
string→string | undefinedwidening ofuseCourseHomeMeta/courseHomeQueryKeys.metadatathat #2010 briefly introduced before wesettled on this convention.)
Error logging: global
QueryCache.onError+ the outline's 403 nuancefetchCourselogged each endpoint independently:logErroron failed metadata/courseHomeMeta/toggles, and for the outline a
403 ? logInfo : logErrorsplit (a 403 there is the expectedaccess-denied case — the learner is redirected — so it's a New Relic page action, not a
noticed error that pollutes the error dashboard).
React Query v5 removed
onErrorfromuseQuery(it's only onuseMutationand theQueryCache), so per-hook query logging isn't possible. The home for query error logging is theglobal
QueryCache.onError— permanent app infra, introduced with the QueryCache in #1987and tracked by #2022. By default it
logErrors.What #2010 adds — the courseware outline is the one query that surfaces a 403 as an error
(its getter throws it; the course-home tab getters swallow 401/403/404 →
{}, so their queryerrors are only genuine failures). So:
useCoursewareOutlinetagsmeta: { logStatusAs: { 403: 'info' } }.onErrorreadsquery.meta.logStatusAs— a status →LogLevelmap, defaulting toerror(it says how to log each status, not a "quiet" flag). So an outline 403 →logInfo, restoringfetchCourse's behavior; anything else →logError.LogLevel(
'error' | 'info') derives from a{ error: logError, info: logInfo }map — the only twologgers platform exposes.
fetchCourse'scatch → logError.Typed
meta, no casts.metais typed globally via aRegister.queryMetaaugmentation(
ModelStoreMeta & { logStatusAs?: Record<number, LogLevel> }), so bothonErrorand themodel-store bridge read
query.metawithout a cast, andmetaliterals are checked at thewrite site.
getResponseStatus(data/http-error.ts) reads the error's status. At #1977 theaugmentation drops its
ModelStoreMetahalf along with the bridge.Migration-wide context (#2022): other converted queries dropped their thunks'
logErrortoo,but their getters swallow 401/403/404, so the global
onError(plainlogError) already coverstheir genuine failures without per-query
meta. The outline is the exception that needs thelogStatusAstag.Access-gating + status: a transitional
useCourseStatusBridgeThe old
fetchCoursederivedcourseStatus(request → success/denied/failure) fromcourseAccess.hasAccess+ outline success and dispatchedfetchCourse{Request,Success, Denied,Failure}. That derivation moves out of the thunk into a transitional bridge hook,useCourseStatusBridge(courseware/data/statusBridge.ts): it runs the three query hooks andmirrors their combined state into
state.courseware.courseStatusvia the same status actions,so the still-Redux readers (the container's redirect helpers/selectors,
TabPage's stringstatus,
useContextId) keep working.CoursewareContainerjust callsuseCourseStatusBridge(routeCourseId).Why a bridge, and why a component hook (not the model-store one): it's the same "keep Redux
populated from RQ transitionally" idea as the model-store bridge, but
courseStatusis aderivation across all three queries — which a per-query
QueryCache.onSuccesscan't express —so it's a component-level hook, not part of the centralized bridge. Deleted when those readers
move to RQ.
The status reducers stay.
fetchCourseis thinned, not deleted (see the scope section), socheckFetchCoursestays too — it just dispatches the thinnedfetchCourse(toggles only). RQauto-fetches on courseId change, so no separate metadata-fetch guard is needed.
CourseExit route → self-wrapping (+ transitional slice write for the exit children)
<TabContainer tab="courseware" fetch={fetchCourse}>becomesCourseExitself-wrappingon the query hooks (rendering
TabPageitself), matching the course-home tab pattern.Regression found & fixed: the course-exit children (
CourseCelebration,CourseNonPassing,CourseInProgress, and the recommendation/upgrade helpers) readcourseIdfromstate.courseware— whichfetchCourseused to set on this route. With thethunk gone, that slice field is unset on the CourseExit route, so
useModel('courseHomeMeta', undefined)returns{}andtabs.find(...)throws. Rather than convert all ~7 children offthe slice (that's #1976's job),
CourseExitwritescourseId/courseStatusto the slicetransitionally via
useCourseExitStatusBridge(courseware/data/statusBridge.ts) — theCourseExit sibling of
useCourseStatusBridge(2 queries, no outline). CourseExit owns thetwo queries (it also feeds them to its own
TabPagegating), so it passes them into thebridge rather than the bridge returning them. The children keep reading the slice until they
convert.
useIFrameBehaviorrefetch → query invalidationThe iframe
POST_EVENThandler'spostEvent.mutateonSuccessdiddispatch(fetchCourse(courseId)); it nowqueryClient.invalidateQueriesthe three query keys(
coursewareQueryKeys.metadata,coursewareQueryKeys.outline,courseHomeQueryKeys.metadata)via
useQueryClient, so the refetch goes through RQ.fetchCourserefetched a fourth thing — the sidebar toggles — but we deliberately don'tinvalidate those here, and nothing else is needed for them:
enableCompletionTrackingis astatic course-level setting, and this trigger is a unit-level learner action (
POST_EVENT)that can't change it, so refetching it on every event was redundant. The three invalidated
queries cover exactly the data such an event can change (completion / gating / access), and
courseStatusre-derives viauseCourseStatusBridgeonce those queries resettle.Sidebar toggles peeled aside (to #2013)
getCoursewareOutlineSidebarToggles→setCoursewareOutlineSidebarTogglesfeeds onlythe outline sidebar. It's the one fetch left in the thinned
fetchCourse(which thecontainer still dispatches via the unchanged
checkFetchCourseguard), so the setting keepsloading until the sidebar layer (#2013) converts it and deletes
fetchCourse. Not foldedinto the query hooks here.
Align the toggle + outline-data peels with #1920's data split
Upstream PR #1920 ("Course outline restructure") splits the outline sidebar hook into
useCourseOutlineData()(the data:sections/sequences/units/courseOutlineStatus/activeSequenceId/sequenceStatus, plusisEnabledCompletionTrackingandisActiveEntranceExam) anduseCourseOutlineSidebar()(just open/collapse UI state). Notablyit moves
isEnabledCompletionTrackingfromuseCourseOutlineSidebar()touseCourseOutlineData()— i.e. the completion-tracking toggle is treated as outline data,not sidebar chrome.
Implication for our decomposition (not #2010 — the transitional thunk just writes the slice,
which #1920 still reads via
useSelector, so there's no conflict): when we convert these toReact Query, group the toggle conversion (#2013) with the outline-data conversion
(sections/sequences/units/status) so both feed
useCourseOutlineData. Don't structure thetoggle as its own sidebar-flavored peel. Aligned that way, the eventual rebase over #1920 is
just "point
useCourseOutlineDataat the query hooks." (#1920 is UI-only — nodata//thunk/slice files — and currently stalled/red, so we align the split now and rebase whenever it moves.)
Test seeding (
setupTest.js)initializeTestStore(used by ~37 test files) seeds viaexecuteThunk(fetchCourse).Replace that with direct model-store dispatches replicating
fetchCourse's writes(coursewareMeta / courseHomeMeta / sections / sequences) from the same mocked data, so
the 37 callers keep working unchanged. Contained to
setupTest.js.