From 6e387c077a4507f8f79eb90333d13dbcf1e73ede Mon Sep 17 00:00:00 2001 From: Robert Bunning Date: Mon, 24 Apr 2023 16:09:38 -0400 Subject: [PATCH 01/11] Add aggregate members toggle to show aggregate members on groups Signed-off-by: Robert Bunning --- .changeset/new-roses-fail.md | 5 + plugins/org/package.json | 5 +- .../MembersList/MembersListCard.test.tsx | 119 +++++- .../Group/MembersList/MembersListCard.tsx | 41 +- plugins/org/src/helpers/helpers.test.ts | 143 +++++++ plugins/org/src/helpers/helpers.ts | 117 ++++++ plugins/org/src/test-helpers/catalogMocks.ts | 357 ++++++++++++++++++ yarn.lock | 3 + 8 files changed, 785 insertions(+), 5 deletions(-) create mode 100644 .changeset/new-roses-fail.md create mode 100644 plugins/org/src/helpers/helpers.test.ts create mode 100644 plugins/org/src/helpers/helpers.ts create mode 100644 plugins/org/src/test-helpers/catalogMocks.ts diff --git a/.changeset/new-roses-fail.md b/.changeset/new-roses-fail.md new file mode 100644 index 0000000000..7c92775d7b --- /dev/null +++ b/.changeset/new-roses-fail.md @@ -0,0 +1,5 @@ +--- +'@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. diff --git a/plugins/org/package.json b/plugins/org/package.json index a5b3308922..08d8f4bf9a 100644 --- a/plugins/org/package.json +++ b/plugins/org/package.json @@ -28,6 +28,7 @@ "clean": "backstage-cli package clean" }, "dependencies": { + "@backstage/catalog-client": "workspace:^", "@backstage/catalog-model": "workspace:^", "@backstage/core-components": "workspace:^", "@backstage/core-plugin-api": "workspace:^", @@ -47,11 +48,13 @@ "react-router-dom": "6.0.0-beta.0 || ^6.3.0" }, "devDependencies": { - "@backstage/catalog-client": "workspace:^", "@backstage/cli": "workspace:^", "@backstage/core-app-api": "workspace:^", "@backstage/dev-utils": "workspace:^", + "@backstage/plugin-catalog": "workspace:^", + "@backstage/plugin-permission-react": "workspace:^", "@backstage/test-utils": "workspace:^", + "@backstage/types": "workspace:^", "@testing-library/dom": "^8.0.0", "@testing-library/jest-dom": "^5.10.1", "@testing-library/react": "^12.1.3", 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 cc524b60a2..4ef5939d33 100644 --- a/plugins/org/src/components/Cards/Group/MembersList/MembersListCard.test.tsx +++ b/plugins/org/src/components/Cards/Group/MembersList/MembersListCard.test.tsx @@ -1,5 +1,5 @@ /* - * Copyright 2020 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. @@ -19,14 +19,33 @@ import { CatalogApi, catalogApiRef, EntityProvider, + StarredEntitiesApi, + starredEntitiesApiRef, } from '@backstage/plugin-catalog-react'; import { + renderInTestApp, renderWithEffects, TestApiProvider, wrapInTestApp, } from '@backstage/test-utils'; import React from 'react'; 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'; +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 @@ -41,6 +60,20 @@ jest.mock('react-text-truncate', () => { }; }); +const mockedStarredEntitiesApi: Partial = { + starredEntitie$: () => { + return { + subscribe: () => { + return { + unsubscribe() { + // This is intentional + }, + }; + }, + } as Observable>; + }, +}; + describe('MemberTab Test', () => { const groupEntity: GroupEntity = { apiVersion: 'backstage.io/v1alpha1', @@ -131,4 +164,88 @@ describe('MemberTab Test', () => { expect(rendered.getByText('Testers (1)')).toBeInTheDocument(); }); + + describe('Aggregate members toggle', () => { + it('Shows only direct members if the aggregate users 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( + + + + + + + + + , + ); + + // Click the toggle switch + await userEvent.click(screen.getByRole('checkbox')); + + const displayedMemberNames = screen.queryAllByTestId('user-link'); + const duplicatedUserText = screen.getByText('Duplicated User'); + const groupAUserOneText = screen.getByText('Group A User One'); + const groupBUserOneText = screen.getByText('Group B User One'); + const groupDUserOneText = screen.getByText('Group D User One'); + const groupEUserOneText = screen.getByText('Group E User One'); + + expect(displayedMemberNames).toHaveLength(5); + + expect(duplicatedUserText).toBeInTheDocument(); + expect(groupAUserOneText).toBeInTheDocument(); + expect(groupBUserOneText).toBeInTheDocument(); + expect(groupDUserOneText).toBeInTheDocument(); + expect(groupEUserOneText).toBeInTheDocument(); + + expect( + duplicatedUserText.compareDocumentPosition(groupAUserOneText), + ).toBe(Node.DOCUMENT_POSITION_FOLLOWING); + expect(groupAUserOneText.compareDocumentPosition(groupBUserOneText)).toBe( + Node.DOCUMENT_POSITION_FOLLOWING, + ); + expect(groupBUserOneText.compareDocumentPosition(groupDUserOneText)).toBe( + Node.DOCUMENT_POSITION_FOLLOWING, + ); + expect(groupDUserOneText.compareDocumentPosition(groupEUserOneText)).toBe( + Node.DOCUMENT_POSITION_FOLLOWING, + ); + }); + }); }); diff --git a/plugins/org/src/components/Cards/Group/MembersList/MembersListCard.tsx b/plugins/org/src/components/Cards/Group/MembersList/MembersListCard.tsx index eb61989144..4e9be6d6ad 100644 --- a/plugins/org/src/components/Cards/Group/MembersList/MembersListCard.tsx +++ b/plugins/org/src/components/Cards/Group/MembersList/MembersListCard.tsx @@ -1,5 +1,5 @@ /* - * Copyright 2020 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. @@ -30,11 +30,12 @@ import { createStyles, Grid, makeStyles, + Switch, Theme, Typography, } from '@material-ui/core'; import Pagination from '@material-ui/lab/Pagination'; -import React from 'react'; +import React, { useState } from 'react'; import { generatePath } from 'react-router-dom'; import useAsync from 'react-use/lib/useAsync'; @@ -47,6 +48,10 @@ import { OverflowTooltip, } from '@backstage/core-components'; import { useApi } from '@backstage/core-plugin-api'; +import { + getAllDesendantMembersForGroupEntity, + removeDuplicateEntitiesFrom, +} from '../../../../helpers/helpers'; const useStyles = makeStyles((theme: Theme) => createStyles({ @@ -98,6 +103,7 @@ const MemberComponent = (props: { member: UserEntity }) => { > + await getAllDesendantMembersForGroupEntity(groupEntity, catalogApi), + [catalogApi, groupEntity], + ); const { loading, error, - value: members, + value: directMembers, } = useAsync(async () => { const membersList = await catalogApi.getEntities({ filter: { @@ -165,6 +178,18 @@ export const MembersListCard = (props: { return membersList.items as UserEntity[]; }, [catalogApi, groupEntity]); + const members = removeDuplicateEntitiesFrom( + [ + ...(directMembers ?? []), + ...(descendantMembers && showAggregateUsers ? descendantMembers : []), + ].sort((a, b) => { + const nameToCompareInA = a.spec.profile?.displayName ?? a.metadata.name; + const nameToCompareInB = b.spec.profile?.displayName ?? b.metadata.name; + + return nameToCompareInA.localeCompare(nameToCompareInB); + }), + ) as UserEntity[]; + if (loading) { return ; } else if (error) { @@ -193,6 +218,16 @@ export const MembersListCard = (props: { subheader={`of ${displayName}`} {...(nbPages <= 1 ? {} : { actions: pagination })} > + Direct Members + { + setShowAggregateUsers(!showAggregateUsers); + }} + 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 new file mode 100644 index 0000000000..b80be83a8b --- /dev/null +++ b/plugins/org/src/helpers/helpers.test.ts @@ -0,0 +1,143 @@ +/* + * 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 { CatalogApi } from '@backstage/catalog-client'; +import { Entity } from '@backstage/catalog-model'; +import { + getAllDesendantMembersForGroupEntity, + getMembersFromGroups, + getDescendantGroupsFromGroup, + removeDuplicateEntitiesFrom, +} from './helpers'; +import { + groupA, + groupARef, + groupB, + groupBRef, + groupC, + groupCRef, + groupD, + groupDRef, + groupE, + groupERef, + groupAUserOne, + groupBUserOne, + groupDUserOne, + 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; + + const actualGroupADescendantMembers = + await getAllDesendantMembersForGroupEntity(groupA, catalogApi); + const actualGroupCDescendantMembers = + await getAllDesendantMembersForGroupEntity(groupC, catalogApi); + + expect(actualGroupADescendantMembers).toStrictEqual([ + groupBUserOne, + groupDUserOne, + duplicatedUser, + groupEUserOne, + duplicatedUser, + ]); + expect(actualGroupCDescendantMembers).toStrictEqual([]); + }); + + it('getMembersFromGroups correctly returns all members of provided groups', async () => { + const catalogApi = mockedCatalogApiSupportingGroups as CatalogApi; + + const actualNoGroupsMembers = await getMembersFromGroups([], catalogApi); + const actualGroupAMembers = await getMembersFromGroups( + [groupARef], + catalogApi, + ); + const actualGroupCMembers = await getMembersFromGroups( + [groupCRef], + catalogApi, + ); + const actualMultipleGroupMembers = await getMembersFromGroups( + [groupARef, groupBRef, groupCRef], + catalogApi, + ); + + expect(actualNoGroupsMembers).toStrictEqual([]); + expect(actualGroupAMembers).toStrictEqual([groupAUserOne, duplicatedUser]); + expect(actualGroupCMembers).toStrictEqual([]); + expect(actualMultipleGroupMembers).toStrictEqual([ + groupAUserOne, + duplicatedUser, + groupBUserOne, + ]); + }); + + it('removeDuplicateEntitiesFrom correctly removes duplicate entities', () => { + const actualNoDuplicatedEntitiesResult = removeDuplicateEntitiesFrom([ + groupA, + groupB, + groupC, + groupD, + ]); + const actualDuplicatedEntitiesResult = removeDuplicateEntitiesFrom([ + groupA, + groupB, + groupB, + groupC, + groupC, + groupD, + groupE, + ]); + const actualNoEntitiesProvidedResult = removeDuplicateEntitiesFrom([]); + + expect(actualNoDuplicatedEntitiesResult).toStrictEqual([ + groupA, + groupB, + groupC, + groupD, + ]); + expect(actualDuplicatedEntitiesResult).toStrictEqual([ + groupA, + groupB, + groupC, + groupD, + groupE, + ]); + expect(actualNoEntitiesProvidedResult).toStrictEqual([]); + }); + + it('getDescendantGroupsFromGroup correctly recursively returns descendant groups, ignoring duplicates', async () => { + const actualDescendantGroups = await getDescendantGroupsFromGroup( + groupA, + mockedCatalogApiSupportingGroups as CatalogApi, + ); + expect(actualDescendantGroups).toStrictEqual([ + groupBRef, + groupCRef, + groupDRef, + groupERef, + ]); + }); +}); diff --git a/plugins/org/src/helpers/helpers.ts b/plugins/org/src/helpers/helpers.ts new file mode 100644 index 0000000000..e0e83bec67 --- /dev/null +++ b/plugins/org/src/helpers/helpers.ts @@ -0,0 +1,117 @@ +/* + * 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, + DEFAULT_NAMESPACE, + Entity, + GroupEntity, + RELATION_PARENT_OF, + stringifyEntityRef, + UserEntity, +} from '@backstage/catalog-model'; +import { + CatalogApi, + getEntityRelations, +} from '@backstage/plugin-catalog-react'; + +export const getMembersFromGroups = async ( + groups: CompoundEntityRef[], + catalogApi: CatalogApi, +) => { + const membersList = + groups.length === 0 + ? { items: [] } + : 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'), + }), + ), + }, + }); + + return membersList.items as UserEntity[]; +}; + +export const getDescendantGroupsFromGroup = async ( + group: GroupEntity, + catalogApi: CatalogApi, +) => { + const alreadyQueuedOrExpandedGroupNames = new Map(); + const groupRef: CompoundEntityRef = { + kind: group.kind, + namespace: group.metadata.namespace ?? DEFAULT_NAMESPACE, + name: group.metadata.name, + }; + + const groupQueue = [groupRef]; + const resultantGroupRefs: CompoundEntityRef[] = []; + + // Continue expanding groups until there are no more + while (groupQueue.length > 0) { + const activeGroupRef = groupQueue.shift() as CompoundEntityRef; + const activeGroup = await catalogApi.getEntityByRef(activeGroupRef); + alreadyQueuedOrExpandedGroupNames.set( + stringifyEntityRef(activeGroupRef), + true, + ); + + const childGroups = getEntityRelations(activeGroup, RELATION_PARENT_OF, { + kind: 'Group', + }).filter( + currentGroup => + !alreadyQueuedOrExpandedGroupNames.has( + stringifyEntityRef(currentGroup), + ), + ); + childGroups.forEach(childGroup => + alreadyQueuedOrExpandedGroupNames.set( + stringifyEntityRef(childGroup), + true, + ), + ); + + groupQueue.push(...childGroups); + resultantGroupRefs.push(...childGroups); + } + + return resultantGroupRefs; +}; + +export const getAllDesendantMembersForGroupEntity = async ( + groupEntity: GroupEntity, + catalogApi: CatalogApi, +) => + getMembersFromGroups( + await getDescendantGroupsFromGroup(groupEntity, catalogApi), + catalogApi, + ); + +export const removeDuplicateEntitiesFrom = (entityArray: Entity[]) => { + const seenEntities = new Map(); + return entityArray.filter(entity => { + const stringifiedEntity = stringifyEntityRef(entity); + const isDuplicate = seenEntities.has(stringifiedEntity); + + seenEntities.set(stringifiedEntity, true); + return !isDuplicate; + }); +}; diff --git a/plugins/org/src/test-helpers/catalogMocks.ts b/plugins/org/src/test-helpers/catalogMocks.ts new file mode 100644 index 0000000000..e979247b94 --- /dev/null +++ b/plugins/org/src/test-helpers/catalogMocks.ts @@ -0,0 +1,357 @@ +/* + * 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 { + CatalogApi, + GetEntitiesByRefsRequest, + GetEntitiesRequest, +} from '@backstage/catalog-client'; +import { + CompoundEntityRef, + Entity, + GroupEntity, + stringifyEntityRef, +} from '@backstage/catalog-model'; + +export const groupA: GroupEntity = { + apiVersion: 'backstage.io/v1alpha1', + kind: 'Group', + metadata: { + name: 'group-a', + title: 'Group A', + description: 'Testing Group A', + }, + spec: { + type: 'testing-group', + children: [], + }, +}; + +export const groupAWithALeader: GroupEntity = { + apiVersion: 'backstage.io/v1alpha1', + kind: 'Group', + metadata: { + name: 'group-a', + title: 'Group A', + description: 'Testing Group A', + }, + spec: { + type: 'testing-group', + children: [], + }, + relations: [ + { + type: 'teamLeadBy', + targetRef: 'user:default/group-a-user-one', + }, + ], +}; + +export const groupARef: CompoundEntityRef = { + kind: 'Group', + name: 'group-a', + namespace: 'default', +}; + +export const groupB: GroupEntity = { + apiVersion: 'backstage.io/v1alpha1', + kind: 'Group', + metadata: { + name: 'group-b', + title: 'Group B', + description: 'Testing Group B', + }, + spec: { + type: 'testing-group', + children: [], + }, +}; + +export const groupBRef: CompoundEntityRef = { + kind: 'Group', + name: 'group-b', + namespace: 'default', +}; + +export const groupC: GroupEntity = { + apiVersion: 'backstage.io/v1alpha1', + kind: 'Group', + metadata: { + name: 'group-c', + title: 'Group C', + description: 'Testing Group C', + }, + spec: { + type: 'testing-group', + children: [], + }, +}; + +export const groupCRef: CompoundEntityRef = { + kind: 'Group', + name: 'group-c', + namespace: 'default', +}; + +export const groupD: GroupEntity = { + apiVersion: 'backstage.io/v1alpha1', + kind: 'Group', + metadata: { + name: 'group-d', + title: 'Group D', + description: 'Testing Group D', + }, + spec: { + type: 'testing-group', + children: [], + }, +}; + +export const groupDRef: CompoundEntityRef = { + kind: 'Group', + name: 'group-d', + namespace: 'default', +}; + +export const groupE: GroupEntity = { + apiVersion: 'backstage.io/v1alpha1', + kind: 'Group', + metadata: { + name: 'group-e', + title: 'Group E', + description: 'Testing Group E', + }, + spec: { + type: 'testing-group', + children: [], + }, +}; + +export const groupERef: CompoundEntityRef = { + kind: 'Group', + name: 'group-e', + namespace: 'default', +}; + +export const groupF: GroupEntity = { + apiVersion: 'backstage.io/v1alpha1', + kind: 'Group', + metadata: { + name: 'group-f', + title: 'Group F', + description: 'Testing Group F', + }, + spec: { + type: 'testing-group', + children: [], + }, +}; + +interface MockedRelationsType { + [index: string]: CompoundEntityRef[]; + ownedBy: CompoundEntityRef[]; + parentOf: CompoundEntityRef[]; +} + +export const groupAUserOne: Entity = { + apiVersion: 'backstage.io/v1alpha1', + kind: 'User', + metadata: { + name: 'group-a-user-one', + }, + spec: { + profile: { + displayName: 'Group A User One', + email: 'group-a-user-one@testing.email', + picture: + 'https://avatars.dicebear.com/api/avataaars/breanna-davison@example.com.svg', + }, + }, +}; + +export const groupBUserOne: Entity = { + apiVersion: 'backstage.io/v1alpha1', + kind: 'User', + metadata: { + name: 'group-b-user-one', + }, + spec: { + profile: { + displayName: 'Group B User One', + email: 'group-b-user-one@testing.email', + picture: + 'https://avatars.dicebear.com/api/avataaars/breanna-davison@example.com.svg', + }, + }, +}; + +export const groupDUserOne: Entity = { + apiVersion: 'backstage.io/v1alpha1', + kind: 'User', + metadata: { + name: 'group-d-user-one', + }, + spec: { + profile: { + displayName: 'Group D User One', + email: 'group-d-user-one@testing.email', + picture: + 'https://avatars.dicebear.com/api/avataaars/breanna-davison@example.com.svg', + }, + }, +}; + +export const groupEUserOne: Entity = { + apiVersion: 'backstage.io/v1alpha1', + kind: 'User', + metadata: { + name: 'group-e-user-one', + }, + spec: { + profile: { + displayName: 'Group E User One', + email: 'group-e-user-one@testing.email', + picture: + 'https://avatars.dicebear.com/api/avataaars/breanna-davison@example.com.svg', + }, + }, +}; + +export const groupFUserOne: Entity = { + apiVersion: 'backstage.io/v1alpha1', + kind: 'User', + metadata: { + name: 'group-f-user-one', + }, + spec: { + profile: { + displayName: 'Group F User One', + email: 'group-f-user-one@testing.email', + picture: + 'https://avatars.dicebear.com/api/avataaars/breanna-davison@example.com.svg', + }, + }, +}; + +export const duplicatedUser: Entity = { + apiVersion: 'backstage.io/v1alpha1', + kind: 'User', + metadata: { + name: 'duplicated-user', + }, + spec: { + profile: { + displayName: 'Duplicated User', + email: 'duplicated-user@testing.email', + picture: + 'https://avatars.dicebear.com/api/avataaars/breanna-davison@example.com.svg', + }, + }, +}; + +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], + ['group:default/group-c', groupC], + ['group:default/group-d', groupD], + ['group:default/group-e', groupE], + ['group:default/group-f', groupF], +]); + +const mockedMembersMapping = new Map([ + ['group:default/group-a', [groupAUserOne, duplicatedUser]], + ['group:default/group-b', [groupBUserOne]], + ['group:default/group-c', []], + ['group:default/group-d', [groupDUserOne, duplicatedUser]], + ['group:default/group-e', [groupEUserOne, duplicatedUser]], + ['group:default/group-f', [groupFUserOne]], +]); + +type Nullable = T | undefined; +const nullMockedRelationsType: MockedRelationsType = { + ownedBy: [], + parentOf: [], +}; + +export const mockedCatalogApiSupportingGroups: Partial = { + getEntities: async (request?: GetEntitiesRequest) => { + const actualFilter = (request?.filter as Nullable<{ + 'relations.memberof': string[]; + }>) ?? { 'relations.memberof': [] }; + const items = + actualFilter['relations.memberof'].length > 0 + ? (actualFilter['relations.memberof'] + .map(group => mockedMembersMapping.get(group)) + .flat() + .filter(entity => !!entity) as Entity[]) + : [...mockedMembersMapping.values()].flat(); + + return { items }; + }, + getEntityByRef: async (entityRef: CompoundEntityRef) => + mockedRefsToRelationsMap.get(stringifyEntityRef(entityRef)), + getEntitiesByRefs: async (request: GetEntitiesByRefsRequest) => { + const items = request.entityRefs.map(entityRef => + mockedRefsToRelationsMap.get(entityRef), + ); + return { items }; + }, +}; + +export const mockedGetEntityRelations = ( + entity: Nullable, + relationType: string, +) => + (mockedRelationsCatalog.get(entity?.metadata.name ?? 'NULL') ?? + nullMockedRelationsType)[relationType]; diff --git a/yarn.lock b/yarn.lock index 95a9a07c70..79ab29cc49 100644 --- a/yarn.lock +++ b/yarn.lock @@ -7465,9 +7465,12 @@ __metadata: "@backstage/core-components": "workspace:^" "@backstage/core-plugin-api": "workspace:^" "@backstage/dev-utils": "workspace:^" + "@backstage/plugin-catalog": "workspace:^" "@backstage/plugin-catalog-react": "workspace:^" + "@backstage/plugin-permission-react": "workspace:^" "@backstage/test-utils": "workspace:^" "@backstage/theme": "workspace:^" + "@backstage/types": "workspace:^" "@material-ui/core": ^4.12.2 "@material-ui/icons": ^4.9.1 "@material-ui/lab": 4.0.0-alpha.61 From 54da67d820f876b1b4000c8d6a76215689f79caa Mon Sep 17 00:00:00 2001 From: Robert Bunning Date: Mon, 24 Apr 2023 16:32:18 -0400 Subject: [PATCH 02/11] Adjust formatting Signed-off-by: Robert Bunning --- plugins/org/src/helpers/helpers.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/plugins/org/src/helpers/helpers.ts b/plugins/org/src/helpers/helpers.ts index e0e83bec67..7da4ee32d8 100644 --- a/plugins/org/src/helpers/helpers.ts +++ b/plugins/org/src/helpers/helpers.ts @@ -107,6 +107,7 @@ export const getAllDesendantMembersForGroupEntity = async ( export const removeDuplicateEntitiesFrom = (entityArray: Entity[]) => { const seenEntities = new Map(); + return entityArray.filter(entity => { const stringifiedEntity = stringifyEntityRef(entity); const isDuplicate = seenEntities.has(stringifiedEntity); From 288f7360bb5d1f276ea5d99730e5b1999ece1a43 Mon Sep 17 00:00:00 2001 From: Robert Bunning Date: Thu, 27 Apr 2023 14:55:23 -0400 Subject: [PATCH 03/11] Code refactor/ address feedback Added usage code snippet in change set. Made the aggregate members toggle switch opt-in. Updated storybook story to have an entry for aggregate members. Refactored tests to not mock getEntityRelations. Removed usages of the word user in favor of member. Signed-off-by: Robert Bunning --- .changeset/new-roses-fail.md | 13 ++- .../MembersList/MembersListCard.stories.tsx | 20 +++- .../MembersList/MembersListCard.test.tsx | 87 +++++++++++++-- .../Group/MembersList/MembersListCard.tsx | 35 +++--- plugins/org/src/helpers/helpers.test.ts | 8 -- plugins/org/src/test-helpers/catalogMocks.ts | 103 ++++++++---------- 6 files changed, 174 insertions(+), 92 deletions(-) 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]; From af9c4ed9e90b2e79276b397e22a68c2fd64e69a5 Mon Sep 17 00:00:00 2001 From: Robert Bunning Date: Mon, 1 May 2023 15:41:53 -0400 Subject: [PATCH 04/11] Changed loading behavior of descendant members Signed-off-by: Robert Bunning --- .../Group/MembersList/MembersListCard.tsx | 50 +++++++++++-------- 1 file changed, 30 insertions(+), 20 deletions(-) diff --git a/plugins/org/src/components/Cards/Group/MembersList/MembersListCard.tsx b/plugins/org/src/components/Cards/Group/MembersList/MembersListCard.tsx index 9973beee6c..692dc032b9 100644 --- a/plugins/org/src/components/Cards/Group/MembersList/MembersListCard.tsx +++ b/plugins/org/src/components/Cards/Group/MembersList/MembersListCard.tsx @@ -157,11 +157,17 @@ export const MembersListCard = (props: { const [showAggregateMembers, setShowAggregateMembers] = useState(false); - const { value: descendantMembers } = useAsync( - async () => - await getAllDesendantMembersForGroupEntity(groupEntity, catalogApi), - [catalogApi, groupEntity], - ); + const { loading: loadingDescendantMembers, value: descendantMembers } = + useAsync(async () => { + if (!showAggregateMembersToggle) { + return [] as UserEntity[]; + } + + return await getAllDesendantMembersForGroupEntity( + groupEntity, + catalogApi, + ); + }, [catalogApi, groupEntity]); const { loading, error, @@ -237,21 +243,25 @@ export const MembersListCard = (props: { Aggregated Members )} - - {members && members.length > 0 ? ( - members - .slice(pageSize * (page - 1), pageSize * page) - .map(member => ( - - )) - ) : ( - - - This group has no {memberDisplayTitle.toLocaleLowerCase()}. - - - )} - + {showAggregateMembers && loadingDescendantMembers ? ( + + ) : ( + + {members && members.length > 0 ? ( + members + .slice(pageSize * (page - 1), pageSize * page) + .map(member => ( + + )) + ) : ( + + + This group has no {memberDisplayTitle.toLocaleLowerCase()}. + + + )} + + )} ); From c7af1cef82189734f3bd923ff2c7948614b8307d Mon Sep 17 00:00:00 2001 From: Robert Bunning Date: Mon, 1 May 2023 16:05:27 -0400 Subject: [PATCH 05/11] Generated API report for changes Signed-off-by: Robert Bunning --- 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 1111253119..7f21bfbb6b 100644 --- a/plugins/org/api-report.md +++ b/plugins/org/api-report.md @@ -20,6 +20,7 @@ export const EntityGroupProfileCard: (props: { export const EntityMembersListCard: (props: { memberDisplayTitle?: string | undefined; pageSize?: number | undefined; + showAggregateMembersToggle?: boolean | undefined; }) => JSX.Element; // @public (undocumented) @@ -47,6 +48,7 @@ export const GroupProfileCard: (props: { export const MembersListCard: (props: { memberDisplayTitle?: string; pageSize?: number; + showAggregateMembersToggle?: boolean; }) => JSX.Element; // @public From c871d954d085e695115c8e43989d30cb905759de Mon Sep 17 00:00:00 2001 From: Robert Bunning <129326788+wss-rbunning@users.noreply.github.com> Date: Wed, 3 May 2023 14:48:47 -0400 Subject: [PATCH 06/11] Update plugins/org/src/components/Cards/Group/MembersList/MembersListCard.test.tsx Co-authored-by: Patrik Oldsberg Signed-off-by: Robert Bunning <129326788+wss-rbunning@users.noreply.github.com> --- .../components/Cards/Group/MembersList/MembersListCard.test.tsx | 2 +- 1 file changed, 1 insertion(+), 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 3f4d6d2f03..fa823e1678 100644 --- a/plugins/org/src/components/Cards/Group/MembersList/MembersListCard.test.tsx +++ b/plugins/org/src/components/Cards/Group/MembersList/MembersListCard.test.tsx @@ -1,5 +1,5 @@ /* - * Copyright 2023 The Backstage Authors + * Copyright 2020 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. From d8bd18ab3ff0c8a461d6e3ca114c65e7fd901b2e Mon Sep 17 00:00:00 2001 From: Robert Bunning <129326788+wss-rbunning@users.noreply.github.com> Date: Wed, 3 May 2023 14:48:58 -0400 Subject: [PATCH 07/11] Update plugins/org/src/components/Cards/Group/MembersList/MembersListCard.stories.tsx Co-authored-by: Patrik Oldsberg Signed-off-by: Robert Bunning <129326788+wss-rbunning@users.noreply.github.com> --- .../Cards/Group/MembersList/MembersListCard.stories.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 4342c9a612..ec5718349d 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 2023 The Backstage Authors + * Copyright 2020 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. From ecde474d4c35b0c58e8bf4363d76c6b2b428e594 Mon Sep 17 00:00:00 2001 From: Robert Bunning <129326788+wss-rbunning@users.noreply.github.com> Date: Wed, 3 May 2023 14:49:08 -0400 Subject: [PATCH 08/11] Update plugins/org/src/components/Cards/Group/MembersList/MembersListCard.tsx Co-authored-by: Patrik Oldsberg Signed-off-by: Robert Bunning <129326788+wss-rbunning@users.noreply.github.com> --- .../src/components/Cards/Group/MembersList/MembersListCard.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/org/src/components/Cards/Group/MembersList/MembersListCard.tsx b/plugins/org/src/components/Cards/Group/MembersList/MembersListCard.tsx index 692dc032b9..a27c9ab8ca 100644 --- a/plugins/org/src/components/Cards/Group/MembersList/MembersListCard.tsx +++ b/plugins/org/src/components/Cards/Group/MembersList/MembersListCard.tsx @@ -1,5 +1,5 @@ /* - * Copyright 2023 The Backstage Authors + * Copyright 2020 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. From 9ca3e44a0698ddff7670a45726280be0d6d51ec4 Mon Sep 17 00:00:00 2001 From: Robert Bunning Date: Wed, 3 May 2023 15:57:05 -0400 Subject: [PATCH 09/11] Moved test helpers to __testUtils__ folder. Added stringifyEntityRef as a dependency in MembersListCard Signed-off-by: Robert Bunning --- plugins/org/package.json | 2 +- plugins/org/src/{test-helpers => __testUtils__}/catalogMocks.ts | 0 .../Cards/Group/MembersList/MembersListCard.stories.tsx | 2 +- .../components/Cards/Group/MembersList/MembersListCard.test.tsx | 2 +- .../src/components/Cards/Group/MembersList/MembersListCard.tsx | 2 +- plugins/org/src/helpers/helpers.test.ts | 2 +- 6 files changed, 5 insertions(+), 5 deletions(-) rename plugins/org/src/{test-helpers => __testUtils__}/catalogMocks.ts (100%) diff --git a/plugins/org/package.json b/plugins/org/package.json index 08d8f4bf9a..be270ff041 100644 --- a/plugins/org/package.json +++ b/plugins/org/package.json @@ -28,7 +28,6 @@ "clean": "backstage-cli package clean" }, "dependencies": { - "@backstage/catalog-client": "workspace:^", "@backstage/catalog-model": "workspace:^", "@backstage/core-components": "workspace:^", "@backstage/core-plugin-api": "workspace:^", @@ -48,6 +47,7 @@ "react-router-dom": "6.0.0-beta.0 || ^6.3.0" }, "devDependencies": { + "@backstage/catalog-client": "workspace:^", "@backstage/cli": "workspace:^", "@backstage/core-app-api": "workspace:^", "@backstage/dev-utils": "workspace:^", diff --git a/plugins/org/src/test-helpers/catalogMocks.ts b/plugins/org/src/__testUtils__/catalogMocks.ts similarity index 100% rename from plugins/org/src/test-helpers/catalogMocks.ts rename to plugins/org/src/__testUtils__/catalogMocks.ts 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 ec5718349d..a52752057b 100644 --- a/plugins/org/src/components/Cards/Group/MembersList/MembersListCard.stories.tsx +++ b/plugins/org/src/components/Cards/Group/MembersList/MembersListCard.stories.tsx @@ -23,7 +23,7 @@ import { MemoryRouter } from 'react-router-dom'; import { groupA, mockedCatalogApiSupportingGroups, -} from '../../../../test-helpers/catalogMocks'; +} from '../../../../__testUtils__/catalogMocks'; import { MembersListCard } from './MembersListCard'; export default { 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 fa823e1678..47e954eb17 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,7 @@ import { MembersListCard } from './MembersListCard'; import { groupA, mockedCatalogApiSupportingGroups, -} from '../../../../test-helpers/catalogMocks'; +} from '../../../../__testUtils__/catalogMocks'; import { permissionApiRef } from '@backstage/plugin-permission-react'; import { EntityLayout } from '@backstage/plugin-catalog'; import { screen } from '@testing-library/react'; diff --git a/plugins/org/src/components/Cards/Group/MembersList/MembersListCard.tsx b/plugins/org/src/components/Cards/Group/MembersList/MembersListCard.tsx index a27c9ab8ca..0283bf8588 100644 --- a/plugins/org/src/components/Cards/Group/MembersList/MembersListCard.tsx +++ b/plugins/org/src/components/Cards/Group/MembersList/MembersListCard.tsx @@ -167,7 +167,7 @@ export const MembersListCard = (props: { groupEntity, catalogApi, ); - }, [catalogApi, groupEntity]); + }, [catalogApi, groupEntity, showAggregateMembersToggle]); const { loading, error, diff --git a/plugins/org/src/helpers/helpers.test.ts b/plugins/org/src/helpers/helpers.test.ts index 714371d83d..4cee5422ce 100644 --- a/plugins/org/src/helpers/helpers.test.ts +++ b/plugins/org/src/helpers/helpers.test.ts @@ -38,7 +38,7 @@ import { groupEUserOne, duplicatedUser, mockedCatalogApiSupportingGroups, -} from '../test-helpers/catalogMocks'; +} from '../__testUtils__/catalogMocks'; describe('Helper functions', () => { it('getAllDesendantMembersForGroupEntity correctly recursively returns all descendant members', async () => { From 5cd1e26d50c9c5f64e9a632a8f084b8d705f89d7 Mon Sep 17 00:00:00 2001 From: Robert Bunning Date: Wed, 3 May 2023 16:26:28 -0400 Subject: [PATCH 10/11] Switched to ordering based off of stringifyEntityRef Signed-off-by: Robert Bunning --- .../Cards/Group/MembersList/MembersListCard.tsx | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/plugins/org/src/components/Cards/Group/MembersList/MembersListCard.tsx b/plugins/org/src/components/Cards/Group/MembersList/MembersListCard.tsx index 0283bf8588..0a5386ea20 100644 --- a/plugins/org/src/components/Cards/Group/MembersList/MembersListCard.tsx +++ b/plugins/org/src/components/Cards/Group/MembersList/MembersListCard.tsx @@ -193,12 +193,9 @@ export const MembersListCard = (props: { [ ...(directMembers ?? []), ...(descendantMembers && showAggregateMembers ? descendantMembers : []), - ].sort((a, b) => { - const nameToCompareInA = a.spec.profile?.displayName ?? a.metadata.name; - const nameToCompareInB = b.spec.profile?.displayName ?? b.metadata.name; - - return nameToCompareInA.localeCompare(nameToCompareInB); - }), + ].sort((a, b) => + stringifyEntityRef(a).localeCompare(stringifyEntityRef(b)), + ), ) as UserEntity[]; if (loading) { From 359c19b3dc93b2eac0fab25d9942a5ba2af2f775 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Thu, 4 May 2023 11:30:26 +0200 Subject: [PATCH 11/11] Update plugins/org/src/components/Cards/Group/MembersList/MembersListCard.stories.tsx Signed-off-by: Patrik Oldsberg --- .../Cards/Group/MembersList/MembersListCard.stories.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 a52752057b..6b34d8b934 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 2020 The Backstage Authors + * Copyright 2021 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.