From e38ee46137506c050f69473532292578b7f8c52c Mon Sep 17 00:00:00 2001 From: ecarrill Date: Thu, 8 Feb 2024 08:33:57 -0800 Subject: [PATCH 01/11] upcoming: [M3-7611] - PlacementGroups Summary --- .../PlacementGroupsDetail.tsx | 5 +- .../PlacementGroupsSummary.tsx | 81 +++++++++++++++++++ 2 files changed, 85 insertions(+), 1 deletion(-) create mode 100644 packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsSummary/PlacementGroupsSummary.tsx diff --git a/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsDetail.tsx b/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsDetail.tsx index 481c25e4e5f..799eab80187 100644 --- a/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsDetail.tsx +++ b/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsDetail.tsx @@ -20,6 +20,7 @@ import { getErrorStringOrDefault } from 'src/utilities/errorUtils'; import { getPlacementGroupLinodeCount } from '../utils'; import { PlacementGroupsLinodes } from './PlacementGroupsLinodes/PlacementGroupsLinodes'; +import { PlacementGroupsSummary } from './PlacementGroupsSummary/PlacementGroupsSummary'; export const PlacementGroupsDetail = () => { const flags = useFlags(); @@ -108,7 +109,9 @@ export const PlacementGroupsDetail = () => { > - TODO VM_Placement: summary + + + diff --git a/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsSummary/PlacementGroupsSummary.tsx b/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsSummary/PlacementGroupsSummary.tsx new file mode 100644 index 00000000000..0ac4d7bc075 --- /dev/null +++ b/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsSummary/PlacementGroupsSummary.tsx @@ -0,0 +1,81 @@ +import { AFFINITY_TYPES } from '@linode/api-v4'; +import Grid from '@mui/material/Unstable_Grid2'; +import { styled } from '@mui/material/styles'; +import * as React from 'react'; + +import { Box } from 'src/components/Box'; +import { Paper } from 'src/components/Paper'; +import { TooltipIcon } from 'src/components/TooltipIcon'; +import { Typography } from 'src/components/Typography'; +import { useRegionsQuery } from 'src/queries/regions'; + +import type { PlacementGroup } from '@linode/api-v4'; + +const tooltipText = `The Affinity Type and Region determine the maximum number of VMs per group`; + +interface Props { + placementGroup: PlacementGroup; +} + +export const PlacementGroupsSummary = (props: Props) => { + const { placementGroup } = props; + const { data: regions } = useRegionsQuery(); + const regionLabel = + regions?.find((region) => region.id === placementGroup.region)?.label ?? + placementGroup.region; + + return ( + + ({ marginBottom: theme.spacing(3) })} + variant="h3" + > + Placement Group Configuration + + + + + + Linodes + + {`${placementGroup.linode_ids.length} of ${placementGroup.capacity}`} + + + + + + + + Affinity Type + + {AFFINITY_TYPES[placementGroup?.affinity_type]} + + + + + + + Region + {regionLabel} + + + + + ); +}; + +export const StyledLabel = styled(Typography, { + label: 'StyledLabel', +})(({ theme }) => ({ + fontFamily: theme.font.bold, + marginRight: theme.spacing(8), + width: theme.spacing(10), +})); From 342169c7d2035183b3fb08d8189eed3413e0edfc Mon Sep 17 00:00:00 2001 From: ecarrill Date: Thu, 8 Feb 2024 10:22:08 -0800 Subject: [PATCH 02/11] Add notice for non-compliant placement groups --- .../PlacementGroupsSummary.tsx | 101 ++++++++++-------- 1 file changed, 58 insertions(+), 43 deletions(-) diff --git a/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsSummary/PlacementGroupsSummary.tsx b/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsSummary/PlacementGroupsSummary.tsx index 0ac4d7bc075..ef001b7dafa 100644 --- a/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsSummary/PlacementGroupsSummary.tsx +++ b/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsSummary/PlacementGroupsSummary.tsx @@ -2,8 +2,10 @@ import { AFFINITY_TYPES } from '@linode/api-v4'; import Grid from '@mui/material/Unstable_Grid2'; import { styled } from '@mui/material/styles'; import * as React from 'react'; +import { Link } from 'react-router-dom'; import { Box } from 'src/components/Box'; +import { Notice } from 'src/components/Notice/Notice'; import { Paper } from 'src/components/Paper'; import { TooltipIcon } from 'src/components/TooltipIcon'; import { Typography } from 'src/components/Typography'; @@ -20,55 +22,68 @@ interface Props { export const PlacementGroupsSummary = (props: Props) => { const { placementGroup } = props; const { data: regions } = useRegionsQuery(); + const regionLabel = regions?.find((region) => region.id === placementGroup.region)?.label ?? placementGroup.region; - return ( - - ({ marginBottom: theme.spacing(3) })} - variant="h3" - > - Placement Group Configuration - - - - - - Linodes - - {`${placementGroup.linode_ids.length} of ${placementGroup.capacity}`} - - - - + const warningNoticeText = `Placement Group ${placementGroup.label} (${placementGroup.affinity_type}) is non-compliant. + We are working to resolve compliance issues so that you can continue assigning Linodes to this Placement Group.`; - - - Affinity Type - - {AFFINITY_TYPES[placementGroup?.affinity_type]} - - - - - - - Region - {regionLabel} - + return ( + <> + {!placementGroup.compliant && ( + + {warningNoticeText} +
+ {/* TODO VM_Placement: Get link location */} + + Learn more. + + + )} + + ({ marginBottom: theme.spacing(3) })} + variant="h3" + > + Placement Group Configuration + + + + + Linodes + + {`${placementGroup.linode_ids.length} of ${placementGroup.capacity}`} + + + + + + + Affinity Type + + {AFFINITY_TYPES[placementGroup?.affinity_type]} + + + + + + Region + {regionLabel} + + - - + + ); }; From 869c4d8ee2d4b85c7ea7b96eb6b77735312cb90d Mon Sep 17 00:00:00 2001 From: ecarrill Date: Thu, 8 Feb 2024 10:30:12 -0800 Subject: [PATCH 03/11] formatting --- .../PlacementGroupsSummary/PlacementGroupsSummary.tsx | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsSummary/PlacementGroupsSummary.tsx b/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsSummary/PlacementGroupsSummary.tsx index ef001b7dafa..2595aff5f6e 100644 --- a/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsSummary/PlacementGroupsSummary.tsx +++ b/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsSummary/PlacementGroupsSummary.tsx @@ -27,7 +27,9 @@ export const PlacementGroupsSummary = (props: Props) => { regions?.find((region) => region.id === placementGroup.region)?.label ?? placementGroup.region; - const warningNoticeText = `Placement Group ${placementGroup.label} (${placementGroup.affinity_type}) is non-compliant. + const warningNoticeText = `Placement Group ${placementGroup.label} (${ + AFFINITY_TYPES[placementGroup?.affinity_type] + }) is non-compliant. We are working to resolve compliance issues so that you can continue assigning Linodes to this Placement Group.`; return ( From 4a4db26ac5bccde6bb1979258569a527cf5a073e Mon Sep 17 00:00:00 2001 From: ecarrill Date: Fri, 9 Feb 2024 05:54:10 -0800 Subject: [PATCH 04/11] PR feedback --- .../PlacementGroupsDetail.tsx | 24 ++++ .../PlacementGroupsSummary.test.tsx | 41 +++++++ .../PlacementGroupsSummary.tsx | 103 +++++++----------- .../src/features/PlacementGroups/constants.ts | 2 + .../src/features/PlacementGroups/utils.ts | 10 ++ 5 files changed, 119 insertions(+), 61 deletions(-) create mode 100644 packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsSummary/PlacementGroupsSummary.test.tsx diff --git a/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsDetail.tsx b/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsDetail.tsx index 799eab80187..374aefd5da3 100644 --- a/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsDetail.tsx +++ b/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsDetail.tsx @@ -1,16 +1,20 @@ import { AFFINITY_TYPES } from '@linode/api-v4'; +import { useTheme } from '@mui/material'; import * as React from 'react'; import { useHistory, useParams } from 'react-router-dom'; +import { Link } from 'react-router-dom'; import { CircleProgress } from 'src/components/CircleProgress'; import { DocumentTitleSegment } from 'src/components/DocumentTitle'; import { ErrorState } from 'src/components/ErrorState/ErrorState'; import { LandingHeader } from 'src/components/LandingHeader'; import { NotFound } from 'src/components/NotFound'; +import { Notice } from 'src/components/Notice/Notice'; import { SafeTabPanel } from 'src/components/Tabs/SafeTabPanel'; import { TabLinkList } from 'src/components/Tabs/TabLinkList'; import { TabPanels } from 'src/components/Tabs/TabPanels'; import { Tabs } from 'src/components/Tabs/Tabs'; +import { Typography } from 'src/components/Typography'; import { useFlags } from 'src/hooks/useFlags'; import { useMutatePlacementGroup, @@ -19,6 +23,7 @@ import { import { getErrorStringOrDefault } from 'src/utilities/errorUtils'; import { getPlacementGroupLinodeCount } from '../utils'; +import { getWarningNoticeText } from '../utils'; import { PlacementGroupsLinodes } from './PlacementGroupsLinodes/PlacementGroupsLinodes'; import { PlacementGroupsSummary } from './PlacementGroupsSummary/PlacementGroupsSummary'; @@ -26,17 +31,21 @@ export const PlacementGroupsDetail = () => { const flags = useFlags(); const { id, tab } = useParams<{ id: string; tab?: string }>(); const history = useHistory(); + const theme = useTheme(); const placementGroupId = Number(id); + const { data: placementGroup, error: placementGroupError, isLoading, } = usePlacementGroupQuery(placementGroupId, Boolean(flags.vmPlacement)); + const { error: updatePlacementGroupError, mutateAsync: updatePlacementGroup, reset, } = useMutatePlacementGroup(placementGroupId); + const errorText = getErrorStringOrDefault(updatePlacementGroupError ?? ''); if (isLoading) { @@ -110,6 +119,21 @@ export const PlacementGroupsDetail = () => { + {!placementGroup.compliant && ( + + + {getWarningNoticeText(placementGroup)} + {/* TODO VM_Placement: Get link location */} + + Learn more. + + + + )} diff --git a/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsSummary/PlacementGroupsSummary.test.tsx b/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsSummary/PlacementGroupsSummary.test.tsx new file mode 100644 index 00000000000..0055c9ee70e --- /dev/null +++ b/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsSummary/PlacementGroupsSummary.test.tsx @@ -0,0 +1,41 @@ +import * as React from 'react'; + +import { placementGroupFactory } from 'src/factories'; +import { renderWithTheme } from 'src/utilities/testHelpers'; + +import { PlacementGroupsSummary } from './PlacementGroupsSummary'; + +const queryMocks = vi.hoisted(() => ({ + usePlacementGroupQuery: vi.fn().mockReturnValue({}), +})); + +vi.mock('src/queries/placementGroups', async () => { + const actual = await vi.importActual('src/queries/placementGroups'); + return { + ...actual, + usePlacementGroupQuery: queryMocks.usePlacementGroupQuery, + }; +}); + +describe('PlacementGroups Summary', () => { + it('renders the placement group detail summary panel', () => { + const { getByText } = renderWithTheme( + + ); + + expect(getByText('Placement Group Configuration')).toBeInTheDocument(); + expect(getByText('Linodes')).toBeInTheDocument(); + expect(getByText('Affinity Type')).toBeInTheDocument(); + expect(getByText('Region')).toBeInTheDocument(); + }); +}); diff --git a/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsSummary/PlacementGroupsSummary.tsx b/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsSummary/PlacementGroupsSummary.tsx index 2595aff5f6e..b4ba2a2ded5 100644 --- a/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsSummary/PlacementGroupsSummary.tsx +++ b/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsSummary/PlacementGroupsSummary.tsx @@ -2,18 +2,16 @@ import { AFFINITY_TYPES } from '@linode/api-v4'; import Grid from '@mui/material/Unstable_Grid2'; import { styled } from '@mui/material/styles'; import * as React from 'react'; -import { Link } from 'react-router-dom'; import { Box } from 'src/components/Box'; -import { Notice } from 'src/components/Notice/Notice'; import { Paper } from 'src/components/Paper'; import { TooltipIcon } from 'src/components/TooltipIcon'; import { Typography } from 'src/components/Typography'; import { useRegionsQuery } from 'src/queries/regions'; -import type { PlacementGroup } from '@linode/api-v4'; +import { PLACEMENT_GROUP_TOOLTIP_TEXT } from '../../constants'; -const tooltipText = `The Affinity Type and Region determine the maximum number of VMs per group`; +import type { PlacementGroup } from '@linode/api-v4'; interface Props { placementGroup: PlacementGroup; @@ -27,65 +25,48 @@ export const PlacementGroupsSummary = (props: Props) => { regions?.find((region) => region.id === placementGroup.region)?.label ?? placementGroup.region; - const warningNoticeText = `Placement Group ${placementGroup.label} (${ - AFFINITY_TYPES[placementGroup?.affinity_type] - }) is non-compliant. - We are working to resolve compliance issues so that you can continue assigning Linodes to this Placement Group.`; - return ( - <> - {!placementGroup.compliant && ( - - {warningNoticeText} -
- {/* TODO VM_Placement: Get link location */} - - Learn more. - - - )} - - ({ marginBottom: theme.spacing(3) })} - variant="h3" - > - Placement Group Configuration - - - - - Linodes - - {`${placementGroup.linode_ids.length} of ${placementGroup.capacity}`} - - - - - - - Affinity Type - - {AFFINITY_TYPES[placementGroup?.affinity_type]} - - - - - - Region - {regionLabel} - - + + ({ marginBottom: theme.spacing(3) })} + variant="h3" + > + Placement Group Configuration + + + + + Linodes + + {`${placementGroup.linode_ids.length} of ${placementGroup.capacity}`} + + + + + + + Affinity Type + + {AFFINITY_TYPES[placementGroup?.affinity_type]} + + + + + + Region + {regionLabel} + - - + + ); }; diff --git a/packages/manager/src/features/PlacementGroups/constants.ts b/packages/manager/src/features/PlacementGroups/constants.ts index 4409b6f113b..8a8d484e1eb 100644 --- a/packages/manager/src/features/PlacementGroups/constants.ts +++ b/packages/manager/src/features/PlacementGroups/constants.ts @@ -8,3 +8,5 @@ export const MAX_NUMBER_OF_LINODES_IN_PLACEMENT_GROUP_MESSAGE = export const PLACEMENT_GROUP_LINODES_ERROR_MESSAGE = 'There was an error loading Linodes for this Placement Group.'; + +export const PLACEMENT_GROUP_TOOLTIP_TEXT = `The Affinity Type and Region determine the maximum number of VMs per group`; diff --git a/packages/manager/src/features/PlacementGroups/utils.ts b/packages/manager/src/features/PlacementGroups/utils.ts index 3513d5e32ac..9c7d8b0874b 100644 --- a/packages/manager/src/features/PlacementGroups/utils.ts +++ b/packages/manager/src/features/PlacementGroups/utils.ts @@ -32,3 +32,13 @@ export const affinityTypeOptions = Object.entries(AFFINITY_TYPES).map( value: key as CreatePlacementGroupPayload['affinity_type'], }) ); + +/** + * Helper to generate the text content of the non-compliant warning notice. + */ +export const getWarningNoticeText = (placementGroup: PlacementGroup) => { + return `Placement Group ${placementGroup.label} (${ + AFFINITY_TYPES[placementGroup?.affinity_type] + }) is non-compliant. +We are working to resolve compliance issues so that you can continue assigning Linodes to this Placement Group. `; +}; From 816c88586628ae824b0e0526317e264eb13b53a1 Mon Sep 17 00:00:00 2001 From: ecarrill Date: Fri, 9 Feb 2024 06:03:59 -0800 Subject: [PATCH 05/11] Add changeset --- .../.changeset/pr-10164-upcoming-features-1707487417041.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 packages/manager/.changeset/pr-10164-upcoming-features-1707487417041.md diff --git a/packages/manager/.changeset/pr-10164-upcoming-features-1707487417041.md b/packages/manager/.changeset/pr-10164-upcoming-features-1707487417041.md new file mode 100644 index 00000000000..f3958ffdf5a --- /dev/null +++ b/packages/manager/.changeset/pr-10164-upcoming-features-1707487417041.md @@ -0,0 +1,5 @@ +--- +"@linode/manager": Upcoming Features +--- + +Create PlacementGroups Summary component ([#10164](https://github.com/linode/manager/pull/10164)) From 82f8367a490cf50147a0c45e3f3762195bf14774 Mon Sep 17 00:00:00 2001 From: ecarrill Date: Fri, 9 Feb 2024 09:28:12 -0800 Subject: [PATCH 06/11] Remove stale code block from test --- .../PlacementGroupsSummary.test.tsx | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsSummary/PlacementGroupsSummary.test.tsx b/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsSummary/PlacementGroupsSummary.test.tsx index 0055c9ee70e..25f085090a0 100644 --- a/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsSummary/PlacementGroupsSummary.test.tsx +++ b/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsSummary/PlacementGroupsSummary.test.tsx @@ -5,18 +5,6 @@ import { renderWithTheme } from 'src/utilities/testHelpers'; import { PlacementGroupsSummary } from './PlacementGroupsSummary'; -const queryMocks = vi.hoisted(() => ({ - usePlacementGroupQuery: vi.fn().mockReturnValue({}), -})); - -vi.mock('src/queries/placementGroups', async () => { - const actual = await vi.importActual('src/queries/placementGroups'); - return { - ...actual, - usePlacementGroupQuery: queryMocks.usePlacementGroupQuery, - }; -}); - describe('PlacementGroups Summary', () => { it('renders the placement group detail summary panel', () => { const { getByText } = renderWithTheme( From ec51f1309f627d4b5428ca3f3aea6ef882b98925 Mon Sep 17 00:00:00 2001 From: ecarrill Date: Fri, 9 Feb 2024 12:17:48 -0800 Subject: [PATCH 07/11] Move notice component into summary panel --- .../PlacementGroupsDetail.tsx | 21 ---- .../PlacementGroupsSummary.tsx | 110 +++++++++++------- .../src/features/PlacementGroups/utils.ts | 10 -- 3 files changed, 67 insertions(+), 74 deletions(-) diff --git a/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsDetail.tsx b/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsDetail.tsx index 374aefd5da3..653f6f4cf06 100644 --- a/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsDetail.tsx +++ b/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsDetail.tsx @@ -1,20 +1,16 @@ import { AFFINITY_TYPES } from '@linode/api-v4'; -import { useTheme } from '@mui/material'; import * as React from 'react'; import { useHistory, useParams } from 'react-router-dom'; -import { Link } from 'react-router-dom'; import { CircleProgress } from 'src/components/CircleProgress'; import { DocumentTitleSegment } from 'src/components/DocumentTitle'; import { ErrorState } from 'src/components/ErrorState/ErrorState'; import { LandingHeader } from 'src/components/LandingHeader'; import { NotFound } from 'src/components/NotFound'; -import { Notice } from 'src/components/Notice/Notice'; import { SafeTabPanel } from 'src/components/Tabs/SafeTabPanel'; import { TabLinkList } from 'src/components/Tabs/TabLinkList'; import { TabPanels } from 'src/components/Tabs/TabPanels'; import { Tabs } from 'src/components/Tabs/Tabs'; -import { Typography } from 'src/components/Typography'; import { useFlags } from 'src/hooks/useFlags'; import { useMutatePlacementGroup, @@ -23,7 +19,6 @@ import { import { getErrorStringOrDefault } from 'src/utilities/errorUtils'; import { getPlacementGroupLinodeCount } from '../utils'; -import { getWarningNoticeText } from '../utils'; import { PlacementGroupsLinodes } from './PlacementGroupsLinodes/PlacementGroupsLinodes'; import { PlacementGroupsSummary } from './PlacementGroupsSummary/PlacementGroupsSummary'; @@ -31,7 +26,6 @@ export const PlacementGroupsDetail = () => { const flags = useFlags(); const { id, tab } = useParams<{ id: string; tab?: string }>(); const history = useHistory(); - const theme = useTheme(); const placementGroupId = Number(id); const { @@ -119,21 +113,6 @@ export const PlacementGroupsDetail = () => { - {!placementGroup.compliant && ( - - - {getWarningNoticeText(placementGroup)} - {/* TODO VM_Placement: Get link location */} - - Learn more. - - - - )} diff --git a/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsSummary/PlacementGroupsSummary.tsx b/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsSummary/PlacementGroupsSummary.tsx index b4ba2a2ded5..18589c6050d 100644 --- a/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsSummary/PlacementGroupsSummary.tsx +++ b/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsSummary/PlacementGroupsSummary.tsx @@ -1,9 +1,12 @@ import { AFFINITY_TYPES } from '@linode/api-v4'; +import { useTheme } from '@mui/material'; import Grid from '@mui/material/Unstable_Grid2'; import { styled } from '@mui/material/styles'; import * as React from 'react'; import { Box } from 'src/components/Box'; +import { Link } from 'src/components/Link'; +import { Notice } from 'src/components/Notice/Notice'; import { Paper } from 'src/components/Paper'; import { TooltipIcon } from 'src/components/TooltipIcon'; import { Typography } from 'src/components/Typography'; @@ -20,53 +23,74 @@ interface Props { export const PlacementGroupsSummary = (props: Props) => { const { placementGroup } = props; const { data: regions } = useRegionsQuery(); + const theme = useTheme(); - const regionLabel = - regions?.find((region) => region.id === placementGroup.region)?.label ?? - placementGroup.region; + const regionLabel = regions?.find( + (region) => region.id === placementGroup.region + )?.label; return ( - - ({ marginBottom: theme.spacing(3) })} - variant="h3" - > - Placement Group Configuration - - - - - Linodes - - {`${placementGroup.linode_ids.length} of ${placementGroup.capacity}`} - - - + <> + {!placementGroup.compliant && ( + + + {`Placement Group ${placementGroup.label} (${ + AFFINITY_TYPES[placementGroup.affinity_type] + }) is non-compliant. + We are working to resolve compliance issues so that you can continue assigning Linodes to this Placement Group. `} + {/* TODO VM_Placement: Get link location */} + + Learn more. + + + + )} + + ({ marginBottom: theme.spacing(3) })} + variant="h3" + > + Placement Group Configuration + + + + + Linodes + + {`${placementGroup.linode_ids.length} of ${placementGroup.capacity}`} + + + + + + + Affinity Type + + {AFFINITY_TYPES[placementGroup?.affinity_type]} + + + + + + Region + {regionLabel} + + - - - Affinity Type - - {AFFINITY_TYPES[placementGroup?.affinity_type]} - - - - - - Region - {regionLabel} - - - - + + ); }; diff --git a/packages/manager/src/features/PlacementGroups/utils.ts b/packages/manager/src/features/PlacementGroups/utils.ts index 9c7d8b0874b..3513d5e32ac 100644 --- a/packages/manager/src/features/PlacementGroups/utils.ts +++ b/packages/manager/src/features/PlacementGroups/utils.ts @@ -32,13 +32,3 @@ export const affinityTypeOptions = Object.entries(AFFINITY_TYPES).map( value: key as CreatePlacementGroupPayload['affinity_type'], }) ); - -/** - * Helper to generate the text content of the non-compliant warning notice. - */ -export const getWarningNoticeText = (placementGroup: PlacementGroup) => { - return `Placement Group ${placementGroup.label} (${ - AFFINITY_TYPES[placementGroup?.affinity_type] - }) is non-compliant. -We are working to resolve compliance issues so that you can continue assigning Linodes to this Placement Group. `; -}; From e74ed232129619c44abcf7f25308fe25ab2eaef4 Mon Sep 17 00:00:00 2001 From: ecarrill Date: Fri, 9 Feb 2024 14:43:50 -0800 Subject: [PATCH 08/11] Add PR feedback changes --- .../PlacementGroupsSummary/PlacementGroupsSummary.tsx | 3 ++- packages/manager/src/features/PlacementGroups/constants.ts | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsSummary/PlacementGroupsSummary.tsx b/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsSummary/PlacementGroupsSummary.tsx index 18589c6050d..c3918867a05 100644 --- a/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsSummary/PlacementGroupsSummary.tsx +++ b/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsSummary/PlacementGroupsSummary.tsx @@ -44,8 +44,9 @@ export const PlacementGroupsSummary = (props: Props) => { data-testid="pg-non-compliant-notice-link" to={'#'} > - Learn more. + Learn more + . )} diff --git a/packages/manager/src/features/PlacementGroups/constants.ts b/packages/manager/src/features/PlacementGroups/constants.ts index 8a8d484e1eb..f82c19f0b92 100644 --- a/packages/manager/src/features/PlacementGroups/constants.ts +++ b/packages/manager/src/features/PlacementGroups/constants.ts @@ -9,4 +9,4 @@ export const MAX_NUMBER_OF_LINODES_IN_PLACEMENT_GROUP_MESSAGE = export const PLACEMENT_GROUP_LINODES_ERROR_MESSAGE = 'There was an error loading Linodes for this Placement Group.'; -export const PLACEMENT_GROUP_TOOLTIP_TEXT = `The Affinity Type and Region determine the maximum number of VMs per group`; +export const PLACEMENT_GROUP_TOOLTIP_TEXT = `The Affinity Type and Region determine the maximum number of VMs per group.`; From 9564a615ad5ba747a5e6478aae9e567c41a4dc0a Mon Sep 17 00:00:00 2001 From: ecarrill Date: Mon, 12 Feb 2024 04:50:11 -0800 Subject: [PATCH 09/11] Fix failing test --- .../PlacementGroupsDetail/PlacementGroupsDetail.test.tsx | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsDetail.test.tsx b/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsDetail.test.tsx index 985f36349a7..8b50a031776 100644 --- a/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsDetail.test.tsx +++ b/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsDetail.test.tsx @@ -47,6 +47,7 @@ describe('PlacementGroupsLanding', () => { queryMocks.usePlacementGroupQuery.mockReturnValue({ data: placementGroupFactory.build({ affinity_type: 'anti_affinity', + compliant: true, id: 1, label: 'My first PG', }), From 39b0dbabecac8d3d74ff86a2e095e1a5cf642571 Mon Sep 17 00:00:00 2001 From: ecarrill Date: Mon, 12 Feb 2024 07:06:21 -0800 Subject: [PATCH 10/11] Add check for tooltip in unit test --- .../PlacementGroupsSummary/PlacementGroupsSummary.test.tsx | 3 ++- .../PlacementGroupsSummary/PlacementGroupsSummary.tsx | 1 + 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsSummary/PlacementGroupsSummary.test.tsx b/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsSummary/PlacementGroupsSummary.test.tsx index 25f085090a0..64ffe1c26bf 100644 --- a/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsSummary/PlacementGroupsSummary.test.tsx +++ b/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsSummary/PlacementGroupsSummary.test.tsx @@ -7,7 +7,7 @@ import { PlacementGroupsSummary } from './PlacementGroupsSummary'; describe('PlacementGroups Summary', () => { it('renders the placement group detail summary panel', () => { - const { getByText } = renderWithTheme( + const { getByTestId, getByText } = renderWithTheme( { expect(getByText('Placement Group Configuration')).toBeInTheDocument(); expect(getByText('Linodes')).toBeInTheDocument(); + expect(getByTestId('HelpOutlineIcon')).toBeInTheDocument(); expect(getByText('Affinity Type')).toBeInTheDocument(); expect(getByText('Region')).toBeInTheDocument(); }); diff --git a/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsSummary/PlacementGroupsSummary.tsx b/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsSummary/PlacementGroupsSummary.tsx index c3918867a05..97b71109b54 100644 --- a/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsSummary/PlacementGroupsSummary.tsx +++ b/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsSummary/PlacementGroupsSummary.tsx @@ -68,6 +68,7 @@ export const PlacementGroupsSummary = (props: Props) => { marginLeft: '10px', padding: 0, }} + // data-testid="help-icon" status="help" text={PLACEMENT_GROUP_TOOLTIP_TEXT} tooltipPosition="right" From 7f3707d4e7b70ebc8c33ca5fcdae177cff769f3d Mon Sep 17 00:00:00 2001 From: ecarrill Date: Mon, 12 Feb 2024 07:21:51 -0800 Subject: [PATCH 11/11] Remove comment --- .../PlacementGroupsSummary/PlacementGroupsSummary.tsx | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsSummary/PlacementGroupsSummary.tsx b/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsSummary/PlacementGroupsSummary.tsx index 97b71109b54..c3918867a05 100644 --- a/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsSummary/PlacementGroupsSummary.tsx +++ b/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsSummary/PlacementGroupsSummary.tsx @@ -68,7 +68,6 @@ export const PlacementGroupsSummary = (props: Props) => { marginLeft: '10px', padding: 0, }} - // data-testid="help-icon" status="help" text={PLACEMENT_GROUP_TOOLTIP_TEXT} tooltipPosition="right"