Skip to content

refactor: convert outline tab from Redux to React Query - #1997

Draft
brian-smith-tcril wants to merge 1 commit into
bsmith/react-query-request-cert-mutationfrom
bsmith/react-query-course-home-outline-tab
Draft

refactor: convert outline tab from Redux to React Query#1997
brian-smith-tcril wants to merge 1 commit into
bsmith/react-query-request-cert-mutationfrom
bsmith/react-query-course-home-outline-tab

Conversation

@brian-smith-tcril

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

Copy link
Copy Markdown
Contributor

Summary

Convert the course-home outline tab from Redux to React Query. Part of the Redux → React Query migration (#1946), and the next course-home tab through the door the dates-tab conversion opened. Closes #1991.

OutlineTab becomes self-wrapping — it owns its data (useCourseHomeMeta + a new useOutlineTabData) and renders TabWithTimer itself, so the index.jsx route drops its <TabContainer tab="outline" fetch={fetchOutlineTab}> wrapper for a bare <OutlineTab /> and fetchOutlineTab is deleted.

What changed

  • apiHooks.ts / queryKeys.ts — add useOutlineTabData (tagged meta: { modelType: 'outline', courseId } so the model-store bridge still populates the subtree's useModel('outline') reads) and the outlineTab(courseId) key.
  • OutlineTab.jsx — split into a thin data-owning OutlineTab (runs the two queries, renders <TabWithTimer courseStatus={{ metadataQuery, tabDataQuery }}><OutlineTabContent/></TabWithTimer>) and an OutlineTabContent in the same file. The split keeps the useModel('outline') destructure out of the loading phase, where the model is still {}. courseId from useParams; proctoringPanelStatus stays on useSelector.
  • Subtree courseIduseParams — the seven slice-reading widgets (DateSummary, CourseDates, CourseHandouts, CourseTools, StartOrResumeCourseCard, WeeklyLearningGoalCard, ProctoringInfoPanel) and the three section-outline components that reached the slice via useContextId (Section, SequenceDueDate, SequenceTitle). Required: the route no longer runs a thunk to set state.courseHome.courseId.
  • ShiftDatesAlert.jsxrefreshTabData now invalidates both datesTab and outlineTab; drops its fetch/useDispatch (outline was the last fetch= caller).
  • Outline-only POST writers → mutationsdismissWelcomeMessageuseDismissWelcomeMessage (thunk deleted) and saveWeeklyLearningGoaluseSaveWeeklyLearningGoal (moved out of thunks.js, where it was a loose non-thunk helper).

Behavior

No user-facing change. proctoringPanelStatus deliberately stays in Redux (client UI state, out of scope for a data-layer conversion). The mutations add onError: logError, which the thunks lacked — a strict improvement.

Testing

npm run types, npm run lint, and the full npm test suite (106 suites, 3 pre-existing skips) pass. OutlineTab.test.jsx and ProductTours.test.jsx now render the real self-wrapping tab through the bridged query client; new mutation coverage (useOutlineTabData, useDismissWelcomeMessage, useSaveWeeklyLearningGoal) in apiHooks.test.tsx. Manual browser verification and the test-coverage split are documented in the decision log below.

Decisions

Full decision log

Decisions — Redux → React Query: the outline tab (#1991)

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 dates-tab conversion (#1984), which set
the self-wrapping pattern.

Scope: same self-wrapping conversion the dates tab established

Decision. OutlineTab becomes self-wrapping — it renders <TabWithTimer>
itself and owns its data via useCourseHomeMeta + a new useOutlineTabData
hook. courseId comes from useParams. The index.jsx route drops its
<TabContainer tab="outline" fetch={fetchOutlineTab}> wrapper for a bare
<OutlineTab />, and fetchOutlineTab is deleted.

Why. This is the pattern the dates tab set (#1984), which itself converges
on the shape CoursewareContainer already uses. Nothing tab-specific here —
the outline tab is just the next tab through the same door.

OutlineTab splits into a thin wrapper + OutlineTabContent

Decision. OutlineTab is now just the data-owning wrapper: it runs the two
queries and renders <TabWithTimer courseStatus={{ metadataQuery, tabDataQuery }}> <OutlineTabContent /></TabWithTimer>. The heavy body — everything that reads
useModel('outline', courseId) — moves into an OutlineTabContent component in
the same file.

Why. Self-wrapping means OutlineTab renders before its data resolves (the
spinner phase lives inside TabWithTimer). The body destructures the outline
model (const { courseBlocks, … } = useModel('outline', courseId)), and
useModel returns {} for a missing model, so that destructure ran against {}
during loading and crashed. TabPage only renders its children once the data has
loaded, so moving the body into a child (OutlineTabContent) means the
destructure never runs empty — no defensive defaults, no guard, and the wrapper
stays a thin data-owner. Same file, so it reads as one unit.

The outline model stays on the transitional bridge — it isn't outline-only

Decision. Keep every useModel('outline'|'courseHomeMeta', …) read in the
subtree exactly as-is, populated by the model-store bridge (the meta: { modelType, courseId } tag on the query drives addModel on success). Do
not convert any of these reads to consume useOutlineTabData's data
directly in this PR. Dissolving the model store is its own coordinated epic
step (#1977).

Why — the mechanism. We looked into who actually reads these models, to see
if any were outline-tab-only and could be pulled forward:

  • courseHomeMeta is read all over the app (courseware, every tab, dozens of
    shared alerts, TabPage) — plainly shared.
  • outline looked outline-scoped — every reader is under outline-tab/
    except two: useEnrollmentAlert and useLogistrationAlert. Those run
    inside LoadedTabPage, which renders on every tab and courseware, and
    they read useModel('outline', courseId) passively and defensively
    (outline && outline.courseBlocks && …) — on non-outline pages the model is
    empty and the alert stays hidden.

The distinction that matters: useModel is a passive read (returns whatever
is cached, fetches nothing); useOutlineTabData(courseId) is an active query
(calling it triggers the outline fetch). Swapping those shared alert hooks to
the query would fire the outline API on the dates tab, the progress tab, and
courseware — a real regression. The passive-read-plus-bridge is exactly what
makes the shared read safe today, which is why untangling it is a coordinated
store dissolution (#1977), not per-tab work.

Alternative rejected. Convert just the outline-tab-only readers to
useOutlineTabData (child calls dedupe against OutlineTab's query, so no
extra fetch), leaving the bridge for the two shared alerts.
It can't remove the
bridge or the meta tag (the shared alerts still need the model store
populated), and it leaves the outline model read two different ways for #1977
to reconcile anyway — churn without retiring the transitional mechanism.

proctoringPanelStatus stays in Redux

Decision. OutlineTab keeps reading proctoringPanelStatus from
useSelector(state.courseHome); only courseId moves to useParams.
ProctoringInfoPanel keeps its useDispatch(fetchProctoringInfoResolved()).

Why. It's client UI state (a reducer flips it to loaded when the panel
resolves, gating when the goal widget appears), not server data — out of scope
for a data-layer conversion. It's retired when the store is reduced to the
external specialExams reducer (#1978).

Subtree courseId: slice / useContextId reads → useParams

Decision. Every subtree component that derived courseId from the slice
moves to useParams:

  • direct useSelector(state.courseHome) readers — DateSummary, CourseDates,
    CourseHandouts, CourseTools, StartOrResumeCourseCard,
    WeeklyLearningGoalCard, ProctoringInfoPanel (keeps its useDispatch).
  • the section-outline readers — Section, SequenceDueDate, SequenceTitle
    — which reached the slice indirectly through useContextId()
    (state.courseware.courseId ?? state.courseHome.courseId). Only these three
    outline-only callers move; the shared useContextId itself is left alone
    (~30 progress/course-exit components rely on it).

Why — required, not cosmetic. state.courseHome.courseId is only ever set
by a tab's fetch thunk (fetchTabRequest). Once the outline route stops running
one, any descendant still reading it — directly or via useContextId() — gets
undefined, so its useModel(…, courseId) misses or its sequences[id] lookup
crashes: the same breakage class as the UpgradeToCompleteAlert slot child in
the dates PR.

ShiftDatesAlert: invalidate all date-bearing caches, and finish its bridge

Decision. refreshTabData invalidates both datesTab(courseId) and
outlineTab(courseId), independent of which tab the alert is on. The model
prop stays only for the useModel(model) visibility gate and the
resetDeadlines.mutate({ courseId, model }) call. Outline is the last
fetch={fetchOutlineTab} caller, so the fetch prop and useDispatch are
removed entirely.

Why. The alert is about dates; shifting them changes date info wherever
it's cached — the dates model (datesTab query) and the outline model's
datesWidget/SequenceDueDate (outlineTab query). So invalidation is
model-independent: both keys, always.

Not a fix to the dates PR. The dates PR invalidating only datesTab was
correct for its moment — outline was still Redux/thunk-fetched then, so no
outlineTab query existed to invalidate; its on-mount fetchOutlineTab handled
staleness. This PR is where the outline cache first exists, so it's where it
joins the alert's invalidation set.

Bucket 3: outline-only POST writers become mutations

Decision. Convert the outline tab's fire-and-forget POST writers to React
Query mutations, removing useDispatch from their components:

  • dismissWelcomeMessage (WelcomeMessage) → useDismissWelcomeMessage; the
    thunk is deleted.
  • saveWeeklyLearningGoal (WeeklyLearningGoalCard) →
    useSaveWeeklyLearningGoal. This one wasn't even a thunk — it was a plain
    postWeeklyLearningGoal wrapper sitting in thunks.js. Since we're converting
    the outline writers anyway, it moves out of thunks.js and becomes a mutation
    like the others rather than a loose helper.

Why. They only POST, dispatch no action, and touch no state — the same kind
of writer the CTA-toast PR (#1982) converted (resetDeadlines/processEvent
mutations). Converting them here gets the outline tab's genuine remaining Redux
off the store rather than leaving it for a later sweep (#1973).

requestCert was peeled into a prerequisite PR (#1995), not converted here.
It looked like a third outline writer (CertificateStatusAlert), but it's shared
CourseCelebration (course-exit) and CertificateStatus (progress) dispatch
it too. Converting a shared thunk mid-outline-PR would drag two unrelated
subtrees into this diff, so it became its own lower stack layer (useRequestCert,
all three callers converted); this PR builds on top of it.

Minor behavior note. The mutations use onError: logError to match the
sibling mutations in apiHooks.ts; the original thunks logged nothing (a failed
POST was an unhandled rejection), so this is a small improvement, not a
regression.

Verification: what was checked in a browser vs. left to the tests

The risky part of this PR is mechanical — the query conversion plus the
courseId rewrites, where a wrong courseId makes a useModel read miss
silently or crashes a sequences[id] lookup. So the manual pass targeted the
paths where that failure mode is only visible at runtime, and leaned on the
automated suite for the rest.

Manually verified in a browser:

  • Core load — spinner → content with no mid-load crash (the OutlineTabContent
    split), including a hard-reload directly on the outline URL.
  • The course outline structure — the section-outline useContextId
    useParams move: sections expand/collapse, sequence titles link into
    courseware, due dates and completion state render.
  • The useParams-fed sidebar widgets that a normal course surfaces — dates
    widget, course handouts, course tools, Start/Resume card.
  • Smoke: launch tour from outline, outline ↔ dates full-page navigation,
    masquerade-as-learner, and a no-access course.

Left to the automated tests (each renders the real tab through the bridged
query path — the same code path production uses — so they exercise the actual
useParams/query wiring, not a mock of it):

  • Shift due dates from outline — OutlineTab.test "handles shift due dates
    click" seeds missed_deadlines: true, swaps the refetch to false, clicks,
    and asserts the button disappears; it only can if refreshTabData invalidated
    outlineTab and refetched. This is the one genuinely new behavior in the PR,
    and it's directly asserted.
  • Welcome message render + dismiss — OutlineTab.test "Welcome Message" block
    plus apiHooks.test useDismissWelcomeMessage (POST + error logging).
  • Weekly learning goal — OutlineTab.test "Weekly Learning Goal" block (with the
    proctoring endpoint mocked, so the proctoring-gated render is exercised) plus
    apiHooks.test useSaveWeeklyLearningGoal.
  • Proctoring info panel — its courseId is the same useParams value proven
    good by the widgets verified above; the proctoring GET is mocked in the tab
    tests.

Why that split is safe. What a browser pass would add beyond the tests is
narrow: whether the live backend persists a dismiss / goal / shift across a
real reload. But those three endpoints (dismiss_welcome_message,
save_course_goal, reset_course_deadlines) are unchanged by this PR — the same
api.js functions, now called from a mutation instead of a thunk — so that path
is the platform's behavior, not this diff's. Standing up the fixtures they need
(a set welcome message, a self-paced course with a past-due deadline, proctored
exams) buys little signal for this change.

🤖 Generated with Claude Code

Make OutlineTab self-wrapping — it owns useCourseHomeMeta + a new
useOutlineTabData query and renders TabWithTimer itself, mirroring the
dates-tab conversion. The heavy body moves into an OutlineTabContent
component so the useModel('outline') reads never run against the empty
model during the loading phase.

Subtree courseId reads move from the courseHome slice (directly, or via
useContextId in section-outline) to useParams, since the outline route
no longer runs a fetch thunk to populate state.courseHome.courseId.

ShiftDatesAlert now invalidates both datesTab and outlineTab and drops
its fetch/useDispatch. The outline-only POST writers dismissWelcomeMessage
and saveWeeklyLearningGoal become useDismissWelcomeMessage /
useSaveWeeklyLearningGoal mutations, and the fetchOutlineTab thunk is
deleted. proctoringPanelStatus stays in Redux (client UI state).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@codecov

codecov Bot commented Aug 14, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 92.90%. Comparing base (82889d7) to head (5043f3f).

Additional details and impacted files
@@                             Coverage Diff                              @@
##           bsmith/react-query-request-cert-mutation    #1997      +/-   ##
============================================================================
- Coverage                                     92.96%   92.90%   -0.07%     
============================================================================
  Files                                           363      363              
  Lines                                          5939     5945       +6     
  Branches                                       1381     1380       -1     
============================================================================
+ Hits                                           5521     5523       +2     
- Misses                                          399      402       +3     
- Partials                                         19       20       +1     

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

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 outline tab to React Query

1 participant