Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion src/components/NumFound/NumFound.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ const sanitizeNum = (num: number): string => {

export const NumFound = (props: INumFoundProps): ReactElement => {
const { count = 0, isLoading } = props;
const searchStatus = useStore((state) => state.searchStatus);

if (isLoading) {
return (
Expand All @@ -35,7 +36,7 @@ export const NumFound = (props: INumFoundProps): ReactElement => {
<Text as="span" fontWeight="bold">
{countString}
</Text>{' '}
results <SortStats />
results {searchStatus === 'success' ? <SortStats /> : null}
</Text>
</Box>
);
Expand Down
27 changes: 9 additions & 18 deletions src/components/SearchFacet/FacetList.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ import {
PopoverCloseButton,
PopoverContent,
PopoverHeader,
Skeleton,
Spinner,
Stack,
Text,
Expand Down Expand Up @@ -122,7 +121,7 @@ export const NodeList = memo(
const updateModal = useFacetStore(selectors.updateModal);
const depth = getLevelFromKey(prefix) + 1;
const expandable = params.hasChildren && (level === 'root' || params.maxDepth > depth);
const { treeData, isFetching, isLoading, isError } = useGetFacetData({
const { treeData, isFetching, isLoading, isSearchLoading, isError } = useGetFacetData({
...params,
prefix,
level,
Expand Down Expand Up @@ -158,10 +157,10 @@ export const NodeList = memo(
);
}

if (isFetching || isLoading) {
if (isFetching || isLoading || isSearchLoading) {
return (
<Center data-testid="search-facet-loading">
<Spinner size="sm" />
<Center py="4" data-testid="search-facet-loading">
<Spinner size="sm" color="gray.400" />
</Center>
);
} else if (treeData?.length === 0) {
Expand Down Expand Up @@ -298,6 +297,7 @@ export const NodeListModal = (props: INodeListProps) => {
treeData,
isFetching,
isLoading,
isSearchLoading,
isError,
pagination,
handleLoadMore,
Expand All @@ -313,20 +313,11 @@ export const NodeListModal = (props: INodeListProps) => {
sortDir,
});

if (isFetching || isLoading) {
if (isFetching || isLoading || isSearchLoading) {
return (
<Stack spacing="2" data-testid="search-facet-loading">
<Skeleton h="24px" />
<Skeleton h="24px" />
<Skeleton h="24px" />
<Skeleton h="24px" />
<Skeleton h="24px" />
<Skeleton h="24px" />
<Skeleton h="24px" />
<Skeleton h="24px" />
<Skeleton h="24px" />
<Skeleton h="24px" />
</Stack>
<Center py="4" data-testid="search-facet-loading">
<Spinner size="sm" color="gray.400" />
</Center>
);
} else if (isEmpty(treeData)) {
return (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ import { join, last, map, pipe, pluck, split } from 'ramda';
import { parseAPIError } from '@/utils/common/parseAPIError';
import { FacetItem, FacetLogic } from '../types';

interface ISearchFacetModalProps extends Omit<IFacetListProps, 'onError'> {
interface ISearchFacetModalProps extends Omit<IFacetListProps, 'onError' | 'children'> {
children: (props: { searchTerm: string }) => ReactNode;
}

Expand Down
3 changes: 2 additions & 1 deletion src/components/SearchFacet/YearHistogramSlider.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ export interface IYearHistogramSliderProps {

const Component = ({ onQueryUpdate, width, height, onExpand, expanded }: IYearHistogramSliderProps) => {
const query = useStore((state) => state.latestQuery);
const searchStatus = useStore((state) => state.searchStatus);

// query without the year range filter, to show all years on the histogram
const cleanedQuery = useMemo(() => {
Expand All @@ -38,7 +39,7 @@ const Component = ({ onQueryUpdate, width, height, onExpand, expanded }: IYearHi
}, [query]);

const { data } = useGetSearchFacetCounts(getSearchFacetYearsParams(cleanedQuery), {
enabled: !!cleanedQuery && cleanedQuery.q.trim().length > 0,
enabled: searchStatus === 'success' && !!cleanedQuery && cleanedQuery.q.trim().length > 0,
suspense: true,
});

Expand Down
4 changes: 2 additions & 2 deletions src/components/SearchFacet/store/FacetStore.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { facetConfig } from '@/components/SearchFacet/config';
import { FacetItem, IFacetParams, SearchFacetID } from '@/components/SearchFacet/types';
import { omit, pick, uniq } from 'ramda';
import { createElement, FC } from 'react';
import { createElement, FC, ReactNode } from 'react';
import create from 'zustand';
import createContext from 'zustand/context';
import { computeNextSelectionState, createNodes, getSelected } from './helpers';
Expand Down Expand Up @@ -128,7 +128,7 @@ const createStore = (preloadedState: Partial<IFacetStoreState>) => () =>
const FacetStoreContext = createContext<IFacetStoreState & FacetStoreEvents>();
export const useFacetStore = FacetStoreContext.useStore;

export const FacetStoreProvider: FC<{ facetId: SearchFacetID }> = ({ children, facetId }) => {
export const FacetStoreProvider: FC<{ facetId: SearchFacetID; children?: ReactNode }> = ({ children, facetId }) => {
const params = pick(
[
'label',
Expand Down
135 changes: 129 additions & 6 deletions src/components/SearchFacet/useGetFacetData.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import { describe, test, expect, vi, TestContext } from 'vitest';
import { renderHook, waitFor, createServerListenerMocks, urls } from '@/test-utils';
import { renderHook, waitFor, act, createServerListenerMocks, urls } from '@/test-utils';
import { useStore } from '@/store';
import { IUseGetFacetDataProps } from './useGetFacetData';
import { useGetFacetData } from './useGetFacetData';
import { defaultQueryParams } from '@/store/slices/search';
import { FacetField } from '@/api/search/types';
Expand All @@ -14,22 +16,27 @@ const defaultProps = {
level: 'root' as const,
};

const useCompound = (props: IUseGetFacetDataProps) => ({
setSearchStatus: useStore((state) => state.setSearchStatus),
facet: useGetFacetData(props),
});

describe('useGetFacetData', () => {
test('does not fire a request when latestQuery.q is empty', async ({ server }: TestContext) => {
test('does not fire a request when searchStatus is idle', async ({ server }: TestContext) => {
const { onRequest } = createServerListenerMocks(server);
renderHook(() => useGetFacetData(defaultProps), {
initialStore: { latestQuery: { ...defaultQueryParams, q: '' } },
initialStore: { searchStatus: 'idle', latestQuery: { ...defaultQueryParams, q: 'star' } },
});

await new Promise((r) => setTimeout(r, 200));
const searchRequests = urls(onRequest).filter((u) => u === '/search/query');
expect(searchRequests).toHaveLength(0);
});

test('does not fire a request when latestQuery.q is whitespace-only', async ({ server }: TestContext) => {
test('does not fire a request when searchStatus is not success (empty)', async ({ server }: TestContext) => {
const { onRequest } = createServerListenerMocks(server);
renderHook(() => useGetFacetData(defaultProps), {
initialStore: { latestQuery: { ...defaultQueryParams, q: ' ' } },
initialStore: { searchStatus: 'empty', latestQuery: { ...defaultQueryParams, q: 'star' } },
});

await new Promise((r) => setTimeout(r, 200));
Expand All @@ -40,7 +47,7 @@ describe('useGetFacetData', () => {
test('fires a request when latestQuery.q is non-empty', async ({ server }: TestContext) => {
const { onRequest } = createServerListenerMocks(server);
renderHook(() => useGetFacetData(defaultProps), {
initialStore: { latestQuery: { ...defaultQueryParams, q: 'star' } },
initialStore: { searchStatus: 'success', latestQuery: { ...defaultQueryParams, q: 'star' } },
});

await waitFor(() => {
Expand All @@ -49,3 +56,119 @@ describe('useGetFacetData', () => {
});
});
});

describe('useGetFacetData — searchStatus gating', () => {
test('does not fire when searchStatus is loading', async ({ server }: TestContext) => {
const { onRequest } = createServerListenerMocks(server);

renderHook(() => useCompound(defaultProps), {
initialStore: {
searchStatus: 'loading',
latestQuery: { ...defaultQueryParams, q: 'star' },
},
});

await new Promise((r) => setTimeout(r, 200));
const facetRequests = urls(onRequest).filter((u) => u === '/search/query');
expect(facetRequests).toHaveLength(0);
});

test('does not fire when searchStatus is empty', async ({ server }: TestContext) => {
const { onRequest } = createServerListenerMocks(server);

renderHook(() => useCompound(defaultProps), {
initialStore: {
searchStatus: 'empty',
latestQuery: { ...defaultQueryParams, q: 'star' },
},
});

await new Promise((r) => setTimeout(r, 200));
const facetRequests = urls(onRequest).filter((u) => u === '/search/query');
expect(facetRequests).toHaveLength(0);
});

test('does not fire when searchStatus is error', async ({ server }: TestContext) => {
const { onRequest } = createServerListenerMocks(server);

renderHook(() => useCompound(defaultProps), {
initialStore: {
searchStatus: 'error',
latestQuery: { ...defaultQueryParams, q: 'star' },
},
});

await new Promise((r) => setTimeout(r, 200));
const facetRequests = urls(onRequest).filter((u) => u === '/search/query');
expect(facetRequests).toHaveLength(0);
});

test('fires and returns data when searchStatus is success', async ({ server }: TestContext) => {
const { onRequest } = createServerListenerMocks(server);

const { result } = renderHook(() => useCompound(defaultProps), {
initialStore: {
searchStatus: 'success',
latestQuery: { ...defaultQueryParams, q: 'star' },
},
});

await waitFor(() => {
const facetRequests = urls(onRequest).filter((u) => u === '/search/query');
expect(facetRequests.length).toBeGreaterThan(0);
});

await waitFor(() => {
expect(result.current.facet.treeData.length).toBeGreaterThan(0);
});
});

test('regression: loading→success transition unblocks fetch and populates data', async ({ server }: TestContext) => {
const { onRequest } = createServerListenerMocks(server);

const { result } = renderHook(() => useCompound(defaultProps), {
initialStore: {
searchStatus: 'loading',
latestQuery: { ...defaultQueryParams, q: 'star' },
},
});

await new Promise((r) => setTimeout(r, 200));
expect(urls(onRequest).filter((u) => u === '/search/query')).toHaveLength(0);
expect(result.current.facet.treeData).toHaveLength(0);

act(() => {
result.current.setSearchStatus('success');
});

await waitFor(() => {
expect(urls(onRequest).filter((u) => u === '/search/query').length).toBeGreaterThan(0);
});

await waitFor(() => {
expect(result.current.facet.treeData.length).toBeGreaterThan(0);
});
});

test('success→loading transition clears treeData synchronously', async ({ server }: TestContext) => {
createServerListenerMocks(server);

const { result } = renderHook(() => useCompound(defaultProps), {
initialStore: {
searchStatus: 'success',
latestQuery: { ...defaultQueryParams, q: 'star' },
},
});

await waitFor(() => {
expect(result.current.facet.treeData.length).toBeGreaterThan(0);
});

act(() => {
result.current.setSearchStatus('loading');
});

expect(result.current.facet.treeData).toHaveLength(0);
expect(result.current.facet.totalResults).toBe(0);
});
});
12 changes: 7 additions & 5 deletions src/components/SearchFacet/useGetFacetData.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ const querySelector = (state: AppState) => omit(['fl', 'start', 'rows'], state.l

export const useGetFacetData = (props: IUseGetFacetDataProps) => {
const searchQuery = useStore(querySelector);
const searchStatus = useStore((state: AppState) => state.searchStatus);
const {
field,
query = '',
Expand All @@ -59,7 +60,7 @@ export const useGetFacetData = (props: IUseGetFacetDataProps) => {
setPagination(calculatePagination({ page: 0, numPerPage: FACET_DEFAULT_LIMIT }));
}, [prefix, searchTerm, sortDir]);

const isQueryEnabled = enabled && isNonEmptyString(searchQuery?.q?.trim());
const isQueryEnabled = enabled && searchStatus === 'success';

// fetch the data
const { data, ...result } = useGetSearchFacetJSON(
Expand All @@ -83,12 +84,12 @@ export const useGetFacetData = (props: IUseGetFacetDataProps) => {
},
{
enabled: isQueryEnabled,
keepPreviousData: true,
},
);

const res = data?.[field];
const treeData = useMemo(() => formatTreeData(res?.buckets ?? []), [res?.buckets]);
const rawTreeData = useMemo(() => formatTreeData(res?.buckets ?? []), [res?.buckets]);
const treeData = searchStatus === 'success' ? rawTreeData : ([] as FacetItem[]);

const identifiers = useMemo(
() =>
Expand Down Expand Up @@ -160,12 +161,13 @@ export const useGetFacetData = (props: IUseGetFacetDataProps) => {

return {
treeData: enhancedTreeData,
totalResults: res?.numBuckets ?? 0,
totalResults: searchStatus === 'success' ? res?.numBuckets ?? 0 : 0,
pagination: pagination,
handlePrevious,
handleLoadMore,
handlePageChange,
canLoadMore: res?.numBuckets !== treeData?.length,
canLoadMore: searchStatus === 'success' && res?.numBuckets !== treeData?.length,
isSearchLoading: searchStatus === 'loading',
...result,
isLoading: (isQueryEnabled && result.isLoading) || (hasIdentifiers && isLoading),
isFetching: result.isFetching || (hasIdentifiers && isFetching),
Expand Down
Loading
Loading