refactor: convert courseware search from Redux to React Query

[brian-smith-tcril]

Summary

Converts the courseware search feature from Redux to React Query, per OEP-0067 ADR-0010. Part of the Redux → React Query migration (#1946), stacked on the product-tours conversion (the branch below).

Behavior is preserved — same open/close, spinner-per-search, results and "Results for …" summary, client-side type-filter tabs, deep-link auto-open, and close/backspace clear. Verified with the full test suite and a live manual smoke pass.

What changed

• React Query data layer: courseware-search/data/queryKeys.ts (rooted at appId, keyed by the URL keyword) + data/apiHooks.ts — useCoursewareSearchResults (a useQuery keyed off ?q=, enabled when there's a keyword) and useCoursewareSearchEnabled (the feature-flag check, retry: false to match the original one-shot fail-to-false).
• Client state → context: CoursewareSearchContext (show/open/close) replaces the showSearch slice member. The provider wraps the PluginSlot in CourseTabsNavigationSlot, so a plugin in that slot can still hook into search via the exported useCoursewareSearch() — Redux made showSearch globally reachable, and this preserves that capability rather than burying it in local state.
• Consumers: CoursewareSearch (panel) and CoursewareResultsFilter read results from the query (isLoading/isError/data) instead of the contentSearchResults model; the type-filter tabs stay client-side (the API returns a flat list, so switching tabs doesn't refetch); CoursewareSearchToggle / CourseTabsNavigation use the context.
• Redux removed: the searchCourseContent + fetchCoursewareSearchSettings thunks, the setShowSearch/showSearch members of the course-home slice, and the contentSearchResults model write.
• Opportunistic fix: corrected a rules-of-hooks ordering in CoursewareResultsFilter (two useMemos were running after early returns).
• Search-on-type readiness: architected — following frontend-app-catalog's search — so switching to search-on-type later is an input-handler-only change (debounce + keepPreviousData), with no rework of the query, query key, or client state. See the decision log.

Testing

Automated: npm run types, npm run lint, npm run build, and the full npm test suite pass.

Manual smoke (dev, with the courseware.mfe_courseware_search flag on): toggle open (scroll-lock, no URL change) → submit → spinner → results + "Results for …" summary → filter tabs drive ?f= → ?q= set on submit → close/backspace clear → deep-link auto-opens and runs the search on the right tab → a second search blanks + re-spins (matches the on-submit baseline). No search-related console errors.

Decisions

Full decision log

Decisions — Redux → React Query: courseware search

Working notes for this PR (part of the wider Redux → React Query migration,
#1946). Not checked in — referenced when opening the PR. Stacked on the
product-tours conversion (#1968).

Scope

Convert the courseware-search feature off Redux:

• searchCourseContent thunk (writes the contentSearchResults model) → a
  React Query query keyed by the URL search keyword.
• setShowSearch action + showSearch state (client UI flag) → a
  CoursewareSearchProvider React context (see Client state below).
• fetchCoursewareSearchSettings (already a plain async fn, not Redux) → a
  useQuery for the feature flag (dedupe/caching; improvement, not Redux removal).

Removes the thunk, the setShowSearch/showSearch slice members, and one
models write (contentSearchResults). The courseHome slice stays (tab
machinery).

Architecture: search-on-type readiness (from frontend-app-catalog)

We convert on-submit now (faithful to current behavior), but architect so
adopting search-on-type later is an input-handler-only change — no rework of
the React Query hook, query key, or client state. Basis: frontend-app-catalog's
search, which landed search-on-type starting from an on-submit design without
touching its query layer (catalog PR #42, tracker #48, polish #76). The
load-bearing lesson from that thread: "only searching using onSubmit is hiding
issues" — on-submit masks the state bugs (cyclic re-renders, flicker, stale
results) that on-type exposes, so build the query/state layer right up front.

Query hook — useCoursewareSearchResults(courseId, keyword):

• queryKey: [appId, courseId, 'search', keyword] — the term is in the key.
  The ?f= type filter stays client-side on the results (as today,
  CoursewareResultsFilter), so it is not in the key (unlike catalog, which
  keyed filters into the query).
• placeholderData: keepPreviousData — deliberately NOT added now (deferred to
  the search-on-type work). Tested the current behavior: a second search (with the
  panel open) blanks the results and shows the spinner each time. That's the
  correct feedback for search-on-click (a click should visibly start a fresh
  search), and only becomes wrong for search-on-type (where you want the
  previous results to stay put between keystrokes). Since this PR stays on-click, we
  keep the faithful blank+spinner behavior — spinner driven off isPending
  (default), no placeholderData.
  • On-type follow-up: add placeholderData: keepPreviousData and drive the
    spinner off isFetching (not isPending) so previous results persist while the
    next keystroke's fetch runs (also gives out-of-order-response protection without
    an AbortSignal). This is a small, localized query-hook + spinner change — it
    does not touch the query key, the term/URL flow, or the client state, so the
    on-type-readiness goal holds. (Note: the v5 form is the imported helper
    placeholderData: keepPreviousData; the keepPreviousData: true boolean
    option was removed in v5, but the helper import remains — catalog's
    (prev) => prev is the exact inline equivalent.)
• enabled: !!keyword — courseware search is idle-when-empty (shows
  nothing until you search), unlike catalog's browse-all-when-empty (no enabled).
  Orthogonal to on-type. No min-length gating (catalog deliberately didn't).

Loading status uses isLoading (= isPending && isFetching), not isFetching
or isPending alone. In v5 the disabled (no-keyword) query is isPending: true
with fetchStatus: 'idle', so isPending alone would show a spinner at the idle
state; isFetching alone would flash the spinner on a background focus-refetch
(data present). isLoading is true only while fetching with no data to show —
which reproduces the baseline exactly (spinner on each fresh/new-keyword search,
nothing at idle) and degrades gracefully if a background refetch ever runs. It also
means we don't need refetchOnWindowFocus: false here (unlike the tours query): a
focus refetch is invisible because isLoading stays false while data is present.

Feature-flag query — useCoursewareSearchEnabled(courseId) uses retry: false
(consumer reads data?.enabled ?? false). This matches the original
fetchCoursewareSearchSettings, which wrapped getCoursewareSearchEnabled in a
try/catch and returned { enabled: false } one-shot: fail fast to "not enabled"
rather than retrying a "should I show this button" probe. The results query keeps
the bare-client default retry.

Term flow / choke point — the keyword stays in the URL (?q=, via
useCoursewareSearchParams); handleSubmit → setQuery is the single choke point.
Adopting on-type later = debounce the input's onChange → setQuery in a separate
input hook, with setSearchParams(..., { replace: true }) (avoid history spam)
and a lastQueryRef guard on the debounced dispatch — not the live term — to
avoid the cyclic re-fire catalog hit, and don't fully control the input value
while debouncing (their "repeating" bug). None of that touches the query hook or
the showSearch state.

Client state (showSearch) → context, not local state

showSearch becomes a CoursewareSearchProvider context (show + open() +
close()), exported from the feature alongside a useCoursewareSearch() hook.

Why context and not lifted local state (the first instinct — CourseTabsNavigation
is the single common parent of the toggle + panel). Redux's showSearch was
globally readable, so a plugin dropped into CourseTabsNavigationSlot could hook
into search today via useSelector/dispatch. That's not an official API (no
pluginProp), so breaking a plugin's old Redux code is acceptable — but making it
impossible to interact with search would be a needless capability regression.
Local state (or a context buried inside the default CourseTabsNavigation) would
do exactly that: a plugin that wraps or replaces the nav via the slot renders
outside it and couldn't reach the state. Context also keeps the state with the
search feature (not the nav component) and drops prop-drilling — consistent with
the tours conversion and OEP-0067.

Placement: the provider wraps the PluginSlot inside CourseTabsNavigationSlot,
so everything the slot renders — the default nav, the toggle, the panel, and any
plugin — is inside it and can call the exported useCoursewareSearch(). It has to
sit above the PluginSlot (not inside CourseTabsNavigation) precisely so
slot plugins keep access. The panel stays conditionally mounted
({show && <CoursewareSearch />}) so its showModal()/scroll-lock mount effects
still fire on open; CourseTabsNavigation reads show for that gate, the toggle
reads open, the panel reads close.

Lifecycle: the provider is per-tab, so show resets on tab navigation — but
this is unobservable, because search can't be open across a tab change: the
panel is a modal (dialog.showModal()) that inerts the background (the tab bar
isn't clickable while it's open), closing via the X clears the query string, and
popstate is wired to close it. So per-tab scope is fine; Redux's global
persistence of showSearch had no reachable effect here.

Follow-up option: if we later want a first-class API rather than "import our
hook," expose search controls as a pluginProp on the slot. Not needed now.

Results filtering stays client-side

The /search/{courseId} endpoint returns a flat array of all results — it takes
no filter param. CoursewareResultsFilter partitions that array client-side (a
useMemo reducer) by each result's type into buckets (all + text / video /
sequence / other), renders one Paragon <Tab> per non-empty bucket, and the
?f= param just selects which already-computed bucket to show. Switching filters
does not refetch. (It also skips the tabs when there'd be < 3, i.e. "all" plus
a single identical bucket.)

• In RQ, the grouping logic stays put in CoursewareResultsFilter; only the
  source of the flat array changes: useModel('contentSearchResults', courseId)
  → data.results from useCoursewareSearchResults(courseId, keyword). The ?f=
  tab selection is untouched.
• The ?f= filter is deliberately NOT in the query key — switching tabs must
  not refetch (matches today). This is the key contrast with catalog, which put
  filters in the query because its API filtered server-side.
• Opportunistic fix (rules of hooks): today CoursewareResultsFilter calls its
  useMemos after two early returns (if (!lastSearch) return null,
  if (!data.length) return …) — a conditional-hooks violation that happens to
  work. While rewiring this file, hoist the early returns below the hooks so
  the hooks always run.

Results-summary label keyword: use the URL term, drop lastSearchKeyword

The results summary reads Results for "<keyword>":. The original sourced that
keyword from the model (contentSearchResults.searchKeyword, aliased
lastSearchKeyword) — the keyword the displayed results are for — separate from
searchKeyword (the current URL/?q= term). The RQ port uses searchKeyword
(the URL term) and drops lastSearchKeyword.

Why that's correct, not a regression:

• In the on-submit flow the two always coincide when the summary is visible.
  The URL only changes on submit (setQuery, same handler that starts the fetch);
  while fetching, status === 'loading' hides the summary; when results arrive the
  model's keyword equals the submitted value equals the URL. So lastSearchKeyword
  was redundant here — the original read from the model only because results +
  keyword were stored together, not to guard a differ-case. (The existing test's
  model='fubar' / URL='' split is an artificial mock, not a real state.)
• frontend-app-catalog does exactly this — its "Search results for {query}"
  heading uses the live input/URL searchString, not the term the data is for,
  and it does not guard the keepPreviousData mismatch window (heading shows
  the new term while the list still shows the previous term mid-fetch). Catalog
  PR fix: “current.” was left over from when the implementation used refs #42 even removed a dedicated "last search query" heading state in favor of
  the live term. So using the URL term matches the reference even under
  search-on-type.

Consequences: no need to echo the keyword back in the query data (an earlier
idea, dropped); and no on-type follow-up for the label — the URL term is the
intended behavior for on-type too. If we ever wanted the label to always match the
on-screen rows under keepPreviousData (stricter than catalog), we'd derive it
from the query data instead — but that's a deliberate deviation we're not making.

Tests & coverage

• CoursewareSearchContext.test.tsx — provider/hook unit tests (initial show,
  open/close, the throw-outside-provider). These were the biggest patch gap
  since every consumer test mocks the context.
• data/apiHooks.test.tsx — useCoursewareSearchEnabled and
  useCoursewareSearchResults (fetch + mapSearchResponse, and the enabled gate
  that skips fetching with no keyword); also exercises queryKeys.
• Consumer tests (CoursewareSearch, CoursewareResultsFilter, CoursewareSearchToggle,
  CourseTabsNavigation, hooks) were updated to mock the query + context instead
  of the model/thunk/slice.
• Deliberately did NOT add a CourseTabsNavigationSlot test. The only way to
  cover the slot's provider wrapper is to assert its default content is
  CourseTabsNavigation — which over-commits a test to a default the Slot API
  intentionally leaves replaceable. So the few provider-wrapper lines there stay
  uncovered on purpose rather than baking in that assumption.

Current behavior (baseline — from live manual testing on master)

Feature gated by the courseware.mfe_courseware_search CourseWaffleFlag
(COURSEWARE_MICROFRONTEND_SEARCH_ENABLED); the /courseware-search/enabled/
endpoint just checks that flag, which is what makes the "Content search" toggle
appear in the course tab bar.

• Open: the dialog covers everything below the header/masquerade bar (anchored
  under courseTabsNavigation), showing the "Search this course" idle state (title

  • search box + button). Opening does not change the URL. Background scroll is
    locked while open (useLockScroll).

• Typing vs submitting: typing in the box does not change the URL; only
  submitting (the search button) sets ?q=<term>. (onChange only clears on
  empty; handleSubmit calls setQuery.) → this is why keying the query off the
  URL keyword is faithful: it re-runs on submit, not per keystroke.

• Submitting a search: shows a spinner while loading, then results. Above the
  results, a summary line reads Results for "<keyword>": — there is no
  numeric count (the searchResultsLabel message only uses {keyword}; total
  is passed but unused, and only gates whether the summary renders, via
  total > 0). Results have filter tabs — all content / text / other — backed
  by the ?f= URL param (CoursewareResultsFilter): text → f=text,
  other → f=other. Each result row is a content-type icon (a "T" for text vs
  a document icon for other) + the unit title with a per-result match-count
  badge (e.g. "Code Grading 3") + a breadcrumb path to the unit. (Confirmed
  against a live screenshot.)

• Closing / clearing: the X clears ?q=/?f= and the results; reopening
  starts fresh. Backspacing the box to empty also clears the query param and
  results immediately — no submit needed (handleOnChange: if (!value) clearSearch()). Feels odd but preserved as-is.

• Deep link: loading a course URL that already has ?q=<term> (and optional
  ?f=) auto-opens the search dialog and runs the search, landing on the
  ?f= results tab. Two effects: CoursewareSearchToggle opens when
  enabled && query; CoursewareSearch runs handleSubmit on mount when there's
  a keyword. The URL-keyed query preserves this (it auto-runs off the URL keyword
  on load); the auto-open becomes: initialize the lifted showSearch from
  enabled && query.

Known quirks (preserve-vs-fix TBD during conversion)

• Closing the dialog stamps empty ?q=&f= onto the URL, even when no search
  was performed. Cause: close() → clearSearch() → clearSearchParams() does
  setSearchParams({ q: '', f: '' }) (the initSearchParams), and React Router
  writes empty-valued params. Lives in useCoursewareSearchParams (a URL hook,
  orthogonal to the Redux work). Almost certainly unintended — candidate for an
  opportunistic fix (remove the params, or don't touch the URL when nothing to
  clear) when we rework close().
• "All content" filter is represented two ways. On first search the
  all-content tab is active with f= (blank), but explicitly clicking the
  all-content tab sets f=all. So the same filter is both "" and "all".
  Cosmetic URL inconsistency in CoursewareResultsFilter; candidate to normalize.

Closes #1979

🤖 Generated with Claude Code

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 92.61%. Comparing base (a3ce0af) to head (75718d7).

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1970      +/-   ##
==========================================
+ Coverage   92.34%   92.61%   +0.27%     
==========================================
  Files         355      358       +3     
  Lines        5852     5851       -1     
  Branches     1404     1368      -36     
==========================================
+ Hits         5404     5419      +15     
+ Misses        429      413      -16     
  Partials       19       19              

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

[arbrandes]

Looks good, but have two inline questions.

[arbrandes]

Was dropping the feature-flag check here intentional?

useCoursewareSearchState() returned { show: enabled && show }, so the panel below ({show && <CoursewareSearch />}) was gated on the waffle flag too. useCoursewareSearch() returns the raw context flag, and enabled isn't reinstated anywhere. Only CoursewareSearchToggle still checks it, and only to decide whether to render its own button.

[brian-smith-tcril]

Good catch!

My manual testing didn't include a "try to load a deep link to a search when the waffle flag is off" case, so I missed this there, and the change to the test file slipped past me.

Addressed in https://github.com/openedx/frontend-app-learning/compare/ab03391b0fd50739feeed7a9e71a223800d22db7..2ba67fb5627c73aec6bd716f2527d29e0cd8db9e.

I restructured to have the checks live in CoursewareSearch.jsx, making the CoursewareSearch component a thin wrapper around what is now CoursewareSearchModal gating on show and enabled. The wrapper component ensures we're following rules of hooks (instead of throwing an early return before the useEffect, useCoursewareSearchParams, useLockScroll etc.)

[arbrandes]

slice.test.js still references this field — four hand-built initialState literals include showSearch, and line 141 asserts expect(newState.showSearch).toBe(initialState.showSearch) on something the slice no longer defines.

It passes rather than fails: those tests feed their own state object to the reducer, so the extra key rides through Immer untouched and the assertion is now vacuous. git grep showSearch is otherwise clean.

[brian-smith-tcril]

Addressed in https://github.com/openedx/frontend-app-learning/compare/2ba67fb5627c73aec6bd716f2527d29e0cd8db9e..75718d7590558c3fe9f16c90b3bfff2926861fb7

[arbrandes]

Thanks for addressing my comments! 👍🏼