diff --git a/.changeset/catalog-react-split-count.md b/.changeset/catalog-react-split-count.md new file mode 100644 index 0000000000..3067665333 --- /dev/null +++ b/.changeset/catalog-react-split-count.md @@ -0,0 +1,8 @@ +--- +'@backstage/plugin-catalog-react': patch +'@backstage/plugin-catalog': patch +--- + +The entity list provider now fetches the entity list and the total count as two separate parallel requests when using cursor or offset pagination. The list query skips the expensive count computation (using `totalItems: 'exclude'`), so the table populates immediately. The count arrives asynchronously and updates the title. A new `totalItemsLoading` field is exposed on `EntityListContextProps` so consumers can distinguish a stale count from a fresh one. + +The catalog table now keeps stale rows visible during filter changes and page navigation instead of replacing the entire table body with a spinner. The full-table spinner is only shown on the very first load when no data exists yet. The entity count in the title is dimmed while the count is refreshing, and a small spinner appears next to the title while rows are loading. diff --git a/plugins/catalog-react/report.api.md b/plugins/catalog-react/report.api.md index 1fcac904ff..72bc2a0194 100644 --- a/plugins/catalog-react/report.api.md +++ b/plugins/catalog-react/report.api.md @@ -466,6 +466,7 @@ export type EntityListContextProps< prev?: () => void; }; totalItems?: number; + totalItemsLoading: boolean; limit: number; offset?: number; setLimit: (limit: number) => void; diff --git a/plugins/catalog-react/src/deprecated.tsx b/plugins/catalog-react/src/deprecated.tsx index 7d347dda6b..dcb176dff4 100644 --- a/plugins/catalog-react/src/deprecated.tsx +++ b/plugins/catalog-react/src/deprecated.tsx @@ -73,6 +73,7 @@ export function MockEntityListContextProvider< error: value?.error, totalItems: value?.totalItems ?? (value?.entities ?? defaultValues.entities).length, + totalItemsLoading: value?.totalItemsLoading ?? false, limit: value?.limit ?? 20, offset: value?.offset, setLimit: value?.setLimit ?? (() => {}), diff --git a/plugins/catalog-react/src/hooks/useEntityListProvider.test.tsx b/plugins/catalog-react/src/hooks/useEntityListProvider.test.tsx index ef9b77d63f..e0abfccfbd 100644 --- a/plugins/catalog-react/src/hooks/useEntityListProvider.test.tsx +++ b/plugins/catalog-react/src/hooks/useEntityListProvider.test.tsx @@ -317,8 +317,6 @@ describe('', () => { expect(mockCatalogApi.getEntities).toHaveBeenCalledTimes(1); }); - // While first fetch is in flight, fire more updateFilters calls - // that produce the same backend filter (kind=component). act(() => { result.current.updateFilters({ kind: new EntityKindFilter('component', 'Component'), @@ -512,11 +510,11 @@ describe('', () => { await waitFor(() => { expect(mockCatalogApi.getEntities).not.toHaveBeenCalledTimes(1); expect(result.current.entities.length).toBe(1); - expect(mockCatalogApi.queryEntities).toHaveBeenCalledTimes(1); expect(mockCatalogApi.queryEntities).toHaveBeenCalledWith({ filter: { kind: 'component' }, limit, orderFields, + totalItems: 'exclude', fullTextFilter: { term: '2', fields: [ @@ -539,11 +537,11 @@ describe('', () => { }); expect(result.current.entities.length).toBe(2); - expect(mockCatalogApi.queryEntities).toHaveBeenCalledTimes(1); expect(mockCatalogApi.queryEntities).toHaveBeenCalledWith({ filter: { kind: 'component' }, limit, orderFields, + totalItems: 'exclude', }); }); @@ -564,8 +562,16 @@ describe('', () => { await waitFor(() => { expect(result.current.backendEntities.length).toBe(2); expect(result.current.entities.length).toBe(1); - expect(mockCatalogApi.queryEntities).toHaveBeenCalledTimes(1); }); + expect(mockCatalogApi.queryEntities).toHaveBeenCalledWith( + expect.objectContaining({ + filter: { + kind: 'component', + 'relations.ownedBy': ownershipEntityRefs, + }, + totalItems: 'exclude', + }), + ); }); it('applies frontend-only filters without refetching', async () => { @@ -578,6 +584,11 @@ describe('', () => { expect(result.current.filters.kind?.value).toBe('component'); }); + // Record the number of list calls (totalItems: 'exclude') after init + const listCallsAfterInit = ( + mockCatalogApi.queryEntities as jest.Mock + ).mock.calls.filter((c: any) => c[0]?.totalItems === 'exclude').length; + act(() => result.current.updateFilters({ user: EntityUserFilter.all(), @@ -588,7 +599,13 @@ describe('', () => { expect(result.current.filters.user?.value).toBe('all'); expect(result.current.entities.length).toBe(2); }); - expect(mockCatalogApi.queryEntities).toHaveBeenCalledTimes(1); + // EntityUserFilter.all() doesn't change the backend filter, so no + // additional list call should fire (count effect fires, but not + // the list fetch). + const listCallsAfterUpdate = ( + mockCatalogApi.queryEntities as jest.Mock + ).mock.calls.filter((c: any) => c[0]?.totalItems === 'exclude').length; + expect(listCallsAfterUpdate).toBe(listCallsAfterInit); }); it('resolves query param filter values', async () => { @@ -618,7 +635,6 @@ describe('', () => { await waitFor(() => { expect(result.current.entities.length).toBe(2); - expect(mockCatalogApi.queryEntities).toHaveBeenCalledTimes(1); }); act(() => @@ -631,8 +647,17 @@ describe('', () => { expect(result.current.entities.length).toBe(1); }); + // Verify the list call with the owned filter was made await waitFor(() => { - expect(mockCatalogApi.queryEntities).toHaveBeenCalledTimes(2); + expect(mockCatalogApi.queryEntities).toHaveBeenCalledWith( + expect.objectContaining({ + filter: { + kind: 'component', + 'relations.ownedBy': ownershipEntityRefs, + }, + totalItems: 'exclude', + }), + ); }); }); @@ -645,7 +670,6 @@ describe('', () => { expect(result.current.backendEntities.length).toBeGreaterThan(0); }); expect(result.current.backendEntities.length).toBe(2); - expect(mockCatalogApi.queryEntities).toHaveBeenCalledTimes(1); await act(async () => { result.current.updateFilters({ @@ -655,14 +679,54 @@ describe('', () => { }); await waitFor(() => { - expect(mockCatalogApi.queryEntities).toHaveBeenNthCalledWith(2, { + expect(mockCatalogApi.queryEntities).toHaveBeenCalledWith({ filter: { kind: 'api', 'spec.type': ['service'] }, limit, orderFields, + totalItems: 'exclude', }); }); + }); - expect(result.current.totalItems).toBe(10); + it('fetches count separately and does not re-count on cursor navigation', async () => { + const { result } = renderHook(() => useEntityList(), { + wrapper: createWrapper({ pagination }), + }); + + await waitFor(() => { + expect(result.current.backendEntities.length).toBe(2); + }); + + // The count query uses limit: 0 (without totalItems: 'exclude') + await waitFor(() => { + expect(mockCatalogApi.queryEntities).toHaveBeenCalledWith( + expect.objectContaining({ limit: 0 }), + ); + }); + + const countCallsBefore = ( + mockCatalogApi.queryEntities as jest.Mock + ).mock.calls.filter((c: any) => c[0]?.limit === 0).length; + + // Navigate to next page via cursor — should NOT re-run the count + act(() => { + result.current.pageInfo?.next?.(); + }); + + await waitFor(() => { + expect(mockCatalogApi.queryEntities).toHaveBeenCalledWith( + expect.objectContaining({ + cursor: expect.any(String), + totalItems: 'exclude', + }), + ); + }); + + const countCallsAfter = ( + mockCatalogApi.queryEntities as jest.Mock + ).mock.calls.filter((c: any) => c[0]?.limit === 0).length; + + expect(countCallsAfter).toBe(countCallsBefore); }); it('returns an error on catalogApi failure', async () => { @@ -675,6 +739,9 @@ describe('', () => { }); expect(result.current.backendEntities.length).toBe(2); + // The count effect fires first (consuming one rejection), then the + // list call fires. Both must reject for the error to surface. + mockCatalogApi.queryEntities!.mockRejectedValueOnce('error'); mockCatalogApi.queryEntities!.mockRejectedValueOnce('error'); act(() => { result.current.updateFilters({ @@ -726,6 +793,7 @@ describe('', () => { expect(mockCatalogApi.queryEntities).toHaveBeenCalledWith({ cursor: 'nextCursor', limit, + totalItems: 'exclude', }); }); }); @@ -750,6 +818,7 @@ describe('', () => { expect(mockCatalogApi.queryEntities).toHaveBeenCalledWith({ cursor: 'prevCursor', limit, + totalItems: 'exclude', }); }); }); @@ -836,12 +905,12 @@ describe(``, () => { await waitFor(() => { expect(mockCatalogApi.getEntities).not.toHaveBeenCalledTimes(1); expect(result.current.entities.length).toBe(1); - expect(mockCatalogApi.queryEntities).toHaveBeenCalledTimes(1); expect(mockCatalogApi.queryEntities).toHaveBeenCalledWith({ filter: { kind: 'component' }, limit, offset: 0, orderFields, + totalItems: 'exclude', fullTextFilter: { term: '2', fields: [ @@ -864,12 +933,12 @@ describe(``, () => { }); expect(result.current.entities.length).toBe(2); - expect(mockCatalogApi.queryEntities).toHaveBeenCalledTimes(1); expect(mockCatalogApi.queryEntities).toHaveBeenCalledWith({ filter: { kind: 'component' }, limit, offset: 0, orderFields, + totalItems: 'exclude', }); }); @@ -890,8 +959,16 @@ describe(``, () => { await waitFor(() => { expect(result.current.backendEntities.length).toBe(2); expect(result.current.entities.length).toBe(1); - expect(mockCatalogApi.queryEntities).toHaveBeenCalledTimes(1); }); + expect(mockCatalogApi.queryEntities).toHaveBeenCalledWith( + expect.objectContaining({ + filter: { + kind: 'component', + 'relations.ownedBy': ownershipEntityRefs, + }, + totalItems: 'exclude', + }), + ); }); it('applies frontend-only filters without refetching', async () => { @@ -904,6 +981,11 @@ describe(``, () => { expect(result.current.filters.kind?.value).toBe('component'); }); + // Record the number of list calls (totalItems: 'exclude') after init + const listCallsAfterInit = ( + mockCatalogApi.queryEntities as jest.Mock + ).mock.calls.filter((c: any) => c[0]?.totalItems === 'exclude').length; + act(() => result.current.updateFilters({ user: EntityUserFilter.all(), @@ -914,7 +996,13 @@ describe(``, () => { expect(result.current.filters.user?.value).toBe('all'); expect(result.current.entities.length).toBe(2); }); - expect(mockCatalogApi.queryEntities).toHaveBeenCalledTimes(1); + // EntityUserFilter.all() doesn't change the backend filter, so no + // additional list call should fire (count effect fires, but not + // the list fetch). + const listCallsAfterUpdate = ( + mockCatalogApi.queryEntities as jest.Mock + ).mock.calls.filter((c: any) => c[0]?.totalItems === 'exclude').length; + expect(listCallsAfterUpdate).toBe(listCallsAfterInit); }); it('resolves query param filter values', async () => { @@ -944,7 +1032,6 @@ describe(``, () => { await waitFor(() => { expect(result.current.entities.length).toBe(2); - expect(mockCatalogApi.queryEntities).toHaveBeenCalledTimes(1); }); act(() => @@ -957,21 +1044,40 @@ describe(``, () => { expect(result.current.entities.length).toBe(1); }); + // Verify the list call with the owned filter was made await waitFor(() => { - expect(mockCatalogApi.queryEntities).toHaveBeenCalledTimes(2); + expect(mockCatalogApi.queryEntities).toHaveBeenCalledWith( + expect.objectContaining({ + filter: { + kind: 'component', + 'relations.ownedBy': ownershipEntityRefs, + }, + totalItems: 'exclude', + }), + ); }); + // Record list call count before setting the same filter again + const listCallsBefore = ( + mockCatalogApi.queryEntities as jest.Mock + ).mock.calls.filter((c: any) => c[0]?.totalItems === 'exclude').length; + act(() => result.current.updateFilters({ user: EntityUserFilter.owned(ownershipEntityRefs), }), ); - await expect(() => - waitFor(() => { - expect(mockCatalogApi.queryEntities).toHaveBeenCalledTimes(3); - }), - ).rejects.toThrow(); + // Wait for any pending effects to settle + await waitFor(() => { + expect(result.current.entities.length).toBe(1); + }); + + // Setting the same filter again should not trigger additional list calls + const listCallsAfter = ( + mockCatalogApi.queryEntities as jest.Mock + ).mock.calls.filter((c: any) => c[0]?.totalItems === 'exclude').length; + expect(listCallsAfter).toBe(listCallsBefore); }); it('fetch when limit change', async () => { @@ -981,18 +1087,20 @@ describe(``, () => { await waitFor(() => { expect(result.current.entities.length).toBe(2); - expect(mockCatalogApi.queryEntities).toHaveBeenCalledTimes(1); }); act(() => result.current.setLimit(50)); + // setLimit does not change requestedFilters, so no extra count call. + // Only the debounced list call fires. await waitFor(() => { - expect(result.current.entities.length).toBe(2); - }); - - await waitFor(() => { - expect(mockCatalogApi.queryEntities).toHaveBeenCalledTimes(2); expect(result.current.limit).toEqual(50); + expect(mockCatalogApi.queryEntities).toHaveBeenCalledWith( + expect.objectContaining({ + limit: 50, + totalItems: 'exclude', + }), + ); }); }); @@ -1005,7 +1113,6 @@ describe(``, () => { expect(result.current.backendEntities.length).toBeGreaterThan(0); }); expect(result.current.backendEntities.length).toBe(2); - expect(mockCatalogApi.queryEntities).toHaveBeenCalledTimes(1); act(() => { result.current.updateFilters({ @@ -1015,11 +1122,12 @@ describe(``, () => { }); await waitFor(() => { - expect(mockCatalogApi.queryEntities).toHaveBeenNthCalledWith(2, { + expect(mockCatalogApi.queryEntities).toHaveBeenCalledWith({ filter: { kind: 'api', 'spec.type': ['service'] }, limit, offset: 0, orderFields, + totalItems: 'exclude', }); }); }); @@ -1033,7 +1141,6 @@ describe(``, () => { expect(result.current.backendEntities.length).toBeGreaterThan(0); }); expect(result.current.backendEntities.length).toBe(2); - expect(mockCatalogApi.queryEntities).toHaveBeenCalledTimes(1); act(() => { result.current.setOffset!(5); @@ -1041,11 +1148,12 @@ describe(``, () => { }); await waitFor(() => { - expect(mockCatalogApi.queryEntities).toHaveBeenNthCalledWith(2, { + expect(mockCatalogApi.queryEntities).toHaveBeenCalledWith({ filter: { kind: 'component' }, limit, offset: 10, orderFields, + totalItems: 'exclude', }); expect(result.current.offset).toEqual(10); }); @@ -1061,6 +1169,9 @@ describe(``, () => { }); expect(result.current.backendEntities.length).toBe(2); + // The count effect fires first (consuming one rejection), then the + // list call fires. Both must reject for the error to surface. + mockCatalogApi.queryEntities!.mockRejectedValueOnce('error'); mockCatalogApi.queryEntities!.mockRejectedValueOnce('error'); act(() => { result.current.updateFilters({ @@ -1128,6 +1239,7 @@ describe('versioned context', () => { updateFilters: jest.fn(), queryParameters: {}, loading: true, + totalItemsLoading: false, limit: 277, setLimit: jest.fn(), setOffset: jest.fn(), diff --git a/plugins/catalog-react/src/hooks/useEntityListProvider.tsx b/plugins/catalog-react/src/hooks/useEntityListProvider.tsx index db7b48822a..e42e391ee9 100644 --- a/plugins/catalog-react/src/hooks/useEntityListProvider.tsx +++ b/plugins/catalog-react/src/hooks/useEntityListProvider.tsx @@ -35,6 +35,7 @@ import { useState, } from 'react'; import { useLocation } from 'react-router-dom'; +import useAsyncFn from 'react-use/esm/useAsyncFn'; import useDebounce from 'react-use/esm/useDebounce'; import useMountedState from 'react-use/esm/useMountedState'; import { catalogApiRef } from '../api'; @@ -120,6 +121,7 @@ export type EntityListContextProps< prev?: () => void; }; totalItems?: number; + totalItemsLoading: boolean; limit: number; offset?: number; setLimit: (limit: number) => void; @@ -263,11 +265,11 @@ export const EntityListProvider = ( const response = await catalogApi.queryEntities({ cursor, limit, + totalItems: 'exclude', }); return { backendEntities: response.items, pageInfo: response.pageInfo, - totalItems: response.totalItems, }; }; } else { @@ -278,11 +280,11 @@ export const EntityListProvider = ( ...backendFilter, limit, offset, + totalItems: 'exclude', }); return { backendEntities: response.items, pageInfo: response.pageInfo, - totalItems: response.totalItems, }; }; } @@ -345,6 +347,33 @@ export const EntityListProvider = ( // several filters will be calling updateFilters in rapid succession. useDebounce(refresh, 10, [adjustedFilters, cursor, limit, offset]); + // Fetch the total count separately, only when filters change. This is + // decoupled from the main list fetch so that page navigation doesn't + // re-run the expensive count query, and so that the count can arrive + // asynchronously without blocking the list response. + const [{ value: totalItems, loading: totalItemsLoading }, refreshCount] = + useAsyncFn(async () => { + if (paginationMode === 'none') { + return undefined; + } + const compacted = compact(Object.values(adjustedFilters)); + if (compacted.length === 0) { + return undefined; + } + const backendFilter = reduceCatalogFilters(compacted); + try { + const response = await catalogApi.queryEntities({ + ...backendFilter, + limit: 0, + }); + return response.totalItems; + } catch { + return undefined; + } + }, [catalogApi, paginationMode, adjustedFilters]); + + useDebounce(refreshCount, 10, [adjustedFilters]); + // Frontend filtering — synchronous, no debounce needed. Updates // instantly when requestedFilters or backendEntities change. const entities = useMemo(() => { @@ -446,7 +475,10 @@ export const EntityListProvider = ( error, pageInfo, totalItems: - paginationMode === 'none' ? entities.length : backendState.totalItems, + paginationMode === 'none' + ? entities.length + : totalItems ?? backendState.totalItems, + totalItemsLoading: paginationMode !== 'none' && totalItemsLoading, limit, offset, setLimit, @@ -457,6 +489,8 @@ export const EntityListProvider = ( requestedFilters, entities, backendState, + totalItems, + totalItemsLoading, updateFilters, queryParameters, loading, diff --git a/plugins/catalog-react/src/testUtils/MockEntityListContextProvider.tsx b/plugins/catalog-react/src/testUtils/MockEntityListContextProvider.tsx index e75e7e20e3..c13821b815 100644 --- a/plugins/catalog-react/src/testUtils/MockEntityListContextProvider.tsx +++ b/plugins/catalog-react/src/testUtils/MockEntityListContextProvider.tsx @@ -73,6 +73,7 @@ export function MockEntityListContextProvider< error: value?.error, totalItems: value?.totalItems ?? (value?.entities ?? defaultValues.entities).length, + totalItemsLoading: value?.totalItemsLoading ?? false, limit: value?.limit ?? 20, offset: value?.offset, setLimit: value?.setLimit ?? (() => {}), diff --git a/plugins/catalog/src/components/CatalogTable/CatalogTable.test.tsx b/plugins/catalog/src/components/CatalogTable/CatalogTable.test.tsx index 0d81753653..df796ff321 100644 --- a/plugins/catalog/src/components/CatalogTable/CatalogTable.test.tsx +++ b/plugins/catalog/src/components/CatalogTable/CatalogTable.test.tsx @@ -126,7 +126,8 @@ describe('CatalogTable component', () => { }, }, ); - expect(screen.getByText(/Owned Components \(3\)/)).toBeInTheDocument(); + expect(screen.getByText(/Owned Components/)).toBeInTheDocument(); + expect(screen.getByText(/\(3\)/)).toBeInTheDocument(); expect(screen.getByText(/component1/)).toBeInTheDocument(); expect(screen.getByText(/component2/)).toBeInTheDocument(); expect(screen.getByText(/component3/)).toBeInTheDocument(); @@ -294,11 +295,15 @@ describe('CatalogTable component', () => { ])( 'should render correct columns with kind filter $kind', async ({ kind, expectedColumns }) => { + const kindEntities = entities.map(e => ({ + ...e, + kind: kind ?? e.kind, + })); await renderInTestApp( { filters, pageInfo, totalItems, + totalItemsLoading, paginationMode, } = entityListContext; - // For non-paginated tables, only show the full loading indicator when - // there's no data yet (initial load). During filter changes we keep stale - // data visible and let the new results swap in seamlessly. For paginated - // tables we always show loading, since stale data from a different page - // would be misleading. - const isLoading = - paginationMode === 'none' ? loading && entities.length === 0 : loading; + // Track whether we've ever received data. The full-table spinner should + // only show on the truly initial load — not when a filter change + // empties the client-side entity list before the backend responds. + const hasHadData = useRef(false); + if (entities.length > 0) { + hasHadData.current = true; + } + const isLoading = loading && !hasHadData.current; const tableColumns = useMemo( () => @@ -195,29 +197,46 @@ export const CatalogTable = (props: CatalogTableProps) => { }, ]; - const currentKind = filters.kind?.label || ''; + // Derive the title's kind label from the displayed entities so the + // title stays consistent with the rows during filter transitions. + // Use the filter's label when it matches (it has proper casing), + // otherwise fall back to the entity's kind field directly. + const displayedKind = entities[0]?.kind ?? filters.kind?.value; + const displayedKindLabel = + displayedKind?.toLocaleLowerCase('en-US') === + filters.kind?.value?.toLocaleLowerCase('en-US') + ? filters.kind?.label || '' + : displayedKind || ''; const currentType = filters.type?.value || ''; - const currentCount = typeof totalItems === 'number' ? `(${totalItems})` : ''; + // Show the count as long as we have one. Hide it only when new rows + // have arrived but the count hasn't caught up yet — at that point + // the old count would be wrong for the new data. + const countIsStale = !loading && totalItemsLoading; + const currentCount = + typeof totalItems === 'number' && !countIsStale ? ` (${totalItems})` : ''; + const somethingIsLoading = loading || totalItemsLoading; // TODO(timbonicus): remove the title from the CatalogTable once using EntitySearchBar const titlePreamble = capitalize( filters.user?.value ?? t('catalogTable.allFilters'), ); - const titleText = + const titleBase = props.title || - [titlePreamble, currentType, pluralize(currentKind), currentCount] + [titlePreamble, currentType, pluralize(displayedKindLabel)] .filter(s => s) .join(' '); - const title = - loading && !isLoading ? ( - - {titleText} + const title = props.title ? ( + titleBase + ) : ( + + {titleBase} + {currentCount} + {somethingIsLoading && !isLoading && ( - - ) : ( - titleText - ); + )} + + ); const actions = props.actions || defaultActions; const options: TableProps['options'] = { diff --git a/plugins/catalog/src/components/CatalogTable/defaultCatalogTableColumnsFunc.tsx b/plugins/catalog/src/components/CatalogTable/defaultCatalogTableColumnsFunc.tsx index 54dd701770..fe2f9d19e4 100644 --- a/plugins/catalog/src/components/CatalogTable/defaultCatalogTableColumnsFunc.tsx +++ b/plugins/catalog/src/components/CatalogTable/defaultCatalogTableColumnsFunc.tsx @@ -25,11 +25,17 @@ export const defaultCatalogTableColumnsFunc: CatalogTableColumnsFunc = ({ filters, entities, }) => { + // Derive the effective kind from the displayed entities so that both + // the column layout and the name column's defaultKind stay consistent + // with the rows during filter transitions (when stale rows are kept + // visible while new data loads). + const effectiveKind = + entities[0]?.kind?.toLocaleLowerCase('en-US') ?? filters.kind?.value; const showTypeColumn = filters.type === undefined; return [ columnFactories.createTitleColumn({ hidden: true }), - columnFactories.createNameColumn({ defaultKind: filters.kind?.value }), + columnFactories.createNameColumn({ defaultKind: effectiveKind }), ...createEntitySpecificColumns(), ]; @@ -44,7 +50,7 @@ export const defaultCatalogTableColumnsFunc: CatalogTableColumnsFunc = ({ columnFactories.createSpecTypeColumn({ hidden: !showTypeColumn }), columnFactories.createSpecLifecycleColumn(), ]; - switch (filters.kind?.value) { + switch (effectiveKind) { case 'user': return [...descriptionTagColumns]; case 'domain':