Skip to content

Convert the courseware outline sidebar to React Query #2013

Description

@brian-smith-tcril

Part of #1946 — Redux → React Query migration (Stage 1). Part of the #1976 courseware decomposition (plan) — Target 2 (outline sidebar). Stacked on the checkBlockCompletion peel.

Goal: convert the courseware outline sidebar's data + UI state off Redux.

Tasks

  • Convert getCourseOutlineStructure → query (replacing courseware.courseOutline / courseOutlineStatus).
  • Convert the getCoursewareOutlineSidebarToggles fetch (peeled out of fetchCourse in the metadata layer) → query.
  • Move courseOutlineShouldUpdate and the completion rollups to setQueryData cache updates.
  • Move coursewareOutlineSidebarSettings (UI/config) → React context / local state.
  • Rework src/courseware/course/sidebar/sidebars/course-outline/hooks.js to read from the new hooks.

Verify: the outline sidebar loads, expands/collapses, reflects completion, and refetches on locked-sequence completion; sidebar settings behave identically.

Plan

Note

The findings and plan below were generated by Claude (Claude Code) and reviewed before posting.

Investigation findings that adjust the task list above:

  • No new context at this layer. coursewareOutlineSidebarSettings isn't UI/client state — it's the camelCased result of the waffle-toggles fetch (server state), so it becomes the toggles query, not a context. Nothing else needs one either: isOpen is already local state and currentSidebar/toggleSidebar already live in SidebarContext. The "+ context" in this issue's original title had no remaining referent (title since updated).
  • courseOutlineShouldUpdate becomes invalidation, not cache data. The completion rollups become setQueryData; the flag existed only to trigger a refetch, and queryClient.invalidateQueries is that trigger. This is the repo's first setQueryData/invalidateQueries use (the epic plan's wording for this exact layer).
  • Two deliberate behavior changes (details in the full plan):
    • Rollup against a never-loaded outline: today the updateCourseOutlineCompletion reducer throws a TypeError (state.courseOutline.units is undefined) that falls into a catch-all logError. The new shape is a clean cache-miss no-op — the log was an accident of the reducer's shape, not a signal, and the state is legitimate (outline fetch failed or still in flight when a completion lands).
    • The locked-sequence refetch no longer blanks the sidebar: today fetchCourseOutlineRequest resets the outline to {}/LOADING, so the sidebar drops to its spinner (and every sequence's collapse state resets) while refetching. invalidateQueries keeps the rolled-up tree visible until fresh data lands. (resetQueries would reproduce the blanking exactly; rejected as an artifact of the request-action pattern, not a chosen behavior.) Side-effect audit: no test references courseOutlineShouldUpdate or courseOutlineStatus at all, and the status has exactly one reader — the spinner branch in CourseOutline.tsx.
  • The rollup logic survives as a pure helper, ported faithfully from the reducer — including finding the containing sequence by scanning unitIds (the reducer ignored the payload sequenceId; the scan is authoritative for the sidebar tree).
  • A latent crash gets hardened away: getCourseOutline (the api fn) returns null when the response has no blocks; today a null outline would crash useCourseOutlineSidebar's destructuring. The query consumer's ?? {} covers null and undefined alike.
  • What stays Redux on purpose: the hook's sequenceId/sequenceStatus reads (courseware-slice fields still written by the status bridge — Tear down the courseware Redux slice + replace useContextId #1976's) and useModel('coursewareMeta') for entranceExamData (Dissolve the model-store normalized cache #1977's). After this layer the courseware slice is down to exactly the re-scoped Tear down the courseware Redux slice + replace useContextId #1976 teardown set.
Full plan

Plan: #2013 — Convert the courseware outline sidebar to React Query

Context

Part of epic #1946 (Redux → React Query, Stage 1) and the #1976 courseware
decomposition — Target 2 (outline sidebar), the target itself. New stack layer on
top of #2063 (checkBlockCompletion → mutation). This converts the last data the
sidebar reads from Redux:

  1. getCourseOutlineStructure thunk → state.courseware.courseOutline /
    courseOutlineStatus (the navigation tree the sidebar renders), fetched from
    /api/course_home/v1/navigation/{courseId} and normalized by
    normalizeOutlineBlocks.
  2. fetchCourse thunk (already reduced to only the sidebar-toggles fetch by the
    metadata layer, Convert courseware metadata to React Query #2010) → state.courseware.coursewareOutlineSidebarSettings
    (enableCompletionTracking), fetched from
    /courses/{courseId}/courseware-navigation-sidebar/toggles/.
  3. updateCourseOutlineCompletion (courseware slice) — the completion rollups +
    courseOutlineShouldUpdate refetch flag, dispatched transitionally by
    useCheckBlockCompletion's onSuccess since Peel: convert checkBlockCompletion to a React Query mutation #2012.

The only reader of all of this is useCourseOutlineSidebar
(course-outline/hooks.js), consumed by six sidebar components. After this layer, the
courseware slice is down to courseId/courseStatus/sequenceId/sequenceStatus/
sequenceMightBeUnit/errorMessage/errorCode — exactly the re-scoped #1976
teardown set.

Corrections to the issue body.

  • "Move coursewareOutlineSidebarSettings (UI/config) → React context / local state"
    — it isn't client state: it's the camelCased result of the waffle-toggles fetch
    (server state), so it becomes the toggles query, not a context. Nothing in this
    layer needs a new context: isOpen is already local state and
    currentSidebar/toggleSidebar already live in SidebarContext. The "+ context"
    in the issue title had no remaining referent (title since updated).
  • "courseOutlineShouldUpdatesetQueryData" — the rollups become setQueryData;
    the flag becomes queryClient.invalidateQueries on the outline query (the flag
    existed only to trigger a refetch, and invalidation is that trigger).

Key files (all read during investigation)

  • src/courseware/data/api.jsgetCourseOutline (note: returns null when the
    response has no blocks), getCoursewareOutlineSidebarToggles (returns snake_case).
    Both unchanged.
  • src/courseware/data/queryKeys.ts — add courseOutline(courseId) and
    sidebarToggles(courseId) keys.
  • src/courseware/data/apiHooks.ts — add the two queries; rework
    useCheckBlockCompletion's onSuccess.
  • src/courseware/data/courseOutline.tsnew: the normalized outline types +
    the pure completion-rollup helper (ported reducer logic).
  • src/courseware/data/slice.js — remove courseOutline,
    coursewareOutlineSidebarSettings, courseOutlineStatus,
    courseOutlineShouldUpdate state + the five reducers that touch them
    (fetchCourseOutlineRequest/Success/Failure, setCoursewareOutlineSidebarToggles,
    updateCourseOutlineCompletion).
  • src/courseware/data/selectors.js — remove getCourseOutline,
    getCourseOutlineStatus, getCoursewareOutlineSidebarSettings,
    getCourseOutlineShouldUpdate.
  • src/courseware/data/thunks.js — delete fetchCourse and
    getCourseOutlineStructure (+ now-unused imports).
  • src/courseware/data/index.js — drop the fetchCourse re-export.
  • src/courseware/CoursewareContainer.tsx — drop the checkFetchCourse guard and the
    fetchCourse import/dispatch (its only remaining job was the toggles fetch, which
    moves into the sidebar hook).
  • src/courseware/course/sidebar/sidebars/course-outline/hooks.js — the rework target.
  • src/courseware/course/sidebar/sidebars/course-outline/CourseOutline.tsx — untouched
    (the hook keeps returning a string courseOutlineStatus).
  • src/setupTest.jsseedCoursewareModels (drops the fetchCourse executeThunk),
    initializeTestStore (drops the getCourseOutlineStructure executeThunk +
    excludeFetchOutlineSidebar handling; the axios mocks for both URLs stay).
  • Tests: redux.test.js, apiHooks.test.tsx, and the five sidebar test files
    (CourseOutlineTray, CourseOutlineTrigger, SidebarSection, SidebarSequence,
    SidebarUnit).

The conversion

1. queryKeys.ts

courseOutline: (courseId: string) => [...coursewareQueryKeys.all, 'courseOutline', courseId] as const,
sidebarToggles: (courseId: string) => [...coursewareQueryKeys.all, 'sidebarToggles', courseId] as const,

(courseOutline matches the feature dir / thunk name; the existing outline key is
the learning-sequences outline — distinct on purpose, called out in the decision doc.)

2. courseOutline.ts — types + pure rollup helper (new file, TS)

Types for the normalizeOutlineBlocks shape (CourseOutlineUnit / Sequence /
Section / CourseOutlineData, with completionStat: { completed?, total? } and the
optional fields the normalizer can leave undefined), plus:

export function applyUnitCompletion(outline: CourseOutlineData, unitId: string):
  { outline: CourseOutlineData, refetchNeeded: boolean }

A faithful, immutable port of the updateCourseOutlineCompletion reducer body:

  • marks the unit complete;
  • finds the containing sequence by scanning unitIds (the reducer ignored the
    payload sequenceId; keep the scan — it's authoritative for the sidebar tree);
  • recounts sequences[id].completionStat.completed, sets complete when all units
    are complete;
  • finds the containing section, re-sums its completionStat.completed, sets
    complete when all sequences are complete;
  • refetchNeeded = the old courseOutlineShouldUpdate condition (all units in the
    sequence complete AND the section has a type: 'lock' sequence).

3. apiHooks.ts — two queries

export const useCourseOutlineStructure = (courseId: string | undefined) => useQuery({
  queryKey: coursewareQueryKeys.courseOutline(courseId!),
  queryFn: () => getCourseOutline(courseId),
  enabled: !!courseId,
});

export const useCoursewareOutlineSidebarToggles = (courseId: string | undefined) => useQuery({
  queryKey: coursewareQueryKeys.sidebarToggles(courseId!),
  queryFn: async () => {
    const { enable_completion_tracking: enableCompletionTracking } = await getCoursewareOutlineSidebarToggles(courseId);
    return { enableCompletionTracking };
  },
  enabled: !!courseId,
});
  • No meta.models on either — this state has no useModel readers (the model-store
    bridge isn't involved; the sidebar tree was never in the model store).
  • Error handling comes from the global QueryCache.onError (Restore dropped query error logging via a global QueryCache.onError #2022) — the thunks'
    logError catch-alls are preserved by infrastructure. Retry policy is the standard
    Smart query retry: skip 4xx, retry 5xx/network errors #2024 one (the thunks were single-attempt; all converted queries accepted this).
  • Faithfulness note: getCourseOutline can return null (no blocks); consumers use
    ?? {} (below), which also covers it. Today a null outline would crash the hook's
    destructuring — ?? {} is strictly safer, noted in the decision doc.

4. useCheckBlockCompletion — swap the outline dispatch for cache updates

onSuccess keeps the updateModel units dispatch (model store is still the merged
source of truth for units until #1977) and replaces the
updateCourseOutlineCompletion dispatch:

onSuccess: (isComplete, { courseId, unitId }) => {
  dispatch(updateModel({ modelType: 'units', model: { id: unitId, complete: isComplete } }));
  if (!isComplete || !unitId || !courseId) {
    return; // the reducer's early return for incomplete units
  }
  const queryKey = coursewareQueryKeys.courseOutline(courseId);
  const cachedOutline = queryClient.getQueryData(queryKey);
  if (!cachedOutline) {
    return; // sidebar outline never fetched (e.g. never opened)
  }
  const { outline, refetchNeeded } = applyUnitCompletion(cachedOutline, unitId);
  queryClient.setQueryData(queryKey, outline);
  if (refetchNeeded) {
    queryClient.invalidateQueries({ queryKey });
  }
},
  • queryClient from useQueryClient(); captured by the hook-level onSuccess
    closure, so the unmount-survival semantics from Peel: convert checkBlockCompletion to a React Query mutation #2012 are unchanged.
  • This is the repo's first setQueryData/invalidateQueries — sanctioned by the epic
    plan's wording for this exact layer.
  • Deliberate behavior changes (captured in the decision doc):
    • Outline-never-loaded + complete=true: was reducer-throw → logError; now a clean
      cache-miss no-op. The log was incidental noise from the throwing reducer (a
      TypeError falling into the thunk's catch-all), not a signal anyone acts on.
    • The old refetch (courseOutlineShouldUpdatefetchCourseOutlineRequest) blanked
      the sidebar to its spinner while refetching; invalidateQueries keeps showing the
      rolled-up data until fresh data lands. (resetQueries would reproduce the blanking
      exactly; rejected as an artifact of the request-action pattern, not a chosen
      behavior.)
  • Side-effect checks for the stale-while-revalidate change (verified during
    planning; re-verify at review):
    • No test pins the spinner-during-refetch or courseOutlineShouldUpdate — grep of
      src/**/*.test.* finds zero references to either.
    • courseOutlineStatus has exactly one reader (CourseOutline.tsx's LOADING
      spinner branch); nothing keys remounts, scroll, or effects off the status.
    • The one genuinely new runtime state: the tree stays interactive during the
      refetch (old behavior unmounted it behind the spinner). handleUnitClick /
      logEvent scan the stale-but-valid maps, so clicks mid-refetch behave like
      clicks pre-refetch; manual smoke covers this.
    • Collapse state now survives the refetch (old blanking reset each
      SidebarSequence's local useState(defaultOpen)); a newly unlocked sequence
      mounts fresh with defaultOpen derived from the active sequence, as before.
  • The try/catch around the old dispatch goes away with the dispatch (the helper is
    pure and only runs against a present cache; nothing left to throw).

5. course-outline/hooks.js — rework useCourseOutlineSidebar

  • Replace the four Redux reads + fetch effect:
const { data: sidebarToggles } = useCoursewareOutlineSidebarToggles(courseId);
const isEnabledCompletionTracking = sidebarToggles?.enableCompletionTracking;
const outlineQuery = useCourseOutlineStructure(courseId);
const courseOutlineStatus = outlineQuery.isPending ? LOADING : (outlineQuery.isError ? FAILED : LOADED);
const { sections = {}, sequences = {}, units = {} } = outlineQuery.data ?? {};

(useParams already supplies courseId above the removed lines. The string status
keeps CourseOutline.tsx byte-identical; pending → falsy flag matches the old
initial {} settings exactly.)

  • Delete the useEffect that dispatched getCourseOutlineStructure — mounting the
    query replaces "fetch when not LOADED", and the mutation's invalidateQueries
    replaces "refetch when courseOutlineShouldUpdate". Equivalence: the trigger/tray
    keep the hook mounted whenever courseware renders, so an invalidation refetches
    immediately, like the old flag-watching effect did.
  • Keep untouched: useDispatch goes (nothing left to dispatch); getSequenceId /
    getSequenceStatus selectors stay (courseware-slice fields still written by the
    status bridge — they're Tear down the courseware Redux slice + replace useContextId #1976's), as does useModel('coursewareMeta', …) for
    entranceExamData (Dissolve the model-store normalized cache #1977's).

6. CoursewareContainer.tsx

Remove the fetchCourse import and the checkFetchCourse memoized guard + its call.
The toggles fetch now belongs to the sidebar hook (its only consumer); it starts on
first sidebar-hook mount (same page render, marginally later in the waterfall —
noted, harmless: it gates icon decoration, not layout).

Cleanup

7. Slice / selectors / thunks / index

As listed under Key files. thunks.js keeps saveSequencePosition,
saveIntegritySignature (#2015) and getCourseDiscussionTopics (#2016).

8. setupTest.js

  • seedCoursewareModels: drop await executeThunk(fetchCourse(courseId), …) + the
    fetchCourse import.
  • initializeTestStore: drop the getCourseOutlineStructure executeThunk + import.
    Replace excludeFetchOutlineSidebar with what tests now actually need: the outline
    URL mock is registered as never-resolving when options.outlineSidebarPending is
    set (the only way to hold the query in its pending state), 200 otherwise. Both URL
    mocks stay — the queries fetch them at render.

Tests

9. redux.test.js

Delete the Test fetchCourse describe (+ now-unused constants). Remaining describes
(saveSequencePosition, saveIntegritySignature) are #2015's.

10. apiHooks.test.tsx

  • New describe useCourseOutlineStructure: fetches and returns the
    normalizeOutlineBlocks shape (assert against the factory-built blocks).
  • New describe useCoursewareOutlineSidebarToggles: returns the camelCased flag
    on success; on network error, logError called (global cache) and data undefined
    (ports the two Test fetchCourse cases).
  • Rework useCheckBlockCompletion describe: hoist the query client to test scope;
    seedOutline becomes queryClient.setQueryData(coursewareQueryKeys.courseOutline(courseId), await getCourseOutline(courseId)) against the existing navigation mock; the
    outline() reader becomes queryClient.getQueryData(...). Same five cases, plus:
    • "outline never loaded" loses its logError assertion (now asserts: units model
      written, no cached outline appears, nothing logged) — renamed to match;
    • new: locked-sequence refetch — seed an outline whose section contains a
      type: 'lock' sequence, complete the last unit, assert the rolled-up cache and
      a second GET to the navigation URL (invalidation refetch).

11. Sidebar test files (five)

  • Fixture derivation: state.courseware.courseOutline.…
    await getCourseOutline(courseId) (the api fn, against the initializeTestStore
    mock) in each initTestStore helper.
  • Rendering now fetches through the per-render query client (matching how every
    converted suite works — OutlineTab etc. fetch through mocks and await), so the first
    data-dependent assertion per test becomes await screen.findBy…/waitFor (the
    completion sr-only text, section titles, icon classes). Assertions on prop-driven
    content stay synchronous.
  • CourseOutlineTray "renders correctly when course outline is loading": switch
    excludeFetchOutlineSidebar: trueoutlineSidebarPending: true (hanging mock).
  • CourseOutlineTrigger.test.jsx renders no query-driven content — verify it stays
    green as-is (act warnings are the only risk).

12. Existing coverage to sweep

  • CoursewareContainer.test.jsx builds its own axios mocks: the trigger now fetches
    the navigation + toggles URLs at render — add those two mocks if missing so the
    global error logger stays quiet.
  • Suites that render Course/Sequence via initializeTestStore (Course.test,
    Sequence.test, content-tools, discussions widgets, TabPage) get the two new
    render-time fetches against already-registered mocks — expect at most act/waitFor
    hygiene fixes, no assertion changes.

Conventions: userEvent, no eslint-disable, new files TS, rationale in the decision
doc not inline.

Decision doc

Capture in the decision doc (folded into the PR at submit time): the issue-body
corrections (settings are server state → query, no context; shouldUpdate →
invalidation); first setQueryData/invalidateQueries use and why get-then-set beats
an updater function (the refetchNeeded side-signal); the two deliberate behavior
changes (no log on cache-miss rollup; no spinner-flash on locked-sequence refetch);
the null-outline (?? {}) hardening; key naming vs the existing outline key; what
stays Redux on purpose (sequenceId/sequenceStatus#1976, useModel reads →
#1977).

Stack

New layer stacked on #2063 (checkBlockCompletion → mutation); submitted once green.

Verification

  • npm run test -- src/courseware/data src/courseware/course/sidebar src/courseware/CoursewareContainer.test.jsx src/courseware/course/Course.test.jsx.
  • npm run types and npm run lint.
  • Manual smoke (tutor local, DemoX): sidebar loads + expands/collapses; completing a
    unit ticks the open sidebar's rollups; completion icons hidden when the waffle flag
    is off; mobile collapse still lands completion; locked-sequence refetch needs a
    prereq-gated course — covered by the new unit test if none is handy. If a
    prereq-gated course is available, also click around the sidebar during the
    invalidation refetch (the tree now stays interactive where it used to blank to the
    spinner) and confirm expanded/collapsed sections survive the refetch.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Labels

No labels
No labels

Projects

Milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions