Peel: convert checkBlockCompletion to a React Query mutation

[brian-smith-tcril]

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

Goal: convert checkBlockCompletion to a React Query mutation.

Why its own layer: it's the courseware requestCert — a shared writer that patches both the units model (complete) and the sidebar's courseware.courseOutline completion rollups (updateCourseOutlineCompletion), and it has two consumers in different layers (CoursewareContainer on unit render, and the outline sidebar on unit click). Peeling it below both consumers keeps them from fighting over completion state once the sidebar is also React-Query-backed.

Tasks

• Convert checkBlockCompletion → useMutation; on success update the units cache (via the bridge) and the outline cache (setQueryData, replacing updateCourseOutlineCompletion).
• Preserve the "already complete → no-op" short-circuit.

Verify: completing a unit updates the unit indicator and the sidebar completion rollups (incl. the courseOutlineShouldUpdate refetch trigger for locked sequences).

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 outline setQueryData at this layer. The sidebar outline (getCourseOutlineStructure → state.courseware.courseOutline) is still Redux until Convert the courseware outline sidebar to React Query #2013 converts it — there is no outline query to write into yet. This layer keeps dispatching updateCourseOutlineCompletion (transitional); Convert the courseware outline sidebar to React Query #2013 swaps that dispatch for a cache update. Likewise the units write stays a direct updateModel dispatch rather than going "via the bridge": the model-store bridge is wired to the QueryCache only, and the payload is shaped from mutation variables + result ({ id: unitId, complete }), which the declarative meta.models mirror can't express. Dissolve the model-store normalized cache #1977 later moves that write to the sequence query cache.
• The transitional dispatches must live in the hook-level onSuccess, not mutate()-site callbacks. TanStack v5 skips mutate(vars, { onSuccess }) callbacks if the observing component unmounted, and the sidebar unmounts routinely before the POST resolves (mobile collapses it immediately on unit click; any unit click navigates away from the old sequence's UnitLinkWrappers). The thunk always ran to completion, so hook-level onSuccess — which runs on the mutation itself regardless of unmount — is the faithful home.
• A hidden behavior to preserve: when the sidebar outline was never fetched (courseOutline: {} — sidebar disabled or unopened) and a unit completes, the updateCourseOutlineCompletion reducer throws (state.courseOutline.units is undefined). The thunk's catch-all logged that error after the units-model write had already landed. The conversion keeps an explicit try/catch reproducing exactly that, plus a test pinning it.
• The "already complete → no-op" guard needs a call-time store read: useStore().getState().models.units[unitId]?.complete, not useSelector (which would re-render both consumers on every unit write).
• Consumer detail: the container's call fires on unit navigation click (handleUnitNavigationClick, passed down as unitNavigationHandler), not on unit render; it checks the unit being navigated away from. The sidebar's handleUnitClick checks the previously-active unit.

Full plan

Plan: #2012 — Convert checkBlockCompletion to a React Query mutation

Context

Part of epic #1946 (Redux → React Query, Stage 1) and the #1976 courseware
decomposition — Target 2 (outline sidebar), peel. New stack layer on top of #2061
(sequence data → RQ). checkBlockCompletion is a shared writer: after the learner
views/leaves a unit it asks the LMS get_completion handler whether the unit is now
complete, and on true fans the answer out to two stores:

1. updateModel({ modelType: 'units', ... }) — the model-store unit's complete flag
   (read by sequence-navigation unit buttons / UnitButton), and
2. updateCourseOutlineCompletion (courseware slice) — the sidebar outline's
   completion rollups (unit → sequence completionStat/complete → section
   completionStat/complete, plus the courseOutlineShouldUpdate refetch flag for
   sections containing a locked/prerequisite sequence).

It has two consumers in different layers:

• CoursewareContainer.tsx — handleUnitNavigationClick (passed down as
  unitNavigationHandler through Course → Sequence → sequence-navigation), checking
  the unit being navigated away from (routeUnitId).
• the outline sidebar — useCourseOutlineSidebar's handleUnitClick
  (course-outline/hooks.js), fired from UnitLinkWrapper on unit click, checking the
  previously-active unit (activeUnitId).

Peeling it below both consumers now means #2013 (sidebar outline → RQ) and the
container teardown don't fight over completion state later.

Correction to the issue body. #2012's body says on success to update "the outline
cache (setQueryData, replacing updateCourseOutlineCompletion)" — that's not possible
at this layer: the sidebar outline (getCourseOutlineStructure →
state.courseware.courseOutline) is still Redux until #2013 converts it. There is no
outline query to setQueryData into. So this layer keeps dispatching
updateCourseOutlineCompletion (transitional), and #2013 swaps that dispatch for a
cache update when the outline itself moves. Same for the units write: the model store
is still the merged source of truth for units (written by the sequence-query bridge +
this mutation), so the mutation dispatches updateModel directly; #1977 later moves
that to the sequence query cache.

Key files (all read during investigation)

• src/courseware/data/thunks.js — checkBlockCompletion (lines 40–63): the thunk being converted.
• src/courseware/data/api.js — getBlockCompletion(courseId, sequenceId, usageKey) (POST .../handler/get_completion, returns data.complete === true). Unchanged.
• src/courseware/data/slice.js — updateCourseOutlineCompletion (lines 78–124). Kept; now dispatched by the hook.
• src/courseware/data/apiHooks.ts — where the queries live; add useCheckBlockCompletion here (mutation precedent: course-home's usePostCourseDeadlines/useRequestCert, which do hook-level onSuccess side effects + onError: logError).
• src/courseware/CoursewareContainer.tsx — consumer 1 (handleUnitNavigationClick, line 375–377; checkBlockCompletion import from ./data).
• src/courseware/course/sidebar/sidebars/course-outline/hooks.js — consumer 2 (handleUnitClick, line 90; import from @src/courseware/data/thunks).
• src/courseware/data/index.js — checkBlockCompletion re-export (remove).
• src/store.ts — RootState (for the guard's useStore<RootState>() read).
• src/courseware/data/redux.test.js — Test checkBlockCompletion describe (lines 79–149) to port.
• src/courseware/data/apiHooks.test.tsx — where the ported cases land.
• The five sidebar test files (CourseOutlineTray, CourseOutlineTrigger, SidebarSection, SidebarSequence, SidebarUnit) — raw @testing-library/react render with hand-built providers, no QueryClientProvider; all six components that call useCourseOutlineSidebar will now transitively mount useMutation.

The conversion

1. apiHooks.ts — useCheckBlockCompletion()

export const useCheckBlockCompletion = () => {
  const store = useStore<RootState>();
  const dispatch = useDispatch();
  const { mutate } = useMutation({
    mutationFn: ({ courseId, sequenceId, unitId }: CheckBlockCompletionVars) => (
      getBlockCompletion(courseId, sequenceId, unitId)
    ),
    onSuccess: (isComplete, { sequenceId, unitId }) => {
      dispatch(updateModel({ modelType: 'units', model: { id: unitId, complete: isComplete } }));
      try {
        dispatch(updateCourseOutlineCompletion({ sequenceId, unitId, isComplete }));
      } catch (error) {
        logError(error); // reducer throws when the sidebar outline isn't loaded
      }
    },
    onError: (error) => logError(error),
  });
  return useCallback((courseId, sequenceId, unitId) => {
    if (store.getState().models.units[unitId]?.complete) {
      return; // things don't get uncompleted after they are completed
    }
    mutate({ courseId, sequenceId, unitId });
  }, [mutate, store]);
};

Points that make this faithful:

• The returned callback keeps the thunk's call signature (courseId, sequenceId, unitId), so both call sites are a rename-on-read (const checkBlockCompletion = useCheckBlockCompletion(); and the call line loses only its dispatch(...)
  wrapper). Param types follow what the container actually passes (nullable
  courseId/sequenceId from the slice selectors) — no as any.
• The already-complete short-circuit read getState().models.units[unitId]?.complete
  at call time; useStore() (first use in the repo, standard react-redux) reproduces a
  call-time read without subscribing the caller to every model change (a useSelector
  on models.units would re-render both consumers on every unit write).
• Transitional dispatches live in the hook-level onSuccess, not mutate-site
  callbacks. This is load-bearing: TanStack v5 skips mutate(vars, { onSuccess })
  callbacks if the observing component unmounted, and the sidebar unmounts routinely
  before the POST resolves (mobile: handleUnitClick collapses the sidebar
  immediately; any unit click navigates and unmounts the old sequence's
  UnitLinkWrappers). Hook-level onSuccess runs on the mutation itself regardless,
  matching the thunk, which always ran to completion.
• The try/catch around the outline dispatch preserves the thunk's catch-all. When
  the sidebar outline was never fetched (courseOutline: {} — sidebar disabled or not
  yet opened) and isComplete is true, the updateCourseOutlineCompletion reducer
  throws (state.courseOutline.units is undefined). The thunk's single try/catch
  caught that and logError'd it, after the units-model write had already landed.
  The explicit catch reproduces exactly that (one log, units write intact, no reliance
  on TanStack's onSuccess-throw → onError routing).
• No retry config needed: mutations default to retry: 0 in TanStack (the client's
  shouldRetryQuery and Smart query retry: skip 4xx, retry 5xx/network errors #2024 apply to queries only), matching the thunk's
  single-attempt behavior.
• No meta.models: the model-store bridge is wired to the QueryCache only, and the
  units payload is shaped from variables + result ({ id: unitId, complete }), which
  the declarative mirror can't express. Explicit dispatch, like the thunk did.

Placement considered and rejected: statusBridge.ts — its charter is mirroring query
status into slice status fields; this is a writer, and course-home already set the
precedent of mutations with side-effectful hook-level onSuccess living in
apiHooks.ts. The hook survives #2013/#1977; only its onSuccess internals shift from
dispatches to setQueryData.

2. CoursewareContainer.tsx

• Remove checkBlockCompletion from the ./data import; add
  const checkBlockCompletion = useCheckBlockCompletion(); (import from
  ./data/apiHooks).
• handleUnitNavigationClick body becomes
  checkBlockCompletion(courseId, sequenceId, routeUnitId); — otherwise untouched.

3. course-outline/hooks.js

• Drop checkBlockCompletion from the @src/courseware/data/thunks import (keep
  getCourseOutlineStructure — that's Convert the courseware outline sidebar to React Query #2013).
• const checkBlockCompletion = useCheckBlockCompletion(); at the top of
  useCourseOutlineSidebar; handleUnitClick line 90 becomes
  checkBlockCompletion(courseId, sequenceId, activeUnitId);.

Cleanup

4. Delete the thunk

• Remove checkBlockCompletion from thunks.js, plus its now-unused imports there
  (getBlockCompletion from ./api, updateCourseOutlineCompletion from ./slice;
  updateModel stays — saveSequencePosition/saveIntegritySignature still use it).
• Remove the checkBlockCompletion re-export from courseware/data/index.js.
• slice.js keeps updateCourseOutlineCompletion (now hook-dispatched; Convert the courseware outline sidebar to React Query #2013 deletes it).
• The pact test (lmsPact.test.jsx) drives getBlockCompletion (the api fn) directly — unaffected.

Tests

5. Port the redux.test.js describe → apiHooks.test.tsx

Delete Test checkBlockCompletion (lines 79–149) and rebuild the cases as hook tests
(renderHook with a wrapper providing both the Redux Provider/AppProvider and
QueryClientProvider with createTestQueryClient(store) — the mutation dispatches, so
it needs the store context, unlike the existing query tests in that file):

• complete=true, outline loaded (seed via executeThunk(getCourseOutlineStructure)
  against the axios mocks, as the old test did): callback fires the POST;
  models.units[unitId].complete is true; outline unit/sequence/section complete and
  completionStat.completed rollups update — same assertions as redux.test.js lines
  120–125.
• complete=false: no model write, no outline change (old lines 128–148).
• network error: POST attempted, logError called once, no writes (old lines 83–100).
• already complete (new — the guard is now hook logic): seed
  models.units[unitId].complete = true, call the callback, assert no POST in
  axiosMock.history.post.
• complete=true, outline never loaded (new — pins the preserved catch-all):
  models.units[unitId].complete becomes true, logError called once,
  state.courseware.courseOutline stays {}.

6. Sidebar test wrappers

All five sidebar test files build providers by hand around raw render; add
<QueryClientProvider client={createTestQueryClient(store)}> (from @src/setupTest)
inside their AppProvider so the hook's useMutation mounts. Mechanical, one wrapper
edit per file; no assertion changes expected.

7. Existing coverage that must stay green untouched

• CoursewareContainer.test.jsx — already renders with createTestQueryClient; the
  "marks the current unit complete when navigating to the next unit" test (line 406)
  clicks next and asserts the get_completion POST — exercises the converted path
  end-to-end.
• redux.test.js — the shared "Thunks that require fetched sequences" seeding stays
  (still used by saveSequencePosition / saveIntegritySignature tests, Convert saveIntegritySignature + saveSequencePosition to React Query mutations #2015).

Conventions: userEvent, no eslint-disable, rationale in the decision doc not in
comments (the one inline comment above is the terse per-case kind: the reducer-throw
branch is the surprising one).

Decision doc

Capture in the decision doc (folded into the PR at submit time): the issue-body
correction (no outline setQueryData until #2013, and why); hook-level vs mutate-site
callbacks (unmount semantics); the useStore call-time guard; the preserved catch-all
around the outline reducer (and that the outline-not-loaded logError is pre-existing
behavior, kept deliberately); placement in apiHooks.ts; and what #2013/#1977 will
each replace.

Stack

New layer stacked on #2061 (sequence data → RQ); submitted once green.

Verification

• npm run test -- src/courseware/data src/courseware/CoursewareContainer.test.jsx src/courseware/course/sidebar — ported hook tests + container matrix + sidebar suites.
• npm run types and npm run lint.
• Manual smoke (tutor local, DemoX): navigate past a unit → its nav button gets the
  completion check and the open sidebar's rollups tick up; click a unit in the
  sidebar (previous unit gets checked); sidebar closed on mobile → completion still
  lands after collapse. The locked-sequence courseOutlineShouldUpdate refetch needs a
  prereq-gated course — verify via the existing unit tests if no such course is handy.