diff --git a/.changeset/healthy-spiders-arrive.md b/.changeset/healthy-spiders-arrive.md new file mode 100644 index 0000000000..e8d110ccaa --- /dev/null +++ b/.changeset/healthy-spiders-arrive.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-org': patch +--- + +Ensure direct relations are shown for User entities while keeping support for aggregating closest parent group ownership diff --git a/plugins/org/api-report.md b/plugins/org/api-report.md index 704401e8d4..d85df7125c 100644 --- a/plugins/org/api-report.md +++ b/plugins/org/api-report.md @@ -30,10 +30,13 @@ export const EntityOwnershipCard: (props: { variant?: InfoCardVariants | undefined; entityFilterKind?: string[] | undefined; hideRelationsToggle?: boolean | undefined; - relationsType?: string | undefined; + relationsType?: EntityRelationAggregation | undefined; entityLimit?: number | undefined; }) => JSX_2.Element; +// @public (undocumented) +export type EntityRelationAggregation = 'direct' | 'aggregated'; + // @public (undocumented) export const EntityUserProfileCard: (props: { variant?: InfoCardVariants | undefined; @@ -77,7 +80,7 @@ export const OwnershipCard: (props: { variant?: InfoCardVariants; entityFilterKind?: string[]; hideRelationsToggle?: boolean; - relationsType?: string; + relationsType?: EntityRelationAggregation; entityLimit?: number; }) => React_2.JSX.Element; diff --git a/plugins/org/package.json b/plugins/org/package.json index 0193a1592b..81e5a2aa6f 100644 --- a/plugins/org/package.json +++ b/plugins/org/package.json @@ -60,6 +60,7 @@ "@testing-library/dom": "^8.0.0", "@testing-library/jest-dom": "^5.10.1", "@testing-library/react": "^12.1.3", + "@testing-library/react-hooks": "^8.0.1", "@testing-library/user-event": "^14.0.0", "@types/node": "^16.11.26", "msw": "^1.0.0" diff --git a/plugins/org/src/components/Cards/OwnershipCard/ComponentsGrid.tsx b/plugins/org/src/components/Cards/OwnershipCard/ComponentsGrid.tsx index 8dab5c4290..2725991d0d 100644 --- a/plugins/org/src/components/Cards/OwnershipCard/ComponentsGrid.tsx +++ b/plugins/org/src/components/Cards/OwnershipCard/ComponentsGrid.tsx @@ -34,6 +34,7 @@ import React from 'react'; import pluralize from 'pluralize'; import { catalogIndexRouteRef } from '../../../routes'; import { useGetEntities } from './useGetEntities'; +import { EntityRelationAggregation } from './types'; const useStyles = makeStyles((theme: BackstageTheme) => createStyles({ @@ -109,13 +110,11 @@ const EntityCountTile = ({ export const ComponentsGrid = ({ entity, relationsType, - isGroup, entityFilterKind, entityLimit = 6, }: { entity: Entity; - relationsType: string; - isGroup: boolean; + relationsType: EntityRelationAggregation; entityFilterKind?: string[]; entityLimit?: number; }) => { @@ -123,7 +122,6 @@ export const ComponentsGrid = ({ const { componentsWithCounters, loading, error } = useGetEntities( entity, relationsType, - isGroup, entityFilterKind, entityLimit, ); diff --git a/plugins/org/src/components/Cards/OwnershipCard/OwnershipCard.test.tsx b/plugins/org/src/components/Cards/OwnershipCard/OwnershipCard.test.tsx index 30b7dee2de..faba9b5627 100644 --- a/plugins/org/src/components/Cards/OwnershipCard/OwnershipCard.test.tsx +++ b/plugins/org/src/components/Cards/OwnershipCard/OwnershipCard.test.tsx @@ -263,10 +263,11 @@ describe('OwnershipCard', () => { }, ); - expect(getByText('OPENAPI').closest('a')).toHaveAttribute( - 'href', - '/create/?filters%5Bkind%5D=api&filters%5Btype%5D=openapi&filters%5Bowners%5D=my-team&filters%5Buser%5D=all', - ); + const href = getByText('OPENAPI').closest('a')?.href ?? ''; + // This env does not support URLSearchParams + const queryParams = decodeURIComponent(href); + + expect(queryParams).toContain('filters[owners]=my-team'); }); it('links to the catalog with the user and groups filters from an user profile', async () => { @@ -299,7 +300,7 @@ describe('OwnershipCard', () => { const { getByText } = await renderInTestApp( - + , { @@ -309,9 +310,11 @@ describe('OwnershipCard', () => { }, ); - expect(getByText('OPENAPI').closest('a')).toHaveAttribute( - 'href', - '/create/?filters%5Bkind%5D=api&filters%5Btype%5D=openapi&filters%5Bowners%5D=user%3Athe-user&filters%5Bowners%5D=my-team&filters%5Bowners%5D=custom%2Fsome-team&filters%5Buser%5D=all', + const href = getByText('OPENAPI').closest('a')?.href ?? ''; + // This env does not support URLSearchParams + const queryParams = decodeURIComponent(href); + expect(queryParams).toMatch( + /filters\[owners\]=custom\/some\-team.*filters\[owners\]=user:the-user/, ); }); diff --git a/plugins/org/src/components/Cards/OwnershipCard/OwnershipCard.tsx b/plugins/org/src/components/Cards/OwnershipCard/OwnershipCard.tsx index d104eb032d..2fb7826e52 100644 --- a/plugins/org/src/components/Cards/OwnershipCard/OwnershipCard.tsx +++ b/plugins/org/src/components/Cards/OwnershipCard/OwnershipCard.tsx @@ -27,6 +27,7 @@ import { } from '@material-ui/core'; import React, { useState } from 'react'; import { ComponentsGrid } from './ComponentsGrid'; +import { EntityRelationAggregation } from './types'; const useStyles = makeStyles(theme => ({ list: { @@ -56,7 +57,7 @@ export const OwnershipCard = (props: { variant?: InfoCardVariants; entityFilterKind?: string[]; hideRelationsToggle?: boolean; - relationsType?: string; + relationsType?: EntityRelationAggregation; entityLimit?: number; }) => { const { @@ -70,7 +71,6 @@ export const OwnershipCard = (props: { hideRelationsToggle === undefined ? false : hideRelationsToggle; const classes = useStyles(); const { entity } = useEntity(); - const isGroup = entity.kind === 'Group'; const [getRelationsType, setRelationsType] = useState( relationsType || 'direct', ); @@ -102,7 +102,6 @@ export const OwnershipCard = (props: { } name="pin" inputProps={{ 'aria-label': 'Ownership Type Switch' }} - disabled={!isGroup} /> Aggregated Relations @@ -114,7 +113,6 @@ export const OwnershipCard = (props: { entity={entity} entityLimit={entityLimit} relationsType={getRelationsType} - isGroup={isGroup} entityFilterKind={entityFilterKind} /> diff --git a/plugins/org/src/components/Cards/OwnershipCard/index.ts b/plugins/org/src/components/Cards/OwnershipCard/index.ts index dc0cef0226..a4798cc4bf 100644 --- a/plugins/org/src/components/Cards/OwnershipCard/index.ts +++ b/plugins/org/src/components/Cards/OwnershipCard/index.ts @@ -14,3 +14,4 @@ * limitations under the License. */ export * from './OwnershipCard'; +export * from './types'; diff --git a/plugins/org/src/components/Cards/OwnershipCard/types.ts b/plugins/org/src/components/Cards/OwnershipCard/types.ts new file mode 100644 index 0000000000..c1e95ee94e --- /dev/null +++ b/plugins/org/src/components/Cards/OwnershipCard/types.ts @@ -0,0 +1,18 @@ +/* + * 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. + */ + +/** @public */ +export type EntityRelationAggregation = 'direct' | 'aggregated'; diff --git a/plugins/org/src/components/Cards/OwnershipCard/useGetEntities.test.ts b/plugins/org/src/components/Cards/OwnershipCard/useGetEntities.test.ts new file mode 100644 index 0000000000..191bd10284 --- /dev/null +++ b/plugins/org/src/components/Cards/OwnershipCard/useGetEntities.test.ts @@ -0,0 +1,150 @@ +/* + * 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 { CompoundEntityRef, Entity } from '@backstage/catalog-model'; +import { useGetEntities } from './useGetEntities'; +import { CatalogApi } from '@backstage/catalog-client'; +import { renderHook } from '@testing-library/react-hooks'; +import { getEntityRelations } from '@backstage/plugin-catalog-react'; + +const givenParentGroup = 'team.squad1'; +const givenLeafGroup = 'team.squad2'; +const givenUser = 'user.john'; +const givenParentGroupEntity = { + kind: 'Group', + metadata: { + name: givenParentGroup, + }, +} as Partial as Entity; +const givenLeafGroupEntity = { + kind: 'Group', + metadata: { + name: givenLeafGroup, + }, +} as Partial as Entity; +const givenUserEntity = { + kind: 'User', + metadata: { + name: givenUser, + }, +} as Partial as Entity; + +const catalogApiMock: Pick = { + getEntities: jest.fn(async () => Promise.resolve({ items: [] })), + getEntitiesByRefs: jest.fn(async ({ entityRefs: [ref] }) => + ref.includes(givenParentGroup) + ? { items: [givenParentGroupEntity] } + : { items: [givenLeafGroupEntity] }, + ), +}; + +jest.mock('@backstage/core-plugin-api', () => ({ + useApi: jest.fn(() => catalogApiMock), +})); +jest.mock('@backstage/plugin-catalog-react', () => ({ + catalogApiRef: {}, + getEntityRelations: jest.fn(entity => { + if (entity?.metadata.name === givenParentGroup) { + return [ + { + kind: 'Group', + namespace: 'default', + name: givenLeafGroup, + } as CompoundEntityRef, + ]; + } else if (entity?.kind === 'User') { + return [ + { + kind: 'Group', + namespace: 'default', + name: givenLeafGroup, + } as CompoundEntityRef, + ]; + } + + return []; + }) as typeof getEntityRelations, +})); + +describe('useGetEntities', () => { + const ownersFilter = (...owners: string[]) => + expect.objectContaining({ + filter: expect.arrayContaining([ + expect.objectContaining({ + 'relations.ownedBy': owners, + }), + ]), + }); + + describe('given aggregated relationsType', () => { + const whenHookIsCalledWith = async (_entity: Entity) => { + const hook = renderHook( + ({ entity }) => useGetEntities(entity, 'aggregated'), + { + initialProps: { entity: _entity }, + }, + ); + + await hook.waitForNextUpdate(); + }; + + it('given group entity should aggregate child ownership', async () => { + await whenHookIsCalledWith(givenParentGroupEntity); + expect(catalogApiMock.getEntities).toHaveBeenCalledWith( + ownersFilter( + `group:default/${givenParentGroup}`, + `group:default/${givenLeafGroup}`, + ), + ); + }); + + it('given user entity should aggregate parent ownership and direct', async () => { + await whenHookIsCalledWith(givenUserEntity); + expect(catalogApiMock.getEntities).toHaveBeenCalledWith( + ownersFilter( + `group:default/${givenLeafGroup}`, + `user:default/${givenUser}`, + ), + ); + }); + }); + + describe('given direct relationsType', () => { + const whenHookIsCalledWith = async (_entity: Entity) => { + const hook = renderHook( + ({ entity }) => useGetEntities(entity, 'direct'), + { + initialProps: { entity: _entity }, + }, + ); + + await hook.waitForNextUpdate(); + }; + + it('given group entity should return directly owned entities', async () => { + await whenHookIsCalledWith(givenLeafGroupEntity); + expect(catalogApiMock.getEntities).toHaveBeenCalledWith( + ownersFilter(`group:default/${givenLeafGroup}`), + ); + }); + + it('given user entity should return directly owned entities', async () => { + await whenHookIsCalledWith(givenUserEntity); + expect(catalogApiMock.getEntities).toHaveBeenCalledWith( + ownersFilter(`user:default/${givenUser}`), + ); + }); + }); +}); diff --git a/plugins/org/src/components/Cards/OwnershipCard/useGetEntities.ts b/plugins/org/src/components/Cards/OwnershipCard/useGetEntities.ts index a59f534300..1ff01cd8e1 100644 --- a/plugins/org/src/components/Cards/OwnershipCard/useGetEntities.ts +++ b/plugins/org/src/components/Cards/OwnershipCard/useGetEntities.ts @@ -31,6 +31,7 @@ import limiterFactory from 'p-limit'; import { useApi } from '@backstage/core-plugin-api'; import useAsync from 'react-use/lib/useAsync'; import qs from 'qs'; +import { EntityRelationAggregation as EntityRelationsAggregation } from './types'; const limiter = limiterFactory(10); @@ -57,78 +58,109 @@ const getQueryParams = ( return qs.stringify({ filters }, { arrayFormat: 'repeat' }); }; -const getOwnersEntityRef = (owner: Entity): string[] => { - let owners = [stringifyEntityRef(owner)]; - if (owner.kind === 'User') { - const ownerGroups = getEntityRelations(owner, RELATION_MEMBER_OF, { - kind: 'Group', +const getMemberOfEntityRefs = (owner: Entity): string[] => { + const parentGroups = getEntityRelations(owner, RELATION_MEMBER_OF, { + kind: 'Group', + }); + + const ownerGroupsNames = parentGroups.map(({ kind, namespace, name }) => + stringifyEntityRef({ + kind, + namespace, + name, + }), + ); + + return [...ownerGroupsNames, stringifyEntityRef(owner)]; +}; + +const isEntity = (entity: Entity | undefined): entity is Entity => + entity !== undefined; + +const getChildOwnershipEntityRefs = async ( + entity: Entity, + catalogApi: CatalogApi, +): Promise => { + const childGroups = getEntityRelations(entity, RELATION_PARENT_OF, { + kind: 'Group', + }); + + const hasChildGroups = childGroups.length > 0; + + if (hasChildGroups) { + const entityRefs = childGroups.map(r => stringifyEntityRef(r)); + const childGroupResponse = await catalogApi.getEntitiesByRefs({ + fields: ['kind', 'metadata.namespace', 'metadata.name'], + entityRefs, }); - const ownerGroupsName = ownerGroups.map(ownerGroup => - stringifyEntityRef({ - kind: ownerGroup.kind, - namespace: ownerGroup.namespace, - name: ownerGroup.name, - }), - ); - owners = [...owners, ...ownerGroupsName]; + const childGroupEntities = childGroupResponse.items.filter(isEntity); + + return ( + await Promise.all( + childGroupEntities.map(childGroupEntity => + limiter(() => + getChildOwnershipEntityRefs(childGroupEntity, catalogApi), + ), + ), + ) + ).flatMap(aggregated => aggregated); } + + return [stringifyEntityRef(entity)]; +}; + +const getOwners = async ( + entity: Entity, + relations: EntityRelationsAggregation, + catalogApi: CatalogApi, +): Promise => { + const isGroup = entity.kind === 'Group'; + const isAggregated = relations === 'aggregated'; + const isUserEntity = entity.kind === 'User'; + + const owners: string[] = []; + + if (isAggregated && isGroup) { + const childEntityRefs = await getChildOwnershipEntityRefs( + entity, + catalogApi, + ); + owners.push(stringifyEntityRef(entity)); + owners.push.apply(owners, childEntityRefs); + } else if (isAggregated && isUserEntity) { + const parentEntityRefs = getMemberOfEntityRefs(entity); + owners.push.apply(owners, parentEntityRefs); + } else { + owners.push(stringifyEntityRef(entity)); + } + return owners; }; -const getAggregatedOwnersEntityRef = async ( - parentGroup: Entity, +const getOwnedEntitiesByOwners = ( + owners: string[], + kinds: string[], catalogApi: CatalogApi, -): Promise => { - const requestedEntities: Entity[] = []; - const outstandingEntities = new Map>(); - const processedEntities = new Set(); - requestedEntities.push(parentGroup); - let currentEntity = parentGroup; - - while (requestedEntities.length > 0) { - const childRelations = getEntityRelations( - currentEntity, - RELATION_PARENT_OF, +) => + catalogApi.getEntities({ + filter: [ { - kind: 'Group', + kind: kinds, + 'relations.ownedBy': owners, }, - ); - - await Promise.all( - childRelations.map(childGroup => - limiter(async () => { - const promise = catalogApi.getEntityByRef(childGroup); - outstandingEntities.set(childGroup.name, promise); - try { - const processedEntity = await promise; - if (processedEntity) { - requestedEntities.push(processedEntity); - } - } finally { - outstandingEntities.delete(childGroup.name); - } - }), - ), - ); - requestedEntities.shift(); - processedEntities.add( - stringifyEntityRef({ - kind: currentEntity.kind, - namespace: currentEntity.metadata.namespace, - name: currentEntity.metadata.name, - }), - ); - // always set currentEntity to the first element of array requestedEntities - currentEntity = requestedEntities[0]; - } - - return Array.from(processedEntities); -}; + ], + fields: [ + 'kind', + 'metadata.name', + 'metadata.namespace', + 'spec.type', + 'relations', + ], + }); export function useGetEntities( entity: Entity, - relationsType: string, - isGroup: boolean, + relations: EntityRelationsAggregation, entityFilterKind?: string[], entityLimit = 6, ): { @@ -151,25 +183,13 @@ export function useGetEntities( error, value: componentsWithCounters, } = useAsync(async () => { - const owners = - relationsType === 'aggregated' && isGroup - ? await getAggregatedOwnersEntityRef(entity, catalogApi) - : getOwnersEntityRef(entity); - const ownedEntitiesList = await catalogApi.getEntities({ - filter: [ - { - kind: kinds, - 'relations.ownedBy': owners, - }, - ], - fields: [ - 'kind', - 'metadata.name', - 'metadata.namespace', - 'spec.type', - 'relations', - ], - }); + const owners = await getOwners(entity, relations, catalogApi); + + const ownedEntitiesList = await getOwnedEntitiesByOwners( + owners, + kinds, + catalogApi, + ); const counts = ownedEntitiesList.items.reduce( (acc: EntityTypeProps[], ownedEntity) => { @@ -204,7 +224,7 @@ export function useGetEntities( kind: string; queryParams: string; }>; - }, [catalogApi, entity, relationsType]); + }, [catalogApi, entity, relations]); return { componentsWithCounters, diff --git a/plugins/org/src/setupTests.ts b/plugins/org/src/setupTests.ts index 28a35d2b06..09cb508b22 100644 --- a/plugins/org/src/setupTests.ts +++ b/plugins/org/src/setupTests.ts @@ -14,3 +14,7 @@ * limitations under the License. */ import '@testing-library/jest-dom'; + +beforeEach(() => { + jest.clearAllMocks(); +}); diff --git a/yarn.lock b/yarn.lock index fc584e3110..363d1b26cf 100644 --- a/yarn.lock +++ b/yarn.lock @@ -7918,6 +7918,7 @@ __metadata: "@testing-library/dom": ^8.0.0 "@testing-library/jest-dom": ^5.10.1 "@testing-library/react": ^12.1.3 + "@testing-library/react-hooks": ^8.0.1 "@testing-library/user-event": ^14.0.0 "@types/node": ^16.11.26 "@types/react": ^16.13.1 || ^17.0.0