Convert saveIntegritySignature + saveSequencePosition to React Query mutations

[brian-smith-tcril]

Part of #1946 — Redux → React Query migration (Stage 1). Part of the #1976 courseware decomposition (plan) — Target 4 (remaining writers). Stacked on the metadata + sequence conversions (models already exist; no new peel).

Goal: convert the two remaining courseware write thunks to React Query mutations.

Tasks

• saveIntegritySignature (HonorCode.jsx) → useMutation; on success patch coursewareMeta.userNeedsIntegritySignature. Preserve the masquerade skip (no backend POST when masquerading).
• saveSequencePosition (CoursewareContainer) → useMutation; preserve the optimistic sequences.activeUnitIndex update + rollback on error.

Verify: honor-code acceptance dismisses the modal and clears the integrity prompt; sequence position persists across reloads; rollback on failure.

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:

• One stack layer, not a Convert bookmarking to React Query + de-class UnitButton #2014-style split. Both conversions are the same kind of change in the same files — courseware/data/apiHooks.ts gains two mutations, and courseware/data/thunks.js / redux.test.js shrink and die together — so there's nothing to peel and no benefit to splitting; a single PR closes this issue.
• The models stay the write target until Dissolve the model-store normalized cache #1977. Every reader of the written state is a model read (sequences.activeUnitIndex via CoursewareContainer's selectors and useIFrameBehavior; coursewareMeta.userNeedsIntegritySignature via useShouldDisplayHonorCode's useModel), so both mutations keep dispatching updateModel rather than patching the sequence/metadata query caches — the same correction verified for Convert bookmarking to React Query + de-class UnitButton #2014: the bridge rewrites models only on a real fetch onSuccess, and a real refetch carries server truth including the saved position / signature. (One pre-existing caveat, unchanged by the port: after a masqueraded dismissal the server still reports the signature as needed, so a metadata refetch can resurrect the prompt — the thunk behaved identically.)
• The thunk's getState() rollback pre-read becomes onMutate context. onMutate reads the current activeUnitIndex, writes the optimistic value (synchronously, before the POST — same timing as the thunk's first dispatch), and returns the old index as React Query context for onError to revert to. That's the RQ-native home for rollback state, and it keeps the returned callback's signature identical to the thunk's.
• The redundant success re-write is kept. saveSequencePosition writes the same value again after the POST settles ("update again under the assumption that the above call succeeded, since it doesn't return a meaningful response"). Dropping it would change interleaving behavior under rapid unit switches, so the faithful port keeps it, comment and all.
• The masquerade skip becomes a resolve-without-request mutationFn branch. When masquerading as a specific learner, mutationFn resolves without posting, so onSuccess still dismisses the honor-code prompt — 1:1 with the thunk's if (!isMasquerading) guard ahead of its unconditional dispatch. On failure: logError only, no model write, the prompt stays (the thunk's catch path).
• useDispatch leaves CoursewareContainer entirely — saveSequencePosition was its last use. Capturing the hook's callback in the once-created guards.current closure is safe because React Query's mutate is a stable reference.
• redux.test.js is deleted, not trimmed — its three cases are exactly these two thunks. They port to apiHooks.test.tsx, plus three new cases: the 1-indexed { position: index + 1 } wire body (the old test only asserted the URL), a mid-flight optimistic write (hanging mock), and the integrity-signature failure path, which had no coverage at all. Pact tests call the raw api functions and are untouched; HonorCode.test.jsx's four cases already pin the component behavior and stay as-is.
• No behavior changes intended — same optimistic write timing, same revert values, same logError calls, same masquerade skip, fire-and-forget at both call sites.

Full plan

Plan: #2015 — Convert saveIntegritySignature + saveSequencePosition to React Query mutations

Context

Part of epic #1946 (Redux → React Query, Stage 1) and the #1976 courseware
decomposition — Target 4 (remaining writers). These are the last two
courseware write thunks (src/courseware/data/thunks.js); converting them
leaves only getCourseDiscussionTopics (#2016) between here and the #1976
slice teardown. The issue is right that no peel is needed — both models
(sequences, coursewareMeta) already exist and both call sites are already
function components.

Unlike #2014's bookmark split, the two tasks here are the same kind of change
in the same files (courseware/data/apiHooks.ts gains two mutations;
courseware/data/thunks.js + redux.test.js shrink/die together), so this
ships as one stack layer on top of #2066 (the bookmark conversion),
closing this issue.

The two thunks and their call sites:

1. saveSequencePosition(courseId, sequenceId, activeUnitIndex) —
   optimistic updateModel({ modelType: 'sequences', … activeUnitIndex }),
   postSequencePosition (1-indexed on the wire: position: activeUnitIndex + 1),
   then a second identical updateModel on success ("update again under the
   assumption that the above call succeeded, since it doesn't return a
   meaningful response"), or logError + revert to the pre-read
   initialActiveUnitIndex on failure. Sole dispatcher:
   CoursewareContainer.tsx:255, inside the guards.current.checkSaveSequencePosition
   memoized closure (fires on route-unit change when
   sequence.saveUnitPosition — normalized from save_position — is set).
2. saveIntegritySignature(courseId, isMasquerading) — when not
   masquerading-as-a-specific-learner, postIntegritySignature; in both cases
   on success updateModel({ modelType: 'coursewareMeta', … userNeedsIntegritySignature: false }); on failure logError only (no model
   write — the honor-code prompt stays). Sole dispatcher: HonorCode.jsx:28
   (handleAgree). The masquerade predicate
   (isMasquerading && username !== authUser.username) is computed by the
   component and stays there.

Readers of the written state (all via models; none change):

• sequences.activeUnitIndex: CoursewareContainer's
  checkSequenceToSequenceUnitRedirect (via currentSequenceSelector) and
  useIFrameBehavior.ts:45 (via the active-sequence model read).
• coursewareMeta.userNeedsIntegritySignature:
  useShouldDisplayHonorCode.js:13 (useModel).

Model store stays the write target — same correction as #2014 Layer B: the
readers are model-store reads, which remain the merged source of truth until
#1977, so both mutations keep dispatching updateModel, not setQueryData on
the sequence/metadata query caches. Durability holds the same way: the bridge
(src/data/modelStoreBridge.ts) rewrites sequences/coursewareMeta only on
a real fetch onSuccess, and a real refetch carries server truth including
the saved position / signature. (Masquerade caveat: after a masqueraded
dismissal the server still reports user_needs_integrity_signature: true, so
a metadata refetch can resurrect the prompt — byte-identical to the thunk
behavior, since a re-fetch overwrote the Redux state the same way. Not a
regression; the dismissal was always session-scoped.) Query-cache patching is
deferred to #1977, as noted there for bookmarks.

---

1. courseware/data/apiHooks.ts — two mutations

Both go in this file alongside useCheckBlockCompletion (these are
courseware/data thunks — no feature subdirectory of their own, unlike
bookmarks). No mutation keys, explicit logError in onError (mutations
aren't covered by the global QueryCache.onError), return a callback rather
than the mutation object — all per the useCheckBlockCompletion /
useSetBookmarked precedent.

useSaveSequencePosition

interface SaveSequencePositionVars {
  courseId: string | null;   // from the still-untyped Redux slice via latest.current (#1976)
  sequenceId: string | null;
  activeUnitIndex: number;
}

export const useSaveSequencePosition = () => {
  const store = useStore();
  const dispatch = useDispatch();
  const setPosition = (sequenceId: string | null, activeUnitIndex: number) => {
    dispatch(updateModel({ modelType: 'sequences', model: { id: sequenceId, activeUnitIndex } }));
  };
  const { mutate } = useMutation({
    mutationFn: ({ courseId, sequenceId, activeUnitIndex }: SaveSequencePositionVars) => (
      postSequencePosition(courseId, sequenceId, activeUnitIndex)
    ),
    // Optimistically update the position; remember the old one for rollback.
    onMutate: ({ sequenceId, activeUnitIndex }) => {
      const { models } = store.getState() as { models: { sequences: Record<string, { activeUnitIndex: number }> } };
      const initialActiveUnitIndex = models.sequences[sequenceId].activeUnitIndex;
      setPosition(sequenceId, activeUnitIndex);
      return { initialActiveUnitIndex };
    },
    // Update again under the assumption that the call succeeded, since it
    // doesn't return a meaningful response.
    onSuccess: (_data, { sequenceId, activeUnitIndex }) => setPosition(sequenceId, activeUnitIndex),
    onError: (error, { sequenceId }, context) => {
      logError(error);
      setPosition(sequenceId, context.initialActiveUnitIndex);
    },
  });
  return useCallback((courseId, sequenceId, activeUnitIndex) => {
    mutate({ courseId, sequenceId, activeUnitIndex });
  }, [mutate]);
};

(Exact spelling — including the store.getState() cast shape and callback
param types — at implementation time.)

• The thunk's getState() pre-read maps onto onMutate returning RQ
  context consumed by onError — the React Query-native home for
  rollback state; keeps the returned callback's signature identical to the
  thunk's. useStore (not a selector) for the read, per the
  useCheckBlockCompletion guard precedent.
• onMutate runs synchronously before mutationFn, so the optimistic write
  lands before the POST exactly as the thunk's first dispatch did.
• The redundant-looking success re-write is kept, with its original
  comment — faithful port (it re-asserts the position after settle; dropping
  it would change interleaving behavior under rapid unit switches).

useSaveIntegritySignature

interface SaveIntegritySignatureVars {
  courseId: string;
  isMasquerading: boolean;
}

export const useSaveIntegritySignature = () => {
  const dispatch = useDispatch();
  const { mutate } = useMutation({
    // If the request is made by a staff user masquerading as a specific learner,
    // don't actually create a signature for them on the backend,
    // only the modal dialog will be dismissed
    mutationFn: async ({ courseId, isMasquerading }: SaveIntegritySignatureVars) => (
      isMasquerading ? null : postIntegritySignature(courseId)
    ),
    onSuccess: (_data, { courseId }) => {
      dispatch(updateModel({ modelType: 'coursewareMeta', model: { id: courseId, userNeedsIntegritySignature: false } }));
    },
    onError: (error) => logError(error),
  });
  return useCallback((courseId, isMasquerading) => {
    mutate({ courseId, isMasquerading });
  }, [mutate]);
};

• The masquerade skip lives in mutationFn (resolve without a request →
  onSuccess still fires → prompt dismissed), mapping 1:1 onto the thunk's
  if (!isMasquerading) guard ahead of the unconditional dispatch.
• onError: logError only, no model write — the thunk's catch path.

2. CoursewareContainer.tsx

• const saveSequencePosition = useSaveSequencePosition(); (rename-on-read),
  so line 255 becomes saveSequencePosition(cId, sId, activeUnitIndex); —
  the dispatch(…) wrapper is the only change to the guard body.
• Safe to capture in the once-created guards.current closure: RQ's mutate
  is a stable reference and store is stable, so the useCallback result
  never goes stale.
• useDispatch leaves the file entirely (this was its last use);
  saveSequencePosition drops out of the ./data import and
  useSaveSequencePosition joins the ./data/apiHooks import next to
  useCheckBlockCompletion.

3. HonorCode.jsx

• Drop useDispatch + the saveIntegritySignature import from ../../../data;
  const saveIntegritySignature = useSaveIntegritySignature(); (rename-on-read).
• handleAgree keeps its shape and its masquerade comment:
  () => saveIntegritySignature(courseId, isMasquerading && username !== authUser.username).
• File stays .jsx — minimal diff, no structural change (the TS-for-new-files
  rule applies to new files only).

4. Cleanup

• courseware/data/thunks.js: both thunks deleted; getCourseDiscussionTopics
  remains (dies in Convert getCourseDiscussionTopics to React Query #2016). Trim now-unused imports (updateModel,
  postIntegritySignature, postSequencePosition).
• courseware/data/index.js: the whole from './thunks' export block goes
  (it exports exactly these two); api/selectors/slice exports stay.
• courseware/data/redux.test.js: deleted — its three cases (all on these
  two thunks; nothing else in the file) port to apiHooks.test.tsx.

Tests

courseware/data/apiHooks.test.tsx — two new describes on the existing
mutation pattern (renderHook + AppProvider store × QueryClientProvider
via createTestQueryClient, seedSequenceModels for the sequence cases,
axios-mock on the handler URLs):

useSaveSequencePosition:

• success → models.sequences[sequenceId].activeUnitIndex updated; POST to the
  goto_position handler URL with body { position: newIndex + 1 } (the body
  assertion is new — redux.test.js only checked the URL; it pins the
  1-indexed wire format the api comment promises).
• network error → logError called, activeUnitIndex reverted to the seeded
  value.
• optimistic mid-flight (new, per the Convert bookmarking to React Query + de-class UnitButton #2014 precedent): hanging mock →
  activeUnitIndex is already the new value before the request resolves.

useSaveIntegritySignature:

• success → userNeedsIntegritySignature flips true → false, POST to the
  agreements URL.
• masquerading (new at hook level; component-level coverage exists in
  HonorCode.test.jsx) → no request recorded, flag still flips to false.
• failure (new — the thunk's error path was untested): POST network error →
  logError called, flag stays true.

HonorCode.test.jsx — expected green as-is: renders through setupTest's
render (store + query client both provided), keeps its axios mocks, and its
four cases assert exactly the behavior the hook preserves (navigate on cancel,
POST when agreeing, POST when generally masquerading, no POST when
masquerading a specific student). No fireEvent → userEvent churn unless the
file needs touching anyway.

CoursewareContainer.test.jsx — no goto_position coverage exists;
no edits expected, verify green.

Pact tests (lmsPact.test.jsx) — call postSequencePosition /
the api layer directly; untouched.

Behavior changes

None intended — faithful ports: same optimistic write timing, same
revert-on-error with the pre-mutation index, same success re-write, same
logError calls, same masquerade skip, same no-write-on-failure for the
signature, fire-and-forget at both call sites.

Decision doc

For the PR body at submit time:

• One layer, not two — same-kind changes in the same files (contrast with the
  Convert bookmarking to React Query + de-class UnitButton #2014 peel+convert split).
• Hooks live in courseware/data/apiHooks.ts (no feature subdirectory —
  these are courseware/data thunks, unlike bookmarks).
• onMutate-context rollback for the sequence position (vs pre-reading in the
  returned callback) — RQ-native; callback signature matches the thunk.
• The redundant onSuccess re-write kept, comment and all (faithful port).
• Masquerade skip as a resolve-without-request mutationFn branch.
• Model store stays the write target (no setQueryData; bridge durability;
  masquerade-refetch caveat is pre-existing) — deferred to Dissolve the model-store normalized cache #1977.
• Guard-closure capture in CoursewareContainer is safe (mutate stable).
• redux.test.js deleted (nothing left in it); new hook-level cases:
  1-indexed body, mid-flight optimistic write, masquerade skip,
  integrity-failure path.

Stack

One new layer stacked on #2066 (the bookmark conversion), closing this issue.
Submitted once green.

Verification

• npm run test -- src/courseware/data src/courseware/course/sequence/honor-code src/courseware/CoursewareContainer.
• npm run types and npm run lint.
• Manual smoke (tutor local, DemoX): sequence position — open a sequence whose
  save_position is set (exam-style subsections; the Demo Course "Homework -
  Question Styles" sequence reports save_position: false, so pick/author one
  that saves), click through units, watch goto_position POSTs in the Network
  tab, reload → lands on the last-active unit; offline toggle → position
  reverts + console error. Honor code — needs a course with the integrity
  signature feature enabled (user_needs_integrity_signature: true on
  courseware metadata); if no local course qualifies, the hook-level +
  HonorCode.test.jsx coverage carries it, noted in the manual-testing doc.