refactor: convert the live tab from Redux to React Query - #2006
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## bsmith/react-query-course-home-discussion-tab #2006 +/- ##
=================================================================================
+ Coverage 93.11% 93.29% +0.18%
=================================================================================
Files 364 364
Lines 5939 5906 -33
Branches 1411 1404 -7
=================================================================================
- Hits 5530 5510 -20
+ Misses 391 381 -10
+ Partials 18 15 -3 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
9475085 to
62d92eb
Compare
|
Moved back to draft — this conversion drops the query-side error logging its thunk used to emit (React Query v5 removed 🤖 Comment via Claude Code. |
62d92eb to
5005683
Compare
Self-wrap the live tab (LiveTab owns useCourseHomeMeta + a new useLiveTabData, renders TabWithTimer, splits the iframe into a LiveTabContent child). Delete fetchLiveTab and the now-orphaned shared fetchTab helper (discussion, its other consumer, converted in the layer below), and remove the Test fetchTab block from redux.test.js. Part of #1975; closes #2002. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
5005683 to
4568fed
Compare
addressed in https://github.com/openedx/frontend-app-learning/compare/5005683e0cee3acad4dbd29809bd6b8d70418d67..4568fedb91eb6f7e926ca5c7a40057968d1226fe (part of a stack rebase, see #1987) |
Summary
Convert the course-home live tab from Redux to React Query. Part of the Redux → React Query migration (#1946), stacked on the discussion-tab conversion (#2005) as the top course-home tab layer. Closes #2002.
The live tab has real tab data (the LTI iframe HTML) but needs no model-store bridge — its only reader was the tab itself. It's the last course-home tab on the shared
fetchTabhelper, so this layer also retiresfetchTab.What changed
course-home/data/queryKeys.ts/apiHooks.ts— adduseLiveTabData(courseId)(getLiveTabIframe, nometatag) and theliveTab(courseId)key.course-home/live-tab/LiveTab.jsx— self-wrapping: a thin data-owningLiveTab(runsuseCourseHomeMeta+useLiveTabData, renders<TabWithTimer>) + aLiveTabContentchild holding thedangerouslySetInnerHTMLdiv and thegetElementById('lti-tab-embed')sizinguseEffect, so it mounts post-load.courseIdfromuseParams.index.jsx— live route → bare<LiveTab />; thefetchLiveTabimport/wiring dropped.course-home/data/thunks.js— deletefetchLiveTaband the now-orphaned sharedfetchTabhelper (discussion, its other consumer, was converted in refactor: convert the discussion tab from Redux to React Query #2005), plus their now-unused imports.eventTypes,deprecatedSaveCourseGoal, andfetchExamAttemptsDataremain.course-home/data/slice.js— remove the three now-dead reducers orphaned by thefetchTabdeletion (fetchTabRequest/fetchTabDenied/fetchTabSuccess);fetchTabFailurestays (still dispatched byTabPage.test.jsxfor theerrorMessagedisplay).Behavior
No user-facing change. No
livemodel bridge: the masquerade banner readsuseModel('lti_live'), butfetchLiveTabwrote modellive(slug ≠ model), so that read was already empty and the banner never rendered here — preserved.TabWithTimeris retained, so the outer exam timer still mounts as it did viaTabContainer. The threefetchTabrequest/denied/success reducers inslice.js, orphaned by the deletion, are removed;fetchTabFailurestays (still exercised byTabPage.test.jsx) and retires with thecourseHomereducer in #1975.Testing
npm run types,npm run lint, and the affected suites pass. NewLiveTab.test.jsxrenders<LiveTab />through the bridged query client;apiHooks.test.tsxgains auseLiveTabDatablock (success / 404→{}/ error);redux.test.jsdrops theTest fetchTabblock wholesale (its subjectfetchTabis deleted here). Manual browser verification (HAR-confirmed clean load) is documented in the decision log below.Decisions
Full decision log
Decisions — Redux → React Query: the live tab (#1946)
Working notes for this PR (part of the wider Redux → React Query migration,
#1946). Not checked in — referenced when opening the PR. Part of #1975
(course-home tab data), stacked on the discussion-tab conversion (#2005) as the
top course-home tab layer. Closes #2002.
The live tab has real tab data (the LTI iframe HTML) but still needs no
model-store bridge — its only reader was the tab itself. It's the last
course-home tab on the shared
fetchTabhelper, so this layer also retiresfetchTab.Scope: self-wrapping conversion
Decision.
LiveTabbecomes self-wrapping — it owns its data viauseCourseHomeMeta+ a newuseLiveTabDatahook and renders<TabWithTimer>itself. The
index.jsxroute drops<TabContainer tab="lti_live" fetch={fetchLiveTab} slice="courseHome">for a bare<LiveTab />.No transitional bridge (cleaner than progress)
Decision.
useLiveTabDatacarries nometatag:state.models.liveisLiveTabitself(
state.models.live[courseId]?.iframe), which moves to the query.useModel(tab, courseId)withtab="lti_live", butfetchLiveTabwrote its data under modellive, notlti_live— souseModel('lti_live', …)already resolved empty and the bannernever rendered on this tab.
TabWithTimer activeTabSlugstays"lti_live", sothat's preserved exactly. No
#1999-style holdout.LiveTabsplits into a thin wrapper +LiveTabContentDecision.
LiveTabruns the two queries and renders<TabWithTimer courseStatus={{ metadataQuery, tabDataQuery }}><LiveTabContent /></TabWithTimer>;the iframe
<div>and itsgetElementById('lti-tab-embed')sizinguseEffectmove to
LiveTabContent.TabPagerenders children only once loaded, so puttingthe effect in the gated child preserves today's timing (where
LiveTabwasTabContainer's already-loaded child); in the self-wrapping shape the data-owningcomponent mounts before load.
The iframe HTML is a prop named
html, notiframeDecision.
LiveTabContenttakeshtml(the iframe markup) and renders it viadangerouslySetInnerHTML. It's namedhtmlrather thaniframeso it doesn'tshadow the effect's
const iframe = document.getElementById('lti-tab-embed')DOM-node variable — which keeps that effect byte-identical to the original.
getLiveTabIframereused unchangedDecision.
useLiveTabData'squeryFnis the existinggetLiveTabIframe(
GET /api/course_live/iframe/<courseId>/; 404 →{}, else throw), reusedas-is. Its name doesn't match the
get<Tab>TabDatasiblings; a rename wasconsidered and rejected as out-of-scope churn for a conversion.
fetchTabretires here (the last consumer)Decision. Deleting
fetchLiveTableaves the sharedfetchTabwith no callers(discussion, its other consumer, was converted in #2005). So
fetchTaband itsnow-unused imports (
getCourseHomeCourseMetadata,addModel,fetchTab{Request,Denied,Failure,Success}) are removed fromthunks.js. What'sleft there is
eventTypes,deprecatedSaveCourseGoal, andfetchExamAttemptsData.Slice cleanup: deleting
fetchTaborphaned three of its reducers —fetchTabRequest,fetchTabDenied, andfetchTabSuccesshad no remainingdispatcher, so they're removed from
slice.js(along with the now-unusedLOADING/DENIEDimports).fetchTabFailurestays:TabPage.test.jsxstill dispatches it to exercise
TabPage'serrorMessagedisplay. The residualcourseStatus/errorMessagefields retire with thecourseHome-reducerteardown (#1975), not this layer.
redux.test.js:Test fetchTabblock removed wholesaleDecision. With
fetchTabgone, theTest fetchTabblock (which drove it viafetchLiveTab) is removed entirely, along with its now-orphanedcourseMetadataUrl/
courseHomeAccessDeniedMetadatadeclarations and theappendBrowserTimezoneToUrlimport. This is the clean, single deletion the below-live ordering was chosen to
enable — no retarget churn.
Tests
LiveTab.test.jsx(new) renders<LiveTab />through the bridged query clientand asserts the iframe HTML from
useLiveTabDatarenders — exercisingLiveTab,LiveTabContent, and the sizing effect.apiHooks.test.tsxadds auseLiveTabDatablock: success (iframe payload),404 →
{}, and non-404 → error. The 404/500 mocks attachcustomAttributes(via
Object.assignon a realError, so it's typed and lint-clean) becausegetLiveTabIframereadserror.customAttributes.httpErrorStatus— matching theapproach in
api.test.js.redux.test.js—Test fetchTabblock removed (see above).Manual testing (in-browser)
Verified (no LTI live provider configured — the realistic demo case): a direct,
cold load of
/course/<id>/liverenders cleanly. The HAR showsGET /api/course_home/course_metadata/…→ 200 (useCourseHomeMeta) andGET /api/course_live/iframe/<id>/→ 200 (useLiveTabData), the latter with anempty payload, so
#live_tabrenders empty — no crash, full page render. TheOuterExamTimerpath fired (…/proctored_exam/attempt/…→ 200), confirmingTabWithTimeris preserved. Zero 4xx/5xx, and nothing hit afetchTab/courseStatus/courseware/sequencepath — i.e. deleting the sharedfetchTabhelper broke nothing on this render.
Not exercised: a fully-configured LTI live provider (visible iframe content +
the
#lti-tab-embedsizing effect) — none available locally. The automatedLiveTab.test.jsxcovers the iframe-render + effect path with a mocked payload.🤖 Generated with Claude Code