upcoming: [M3-7986] - Fix & Improve Placement Groups feature restrict…

…ion (linode#10372) * Initial commit: save work * Add coverage * fix test * Added changeset: Fix & Improve Placement Groups feature restriction * Fix e2e
cpathipa · Apr 15, 2024 · 68f0add · 68f0add
1 parent 391b846
commit 68f0add
Show file tree

Hide file tree

Showing 15 changed files with 173 additions and 48 deletions.
diff --git a/packages/manager/.changeset/pr-10372-upcoming-features-1712891100686.md 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
@@ -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
@@ -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
@@ -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 = () => {
                       <React.Suspense fallback={<SuspenseLoader />}>
                         <Switch>
                           <Route component={LinodesRoutes} path="/linodes" />
-                          <Route
-                            component={PlacementGroups}
-                            path="/placement-groups"
-                          />
+                          {isPlacementGroupsEnabled && (
+                            <Route
+                              component={PlacementGroups}
+                              path="/placement-groups"
+                            />
+                          )}
                           <Route component={Volumes} path="/volumes" />
                           <Redirect path="/volumes*" to="/volumes" />
                           {isACLBEnabled && (

diff --git 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 (
     <Paper
@@ -60,8 +58,7 @@ export const DetailsPanel = (props: DetailsPanelProps) => {
       />
 
       {tagsInputProps && <TagsInput {...tagsInputProps} />}
-
-      {showPlacementGroups && (
+      {isPlacementGroupsEnabled && (
         <PlacementGroupsDetailPanel
           handlePlacementGroupChange={handlePlacementGroupChange}
           selectedRegionId={selectedRegionId}

diff --git a/packages/manager/src/components/PrimaryNav/PrimaryNav.tsx b/packages/manager/src/components/PrimaryNav/PrimaryNav.tsx
@@ -26,6 +26,7 @@ import { BetaChip } from 'src/components/BetaChip/BetaChip';
 import { Box } from 'src/components/Box';
 import { Divider } from 'src/components/Divider';
 import { useIsACLBEnabled } from 'src/features/LoadBalancers/utils';
+import { useIsPlacementGroupsEnabled } from 'src/features/PlacementGroups/utils';
 import { useAccountManagement } from 'src/hooks/useAccountManagement';
 import { useFlags } from 'src/hooks/useFlags';
 import { usePrefetch } from 'src/hooks/usePreFetch';
@@ -170,6 +171,7 @@ export const PrimaryNav = (props: PrimaryNavProps) => {
     (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: <PlacementGroups />,
           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
@@ -46,6 +46,7 @@ export const accountFactory = Factory.Sync.makeFactory<Account>({
     '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
@@ -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,17 +36,19 @@ 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: <Details />,
       options: {
         flags: { placementGroups: { beta: true, enabled: true } },
       },
     });
 
-    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
@@ -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<CreateLinodeRequest>();
-  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 && <PlacementGroupPanel />}
+      {isPlacementGroupsEnabled && <PlacementGroupPanel />}
     </Paper>
   );
 };
diff --git 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/...ages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsDetail.tsx b/...ages/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,18 +22,14 @@ import { PlacementGroupsLinodes } from './PlacementGroupsLinodes/PlacementGroups
 import { PlacementGroupsSummary } from './PlacementGroupsSummary/PlacementGroupsSummary';
 
 export const PlacementGroupsDetail = () => {
-  const flags = useFlags();
   const { id } = useParams<{ id: string }>();
   const placementGroupId = +id;
 
   const {
     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
@@ -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) => {
           </Button>
         )}
       </Box>
-      {flags.placementGroups?.enabled && (
-        <PlacementGroupsCreateDrawer
-          allPlacementGroups={allPlacementGroups || []}
-          disabledPlacementGroupCreateButton={isLinodeReadOnly}
-          onClose={() => setIsCreatePlacementGroupDrawerOpen(false)}
-          onPlacementGroupCreate={handlePlacementGroupCreated}
-          open={isCreatePlacementGroupDrawerOpen}
-          selectedRegionId={selectedRegionId}
-        />
-      )}
+      <PlacementGroupsCreateDrawer
+        allPlacementGroups={allPlacementGroups || []}
+        disabledPlacementGroupCreateButton={isLinodeReadOnly}
+        onClose={() => setIsCreatePlacementGroupDrawerOpen(false)}
+        onPlacementGroupCreate={handlePlacementGroupCreated}
+        open={isCreatePlacementGroupDrawerOpen}
+        selectedRegionId={selectedRegionId}
+      />
     </>
   );
 };