diff --git a/.changeset/new-roses-fail.md b/.changeset/new-roses-fail.md index 7c92775d7b..77495c5f54 100644 --- a/.changeset/new-roses-fail.md +++ b/.changeset/new-roses-fail.md @@ -2,4 +2,15 @@ '@backstage/plugin-org': patch --- -Changed the MembersListCard component to allow displaying aggregated members when viewing a group. Now, a toggle switch is displayed that lets you switch between showing direct members and aggregated members. +Changed the MembersListCard component to allow displaying aggregated members when viewing a group. Now, a toggle switch can be displayed that lets you switch between showing direct members and aggregated members. + +To enable this new feature, set the showAggregateMembersToggle prop on EntityMembersListCard: + +```jsx +// In packages/app/src/components/catalog/EntityPage.tsx +const groupPage = ( + // ... + + // ... +); +``` diff --git a/plugins/org/src/components/Cards/Group/MembersList/MembersListCard.stories.tsx b/plugins/org/src/components/Cards/Group/MembersList/MembersListCard.stories.tsx index 073f892302..4342c9a612 100644 --- a/plugins/org/src/components/Cards/Group/MembersList/MembersListCard.stories.tsx +++ b/plugins/org/src/components/Cards/Group/MembersList/MembersListCard.stories.tsx @@ -1,5 +1,5 @@ /* - * Copyright 2021 The Backstage Authors + * 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. @@ -20,6 +20,10 @@ import { TestApiProvider } from '@backstage/test-utils'; import { Grid } from '@material-ui/core'; import React from 'react'; import { MemoryRouter } from 'react-router-dom'; +import { + groupA, + mockedCatalogApiSupportingGroups, +} from '../../../../test-helpers/catalogMocks'; import { MembersListCard } from './MembersListCard'; export default { @@ -132,3 +136,17 @@ export const Empty = () => ( ); + +export const AggregateMembersToggle = () => ( + + + + + + + + + + + +); 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 4ef5939d33..3f4d6d2f03 100644 --- a/plugins/org/src/components/Cards/Group/MembersList/MembersListCard.test.tsx +++ b/plugins/org/src/components/Cards/Group/MembersList/MembersListCard.test.tsx @@ -33,7 +33,6 @@ import { MembersListCard } from './MembersListCard'; import { groupA, mockedCatalogApiSupportingGroups, - mockedGetEntityRelations, } from '../../../../test-helpers/catalogMocks'; import { permissionApiRef } from '@backstage/plugin-permission-react'; import { EntityLayout } from '@backstage/plugin-catalog'; @@ -41,12 +40,6 @@ import { screen } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import { Observable } from '@backstage/types'; -jest.mock('@backstage/plugin-catalog-react', () => ({ - ...jest.requireActual('@backstage/plugin-catalog-react'), - getEntityRelations: (entity: Entity | undefined, relationType: string) => - mockedGetEntityRelations(entity, relationType), -})); - // Mock needed because jsdom doesn't correctly implement box-sizing // https://github.com/ShinyChang/React-Text-Truncate/issues/70 // https://stackoverflow.com/questions/71916701/how-to-mock-a-react-function-component-that-takes-a-ref-prop @@ -166,7 +159,52 @@ describe('MemberTab Test', () => { }); describe('Aggregate members toggle', () => { - it('Shows only direct members if the aggregate users switch is turned off', async () => { + it('Does not show the aggregate members toggle if the showAggregateMembersToggle prop is undefined', async () => { + await renderInTestApp( + + + + + + + + + , + ); + + const toggleSwitch = screen.queryByRole('checkbox'); + expect(toggleSwitch).toBeNull(); + }); + + it('Shows the aggregate members toggle if the showAggregateMembersToggle prop is true', async () => { + await renderInTestApp( + + + + + + + + + , + ); + + expect(screen.queryByRole('checkbox')).toBeInTheDocument(); + }); + + it('Shows only direct members if the showAggregateMembersToggle prop is undefined', async () => { await renderInTestApp( { ).toBe(Node.DOCUMENT_POSITION_FOLLOWING); }); + it('Shows only direct members if the aggregate members switch is turned off', async () => { + await renderInTestApp( + + + + + + + + + , + ); + + const displayedMemberNames = screen.queryAllByTestId('user-link'); + const duplicatedUserText = screen.getByText('Duplicated User'); + const groupAUserOneText = screen.getByText('Group A User One'); + + expect(displayedMemberNames).toHaveLength(2); + expect(duplicatedUserText).toBeInTheDocument(); + expect(groupAUserOneText).toBeInTheDocument(); + expect( + duplicatedUserText.compareDocumentPosition(groupAUserOneText), + ).toBe(Node.DOCUMENT_POSITION_FOLLOWING); + }); + it('Shows all descendant members of the group when the aggregate users switch is turned on, showing duplicated members only once', 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 4e9be6d6ad..9973beee6c 100644 --- a/plugins/org/src/components/Cards/Group/MembersList/MembersListCard.tsx +++ b/plugins/org/src/components/Cards/Group/MembersList/MembersListCard.tsx @@ -131,8 +131,13 @@ const MemberComponent = (props: { member: UserEntity }) => { export const MembersListCard = (props: { memberDisplayTitle?: string; pageSize?: number; + showAggregateMembersToggle?: boolean; }) => { - const { memberDisplayTitle = 'Members', pageSize = 50 } = props; + const { + memberDisplayTitle = 'Members', + pageSize = 50, + showAggregateMembersToggle, + } = props; const { entity: groupEntity } = useEntity(); const { @@ -150,7 +155,7 @@ export const MembersListCard = (props: { setPage(pageIndex); }; - const [showAggregateUsers, setShowAggregateUsers] = useState(false); + const [showAggregateMembers, setShowAggregateMembers] = useState(false); const { value: descendantMembers } = useAsync( async () => @@ -181,7 +186,7 @@ export const MembersListCard = (props: { const members = removeDuplicateEntitiesFrom( [ ...(directMembers ?? []), - ...(descendantMembers && showAggregateUsers ? descendantMembers : []), + ...(descendantMembers && showAggregateMembers ? descendantMembers : []), ].sort((a, b) => { const nameToCompareInA = a.spec.profile?.displayName ?? a.metadata.name; const nameToCompareInB = b.spec.profile?.displayName ?? b.metadata.name; @@ -218,16 +223,20 @@ export const MembersListCard = (props: { subheader={`of ${displayName}`} {...(nbPages <= 1 ? {} : { actions: pagination })} > - Direct Members - { - setShowAggregateUsers(!showAggregateUsers); - }} - inputProps={{ 'aria-label': 'Users Type Switch' }} - /> - Aggregated Members + {showAggregateMembersToggle && ( + <> + Direct Members + { + setShowAggregateMembers(!showAggregateMembers); + }} + inputProps={{ 'aria-label': 'Users Type Switch' }} + /> + Aggregated Members + + )} {members && members.length > 0 ? ( members diff --git a/plugins/org/src/helpers/helpers.test.ts b/plugins/org/src/helpers/helpers.test.ts index b80be83a8b..714371d83d 100644 --- a/plugins/org/src/helpers/helpers.test.ts +++ b/plugins/org/src/helpers/helpers.test.ts @@ -15,7 +15,6 @@ */ import { CatalogApi } from '@backstage/catalog-client'; -import { Entity } from '@backstage/catalog-model'; import { getAllDesendantMembersForGroupEntity, getMembersFromGroups, @@ -39,15 +38,8 @@ import { groupEUserOne, duplicatedUser, mockedCatalogApiSupportingGroups, - mockedGetEntityRelations, } from '../test-helpers/catalogMocks'; -jest.mock('@backstage/plugin-catalog-react', () => ({ - ...jest.requireActual('@backstage/plugin-catalog-react'), - getEntityRelations: (entity: Entity | undefined, relationType: string) => - mockedGetEntityRelations(entity, relationType), -})); - describe('Helper functions', () => { it('getAllDesendantMembersForGroupEntity correctly recursively returns all descendant members', async () => { const catalogApi = mockedCatalogApiSupportingGroups as CatalogApi; diff --git a/plugins/org/src/test-helpers/catalogMocks.ts b/plugins/org/src/test-helpers/catalogMocks.ts index e979247b94..b14856e345 100644 --- a/plugins/org/src/test-helpers/catalogMocks.ts +++ b/plugins/org/src/test-helpers/catalogMocks.ts @@ -38,6 +38,16 @@ export const groupA: GroupEntity = { type: 'testing-group', children: [], }, + relations: [ + { + type: 'parentOf', + targetRef: 'group:default/group-b', + }, + { + type: 'parentOf', + targetRef: 'group:default/group-c', + }, + ], }; export const groupAWithALeader: GroupEntity = { @@ -57,11 +67,19 @@ export const groupAWithALeader: GroupEntity = { type: 'teamLeadBy', targetRef: 'user:default/group-a-user-one', }, + { + type: 'parentOf', + targetRef: 'group:default/group-b', + }, + { + type: 'parentOf', + targetRef: 'group:default/group-c', + }, ], }; export const groupARef: CompoundEntityRef = { - kind: 'Group', + kind: 'group', name: 'group-a', namespace: 'default', }; @@ -78,10 +96,20 @@ export const groupB: GroupEntity = { type: 'testing-group', children: [], }, + relations: [ + { + type: 'parentOf', + targetRef: 'group:default/group-a', + }, + { + type: 'parentOf', + targetRef: 'group:default/group-d', + }, + ], }; export const groupBRef: CompoundEntityRef = { - kind: 'Group', + kind: 'group', name: 'group-b', namespace: 'default', }; @@ -101,7 +129,7 @@ export const groupC: GroupEntity = { }; export const groupCRef: CompoundEntityRef = { - kind: 'Group', + kind: 'group', name: 'group-c', namespace: 'default', }; @@ -118,10 +146,20 @@ export const groupD: GroupEntity = { type: 'testing-group', children: [], }, + relations: [ + { + type: 'parentOf', + targetRef: 'group:default/group-c', + }, + { + type: 'parentOf', + targetRef: 'group:default/group-e', + }, + ], }; export const groupDRef: CompoundEntityRef = { - kind: 'Group', + kind: 'group', name: 'group-d', namespace: 'default', }; @@ -141,7 +179,7 @@ export const groupE: GroupEntity = { }; export const groupERef: CompoundEntityRef = { - kind: 'Group', + kind: 'group', name: 'group-e', namespace: 'default', }; @@ -160,12 +198,6 @@ export const groupF: GroupEntity = { }, }; -interface MockedRelationsType { - [index: string]: CompoundEntityRef[]; - ownedBy: CompoundEntityRef[]; - parentOf: CompoundEntityRef[]; -} - export const groupAUserOne: Entity = { apiVersion: 'backstage.io/v1alpha1', kind: 'User', @@ -262,44 +294,6 @@ export const duplicatedUser: Entity = { }, }; -const mockedRelationsCatalog = new Map([ - [ - 'group-a', - { - ownedBy: [], - parentOf: [groupBRef, groupCRef], - }, - ], - [ - 'group-b', - { - ownedBy: [], - parentOf: [groupARef, groupDRef], - }, - ], - [ - 'group-c', - { - ownedBy: [], - parentOf: [], - }, - ], - [ - 'group-d', - { - ownedBy: [], - parentOf: [groupCRef, groupERef], - }, - ], - [ - 'group-e', - { - ownedBy: [], - parentOf: [], - }, - ], -]); - const mockedRefsToRelationsMap = new Map([ ['group:default/group-a', groupA], ['group:default/group-b', groupB], @@ -319,10 +313,6 @@ const mockedMembersMapping = new Map([ ]); type Nullable = T | undefined; -const nullMockedRelationsType: MockedRelationsType = { - ownedBy: [], - parentOf: [], -}; export const mockedCatalogApiSupportingGroups: Partial = { getEntities: async (request?: GetEntitiesRequest) => { @@ -348,10 +338,3 @@ export const mockedCatalogApiSupportingGroups: Partial = { return { items }; }, }; - -export const mockedGetEntityRelations = ( - entity: Nullable, - relationType: string, -) => - (mockedRelationsCatalog.get(entity?.metadata.name ?? 'NULL') ?? - nullMockedRelationsType)[relationType];