From 4078bd73bbf7a9df06c8f3b5a6a169686dff9546 Mon Sep 17 00:00:00 2001 From: Vincenzo Scamporlino Date: Fri, 20 Oct 2023 13:16:46 +0200 Subject: [PATCH] catalog-react: fixes for react 18 Signed-off-by: Vincenzo Scamporlino --- .../UserListPicker/UserListPicker.test.tsx | 13 +++---- .../useAllEntitiesCount.test.tsx | 6 ++-- .../useOwnedEntitiesCount.test.tsx | 12 +++---- .../UserListPicker/useOwnedEntitiesCount.ts | 36 ++++++++++++++----- .../useStarredEntitiesCount.test.tsx | 29 ++++++++------- .../src/hooks/useStarredEntities.test.tsx | 3 +- .../src/hooks/useStarredEntity.test.tsx | 3 +- 7 files changed, 61 insertions(+), 41 deletions(-) diff --git a/plugins/catalog-react/src/components/UserListPicker/UserListPicker.test.tsx b/plugins/catalog-react/src/components/UserListPicker/UserListPicker.test.tsx index d7572c2ac3..ad56681d6c 100644 --- a/plugins/catalog-react/src/components/UserListPicker/UserListPicker.test.tsx +++ b/plugins/catalog-react/src/components/UserListPicker/UserListPicker.test.tsx @@ -142,6 +142,7 @@ describe('', () => { afterEach(() => { jest.resetAllMocks(); }); + it('renders filter groups', async () => { render( @@ -357,7 +358,7 @@ describe('', () => { }); describe('filter resetting', () => { - let updateFilters: jest.Mock; + const updateFilters = jest.fn(); const Picker = ({ ...props }: UserListPickerProps) => ( @@ -372,15 +373,9 @@ describe('', () => { ); - beforeEach(() => { - updateFilters = jest.fn(); - }); - describe(`when there are no owned entities matching the filter`, () => { it('does not reset the filter while entities are loading', async () => { - mockCatalogApi.queryEntities?.mockImplementation( - () => new Promise(() => {}), - ); + mockCatalogApi.queryEntities?.mockReturnValue(new Promise(() => {})); render(); @@ -388,7 +383,7 @@ describe('', () => { expect(mockCatalogApi.queryEntities).toHaveBeenCalled(), ); - await expect( + await expect(() => waitFor(() => expect(updateFilters).toHaveBeenCalled()), ).rejects.toThrow(); }); diff --git a/plugins/catalog-react/src/components/UserListPicker/useAllEntitiesCount.test.tsx b/plugins/catalog-react/src/components/UserListPicker/useAllEntitiesCount.test.tsx index fc2c0e65bd..dc3bc5d2bd 100644 --- a/plugins/catalog-react/src/components/UserListPicker/useAllEntitiesCount.test.tsx +++ b/plugins/catalog-react/src/components/UserListPicker/useAllEntitiesCount.test.tsx @@ -16,7 +16,7 @@ import React, { PropsWithChildren } from 'react'; import { CatalogApi } from '@backstage/catalog-client'; import { useAllEntitiesCount } from './useAllEntitiesCount'; -import { renderHook } from '@testing-library/react-hooks'; +import { renderHook, waitFor } from '@testing-library/react'; import { EntityListProvider, useEntityList } from '../../hooks'; import { catalogApiRef } from '../../api'; import { ApiRef } from '@backstage/core-plugin-api'; @@ -61,7 +61,7 @@ describe('useAllEntitiesCount', () => { return <>{props.children}; } - const { result, waitFor } = renderHook(() => useAllEntitiesCount(), { + const { result } = renderHook(() => useAllEntitiesCount(), { wrapper: ({ children }) => ( @@ -89,7 +89,7 @@ describe('useAllEntitiesCount', () => { pageInfo: {}, }); - const { result, waitFor } = renderHook(() => useAllEntitiesCount(), { + const { result } = renderHook(() => useAllEntitiesCount(), { wrapper: ({ children }) => ( {children} diff --git a/plugins/catalog-react/src/components/UserListPicker/useOwnedEntitiesCount.test.tsx b/plugins/catalog-react/src/components/UserListPicker/useOwnedEntitiesCount.test.tsx index 4ce3503394..1f6fbfd53d 100644 --- a/plugins/catalog-react/src/components/UserListPicker/useOwnedEntitiesCount.test.tsx +++ b/plugins/catalog-react/src/components/UserListPicker/useOwnedEntitiesCount.test.tsx @@ -15,7 +15,7 @@ */ import React, { PropsWithChildren } from 'react'; import { CatalogApi } from '@backstage/catalog-client'; -import { renderHook } from '@testing-library/react-hooks'; +import { renderHook, waitFor } from '@testing-library/react'; import { DefaultEntityFilters, EntityListProvider, @@ -82,7 +82,7 @@ describe('useOwnedEntitiesCount', () => { pageInfo: {}, }); - const { result, waitFor } = renderHook(() => useOwnedEntitiesCount(), { + const { result } = renderHook(() => useOwnedEntitiesCount(), { wrapper: createWrapperWithInitialFilters({}), }); @@ -110,7 +110,7 @@ describe('useOwnedEntitiesCount', () => { pageInfo: {}, }); - const { result, waitFor } = renderHook(() => useOwnedEntitiesCount(), { + const { result } = renderHook(() => useOwnedEntitiesCount(), { wrapper: createWrapperWithInitialFilters({ namespace: new EntityNamespaceFilter(['a-namespace']), }), @@ -139,14 +139,14 @@ describe('useOwnedEntitiesCount', () => { }); }); - it(`should return count 0 without invoking queryEntities if owners filter doesn't have claims on common with logged in user`, async () => { + it(`should return count 0 without invoking queryEntities if owners filter doesn't have claims in common with logged in user`, async () => { mockQueryEntities.mockResolvedValue({ items: [], totalItems: 10, pageInfo: {}, }); - const { result, waitFor } = renderHook(() => useOwnedEntitiesCount(), { + const { result } = renderHook(() => useOwnedEntitiesCount(), { wrapper: createWrapperWithInitialFilters({ namespace: new EntityNamespaceFilter(['a-namespace']), owners: new EntityOwnerFilter(['group:default/monsters']), @@ -177,7 +177,7 @@ describe('useOwnedEntitiesCount', () => { pageInfo: {}, }); - const { result, waitFor } = renderHook(() => useOwnedEntitiesCount(), { + const { result } = renderHook(() => useOwnedEntitiesCount(), { wrapper: createWrapperWithInitialFilters({ namespace: new EntityNamespaceFilter(['a-namespace']), owners: new EntityOwnerFilter([ diff --git a/plugins/catalog-react/src/components/UserListPicker/useOwnedEntitiesCount.ts b/plugins/catalog-react/src/components/UserListPicker/useOwnedEntitiesCount.ts index d86610d9ae..1adab49361 100644 --- a/plugins/catalog-react/src/components/UserListPicker/useOwnedEntitiesCount.ts +++ b/plugins/catalog-react/src/components/UserListPicker/useOwnedEntitiesCount.ts @@ -17,12 +17,13 @@ import { QueryEntitiesInitialRequest } from '@backstage/catalog-client'; import { identityApiRef, useApi } from '@backstage/core-plugin-api'; import { compact, intersection, isEqual } from 'lodash'; -import { useMemo, useRef } from 'react'; +import { useEffect, useMemo, useRef } from 'react'; import useAsync from 'react-use/lib/useAsync'; import { catalogApiRef } from '../../api'; import { EntityOwnerFilter, EntityUserFilter } from '../../filters'; import { useEntityList } from '../../hooks'; import { reduceCatalogFilters } from '../../utils'; +import useAsyncFn from 'react-use/lib/useAsyncFn'; export function useOwnedEntitiesCount() { const identityApi = useApi(identityApiRef); @@ -71,14 +72,33 @@ export function useOwnedEntitiesCount() { return newRequest; }, [filters, ownershipEntityRefs]); - const { value: count, loading: loadingEntityOwnership } = - useAsync(async () => { - if (!request) { - return 0; + const [{ value: count, loading: loadingEntityOwnership }, fetchEntities] = + useAsyncFn( + async ( + req: QueryEntitiesInitialRequest | undefined, + ownershipEntityRefsParam: string[], + ) => { + if (ownershipEntityRefsParam && !req) { + // this implicitly means that there aren't claims in common with + // the logged in users, so avoid invoking the queryEntities endpoint + // which will implicitly returns 0 + return 0; + } + const { totalItems } = await catalogApi.queryEntities(req); + return totalItems; + }, + [], + { loading: true }, + ); + + useEffect(() => { + if (ownershipEntityRefs) { + if (request && Object.keys(request).length === 0) { + return; } - const { totalItems } = await catalogApi.queryEntities(request); - return totalItems; - }, [request]); + fetchEntities(request, ownershipEntityRefs); + } + }, [fetchEntities, request, ownershipEntityRefs]); const loading = loadingEntityRefs || loadingEntityOwnership; const filter = useMemo( diff --git a/plugins/catalog-react/src/components/UserListPicker/useStarredEntitiesCount.test.tsx b/plugins/catalog-react/src/components/UserListPicker/useStarredEntitiesCount.test.tsx index 1a02f3996b..5fe648e1bc 100644 --- a/plugins/catalog-react/src/components/UserListPicker/useStarredEntitiesCount.test.tsx +++ b/plugins/catalog-react/src/components/UserListPicker/useStarredEntitiesCount.test.tsx @@ -20,7 +20,7 @@ import { catalogApiRef } from '../../api'; import { ApiRef } from '@backstage/core-plugin-api'; import { MemoryRouter } from 'react-router-dom'; import { useStarredEntitiesCount } from './useStarredEntitiesCount'; -import { renderHook } from '@testing-library/react-hooks'; +import { renderHook, waitFor } from '@testing-library/react'; const mockQueryEntities: jest.MockedFn = jest.fn(); const mockCatalogApi: jest.Mocked> = { @@ -75,7 +75,7 @@ describe('useStarredEntitiesCount', () => { pageInfo: {}, }); - const { result, waitFor } = renderHook(() => useStarredEntitiesCount(), { + const { result } = renderHook(() => useStarredEntitiesCount(), { wrapper: ({ children }) => ( {children} @@ -83,28 +83,31 @@ describe('useStarredEntitiesCount', () => { ), }); - await waitFor(() => + await waitFor(() => { expect(mockQueryEntities).toHaveBeenCalledWith({ filter: { 'metadata.name': ['favourite1', 'favourite2'], }, limit: 1000, - }), - ); - expect(result.current).toEqual({ - count: 2, - loading: false, - filter: { - refs: ['component:default/favourite1', 'component:default/favourite2'], - value: 'starred', - }, + }); + expect(result.current).toEqual({ + count: 2, + loading: false, + filter: { + refs: [ + 'component:default/favourite1', + 'component:default/favourite2', + ], + value: 'starred', + }, + }); }); }); it(`shouldn't invoke the endpoint if there are no starred entities`, async () => { mockStarredEntities.mockReturnValue(new Set()); - const { result, waitFor } = renderHook(() => useStarredEntitiesCount(), { + const { result } = renderHook(() => useStarredEntitiesCount(), { wrapper: ({ children }) => ( {children} diff --git a/plugins/catalog-react/src/hooks/useStarredEntities.test.tsx b/plugins/catalog-react/src/hooks/useStarredEntities.test.tsx index e3010a0f4a..07738fd3d9 100644 --- a/plugins/catalog-react/src/hooks/useStarredEntities.test.tsx +++ b/plugins/catalog-react/src/hooks/useStarredEntities.test.tsx @@ -16,7 +16,8 @@ import { Entity } from '@backstage/catalog-model'; import { TestApiProvider } from '@backstage/test-utils'; -import { act, renderHook, waitFor } from '@testing-library/react'; +import { act, renderHook } from '@testing-library/react'; +import { waitFor } from '@testing-library/react'; import React, { PropsWithChildren } from 'react'; import { starredEntitiesApiRef, diff --git a/plugins/catalog-react/src/hooks/useStarredEntity.test.tsx b/plugins/catalog-react/src/hooks/useStarredEntity.test.tsx index e64a9bbdf8..44a4c7aaa3 100644 --- a/plugins/catalog-react/src/hooks/useStarredEntity.test.tsx +++ b/plugins/catalog-react/src/hooks/useStarredEntity.test.tsx @@ -16,7 +16,8 @@ import { Entity, CompoundEntityRef } from '@backstage/catalog-model'; import { TestApiProvider } from '@backstage/test-utils'; -import { renderHook, waitFor } from '@testing-library/react'; +import { waitFor } from '@testing-library/react'; +import { renderHook } from '@testing-library/react'; import React, { PropsWithChildren } from 'react'; import Observable from 'zen-observable'; import { StarredEntitiesApi, starredEntitiesApiRef } from '../apis';