SCIX-871 fix: gate facets on searchStatus to prevent stale data#860
SCIX-871 fix: gate facets on searchStatus to prevent stale data#860thostetler wants to merge 6 commits intoadsabs:masterfrom
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #860 +/- ##
========================================
- Coverage 62.7% 62.6% -0.0%
========================================
Files 323 323
Lines 38011 38031 +20
Branches 1723 1720 -3
========================================
- Hits 23802 23797 -5
- Misses 14169 14194 +25
Partials 40 40
🚀 New features to boost your workflow:
|
SCIX-871 test: add searchStatus facet gating integration tests SCIX-871 feat: gate facets on searchStatus to prevent stale data after empty/failed searches SCIX-871 fix: drive searchStatus from search page to gate facets SCIX-871 perf: use useLayoutEffect to start facets in same frame as search results
SCIX-871 fix: use varied two-column skeleton rows in facets during search loading SCIX-871 fix: use spinner instead of skeleton rows in facets during search loading
78b4903 to
6f40e80
Compare
There was a problem hiding this comment.
Pull request overview
Risk summary: High — the new search-status gating moves the stale-facet fix in the right direction, but there are still stale latestQuery paths and an error-state regression in the facet UI.
This PR adds an explicit main-search status to the shared search store so facet-related UI can stop showing cached data from an earlier query when the primary search is loading, empty, or errored. It also tightens test setup around Zustand store creation and fixes a couple of TypeScript prop-type issues in facet components.
Changes:
- Add
searchStatusto the search slice and drive its transitions from/search. - Gate facet requests/rendered facet data on successful main-search completion and show loading spinners in facet UIs.
- Update test utilities to create fresh stores per test and fix TS typing conflicts in facet provider/modal components.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
src/test-utils.tsx |
Test wrapper now builds a fresh app store instead of reusing the cached client store helper. |
src/store/slices/search.ts |
Adds SearchStatus state and setter to the global search slice. |
src/pages/search/index.tsx |
Drives search-status changes from the main search query lifecycle. |
src/components/SearchFacet/useGetFacetData.ts |
Gates facet fetching/data exposure on main-search success. |
src/components/SearchFacet/useGetFacetData.test.ts |
Adds hook tests for status-based facet gating. |
src/components/SearchFacet/store/FacetStore.ts |
Adjusts facet store provider typing for children. |
src/components/SearchFacet/SearchFacetModal/SearchFacetModal.tsx |
Resolves the modal children render-prop typing conflict. |
src/components/SearchFacet/FacetList.tsx |
Updates facet loading UI to show spinners for both facet-fetch and main-search loading states. |
- Replace useLayoutEffect with useIsomorphicLayoutEffect (falls back to useEffect on the server) to eliminate the React SSR warning that fires when useLayoutEffect is used on a server-rendered page. - Move setQuery/submitQuery inside the numFound > 0 branch so empty-result searches do not open a Sentry performance span that can never close (providers.tsx only closes spans when docs.current is non-empty).
NumFound's SortStats sub-component and YearHistogramSlider both read latestQuery directly and fire API requests even when the main search has errored, showing stale data from the previous query. Gate both on searchStatus === 'success' so they stay quiet until the search completes successfully.
enabled:false stops new requests but React Query keeps the cached data in memory. Gate histogramData derivation on searchStatus so the stale histogram clears immediately when the search transitions away from success.
| <Center data-testid="search-facet-loading"> | ||
| <Spinner size="sm" /> | ||
| <Center py="4" data-testid="search-facet-loading"> | ||
| <Spinner size="sm" color="gray.400" /> |
There was a problem hiding this comment.
The hard coded color here might be an accessibility issue (although it doesn't stay on screen that long). I believe gray.400 doesn't have enough contrast on white background, and also considering we also have dark mode, maybe it's best to specify the color using the useColorModeColor hook.
| <Skeleton h="24px" /> | ||
| </Stack> | ||
| <Center py="4" data-testid="search-facet-loading"> | ||
| <Spinner size="sm" color="gray.400" /> |
Facets were showing stale data from the previous search when a new search returned zero results or errored. The root cause: latestQuery (what facets use to build their requests) only updated when docs.length > 0, and facets had no signal for whether the main search was still in flight, empty, or failed.
isLoadingrather than isFetching to avoid toggling facets off during background refetches of the same query