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 <rbunning@webstaurantstore.com>
This commit is contained in:
Robert Bunning
2023-04-27 14:55:23 -04:00
parent 54da67d820
commit 288f7360bb
6 changed files with 174 additions and 92 deletions
+12 -1
View File
@@ -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 = (
// ...
<EntityMembersListCard showAggregateMembersToggle />
// ...
);
```
@@ -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 = () => (
</TestApiProvider>
</MemoryRouter>
);
export const AggregateMembersToggle = () => (
<MemoryRouter>
<TestApiProvider apis={[[catalogApiRef, mockedCatalogApiSupportingGroups]]}>
<EntityProvider entity={groupA}>
<Grid container spacing={4}>
<Grid item xs={12} md={6}>
<MembersListCard showAggregateMembersToggle />
</Grid>
</Grid>
</EntityProvider>
</TestApiProvider>
</MemoryRouter>
);
@@ -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(
<TestApiProvider
apis={[
[catalogApiRef, mockedCatalogApiSupportingGroups],
[starredEntitiesApiRef, mockedStarredEntitiesApi],
[permissionApiRef, {}],
]}
>
<EntityProvider entity={groupA}>
<EntityLayout>
<EntityLayout.Route path="/" title="Title">
<MembersListCard />
</EntityLayout.Route>
</EntityLayout>
</EntityProvider>
</TestApiProvider>,
);
const toggleSwitch = screen.queryByRole('checkbox');
expect(toggleSwitch).toBeNull();
});
it('Shows the aggregate members toggle if the showAggregateMembersToggle prop is true', async () => {
await renderInTestApp(
<TestApiProvider
apis={[
[catalogApiRef, mockedCatalogApiSupportingGroups],
[starredEntitiesApiRef, mockedStarredEntitiesApi],
[permissionApiRef, {}],
]}
>
<EntityProvider entity={groupA}>
<EntityLayout>
<EntityLayout.Route path="/" title="Title">
<MembersListCard showAggregateMembersToggle />
</EntityLayout.Route>
</EntityLayout>
</EntityProvider>
</TestApiProvider>,
);
expect(screen.queryByRole('checkbox')).toBeInTheDocument();
});
it('Shows only direct members if the showAggregateMembersToggle prop is undefined', async () => {
await renderInTestApp(
<TestApiProvider
apis={[
@@ -197,6 +235,37 @@ describe('MemberTab Test', () => {
).toBe(Node.DOCUMENT_POSITION_FOLLOWING);
});
it('Shows only direct members if the aggregate members switch is turned off', async () => {
await renderInTestApp(
<TestApiProvider
apis={[
[catalogApiRef, mockedCatalogApiSupportingGroups],
[starredEntitiesApiRef, mockedStarredEntitiesApi],
[permissionApiRef, {}],
]}
>
<EntityProvider entity={groupA}>
<EntityLayout>
<EntityLayout.Route path="/" title="Title">
<MembersListCard showAggregateMembersToggle />
</EntityLayout.Route>
</EntityLayout>
</EntityProvider>
</TestApiProvider>,
);
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(
<TestApiProvider
@@ -209,7 +278,7 @@ describe('MemberTab Test', () => {
<EntityProvider entity={groupA}>
<EntityLayout>
<EntityLayout.Route path="/" title="Title">
<MembersListCard />
<MembersListCard showAggregateMembersToggle />
</EntityLayout.Route>
</EntityLayout>
</EntityProvider>
@@ -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<GroupEntity>();
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
<Switch
color="primary"
checked={showAggregateUsers}
onChange={() => {
setShowAggregateUsers(!showAggregateUsers);
}}
inputProps={{ 'aria-label': 'Users Type Switch' }}
/>
Aggregated Members
{showAggregateMembersToggle && (
<>
Direct Members
<Switch
color="primary"
checked={showAggregateMembers}
onChange={() => {
setShowAggregateMembers(!showAggregateMembers);
}}
inputProps={{ 'aria-label': 'Users Type Switch' }}
/>
Aggregated Members
</>
)}
<Grid container spacing={3}>
{members && members.length > 0 ? (
members
-8
View File
@@ -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;
+43 -60
View File
@@ -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<string, MockedRelationsType>([
[
'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<string, GroupEntity>([
['group:default/group-a', groupA],
['group:default/group-b', groupB],
@@ -319,10 +313,6 @@ const mockedMembersMapping = new Map<string, Entity[]>([
]);
type Nullable<T> = T | undefined;
const nullMockedRelationsType: MockedRelationsType = {
ownedBy: [],
parentOf: [],
};
export const mockedCatalogApiSupportingGroups: Partial<CatalogApi> = {
getEntities: async (request?: GetEntitiesRequest) => {
@@ -348,10 +338,3 @@ export const mockedCatalogApiSupportingGroups: Partial<CatalogApi> = {
return { items };
},
};
export const mockedGetEntityRelations = (
entity: Nullable<Entity>,
relationType: string,
) =>
(mockedRelationsCatalog.get(entity?.metadata.name ?? 'NULL') ??
nullMockedRelationsType)[relationType];