From 0e797b37f4a5fa74d00bd0065aad73dfe140e55c Mon Sep 17 00:00:00 2001 From: Brian Phillips <28457+brianphillips@users.noreply.github.com> Date: Wed, 29 May 2024 10:04:09 -0500 Subject: [PATCH 1/6] Add ability to specify a different relationship name for MembersListCard The default relationship remains `memberOf` but could be overridden to specify (for instance) `leaderOf` if you wanted to have multiple MembersListCard components on a `Group` page (one for "Members" and another for "Leaders") Signed-off-by: Brian Phillips <28457+brianphillips@users.noreply.github.com> --- .../MembersList/MembersListCard.test.tsx | 30 +++++++++++++++++++ .../Group/MembersList/MembersListCard.tsx | 4 ++- 2 files changed, 33 insertions(+), 1 deletion(-) diff --git a/plugins/org/src/components/Cards/Group/MembersList/MembersListCard.test.tsx b/plugins/org/src/components/Cards/Group/MembersList/MembersListCard.test.tsx index 793ffe274b..539b0bd892 100644 --- a/plugins/org/src/components/Cards/Group/MembersList/MembersListCard.test.tsx +++ b/plugins/org/src/components/Cards/Group/MembersList/MembersListCard.test.tsx @@ -99,6 +99,7 @@ describe('MemberTab Test', () => { ] as Entity[], }), }; + const getEntitiesSpy = jest.spyOn(catalogApi, 'getEntities'); it('Display Profile Card', async () => { await renderInTestApp( @@ -115,6 +116,12 @@ describe('MemberTab Test', () => { }, }, ); + expect(getEntitiesSpy).toHaveBeenCalledWith({ + filter: { + kind: 'User', + 'relations.memberOf': ['group:default/team-d'], + }, + }); expect(screen.getByAltText('Tara MacGovern')).toHaveAttribute( 'src', @@ -149,6 +156,29 @@ describe('MemberTab Test', () => { expect(screen.getByText('Testers (1)')).toBeInTheDocument(); }); + it('Can query a different relationship', async () => { + await renderInTestApp( + + + + + , + { + mountedRoutes: { + '/catalog/:namespace/:kind/:name': entityRouteRef, + '/catalog': rootRouteRef, + }, + }, + ); + + expect(getEntitiesSpy).toHaveBeenCalledWith({ + filter: { + kind: 'User', + 'relations.leaderOf': ['group:default/team-d'], + }, + }); + }); + describe('Aggregate members toggle', () => { it('Does not show the aggregate members toggle if the showAggregateMembersToggle prop is undefined', async () => { await renderInTestApp( diff --git a/plugins/org/src/components/Cards/Group/MembersList/MembersListCard.tsx b/plugins/org/src/components/Cards/Group/MembersList/MembersListCard.tsx index 418a83f5b2..b21ae20c23 100644 --- a/plugins/org/src/components/Cards/Group/MembersList/MembersListCard.tsx +++ b/plugins/org/src/components/Cards/Group/MembersList/MembersListCard.tsx @@ -138,12 +138,14 @@ export const MembersListCard = (props: { memberDisplayTitle?: string; pageSize?: number; showAggregateMembersToggle?: boolean; + relationship?: string; relationsType?: EntityRelationAggregation; }) => { const { memberDisplayTitle = 'Members', pageSize = 50, showAggregateMembersToggle, + relationship = 'memberOf', relationsType = 'direct', } = props; const classes = useListStyles(); @@ -187,7 +189,7 @@ export const MembersListCard = (props: { const membersList = await catalogApi.getEntities({ filter: { kind: 'User', - 'relations.memberof': [ + [`relations.${relationship}`]: [ stringifyEntityRef({ kind: 'group', namespace: groupNamespace.toLocaleLowerCase('en-US'), From c307ef471a3d9868395213d4b69eb3c444b4039e Mon Sep 17 00:00:00 2001 From: Brian Phillips <28457+brianphillips@users.noreply.github.com> Date: Wed, 29 May 2024 10:29:23 -0500 Subject: [PATCH 2/6] add changeset Signed-off-by: Brian Phillips <28457+brianphillips@users.noreply.github.com> --- .changeset/little-games-fail.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/little-games-fail.md diff --git a/.changeset/little-games-fail.md b/.changeset/little-games-fail.md new file mode 100644 index 0000000000..dc5d2f7795 --- /dev/null +++ b/.changeset/little-games-fail.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-org': patch +--- + +Added relationship option to EntityMembersListCard component From 2644eeccb955fcf0b6e26f9df1c6b6f3e6d88afc Mon Sep 17 00:00:00 2001 From: Brian Phillips <28457+brianphillips@users.noreply.github.com> Date: Wed, 29 May 2024 13:40:23 -0500 Subject: [PATCH 3/6] Add API report Signed-off-by: Brian Phillips <28457+brianphillips@users.noreply.github.com> --- plugins/org/api-report.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/plugins/org/api-report.md b/plugins/org/api-report.md index 1960ce839b..fe641e5a8b 100644 --- a/plugins/org/api-report.md +++ b/plugins/org/api-report.md @@ -23,6 +23,7 @@ export const EntityMembersListCard: (props: { memberDisplayTitle?: string | undefined; pageSize?: number | undefined; showAggregateMembersToggle?: boolean | undefined; + relationship?: string | undefined; relationsType?: EntityRelationAggregation | undefined; }) => JSX_2.Element; @@ -55,6 +56,7 @@ export const MembersListCard: (props: { memberDisplayTitle?: string; pageSize?: number; showAggregateMembersToggle?: boolean; + relationship?: string; relationsType?: EntityRelationAggregation; }) => React_2.JSX.Element; From 05ad87ed3aff16d87aa65a62972b0bc529457770 Mon Sep 17 00:00:00 2001 From: Brian Phillips <28457+brianphillips@users.noreply.github.com> Date: Wed, 29 May 2024 14:37:35 -0500 Subject: [PATCH 4/6] fix case mismatch in relationship name Signed-off-by: Brian Phillips <28457+brianphillips@users.noreply.github.com> --- .../Group/MembersList/MembersListCard.test.tsx | 4 ++-- .../Group/MembersList/MembersListCard.tsx | 5 +++-- plugins/org/src/helpers/helpers.ts | 18 +++++++++++------- 3 files changed, 16 insertions(+), 11 deletions(-) diff --git a/plugins/org/src/components/Cards/Group/MembersList/MembersListCard.test.tsx b/plugins/org/src/components/Cards/Group/MembersList/MembersListCard.test.tsx index 539b0bd892..3a129348c7 100644 --- a/plugins/org/src/components/Cards/Group/MembersList/MembersListCard.test.tsx +++ b/plugins/org/src/components/Cards/Group/MembersList/MembersListCard.test.tsx @@ -119,7 +119,7 @@ describe('MemberTab Test', () => { expect(getEntitiesSpy).toHaveBeenCalledWith({ filter: { kind: 'User', - 'relations.memberOf': ['group:default/team-d'], + 'relations.memberof': ['group:default/team-d'], }, }); @@ -174,7 +174,7 @@ describe('MemberTab Test', () => { expect(getEntitiesSpy).toHaveBeenCalledWith({ filter: { kind: 'User', - 'relations.leaderOf': ['group:default/team-d'], + 'relations.leaderof': ['group:default/team-d'], }, }); }); diff --git a/plugins/org/src/components/Cards/Group/MembersList/MembersListCard.tsx b/plugins/org/src/components/Cards/Group/MembersList/MembersListCard.tsx index b21ae20c23..d1e74200fd 100644 --- a/plugins/org/src/components/Cards/Group/MembersList/MembersListCard.tsx +++ b/plugins/org/src/components/Cards/Group/MembersList/MembersListCard.tsx @@ -145,7 +145,7 @@ export const MembersListCard = (props: { memberDisplayTitle = 'Members', pageSize = 50, showAggregateMembersToggle, - relationship = 'memberOf', + relationship = 'memberof', relationsType = 'direct', } = props; const classes = useListStyles(); @@ -179,6 +179,7 @@ export const MembersListCard = (props: { return await getAllDesendantMembersForGroupEntity( groupEntity, catalogApi, + relationship, ); }, [catalogApi, groupEntity, showAggregateMembers]); const { @@ -189,7 +190,7 @@ export const MembersListCard = (props: { const membersList = await catalogApi.getEntities({ filter: { kind: 'User', - [`relations.${relationship}`]: [ + [`relations.${relationship.toLocaleLowerCase('en-US')}`]: [ stringifyEntityRef({ kind: 'group', namespace: groupNamespace.toLocaleLowerCase('en-US'), diff --git a/plugins/org/src/helpers/helpers.ts b/plugins/org/src/helpers/helpers.ts index 7da4ee32d8..465f89afa1 100644 --- a/plugins/org/src/helpers/helpers.ts +++ b/plugins/org/src/helpers/helpers.ts @@ -31,6 +31,7 @@ import { export const getMembersFromGroups = async ( groups: CompoundEntityRef[], catalogApi: CatalogApi, + relationship = 'memberof', ) => { const membersList = groups.length === 0 @@ -38,13 +39,14 @@ export const getMembersFromGroups = async ( : await catalogApi.getEntities({ filter: { kind: 'User', - 'relations.memberof': groups.map(group => - stringifyEntityRef({ - kind: 'group', - namespace: group.namespace.toLocaleLowerCase('en-US'), - name: group.name.toLocaleLowerCase('en-US'), - }), - ), + [`relations.${relationship.toLocaleLowerCase('en-US')}`]: + groups.map(group => + stringifyEntityRef({ + kind: 'group', + namespace: group.namespace.toLocaleLowerCase('en-US'), + name: group.name.toLocaleLowerCase('en-US'), + }), + ), }, }); @@ -99,10 +101,12 @@ export const getDescendantGroupsFromGroup = async ( export const getAllDesendantMembersForGroupEntity = async ( groupEntity: GroupEntity, catalogApi: CatalogApi, + relationship = 'memberof', ) => getMembersFromGroups( await getDescendantGroupsFromGroup(groupEntity, catalogApi), catalogApi, + relationship, ); export const removeDuplicateEntitiesFrom = (entityArray: Entity[]) => { From 0391e693babafe46b9d25b86862ac6003b1890f6 Mon Sep 17 00:00:00 2001 From: Brian Phillips <28457+brianphillips@users.noreply.github.com> Date: Thu, 6 Jun 2024 06:37:35 -0500 Subject: [PATCH 5/6] use more sensible property for specifying the relationship type In the process, the `relationsType` prop is deprecated and renamed to `relationAggregation`. Signed-off-by: Brian Phillips <28457+brianphillips@users.noreply.github.com> --- .../Group/MembersList/MembersListCard.test.tsx | 6 +++--- .../Cards/Group/MembersList/MembersListCard.tsx | 15 +++++++++------ 2 files changed, 12 insertions(+), 9 deletions(-) diff --git a/plugins/org/src/components/Cards/Group/MembersList/MembersListCard.test.tsx b/plugins/org/src/components/Cards/Group/MembersList/MembersListCard.test.tsx index 3a129348c7..69d6090687 100644 --- a/plugins/org/src/components/Cards/Group/MembersList/MembersListCard.test.tsx +++ b/plugins/org/src/components/Cards/Group/MembersList/MembersListCard.test.tsx @@ -160,7 +160,7 @@ describe('MemberTab Test', () => { await renderInTestApp( - + , { @@ -376,7 +376,7 @@ describe('MemberTab Test', () => { @@ -414,7 +414,7 @@ describe('MemberTab Test', () => { - + diff --git a/plugins/org/src/components/Cards/Group/MembersList/MembersListCard.tsx b/plugins/org/src/components/Cards/Group/MembersList/MembersListCard.tsx index d1e74200fd..f27b071114 100644 --- a/plugins/org/src/components/Cards/Group/MembersList/MembersListCard.tsx +++ b/plugins/org/src/components/Cards/Group/MembersList/MembersListCard.tsx @@ -138,16 +138,19 @@ export const MembersListCard = (props: { memberDisplayTitle?: string; pageSize?: number; showAggregateMembersToggle?: boolean; - relationship?: string; + relationType?: string; + /** @deprecated Please use `relationAggregation` instead */ relationsType?: EntityRelationAggregation; + relationAggregation?: EntityRelationAggregation; }) => { const { memberDisplayTitle = 'Members', pageSize = 50, showAggregateMembersToggle, - relationship = 'memberof', - relationsType = 'direct', + relationType = 'memberof', } = props; + const relationAggregation = + props.relationAggregation ?? props.relationsType ?? 'direct'; const classes = useListStyles(); const { entity: groupEntity } = useEntity(); @@ -167,7 +170,7 @@ export const MembersListCard = (props: { }; const [showAggregateMembers, setShowAggregateMembers] = useState( - relationsType === 'aggregated', + relationAggregation === 'aggregated', ); const { loading: loadingDescendantMembers, value: descendantMembers } = @@ -179,7 +182,7 @@ export const MembersListCard = (props: { return await getAllDesendantMembersForGroupEntity( groupEntity, catalogApi, - relationship, + relationType, ); }, [catalogApi, groupEntity, showAggregateMembers]); const { @@ -190,7 +193,7 @@ export const MembersListCard = (props: { const membersList = await catalogApi.getEntities({ filter: { kind: 'User', - [`relations.${relationship.toLocaleLowerCase('en-US')}`]: [ + [`relations.${relationType.toLocaleLowerCase('en-US')}`]: [ stringifyEntityRef({ kind: 'group', namespace: groupNamespace.toLocaleLowerCase('en-US'), From 4a99e6463d2c9cb8786bad750b1915b89c2b0c8a Mon Sep 17 00:00:00 2001 From: Brian Phillips <28457+brianphillips@users.noreply.github.com> Date: Thu, 6 Jun 2024 07:05:29 -0500 Subject: [PATCH 6/6] deprecate all instances of the relationsType property in favor of relationAggregation Signed-off-by: Brian Phillips <28457+brianphillips@users.noreply.github.com> --- .changeset/little-games-fail.md | 4 ++- plugins/org/api-report.md | 8 +++-- .../Cards/OwnershipCard/ComponentsGrid.tsx | 12 +++++-- .../OwnershipCard/OwnershipCard.test.tsx | 2 +- .../Cards/OwnershipCard/OwnershipCard.tsx | 31 +++++++++++-------- .../OwnershipCard/useGetEntities.test.ts | 4 +-- .../Cards/OwnershipCard/useGetEntities.ts | 10 +++--- 7 files changed, 45 insertions(+), 26 deletions(-) diff --git a/.changeset/little-games-fail.md b/.changeset/little-games-fail.md index dc5d2f7795..199f983be8 100644 --- a/.changeset/little-games-fail.md +++ b/.changeset/little-games-fail.md @@ -2,4 +2,6 @@ '@backstage/plugin-org': patch --- -Added relationship option to EntityMembersListCard component +Added `relationType` property to EntityMembersListCard component that allows for display users related to a group via some other relationship aside from `memberOf`. + +Also, as a side effect, the `relationsType` property has been deprecated in favor of a more accurately named `relationAggregation` property. diff --git a/plugins/org/api-report.md b/plugins/org/api-report.md index fe641e5a8b..884832c111 100644 --- a/plugins/org/api-report.md +++ b/plugins/org/api-report.md @@ -23,8 +23,9 @@ export const EntityMembersListCard: (props: { memberDisplayTitle?: string | undefined; pageSize?: number | undefined; showAggregateMembersToggle?: boolean | undefined; - relationship?: string | undefined; + relationType?: string | undefined; relationsType?: EntityRelationAggregation | undefined; + relationAggregation?: EntityRelationAggregation | undefined; }) => JSX_2.Element; // @public (undocumented) @@ -33,6 +34,7 @@ export const EntityOwnershipCard: (props: { entityFilterKind?: string[] | undefined; hideRelationsToggle?: boolean | undefined; relationsType?: EntityRelationAggregation | undefined; + relationAggregation?: EntityRelationAggregation | undefined; entityLimit?: number | undefined; }) => JSX_2.Element; @@ -56,8 +58,9 @@ export const MembersListCard: (props: { memberDisplayTitle?: string; pageSize?: number; showAggregateMembersToggle?: boolean; - relationship?: string; + relationType?: string; relationsType?: EntityRelationAggregation; + relationAggregation?: EntityRelationAggregation; }) => React_2.JSX.Element; // @public @@ -84,6 +87,7 @@ export const OwnershipCard: (props: { entityFilterKind?: string[]; hideRelationsToggle?: boolean; relationsType?: EntityRelationAggregation; + relationAggregation?: EntityRelationAggregation; entityLimit?: number; }) => React_2.JSX.Element; diff --git a/plugins/org/src/components/Cards/OwnershipCard/ComponentsGrid.tsx b/plugins/org/src/components/Cards/OwnershipCard/ComponentsGrid.tsx index ff16a4ab2d..586e612e94 100644 --- a/plugins/org/src/components/Cards/OwnershipCard/ComponentsGrid.tsx +++ b/plugins/org/src/components/Cards/OwnershipCard/ComponentsGrid.tsx @@ -107,19 +107,27 @@ export const ComponentsGrid = ({ className, entity, relationsType, + relationAggregation, entityFilterKind, entityLimit = 6, }: { className?: string; entity: Entity; - relationsType: EntityRelationAggregation; + /** @deprecated Please use relationAggregation instead */ + relationsType?: EntityRelationAggregation; + relationAggregation?: EntityRelationAggregation; entityFilterKind?: string[]; entityLimit?: number; }) => { const catalogLink = useRouteRef(catalogIndexRouteRef); + if (!relationsType && !relationAggregation) { + throw new Error( + 'The relationAggregation property must be set as an EntityRelationAggregation type.', + ); + } const { componentsWithCounters, loading, error } = useGetEntities( entity, - relationsType, + (relationAggregation ?? relationsType)!, // we can safely use the non-null assertion here because of the run-time check above 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 f2b949f768..004fb8b964 100644 --- a/plugins/org/src/components/Cards/OwnershipCard/OwnershipCard.test.tsx +++ b/plugins/org/src/components/Cards/OwnershipCard/OwnershipCard.test.tsx @@ -288,7 +288,7 @@ describe('OwnershipCard', () => { const { getByText } = await renderInTestApp( - + , { diff --git a/plugins/org/src/components/Cards/OwnershipCard/OwnershipCard.tsx b/plugins/org/src/components/Cards/OwnershipCard/OwnershipCard.tsx index a5b0b7bb19..fce47845bc 100644 --- a/plugins/org/src/components/Cards/OwnershipCard/OwnershipCard.tsx +++ b/plugins/org/src/components/Cards/OwnershipCard/OwnershipCard.tsx @@ -67,31 +67,34 @@ export const OwnershipCard = (props: { variant?: InfoCardVariants; entityFilterKind?: string[]; hideRelationsToggle?: boolean; + /** @deprecated Please use relationAggregation instead */ relationsType?: EntityRelationAggregation; + relationAggregation?: EntityRelationAggregation; entityLimit?: number; }) => { const { variant, entityFilterKind, hideRelationsToggle, - relationsType, entityLimit = 6, } = props; + const relationAggregation = props.relationAggregation ?? props.relationsType; const relationsToggle = hideRelationsToggle === undefined ? false : hideRelationsToggle; const classes = useStyles(); const { entity } = useEntity(); - const defaultRelationsType = entity.kind === 'User' ? 'aggregated' : 'direct'; - const [getRelationsType, setRelationsType] = useState( - relationsType ?? defaultRelationsType, + const defaultRelationAggregation = + entity.kind === 'User' ? 'aggregated' : 'direct'; + const [getRelationAggregation, setRelationAggregation] = useState( + relationAggregation ?? defaultRelationAggregation, ); useEffect(() => { - if (!relationsType) { - setRelationsType(defaultRelationsType); + if (!relationAggregation) { + setRelationAggregation(defaultRelationAggregation); } - }, [setRelationsType, defaultRelationsType, relationsType]); + }, [setRelationAggregation, defaultRelationAggregation, relationAggregation]); return ( { - const updatedRelationsType = - getRelationsType === 'direct' ? 'aggregated' : 'direct'; - setRelationsType(updatedRelationsType); + const updatedRelationAggregation = + getRelationAggregation === 'direct' + ? 'aggregated' + : 'direct'; + setRelationAggregation(updatedRelationAggregation); }} name="pin" inputProps={{ 'aria-label': 'Ownership Type Switch' }} @@ -136,7 +141,7 @@ export const OwnershipCard = (props: { className={classes.grid} entity={entity} entityLimit={entityLimit} - relationsType={getRelationsType} + relationAggregation={getRelationAggregation} entityFilterKind={entityFilterKind} /> diff --git a/plugins/org/src/components/Cards/OwnershipCard/useGetEntities.test.ts b/plugins/org/src/components/Cards/OwnershipCard/useGetEntities.test.ts index eac5a171ba..c6cec557c1 100644 --- a/plugins/org/src/components/Cards/OwnershipCard/useGetEntities.test.ts +++ b/plugins/org/src/components/Cards/OwnershipCard/useGetEntities.test.ts @@ -64,7 +64,7 @@ describe('useGetEntities', () => { ]), }); - describe('given aggregated relationsType', () => { + describe('given aggregated relationAggregation', () => { const whenHookIsCalledWith = async (_entity: Entity) => { const { result } = renderHook( ({ entity }) => useGetEntities(entity, 'aggregated'), @@ -205,7 +205,7 @@ describe('useGetEntities', () => { }); }); - describe('given direct relationsType', () => { + describe('given direct relationAggregation', () => { const whenHookIsCalledWith = async (_entity: Entity) => { const { result } = renderHook( ({ entity }) => useGetEntities(entity, 'direct'), diff --git a/plugins/org/src/components/Cards/OwnershipCard/useGetEntities.ts b/plugins/org/src/components/Cards/OwnershipCard/useGetEntities.ts index 5a708bcfee..d806bbfef2 100644 --- a/plugins/org/src/components/Cards/OwnershipCard/useGetEntities.ts +++ b/plugins/org/src/components/Cards/OwnershipCard/useGetEntities.ts @@ -125,11 +125,11 @@ const getChildOwnershipEntityRefs = async ( const getOwners = async ( entity: Entity, - relations: EntityRelationAggregation, + relationAggregation: EntityRelationAggregation, catalogApi: CatalogApi, ): Promise => { const isGroup = entity.kind === 'Group'; - const isAggregated = relations === 'aggregated'; + const isAggregated = relationAggregation === 'aggregated'; const isUserEntity = entity.kind === 'User'; if (isAggregated && isGroup) { @@ -166,7 +166,7 @@ const getOwnedEntitiesByOwners = ( export function useGetEntities( entity: Entity, - relations: EntityRelationAggregation, + relationAggregation: EntityRelationAggregation, entityFilterKind?: string[], entityLimit = 6, ): { @@ -189,7 +189,7 @@ export function useGetEntities( error, value: componentsWithCounters, } = useAsync(async () => { - const owners = await getOwners(entity, relations, catalogApi); + const owners = await getOwners(entity, relationAggregation, catalogApi); const ownedEntitiesList = await getOwnedEntitiesByOwners( owners, @@ -230,7 +230,7 @@ export function useGetEntities( kind: string; queryParams: string; }>; - }, [catalogApi, entity, relations]); + }, [catalogApi, entity, relationAggregation]); return { componentsWithCounters,