Skip to content

refactor: convert the live tab from Redux to React Query - #2006

Open
brian-smith-tcril wants to merge 1 commit into
bsmith/react-query-course-home-discussion-tabfrom
bsmith/react-query-course-home-live-tab
Open

refactor: convert the live tab from Redux to React Query#2006
brian-smith-tcril wants to merge 1 commit into
bsmith/react-query-course-home-discussion-tabfrom
bsmith/react-query-course-home-live-tab

Conversation

@brian-smith-tcril

@brian-smith-tcril brian-smith-tcril commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

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 fetchTab helper, so this layer also retires fetchTab.

What changed

  • course-home/data/queryKeys.ts / apiHooks.ts — add useLiveTabData(courseId) (getLiveTabIframe, no meta tag) and the liveTab(courseId) key.
  • course-home/live-tab/LiveTab.jsx — self-wrapping: a thin data-owning LiveTab (runs useCourseHomeMeta + useLiveTabData, renders <TabWithTimer>) + a LiveTabContent child holding the dangerouslySetInnerHTML div and the getElementById('lti-tab-embed') sizing useEffect, so it mounts post-load. courseId from useParams.
  • index.jsx — live route → bare <LiveTab />; the fetchLiveTab import/wiring dropped.
  • course-home/data/thunks.js — delete fetchLiveTab and the now-orphaned shared fetchTab helper (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, and fetchExamAttemptsData remain.
  • course-home/data/slice.js — remove the three now-dead reducers orphaned by the fetchTab deletion (fetchTabRequest / fetchTabDenied / fetchTabSuccess); fetchTabFailure stays (still dispatched by TabPage.test.jsx for the errorMessage display).

Behavior

No user-facing change. No live model bridge: the masquerade banner reads useModel('lti_live'), but fetchLiveTab wrote model live (slug ≠ model), so that read was already empty and the banner never rendered here — preserved. TabWithTimer is retained, so the outer exam timer still mounts as it did via TabContainer. The three fetchTab request/denied/success reducers in slice.js, orphaned by the deletion, are removed; fetchTabFailure stays (still exercised by TabPage.test.jsx) and retires with the courseHome reducer in #1975.

Testing

npm run types, npm run lint, and the affected suites pass. New LiveTab.test.jsx renders <LiveTab /> through the bridged query client; apiHooks.test.tsx gains a useLiveTabData block (success / 404→{} / error); redux.test.js drops the Test fetchTab block wholesale (its subject fetchTab is 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 fetchTab helper, so this layer also retires
fetchTab.

Scope: self-wrapping conversion

Decision. LiveTab becomes self-wrapping — it owns its data via
useCourseHomeMeta + a new useLiveTabData hook and renders <TabWithTimer>
itself. The index.jsx route drops <TabContainer tab="lti_live" fetch={fetchLiveTab} slice="courseHome"> for a bare <LiveTab />.

No transitional bridge (cleaner than progress)

Decision. useLiveTabData carries no meta tag:

  • The only reader of state.models.live is LiveTab itself
    (state.models.live[courseId]?.iframe), which moves to the query.
  • The shared masquerade banner reads useModel(tab, courseId) with
    tab="lti_live", but fetchLiveTab wrote its data under model live, not
    lti_live — so useModel('lti_live', …) already resolved empty and the banner
    never rendered on this tab. TabWithTimer activeTabSlug stays "lti_live", so
    that's preserved exactly. No #1999-style holdout.

LiveTab splits into a thin wrapper + LiveTabContent

Decision. LiveTab runs the two queries and renders <TabWithTimer courseStatus={{ metadataQuery, tabDataQuery }}><LiveTabContent /></TabWithTimer>;
the iframe <div> and its getElementById('lti-tab-embed') sizing useEffect
move to LiveTabContent. TabPage renders children only once loaded, so putting
the effect in the gated child preserves today's timing (where LiveTab was
TabContainer's already-loaded child); in the self-wrapping shape the data-owning
component mounts before load.

The iframe HTML is a prop named html, not iframe

Decision. LiveTabContent takes html (the iframe markup) and renders it via
dangerouslySetInnerHTML. It's named html rather than iframe so it doesn't
shadow the effect's const iframe = document.getElementById('lti-tab-embed')
DOM-node variable — which keeps that effect byte-identical to the original.

getLiveTabIframe reused unchanged

Decision. useLiveTabData's queryFn is the existing getLiveTabIframe
(GET /api/course_live/iframe/<courseId>/; 404 → {}, else throw), reused
as-is. Its name doesn't match the get<Tab>TabData siblings; a rename was
considered and rejected as out-of-scope churn for a conversion.

fetchTab retires here (the last consumer)

Decision. Deleting fetchLiveTab leaves the shared fetchTab with no callers
(discussion, its other consumer, was converted in #2005). So fetchTab and its
now-unused imports (getCourseHomeCourseMetadata, addModel,
fetchTab{Request,Denied,Failure,Success}) are removed from thunks.js. What's
left there is eventTypes, deprecatedSaveCourseGoal, and fetchExamAttemptsData.

Slice cleanup: deleting fetchTab orphaned three of its reducers —
fetchTabRequest, fetchTabDenied, and fetchTabSuccess had no remaining
dispatcher, so they're removed from slice.js (along with the now-unused
LOADING / DENIED imports). fetchTabFailure stays: TabPage.test.jsx
still dispatches it to exercise TabPage's errorMessage display. The residual
courseStatus / errorMessage fields retire with the courseHome-reducer
teardown (#1975), not this layer.

redux.test.js: Test fetchTab block removed wholesale

Decision. With fetchTab gone, the Test fetchTab block (which drove it via
fetchLiveTab) is removed entirely, along with its now-orphaned courseMetadataUrl
/ courseHomeAccessDeniedMetadata declarations and the appendBrowserTimezoneToUrl
import. 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 client
    and asserts the iframe HTML from useLiveTabData renders — exercising LiveTab,
    LiveTabContent, and the sizing effect.
  • apiHooks.test.tsx adds a useLiveTabData block: success (iframe payload),
    404 → {}, and non-404 → error. The 404/500 mocks attach customAttributes
    (via Object.assign on a real Error, so it's typed and lint-clean) because
    getLiveTabIframe reads error.customAttributes.httpErrorStatus — matching the
    approach in api.test.js.
  • redux.test.jsTest fetchTab block removed (see above).

Manual testing (in-browser)

Verified (no LTI live provider configured — the realistic demo case): a direct,
cold load of /course/<id>/live renders cleanly. The HAR shows
GET /api/course_home/course_metadata/… → 200 (useCourseHomeMeta) and
GET /api/course_live/iframe/<id>/ → 200 (useLiveTabData), the latter with an
empty payload, so #live_tab renders empty — no crash, full page render. The
OuterExamTimer path fired (…/proctored_exam/attempt/… → 200), confirming
TabWithTimer is preserved. Zero 4xx/5xx, and nothing hit a fetchTab /
courseStatus / courseware/sequence path — i.e. deleting the shared fetchTab
helper broke nothing on this render.

Not exercised: a fully-configured LTI live provider (visible iframe content +
the #lti-tab-embed sizing effect) — none available locally. The automated
LiveTab.test.jsx covers the iframe-render + effect path with a mocked payload.

🤖 Generated with Claude Code

@codecov

codecov Bot commented Aug 19, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.29%. Comparing base (15be9cc) to head (4568fed).

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.
📢 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.

@brian-smith-tcril
brian-smith-tcril force-pushed the bsmith/react-query-course-home-live-tab branch from 9475085 to 62d92eb Compare August 19, 2026 20:43
@brian-smith-tcril
brian-smith-tcril marked this pull request as ready for review August 19, 2026 20:48
@brian-smith-tcril
brian-smith-tcril marked this pull request as draft August 20, 2026 23:57
@brian-smith-tcril

Copy link
Copy Markdown
Contributor Author

Moved back to draft — this conversion drops the query-side error logging its thunk used to emit (React Query v5 removed useQuery.onError), which is tracked in #2022. Keeping it out of the merge queue until that logging layer is sorted.

🤖 Comment via Claude Code.

@brian-smith-tcril
brian-smith-tcril force-pushed the bsmith/react-query-course-home-live-tab branch from 62d92eb to 5005683 Compare August 21, 2026 00:58
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>
@brian-smith-tcril
brian-smith-tcril force-pushed the bsmith/react-query-course-home-live-tab branch from 5005683 to 4568fed Compare August 21, 2026 01:59
@brian-smith-tcril

Copy link
Copy Markdown
Contributor Author

Moved back to draft — this conversion drops the query-side error logging its thunk used to emit (React Query v5 removed useQuery.onError), which is tracked in #2022. Keeping it out of the merge queue until that logging layer is sorted.

🤖 Comment via Claude Code.

addressed in https://github.com/openedx/frontend-app-learning/compare/5005683e0cee3acad4dbd29809bd6b8d70418d67..4568fedb91eb6f7e926ca5c7a40057968d1226fe (part of a stack rebase, see #1987)

@brian-smith-tcril
brian-smith-tcril marked this pull request as ready for review August 21, 2026 02:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Convert the live tab to React Query

1 participant