From 36123575ad64d91181e91eb712d152b0b971c577 Mon Sep 17 00:00:00 2001 From: Vincenzo Scamporlino Date: Thu, 25 May 2023 11:53:34 +0200 Subject: [PATCH 01/33] catalog-react: add reduceBackendCatalogFilters Signed-off-by: Vincenzo Scamporlino --- plugins/catalog-react/src/utils/filters.ts | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/plugins/catalog-react/src/utils/filters.ts b/plugins/catalog-react/src/utils/filters.ts index 109e864c52..5ab6e43593 100644 --- a/plugins/catalog-react/src/utils/filters.ts +++ b/plugins/catalog-react/src/utils/filters.ts @@ -16,6 +16,7 @@ import { Entity } from '@backstage/catalog-model'; import { EntityFilter } from '../types'; +import { EntityKindFilter, EntityTypeFilter } from '../filters'; export function reduceCatalogFilters( filters: EntityFilter[], @@ -28,6 +29,24 @@ export function reduceCatalogFilters( }, {} as Record); } +export function reduceBackendCatalogFilters(filters: EntityFilter[]) { + const backendCatalogFilters: Record< + string, + string | symbol | (string | symbol)[] + > = {}; + + filters.forEach(filter => { + if ( + filter instanceof EntityKindFilter || + filter instanceof EntityTypeFilter + ) { + Object.assign(backendCatalogFilters, filter.getCatalogFilters()); + } + }); + + return backendCatalogFilters; +} + export function reduceEntityFilters( filters: EntityFilter[], ): (entity: Entity) => boolean { From c9f2a54d71411a253fe2b4a665ed1d427d3defc2 Mon Sep 17 00:00:00 2001 From: Vincenzo Scamporlino Date: Thu, 25 May 2023 11:57:14 +0200 Subject: [PATCH 02/33] catalog-react: pick kind and type filters as backend filters Signed-off-by: Vincenzo Scamporlino --- .../catalog-react/src/hooks/useEntityListProvider.tsx | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/plugins/catalog-react/src/hooks/useEntityListProvider.tsx b/plugins/catalog-react/src/hooks/useEntityListProvider.tsx index b4c464a26c..b7ee8a94ed 100644 --- a/plugins/catalog-react/src/hooks/useEntityListProvider.tsx +++ b/plugins/catalog-react/src/hooks/useEntityListProvider.tsx @@ -41,16 +41,17 @@ import { EntityTypeFilter, UserListFilter, EntityNamespaceFilter, + UserOwnersFilter, } from '../filters'; import { EntityFilter } from '../types'; -import { reduceCatalogFilters, reduceEntityFilters } from '../utils'; +import { reduceBackendCatalogFilters, reduceEntityFilters } from '../utils'; import { useApi } from '@backstage/core-plugin-api'; /** @public */ export type DefaultEntityFilters = { kind?: EntityKindFilter; type?: EntityTypeFilter; - user?: UserListFilter; + user?: UserListFilter | UserOwnersFilter; owners?: EntityOwnerFilter; lifecycles?: EntityLifecycleFilter; tags?: EntityTagFilter; @@ -156,8 +157,8 @@ export const EntityListProvider = ( async () => { const compacted = compact(Object.values(requestedFilters)); const entityFilter = reduceEntityFilters(compacted); - const backendFilter = reduceCatalogFilters(compacted); - const previousBackendFilter = reduceCatalogFilters( + const backendFilter = reduceBackendCatalogFilters(compacted); + const previousBackendFilter = reduceBackendCatalogFilters( compact(Object.values(outputState.appliedFilters)), ); From ae2f2dd18d99c1b23ed25db769d9470e3c617b54 Mon Sep 17 00:00:00 2001 From: Vincenzo Scamporlino Date: Thu, 25 May 2023 11:59:47 +0200 Subject: [PATCH 03/33] catalog-react: add getCatalogFilters methods for backend filtering Signed-off-by: Vincenzo Scamporlino --- plugins/catalog-react/src/filters.ts | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/plugins/catalog-react/src/filters.ts b/plugins/catalog-react/src/filters.ts index c717a35202..1213ab9b7e 100644 --- a/plugins/catalog-react/src/filters.ts +++ b/plugins/catalog-react/src/filters.ts @@ -72,6 +72,10 @@ export class EntityTagFilter implements EntityFilter { return this.values.every(v => (entity.metadata.tags ?? []).includes(v)); } + getCatalogFilters(): Record { + return { 'metadata.tags': this.values }; + } + toQueryValue(): string[] { return this.values; } @@ -136,6 +140,10 @@ export class EntityOwnerFilter implements EntityFilter { }, [] as string[]); } + getCatalogFilters(): Record { + return { 'relations.ownedBy': this.values }; + } + filterEntity(entity: Entity): boolean { return this.values.some(v => getEntityRelations(entity, RELATION_OWNED_BY).some( @@ -160,6 +168,10 @@ export class EntityOwnerFilter implements EntityFilter { export class EntityLifecycleFilter implements EntityFilter { constructor(readonly values: string[]) {} + getCatalogFilters(): Record { + return { 'spec.lifecycle': this.values }; + } + filterEntity(entity: Entity): boolean { return this.values.some(v => entity.spec?.lifecycle === v); } @@ -176,6 +188,9 @@ export class EntityLifecycleFilter implements EntityFilter { export class EntityNamespaceFilter implements EntityFilter { constructor(readonly values: string[]) {} + getCatalogFilters(): Record { + return { 'spec.lifecycle': this.values }; + } filterEntity(entity: Entity): boolean { return this.values.some(v => entity.metadata.namespace === v); } @@ -218,6 +233,11 @@ export class UserListFilter implements EntityFilter { */ export class EntityOrphanFilter implements EntityFilter { constructor(readonly value: boolean) {} + + getCatalogFilters(): Record { + return { 'metadata.annotations.backstage.io/orphan': String(this.value) }; + } + filterEntity(entity: Entity): boolean { const orphan = entity.metadata.annotations?.['backstage.io/orphan']; return orphan !== undefined && this.value.toString() === orphan; @@ -230,6 +250,10 @@ export class EntityOrphanFilter implements EntityFilter { */ export class EntityErrorFilter implements EntityFilter { constructor(readonly value: boolean) {} + + // TODO(vinzscam): is it possible to implement + // getCatalogFilters? ask mammals + filterEntity(entity: Entity): boolean { const error = ((entity as AlphaEntity)?.status?.items?.length as number) > 0; From 8bae84e5f47597a1ebcb1c783b00aa91aa6afb19 Mon Sep 17 00:00:00 2001 From: Vincenzo Scamporlino Date: Thu, 25 May 2023 12:00:07 +0200 Subject: [PATCH 04/33] catalog-react: introduce UserOwnersFilter Signed-off-by: Vincenzo Scamporlino --- plugins/catalog-react/src/filters.ts | 46 ++++++++++++++++++++++++++++ 1 file changed, 46 insertions(+) diff --git a/plugins/catalog-react/src/filters.ts b/plugins/catalog-react/src/filters.ts index 1213ab9b7e..a5a39f9b02 100644 --- a/plugins/catalog-react/src/filters.ts +++ b/plugins/catalog-react/src/filters.ts @@ -200,8 +200,54 @@ export class EntityNamespaceFilter implements EntityFilter { } } +/** + * @public + */ +export class UserOwnersFilter implements EntityFilter { + private constructor( + readonly value: UserListFilterKind, + readonly refs?: string[], + ) {} + + static owned(ownershipEntityRefs: string[]) { + return new UserOwnersFilter('owned', ownershipEntityRefs); + } + + static all() { + return new UserOwnersFilter('all'); + } + + static starred(starredEntityRefs: string[]) { + return new UserOwnersFilter('starred', starredEntityRefs); + } + + getCatalogFilters(): Record { + if (this.value === 'owned') { + return { 'relations.ownedBy': this.refs ?? [] }; + } + if (this.value === 'starred') { + return { + 'metadata.name': this.refs?.map(e => parseEntityRef(e).name) ?? [], + }; + } + return {}; + } + + filterEntity(entity: Entity) { + if (this.value === 'starred') { + return this.refs?.includes(stringifyEntityRef(entity)) ?? true; + } + return true; + } + + toQueryValue(): string { + return this.value; + } +} + /** * Filters entities based on whatever the user has starred or owns them. + * @deprecated use UserOwnersFilter * @public */ export class UserListFilter implements EntityFilter { From 58b6d860d34653be8b968c3f60825d520e28f659 Mon Sep 17 00:00:00 2001 From: Vincenzo Scamporlino Date: Thu, 25 May 2023 12:00:59 +0200 Subject: [PATCH 05/33] catalog-react: add useIsOwnedEntity hook Signed-off-by: Vincenzo Scamporlino --- .../catalog-react/src/hooks/useEntityOwnership.ts | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/plugins/catalog-react/src/hooks/useEntityOwnership.ts b/plugins/catalog-react/src/hooks/useEntityOwnership.ts index f8a37a2c83..1f1e200634 100644 --- a/plugins/catalog-react/src/hooks/useEntityOwnership.ts +++ b/plugins/catalog-react/src/hooks/useEntityOwnership.ts @@ -46,9 +46,15 @@ export function useEntityOwnership(): { return ownershipEntityRefs; }, []); - const isOwnedEntity = useMemo(() => { + const isOwnedEntity = useIsOwnedEntity(refs); + + return useMemo(() => ({ loading, isOwnedEntity }), [loading, isOwnedEntity]); +} + +export function useIsOwnedEntity(refs?: string[]) { + return useMemo(() => { const myOwnerRefs = new Set(refs ?? []); - return (entity: Entity) => { + const isOwnedEntity = (entity: Entity) => { const entityOwnerRefs = getEntityRelations(entity, RELATION_OWNED_BY).map( stringifyEntityRef, ); @@ -59,7 +65,6 @@ export function useEntityOwnership(): { } return false; }; + return isOwnedEntity; }, [refs]); - - return useMemo(() => ({ loading, isOwnedEntity }), [loading, isOwnedEntity]); } From 1b31deed8f7d136993e180b86e27fee03183b4c1 Mon Sep 17 00:00:00 2001 From: Vincenzo Scamporlino Date: Thu, 25 May 2023 12:03:32 +0200 Subject: [PATCH 06/33] catalog-react: decouple UserListPicker from backendEntities Signed-off-by: Vincenzo Scamporlino --- plugins/catalog-react/api-report.md | 35 +- .../UserListPicker/UserListPicker.test.tsx | 695 +++++++++++++----- .../UserListPicker/UserListPicker.tsx | 132 ++-- .../UserListPicker/useAllEntitiesCount.ts | 61 ++ .../UserListPicker/useOwnedEntitiesCount.ts | 107 +++ .../UserListPicker/useStarredEntitiesCount.ts | 80 ++ 6 files changed, 879 insertions(+), 231 deletions(-) create mode 100644 plugins/catalog-react/src/components/UserListPicker/useAllEntitiesCount.ts create mode 100644 plugins/catalog-react/src/components/UserListPicker/useOwnedEntitiesCount.ts create mode 100644 plugins/catalog-react/src/components/UserListPicker/useStarredEntitiesCount.ts diff --git a/plugins/catalog-react/api-report.md b/plugins/catalog-react/api-report.md index a72df6bb3d..3faa5524c9 100644 --- a/plugins/catalog-react/api-report.md +++ b/plugins/catalog-react/api-report.md @@ -142,7 +142,7 @@ export const columnFactories: Readonly<{ export type DefaultEntityFilters = { kind?: EntityKindFilter; type?: EntityTypeFilter; - user?: UserListFilter; + user?: UserListFilter | UserOwnersFilter; owners?: EntityOwnerFilter; lifecycles?: EntityLifecycleFilter; tags?: EntityTagFilter; @@ -224,6 +224,8 @@ export class EntityLifecycleFilter implements EntityFilter { // (undocumented) filterEntity(entity: Entity): boolean; // (undocumented) + getCatalogFilters(): Record; + // (undocumented) toQueryValue(): string[]; // (undocumented) readonly values: string[]; @@ -275,6 +277,8 @@ export class EntityNamespaceFilter implements EntityFilter { // (undocumented) filterEntity(entity: Entity): boolean; // (undocumented) + getCatalogFilters(): Record; + // (undocumented) toQueryValue(): string[]; // (undocumented) readonly values: string[]; @@ -289,6 +293,8 @@ export class EntityOrphanFilter implements EntityFilter { // (undocumented) filterEntity(entity: Entity): boolean; // (undocumented) + getCatalogFilters(): Record; + // (undocumented) readonly value: boolean; } @@ -297,6 +303,8 @@ export class EntityOwnerFilter implements EntityFilter { constructor(values: string[]); // (undocumented) filterEntity(entity: Entity): boolean; + // (undocumented) + getCatalogFilters(): Record; toQueryValue(): string[]; // (undocumented) readonly values: string[]; @@ -447,6 +455,8 @@ export class EntityTagFilter implements EntityFilter { // (undocumented) filterEntity(entity: Entity): boolean; // (undocumented) + getCatalogFilters(): Record; + // (undocumented) toQueryValue(): string[]; // (undocumented) readonly values: string[]; @@ -620,7 +630,7 @@ export function useRelatedEntities( error: Error | undefined; }; -// @public +// @public @deprecated export class UserListFilter implements EntityFilter { constructor( value: UserListFilterKind, @@ -651,8 +661,29 @@ export const UserListPicker: ( export type UserListPickerProps = { initialFilter?: UserListFilterKind; availableFilters?: UserListFilterKind[]; + useServerSideFilters?: boolean; }; +// @public (undocumented) +export class UserOwnersFilter implements EntityFilter { + // (undocumented) + static all(): UserOwnersFilter; + // (undocumented) + filterEntity(entity: Entity): boolean; + // (undocumented) + getCatalogFilters(): Record; + // (undocumented) + static owned(ownershipEntityRefs: string[]): UserOwnersFilter; + // (undocumented) + readonly refs?: string[] | undefined; + // (undocumented) + static starred(starredEntityRefs: string[]): UserOwnersFilter; + // (undocumented) + toQueryValue(): string; + // (undocumented) + readonly value: UserListFilterKind; +} + // @public (undocumented) export function useStarredEntities(): { starredEntities: Set; diff --git a/plugins/catalog-react/src/components/UserListPicker/UserListPicker.test.tsx b/plugins/catalog-react/src/components/UserListPicker/UserListPicker.test.tsx index c31381df20..1315c95061 100644 --- a/plugins/catalog-react/src/components/UserListPicker/UserListPicker.test.tsx +++ b/plugins/catalog-react/src/components/UserListPicker/UserListPicker.test.tsx @@ -16,15 +16,18 @@ import React from 'react'; import { fireEvent, render, waitFor, screen } from '@testing-library/react'; -import { - Entity, - RELATION_OWNED_BY, - UserEntity, -} from '@backstage/catalog-model'; -import { UserListPicker } from './UserListPicker'; +import { Entity, UserEntity } from '@backstage/catalog-model'; +import { UserListPicker, UserListPickerProps } from './UserListPicker'; import { MockEntityListContextProvider } from '../../testUtils/providers'; -import { EntityTagFilter, UserListFilter } from '../../filters'; -import { CatalogApi } from '@backstage/catalog-client'; +import { + EntityTagFilter, + UserListFilter, + UserOwnersFilter, +} from '../../filters'; +import { + CatalogApi, + QueryEntitiesInitialRequest, +} from '@backstage/catalog-client'; import { catalogApiRef } from '../../api'; import { MockStorageApi, TestApiRegistry } from '@backstage/test-utils'; import { ApiProvider } from '@backstage/core-app-api'; @@ -35,7 +38,7 @@ import { identityApiRef, storageApiRef, } from '@backstage/core-plugin-api'; -import { useEntityOwnership } from '../../hooks'; +import { MockStarredEntitiesApi, starredEntitiesApiRef } from '../../apis'; const mockUser: UserEntity = { apiVersion: 'backstage.io/v1alpha1', @@ -54,19 +57,22 @@ const mockConfigApi = { } as Partial; const mockCatalogApi = { - getEntityByRef: () => Promise.resolve(mockUser), -} as Partial; + getEntityByRef: jest.fn(), + queryEntities: jest.fn(), +} as Partial>; const mockIdentityApi = { - getUserId: () => 'testUser', - getIdToken: async () => undefined, -} as Partial; + getBackstageIdentity: jest.fn(), +} as Partial>; + +const mockStarredEntitiesApi = new MockStarredEntitiesApi(); const apis = TestApiRegistry.from( [configApiRef, mockConfigApi], [catalogApiRef, mockCatalogApi], [identityApiRef, mockIdentityApi], [storageApiRef, MockStorageApi.create()], + [starredEntitiesApiRef, mockStarredEntitiesApi], ); const mockIsOwnedEntity = jest.fn( @@ -77,113 +83,117 @@ const mockIsStarredEntity = jest.fn( (entity: Entity) => entity.metadata.name === 'component-3', ); -jest.mock('../../hooks', () => { - const actual = jest.requireActual('../../hooks'); - return { - ...actual, - useEntityOwnership: jest.fn(() => ({ - isOwnedEntity: mockIsOwnedEntity, - })), - useStarredEntities: () => ({ - isStarredEntity: mockIsStarredEntity, - }), - }; -}); - -const backendEntities: Entity[] = [ - { - apiVersion: '1', - kind: 'Component', - metadata: { - namespace: 'namespace-1', - name: 'component-1', - tags: ['tag1'], - }, - relations: [ - { - type: RELATION_OWNED_BY, - targetRef: 'user:default/testuser', - }, - ], - }, - { - apiVersion: '1', - kind: 'Component', - metadata: { - namespace: 'namespace-2', - name: 'component-2', - tags: ['tag1'], - }, - }, - { - apiVersion: '1', - kind: 'Component', - metadata: { - namespace: 'namespace-2', - name: 'component-3', - tags: [], - }, - }, - { - apiVersion: '1', - kind: 'Component', - metadata: { - namespace: 'namespace-2', - name: 'component-4', - tags: [], - }, - relations: [ - { - type: RELATION_OWNED_BY, - targetRef: 'user:default/testuser', - }, - ], - }, -]; - +const ownershipEntityRefs = ['user:default/testuser']; describe('', () => { - it('renders filter groups', () => { + const mockQueryEntitiesImplementation: CatalogApi['queryEntities'] = + async request => { + if ( + ( + (request as QueryEntitiesInitialRequest).filter as Record< + string, + string + > + )['relations.ownedBy'] + ) { + // owned entities + return { items: [], totalItems: 3, pageInfo: {} }; + } + if ( + ( + (request as QueryEntitiesInitialRequest).filter as Record< + string, + string + > + )['metadata.name'] + ) { + // starred entities + return { + items: [ + { + apiVersion: '1', + kind: 'component', + metadata: { name: 'e-1', namespace: 'default' }, + }, + { + apiVersion: '1', + kind: 'component', + metadata: { name: 'e-2', namespace: 'default' }, + }, + ], + totalItems: 2, + pageInfo: {}, + }; + } + // all items + return { items: [], totalItems: 10, pageInfo: {} }; + }; + + beforeAll(() => { + mockStarredEntitiesApi.toggleStarred('component:default/e-1'); + mockStarredEntitiesApi.toggleStarred('component:default/e-2'); + }); + + beforeEach(() => { + mockCatalogApi.getEntityByRef?.mockResolvedValue(mockUser); + mockIdentityApi.getBackstageIdentity?.mockResolvedValue({ + ownershipEntityRefs, + type: 'user', + userEntityRef: 'user:default/testuser', + }); + + mockCatalogApi.queryEntities?.mockImplementation( + mockQueryEntitiesImplementation, + ); + }); + + afterEach(() => { + jest.resetAllMocks(); + }); + it('renders filter groups', async () => { render( - + , ); + await waitFor(() => + expect(mockIdentityApi.getBackstageIdentity).toHaveBeenCalled(), + ); + await waitFor(() => + expect(mockCatalogApi.queryEntities).toHaveBeenCalled(), + ); expect(screen.getByText('Personal')).toBeInTheDocument(); expect(screen.getByText('Test Company')).toBeInTheDocument(); }); - it('renders filters', () => { + it('renders filters', async () => { render( - + , ); - expect( - screen.getAllByRole('menuitem').map(({ textContent }) => textContent), - ).toEqual(['Owned 1', 'Starred 1', 'All 4']); - }); - - it('includes counts alongside each filter', async () => { - render( - - - - - , - ); - - // Material UI renders ListItemSecondaryActions outside the - // menuitem itself, so we pick off the next sibling. - await waitFor(() => { + await waitFor(() => expect( screen.getAllByRole('menuitem').map(({ textContent }) => textContent), - ).toEqual(['Owned 1', 'Starred 1', 'All 4']); + ).toEqual(['Owned 3', 'Starred 2', 'All 10']), + ); + + expect(mockCatalogApi.queryEntities).toHaveBeenCalledWith({ + filter: {}, + limit: 0, + }); + expect(mockCatalogApi.queryEntities).toHaveBeenCalledWith({ + filter: { 'metadata.name': ['e-1', 'e-2'] }, + limit: 1000, + }); + expect(mockCatalogApi.queryEntities).toHaveBeenCalledWith({ + filter: { 'relations.ownedBy': ['user:default/testuser'] }, + limit: 0, }); }); @@ -192,7 +202,6 @@ describe('', () => { @@ -204,35 +213,104 @@ describe('', () => { await waitFor(() => { expect( screen.getAllByRole('menuitem').map(({ textContent }) => textContent), - ).toEqual(['Owned 1', 'Starred 0', 'All 2']); + ).toEqual(['Owned 3', 'Starred 2', 'All 10']); + }); + + expect(mockCatalogApi.queryEntities).toHaveBeenCalledWith({ + filter: { 'metadata.tags': ['tag1'] }, + limit: 0, + }); + expect(mockCatalogApi.queryEntities).toHaveBeenCalledWith({ + filter: { 'metadata.name': ['e-1', 'e-2'], 'metadata.tags': ['tag1'] }, + limit: 1000, + }); + expect(mockCatalogApi.queryEntities).toHaveBeenCalledWith({ + filter: { + 'relations.ownedBy': ['user:default/testuser'], + 'metadata.tags': ['tag1'], + }, + limit: 0, }); }); - it('respects the query parameter filter value', () => { + it('respects the query parameter filter value, legacy', async () => { const updateFilters = jest.fn(); const queryParameters = { user: 'owned' }; render( , ); - expect(updateFilters).toHaveBeenLastCalledWith({ - user: new UserListFilter('owned', mockIsOwnedEntity, mockIsStarredEntity), + await waitFor(() => + expect(updateFilters).toHaveBeenLastCalledWith({ + user: new UserListFilter( + 'owned', + expect.any(Function), + expect.any(Function), + ), + }), + ); + expect(mockCatalogApi.queryEntities).toHaveBeenCalledWith({ + filter: {}, + limit: 0, + }); + expect(mockCatalogApi.queryEntities).toHaveBeenCalledWith({ + filter: { 'metadata.name': ['e-1', 'e-2'] }, + limit: 1000, + }); + expect(mockCatalogApi.queryEntities).toHaveBeenCalledWith({ + filter: { + 'relations.ownedBy': ['user:default/testuser'], + }, + limit: 0, }); }); - it('updates user filter when a menuitem is selected', () => { + it('respects the query parameter filter value', async () => { const updateFilters = jest.fn(); + const queryParameters = { user: 'owned' }; render( + + + , + ); + + await waitFor(() => + expect(updateFilters).toHaveBeenLastCalledWith({ + user: UserOwnersFilter.owned(ownershipEntityRefs), + }), + ); + + expect(mockCatalogApi.queryEntities).toHaveBeenCalledWith({ + filter: {}, + limit: 0, + }); + expect(mockCatalogApi.queryEntities).toHaveBeenCalledWith({ + filter: { 'metadata.name': ['e-1', 'e-2'] }, + limit: 1000, + }); + expect(mockCatalogApi.queryEntities).toHaveBeenCalledWith({ + filter: { + 'relations.ownedBy': ['user:default/testuser'], + }, + limit: 0, + }); + }); + + it('updates user filter when a menuitem is selected, legacy', async () => { + const updateFilters = jest.fn(); + render( + + , @@ -240,22 +318,45 @@ describe('', () => { fireEvent.click(screen.getByText('Starred')); - expect(updateFilters).toHaveBeenLastCalledWith({ - user: new UserListFilter( - 'starred', - mockIsOwnedEntity, - mockIsStarredEntity, - ), - }); + await waitFor(() => + expect(updateFilters).toHaveBeenLastCalledWith({ + user: new UserListFilter( + 'starred', + expect.any(Function), + expect.any(Function), + ), + }), + ); }); - it('responds to external queryParameters changes', () => { + it('updates user filter when a menuitem is selected', async () => { + const updateFilters = jest.fn(); + render( + + + + + , + ); + + fireEvent.click(screen.getByText('Starred')); + + await waitFor(() => + expect(updateFilters).toHaveBeenLastCalledWith({ + user: UserOwnersFilter.starred([ + 'component:default/e-1', + 'component:default/e-2', + ]), + }), + ); + }); + + it('responds to external queryParameters changes, legacy', async () => { const updateFilters = jest.fn(); const rendered = render( ', () => { , ); - expect(updateFilters).toHaveBeenLastCalledWith({ - user: new UserListFilter('all', mockIsOwnedEntity, mockIsStarredEntity), - }); + + await waitFor(() => + expect(updateFilters).toHaveBeenLastCalledWith({ + user: new UserListFilter( + 'all', + expect.any(Function), + expect.any(Function), + ), + }), + ); + rendered.rerender( ', () => { , ); expect(updateFilters).toHaveBeenLastCalledWith({ - user: new UserListFilter('owned', mockIsOwnedEntity, mockIsStarredEntity), + user: new UserListFilter( + 'owned', + expect.any(Function), + expect.any(Function), + ), }); }); - describe.each` - type | filterFn - ${'owned'} | ${mockIsOwnedEntity} - ${'starred'} | ${mockIsStarredEntity} - `('filter resetting for $type entities', ({ type, filterFn }) => { - let updateFilters: jest.Mock; - - const picker = (props: { loading: boolean }) => ( + it('responds to external queryParameters changes', async () => { + const updateFilters = jest.fn(); + const rendered = render( - + + + , + ); + + await waitFor(() => + expect(updateFilters).toHaveBeenLastCalledWith({ + user: UserOwnersFilter.all(), + }), + ); + + rendered.rerender( + + + + + , + ); + expect(updateFilters).toHaveBeenLastCalledWith({ + user: UserOwnersFilter.owned(ownershipEntityRefs), + }); + }); + + describe('filter resetting', () => { + let updateFilters: jest.Mock; + + const Picker = (props: UserListPickerProps) => ( + + + ); @@ -306,57 +450,213 @@ describe('', () => { updateFilters = jest.fn(); }); - describe(`when there are no ${type} entities match the filter`, () => { - beforeEach(() => { - filterFn.mockReturnValue(false); + describe(`when there are no owned entities match the filter`, () => { + it('does not reset the filter while entities are loading', async () => { + mockCatalogApi.queryEntities?.mockImplementation( + () => new Promise(() => {}), + ); + + render(); + + await waitFor(() => + expect(mockCatalogApi.queryEntities).toHaveBeenCalled(), + ); + expect(updateFilters).not.toHaveBeenCalled(); }); - it('does not reset the filter while entities are loading', () => { - render(picker({ loading: true })); + it('does not reset the filter while owned entities are loading', async () => { + mockCatalogApi.queryEntities?.mockImplementation(request => { + if ( + ( + (request as QueryEntitiesInitialRequest).filter as Record< + string, + string + > + )['relations.ownedBy'] + ) { + return new Promise(() => {}); + } + return mockQueryEntitiesImplementation(request); + }); + render(); + + await waitFor(() => + expect(mockCatalogApi.queryEntities).toHaveBeenCalledTimes(3), + ); expect(updateFilters).not.toHaveBeenCalledWith({ - user: new UserListFilter( - 'all', - mockIsOwnedEntity, - mockIsStarredEntity, - ), + user: expect.any(Object), }); }); - it('does not reset the filter while owned entities are loading', () => { - const isOwnedEntity = jest.fn(() => false); - (useEntityOwnership as jest.Mock).mockReturnValueOnce({ - loading: true, - isOwnedEntity, + it('resets the filter to "all" when entities are loaded, legacy', async () => { + mockCatalogApi.queryEntities?.mockImplementation(async request => { + if ( + ( + (request as QueryEntitiesInitialRequest).filter as Record< + string, + string + > + )['relations.ownedBy'] + ) { + return { items: [], totalItems: 0, pageInfo: {} }; + } + return mockQueryEntitiesImplementation(request); }); - render(picker({ loading: false })); - expect(updateFilters).not.toHaveBeenCalledWith({ - user: new UserListFilter('all', isOwnedEntity, mockIsStarredEntity), - }); + render(); + + await waitFor(() => + expect(updateFilters).toHaveBeenLastCalledWith({ + user: new UserListFilter( + 'all', + expect.any(Function), + expect.any(Function), + ), + }), + ); }); - it('resets the filter to "all" when entities are loaded', () => { - render(picker({ loading: false })); - - expect(updateFilters).toHaveBeenLastCalledWith({ - user: new UserListFilter( - 'all', - mockIsOwnedEntity, - mockIsStarredEntity, - ), + it('resets the filter to "all" when entities are loaded', async () => { + mockCatalogApi.queryEntities?.mockImplementation(async request => { + if ( + ( + (request as QueryEntitiesInitialRequest).filter as Record< + string, + string + > + )['relations.ownedBy'] + ) { + return { items: [], totalItems: 0, pageInfo: {} }; + } + return mockQueryEntitiesImplementation(request); }); + + render(); + + await waitFor(() => + expect(updateFilters).toHaveBeenLastCalledWith({ + user: UserOwnersFilter.all(), + }), + ); }); }); - describe(`when there are some ${type} entities present`, () => { - beforeEach(() => { - filterFn.mockReturnValue(true); + describe(`when there are no starred entities match the filter`, () => { + it('does not reset the filter while entities are loading', async () => { + mockCatalogApi.queryEntities?.mockImplementation( + () => new Promise(() => {}), + ); + + render(); + + await waitFor(() => + expect(mockCatalogApi.queryEntities).toHaveBeenCalled(), + ); + expect(updateFilters).not.toHaveBeenCalled(); }); - it('does not reset the filter while entities are loading', () => { - render(picker({ loading: true })); + it('does not reset the filter while starred entities are loading', async () => { + mockCatalogApi.queryEntities?.mockImplementation(request => { + if ( + ( + (request as QueryEntitiesInitialRequest).filter as Record< + string, + string + > + )['metadata.name'] + ) { + return new Promise(() => {}); + } + return mockQueryEntitiesImplementation(request); + }); + render(); + + await waitFor(() => + expect(mockCatalogApi.queryEntities).toHaveBeenCalledTimes(3), + ); + expect(updateFilters).not.toHaveBeenCalledWith({ + user: expect.any(Object), + }); + }); + + it('resets the filter to "all" when entities are loaded, legacy', async () => { + mockCatalogApi.queryEntities?.mockImplementation(async request => { + if ( + ( + (request as QueryEntitiesInitialRequest).filter as Record< + string, + string + > + )['metadata.name'] + ) { + return { items: [], totalItems: 0, pageInfo: {} }; + } + return mockQueryEntitiesImplementation(request); + }); + + render(); + + await waitFor(() => + expect(updateFilters).toHaveBeenLastCalledWith({ + user: new UserListFilter( + 'all', + expect.any(Function), + expect.any(Function), + ), + }), + ); + }); + + it('resets the filter to "all" when entities are loaded', async () => { + mockCatalogApi.queryEntities?.mockImplementation(async request => { + if ( + ( + (request as QueryEntitiesInitialRequest).filter as Record< + string, + string + > + )['metadata.name'] + ) { + return { items: [], totalItems: 0, pageInfo: {} }; + } + return mockQueryEntitiesImplementation(request); + }); + + render(); + + await waitFor(() => + expect(updateFilters).toHaveBeenLastCalledWith({ + user: UserOwnersFilter.all(), + }), + ); + }); + }); + + describe(`when there are some owned entities present`, () => { + it('does not reset the filter while entities are loading', async () => { + mockCatalogApi.queryEntities?.mockImplementation(request => { + if ( + ( + (request as QueryEntitiesInitialRequest).filter as Record< + string, + string + > + )['relations.ownedBy'] + ) { + return new Promise(() => {}); + } + return mockQueryEntitiesImplementation(request); + }); + + render( + , + ); /* picker({ loading: true })*/ + + await waitFor(() => + expect(mockCatalogApi.queryEntities).toHaveBeenCalledTimes(3), + ); expect(updateFilters).not.toHaveBeenCalledWith({ user: new UserListFilter( 'all', @@ -366,17 +666,74 @@ describe('', () => { }); }); - it('does not reset the filter when entities are loaded', () => { - render(picker({ loading: false })); + it('does not reset the filter when entities are loaded', async () => { + render(); - expect(updateFilters).toHaveBeenLastCalledWith({ + await waitFor(() => + expect(mockCatalogApi.queryEntities).toHaveBeenCalledTimes(3), + ); + + await waitFor(() => + expect(updateFilters).toHaveBeenLastCalledWith({ + user: new UserListFilter( + 'owned', + expect.any(Function), + expect.any(Function), + ), + }), + ); + }); + }); + + describe(`when there are some starred entities present`, () => { + it('does not reset the filter while entities are loading', async () => { + mockCatalogApi.queryEntities?.mockImplementation(request => { + if ( + ( + (request as QueryEntitiesInitialRequest).filter as Record< + string, + string + > + )['metadata.name'] + ) { + return new Promise(() => {}); + } + return mockQueryEntitiesImplementation(request); + }); + + render( + , + ); /* picker({ loading: true })*/ + + await waitFor(() => + expect(mockCatalogApi.queryEntities).toHaveBeenCalledTimes(3), + ); + expect(updateFilters).not.toHaveBeenCalledWith({ user: new UserListFilter( - type, + 'all', mockIsOwnedEntity, mockIsStarredEntity, ), }); }); + + it('does not reset the filter when entities are loaded', async () => { + render(); + + await waitFor(() => + expect(mockCatalogApi.queryEntities).toHaveBeenCalledTimes(3), + ); + + await waitFor(() => + expect(updateFilters).toHaveBeenLastCalledWith({ + user: new UserListFilter( + 'starred', + expect.any(Function), + expect.any(Function), + ), + }), + ); + }); }); }); }); diff --git a/plugins/catalog-react/src/components/UserListPicker/UserListPicker.tsx b/plugins/catalog-react/src/components/UserListPicker/UserListPicker.tsx index e7c306e0d0..153cdfdb9e 100644 --- a/plugins/catalog-react/src/components/UserListPicker/UserListPicker.tsx +++ b/plugins/catalog-react/src/components/UserListPicker/UserListPicker.tsx @@ -32,16 +32,14 @@ import { } from '@material-ui/core'; import SettingsIcon from '@material-ui/icons/Settings'; import StarIcon from '@material-ui/icons/Star'; -import { compact } from 'lodash'; import React, { Fragment, useEffect, useMemo, useState } from 'react'; -import { UserListFilter } from '../../filters'; -import { - useEntityList, - useStarredEntities, - useEntityOwnership, -} from '../../hooks'; +import { UserListFilter, UserOwnersFilter } from '../../filters'; +import { useEntityList, useStarredEntities } from '../../hooks'; import { UserListFilterKind } from '../../types'; -import { reduceEntityFilters } from '../../utils'; +import { useOwnedEntitiesCount } from './useOwnedEntitiesCount'; +import { useAllEntitiesCount } from './useAllEntitiesCount'; +import { useStarredEntitiesCount } from './useStarredEntitiesCount'; +import { useIsOwnedEntity } from '../../hooks/useEntityOwnership'; /** @public */ export type CatalogReactUserListPickerClassKey = @@ -122,20 +120,19 @@ function getFilterGroups(orgName: string | undefined): ButtonGroup[] { export type UserListPickerProps = { initialFilter?: UserListFilterKind; availableFilters?: UserListFilterKind[]; + useServerSideFilters?: boolean; }; /** @public */ export const UserListPicker = (props: UserListPickerProps) => { - const { initialFilter, availableFilters } = props; + const { initialFilter, availableFilters, useServerSideFilters } = props; const classes = useStyles(); const configApi = useApi(configApiRef); const orgName = configApi.getOptionalString('organization.name') ?? 'Company'; const { filters, updateFilters, - backendEntities, queryParameters: { kind: kindParameter, user: userParameter }, - loading: loadingBackendEntities, } = useEntityList(); // Remove group items that aren't in availableFilters and exclude @@ -153,21 +150,18 @@ export const UserListPicker = (props: UserListPickerProps) => { })) .filter(({ items }) => !!items.length); - const { isStarredEntity } = useStarredEntities(); - const { isOwnedEntity, loading: loadingEntityOwnership } = - useEntityOwnership(); - - const loading = loadingBackendEntities || loadingEntityOwnership; - - // Static filters; used for generating counts of potentially unselected kinds - const ownedFilter = useMemo( - () => new UserListFilter('owned', isOwnedEntity, isStarredEntity), - [isOwnedEntity, isStarredEntity], - ); - const starredFilter = useMemo( - () => new UserListFilter('starred', isOwnedEntity, isStarredEntity), - [isOwnedEntity, isStarredEntity], - ); + const { + count: ownedEntitiesCount, + loading: loadingOwnedEntities, + filter: ownedEntitiesFilter, + ownershipEntityRefs, + } = useOwnedEntitiesCount(); + const { count: allCount } = useAllEntitiesCount(); + const { + count: starredEntitiesCount, + filter: starredEntitiesFilter, + loading: loadingStarredEntities, + } = useStarredEntitiesCount(); const queryParamUserFilter = useMemo( () => [userParameter].flat()[0], @@ -175,33 +169,19 @@ export const UserListPicker = (props: UserListPickerProps) => { ); const [selectedUserFilter, setSelectedUserFilter] = useState( - queryParamUserFilter ?? initialFilter, + (queryParamUserFilter as UserListFilterKind) ?? initialFilter, ); - // To show proper counts for each section, apply all other frontend filters _except_ the user - // filter that's controlled by this picker. - const entitiesWithoutUserFilter = useMemo( - () => - backendEntities.filter( - reduceEntityFilters( - compact(Object.values({ ...filters, user: undefined })), - ), - ), - [filters, backendEntities], - ); + const filterCounts = useMemo(() => { + return { + all: allCount, + starred: starredEntitiesCount, + owned: ownedEntitiesCount, + }; + }, [starredEntitiesCount, ownedEntitiesCount, allCount]); - const filterCounts = useMemo>( - () => ({ - all: entitiesWithoutUserFilter.length, - starred: entitiesWithoutUserFilter.filter(entity => - starredFilter.filterEntity(entity), - ).length, - owned: entitiesWithoutUserFilter.filter(entity => - ownedFilter.filterEntity(entity), - ).length, - }), - [entitiesWithoutUserFilter, starredFilter, ownedFilter], - ); + const { isStarredEntity } = useStarredEntities(); + const isOwnedEntity = useIsOwnedEntity(ownershipEntityRefs); // Set selected user filter on query parameter updates; this happens at initial page load and from // external updates to the page location. @@ -211,6 +191,8 @@ export const UserListPicker = (props: UserListPickerProps) => { } }, [queryParamUserFilter]); + const loading = loadingOwnedEntities || loadingStarredEntities; + useEffect(() => { if ( !loading && @@ -223,16 +205,46 @@ export const UserListPicker = (props: UserListPickerProps) => { }, [loading, filterCounts, selectedUserFilter, setSelectedUserFilter]); useEffect(() => { - updateFilters({ - user: selectedUserFilter - ? new UserListFilter( - selectedUserFilter as UserListFilterKind, - isOwnedEntity, - isStarredEntity, - ) - : undefined, - }); - }, [selectedUserFilter, isOwnedEntity, isStarredEntity, updateFilters]); + if (!selectedUserFilter) { + return; + } + if (loading) { + return; + } + if (useServerSideFilters) { + const getFilter = () => { + if (selectedUserFilter === 'owned') { + return ownedEntitiesFilter; + } + if (selectedUserFilter === 'starred') { + return starredEntitiesFilter; + } + return UserOwnersFilter.all(); + }; + + updateFilters({ user: getFilter() }); + } else { + // legacy + updateFilters({ + user: selectedUserFilter + ? new UserListFilter( + selectedUserFilter as UserListFilterKind, + isOwnedEntity, + isStarredEntity, + ) + : undefined, + }); + } + }, [ + selectedUserFilter, + starredEntitiesFilter, + ownedEntitiesFilter, + updateFilters, + useServerSideFilters, + isOwnedEntity, + isStarredEntity, + loading, + ]); return ( diff --git a/plugins/catalog-react/src/components/UserListPicker/useAllEntitiesCount.ts b/plugins/catalog-react/src/components/UserListPicker/useAllEntitiesCount.ts new file mode 100644 index 0000000000..499762ff87 --- /dev/null +++ b/plugins/catalog-react/src/components/UserListPicker/useAllEntitiesCount.ts @@ -0,0 +1,61 @@ +/* + * Copyright 2023 The Backstage Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +import { QueryEntitiesInitialRequest } from '@backstage/catalog-client'; +import { useApi } from '@backstage/core-plugin-api'; +import { compact, isEqual } from 'lodash'; +import { useMemo, useRef } from 'react'; +import useAsync from 'react-use/lib/useAsync'; +import { catalogApiRef } from '../../api'; +import { useEntityList } from '../../hooks'; +import { reduceCatalogFilters } from '../../utils'; + +/** + * TODO(vinzscam): we need to find a better way + * for retrieving this value. One possible way, could be to use + * the /entities endpoint: since this method is paginated, + * it should also return how many items matching the provided filters + * are in the catalog + */ +export function useAllEntitiesCount() { + const catalogApi = useApi(catalogApiRef); + const { filters } = useEntityList(); + + const refRequest = useRef(); + useMemo(() => { + const { user, ...allFilters } = filters; + const compacted = compact(Object.values(allFilters)); + const filter = reduceCatalogFilters(compacted); + const request: QueryEntitiesInitialRequest = { + filter, + limit: 0, + }; + + if (isEqual(request, refRequest.current)) { + return refRequest.current; + } + refRequest.current = request; + + return request; + }, [filters]); + + const { value: count, loading } = useAsync(async () => { + const { totalItems } = await catalogApi.queryEntities(refRequest.current); + + return totalItems; + }, [refRequest.current]); + + return { count, loading }; +} diff --git a/plugins/catalog-react/src/components/UserListPicker/useOwnedEntitiesCount.ts b/plugins/catalog-react/src/components/UserListPicker/useOwnedEntitiesCount.ts new file mode 100644 index 0000000000..32e367cc07 --- /dev/null +++ b/plugins/catalog-react/src/components/UserListPicker/useOwnedEntitiesCount.ts @@ -0,0 +1,107 @@ +/* + * Copyright 2023 The Backstage Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +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 useAsync from 'react-use/lib/useAsync'; +import { catalogApiRef } from '../../api'; +import { UserOwnersFilter } from '../../filters'; +import { useEntityList } from '../../hooks'; +import { reduceCatalogFilters } from '../../utils'; + +export function useOwnedEntitiesCount() { + const identityApi = useApi(identityApiRef); + const catalogApi = useApi(catalogApiRef); + + const { filters } = useEntityList(); + // Trigger load only on mount + const { value: ownershipEntityRefs, loading: loadingEntityRefs } = useAsync( + async () => (await identityApi.getBackstageIdentity()).ownershipEntityRefs, + + [], + ); + + const refRequest = useRef(); + + useMemo(async () => { + const compacted = compact(Object.values(filters)); + const allFilter = reduceCatalogFilters(compacted); + const { ['metadata.name']: metadata, ...filter } = allFilter; + + const facet = 'relations.ownedBy'; + + const ownedByFilter = Array.isArray(filter[facet]) + ? (filter[facet] as string[]) + : []; + + const commonOwnedBy = intersection(ownedByFilter, ownershipEntityRefs); + + const ownedBy = + ownedByFilter.length > 0 ? ownedByFilter : ownershipEntityRefs; + if (ownedByFilter.length > 0 && commonOwnedBy.length === 0) { + // don't send any request if another filter sets + // totally different values for relations.ownedBy filter. + // TODO(vinzscam): check conflicts between UserOwnersFilter and EntityOwnerFilter. + // both set filters on the same relations.ownedBy key, so the conflicts need + // to be addressed properly. + refRequest.current = undefined; + return null; + } + const request: QueryEntitiesInitialRequest = { + filter: { + ...filter, + 'relations.ownedBy': ownedBy ?? [], + }, + limit: 0, + }; + + if (isEqual(request, refRequest.current)) { + return refRequest.current; + } + + refRequest.current = request; + + return request; + }, [filters, ownershipEntityRefs]); + + const { value: count, loading: loadingEntityOwnership } = + useAsync(async () => { + if (!ownershipEntityRefs?.length) { + return 0; + } + if (!refRequest.current) { + return 0; + } + const { totalItems } = await catalogApi.queryEntities(refRequest.current); + + return totalItems; + }, [refRequest.current]); + + const loading = loadingEntityRefs || loadingEntityOwnership; + const filter = useMemo( + () => UserOwnersFilter.owned(ownershipEntityRefs ?? []), + [ownershipEntityRefs], + ); + + return { + count, + loading, + filter, + ownershipEntityRefs, + }; +} diff --git a/plugins/catalog-react/src/components/UserListPicker/useStarredEntitiesCount.ts b/plugins/catalog-react/src/components/UserListPicker/useStarredEntitiesCount.ts new file mode 100644 index 0000000000..ea7423efe8 --- /dev/null +++ b/plugins/catalog-react/src/components/UserListPicker/useStarredEntitiesCount.ts @@ -0,0 +1,80 @@ +/* + * Copyright 2023 The Backstage Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { QueryEntitiesInitialRequest } from '@backstage/catalog-client'; +import { parseEntityRef, stringifyEntityRef } from '@backstage/catalog-model'; +import { useApi } from '@backstage/core-plugin-api'; +import { compact, isEqual } from 'lodash'; +import { useMemo, useRef } from 'react'; +import useAsync from 'react-use/lib/useAsync'; +import { catalogApiRef } from '../../api'; +import { UserOwnersFilter } from '../../filters'; +import { useEntityList, useStarredEntities } from '../../hooks'; +import { reduceCatalogFilters } from '../../utils'; + +export function useStarredEntitiesCount() { + const catalogApi = useApi(catalogApiRef); + const { filters } = useEntityList(); + const { starredEntities } = useStarredEntities(); + + const refRequest = useRef(); + useMemo(async () => { + const { user, ...allFilters } = filters; + const compacted = compact(Object.values(allFilters)); + const filter = reduceCatalogFilters(compacted); + + const facet = 'metadata.name'; + + const request: QueryEntitiesInitialRequest = { + filter: { + ...filter, + [facet]: Array.from(starredEntities).map(e => parseEntityRef(e).name), + }, + limit: 1000, + }; + if (isEqual(request, refRequest.current)) { + return refRequest.current; + } + refRequest.current = request; + + return request; + }, [filters, starredEntities]); + + const { value: count, loading } = useAsync(async () => { + if (!starredEntities.size) { + return 0; + } + + const response = await catalogApi.queryEntities(refRequest.current); + + return response.items + .map(e => + stringifyEntityRef({ + kind: e.kind, + namespace: e.metadata.namespace, + name: e.metadata.name, + }), + ) + .filter(e => starredEntities.has(e)).length; + }, [refRequest.current, starredEntities]); + + const filter = useMemo( + () => UserOwnersFilter.starred(Array.from(starredEntities)), + [starredEntities], + ); + + return { count, loading, filter }; +} From eb81d1673d35f9de06d60ae0cb57cbf422bbb140 Mon Sep 17 00:00:00 2001 From: Vincenzo Scamporlino Date: Fri, 26 May 2023 14:04:07 +0200 Subject: [PATCH 07/33] catalog-react: rename filter to EntityUserListFilter Signed-off-by: Vincenzo Scamporlino --- .../UserListPicker/UserListPicker.test.tsx | 14 +++++++------- .../components/UserListPicker/UserListPicker.tsx | 4 ++-- .../UserListPicker/useOwnedEntitiesCount.ts | 4 ++-- .../UserListPicker/useStarredEntitiesCount.ts | 4 ++-- plugins/catalog-react/src/filters.ts | 10 +++++----- .../src/hooks/useEntityListProvider.tsx | 4 ++-- 6 files changed, 20 insertions(+), 20 deletions(-) diff --git a/plugins/catalog-react/src/components/UserListPicker/UserListPicker.test.tsx b/plugins/catalog-react/src/components/UserListPicker/UserListPicker.test.tsx index 1315c95061..20dedf23a6 100644 --- a/plugins/catalog-react/src/components/UserListPicker/UserListPicker.test.tsx +++ b/plugins/catalog-react/src/components/UserListPicker/UserListPicker.test.tsx @@ -22,7 +22,7 @@ import { MockEntityListContextProvider } from '../../testUtils/providers'; import { EntityTagFilter, UserListFilter, - UserOwnersFilter, + EntityUserListFilter, } from '../../filters'; import { CatalogApi, @@ -286,7 +286,7 @@ describe('', () => { await waitFor(() => expect(updateFilters).toHaveBeenLastCalledWith({ - user: UserOwnersFilter.owned(ownershipEntityRefs), + user: EntityUserListFilter.owned(ownershipEntityRefs), }), ); @@ -343,7 +343,7 @@ describe('', () => { await waitFor(() => expect(updateFilters).toHaveBeenLastCalledWith({ - user: UserOwnersFilter.starred([ + user: EntityUserListFilter.starred([ 'component:default/e-1', 'component:default/e-2', ]), @@ -414,7 +414,7 @@ describe('', () => { await waitFor(() => expect(updateFilters).toHaveBeenLastCalledWith({ - user: UserOwnersFilter.all(), + user: EntityUserListFilter.all(), }), ); @@ -431,7 +431,7 @@ describe('', () => { , ); expect(updateFilters).toHaveBeenLastCalledWith({ - user: UserOwnersFilter.owned(ownershipEntityRefs), + user: EntityUserListFilter.owned(ownershipEntityRefs), }); }); @@ -536,7 +536,7 @@ describe('', () => { await waitFor(() => expect(updateFilters).toHaveBeenLastCalledWith({ - user: UserOwnersFilter.all(), + user: EntityUserListFilter.all(), }), ); }); @@ -628,7 +628,7 @@ describe('', () => { await waitFor(() => expect(updateFilters).toHaveBeenLastCalledWith({ - user: UserOwnersFilter.all(), + user: EntityUserListFilter.all(), }), ); }); diff --git a/plugins/catalog-react/src/components/UserListPicker/UserListPicker.tsx b/plugins/catalog-react/src/components/UserListPicker/UserListPicker.tsx index 153cdfdb9e..dff4f995ae 100644 --- a/plugins/catalog-react/src/components/UserListPicker/UserListPicker.tsx +++ b/plugins/catalog-react/src/components/UserListPicker/UserListPicker.tsx @@ -33,7 +33,7 @@ import { import SettingsIcon from '@material-ui/icons/Settings'; import StarIcon from '@material-ui/icons/Star'; import React, { Fragment, useEffect, useMemo, useState } from 'react'; -import { UserListFilter, UserOwnersFilter } from '../../filters'; +import { UserListFilter, EntityUserListFilter } from '../../filters'; import { useEntityList, useStarredEntities } from '../../hooks'; import { UserListFilterKind } from '../../types'; import { useOwnedEntitiesCount } from './useOwnedEntitiesCount'; @@ -219,7 +219,7 @@ export const UserListPicker = (props: UserListPickerProps) => { if (selectedUserFilter === 'starred') { return starredEntitiesFilter; } - return UserOwnersFilter.all(); + return EntityUserListFilter.all(); }; updateFilters({ user: getFilter() }); diff --git a/plugins/catalog-react/src/components/UserListPicker/useOwnedEntitiesCount.ts b/plugins/catalog-react/src/components/UserListPicker/useOwnedEntitiesCount.ts index 32e367cc07..f7fb9d4516 100644 --- a/plugins/catalog-react/src/components/UserListPicker/useOwnedEntitiesCount.ts +++ b/plugins/catalog-react/src/components/UserListPicker/useOwnedEntitiesCount.ts @@ -20,7 +20,7 @@ import { compact, intersection, isEqual } from 'lodash'; import { useMemo, useRef } from 'react'; import useAsync from 'react-use/lib/useAsync'; import { catalogApiRef } from '../../api'; -import { UserOwnersFilter } from '../../filters'; +import { EntityUserListFilter } from '../../filters'; import { useEntityList } from '../../hooks'; import { reduceCatalogFilters } from '../../utils'; @@ -94,7 +94,7 @@ export function useOwnedEntitiesCount() { const loading = loadingEntityRefs || loadingEntityOwnership; const filter = useMemo( - () => UserOwnersFilter.owned(ownershipEntityRefs ?? []), + () => EntityUserListFilter.owned(ownershipEntityRefs ?? []), [ownershipEntityRefs], ); diff --git a/plugins/catalog-react/src/components/UserListPicker/useStarredEntitiesCount.ts b/plugins/catalog-react/src/components/UserListPicker/useStarredEntitiesCount.ts index ea7423efe8..aff5066a74 100644 --- a/plugins/catalog-react/src/components/UserListPicker/useStarredEntitiesCount.ts +++ b/plugins/catalog-react/src/components/UserListPicker/useStarredEntitiesCount.ts @@ -21,7 +21,7 @@ import { compact, isEqual } from 'lodash'; import { useMemo, useRef } from 'react'; import useAsync from 'react-use/lib/useAsync'; import { catalogApiRef } from '../../api'; -import { UserOwnersFilter } from '../../filters'; +import { EntityUserListFilter } from '../../filters'; import { useEntityList, useStarredEntities } from '../../hooks'; import { reduceCatalogFilters } from '../../utils'; @@ -72,7 +72,7 @@ export function useStarredEntitiesCount() { }, [refRequest.current, starredEntities]); const filter = useMemo( - () => UserOwnersFilter.starred(Array.from(starredEntities)), + () => EntityUserListFilter.starred(Array.from(starredEntities)), [starredEntities], ); diff --git a/plugins/catalog-react/src/filters.ts b/plugins/catalog-react/src/filters.ts index a5a39f9b02..d0ab093e3c 100644 --- a/plugins/catalog-react/src/filters.ts +++ b/plugins/catalog-react/src/filters.ts @@ -203,22 +203,22 @@ export class EntityNamespaceFilter implements EntityFilter { /** * @public */ -export class UserOwnersFilter implements EntityFilter { +export class EntityUserListFilter implements EntityFilter { private constructor( readonly value: UserListFilterKind, readonly refs?: string[], ) {} static owned(ownershipEntityRefs: string[]) { - return new UserOwnersFilter('owned', ownershipEntityRefs); + return new EntityUserListFilter('owned', ownershipEntityRefs); } static all() { - return new UserOwnersFilter('all'); + return new EntityUserListFilter('all'); } static starred(starredEntityRefs: string[]) { - return new UserOwnersFilter('starred', starredEntityRefs); + return new EntityUserListFilter('starred', starredEntityRefs); } getCatalogFilters(): Record { @@ -247,7 +247,7 @@ export class UserOwnersFilter implements EntityFilter { /** * Filters entities based on whatever the user has starred or owns them. - * @deprecated use UserOwnersFilter + * @deprecated use EntityUserListFilter * @public */ export class UserListFilter implements EntityFilter { diff --git a/plugins/catalog-react/src/hooks/useEntityListProvider.tsx b/plugins/catalog-react/src/hooks/useEntityListProvider.tsx index b7ee8a94ed..e2a8cfc657 100644 --- a/plugins/catalog-react/src/hooks/useEntityListProvider.tsx +++ b/plugins/catalog-react/src/hooks/useEntityListProvider.tsx @@ -41,7 +41,7 @@ import { EntityTypeFilter, UserListFilter, EntityNamespaceFilter, - UserOwnersFilter, + EntityUserListFilter, } from '../filters'; import { EntityFilter } from '../types'; import { reduceBackendCatalogFilters, reduceEntityFilters } from '../utils'; @@ -51,7 +51,7 @@ import { useApi } from '@backstage/core-plugin-api'; export type DefaultEntityFilters = { kind?: EntityKindFilter; type?: EntityTypeFilter; - user?: UserListFilter | UserOwnersFilter; + user?: UserListFilter | EntityUserListFilter; owners?: EntityOwnerFilter; lifecycles?: EntityLifecycleFilter; tags?: EntityTagFilter; From 78b2610717546e406def7f2a55c3c385b7f0b656 Mon Sep 17 00:00:00 2001 From: Vincenzo Scamporlino Date: Fri, 26 May 2023 14:14:48 +0200 Subject: [PATCH 08/33] catalog-react: remove UserListFilter usage Signed-off-by: Vincenzo Scamporlino --- plugins/catalog-react/api-report.md | 43 ++-- .../UserListPicker/UserListPicker.test.tsx | 215 +----------------- .../UserListPicker/UserListPicker.tsx | 50 ++-- plugins/catalog-react/src/filters.ts | 9 + .../src/hooks/useEntityListProvider.test.tsx | 23 +- .../src/hooks/useEntityOwnership.ts | 14 +- 6 files changed, 73 insertions(+), 281 deletions(-) diff --git a/plugins/catalog-react/api-report.md b/plugins/catalog-react/api-report.md index 3faa5524c9..e73441f7dc 100644 --- a/plugins/catalog-react/api-report.md +++ b/plugins/catalog-react/api-report.md @@ -142,7 +142,7 @@ export const columnFactories: Readonly<{ export type DefaultEntityFilters = { kind?: EntityKindFilter; type?: EntityTypeFilter; - user?: UserListFilter | UserOwnersFilter; + user?: UserListFilter | EntityUserListFilter; owners?: EntityOwnerFilter; lifecycles?: EntityLifecycleFilter; tags?: EntityTagFilter; @@ -507,6 +507,26 @@ export interface EntityTypePickerProps { initialFilter?: string; } +// @public (undocumented) +export class EntityUserListFilter implements EntityFilter { + // (undocumented) + static all(): EntityUserListFilter; + // (undocumented) + filterEntity(entity: Entity): boolean; + // (undocumented) + getCatalogFilters(): Record; + // (undocumented) + static owned(ownershipEntityRefs: string[]): EntityUserListFilter; + // (undocumented) + readonly refs?: string[] | undefined; + // (undocumented) + static starred(starredEntityRefs: string[]): EntityUserListFilter; + // (undocumented) + toQueryValue(): string; + // (undocumented) + readonly value: UserListFilterKind; +} + // @public export const FavoriteEntity: ( props: FavoriteEntityProps, @@ -661,29 +681,8 @@ export const UserListPicker: ( export type UserListPickerProps = { initialFilter?: UserListFilterKind; availableFilters?: UserListFilterKind[]; - useServerSideFilters?: boolean; }; -// @public (undocumented) -export class UserOwnersFilter implements EntityFilter { - // (undocumented) - static all(): UserOwnersFilter; - // (undocumented) - filterEntity(entity: Entity): boolean; - // (undocumented) - getCatalogFilters(): Record; - // (undocumented) - static owned(ownershipEntityRefs: string[]): UserOwnersFilter; - // (undocumented) - readonly refs?: string[] | undefined; - // (undocumented) - static starred(starredEntityRefs: string[]): UserOwnersFilter; - // (undocumented) - toQueryValue(): string; - // (undocumented) - readonly value: UserListFilterKind; -} - // @public (undocumented) export function useStarredEntities(): { starredEntities: Set; diff --git a/plugins/catalog-react/src/components/UserListPicker/UserListPicker.test.tsx b/plugins/catalog-react/src/components/UserListPicker/UserListPicker.test.tsx index 20dedf23a6..ed6c43d12e 100644 --- a/plugins/catalog-react/src/components/UserListPicker/UserListPicker.test.tsx +++ b/plugins/catalog-react/src/components/UserListPicker/UserListPicker.test.tsx @@ -16,14 +16,10 @@ import React from 'react'; import { fireEvent, render, waitFor, screen } from '@testing-library/react'; -import { Entity, UserEntity } from '@backstage/catalog-model'; +import { UserEntity } from '@backstage/catalog-model'; import { UserListPicker, UserListPickerProps } from './UserListPicker'; import { MockEntityListContextProvider } from '../../testUtils/providers'; -import { - EntityTagFilter, - UserListFilter, - EntityUserListFilter, -} from '../../filters'; +import { EntityTagFilter, EntityUserListFilter } from '../../filters'; import { CatalogApi, QueryEntitiesInitialRequest, @@ -75,14 +71,6 @@ const apis = TestApiRegistry.from( [starredEntitiesApiRef, mockStarredEntitiesApi], ); -const mockIsOwnedEntity = jest.fn( - (entity: Entity) => entity.metadata.name === 'component-1', -); - -const mockIsStarredEntity = jest.fn( - (entity: Entity) => entity.metadata.name === 'component-3', -); - const ownershipEntityRefs = ['user:default/testuser']; describe('', () => { const mockQueryEntitiesImplementation: CatalogApi['queryEntities'] = @@ -233,44 +221,6 @@ describe('', () => { }); }); - it('respects the query parameter filter value, legacy', async () => { - const updateFilters = jest.fn(); - const queryParameters = { user: 'owned' }; - render( - - - - - , - ); - - await waitFor(() => - expect(updateFilters).toHaveBeenLastCalledWith({ - user: new UserListFilter( - 'owned', - expect.any(Function), - expect.any(Function), - ), - }), - ); - expect(mockCatalogApi.queryEntities).toHaveBeenCalledWith({ - filter: {}, - limit: 0, - }); - expect(mockCatalogApi.queryEntities).toHaveBeenCalledWith({ - filter: { 'metadata.name': ['e-1', 'e-2'] }, - limit: 1000, - }); - expect(mockCatalogApi.queryEntities).toHaveBeenCalledWith({ - filter: { - 'relations.ownedBy': ['user:default/testuser'], - }, - limit: 0, - }); - }); - it('respects the query parameter filter value', async () => { const updateFilters = jest.fn(); const queryParameters = { user: 'owned' }; @@ -279,7 +229,7 @@ describe('', () => { - + , ); @@ -306,35 +256,12 @@ describe('', () => { }); }); - it('updates user filter when a menuitem is selected, legacy', async () => { - const updateFilters = jest.fn(); - render( - - - - - , - ); - - fireEvent.click(screen.getByText('Starred')); - - await waitFor(() => - expect(updateFilters).toHaveBeenLastCalledWith({ - user: new UserListFilter( - 'starred', - expect.any(Function), - expect.any(Function), - ), - }), - ); - }); - it('updates user filter when a menuitem is selected', async () => { const updateFilters = jest.fn(); render( - + , ); @@ -351,52 +278,6 @@ describe('', () => { ); }); - it('responds to external queryParameters changes, legacy', async () => { - const updateFilters = jest.fn(); - const rendered = render( - - - - - , - ); - - await waitFor(() => - expect(updateFilters).toHaveBeenLastCalledWith({ - user: new UserListFilter( - 'all', - expect.any(Function), - expect.any(Function), - ), - }), - ); - - rendered.rerender( - - - - - , - ); - expect(updateFilters).toHaveBeenLastCalledWith({ - user: new UserListFilter( - 'owned', - expect.any(Function), - expect.any(Function), - ), - }); - }); - it('responds to external queryParameters changes', async () => { const updateFilters = jest.fn(); const rendered = render( @@ -407,7 +288,7 @@ describe('', () => { queryParameters: { user: ['all'] }, }} > - + , ); @@ -426,7 +307,7 @@ describe('', () => { queryParameters: { user: ['owned'] }, }} > - + , ); @@ -489,34 +370,6 @@ describe('', () => { }); }); - it('resets the filter to "all" when entities are loaded, legacy', async () => { - mockCatalogApi.queryEntities?.mockImplementation(async request => { - if ( - ( - (request as QueryEntitiesInitialRequest).filter as Record< - string, - string - > - )['relations.ownedBy'] - ) { - return { items: [], totalItems: 0, pageInfo: {} }; - } - return mockQueryEntitiesImplementation(request); - }); - - render(); - - await waitFor(() => - expect(updateFilters).toHaveBeenLastCalledWith({ - user: new UserListFilter( - 'all', - expect.any(Function), - expect.any(Function), - ), - }), - ); - }); - it('resets the filter to "all" when entities are loaded', async () => { mockCatalogApi.queryEntities?.mockImplementation(async request => { if ( @@ -532,7 +385,7 @@ describe('', () => { return mockQueryEntitiesImplementation(request); }); - render(); + render(); await waitFor(() => expect(updateFilters).toHaveBeenLastCalledWith({ @@ -581,34 +434,6 @@ describe('', () => { }); }); - it('resets the filter to "all" when entities are loaded, legacy', async () => { - mockCatalogApi.queryEntities?.mockImplementation(async request => { - if ( - ( - (request as QueryEntitiesInitialRequest).filter as Record< - string, - string - > - )['metadata.name'] - ) { - return { items: [], totalItems: 0, pageInfo: {} }; - } - return mockQueryEntitiesImplementation(request); - }); - - render(); - - await waitFor(() => - expect(updateFilters).toHaveBeenLastCalledWith({ - user: new UserListFilter( - 'all', - expect.any(Function), - expect.any(Function), - ), - }), - ); - }); - it('resets the filter to "all" when entities are loaded', async () => { mockCatalogApi.queryEntities?.mockImplementation(async request => { if ( @@ -624,7 +449,7 @@ describe('', () => { return mockQueryEntitiesImplementation(request); }); - render(); + render(); await waitFor(() => expect(updateFilters).toHaveBeenLastCalledWith({ @@ -658,11 +483,7 @@ describe('', () => { expect(mockCatalogApi.queryEntities).toHaveBeenCalledTimes(3), ); expect(updateFilters).not.toHaveBeenCalledWith({ - user: new UserListFilter( - 'all', - mockIsOwnedEntity, - mockIsStarredEntity, - ), + user: EntityUserListFilter.all(), }); }); @@ -675,11 +496,7 @@ describe('', () => { await waitFor(() => expect(updateFilters).toHaveBeenLastCalledWith({ - user: new UserListFilter( - 'owned', - expect.any(Function), - expect.any(Function), - ), + user: EntityUserListFilter.owned(expect.any(Array)), }), ); }); @@ -709,11 +526,7 @@ describe('', () => { expect(mockCatalogApi.queryEntities).toHaveBeenCalledTimes(3), ); expect(updateFilters).not.toHaveBeenCalledWith({ - user: new UserListFilter( - 'all', - mockIsOwnedEntity, - mockIsStarredEntity, - ), + user: EntityUserListFilter.all(), }); }); @@ -726,11 +539,7 @@ describe('', () => { await waitFor(() => expect(updateFilters).toHaveBeenLastCalledWith({ - user: new UserListFilter( - 'starred', - expect.any(Function), - expect.any(Function), - ), + user: EntityUserListFilter.starred(expect.any(Array)), }), ); }); diff --git a/plugins/catalog-react/src/components/UserListPicker/UserListPicker.tsx b/plugins/catalog-react/src/components/UserListPicker/UserListPicker.tsx index dff4f995ae..b0a86b1322 100644 --- a/plugins/catalog-react/src/components/UserListPicker/UserListPicker.tsx +++ b/plugins/catalog-react/src/components/UserListPicker/UserListPicker.tsx @@ -33,13 +33,12 @@ import { import SettingsIcon from '@material-ui/icons/Settings'; import StarIcon from '@material-ui/icons/Star'; import React, { Fragment, useEffect, useMemo, useState } from 'react'; -import { UserListFilter, EntityUserListFilter } from '../../filters'; -import { useEntityList, useStarredEntities } from '../../hooks'; +import { EntityUserListFilter } from '../../filters'; +import { useEntityList } from '../../hooks'; import { UserListFilterKind } from '../../types'; import { useOwnedEntitiesCount } from './useOwnedEntitiesCount'; import { useAllEntitiesCount } from './useAllEntitiesCount'; import { useStarredEntitiesCount } from './useStarredEntitiesCount'; -import { useIsOwnedEntity } from '../../hooks/useEntityOwnership'; /** @public */ export type CatalogReactUserListPickerClassKey = @@ -120,12 +119,11 @@ function getFilterGroups(orgName: string | undefined): ButtonGroup[] { export type UserListPickerProps = { initialFilter?: UserListFilterKind; availableFilters?: UserListFilterKind[]; - useServerSideFilters?: boolean; }; /** @public */ export const UserListPicker = (props: UserListPickerProps) => { - const { initialFilter, availableFilters, useServerSideFilters } = props; + const { initialFilter, availableFilters } = props; const classes = useStyles(); const configApi = useApi(configApiRef); const orgName = configApi.getOptionalString('organization.name') ?? 'Company'; @@ -154,7 +152,6 @@ export const UserListPicker = (props: UserListPickerProps) => { count: ownedEntitiesCount, loading: loadingOwnedEntities, filter: ownedEntitiesFilter, - ownershipEntityRefs, } = useOwnedEntitiesCount(); const { count: allCount } = useAllEntitiesCount(); const { @@ -180,9 +177,6 @@ export const UserListPicker = (props: UserListPickerProps) => { }; }, [starredEntitiesCount, ownedEntitiesCount, allCount]); - const { isStarredEntity } = useStarredEntities(); - const isOwnedEntity = useIsOwnedEntity(ownershipEntityRefs); - // Set selected user filter on query parameter updates; this happens at initial page load and from // external updates to the page location. useEffect(() => { @@ -211,38 +205,24 @@ export const UserListPicker = (props: UserListPickerProps) => { if (loading) { return; } - if (useServerSideFilters) { - const getFilter = () => { - if (selectedUserFilter === 'owned') { - return ownedEntitiesFilter; - } - if (selectedUserFilter === 'starred') { - return starredEntitiesFilter; - } - return EntityUserListFilter.all(); - }; - updateFilters({ user: getFilter() }); - } else { - // legacy - updateFilters({ - user: selectedUserFilter - ? new UserListFilter( - selectedUserFilter as UserListFilterKind, - isOwnedEntity, - isStarredEntity, - ) - : undefined, - }); - } + const getFilter = () => { + if (selectedUserFilter === 'owned') { + return ownedEntitiesFilter; + } + if (selectedUserFilter === 'starred') { + return starredEntitiesFilter; + } + return EntityUserListFilter.all(); + }; + + updateFilters({ user: getFilter() }); }, [ selectedUserFilter, starredEntitiesFilter, ownedEntitiesFilter, updateFilters, - useServerSideFilters, - isOwnedEntity, - isStarredEntity, + loading, ]); diff --git a/plugins/catalog-react/src/filters.ts b/plugins/catalog-react/src/filters.ts index d0ab093e3c..bafaae4fb2 100644 --- a/plugins/catalog-react/src/filters.ts +++ b/plugins/catalog-react/src/filters.ts @@ -237,6 +237,15 @@ export class EntityUserListFilter implements EntityFilter { if (this.value === 'starred') { return this.refs?.includes(stringifyEntityRef(entity)) ?? true; } + if (this.value === 'owned') { + return ( + this.refs?.some(v => + getEntityRelations(entity, RELATION_OWNED_BY).some( + o => stringifyEntityRef(o) === v, + ), + ) ?? false + ); + } return true; } diff --git a/plugins/catalog-react/src/hooks/useEntityListProvider.test.tsx b/plugins/catalog-react/src/hooks/useEntityListProvider.test.tsx index d77655ebcc..a4401e3441 100644 --- a/plugins/catalog-react/src/hooks/useEntityListProvider.test.tsx +++ b/plugins/catalog-react/src/hooks/useEntityListProvider.test.tsx @@ -32,7 +32,11 @@ import { MemoryRouter } from 'react-router-dom'; import { catalogApiRef } from '../api'; import { starredEntitiesApiRef, MockStarredEntitiesApi } from '../apis'; import { EntityKindPicker, UserListPicker } from '../components'; -import { EntityKindFilter, EntityTypeFilter, UserListFilter } from '../filters'; +import { + EntityKindFilter, + EntityTypeFilter, + EntityUserListFilter, +} from '../filters'; import { UserListFilterKind } from '../types'; import { EntityListProvider, useEntityList } from './useEntityListProvider'; @@ -62,11 +66,14 @@ const entities: Entity[] = [ const mockConfigApi = { getOptionalString: () => '', } as Partial; + +const ownershipEntityRefs = ['user:default/guest']; + const mockIdentityApi: Partial = { getBackstageIdentity: async () => ({ type: 'user', userEntityRef: 'user:default/guest', - ownershipEntityRefs: [], + ownershipEntityRefs, }), getCredentials: async () => ({ token: undefined }), }; @@ -148,11 +155,7 @@ describe('', () => { act(() => result.current.updateFilters({ - user: new UserListFilter( - 'owned', - entity => entity.metadata.name === 'component-1', - () => true, - ), + user: EntityUserListFilter.owned(ownershipEntityRefs), }), ); @@ -193,11 +196,7 @@ describe('', () => { act(() => result.current.updateFilters({ - user: new UserListFilter( - 'owned', - entity => entity.metadata.name === 'component-1', - () => true, - ), + user: EntityUserListFilter.owned(ownershipEntityRefs), }), ); diff --git a/plugins/catalog-react/src/hooks/useEntityOwnership.ts b/plugins/catalog-react/src/hooks/useEntityOwnership.ts index 1f1e200634..9c86d6e01a 100644 --- a/plugins/catalog-react/src/hooks/useEntityOwnership.ts +++ b/plugins/catalog-react/src/hooks/useEntityOwnership.ts @@ -46,15 +46,10 @@ export function useEntityOwnership(): { return ownershipEntityRefs; }, []); - const isOwnedEntity = useIsOwnedEntity(refs); - - return useMemo(() => ({ loading, isOwnedEntity }), [loading, isOwnedEntity]); -} - -export function useIsOwnedEntity(refs?: string[]) { - return useMemo(() => { + const isOwnedEntity = useMemo(() => { const myOwnerRefs = new Set(refs ?? []); - const isOwnedEntity = (entity: Entity) => { + + return (entity: Entity) => { const entityOwnerRefs = getEntityRelations(entity, RELATION_OWNED_BY).map( stringifyEntityRef, ); @@ -65,6 +60,7 @@ export function useIsOwnedEntity(refs?: string[]) { } return false; }; - return isOwnedEntity; }, [refs]); + + return { loading, isOwnedEntity }; } From 971a108ab168be535f27aab121603b7d7beea2f3 Mon Sep 17 00:00:00 2001 From: Vincenzo Scamporlino Date: Fri, 26 May 2023 14:19:25 +0200 Subject: [PATCH 09/33] catalog-react: add clarifying comment to EntityUserListFilter Signed-off-by: Vincenzo Scamporlino --- plugins/catalog-react/src/filters.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/plugins/catalog-react/src/filters.ts b/plugins/catalog-react/src/filters.ts index bafaae4fb2..5879d91591 100644 --- a/plugins/catalog-react/src/filters.ts +++ b/plugins/catalog-react/src/filters.ts @@ -237,6 +237,10 @@ export class EntityUserListFilter implements EntityFilter { if (this.value === 'starred') { return this.refs?.includes(stringifyEntityRef(entity)) ?? true; } + // used only for retro-compatibility with the old + // non paginated table. This is supposed to return always true + // for paginated owned entities, since the filters are applied + // server side. if (this.value === 'owned') { return ( this.refs?.some(v => From d46acd694893c2291ebbe932efc1e0afcda69a04 Mon Sep 17 00:00:00 2001 From: Vincenzo Scamporlino Date: Fri, 26 May 2023 23:29:13 +0200 Subject: [PATCH 10/33] catalog-react: improve memo readability Signed-off-by: Vincenzo Scamporlino --- .../UserListPicker/useAllEntitiesCount.ts | 26 ++++++--------- .../UserListPicker/useOwnedEntitiesCount.ts | 32 ++++++++----------- .../UserListPicker/useStarredEntitiesCount.ts | 18 +++++------ 3 files changed, 32 insertions(+), 44 deletions(-) diff --git a/plugins/catalog-react/src/components/UserListPicker/useAllEntitiesCount.ts b/plugins/catalog-react/src/components/UserListPicker/useAllEntitiesCount.ts index 499762ff87..13217c6800 100644 --- a/plugins/catalog-react/src/components/UserListPicker/useAllEntitiesCount.ts +++ b/plugins/catalog-react/src/components/UserListPicker/useAllEntitiesCount.ts @@ -22,40 +22,32 @@ import { catalogApiRef } from '../../api'; import { useEntityList } from '../../hooks'; import { reduceCatalogFilters } from '../../utils'; -/** - * TODO(vinzscam): we need to find a better way - * for retrieving this value. One possible way, could be to use - * the /entities endpoint: since this method is paginated, - * it should also return how many items matching the provided filters - * are in the catalog - */ export function useAllEntitiesCount() { const catalogApi = useApi(catalogApiRef); const { filters } = useEntityList(); - const refRequest = useRef(); - useMemo(() => { + const prevRequest = useRef(); + const request = useMemo(() => { const { user, ...allFilters } = filters; const compacted = compact(Object.values(allFilters)); const filter = reduceCatalogFilters(compacted); - const request: QueryEntitiesInitialRequest = { + const newRequest: QueryEntitiesInitialRequest = { filter, limit: 0, }; - if (isEqual(request, refRequest.current)) { - return refRequest.current; + if (isEqual(newRequest, prevRequest.current)) { + return prevRequest.current; } - refRequest.current = request; - - return request; + prevRequest.current = newRequest; + return newRequest; }, [filters]); const { value: count, loading } = useAsync(async () => { - const { totalItems } = await catalogApi.queryEntities(refRequest.current); + const { totalItems } = await catalogApi.queryEntities(request); return totalItems; - }, [refRequest.current]); + }, [request]); return { count, loading }; } diff --git a/plugins/catalog-react/src/components/UserListPicker/useOwnedEntitiesCount.ts b/plugins/catalog-react/src/components/UserListPicker/useOwnedEntitiesCount.ts index f7fb9d4516..f2ff985c09 100644 --- a/plugins/catalog-react/src/components/UserListPicker/useOwnedEntitiesCount.ts +++ b/plugins/catalog-react/src/components/UserListPicker/useOwnedEntitiesCount.ts @@ -32,13 +32,12 @@ export function useOwnedEntitiesCount() { // Trigger load only on mount const { value: ownershipEntityRefs, loading: loadingEntityRefs } = useAsync( async () => (await identityApi.getBackstageIdentity()).ownershipEntityRefs, - [], ); - const refRequest = useRef(); + const prevRequest = useRef(); - useMemo(async () => { + const request = useMemo(() => { const compacted = compact(Object.values(filters)); const allFilter = reduceCatalogFilters(compacted); const { ['metadata.name']: metadata, ...filter } = allFilter; @@ -54,15 +53,12 @@ export function useOwnedEntitiesCount() { const ownedBy = ownedByFilter.length > 0 ? ownedByFilter : ownershipEntityRefs; if (ownedByFilter.length > 0 && commonOwnedBy.length === 0) { - // don't send any request if another filter sets - // totally different values for relations.ownedBy filter. - // TODO(vinzscam): check conflicts between UserOwnersFilter and EntityOwnerFilter. - // both set filters on the same relations.ownedBy key, so the conflicts need - // to be addressed properly. - refRequest.current = undefined; - return null; + // detect whether another filter sets values that will produce + // empty results in order to avoid sending an additional request. + prevRequest.current = undefined; + return undefined; } - const request: QueryEntitiesInitialRequest = { + const newRequest: QueryEntitiesInitialRequest = { filter: { ...filter, 'relations.ownedBy': ownedBy ?? [], @@ -70,13 +66,13 @@ export function useOwnedEntitiesCount() { limit: 0, }; - if (isEqual(request, refRequest.current)) { - return refRequest.current; + if (isEqual(newRequest, prevRequest.current)) { + return prevRequest.current; } - refRequest.current = request; + prevRequest.current = newRequest; - return request; + return newRequest; }, [filters, ownershipEntityRefs]); const { value: count, loading: loadingEntityOwnership } = @@ -84,13 +80,13 @@ export function useOwnedEntitiesCount() { if (!ownershipEntityRefs?.length) { return 0; } - if (!refRequest.current) { + if (!request) { return 0; } - const { totalItems } = await catalogApi.queryEntities(refRequest.current); + const { totalItems } = await catalogApi.queryEntities(request); return totalItems; - }, [refRequest.current]); + }, [request]); const loading = loadingEntityRefs || loadingEntityOwnership; const filter = useMemo( diff --git a/plugins/catalog-react/src/components/UserListPicker/useStarredEntitiesCount.ts b/plugins/catalog-react/src/components/UserListPicker/useStarredEntitiesCount.ts index aff5066a74..93adad0489 100644 --- a/plugins/catalog-react/src/components/UserListPicker/useStarredEntitiesCount.ts +++ b/plugins/catalog-react/src/components/UserListPicker/useStarredEntitiesCount.ts @@ -30,27 +30,27 @@ export function useStarredEntitiesCount() { const { filters } = useEntityList(); const { starredEntities } = useStarredEntities(); - const refRequest = useRef(); - useMemo(async () => { + const prevRequest = useRef(); + const request = useMemo(() => { const { user, ...allFilters } = filters; const compacted = compact(Object.values(allFilters)); const filter = reduceCatalogFilters(compacted); const facet = 'metadata.name'; - const request: QueryEntitiesInitialRequest = { + const newRequest: QueryEntitiesInitialRequest = { filter: { ...filter, [facet]: Array.from(starredEntities).map(e => parseEntityRef(e).name), }, limit: 1000, }; - if (isEqual(request, refRequest.current)) { - return refRequest.current; + if (isEqual(newRequest, prevRequest.current)) { + return prevRequest.current; } - refRequest.current = request; + prevRequest.current = newRequest; - return request; + return newRequest; }, [filters, starredEntities]); const { value: count, loading } = useAsync(async () => { @@ -58,7 +58,7 @@ export function useStarredEntitiesCount() { return 0; } - const response = await catalogApi.queryEntities(refRequest.current); + const response = await catalogApi.queryEntities(request); return response.items .map(e => @@ -69,7 +69,7 @@ export function useStarredEntitiesCount() { }), ) .filter(e => starredEntities.has(e)).length; - }, [refRequest.current, starredEntities]); + }, [request, starredEntities]); const filter = useMemo( () => EntityUserListFilter.starred(Array.from(starredEntities)), From 167d08dca03e6387b8585bae8682a910f167540f Mon Sep 17 00:00:00 2001 From: Vincenzo Scamporlino Date: Fri, 26 May 2023 23:41:36 +0200 Subject: [PATCH 11/33] catalog-react: clarify comment Signed-off-by: Vincenzo Scamporlino --- plugins/catalog-react/src/filters.ts | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/plugins/catalog-react/src/filters.ts b/plugins/catalog-react/src/filters.ts index 5879d91591..d6bd8eee3f 100644 --- a/plugins/catalog-react/src/filters.ts +++ b/plugins/catalog-react/src/filters.ts @@ -237,10 +237,9 @@ export class EntityUserListFilter implements EntityFilter { if (this.value === 'starred') { return this.refs?.includes(stringifyEntityRef(entity)) ?? true; } - // used only for retro-compatibility with the old - // non paginated table. This is supposed to return always true - // for paginated owned entities, since the filters are applied - // server side. + // used only for retro-compatibility with non paginated data. + // This is supposed to return always true for paginated + // owned entities, since the filters are applied server side. if (this.value === 'owned') { return ( this.refs?.some(v => From a996c54f68968712e95b08f09c177c73f1af5ec2 Mon Sep 17 00:00:00 2001 From: Vincenzo Scamporlino Date: Thu, 15 Jun 2023 19:33:44 +0200 Subject: [PATCH 12/33] catalog-react: fix EntityNamespaceFilter Signed-off-by: Vincenzo Scamporlino --- plugins/catalog-react/src/filters.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/catalog-react/src/filters.ts b/plugins/catalog-react/src/filters.ts index d6bd8eee3f..b94b538802 100644 --- a/plugins/catalog-react/src/filters.ts +++ b/plugins/catalog-react/src/filters.ts @@ -189,7 +189,7 @@ export class EntityNamespaceFilter implements EntityFilter { constructor(readonly values: string[]) {} getCatalogFilters(): Record { - return { 'spec.lifecycle': this.values }; + return { 'metadata.namespace': this.values }; } filterEntity(entity: Entity): boolean { return this.values.some(v => entity.metadata.namespace === v); From 633358a05bef97141564407fe10fe732998456fb Mon Sep 17 00:00:00 2001 From: Vincenzo Scamporlino Date: Thu, 15 Jun 2023 19:34:51 +0200 Subject: [PATCH 13/33] catalog-react: improve comment Signed-off-by: Vincenzo Scamporlino --- .../UserListPicker/useOwnedEntitiesCount.ts | 3 ++- .../catalog-react/src/hooks/useEntityOwnership.ts | 12 ++++++++---- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/plugins/catalog-react/src/components/UserListPicker/useOwnedEntitiesCount.ts b/plugins/catalog-react/src/components/UserListPicker/useOwnedEntitiesCount.ts index f2ff985c09..5eac3ced60 100644 --- a/plugins/catalog-react/src/components/UserListPicker/useOwnedEntitiesCount.ts +++ b/plugins/catalog-react/src/components/UserListPicker/useOwnedEntitiesCount.ts @@ -29,9 +29,10 @@ export function useOwnedEntitiesCount() { const catalogApi = useApi(catalogApiRef); const { filters } = useEntityList(); - // Trigger load only on mount + const { value: ownershipEntityRefs, loading: loadingEntityRefs } = useAsync( async () => (await identityApi.getBackstageIdentity()).ownershipEntityRefs, + // load only on mount [], ); diff --git a/plugins/catalog-react/src/hooks/useEntityOwnership.ts b/plugins/catalog-react/src/hooks/useEntityOwnership.ts index 9c86d6e01a..fdd73434ec 100644 --- a/plugins/catalog-react/src/hooks/useEntityOwnership.ts +++ b/plugins/catalog-react/src/hooks/useEntityOwnership.ts @@ -41,10 +41,14 @@ export function useEntityOwnership(): { const identityApi = useApi(identityApiRef); // Trigger load only on mount - const { loading, value: refs } = useAsync(async () => { - const { ownershipEntityRefs } = await identityApi.getBackstageIdentity(); - return ownershipEntityRefs; - }, []); + const { loading, value: refs } = useAsync( + async () => { + const { ownershipEntityRefs } = await identityApi.getBackstageIdentity(); + return ownershipEntityRefs; + }, + // load only on mount + [], + ); const isOwnedEntity = useMemo(() => { const myOwnerRefs = new Set(refs ?? []); From ead3cb82f64ef1601bf14e57c8af41e669c96643 Mon Sep 17 00:00:00 2001 From: Vincenzo Scamporlino Date: Tue, 20 Jun 2023 13:57:23 +0200 Subject: [PATCH 14/33] catalog-react: do not send request when filters are loading Signed-off-by: Vincenzo Scamporlino --- .../src/components/UserListPicker/useAllEntitiesCount.ts | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/plugins/catalog-react/src/components/UserListPicker/useAllEntitiesCount.ts b/plugins/catalog-react/src/components/UserListPicker/useAllEntitiesCount.ts index 13217c6800..ba971b088d 100644 --- a/plugins/catalog-react/src/components/UserListPicker/useAllEntitiesCount.ts +++ b/plugins/catalog-react/src/components/UserListPicker/useAllEntitiesCount.ts @@ -36,6 +36,11 @@ export function useAllEntitiesCount() { limit: 0, }; + if (Object.keys(filter).length === 0) { + prevRequest.current = undefined; + return prevRequest.current; + } + if (isEqual(newRequest, prevRequest.current)) { return prevRequest.current; } @@ -44,6 +49,9 @@ export function useAllEntitiesCount() { }, [filters]); const { value: count, loading } = useAsync(async () => { + if (request === undefined) { + return 0; + } const { totalItems } = await catalogApi.queryEntities(request); return totalItems; From ecf4b77d50bedc1724678447f2c10a712590eb89 Mon Sep 17 00:00:00 2001 From: Vincenzo Scamporlino Date: Tue, 20 Jun 2023 13:58:05 +0200 Subject: [PATCH 15/33] catalog-react: add useAllEntitiesCount tests Signed-off-by: Vincenzo Scamporlino --- .../useAllEntitiesCount.test.tsx | 106 ++++++++++++++++++ 1 file changed, 106 insertions(+) create mode 100644 plugins/catalog-react/src/components/UserListPicker/useAllEntitiesCount.test.tsx diff --git a/plugins/catalog-react/src/components/UserListPicker/useAllEntitiesCount.test.tsx b/plugins/catalog-react/src/components/UserListPicker/useAllEntitiesCount.test.tsx new file mode 100644 index 0000000000..61c2c6eb0a --- /dev/null +++ b/plugins/catalog-react/src/components/UserListPicker/useAllEntitiesCount.test.tsx @@ -0,0 +1,106 @@ +/* + * Copyright 2023 The Backstage Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +import React, { PropsWithChildren } from 'react'; +import { CatalogApi } from '@backstage/catalog-client'; +import { useAllEntitiesCount } from './useAllEntitiesCount'; +import { waitFor } from '@testing-library/react'; +import { renderHook } from '@testing-library/react-hooks'; +import { EntityListProvider, useEntityList } from '../../hooks'; +import { catalogApiRef } from '../../api'; +import { ApiRef } from '@backstage/core-plugin-api'; +import { MemoryRouter } from 'react-router-dom'; +import { EntityOwnerFilter } from '../../filters'; +import { useMountEffect } from '@react-hookz/web'; + +const mockQueryEntities: jest.MockedFn = jest.fn(); +const mockCatalogApi: jest.Mocked> = { + queryEntities: mockQueryEntities, +}; + +jest.mock('@backstage/core-plugin-api', () => { + const actual = jest.requireActual('@backstage/core-plugin-api'); + return { + ...actual, + useApi: (ref: ApiRef) => + ref === catalogApiRef ? mockCatalogApi : actual.useApi(ref), + }; +}); + +describe('useAllEntitiesCount', () => { + beforeEach(() => { + jest.clearAllMocks(); + }); + + it('should return the count', async () => { + mockQueryEntities.mockResolvedValue({ + items: [], + totalItems: 10, + pageInfo: {}, + }); + + function WrapFilters(props: PropsWithChildren<{}>) { + const { updateFilters } = useEntityList(); + + useMountEffect(() => { + updateFilters({ + owners: new EntityOwnerFilter(['user:default/owner']), + }); + }); + return <>{props.children}; + } + + const { result } = renderHook(() => useAllEntitiesCount(), { + wrapper: ({ children }) => ( + + + {children} + + + ), + }); + + await waitFor(() => + expect(mockQueryEntities).toHaveBeenCalledWith({ + filter: { + 'relations.ownedBy': ['user:default/owner'], + }, + limit: 0, + }), + ); + expect(result.current).toEqual({ count: 10, loading: false }); + }); + + it(`shouldn't invoke the endpoint at startup, when filters are missing`, async () => { + mockQueryEntities.mockResolvedValue({ + items: [], + totalItems: 10, + pageInfo: {}, + }); + + const { result } = renderHook(() => useAllEntitiesCount(), { + wrapper: ({ children }) => ( + + {children} + + ), + }); + + await expect( + waitFor(() => expect(mockQueryEntities).toHaveBeenCalled()), + ).rejects.toThrow(); + expect(result.current).toEqual({ count: 0, loading: false }); + }); +}); From 84822daea6914e82967a03c05ae7b8ab5c339630 Mon Sep 17 00:00:00 2001 From: Vincenzo Scamporlino Date: Tue, 20 Jun 2023 13:59:41 +0200 Subject: [PATCH 16/33] catalog-react: send proper filters Signed-off-by: Vincenzo Scamporlino --- .../UserListPicker/useOwnedEntitiesCount.ts | 47 +++++++++++-------- 1 file changed, 28 insertions(+), 19 deletions(-) diff --git a/plugins/catalog-react/src/components/UserListPicker/useOwnedEntitiesCount.ts b/plugins/catalog-react/src/components/UserListPicker/useOwnedEntitiesCount.ts index 5eac3ced60..605c006f44 100644 --- a/plugins/catalog-react/src/components/UserListPicker/useOwnedEntitiesCount.ts +++ b/plugins/catalog-react/src/components/UserListPicker/useOwnedEntitiesCount.ts @@ -20,7 +20,7 @@ import { compact, intersection, isEqual } from 'lodash'; import { useMemo, useRef } from 'react'; import useAsync from 'react-use/lib/useAsync'; import { catalogApiRef } from '../../api'; -import { EntityUserListFilter } from '../../filters'; +import { EntityOwnerFilter, EntityUserListFilter } from '../../filters'; import { useEntityList } from '../../hooks'; import { reduceCatalogFilters } from '../../utils'; @@ -39,30 +39,25 @@ export function useOwnedEntitiesCount() { const prevRequest = useRef(); const request = useMemo(() => { - const compacted = compact(Object.values(filters)); + const { user, owners, ...allFilters } = filters; + const compacted = compact(Object.values(allFilters)); const allFilter = reduceCatalogFilters(compacted); const { ['metadata.name']: metadata, ...filter } = allFilter; - const facet = 'relations.ownedBy'; + const countFilter = getOwnedCountClaims(owners, ownershipEntityRefs); - const ownedByFilter = Array.isArray(filter[facet]) - ? (filter[facet] as string[]) - : []; - - const commonOwnedBy = intersection(ownedByFilter, ownershipEntityRefs); - - const ownedBy = - ownedByFilter.length > 0 ? ownedByFilter : ownershipEntityRefs; - if (ownedByFilter.length > 0 && commonOwnedBy.length === 0) { - // detect whether another filter sets values that will produce - // empty results in order to avoid sending an additional request. + if ( + ownershipEntityRefs?.length === 0 || + countFilter === undefined || + Object.keys(filter).length === 0 + ) { prevRequest.current = undefined; return undefined; } const newRequest: QueryEntitiesInitialRequest = { filter: { ...filter, - 'relations.ownedBy': ownedBy ?? [], + 'relations.ownedBy': countFilter, }, limit: 0, }; @@ -78,14 +73,10 @@ export function useOwnedEntitiesCount() { const { value: count, loading: loadingEntityOwnership } = useAsync(async () => { - if (!ownershipEntityRefs?.length) { - return 0; - } if (!request) { return 0; } const { totalItems } = await catalogApi.queryEntities(request); - return totalItems; }, [request]); @@ -102,3 +93,21 @@ export function useOwnedEntitiesCount() { ownershipEntityRefs, }; } + +function getOwnedCountClaims( + owners: EntityOwnerFilter | undefined, + ownershipEntityRefs: string[] | undefined, +) { + if (ownershipEntityRefs === undefined) { + return undefined; + } + const ownersRefs = owners?.values ?? []; + if (ownersRefs.length) { + const commonOwnedBy = intersection(ownersRefs, ownershipEntityRefs); + if (commonOwnedBy.length === 0) { + return undefined; + } + return commonOwnedBy; + } + return ownershipEntityRefs; +} From 7709c264cabc4c2fc8b0c9d609c02c4cc8abeacf Mon Sep 17 00:00:00 2001 From: Vincenzo Scamporlino Date: Tue, 20 Jun 2023 15:22:50 +0200 Subject: [PATCH 17/33] catalog-react: add useOwnedEntitiesCount tests Signed-off-by: Vincenzo Scamporlino --- .../useOwnedEntitiesCount.test.tsx | 235 ++++++++++++++++++ 1 file changed, 235 insertions(+) create mode 100644 plugins/catalog-react/src/components/UserListPicker/useOwnedEntitiesCount.test.tsx diff --git a/plugins/catalog-react/src/components/UserListPicker/useOwnedEntitiesCount.test.tsx b/plugins/catalog-react/src/components/UserListPicker/useOwnedEntitiesCount.test.tsx new file mode 100644 index 0000000000..6a79e63c8f --- /dev/null +++ b/plugins/catalog-react/src/components/UserListPicker/useOwnedEntitiesCount.test.tsx @@ -0,0 +1,235 @@ +/* + * Copyright 2023 The Backstage Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +import React, { PropsWithChildren } from 'react'; +import { CatalogApi } from '@backstage/catalog-client'; +import { renderHook } from '@testing-library/react-hooks'; +import { + DefaultEntityFilters, + EntityListProvider, + useEntityList, +} from '../../hooks'; +import { catalogApiRef } from '../../api'; +import { + ApiRef, + IdentityApi, + identityApiRef, +} from '@backstage/core-plugin-api'; +import { MemoryRouter } from 'react-router-dom'; +import { useOwnedEntitiesCount } from './useOwnedEntitiesCount'; +import { + EntityNamespaceFilter, + EntityOwnerFilter, + EntityUserListFilter, +} from '../../filters'; +import { useMountEffect } from '@react-hookz/web'; + +const mockQueryEntities: jest.MockedFn = jest.fn(); +const mockCatalogApi: jest.Mocked> = { + queryEntities: mockQueryEntities, +}; + +const mockGetBackstageIdentity: jest.MockedFn< + IdentityApi['getBackstageIdentity'] +> = jest.fn(); + +jest.mock('@backstage/core-plugin-api', () => { + const actual = jest.requireActual('@backstage/core-plugin-api'); + return { + ...actual, + useApi: (ref: ApiRef) => { + if (ref === catalogApiRef) { + return mockCatalogApi; + } + if (ref === identityApiRef) { + return { + getBackstageIdentity: mockGetBackstageIdentity, + }; + } + + return actual.useApi(ref); + }, + }; +}); + +describe('useOwnedEntitiesCount', () => { + beforeEach(() => { + jest.clearAllMocks(); + + mockGetBackstageIdentity.mockResolvedValue({ + ownershipEntityRefs: ['user:default/spiderman', 'user:group/a-group'], + userEntityRef: 'user:default/spiderman', + type: 'user', + }); + }); + + it(`shouldn't invoke queryEntities when filters are loading`, async () => { + mockQueryEntities.mockResolvedValue({ + items: [], + totalItems: 10, + pageInfo: {}, + }); + + const { result, waitFor } = renderHook(() => useOwnedEntitiesCount(), { + wrapper: createWrapperWithInitialFilters({}), + }); + + await waitFor(() => expect(mockGetBackstageIdentity).toHaveBeenCalled()); + + await expect( + waitFor(() => expect(mockQueryEntities).toHaveBeenCalled()), + ).rejects.toThrow(); + + expect(result.current).toEqual({ + count: 0, + loading: false, + filter: EntityUserListFilter.owned([ + 'user:default/spiderman', + 'user:group/a-group', + ]), + ownershipEntityRefs: ['user:default/spiderman', 'user:group/a-group'], + }); + }); + + it(`should properly apply the filters`, async () => { + mockQueryEntities.mockResolvedValue({ + items: [], + totalItems: 10, + pageInfo: {}, + }); + + const { result, waitFor } = renderHook(() => useOwnedEntitiesCount(), { + wrapper: createWrapperWithInitialFilters({ + namespace: new EntityNamespaceFilter(['a-namespace']), + }), + }); + + await waitFor(() => expect(mockGetBackstageIdentity).toHaveBeenCalled()); + + await waitFor(() => + expect(mockQueryEntities).toHaveBeenCalledWith({ + filter: { + 'metadata.namespace': ['a-namespace'], + 'relations.ownedBy': ['user:default/spiderman', 'user:group/a-group'], + }, + limit: 0, + }), + ); + + expect(result.current).toEqual({ + count: 10, + loading: false, + filter: EntityUserListFilter.owned([ + 'user:default/spiderman', + 'user:group/a-group', + ]), + ownershipEntityRefs: ['user:default/spiderman', 'user:group/a-group'], + }); + }); + + it(`should return count 0 without invoking queryEntities if owners filter doesn't have claims on common with logged in user`, async () => { + mockQueryEntities.mockResolvedValue({ + items: [], + totalItems: 10, + pageInfo: {}, + }); + + const { result, waitFor } = renderHook(() => useOwnedEntitiesCount(), { + wrapper: createWrapperWithInitialFilters({ + namespace: new EntityNamespaceFilter(['a-namespace']), + owners: new EntityOwnerFilter(['group:default/monsters']), + }), + }); + + await waitFor(() => expect(mockGetBackstageIdentity).toHaveBeenCalled()); + + await expect( + waitFor(() => expect(mockQueryEntities).toHaveBeenCalled()), + ).rejects.toThrow(); + + expect(result.current).toEqual({ + count: 0, + loading: false, + filter: EntityUserListFilter.owned([ + 'user:default/spiderman', + 'user:group/a-group', + ]), + ownershipEntityRefs: ['user:default/spiderman', 'user:group/a-group'], + }); + }); + + it(`should send claims in common between owners filter and logged in user`, async () => { + mockQueryEntities.mockResolvedValue({ + items: [], + totalItems: 10, + pageInfo: {}, + }); + + const { result, waitFor } = renderHook(() => useOwnedEntitiesCount(), { + wrapper: createWrapperWithInitialFilters({ + namespace: new EntityNamespaceFilter(['a-namespace']), + owners: new EntityOwnerFilter([ + 'group:default/monsters', + 'user:group/a-group', + ]), + }), + }); + + await waitFor(() => expect(mockGetBackstageIdentity).toHaveBeenCalled()); + + await waitFor(() => + expect(mockQueryEntities).toHaveBeenCalledWith({ + filter: { + 'metadata.namespace': ['a-namespace'], + 'relations.ownedBy': ['user:group/a-group'], + }, + limit: 0, + }), + ); + + expect(result.current).toEqual({ + count: 10, + loading: false, + filter: EntityUserListFilter.owned([ + 'user:default/spiderman', + 'user:group/a-group', + ]), + ownershipEntityRefs: ['user:default/spiderman', 'user:group/a-group'], + }); + }); +}); + +function createWrapperWithInitialFilters( + filters: Partial, +) { + function WrapFilters(props: PropsWithChildren<{}>) { + const { updateFilters } = useEntityList(); + + useMountEffect(() => { + updateFilters(filters); + }); + return <>{props.children}; + } + + return function Wrapper(props: PropsWithChildren<{}>) { + return ( + + + {props.children} + + + ); + }; +} From bd4c4cec5d52fc3104159f276c062896fd07ad3f Mon Sep 17 00:00:00 2001 From: Vincenzo Scamporlino Date: Wed, 21 Jun 2023 10:42:04 +0200 Subject: [PATCH 18/33] catalog-react: fix UserListPicker tests Signed-off-by: Vincenzo Scamporlino --- .../UserListPicker/UserListPicker.test.tsx | 84 +++++++++++++++---- 1 file changed, 66 insertions(+), 18 deletions(-) diff --git a/plugins/catalog-react/src/components/UserListPicker/UserListPicker.test.tsx b/plugins/catalog-react/src/components/UserListPicker/UserListPicker.test.tsx index ed6c43d12e..f0b82b550e 100644 --- a/plugins/catalog-react/src/components/UserListPicker/UserListPicker.test.tsx +++ b/plugins/catalog-react/src/components/UserListPicker/UserListPicker.test.tsx @@ -19,7 +19,12 @@ import { fireEvent, render, waitFor, screen } from '@testing-library/react'; import { UserEntity } from '@backstage/catalog-model'; import { UserListPicker, UserListPickerProps } from './UserListPicker'; import { MockEntityListContextProvider } from '../../testUtils/providers'; -import { EntityTagFilter, EntityUserListFilter } from '../../filters'; +import { + EntityKindFilter, + EntityNamespaceFilter, + EntityTagFilter, + EntityUserListFilter, +} from '../../filters'; import { CatalogApi, QueryEntitiesInitialRequest, @@ -159,12 +164,19 @@ describe('', () => { it('renders filters', async () => { render( - + , ); + await waitFor(() => + expect(mockIdentityApi.getBackstageIdentity).toHaveBeenCalled(), + ); await waitFor(() => expect( screen.getAllByRole('menuitem').map(({ textContent }) => textContent), @@ -172,17 +184,25 @@ describe('', () => { ); expect(mockCatalogApi.queryEntities).toHaveBeenCalledWith({ - filter: {}, + filter: { + 'metadata.namespace': ['default'], + }, limit: 0, }); expect(mockCatalogApi.queryEntities).toHaveBeenCalledWith({ - filter: { 'metadata.name': ['e-1', 'e-2'] }, + filter: { + 'metadata.namespace': ['default'], + 'relations.ownedBy': ['user:default/testuser'], + }, + limit: 0, + }); + expect(mockCatalogApi.queryEntities).toHaveBeenCalledWith({ + filter: { + 'metadata.namespace': ['default'], + 'metadata.name': ['e-1', 'e-2'], + }, limit: 1000, }); - expect(mockCatalogApi.queryEntities).toHaveBeenCalledWith({ - filter: { 'relations.ownedBy': ['user:default/testuser'] }, - limit: 0, - }); }); it('respects other frontend filters in counts', async () => { @@ -223,16 +243,23 @@ describe('', () => { it('respects the query parameter filter value', async () => { const updateFilters = jest.fn(); - const queryParameters = { user: 'owned' }; + const queryParameters = { user: 'owned', kind: 'component' }; render( , ); + await waitFor(() => + expect(mockIdentityApi.getBackstageIdentity).toHaveBeenCalled(), + ); await waitFor(() => expect(updateFilters).toHaveBeenLastCalledWith({ @@ -241,15 +268,16 @@ describe('', () => { ); expect(mockCatalogApi.queryEntities).toHaveBeenCalledWith({ - filter: {}, + filter: { kind: 'component' }, limit: 0, }); expect(mockCatalogApi.queryEntities).toHaveBeenCalledWith({ - filter: { 'metadata.name': ['e-1', 'e-2'] }, + filter: { kind: 'component', 'metadata.name': ['e-1', 'e-2'] }, limit: 1000, }); expect(mockCatalogApi.queryEntities).toHaveBeenCalledWith({ filter: { + kind: 'component', 'relations.ownedBy': ['user:default/testuser'], }, limit: 0, @@ -285,7 +313,11 @@ describe('', () => { @@ -293,6 +325,10 @@ describe('', () => { , ); + await waitFor(() => + expect(mockIdentityApi.getBackstageIdentity).toHaveBeenCalled(), + ); + await waitFor(() => expect(updateFilters).toHaveBeenLastCalledWith({ user: EntityUserListFilter.all(), @@ -304,7 +340,11 @@ describe('', () => { @@ -319,9 +359,14 @@ describe('', () => { describe('filter resetting', () => { let updateFilters: jest.Mock; - const Picker = (props: UserListPickerProps) => ( + const Picker = ({ ...props }: UserListPickerProps) => ( - + @@ -331,7 +376,7 @@ describe('', () => { updateFilters = jest.fn(); }); - describe(`when there are no owned entities match the filter`, () => { + 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(() => {}), @@ -342,7 +387,10 @@ describe('', () => { await waitFor(() => expect(mockCatalogApi.queryEntities).toHaveBeenCalled(), ); - expect(updateFilters).not.toHaveBeenCalled(); + + await expect( + waitFor(() => expect(updateFilters).toHaveBeenCalled()), + ).rejects.toThrow(); }); it('does not reset the filter while owned entities are loading', async () => { From 083d556ef6774c8ab3bf52fa886b34ea153f0b7e Mon Sep 17 00:00:00 2001 From: Vincenzo Scamporlino Date: Thu, 22 Jun 2023 23:55:33 +0200 Subject: [PATCH 19/33] catalog: mock queryEntities Signed-off-by: Vincenzo Scamporlino --- .../CatalogPage/DefaultCatalogPage.test.tsx | 70 +++++++++++++++++-- 1 file changed, 63 insertions(+), 7 deletions(-) diff --git a/plugins/catalog/src/components/CatalogPage/DefaultCatalogPage.test.tsx b/plugins/catalog/src/components/CatalogPage/DefaultCatalogPage.test.tsx index b7d3ca3ed8..f16eaf4e93 100644 --- a/plugins/catalog/src/components/CatalogPage/DefaultCatalogPage.test.tsx +++ b/plugins/catalog/src/components/CatalogPage/DefaultCatalogPage.test.tsx @@ -14,7 +14,10 @@ * limitations under the License. */ -import { CatalogApi } from '@backstage/catalog-client'; +import { + CatalogApi, + QueryEntitiesInitialRequest, +} from '@backstage/catalog-client'; import { RELATION_OWNED_BY } from '@backstage/catalog-model'; import { TableColumn, TableProps } from '@backstage/core-components'; import { @@ -36,7 +39,7 @@ import { renderInTestApp, } from '@backstage/test-utils'; import DashboardIcon from '@material-ui/icons/Dashboard'; -import { fireEvent, screen } from '@testing-library/react'; +import { fireEvent, screen, waitFor } from '@testing-library/react'; import React from 'react'; import { createComponentRouteRef } from '../../routes'; import { CatalogTableRow } from '../CatalogTable'; @@ -49,10 +52,12 @@ describe('DefaultCatalogPage', () => { }); afterEach(() => { window.history.replaceState = origReplaceState; + + jest.clearAllMocks(); }); - const catalogApi: Partial = { - getEntities: () => + const catalogApi: jest.Mocked> = { + getEntities: jest.fn().mockImplementation(() => Promise.resolve({ items: [ { @@ -97,17 +102,59 @@ describe('DefaultCatalogPage', () => { }, ], }), - getLocationByRef: () => - Promise.resolve({ id: 'id', type: 'url', target: 'url' }), - getEntityFacets: async () => ({ + ), + getLocationByRef: jest + .fn() + .mockImplementation(() => + Promise.resolve({ id: 'id', type: 'url', target: 'url' }), + ), + getEntityFacets: jest.fn().mockImplementation(async () => ({ facets: { 'relations.ownedBy': [ { count: 1, value: 'group:default/not-tools' }, { count: 1, value: 'group:default/tools' }, ], }, + })), + queryEntities: jest.fn().mockImplementation(async request => { + if ( + ( + (request as QueryEntitiesInitialRequest).filter as Record< + string, + string + > + )['relations.ownedBy'] + ) { + // owned entities + return { items: [], totalItems: 3, pageInfo: {} }; + } + + if ( + ( + (request as QueryEntitiesInitialRequest).filter as Record< + string, + string + > + )['metadata.name'] + ) { + // starred entities + return { + items: [ + { + apiVersion: '1', + kind: 'component', + metadata: { name: 'Entity1', namespace: 'default' }, + }, + ], + totalItems: 1, + pageInfo: {}, + }; + } + // all items + return { items: [], totalItems: 2, pageInfo: {} }; }), }; + const testProfile: Partial = { displayName: 'Display Name', }; @@ -183,6 +230,8 @@ describe('DefaultCatalogPage', () => { it('should render the default actions of an item in the grid', async () => { await renderWrapped(); + await waitFor(() => expect(catalogApi.queryEntities).toHaveBeenCalled()); + fireEvent.click(screen.getByTestId('user-picker-owned')); await expect( screen.findByText(/Owned components \(1\)/), @@ -215,6 +264,8 @@ describe('DefaultCatalogPage', () => { ]; await renderWrapped(); + await waitFor(() => expect(catalogApi.queryEntities).toHaveBeenCalled()); + fireEvent.click(screen.getByTestId('user-picker-owned')); await expect( screen.findByText(/Owned components \(1\)/), @@ -231,7 +282,10 @@ describe('DefaultCatalogPage', () => { // https://github.com/mbrn/material-table/issues/1293 it('should render', async () => { await renderWrapped(); + await waitFor(() => expect(catalogApi.queryEntities).toHaveBeenCalled()); + fireEvent.click(screen.getByTestId('user-picker-owned')); + await expect( screen.findByText(/Owned components \(1\)/), ).resolves.toBeInTheDocument(); @@ -252,6 +306,8 @@ describe('DefaultCatalogPage', () => { // entities defaulting to "owned" filter and not based on the selected filter it('should render the correct entities filtered on the selected filter', async () => { await renderWrapped(); + await waitFor(() => expect(catalogApi.queryEntities).toHaveBeenCalled()); + fireEvent.click(screen.getByTestId('user-picker-owned')); await expect( screen.findByText(/Owned components \(1\)/), From f858e6ee26b16e81a740e6ee77c59ddf0a5b6f14 Mon Sep 17 00:00:00 2001 From: Vincenzo Scamporlino Date: Fri, 23 Jun 2023 00:21:47 +0200 Subject: [PATCH 20/33] catalog: fix flaky namespace column test Signed-off-by: Vincenzo Scamporlino --- .../src/components/CatalogPage/DefaultCatalogPage.test.tsx | 1 + 1 file changed, 1 insertion(+) diff --git a/plugins/catalog/src/components/CatalogPage/DefaultCatalogPage.test.tsx b/plugins/catalog/src/components/CatalogPage/DefaultCatalogPage.test.tsx index f16eaf4e93..b331c0e477 100644 --- a/plugins/catalog/src/components/CatalogPage/DefaultCatalogPage.test.tsx +++ b/plugins/catalog/src/components/CatalogPage/DefaultCatalogPage.test.tsx @@ -83,6 +83,7 @@ describe('DefaultCatalogPage', () => { kind: 'Component', metadata: { name: 'Entity2', + namespace: 'default', }, spec: { owner: 'not-tools', From 5447358dc9e7c817c0d8f470b185e8ba3a5bf93d Mon Sep 17 00:00:00 2001 From: Vincenzo Scamporlino Date: Tue, 3 Oct 2023 17:13:04 +0200 Subject: [PATCH 21/33] catalog-react: use correct waitFor Signed-off-by: Vincenzo Scamporlino --- .../components/UserListPicker/useAllEntitiesCount.test.tsx | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/plugins/catalog-react/src/components/UserListPicker/useAllEntitiesCount.test.tsx b/plugins/catalog-react/src/components/UserListPicker/useAllEntitiesCount.test.tsx index 61c2c6eb0a..fc2c0e65bd 100644 --- a/plugins/catalog-react/src/components/UserListPicker/useAllEntitiesCount.test.tsx +++ b/plugins/catalog-react/src/components/UserListPicker/useAllEntitiesCount.test.tsx @@ -16,7 +16,6 @@ import React, { PropsWithChildren } from 'react'; import { CatalogApi } from '@backstage/catalog-client'; import { useAllEntitiesCount } from './useAllEntitiesCount'; -import { waitFor } from '@testing-library/react'; import { renderHook } from '@testing-library/react-hooks'; import { EntityListProvider, useEntityList } from '../../hooks'; import { catalogApiRef } from '../../api'; @@ -62,7 +61,7 @@ describe('useAllEntitiesCount', () => { return <>{props.children}; } - const { result } = renderHook(() => useAllEntitiesCount(), { + const { result, waitFor } = renderHook(() => useAllEntitiesCount(), { wrapper: ({ children }) => ( @@ -90,7 +89,7 @@ describe('useAllEntitiesCount', () => { pageInfo: {}, }); - const { result } = renderHook(() => useAllEntitiesCount(), { + const { result, waitFor } = renderHook(() => useAllEntitiesCount(), { wrapper: ({ children }) => ( {children} From 62206785147963524c5995fbc2533631eb1c1647 Mon Sep 17 00:00:00 2001 From: Vincenzo Scamporlino Date: Tue, 3 Oct 2023 17:13:36 +0200 Subject: [PATCH 22/33] catalog-react: add useStarredEntitiesCount tests Signed-off-by: Vincenzo Scamporlino --- .../useStarredEntitiesCount.test.tsx | 124 ++++++++++++++++++ 1 file changed, 124 insertions(+) create mode 100644 plugins/catalog-react/src/components/UserListPicker/useStarredEntitiesCount.test.tsx diff --git a/plugins/catalog-react/src/components/UserListPicker/useStarredEntitiesCount.test.tsx b/plugins/catalog-react/src/components/UserListPicker/useStarredEntitiesCount.test.tsx new file mode 100644 index 0000000000..1a02f3996b --- /dev/null +++ b/plugins/catalog-react/src/components/UserListPicker/useStarredEntitiesCount.test.tsx @@ -0,0 +1,124 @@ +/* + * Copyright 2023 The Backstage Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +import React from 'react'; +import { CatalogApi } from '@backstage/catalog-client'; +import { EntityListProvider, useStarredEntities } from '../../hooks'; +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'; + +const mockQueryEntities: jest.MockedFn = jest.fn(); +const mockCatalogApi: jest.Mocked> = { + queryEntities: mockQueryEntities, +}; + +const mockStarredEntities: jest.MockedFn<() => Set> = jest.fn(); + +const mockUseStarredEntities: ReturnType = { + get starredEntities() { + return mockStarredEntities(); + }, +} as ReturnType; + +jest.mock('../../hooks', () => { + const actual = jest.requireActual('../../hooks'); + return { ...actual, useStarredEntities: () => mockUseStarredEntities }; +}); + +jest.mock('@backstage/core-plugin-api', () => { + const actual = jest.requireActual('@backstage/core-plugin-api'); + return { + ...actual, + useApi: (ref: ApiRef) => + ref === catalogApiRef ? mockCatalogApi : actual.useApi(ref), + }; +}); + +describe('useStarredEntitiesCount', () => { + beforeEach(() => { + jest.clearAllMocks(); + }); + + it('should return the count', async () => { + mockStarredEntities.mockReturnValue( + new Set(['component:default/favourite1', 'component:default/favourite2']), + ); + mockQueryEntities.mockResolvedValue({ + items: [ + { + apiVersion: '1', + kind: 'component', + metadata: { name: 'favourite1' }, + }, + { + apiVersion: '1', + kind: 'component', + metadata: { name: 'favourite2' }, + }, + ], + totalItems: 2, + pageInfo: {}, + }); + + const { result, waitFor } = renderHook(() => useStarredEntitiesCount(), { + wrapper: ({ children }) => ( + + {children} + + ), + }); + + 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', + }, + }); + }); + + it(`shouldn't invoke the endpoint if there are no starred entities`, async () => { + mockStarredEntities.mockReturnValue(new Set()); + + const { result, waitFor } = renderHook(() => useStarredEntitiesCount(), { + wrapper: ({ children }) => ( + + {children} + + ), + }); + + await expect( + waitFor(() => expect(mockQueryEntities).toHaveBeenCalled()), + ).rejects.toThrow(); + expect(result.current).toEqual({ + count: 0, + loading: false, + filter: { refs: [], value: 'starred' }, + }); + }); +}); From 1fd53fa0c6e667e0601c417616cf1982582463aa Mon Sep 17 00:00:00 2001 From: Vincenzo Scamporlino Date: Tue, 3 Oct 2023 17:30:01 +0200 Subject: [PATCH 23/33] add UserListPicker changeset Signed-off-by: Vincenzo Scamporlino --- .changeset/shaggy-buses-beg.md | 9 +++++++++ 1 file changed, 9 insertions(+) create mode 100644 .changeset/shaggy-buses-beg.md diff --git a/.changeset/shaggy-buses-beg.md b/.changeset/shaggy-buses-beg.md new file mode 100644 index 0000000000..d2c9351b5e --- /dev/null +++ b/.changeset/shaggy-buses-beg.md @@ -0,0 +1,9 @@ +--- +'@backstage/plugin-catalog-react': minor +--- + +The `UserListPicker` component has undergone improvements to enhance its performance. + +The previous implementation inferred the number of owned and starred entities based on the entities available in the `EntityListContext`. The updated version no longer relies on the `EntityListContext` for inference, allowing for better decoupling. + +The component now loads the entities' count asynchronously, resulting in improved performance and responsiveness. For this purpose, some of the exported filters such as `EntityTagFilter`, `EntityOwnerFilter`, `EntityLifecycleFilter` and `EntityNamespaceFilter` have now the `getCatalogFilters` method implemented. From db647a72fe9b7f05c9ec6c3f400cedb17b7cb367 Mon Sep 17 00:00:00 2001 From: Vincenzo Scamporlino Date: Tue, 3 Oct 2023 19:26:14 +0200 Subject: [PATCH 24/33] api-docs: fix flaky tests Signed-off-by: Vincenzo Scamporlino --- .../DefaultApiExplorerPage.test.tsx | 57 +++++++++++-------- 1 file changed, 34 insertions(+), 23 deletions(-) diff --git a/plugins/api-docs/src/components/ApiExplorerPage/DefaultApiExplorerPage.test.tsx b/plugins/api-docs/src/components/ApiExplorerPage/DefaultApiExplorerPage.test.tsx index fa698ded58..6d0b191d68 100644 --- a/plugins/api-docs/src/components/ApiExplorerPage/DefaultApiExplorerPage.test.tsx +++ b/plugins/api-docs/src/components/ApiExplorerPage/DefaultApiExplorerPage.test.tsx @@ -52,6 +52,10 @@ describe('DefaultApiExplorerPage', () => { kind: 'API', metadata: { name: 'Entity1', + annotations: { + 'backstage.io/view-url': 'viewurl', + 'backstage.io/edit-url': 'editurl', + }, }, spec: { type: 'openapi' }, }, @@ -63,6 +67,11 @@ describe('DefaultApiExplorerPage', () => { getEntityFacets: async () => ({ facets: { 'relations.ownedBy': [] }, }), + queryEntities: async () => ({ + items: [], + pageInfo: {}, + totalItems: 0, + }), }; const configApi: ConfigApi = new ConfigReader({ @@ -152,37 +161,39 @@ describe('DefaultApiExplorerPage', () => { it('should render the default actions of an item in the grid', async () => { await renderWrapped(); - expect(await screen.findByText(/All apis \(1\)/)).toBeInTheDocument(); - expect(await screen.findByTitle(/View/)).toBeInTheDocument(); - expect(await screen.findByTitle(/View/)).toBeInTheDocument(); - expect(await screen.findByTitle(/Edit/)).toBeInTheDocument(); - expect(await screen.findByTitle(/Add to favorites/)).toBeInTheDocument(); + await waitFor(() => { + expect(screen.getByText(/All apis \(1\)/)).toBeInTheDocument(); + }); + + await waitFor(() => { + expect(screen.getByRole('button', { name: /view/i })).toBeInTheDocument(); + }); + expect(screen.getByRole('button', { name: /edit/i })).toBeInTheDocument(); + expect(screen.getByTitle(/Add to favorites/)).toBeInTheDocument(); }); it('should render the custom actions of an item passed as prop', async () => { const actions: TableProps['actions'] = [ - () => { - return { - icon: () => , - tooltip: 'Foo Action', - disabled: false, - onClick: () => jest.fn(), - }; + { + icon: () => , + tooltip: 'Foo Action', + disabled: false, + onClick: jest.fn(), }, - () => { - return { - icon: () => , - tooltip: 'Bar Action', - disabled: true, - onClick: () => jest.fn(), - }; + { + icon: () => , + tooltip: 'Bar Action', + disabled: true, + onClick: jest.fn(), }, ]; await renderWrapped(); - expect(await screen.findByText(/All apis \(1\)/)).toBeInTheDocument(); - expect(await screen.findByTitle(/Foo Action/)).toBeInTheDocument(); - expect(await screen.findByTitle(/Bar Action/)).toBeInTheDocument(); - expect((await screen.findByTitle(/Bar Action/)).firstChild).toBeDisabled(); + await waitFor(() => { + expect(screen.getByText(/All apis \(1\)/)).toBeInTheDocument(); + }); + expect(screen.getByTitle(/Foo Action/)).toBeInTheDocument(); + expect(screen.getByTitle(/Bar Action/)).toBeInTheDocument(); + expect(screen.getByTitle(/Bar Action/).firstChild).toBeDisabled(); }); }); From 4efc4c1cab820bf8176a425c1df8ec121975139e Mon Sep 17 00:00:00 2001 From: Vincenzo Scamporlino Date: Mon, 9 Oct 2023 14:12:27 +0200 Subject: [PATCH 25/33] catalog-react: fix comments Signed-off-by: Vincenzo Scamporlino --- .../UserListPicker/UserListPicker.test.tsx | 13 ++++++------- plugins/catalog-react/src/filters.ts | 9 +++------ 2 files changed, 9 insertions(+), 13 deletions(-) diff --git a/plugins/catalog-react/src/components/UserListPicker/UserListPicker.test.tsx b/plugins/catalog-react/src/components/UserListPicker/UserListPicker.test.tsx index f0b82b550e..63228717a5 100644 --- a/plugins/catalog-react/src/components/UserListPicker/UserListPicker.test.tsx +++ b/plugins/catalog-react/src/components/UserListPicker/UserListPicker.test.tsx @@ -523,9 +523,7 @@ describe('', () => { return mockQueryEntitiesImplementation(request); }); - render( - , - ); /* picker({ loading: true })*/ + render(); await waitFor(() => expect(mockCatalogApi.queryEntities).toHaveBeenCalledTimes(3), @@ -566,9 +564,7 @@ describe('', () => { return mockQueryEntitiesImplementation(request); }); - render( - , - ); /* picker({ loading: true })*/ + render(); await waitFor(() => expect(mockCatalogApi.queryEntities).toHaveBeenCalledTimes(3), @@ -587,7 +583,10 @@ describe('', () => { await waitFor(() => expect(updateFilters).toHaveBeenLastCalledWith({ - user: EntityUserListFilter.starred(expect.any(Array)), + user: EntityUserListFilter.starred([ + 'component:default/e-1', + 'component:default/e-2', + ]), }), ); }); diff --git a/plugins/catalog-react/src/filters.ts b/plugins/catalog-react/src/filters.ts index b94b538802..74ab771999 100644 --- a/plugins/catalog-react/src/filters.ts +++ b/plugins/catalog-react/src/filters.ts @@ -241,11 +241,11 @@ export class EntityUserListFilter implements EntityFilter { // This is supposed to return always true for paginated // owned entities, since the filters are applied server side. if (this.value === 'owned') { + const relations = getEntityRelations(entity, RELATION_OWNED_BY); + return ( this.refs?.some(v => - getEntityRelations(entity, RELATION_OWNED_BY).some( - o => stringifyEntityRef(o) === v, - ), + relations.some(o => stringifyEntityRef(o) === v), ) ?? false ); } @@ -309,9 +309,6 @@ export class EntityOrphanFilter implements EntityFilter { export class EntityErrorFilter implements EntityFilter { constructor(readonly value: boolean) {} - // TODO(vinzscam): is it possible to implement - // getCatalogFilters? ask mammals - filterEntity(entity: Entity): boolean { const error = ((entity as AlphaEntity)?.status?.items?.length as number) > 0; From 0aa3cecddfb201865d58811f0c3bb45026474509 Mon Sep 17 00:00:00 2001 From: Vincenzo Scamporlino Date: Mon, 9 Oct 2023 14:13:44 +0200 Subject: [PATCH 26/33] catalog: simplify typings Signed-off-by: Vincenzo Scamporlino --- .../CatalogPage/DefaultCatalogPage.test.tsx | 60 ++++++++----------- 1 file changed, 24 insertions(+), 36 deletions(-) diff --git a/plugins/catalog/src/components/CatalogPage/DefaultCatalogPage.test.tsx b/plugins/catalog/src/components/CatalogPage/DefaultCatalogPage.test.tsx index b331c0e477..53f031e263 100644 --- a/plugins/catalog/src/components/CatalogPage/DefaultCatalogPage.test.tsx +++ b/plugins/catalog/src/components/CatalogPage/DefaultCatalogPage.test.tsx @@ -117,43 +117,31 @@ describe('DefaultCatalogPage', () => { ], }, })), - queryEntities: jest.fn().mockImplementation(async request => { - if ( - ( - (request as QueryEntitiesInitialRequest).filter as Record< - string, - string - > - )['relations.ownedBy'] - ) { - // owned entities - return { items: [], totalItems: 3, pageInfo: {} }; - } + queryEntities: jest + .fn() + .mockImplementation(async (request: QueryEntitiesInitialRequest) => { + if ((request.filter as any)['relations.ownedBy']) { + // owned entities + return { items: [], totalItems: 3, pageInfo: {} }; + } - if ( - ( - (request as QueryEntitiesInitialRequest).filter as Record< - string, - string - > - )['metadata.name'] - ) { - // starred entities - return { - items: [ - { - apiVersion: '1', - kind: 'component', - metadata: { name: 'Entity1', namespace: 'default' }, - }, - ], - totalItems: 1, - pageInfo: {}, - }; - } - // all items - return { items: [], totalItems: 2, pageInfo: {} }; - }), + if ((request.filter as any)['metadata.name']) { + // starred entities + return { + items: [ + { + apiVersion: '1', + kind: 'component', + metadata: { name: 'Entity1', namespace: 'default' }, + }, + ], + totalItems: 1, + pageInfo: {}, + }; + } + // all items + return { items: [], totalItems: 2, pageInfo: {} }; + }), }; const testProfile: Partial = { From 2dd206a2fd596e80e2673918e4965fd1599e8461 Mon Sep 17 00:00:00 2001 From: Vincenzo Scamporlino Date: Thu, 12 Oct 2023 13:25:47 +0200 Subject: [PATCH 27/33] catalog-react: fix orphan query Signed-off-by: Vincenzo Scamporlino --- plugins/catalog-react/src/filters.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/plugins/catalog-react/src/filters.ts b/plugins/catalog-react/src/filters.ts index 74ab771999..3d871fe412 100644 --- a/plugins/catalog-react/src/filters.ts +++ b/plugins/catalog-react/src/filters.ts @@ -293,7 +293,10 @@ export class EntityOrphanFilter implements EntityFilter { constructor(readonly value: boolean) {} getCatalogFilters(): Record { - return { 'metadata.annotations.backstage.io/orphan': String(this.value) }; + if (this.value) { + return { 'metadata.annotations.backstage.io/orphan': String(this.value) }; + } + return {}; } filterEntity(entity: Entity): boolean { From 3ea09ae6ebfcc8b5d84ed7da8cc9068165b9053a Mon Sep 17 00:00:00 2001 From: Vincenzo Scamporlino Date: Fri, 20 Oct 2023 08:08:03 +0200 Subject: [PATCH 28/33] catalog: flip the allowed backend filters logic Signed-off-by: Vincenzo Scamporlino --- plugins/catalog-react/src/utils/filters.ts | 31 +++++++++++++++++++--- 1 file changed, 27 insertions(+), 4 deletions(-) diff --git a/plugins/catalog-react/src/utils/filters.ts b/plugins/catalog-react/src/utils/filters.ts index 5ab6e43593..f2cfe64960 100644 --- a/plugins/catalog-react/src/utils/filters.ts +++ b/plugins/catalog-react/src/utils/filters.ts @@ -16,7 +16,16 @@ import { Entity } from '@backstage/catalog-model'; import { EntityFilter } from '../types'; -import { EntityKindFilter, EntityTypeFilter } from '../filters'; +import { + EntityLifecycleFilter, + EntityNamespaceFilter, + EntityOrphanFilter, + EntityOwnerFilter, + EntityTagFilter, + EntityTextFilter, + EntityUserListFilter, + UserListFilter, +} from '../filters'; export function reduceCatalogFilters( filters: EntityFilter[], @@ -29,6 +38,13 @@ export function reduceCatalogFilters( }, {} as Record); } +/** + * This function computes and returns an object containing the filters to be sent + * to the backend. Any filter coming from `EntityKindFilter` and `EntityTypeFilter`, together + * with custom filter set by the adopters is allowed. This function is used by `EntityListProvider` + * and it won't be needed anymore in the future once pagination is implemented, as all the filters + * will be applied backend-side. + */ export function reduceBackendCatalogFilters(filters: EntityFilter[]) { const backendCatalogFilters: Record< string, @@ -37,11 +53,18 @@ export function reduceBackendCatalogFilters(filters: EntityFilter[]) { filters.forEach(filter => { if ( - filter instanceof EntityKindFilter || - filter instanceof EntityTypeFilter + filter instanceof EntityTagFilter || + filter instanceof EntityOwnerFilter || + filter instanceof EntityLifecycleFilter || + filter instanceof EntityNamespaceFilter || + filter instanceof EntityUserListFilter || + filter instanceof EntityOrphanFilter || + filter instanceof EntityTextFilter || + filter instanceof UserListFilter ) { - Object.assign(backendCatalogFilters, filter.getCatalogFilters()); + return; } + Object.assign(backendCatalogFilters, filter.getCatalogFilters?.() || {}); }); return backendCatalogFilters; From 89e0babcb8c6fd7b930a70bc62fc7d69090f70e1 Mon Sep 17 00:00:00 2001 From: Vincenzo Scamporlino Date: Fri, 20 Oct 2023 08:10:11 +0200 Subject: [PATCH 29/33] catalog-react: rename EntityUserListFilter to EntityUserFilter Signed-off-by: Vincenzo Scamporlino --- .../UserListPicker/UserListPicker.test.tsx | 22 +++++++++---------- .../UserListPicker/UserListPicker.tsx | 4 ++-- .../useOwnedEntitiesCount.test.tsx | 10 ++++----- .../UserListPicker/useOwnedEntitiesCount.ts | 4 ++-- .../UserListPicker/useStarredEntitiesCount.ts | 4 ++-- plugins/catalog-react/src/filters.ts | 10 ++++----- .../src/hooks/useEntityListProvider.test.tsx | 6 ++--- .../src/hooks/useEntityListProvider.tsx | 4 ++-- plugins/catalog-react/src/utils/filters.ts | 4 ++-- 9 files changed, 34 insertions(+), 34 deletions(-) diff --git a/plugins/catalog-react/src/components/UserListPicker/UserListPicker.test.tsx b/plugins/catalog-react/src/components/UserListPicker/UserListPicker.test.tsx index 63228717a5..d7572c2ac3 100644 --- a/plugins/catalog-react/src/components/UserListPicker/UserListPicker.test.tsx +++ b/plugins/catalog-react/src/components/UserListPicker/UserListPicker.test.tsx @@ -23,7 +23,7 @@ import { EntityKindFilter, EntityNamespaceFilter, EntityTagFilter, - EntityUserListFilter, + EntityUserFilter, } from '../../filters'; import { CatalogApi, @@ -263,7 +263,7 @@ describe('', () => { await waitFor(() => expect(updateFilters).toHaveBeenLastCalledWith({ - user: EntityUserListFilter.owned(ownershipEntityRefs), + user: EntityUserFilter.owned(ownershipEntityRefs), }), ); @@ -298,7 +298,7 @@ describe('', () => { await waitFor(() => expect(updateFilters).toHaveBeenLastCalledWith({ - user: EntityUserListFilter.starred([ + user: EntityUserFilter.starred([ 'component:default/e-1', 'component:default/e-2', ]), @@ -331,7 +331,7 @@ describe('', () => { await waitFor(() => expect(updateFilters).toHaveBeenLastCalledWith({ - user: EntityUserListFilter.all(), + user: EntityUserFilter.all(), }), ); @@ -352,7 +352,7 @@ describe('', () => { , ); expect(updateFilters).toHaveBeenLastCalledWith({ - user: EntityUserListFilter.owned(ownershipEntityRefs), + user: EntityUserFilter.owned(ownershipEntityRefs), }); }); @@ -437,7 +437,7 @@ describe('', () => { await waitFor(() => expect(updateFilters).toHaveBeenLastCalledWith({ - user: EntityUserListFilter.all(), + user: EntityUserFilter.all(), }), ); }); @@ -501,7 +501,7 @@ describe('', () => { await waitFor(() => expect(updateFilters).toHaveBeenLastCalledWith({ - user: EntityUserListFilter.all(), + user: EntityUserFilter.all(), }), ); }); @@ -529,7 +529,7 @@ describe('', () => { expect(mockCatalogApi.queryEntities).toHaveBeenCalledTimes(3), ); expect(updateFilters).not.toHaveBeenCalledWith({ - user: EntityUserListFilter.all(), + user: EntityUserFilter.all(), }); }); @@ -542,7 +542,7 @@ describe('', () => { await waitFor(() => expect(updateFilters).toHaveBeenLastCalledWith({ - user: EntityUserListFilter.owned(expect.any(Array)), + user: EntityUserFilter.owned(expect.any(Array)), }), ); }); @@ -570,7 +570,7 @@ describe('', () => { expect(mockCatalogApi.queryEntities).toHaveBeenCalledTimes(3), ); expect(updateFilters).not.toHaveBeenCalledWith({ - user: EntityUserListFilter.all(), + user: EntityUserFilter.all(), }); }); @@ -583,7 +583,7 @@ describe('', () => { await waitFor(() => expect(updateFilters).toHaveBeenLastCalledWith({ - user: EntityUserListFilter.starred([ + user: EntityUserFilter.starred([ 'component:default/e-1', 'component:default/e-2', ]), diff --git a/plugins/catalog-react/src/components/UserListPicker/UserListPicker.tsx b/plugins/catalog-react/src/components/UserListPicker/UserListPicker.tsx index b0a86b1322..854a1cb1b7 100644 --- a/plugins/catalog-react/src/components/UserListPicker/UserListPicker.tsx +++ b/plugins/catalog-react/src/components/UserListPicker/UserListPicker.tsx @@ -33,7 +33,7 @@ import { import SettingsIcon from '@material-ui/icons/Settings'; import StarIcon from '@material-ui/icons/Star'; import React, { Fragment, useEffect, useMemo, useState } from 'react'; -import { EntityUserListFilter } from '../../filters'; +import { EntityUserFilter } from '../../filters'; import { useEntityList } from '../../hooks'; import { UserListFilterKind } from '../../types'; import { useOwnedEntitiesCount } from './useOwnedEntitiesCount'; @@ -213,7 +213,7 @@ export const UserListPicker = (props: UserListPickerProps) => { if (selectedUserFilter === 'starred') { return starredEntitiesFilter; } - return EntityUserListFilter.all(); + return EntityUserFilter.all(); }; updateFilters({ user: getFilter() }); diff --git a/plugins/catalog-react/src/components/UserListPicker/useOwnedEntitiesCount.test.tsx b/plugins/catalog-react/src/components/UserListPicker/useOwnedEntitiesCount.test.tsx index 6a79e63c8f..4ce3503394 100644 --- a/plugins/catalog-react/src/components/UserListPicker/useOwnedEntitiesCount.test.tsx +++ b/plugins/catalog-react/src/components/UserListPicker/useOwnedEntitiesCount.test.tsx @@ -32,7 +32,7 @@ import { useOwnedEntitiesCount } from './useOwnedEntitiesCount'; import { EntityNamespaceFilter, EntityOwnerFilter, - EntityUserListFilter, + EntityUserFilter, } from '../../filters'; import { useMountEffect } from '@react-hookz/web'; @@ -95,7 +95,7 @@ describe('useOwnedEntitiesCount', () => { expect(result.current).toEqual({ count: 0, loading: false, - filter: EntityUserListFilter.owned([ + filter: EntityUserFilter.owned([ 'user:default/spiderman', 'user:group/a-group', ]), @@ -131,7 +131,7 @@ describe('useOwnedEntitiesCount', () => { expect(result.current).toEqual({ count: 10, loading: false, - filter: EntityUserListFilter.owned([ + filter: EntityUserFilter.owned([ 'user:default/spiderman', 'user:group/a-group', ]), @@ -162,7 +162,7 @@ describe('useOwnedEntitiesCount', () => { expect(result.current).toEqual({ count: 0, loading: false, - filter: EntityUserListFilter.owned([ + filter: EntityUserFilter.owned([ 'user:default/spiderman', 'user:group/a-group', ]), @@ -202,7 +202,7 @@ describe('useOwnedEntitiesCount', () => { expect(result.current).toEqual({ count: 10, loading: false, - filter: EntityUserListFilter.owned([ + filter: EntityUserFilter.owned([ 'user:default/spiderman', 'user:group/a-group', ]), diff --git a/plugins/catalog-react/src/components/UserListPicker/useOwnedEntitiesCount.ts b/plugins/catalog-react/src/components/UserListPicker/useOwnedEntitiesCount.ts index 605c006f44..d86610d9ae 100644 --- a/plugins/catalog-react/src/components/UserListPicker/useOwnedEntitiesCount.ts +++ b/plugins/catalog-react/src/components/UserListPicker/useOwnedEntitiesCount.ts @@ -20,7 +20,7 @@ import { compact, intersection, isEqual } from 'lodash'; import { useMemo, useRef } from 'react'; import useAsync from 'react-use/lib/useAsync'; import { catalogApiRef } from '../../api'; -import { EntityOwnerFilter, EntityUserListFilter } from '../../filters'; +import { EntityOwnerFilter, EntityUserFilter } from '../../filters'; import { useEntityList } from '../../hooks'; import { reduceCatalogFilters } from '../../utils'; @@ -82,7 +82,7 @@ export function useOwnedEntitiesCount() { const loading = loadingEntityRefs || loadingEntityOwnership; const filter = useMemo( - () => EntityUserListFilter.owned(ownershipEntityRefs ?? []), + () => EntityUserFilter.owned(ownershipEntityRefs ?? []), [ownershipEntityRefs], ); diff --git a/plugins/catalog-react/src/components/UserListPicker/useStarredEntitiesCount.ts b/plugins/catalog-react/src/components/UserListPicker/useStarredEntitiesCount.ts index 93adad0489..6fa49d37a5 100644 --- a/plugins/catalog-react/src/components/UserListPicker/useStarredEntitiesCount.ts +++ b/plugins/catalog-react/src/components/UserListPicker/useStarredEntitiesCount.ts @@ -21,7 +21,7 @@ import { compact, isEqual } from 'lodash'; import { useMemo, useRef } from 'react'; import useAsync from 'react-use/lib/useAsync'; import { catalogApiRef } from '../../api'; -import { EntityUserListFilter } from '../../filters'; +import { EntityUserFilter } from '../../filters'; import { useEntityList, useStarredEntities } from '../../hooks'; import { reduceCatalogFilters } from '../../utils'; @@ -72,7 +72,7 @@ export function useStarredEntitiesCount() { }, [request, starredEntities]); const filter = useMemo( - () => EntityUserListFilter.starred(Array.from(starredEntities)), + () => EntityUserFilter.starred(Array.from(starredEntities)), [starredEntities], ); diff --git a/plugins/catalog-react/src/filters.ts b/plugins/catalog-react/src/filters.ts index 3d871fe412..55983d124b 100644 --- a/plugins/catalog-react/src/filters.ts +++ b/plugins/catalog-react/src/filters.ts @@ -203,22 +203,22 @@ export class EntityNamespaceFilter implements EntityFilter { /** * @public */ -export class EntityUserListFilter implements EntityFilter { +export class EntityUserFilter implements EntityFilter { private constructor( readonly value: UserListFilterKind, readonly refs?: string[], ) {} static owned(ownershipEntityRefs: string[]) { - return new EntityUserListFilter('owned', ownershipEntityRefs); + return new EntityUserFilter('owned', ownershipEntityRefs); } static all() { - return new EntityUserListFilter('all'); + return new EntityUserFilter('all'); } static starred(starredEntityRefs: string[]) { - return new EntityUserListFilter('starred', starredEntityRefs); + return new EntityUserFilter('starred', starredEntityRefs); } getCatalogFilters(): Record { @@ -259,7 +259,7 @@ export class EntityUserListFilter implements EntityFilter { /** * Filters entities based on whatever the user has starred or owns them. - * @deprecated use EntityUserListFilter + * @deprecated use EntityUserFilter * @public */ export class UserListFilter implements EntityFilter { diff --git a/plugins/catalog-react/src/hooks/useEntityListProvider.test.tsx b/plugins/catalog-react/src/hooks/useEntityListProvider.test.tsx index a4401e3441..edbfb65c0c 100644 --- a/plugins/catalog-react/src/hooks/useEntityListProvider.test.tsx +++ b/plugins/catalog-react/src/hooks/useEntityListProvider.test.tsx @@ -35,7 +35,7 @@ import { EntityKindPicker, UserListPicker } from '../components'; import { EntityKindFilter, EntityTypeFilter, - EntityUserListFilter, + EntityUserFilter, } from '../filters'; import { UserListFilterKind } from '../types'; import { EntityListProvider, useEntityList } from './useEntityListProvider'; @@ -155,7 +155,7 @@ describe('', () => { act(() => result.current.updateFilters({ - user: EntityUserListFilter.owned(ownershipEntityRefs), + user: EntityUserFilter.owned(ownershipEntityRefs), }), ); @@ -196,7 +196,7 @@ describe('', () => { act(() => result.current.updateFilters({ - user: EntityUserListFilter.owned(ownershipEntityRefs), + user: EntityUserFilter.owned(ownershipEntityRefs), }), ); diff --git a/plugins/catalog-react/src/hooks/useEntityListProvider.tsx b/plugins/catalog-react/src/hooks/useEntityListProvider.tsx index e2a8cfc657..948efc969f 100644 --- a/plugins/catalog-react/src/hooks/useEntityListProvider.tsx +++ b/plugins/catalog-react/src/hooks/useEntityListProvider.tsx @@ -41,7 +41,7 @@ import { EntityTypeFilter, UserListFilter, EntityNamespaceFilter, - EntityUserListFilter, + EntityUserFilter, } from '../filters'; import { EntityFilter } from '../types'; import { reduceBackendCatalogFilters, reduceEntityFilters } from '../utils'; @@ -51,7 +51,7 @@ import { useApi } from '@backstage/core-plugin-api'; export type DefaultEntityFilters = { kind?: EntityKindFilter; type?: EntityTypeFilter; - user?: UserListFilter | EntityUserListFilter; + user?: UserListFilter | EntityUserFilter; owners?: EntityOwnerFilter; lifecycles?: EntityLifecycleFilter; tags?: EntityTagFilter; diff --git a/plugins/catalog-react/src/utils/filters.ts b/plugins/catalog-react/src/utils/filters.ts index f2cfe64960..580d7b0b76 100644 --- a/plugins/catalog-react/src/utils/filters.ts +++ b/plugins/catalog-react/src/utils/filters.ts @@ -23,7 +23,7 @@ import { EntityOwnerFilter, EntityTagFilter, EntityTextFilter, - EntityUserListFilter, + EntityUserFilter, UserListFilter, } from '../filters'; @@ -57,7 +57,7 @@ export function reduceBackendCatalogFilters(filters: EntityFilter[]) { filter instanceof EntityOwnerFilter || filter instanceof EntityLifecycleFilter || filter instanceof EntityNamespaceFilter || - filter instanceof EntityUserListFilter || + filter instanceof EntityUserFilter || filter instanceof EntityOrphanFilter || filter instanceof EntityTextFilter || filter instanceof UserListFilter From 67ee8f155f74764e119856d2801702b48a59c9be Mon Sep 17 00:00:00 2001 From: Vincenzo Scamporlino Date: Fri, 20 Oct 2023 08:19:36 +0200 Subject: [PATCH 30/33] catalog-react: add clarify comments Signed-off-by: Vincenzo Scamporlino --- .../UserListPicker/useStarredEntitiesCount.ts | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/plugins/catalog-react/src/components/UserListPicker/useStarredEntitiesCount.ts b/plugins/catalog-react/src/components/UserListPicker/useStarredEntitiesCount.ts index 6fa49d37a5..f7b2b101f3 100644 --- a/plugins/catalog-react/src/components/UserListPicker/useStarredEntitiesCount.ts +++ b/plugins/catalog-react/src/components/UserListPicker/useStarredEntitiesCount.ts @@ -41,8 +41,17 @@ export function useStarredEntitiesCount() { const newRequest: QueryEntitiesInitialRequest = { filter: { ...filter, + /** + * here we are filtering entities by `name`. Given this filter, + * the response might contain more entities than expected, in case multiple entities + * of different kind or namespace share the same name. Those extra entities are filtered out + * client side by `EntityUserFilter`, so they won't be visible to the user. + */ [facet]: Array.from(starredEntities).map(e => parseEntityRef(e).name), }, + /** + * limit is set to a high value as we are not expecting many starred entities + */ limit: 1000, }; if (isEqual(newRequest, prevRequest.current)) { @@ -58,6 +67,12 @@ export function useStarredEntitiesCount() { return 0; } + /** + * given a list of starred entity refs and some filters coming from CatalogPage, + * it reduces the list of starred entities, to a list of entities that matches the + * provided filters. It won't be possible to getEntitiesByRefs + * as the method doesn't accept any filter. + */ const response = await catalogApi.queryEntities(request); return response.items From 4078bd73bbf7a9df06c8f3b5a6a169686dff9546 Mon Sep 17 00:00:00 2001 From: Vincenzo Scamporlino Date: Fri, 20 Oct 2023 13:16:46 +0200 Subject: [PATCH 31/33] 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'; From 89748e3c2dc5768cf9f1d6938d6a7acfbd8ee2c9 Mon Sep 17 00:00:00 2001 From: Vincenzo Scamporlino Date: Fri, 20 Oct 2023 15:53:34 +0200 Subject: [PATCH 32/33] catalog: wait for async operation Signed-off-by: Vincenzo Scamporlino --- .../src/components/CatalogPage/DefaultCatalogPage.test.tsx | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/plugins/catalog/src/components/CatalogPage/DefaultCatalogPage.test.tsx b/plugins/catalog/src/components/CatalogPage/DefaultCatalogPage.test.tsx index 53f031e263..0ced712344 100644 --- a/plugins/catalog/src/components/CatalogPage/DefaultCatalogPage.test.tsx +++ b/plugins/catalog/src/components/CatalogPage/DefaultCatalogPage.test.tsx @@ -320,10 +320,9 @@ describe('DefaultCatalogPage', () => { // Now that we've starred an entity, the "Starred" menu option should be // enabled. - expect(screen.getByTestId('user-picker-starred')).not.toHaveAttribute( - 'aria-disabled', - 'true', - ); + expect( + await screen.findByTestId('user-picker-starred'), + ).not.toHaveAttribute('aria-disabled', 'true'); fireEvent.click(screen.getByTestId('user-picker-starred')); await expect( screen.findByText(/Starred components \(1\)/), From ff2131abe3f39249a02f8e3f6bb17be2db2b3fe4 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Fri, 20 Oct 2023 17:46:22 +0200 Subject: [PATCH 33/33] catalog-react: update API report Signed-off-by: Patrik Oldsberg --- plugins/catalog-react/api-report.md | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/plugins/catalog-react/api-report.md b/plugins/catalog-react/api-report.md index e73441f7dc..7422440955 100644 --- a/plugins/catalog-react/api-report.md +++ b/plugins/catalog-react/api-report.md @@ -142,7 +142,7 @@ export const columnFactories: Readonly<{ export type DefaultEntityFilters = { kind?: EntityKindFilter; type?: EntityTypeFilter; - user?: UserListFilter | EntityUserListFilter; + user?: UserListFilter | EntityUserFilter; owners?: EntityOwnerFilter; lifecycles?: EntityLifecycleFilter; tags?: EntityTagFilter; @@ -508,19 +508,19 @@ export interface EntityTypePickerProps { } // @public (undocumented) -export class EntityUserListFilter implements EntityFilter { +export class EntityUserFilter implements EntityFilter { // (undocumented) - static all(): EntityUserListFilter; + static all(): EntityUserFilter; // (undocumented) filterEntity(entity: Entity): boolean; // (undocumented) getCatalogFilters(): Record; // (undocumented) - static owned(ownershipEntityRefs: string[]): EntityUserListFilter; + static owned(ownershipEntityRefs: string[]): EntityUserFilter; // (undocumented) readonly refs?: string[] | undefined; // (undocumented) - static starred(starredEntityRefs: string[]): EntityUserListFilter; + static starred(starredEntityRefs: string[]): EntityUserFilter; // (undocumented) toQueryValue(): string; // (undocumented)