Repository navigation
refactor!: delete the model store and its React Query bridge - #2154
Merged
Merged
Conversation
brian-smith-tcril
added this pull request to stack #2141
October 2, 2026 15:41
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #2154 +/- ##
==========================================
+ Coverage 95.59% 95.75% +0.16%
==========================================
Files 374 371 -3
Lines 6087 6012 -75
Branches 1496 1436 -60
==========================================
- Hits 5819 5757 -62
+ Misses 258 246 -12
+ Partials 10 9 -1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
brian-smith-tcril
marked this pull request as ready for review
October 2, 2026 16:05
brian-smith-tcril
force-pushed
the
bsmith/model-store-teardown
branch
2 times, most recently
from
October 5, 2026 17:39
ee7dc48 to
cbe983a
Compare
Base automatically changed from
bsmith/access-expiration-banner-query-read
to
master
October 5, 2026 17:47
With #1999 the last `useModel` reader left the app, so the normalized `models` cache had no readers, no `meta`-driven mirrors and no explicit writers. This layer deletes it: `src/generic/model-store/` (slice, hooks, index), `src/data/modelStoreBridge.ts`, the `QueryCache` `onSuccess` wiring and `ModelStoreMeta` type in `src/queryClient.ts` (whose `createAppQueryCache` and `createQueryClient` no longer take a store), and the `models` reducer and unused `RootState` type in `src/store.ts`, which now registers only the external `specialExams` reducer. No component read changes; no request or behaviour change. Tests: the seeds were inert, since nothing read them, so `initializeTestStore` goes rather than being reshaped. `render` in `setupTest.js` wraps with the app's `initializeStore()`, the store the special-exams components still select from, and `mockCourseRequests(options)` installs the same course endpoint mocks and returns the built payloads with `courseId`, `sequenceId` and `unitId`; the test-side reads of seeded models take their values from those payloads instead. `createTestQueryClient()` builds the default `QueryCache` (the app cache's logging is tested directly in `queryClient.test.ts`). The `excludeFetch*` options, `getTestStoreIds`, the per-test stores and two stale `react-redux` mocks go with it. Closes #1977. Closes #1978. Part of #1946. BREAKING CHANGE: `@src/generic/model-store` (`useModel`, `useModels`, the `models` reducer and its actions) no longer exists. Read course data from the owning query hooks instead: `useCourseHomeMeta`, `useOutlineTabData`, `useDatesTabData`, `useProgressTabData`, `useCoursewareMetadata`, `useMinimalCourseOutline`, `useSequenceMetadata`, `useUnit` and `useDiscussionTopic` (all accept `{ enabled: false }` for a non-fetching read of a cached result). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
brian-smith-tcril
force-pushed
the
bsmith/model-store-teardown
branch
from
October 5, 2026 17:47
cbe983a to
5d95871
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
With #2153 the last
useModelreader left the app, so the normalizedmodelscache had no readers, nometa-driven mirrors and no explicit writers. This layer deletes it: the model-store module, the React Query → model-store bridge, theQueryCacheonSuccesswiring that fed it, and themodelsreducer, which leavesstore.tsregistering only the externalspecialExamsreducer, #1978's stated endpoint. No component read changes; no behaviour or request change. The test setup loses its store-seeding half with it:initializeTestStoreis replaced bymockCourseRequests(options), which installs the same course endpoint mocks and returns the built payloads, and the sharedrenderpasses a Redux store only to the trees that reach the exams library. Breaking for plugins that import@src/generic/model-store; see Plugins — breaking. Part of the Redux → React Query migration (#1946, Stage 1), layer F of the model-store dissolution; stacked above #2153. Closes #1977. Closes #1978.What changed
src/generic/model-store/(slice.js,hooks.js,index.js,hooks.test.tsx),src/data/modelStoreBridge.tsandmodelStoreBridge.test.ts(decision 1).src/queryClient.ts: theonSuccessbridge call, theModelStoreMetatype and thestoreparameters go;createAppQueryCache()andcreateQueryClient()take nothing,Register.queryMetais{ logStatusAs?: Record<number, LogLevel> }.src/index.jsxcallscreateQueryClient().src/store.tsdropsmodelsReducerand the importer-lessRootState.src/setupTest.js(decisions 3, 6):initializeTestStore,seedCoursewareModels,seedSequenceModels,getTestStoreIdsandglobalStorego.mockCourseRequests(options)is the synchronous helper left:initializeMockApp(), the factory build, every axios reply the old setup installed (includingpreventSequenceLoad/preventOutlineSidebarLoad), returning the payloads pluscourseId,sequenceId,unitId.renderpassesAppProviderno store unless given one through astoreoption.createTestQueryClient()builds TanStack's defaultQueryCache.mockCourseRequestsand lose theirawait; the 24getTestStoreIdsreads and the 24getState().modelsreads take ids and values from the returned payloads; the 18excludeFetch*options, the 29 second-store builds and the 44{ store }render options go; six exam-reaching suites opt in with{ store: initializeStore() }and twelve suites with their ownAppProviderpass no store;CourseExit.test.jsxloses its hand seeding;queryClient.test.tsloses the bridge case; two stalereact-reduxjest mocks go.package.json's redux packages anddocs/decisions/0004-model-store.md(decisions 10–11).Plugins — breaking
@src/generic/model-storeno longer exists, so a plugin that importsuseModel,useModels, themodelsreducer or its actions from it fails to resolve the module at build time. Earlier layers left those imports working and returning{}as each model stopped being written (#2110, #2128, #2134, #2140, #2144, #2153 each named the hook to move to); this layer removes the module itself. It affects sidebar widgets registered throughSIDEBAR_WIDGETSandPluginSlotplugins that import this repo's internals; nothing in-repo imports the module and no README documents it.The replacement is the query hook that owns the data, as a non-fetching reader:
courseHomeMetauseCourseHomeMeta(courseId)(course-home/data/apiHooks)outline,dates,progressuseOutlineTabData(courseId),useDatesTabData(courseId),useProgressTabData(courseId, targetUserId)coursewareMetauseCoursewareMetadata(courseId)for the metadata fields,useMinimalCourseOutline(courseId)for the outline'scoursesmapsections, the outline'ssequencesuseMinimalCourseOutline(courseId)sequences(the active one),unitsuseSequenceMetadata(sequenceId),useUnit(sequenceId, unitId)discussionTopicsuseDiscussionTopic(courseId, unitId)Under any course page the page's own observer has fetched the data, so the disabled reader reads the cached result and adds no request, re-rendering when the fetch lands, as the
useModelread did when the bridge wrote the store. Rendered anywhere else,.datastaysundefineduntil something on that page fetches; the query cache is shared, so the simplest shape is the widget mounting the fetch itself, the same hook without the option, beside its read. Unaffected: the sidebar widget config functions,isAvailableandprefetch, still receive the mergedcourseobject. The Accessing Course Data section ofsrc/courseware/course/sidebar/README.mddocuments the hooks.Testing
npm run typesandnpm run lintclean; full suite 120 suites, 1186 passed, 0 skipped.git grep -E "model-store|modelStore|useModel|state\.models|initializeTestStore|getTestStoreIds" srcis empty;git grep react-redux srcreturns only the removed line of a migration diff example ingeneric/plugin-overrides/README.md;git grep "from 'redux'" srcis empty;store.tsregisters onlyspecialExams. Which suites still need a Redux store was established by running the whole suite withrenderpassing none: six failed, each on react-redux's "could not find react-redux context value", each predicted by its tree reachingSequenceExamWrapper,OuterExamTimeroruseExamAccess(decision 3); those six opt in. No request-count assertion observed the deleted seeding requests (decision 5). Manual smoke on tutor local: the five course-home tabs and courseware unit and sequence navigation render and navigate as before.Decisions
Full decision log
Decisions — dissolve the model-store normalized cache (#1977, layer F)
The teardown layer of the model-store dissolution, above #2153 (#1999, layer
E) on the running stack #2141. Closes #1977 and #1978. Planned and settled
with the user 2026-10-02; the plan is embedded in the #1977 issue body.
Deletion only; nothing in the app changes what it reads. After layer E,
non-test source had zero
useModel/useModelsreaders, zerostate.modelsselectors, zero querymetaentries the bridge consumed andzero explicit model-store dispatches; the only
react-reduximport leftwas the model-store hooks file itself. So this layer deletes
src/generic/model-store/(slice, hooks, index, the hooks test),src/data/modelStoreBridge.tsand its test, theQueryCacheonSuccesswiring, the
ModelStoreMetatype and thestoreparameters insrc/queryClient.ts(Register.queryMetais now{ logStatusAs?: Record<number, LogLevel> }), and themodelsreducer andthe importer-less
RootStatetype insrc/store.ts.src/index.jsxcallscreateQueryClient().No rendering fallout; the seeds were inert. The issue body expected to
"fix the fallout across the suites that relied on pre-seeded models". With
no readers, the models
initializeTestStoreseeded reached no component:suites already loaded through the real queries (
MountCourseQueryHooks,CourseQueryGate,seedQueryData,queryClient.fetchQuery). Whatdepended on the seeds was test-side: 24
getState().modelsreads in 11files (first unit id ×8; four block titles for the Helmet title; a
sequence's
gatedContent;courseHomeMeta.verifiedMode×2; the wholenormalized
courseHomeMetato seed a query ×2) and the idsgetTestStoreIdsderived from the model keys. Each has a direct source inthe API-shaped payloads the setup already built (entry 3).
initializeTestStoreis deleted, not reshaped. It did three jobs:build a store, mock the course endpoints, seed the store from them. The
third is gone and the first is
initializeStore()fromsrc/store.tstothe letter, so what is left is the second:
renderinsetupTest.jspassesAppProviderno store by default (AppProviderrenders a react-reduxProvideronly when given one;OptionalReduxProviderreturns barechildren for
null), and takes astoreoption for the suites whosetree reaches one of the three library entry points, which select
state.specialExamsand throw "could not find react-redux contextvalue" under no
Provider. Which suites that is was established byrunning the whole suite with no default store: six failed, each on that
error, and each predicted by its tree —
Course.test.jsx,SequenceContent.test.jsxandUnit/index.test.jsx(Unit→useExamAccess),Sequence.test.jsx(SequenceExamWrapper),CourseExit.test.jsxandProductTours.test.jsx's outline-tab describe(
TabWithTimer→OuterExamTimer);courseware/course/test-utils.jsxrenders
Coursetoo. Those pass{ store: initializeStore() }, the app'sown store, the way
CoursewareContainer.test.jsxand the five tab suitesalready give their own
AppProviderone. Nothing else in any test toucheda store (
git grep -E "specialExams|store\.dispatch|store\.subscribe" -- '*.test.*'matched only the seeds andCourseExit.test.jsx's twoaddModeldispatches), soglobalStore, theoverrideStoreparameter,the 29 second-store builds (
initializeTestStore(…, false)) and the 44{ store }render options that carried seeded models all went. Twelvesuites with their own
AppProviderwhose tree has no exam componentnow pass no store at all: the six that passed a test store
(
CourseBreadcrumbs,CourseOutlineTray,CourseOutlineTrigger,SidebarSection,SidebarSequence,SidebarUnit) and six that builtan app store they did not need (
CoursewareResultsFilter,CoursewareSearch,GoalUnsubscribe,CourseTabsNavigation,CourseBreadcrumbsSlot,preferences-unsubscribe). Rejected: a storeon every
render(the first draft) — the same unconditional store thetwelve suites had just lost, handed to trees with nothing to select from
it, and it changed
AlertList.test.jsx's empty-containerassertion fora wrapper div the component did not render.
mockCourseRequests(options)is the synchronous helper that is left:initializeMockApp(),buildSimpleCourseAndSequenceMetadata(options),every axios reply
initializeTestStoreinstalled (includingpreventSequenceLoad/preventOutlineSidebarLoad), and a return of thebuilt payloads (
courseBlocks,sectionBlocks,sequenceBlocks,unitBlocks,courseMetadata,sequenceMetadata,courseHomeMetadata)plus
courseId,sequenceId,unitIdpicked off them. Seeding was theonly
await, so all 41 call sites lost theirs. Named besidelogUnhandledRequestsfor what it does;setUpMockRequests(thecontainer suite's local helper, less specific) and
buildMockCourse(names the return, not the side effect callers depend on) were
rejected. The
excludeFetchCourse/excludeFetchSequenceoptions,which only skipped seeding, went with their 18 call sites;
preventSequenceLoad's comment lost its "pair withexcludeFetchSequence"clause.
initializeTestStoreand returning the fixturesbeside the store (a bespoke test store whose only content is the app's
own reducer, and 29 second-store builds with nothing to distinguish
them); a
WeakMap<store, fixtures>behind a keptgetTestStoreIds(store)(a store-keyed side table for data the store no longer holds).
The test-side reads take their values from the payloads. First unit id
→
unitId(the eight hand-rolledObject.keys/values(models.units)[0]reads).
Course.test.jsx's Helmet title →unitBlocks[0].display_name,sequenceBlocks[0].display_name,sectionBlocks[0].display_name,courseMetadata.name(the course block'sdisplay_nameiscourseMetadata.name:buildSimpleCourseBlocks(courseId, courseMetadata.name)).SequenceContent.test.jsx'sgatedContent.prereqSectionName→sequenceMetadata[0].gated_content.prereq_section_name.LockPaywall.test.jsx'sverifiedModefields → the defaultcourseHomeMetadata.verified_mode.HonorCode.test.jsxandStreakCelebrationModal.test.jsxseed the metadata query withawait getCourseHomeCourseMetadata(courseId)fetched through the setup'sadapter before each suite installs its own, which is the reason they seed
(their comments say so, now also saying where the value comes from).
courseware/course/test-utils.jsxlost its firstinitializeTestStorecall, which existed only to read ids the second build then overwrote, and
returns
{ courseId, sequenceId, unitId }toCourse.test.jsx.No request-count assertion observed the seeding requests. Twelve
suites assert on
axiosMock.history; each installs its ownnew MockAdapter(getAuthenticatedHttpClient())after the setup call (whichreplaces the setup's adapter, so its history starts empty) or resets before
asserting. The six requests the seeds made were never counted; no
assertion changed.
createTestQueryClient()builds TanStack's defaultQueryCache. Thestore argument was the only thing selecting the app's cache, so with it
gone one cache serves all 65 call sites. Tested against three questions:
does the app cache behave differently (no:
QueryCacheConfigis threeoptional callbacks,
onError/onSuccess/onSettled, fired after aquery settles; no query state, data, retry or timing differs, and the
status-specific hook tests already ran under the default cache); should
its logging be tested (it is, directly, in
queryClient.test.tsagainstcreateAppQueryCache; no hook's ownlogStatusAsmeta was assertedthrough the helper); is the logging useful for diagnosing tests
(marginally, and noise wherever the failure is deliberate). The two
helper-dependent logging assertions in
courseware/data/apiHooks.test.tsx(the sidebar-toggles network error, the discussion-topics config failure)
asserted the cache's generic behaviour, not the hooks', and were dropped
with the
initializeStore()arguments and the comment that waited on thislayer; the cases keep their hook-specific assertions and are renamed for
what they still check. Rejected: always the app cache (production parity
bought with console noise, for behaviour no component can observe); a
flag replacing the store argument (would preserve an accidental split).
CourseExit.test.jsx's pre-fetch was seeding by hand.fetchAndRenderawaited
getCourseMetadata+getCourseHomeCourseMetadata, dispatchedboth with
addModel, thenaxiosMock.resetHistory(); the results had noother use and the per-URL request counts are taken after the reset. The
fetch, the dispatches, the
storeand their imports went;MountCourseQueryHookswas already there for the component to readthrough.
Two stale
react-reduxjest mocks removed (CoursewareSearchResults.test.jsx'sbare
jest.mock('react-redux');CourseTabsNavigation.test.jsx'suseDispatchmock with amockDispatchno assertion read), both left fromthe
pluginsslice (Convert the plugin-store override registry off Redux to React context #2017). The issue's verify line ("git grep react-redux srcreturns only special-exams-adjacent files") was false while theystood; it now returns only the removed line of a migration diff example in
generic/plugin-overrides/README.md.Two incidental second-store patterns in touched files.
OutlineTab.test.jsx'sfetchAndRendertook arenderStoreoption so one case could render on afresh
initializeStore(); with nothing in a store to differ on, the optionand that case's store went.
ProductTours.test.jsxpassed{ store: initializeStore() }to a render ofProductToursalone, whichreaches no exam component; it went.
docs/decisions/0004-model-store.mdis left as it is. The ADR log ishistory: no layer of Convert Learning from redux to Context + react-query #1946 has amended an ADR (
0006still describesthunks,
0007still mentions "redux tests"). The superseding ADR thatrecords React Query as this app's server-state layer (OEP-67 ADR-0010 is
the org-level decision) is the epic's closing work alongside Reduce the Redux store to the external specialExams reducer #1978's
dependency removal, and is named as a task on Reduce the Redux store to the external specialExams reducer #1978.
package.jsonkeepsredux,react-reduxand@reduxjs/toolkit.store.tsstill usesconfigureStore, andAppProvider store=stillneeds a react-redux store for
OuterExamTimer/SequenceExamWrapper/useExamAccess. Dropping them is Reduce the Redux store to the external specialExams reducer #1978's library-gated step (Adopt@edx/frontend-lib-special-examsv5 (React Query, host-agnostic): mountSpecialExamsProvider, own the submitted-timed-exam slot, delete the Redux store and dependencies #2018).refactor!:.@src/generic/model-storehas been importable byplugins; nothing in-repo documents it today, but an external plugin could
still import
useModel, as@edx/unit-translation-selector-pluginimported
@src/generic/plugin-storeuntil Convert the plugin-store override registry off Redux to React context #2017. The footer says themodule is gone and names the query hooks to read instead.
Manual testing
Manual testing — dissolve the model-store normalized cache (#1977)
In-browser smoke test against a live backend (tutor local), agreed 2026-10-02
as minimal for this layer. Run by the user 2026-10-02.
What changed: the
modelsreducer, the model-store module and the ReactQuery bridge are deleted; the app
QueryCacheloses itsonSuccess. Nocomponent read changed (no
useModelreader existed), so no behaviour orrequest change is intended anywhere. The one production code path that
changes shape is the
QueryCache, which every query runs through.Not exercised here: a timed-exam attempt. The exams library's
OuterExamTimer,SequenceExamWrapperanduseExamAccessrender for real in the unit suitesagainst the
specialExams-only store (and threw, as expected, in a run withno store), so the store wiring is covered there.
Checks
🤖 Generated with Claude Code