From e90c18254893e7049ee9b527a1080ce08df70383 Mon Sep 17 00:00:00 2001 From: steff-petro <85114094+steff-petro@users.noreply.github.com> Date: Wed, 9 Aug 2023 20:32:52 +0200 Subject: [PATCH 1/5] replaced static catalog link with dynamic catalog index Signed-off-by: steff-petro <85114094+steff-petro@users.noreply.github.com> --- .../components/Cards/Group/MembersList/MembersListCard.tsx | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/plugins/org/src/components/Cards/Group/MembersList/MembersListCard.tsx b/plugins/org/src/components/Cards/Group/MembersList/MembersListCard.tsx index 0a5386ea20..202f32daa8 100644 --- a/plugins/org/src/components/Cards/Group/MembersList/MembersListCard.tsx +++ b/plugins/org/src/components/Cards/Group/MembersList/MembersListCard.tsx @@ -47,11 +47,12 @@ import { Link, OverflowTooltip, } from '@backstage/core-components'; -import { useApi } from '@backstage/core-plugin-api'; +import { useApi, useRouteRef } from '@backstage/core-plugin-api'; import { getAllDesendantMembersForGroupEntity, removeDuplicateEntitiesFrom, } from '../../../../helpers/helpers'; +import { catalogIndexRouteRef } from '../../../../routes'; const useStyles = makeStyles((theme: Theme) => createStyles({ @@ -70,6 +71,7 @@ const useStyles = makeStyles((theme: Theme) => const MemberComponent = (props: { member: UserEntity }) => { const classes = useStyles(); + const catalogLink = useRouteRef(catalogIndexRouteRef); const { metadata: { name: metaName, description }, spec: { profile }, @@ -105,7 +107,7 @@ const MemberComponent = (props: { member: UserEntity }) => { From 9806e3c8020142da8073d7f93ea985c403ddcf8f Mon Sep 17 00:00:00 2001 From: steff-petro <85114094+steff-petro@users.noreply.github.com> Date: Wed, 9 Aug 2023 21:04:28 +0200 Subject: [PATCH 2/5] corrected test functions Signed-off-by: steff-petro <85114094+steff-petro@users.noreply.github.com> --- .../MembersList/MembersListCard.test.tsx | 36 +++++++++++++++++++ 1 file changed, 36 insertions(+) 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 47e954eb17..ee31481fd0 100644 --- a/plugins/org/src/components/Cards/Group/MembersList/MembersListCard.test.tsx +++ b/plugins/org/src/components/Cards/Group/MembersList/MembersListCard.test.tsx @@ -39,6 +39,7 @@ import { EntityLayout } from '@backstage/plugin-catalog'; import { screen } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import { Observable } from '@backstage/types'; +import { catalogIndexRouteRef } from '../../../../routes'; // Mock needed because jsdom doesn't correctly implement box-sizing // https://github.com/ShinyChang/React-Text-Truncate/issues/70 @@ -124,6 +125,11 @@ describe('MemberTab Test', () => { , , + { + mountedRoutes: { + '/catalog': catalogIndexRouteRef, + }, + }, ), ); @@ -152,6 +158,11 @@ describe('MemberTab Test', () => { , + { + mountedRoutes: { + '/catalog': catalogIndexRouteRef, + }, + }, ), ); @@ -176,6 +187,11 @@ describe('MemberTab Test', () => { , + { + mountedRoutes: { + '/catalog': catalogIndexRouteRef, + }, + }, ); const toggleSwitch = screen.queryByRole('checkbox'); @@ -199,6 +215,11 @@ describe('MemberTab Test', () => { , + { + mountedRoutes: { + '/catalog': catalogIndexRouteRef, + }, + }, ); expect(screen.queryByRole('checkbox')).toBeInTheDocument(); @@ -221,6 +242,11 @@ describe('MemberTab Test', () => { , + { + mountedRoutes: { + '/catalog': catalogIndexRouteRef, + }, + }, ); const displayedMemberNames = screen.queryAllByTestId('user-link'); @@ -252,6 +278,11 @@ describe('MemberTab Test', () => { , + { + mountedRoutes: { + '/catalog': catalogIndexRouteRef, + }, + }, ); const displayedMemberNames = screen.queryAllByTestId('user-link'); @@ -283,6 +314,11 @@ describe('MemberTab Test', () => { , + { + mountedRoutes: { + '/catalog': catalogIndexRouteRef, + }, + }, ); // Click the toggle switch From 50331203e2a28940aeda2e1303d9243d8587881f Mon Sep 17 00:00:00 2001 From: steff-petro <85114094+steff-petro@users.noreply.github.com> Date: Wed, 9 Aug 2023 21:14:19 +0200 Subject: [PATCH 3/5] added changeset Signed-off-by: steff-petro <85114094+steff-petro@users.noreply.github.com> --- .changeset/healthy-tigers-brake.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/healthy-tigers-brake.md diff --git a/.changeset/healthy-tigers-brake.md b/.changeset/healthy-tigers-brake.md new file mode 100644 index 0000000000..c224d50583 --- /dev/null +++ b/.changeset/healthy-tigers-brake.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-org': patch +--- + +Fixed bug in MembersListCard component where the url for the members was static which was breaking the component when catalog route was changed From 743c172375c02032d238102b706183ffa4af84f8 Mon Sep 17 00:00:00 2001 From: steff-petro <85114094+steff-petro@users.noreply.github.com> Date: Wed, 9 Aug 2023 22:19:53 +0200 Subject: [PATCH 4/5] MembersListCard correction, test functions: wip Signed-off-by: steff-petro <85114094+steff-petro@users.noreply.github.com> --- .../MembersList/MembersListCard.test.tsx | 16 +++++++------- .../Group/MembersList/MembersListCard.tsx | 22 ++++--------------- 2 files changed, 12 insertions(+), 26 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 ee31481fd0..a8887baa0a 100644 --- a/plugins/org/src/components/Cards/Group/MembersList/MembersListCard.test.tsx +++ b/plugins/org/src/components/Cards/Group/MembersList/MembersListCard.test.tsx @@ -19,6 +19,7 @@ import { CatalogApi, catalogApiRef, EntityProvider, + entityRouteRef, StarredEntitiesApi, starredEntitiesApiRef, } from '@backstage/plugin-catalog-react'; @@ -39,7 +40,6 @@ import { EntityLayout } from '@backstage/plugin-catalog'; import { screen } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import { Observable } from '@backstage/types'; -import { catalogIndexRouteRef } from '../../../../routes'; // Mock needed because jsdom doesn't correctly implement box-sizing // https://github.com/ShinyChang/React-Text-Truncate/issues/70 @@ -127,7 +127,7 @@ describe('MemberTab Test', () => { , { mountedRoutes: { - '/catalog': catalogIndexRouteRef, + '/catalog/:namespace/:kind/:name': entityRouteRef, }, }, ), @@ -160,7 +160,7 @@ describe('MemberTab Test', () => { , { mountedRoutes: { - '/catalog': catalogIndexRouteRef, + '/catalog/:namespace/:kind/:name': entityRouteRef, }, }, ), @@ -189,7 +189,7 @@ describe('MemberTab Test', () => { , { mountedRoutes: { - '/catalog': catalogIndexRouteRef, + '/catalog/:namespace/:kind/:name': entityRouteRef, }, }, ); @@ -217,7 +217,7 @@ describe('MemberTab Test', () => { , { mountedRoutes: { - '/catalog': catalogIndexRouteRef, + '/catalog/:namespace/:kind/:name': entityRouteRef, }, }, ); @@ -244,7 +244,7 @@ describe('MemberTab Test', () => { , { mountedRoutes: { - '/catalog': catalogIndexRouteRef, + '/catalog/:namespace/:kind/:name': entityRouteRef, }, }, ); @@ -280,7 +280,7 @@ describe('MemberTab Test', () => { , { mountedRoutes: { - '/catalog': catalogIndexRouteRef, + '/catalog/:namespace/:kind/:name': entityRouteRef, }, }, ); @@ -316,7 +316,7 @@ describe('MemberTab Test', () => { , { mountedRoutes: { - '/catalog': catalogIndexRouteRef, + '/catalog/:namespace/:kind/:name': entityRouteRef, }, }, ); diff --git a/plugins/org/src/components/Cards/Group/MembersList/MembersListCard.tsx b/plugins/org/src/components/Cards/Group/MembersList/MembersListCard.tsx index 202f32daa8..0eaa079a4e 100644 --- a/plugins/org/src/components/Cards/Group/MembersList/MembersListCard.tsx +++ b/plugins/org/src/components/Cards/Group/MembersList/MembersListCard.tsx @@ -20,11 +20,7 @@ import { UserEntity, stringifyEntityRef, } from '@backstage/catalog-model'; -import { - catalogApiRef, - entityRouteParams, - useEntity, -} from '@backstage/plugin-catalog-react'; +import { catalogApiRef, useEntity } from '@backstage/plugin-catalog-react'; import { Box, createStyles, @@ -36,7 +32,6 @@ import { } from '@material-ui/core'; import Pagination from '@material-ui/lab/Pagination'; import React, { useState } from 'react'; -import { generatePath } from 'react-router-dom'; import useAsync from 'react-use/lib/useAsync'; import { @@ -47,12 +42,12 @@ import { Link, OverflowTooltip, } from '@backstage/core-components'; -import { useApi, useRouteRef } from '@backstage/core-plugin-api'; +import { useApi } from '@backstage/core-plugin-api'; import { getAllDesendantMembersForGroupEntity, removeDuplicateEntitiesFrom, } from '../../../../helpers/helpers'; -import { catalogIndexRouteRef } from '../../../../routes'; +import { EntityRefLink } from '@backstage/plugin-catalog-react'; const useStyles = makeStyles((theme: Theme) => createStyles({ @@ -71,7 +66,6 @@ const useStyles = makeStyles((theme: Theme) => const MemberComponent = (props: { member: UserEntity }) => { const classes = useStyles(); - const catalogLink = useRouteRef(catalogIndexRouteRef); const { metadata: { name: metaName, description }, spec: { profile }, @@ -104,15 +98,7 @@ const MemberComponent = (props: { member: UserEntity }) => { textAlign="center" > - - - + {profile?.email && ( From beb5b2f4f15652056e1e6181f0f1a7b3331b0ef0 Mon Sep 17 00:00:00 2001 From: Stefan Petrovic Date: Thu, 10 Aug 2023 14:21:49 +0200 Subject: [PATCH 5/5] adjusted tests and added testid prop Signed-off-by: Stefan Petrovic --- .../Group/MembersList/MembersListCard.test.tsx | 15 --------------- .../Cards/Group/MembersList/MembersListCard.tsx | 6 +++++- 2 files changed, 5 insertions(+), 16 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 a8887baa0a..f5a2d50cb5 100644 --- a/plugins/org/src/components/Cards/Group/MembersList/MembersListCard.test.tsx +++ b/plugins/org/src/components/Cards/Group/MembersList/MembersListCard.test.tsx @@ -193,11 +193,9 @@ describe('MemberTab Test', () => { }, }, ); - 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( { }, }, ); - 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(); @@ -260,7 +254,6 @@ describe('MemberTab Test', () => { duplicatedUserText.compareDocumentPosition(groupAUserOneText), ).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(); @@ -296,7 +287,6 @@ describe('MemberTab Test', () => { 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); diff --git a/plugins/org/src/components/Cards/Group/MembersList/MembersListCard.tsx b/plugins/org/src/components/Cards/Group/MembersList/MembersListCard.tsx index 0eaa079a4e..3716af5522 100644 --- a/plugins/org/src/components/Cards/Group/MembersList/MembersListCard.tsx +++ b/plugins/org/src/components/Cards/Group/MembersList/MembersListCard.tsx @@ -98,7 +98,11 @@ const MemberComponent = (props: { member: UserEntity }) => { textAlign="center" > - + {profile?.email && (