From 68f0adda3bbbd951e0f4b1c01bf87ce803ef7394 Mon Sep 17 00:00:00 2001 From: Alban Bailly <130582365+abailly-akamai@users.noreply.github.com> Date: Mon, 15 Apr 2024 15:12:44 -0400 Subject: [PATCH] upcoming: [M3-7986] - Fix & Improve Placement Groups feature restriction (#10372) * Initial commit: save work * Add coverage * fix test * Added changeset: Fix & Improve Placement Groups feature restriction * Fix e2e --- ...r-10372-upcoming-features-1712891100686.md | 5 ++ .../placement-groups-landing-page.spec.ts | 5 ++ packages/manager/src/GoTo.tsx | 14 +++- packages/manager/src/MainContent.tsx | 16 ++-- .../components/DetailsPanel/DetailsPanel.tsx | 9 +-- .../src/components/PrimaryNav/PrimaryNav.tsx | 5 +- packages/manager/src/factories/account.ts | 1 + .../LinodeCreatev2/Details/Details.test.tsx | 13 +-- .../LinodeCreatev2/Details/Details.tsx | 8 +- .../Linodes/LinodesCreate/LinodeCreate.tsx | 5 +- .../PlacementGroupsDetail.tsx | 7 +- .../PlacementGroupsDetailPanel.tsx | 20 ++--- .../features/PlacementGroups/utils.test.ts | 79 +++++++++++++++++++ .../src/features/PlacementGroups/utils.ts | 30 +++++++ .../TopMenu/AddNewMenu/AddNewMenu.tsx | 4 +- 15 files changed, 173 insertions(+), 48 deletions(-) create mode 100644 packages/manager/.changeset/pr-10372-upcoming-features-1712891100686.md diff --git a/packages/manager/.changeset/pr-10372-upcoming-features-1712891100686.md b/packages/manager/.changeset/pr-10372-upcoming-features-1712891100686.md new file mode 100644 index 00000000000..d7ec92b0c1d --- /dev/null +++ b/packages/manager/.changeset/pr-10372-upcoming-features-1712891100686.md @@ -0,0 +1,5 @@ +--- +"@linode/manager": Upcoming Features +--- + +Fix & Improve Placement Groups feature restriction ([#10372](https://github.com/linode/manager/pull/10372)) diff --git a/packages/manager/cypress/e2e/core/placementGroups/placement-groups-landing-page.spec.ts b/packages/manager/cypress/e2e/core/placementGroups/placement-groups-landing-page.spec.ts index 981c0aa2c5b..7709b82b298 100644 --- a/packages/manager/cypress/e2e/core/placementGroups/placement-groups-landing-page.spec.ts +++ b/packages/manager/cypress/e2e/core/placementGroups/placement-groups-landing-page.spec.ts @@ -5,8 +5,12 @@ import { import { makeFeatureFlagData } from 'support/util/feature-flags'; import { mockGetPlacementGroups } from 'support/intercepts/vm-placement'; import { ui } from 'support/ui'; +import { accountFactory } from 'src/factories'; import type { Flags } from 'src/featureFlags'; +import { mockGetAccount } from 'support/intercepts/account'; + +const mockAccount = accountFactory.build(); describe('VM Placement landing page', () => { // Mock the VM Placement Groups feature flag to be enabled for each test in this block. @@ -18,6 +22,7 @@ describe('VM Placement landing page', () => { }), }); mockGetFeatureFlagClientstream(); + mockGetAccount(mockAccount).as('getAccount'); }); /** diff --git a/packages/manager/src/GoTo.tsx b/packages/manager/src/GoTo.tsx index a682323fcb8..fd55ff3a646 100644 --- a/packages/manager/src/GoTo.tsx +++ b/packages/manager/src/GoTo.tsx @@ -7,6 +7,7 @@ import { makeStyles } from 'tss-react/mui'; import EnhancedSelect, { Item } from 'src/components/EnhancedSelect/Select'; import { useIsACLBEnabled } from './features/LoadBalancers/utils'; +import { useIsPlacementGroupsEnabled } from './features/PlacementGroups/utils'; import { useAccountManagement } from './hooks/useAccountManagement'; import { useGlobalKeyboardListener } from './hooks/useGlobalKeyboardListener'; @@ -60,6 +61,7 @@ export const GoTo = React.memo(() => { const { _hasAccountAccess, _isManagedAccount } = useAccountManagement(); const { isACLBEnabled } = useIsACLBEnabled(); + const { isPlacementGroupsEnabled } = useIsPlacementGroupsEnabled(); const { goToOpen, setGoToOpen } = useGlobalKeyboardListener(); const onClose = () => { @@ -113,6 +115,11 @@ export const GoTo = React.memo(() => { display: 'Images', href: '/images', }, + { + display: 'Placement Groups', + hide: !isPlacementGroupsEnabled, + href: '/placement-groups', + }, { display: 'Domains', href: '/domains', @@ -149,7 +156,12 @@ export const GoTo = React.memo(() => { href: '/profile/display', }, ], - [_hasAccountAccess, _isManagedAccount, isACLBEnabled] + [ + _hasAccountAccess, + _isManagedAccount, + isACLBEnabled, + isPlacementGroupsEnabled, + ] ); const options: Item[] = React.useMemo( diff --git a/packages/manager/src/MainContent.tsx b/packages/manager/src/MainContent.tsx index 46bd8cf1a07..c42231b0688 100644 --- a/packages/manager/src/MainContent.tsx +++ b/packages/manager/src/MainContent.tsx @@ -1,5 +1,5 @@ -import Grid from '@mui/material/Unstable_Grid2'; import { Theme } from '@mui/material/styles'; +import Grid from '@mui/material/Unstable_Grid2'; import { isEmpty } from 'ramda'; import * as React from 'react'; import { Redirect, Route, Switch } from 'react-router-dom'; @@ -10,8 +10,8 @@ import { Box } from 'src/components/Box'; import { MainContentBanner } from 'src/components/MainContentBanner'; import { MaintenanceScreen } from 'src/components/MaintenanceScreen'; import { NotFound } from 'src/components/NotFound'; -import { SIDEBAR_WIDTH } from 'src/components/PrimaryNav/SideMenu'; import { SideMenu } from 'src/components/PrimaryNav/SideMenu'; +import { SIDEBAR_WIDTH } from 'src/components/PrimaryNav/SideMenu'; import { SuspenseLoader } from 'src/components/SuspenseLoader'; import { useDialogContext } from 'src/context/useDialogContext'; import { Footer } from 'src/features/Footer'; @@ -33,6 +33,7 @@ import { complianceUpdateContext } from './context/complianceUpdateContext'; import { switchAccountSessionContext } from './context/switchAccountSessionContext'; import { FlagSet } from './featureFlags'; import { useIsACLBEnabled } from './features/LoadBalancers/utils'; +import { useIsPlacementGroupsEnabled } from './features/PlacementGroups/utils'; import { useGlobalErrors } from './hooks/useGlobalErrors'; const useStyles = makeStyles()((theme: Theme) => ({ @@ -226,6 +227,7 @@ export const MainContent = () => { (checkRestrictedUser && !enginesLoading && !enginesError); const { isACLBEnabled } = useIsACLBEnabled(); + const { isPlacementGroupsEnabled } = useIsPlacementGroupsEnabled(); const defaultRoot = _isManagedAccount ? '/managed' : '/linodes'; @@ -337,10 +339,12 @@ export const MainContent = () => { }> - + {isPlacementGroupsEnabled && ( + + )} {isACLBEnabled && ( diff --git a/packages/manager/src/components/DetailsPanel/DetailsPanel.tsx b/packages/manager/src/components/DetailsPanel/DetailsPanel.tsx index 0f93f1b77ee..e1e09ef6645 100644 --- a/packages/manager/src/components/DetailsPanel/DetailsPanel.tsx +++ b/packages/manager/src/components/DetailsPanel/DetailsPanel.tsx @@ -7,7 +7,7 @@ import { TagsInput, TagsInputProps } from 'src/components/TagsInput/TagsInput'; import { TextField, TextFieldProps } from 'src/components/TextField'; import { Typography } from 'src/components/Typography'; import { PlacementGroupsDetailPanel } from 'src/features/PlacementGroups/PlacementGroupsDetailPanel'; -import { useFlags } from 'src/hooks/useFlags'; +import { useIsPlacementGroupsEnabled } from 'src/features/PlacementGroups/utils'; import type { PlacementGroup } from '@linode/api-v4'; @@ -28,9 +28,7 @@ export const DetailsPanel = (props: DetailsPanelProps) => { tagsInputProps, } = props; const theme = useTheme(); - const flags = useFlags(); - - const showPlacementGroups = Boolean(flags.placementGroups?.enabled); + const { isPlacementGroupsEnabled } = useIsPlacementGroupsEnabled(); return ( { /> {tagsInputProps && } - - {showPlacementGroups && ( + {isPlacementGroupsEnabled && ( { (checkRestrictedUser && !enginesLoading && !enginesError); const { isACLBEnabled } = useIsACLBEnabled(); + const { isPlacementGroupsEnabled } = useIsPlacementGroupsEnabled(); const prefetchObjectStorage = () => { if (!enableObjectPrefetch) { @@ -245,7 +247,7 @@ export const PrimaryNav = (props: PrimaryNavProps) => { { betaChipClassName: 'beta-chip-placement-groups', display: 'Placement Groups', - hide: !flags.placementGroups?.enabled, + hide: !isPlacementGroupsEnabled, href: '/placement-groups', icon: , isBeta: flags.placementGroups?.beta, @@ -322,6 +324,7 @@ export const PrimaryNav = (props: PrimaryNavProps) => { allowMarketplacePrefetch, flags.databaseBeta, isACLBEnabled, + isPlacementGroupsEnabled, flags.placementGroups, ] ); diff --git a/packages/manager/src/factories/account.ts b/packages/manager/src/factories/account.ts index 17662c422de..79f8866b75d 100644 --- a/packages/manager/src/factories/account.ts +++ b/packages/manager/src/factories/account.ts @@ -46,6 +46,7 @@ export const accountFactory = Factory.Sync.makeFactory({ 'LKE HA Control Planes', 'Machine Images', 'Managed Databases', + 'Placement Group', ], city: 'Colorado', company: Factory.each((i) => `company-${i}`), diff --git a/packages/manager/src/features/Linodes/LinodeCreatev2/Details/Details.test.tsx b/packages/manager/src/features/Linodes/LinodeCreatev2/Details/Details.test.tsx index b54884d9f20..153792f7441 100644 --- a/packages/manager/src/features/Linodes/LinodeCreatev2/Details/Details.test.tsx +++ b/packages/manager/src/features/Linodes/LinodeCreatev2/Details/Details.test.tsx @@ -1,8 +1,7 @@ import { waitFor } from '@testing-library/react'; import React from 'react'; -import { profileFactory } from 'src/factories'; -import { grantsFactory } from 'src/factories/grants'; +import { grantsFactory, profileFactory } from 'src/factories'; import { HttpResponse, http, server } from 'src/mocks/testServer'; import { renderWithThemeAndHookFormContext } from 'src/utilities/testHelpers'; @@ -37,7 +36,7 @@ describe('Linode Create Details', () => { expect(getByText('Type to choose or create a tag.')).toBeVisible(); }); - it('renders an placement group details if the flag is on', () => { + it('renders an placement group details if the flag is on', async () => { const { getByText } = renderWithThemeAndHookFormContext({ component:
, options: { @@ -45,9 +44,11 @@ describe('Linode Create Details', () => { }, }); - expect( - getByText('Select a region above to see available Placement Groups.') - ).toBeVisible(); + await waitFor(() => { + expect( + getByText('Select a region above to see available Placement Groups.') + ).toBeVisible(); + }); }); it('does not render the placement group select if the flag is off', () => { diff --git a/packages/manager/src/features/Linodes/LinodeCreatev2/Details/Details.tsx b/packages/manager/src/features/Linodes/LinodeCreatev2/Details/Details.tsx index ed1fc1b205a..914c3f78cd7 100644 --- a/packages/manager/src/features/Linodes/LinodeCreatev2/Details/Details.tsx +++ b/packages/manager/src/features/Linodes/LinodeCreatev2/Details/Details.tsx @@ -6,16 +6,14 @@ import { Paper } from 'src/components/Paper'; import { TagsInput } from 'src/components/TagsInput/TagsInput'; import { TextField } from 'src/components/TextField'; import { Typography } from 'src/components/Typography'; -import { useFlags } from 'src/hooks/useFlags'; +import { useIsPlacementGroupsEnabled } from 'src/features/PlacementGroups/utils'; import { useRestrictedGlobalGrantCheck } from 'src/hooks/useRestrictedGlobalGrantCheck'; import { PlacementGroupPanel } from './PlacementGroupPanel'; export const Details = () => { const { control } = useFormContext(); - const flags = useFlags(); - - const showPlacementGroups = Boolean(flags.placementGroups?.enabled); + const { isPlacementGroupsEnabled } = useIsPlacementGroupsEnabled(); const isCreateLinodeRestricted = useRestrictedGlobalGrantCheck({ globalGrantType: 'add_linodes', @@ -51,7 +49,7 @@ export const Details = () => { control={control} name="tags" /> - {showPlacementGroups && } + {isPlacementGroupsEnabled && } ); }; diff --git a/packages/manager/src/features/Linodes/LinodesCreate/LinodeCreate.tsx b/packages/manager/src/features/Linodes/LinodesCreate/LinodeCreate.tsx index 4d8efbd6054..1b3c7f7ac15 100644 --- a/packages/manager/src/features/Linodes/LinodesCreate/LinodeCreate.tsx +++ b/packages/manager/src/features/Linodes/LinodesCreate/LinodeCreate.tsx @@ -861,10 +861,7 @@ export class LinodeCreate extends React.PureComponent< image: this.props.selectedImageID, label: this.props.label, placement_group: - this.props.flags.placementGroups?.enabled && - placement_group_payload.id !== -1 - ? placement_group_payload - : undefined, + placement_group_payload.id !== -1 ? placement_group_payload : undefined, private_ip: this.props.privateIPEnabled, region: this.props.selectedRegionID ?? '', root_pass: this.props.password, diff --git a/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsDetail.tsx b/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsDetail.tsx index 5f4a3e78a0c..ab73505e212 100644 --- a/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsDetail.tsx +++ b/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsDetail.tsx @@ -9,7 +9,6 @@ import { LandingHeader } from 'src/components/LandingHeader'; import { NotFound } from 'src/components/NotFound'; import { Notice } from 'src/components/Notice/Notice'; import { getRestrictedResourceText } from 'src/features/Account/utils'; -import { useFlags } from 'src/hooks/useFlags'; import { useRestrictedGlobalGrantCheck } from 'src/hooks/useRestrictedGlobalGrantCheck'; import { useAllLinodesQuery } from 'src/queries/linodes/linodes'; import { @@ -23,7 +22,6 @@ import { PlacementGroupsLinodes } from './PlacementGroupsLinodes/PlacementGroups import { PlacementGroupsSummary } from './PlacementGroupsSummary/PlacementGroupsSummary'; export const PlacementGroupsDetail = () => { - const flags = useFlags(); const { id } = useParams<{ id: string }>(); const placementGroupId = +id; @@ -31,10 +29,7 @@ export const PlacementGroupsDetail = () => { data: placementGroup, error: placementGroupError, isLoading, - } = usePlacementGroupQuery( - placementGroupId, - Boolean(flags.placementGroups?.enabled) - ); + } = usePlacementGroupQuery(placementGroupId); const { data: linodes, isFetching: isFetchingLinodes } = useAllLinodesQuery( {}, { diff --git a/packages/manager/src/features/PlacementGroups/PlacementGroupsDetailPanel.tsx b/packages/manager/src/features/PlacementGroups/PlacementGroupsDetailPanel.tsx index aea98d6e272..ea4a1db2303 100644 --- a/packages/manager/src/features/PlacementGroups/PlacementGroupsDetailPanel.tsx +++ b/packages/manager/src/features/PlacementGroups/PlacementGroupsDetailPanel.tsx @@ -10,7 +10,6 @@ import { TextTooltip } from 'src/components/TextTooltip'; import { Typography } from 'src/components/Typography'; import { PlacementGroupsCreateDrawer } from 'src/features/PlacementGroups/PlacementGroupsCreateDrawer'; import { hasRegionReachedPlacementGroupCapacity } from 'src/features/PlacementGroups/utils'; -import { useFlags } from 'src/hooks/useFlags'; import { useRestrictedGlobalGrantCheck } from 'src/hooks/useRestrictedGlobalGrantCheck'; import { useAllPlacementGroupsQuery } from 'src/queries/placementGroups'; import { useRegionsQuery } from 'src/queries/regions/regions'; @@ -26,7 +25,6 @@ interface Props { } export const PlacementGroupsDetailPanel = (props: Props) => { - const flags = useFlags(); const theme = useTheme(); const { handlePlacementGroupChange, selectedRegionId } = props; const { data: allPlacementGroups } = useAllPlacementGroupsQuery(); @@ -151,16 +149,14 @@ export const PlacementGroupsDetailPanel = (props: Props) => { )} - {flags.placementGroups?.enabled && ( - setIsCreatePlacementGroupDrawerOpen(false)} - onPlacementGroupCreate={handlePlacementGroupCreated} - open={isCreatePlacementGroupDrawerOpen} - selectedRegionId={selectedRegionId} - /> - )} + setIsCreatePlacementGroupDrawerOpen(false)} + onPlacementGroupCreate={handlePlacementGroupCreated} + open={isCreatePlacementGroupDrawerOpen} + selectedRegionId={selectedRegionId} + /> ); }; diff --git a/packages/manager/src/features/PlacementGroups/utils.test.ts b/packages/manager/src/features/PlacementGroups/utils.test.ts index ac6c9df18f4..28d652fdd30 100644 --- a/packages/manager/src/features/PlacementGroups/utils.test.ts +++ b/packages/manager/src/features/PlacementGroups/utils.test.ts @@ -1,3 +1,5 @@ +import { renderHook } from '@testing-library/react'; + import { linodeFactory, placementGroupFactory, @@ -11,8 +13,30 @@ import { getPlacementGroupLinodes, hasPlacementGroupReachedCapacity, hasRegionReachedPlacementGroupCapacity, + useIsPlacementGroupsEnabled, } from './utils'; +const queryMocks = vi.hoisted(() => ({ + useAccount: vi.fn().mockReturnValue({}), + useFlags: vi.fn().mockReturnValue({}), +})); + +vi.mock('src/queries/account/account', () => { + const actual = vi.importActual('src/queries/account/account'); + return { + ...actual, + useAccount: queryMocks.useAccount, + }; +}); + +vi.mock('src/hooks/useFlags', () => { + const actual = vi.importActual('src/hooks/useFlags'); + return { + ...actual, + useFlags: queryMocks.useFlags, + }; +}); + const initialLinodeData = [ { is_compliant: true, @@ -201,3 +225,58 @@ describe('getPlacementGroupLinodes', () => { expect(getPlacementGroupLinodes(placementGroup, linodes)).toBeUndefined(); }); }); + +describe('useIsPlacementGroupsEnabled', () => { + it('returns true if the feature flag is enabled and the account has the Placement Group capability', () => { + queryMocks.useFlags.mockReturnValue({ + placementGroups: { + enabled: true, + }, + }); + queryMocks.useAccount.mockReturnValue({ + data: { + capabilities: ['Placement Group'], + }, + }); + + const { result } = renderHook(() => useIsPlacementGroupsEnabled()); + expect(result.current).toStrictEqual({ + isPlacementGroupsEnabled: true, + }); + }); + + it('returns false if the feature flag is disabled', () => { + queryMocks.useFlags.mockReturnValue({ + placementGroups: { + enabled: false, + }, + }); + queryMocks.useAccount.mockReturnValue({ + data: { + capabilities: ['Placement Group'], + }, + }); + + const { result } = renderHook(() => useIsPlacementGroupsEnabled()); + expect(result.current).toStrictEqual({ + isPlacementGroupsEnabled: false, + }); + }); + it('returns false if the account does not have the Placement Group capability', () => { + queryMocks.useFlags.mockReturnValue({ + placementGroups: { + enabled: true, + }, + }); + queryMocks.useAccount.mockReturnValue({ + data: { + capabilities: [], + }, + }); + + const { result } = renderHook(() => useIsPlacementGroupsEnabled()); + expect(result.current).toStrictEqual({ + isPlacementGroupsEnabled: false, + }); + }); +}); diff --git a/packages/manager/src/features/PlacementGroups/utils.ts b/packages/manager/src/features/PlacementGroups/utils.ts index d462862a1dd..f7de317351c 100644 --- a/packages/manager/src/features/PlacementGroups/utils.ts +++ b/packages/manager/src/features/PlacementGroups/utils.ts @@ -1,5 +1,8 @@ import { AFFINITY_TYPES } from '@linode/api-v4/lib/placement-groups'; +import { useFlags } from 'src/hooks/useFlags'; +import { useAccount } from 'src/queries/account/account'; + import type { AffinityEnforcement, CreatePlacementGroupPayload, @@ -118,3 +121,30 @@ export const getLinodesFromAllPlacementGroups = ( return Array.from(new Set(linodeIds)); }; + +/** + * Hook to determine if the Placement Group feature should be visible to the user. + * Dased on the user's account capability and the feature flag. + * + * @returns {boolean} - Whether the Placement Group feature is enabled for the current user. + */ +export const useIsPlacementGroupsEnabled = (): { + isPlacementGroupsEnabled: boolean; +} => { + const { data: account, error } = useAccount(); + const flags = useFlags(); + + if (error || !flags) { + return { isPlacementGroupsEnabled: false }; + } + + const hasAccountCapability = account?.capabilities?.includes( + 'Placement Group' + ); + const isFeatureFlagEnabled = flags.placementGroups?.enabled; + const isPlacementGroupsEnabled = Boolean( + hasAccountCapability && isFeatureFlagEnabled + ); + + return { isPlacementGroupsEnabled }; +}; diff --git a/packages/manager/src/features/TopMenu/AddNewMenu/AddNewMenu.tsx b/packages/manager/src/features/TopMenu/AddNewMenu/AddNewMenu.tsx index 35f703b238a..1b0b629d373 100644 --- a/packages/manager/src/features/TopMenu/AddNewMenu/AddNewMenu.tsx +++ b/packages/manager/src/features/TopMenu/AddNewMenu/AddNewMenu.tsx @@ -27,6 +27,7 @@ import VPCIcon from 'src/assets/icons/entityIcons/vpc.svg'; import { Button } from 'src/components/Button/Button'; import { Divider } from 'src/components/Divider'; import { useIsACLBEnabled } from 'src/features/LoadBalancers/utils'; +import { useIsPlacementGroupsEnabled } from 'src/features/PlacementGroups/utils'; import { useFlags } from 'src/hooks/useFlags'; import { useAccount } from 'src/queries/account/account'; import { useDatabaseEnginesQuery } from 'src/queries/databases'; @@ -63,6 +64,7 @@ export const AddNewMenu = () => { (checkRestrictedUser && !enginesLoading && !enginesError); const { isACLBEnabled } = useIsACLBEnabled(); + const { isPlacementGroupsEnabled } = useIsPlacementGroupsEnabled(); const handleClick = (event: React.MouseEvent) => { setAnchorEl(event.currentTarget); @@ -114,7 +116,7 @@ export const AddNewMenu = () => { { description: "Control your Linodes' physical placement", entity: 'Placement Groups', - hide: !flags.placementGroups?.enabled, + hide: !isPlacementGroupsEnabled, icon: PlacementGroupsIcon, link: '/placement-groups/create', },