Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

upcoming: [M3-7613] - Grants and Permissions for Placement Groups #10257

Merged
merged 9 commits into from
Mar 22, 2024
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@linode/manager": Upcoming Features
---

Set up grants and permissions for Placement Groups ([#10257](https://github.com/linode/manager/pull/10257))
6 changes: 6 additions & 0 deletions packages/manager/src/components/Breadcrumb/Breadcrumb.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,10 @@ export interface BreadcrumbProps {
* An array of objects that can be used to customize any crumb.
*/
crumbOverrides?: CrumbOverridesProps[];
/**
* A boolean that if true will disable the pencil icon button.
*/
disableEditButton?: boolean;
/**
* A boolean that if true will only show the first and last crumb.
*/
Expand Down Expand Up @@ -48,6 +52,7 @@ export const Breadcrumb = (props: BreadcrumbProps) => {
const {
breadcrumbDataAttrs,
crumbOverrides,
disableEditButton,
firstAndLastOnly,
labelOptions,
labelTitle,
Expand Down Expand Up @@ -75,6 +80,7 @@ export const Breadcrumb = (props: BreadcrumbProps) => {
>
<Crumbs
crumbOverrides={crumbOverrides}
disableEditButton={disableEditButton}
firstAndLastOnly={firstAndLastOnly}
labelOptions={labelOptions}
labelTitle={labelTitle}
Expand Down
3 changes: 3 additions & 0 deletions packages/manager/src/components/Breadcrumb/Crumbs.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ export interface CrumbOverridesProps {

interface Props {
crumbOverrides?: CrumbOverridesProps[];
disableEditButton?: boolean;
firstAndLastOnly?: boolean;
labelOptions?: LabelProps;
labelTitle?: string;
Expand All @@ -30,6 +31,7 @@ interface Props {
export const Crumbs = React.memo((props: Props) => {
const {
crumbOverrides,
disableEditButton,
firstAndLastOnly,
labelOptions,
labelTitle,
Expand Down Expand Up @@ -92,6 +94,7 @@ export const Crumbs = React.memo((props: Props) => {
{/* the final crumb has the possibility of being a link, editable text or just static text */}
<FinalCrumb
crumb={labelTitle || lastCrumb}
disableEditButton={disableEditButton}
labelOptions={labelOptions}
onEditHandlers={onEditHandlers}
/>
Expand Down
4 changes: 3 additions & 1 deletion packages/manager/src/components/Breadcrumb/FinalCrumb.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,17 +9,19 @@ import { EditableProps, LabelProps } from './types';

interface Props {
crumb: string;
disableEditButton?: boolean;
labelOptions?: LabelProps;
onEditHandlers?: EditableProps;
}

export const FinalCrumb = React.memo((props: Props) => {
const { crumb, labelOptions, onEditHandlers } = props;
const { crumb, disableEditButton, labelOptions, onEditHandlers } = props;

if (onEditHandlers) {
return (
<StyledEditableText
data-qa-editable-text
disableEditButton={disableEditButton}
errorText={onEditHandlers.errorText}
handleAnalyticsEvent={onEditHandlers.handleAnalyticsEvent}
labelLink={labelOptions && labelOptions.linkTo}
Expand Down
2 changes: 1 addition & 1 deletion packages/manager/src/components/Drawer.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import Close from '@mui/icons-material/Close';
import _Drawer, { DrawerProps } from '@mui/material/Drawer';
import Grid from '@mui/material/Unstable_Grid2';
import { Theme } from '@mui/material/styles';
import Grid from '@mui/material/Unstable_Grid2';
import * as React from 'react';
import { makeStyles } from 'tss-react/mui';

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,7 @@ const useStyles = makeStyles<void, 'editIcon' | 'icon'>()(

interface Props {
className?: string;
disableEditButton?: boolean;
errorText?: string;
/**
* Send event analytics
Expand Down Expand Up @@ -139,6 +140,7 @@ export const EditableText = (props: PassThroughProps) => {
const [text, setText] = React.useState(props.text);
const {
className,
disableEditButton,
errorText,
handleAnalyticsEvent,
labelLink,
Expand Down Expand Up @@ -228,6 +230,7 @@ export const EditableText = (props: PassThroughProps) => {
aria-label={`Edit ${text}`}
className={`${classes.button} ${classes.editIcon}`}
data-qa-edit-button
disabled={disableEditButton}
onClick={openEdit}
>
<Edit className={classes.icon} />
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import Grid from '@mui/material/Unstable_Grid2';
import { Theme, styled, useTheme } from '@mui/material/styles';
import Grid from '@mui/material/Unstable_Grid2';
import useMediaQuery from '@mui/material/useMediaQuery';
import * as React from 'react';

Expand All @@ -19,6 +19,7 @@ export interface LandingHeaderProps {
buttonDataAttrs?: { [key: string]: boolean | string };
createButtonText?: string;
disabledCreateButton?: boolean;
disableEditButton?: boolean;
docsLabel?: string;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this should read disabledBreadcrumbEditButton for clarity since Landing header has other CTAs

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good call. The recent code push reflects this change.

docsLink?: string;
entity?: string;
Expand All @@ -44,6 +45,7 @@ export const LandingHeader = ({
buttonDataAttrs,
createButtonText,
disabledCreateButton,
disableEditButton,
docsLabel,
docsLink,
entity,
Expand Down Expand Up @@ -89,6 +91,7 @@ export const LandingHeader = ({
removeCrumbX={removeCrumbX}
{...breadcrumbDataAttrs}
{...breadcrumbProps}
disableEditButton={disableEditButton}
/>
</Grid>
{!shouldHideDocsAndCreateButtons && (
Expand Down
1 change: 1 addition & 0 deletions packages/manager/src/features/Account/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ export const grantTypeMap = {
linode: 'Linodes',
longview: 'Longview Clients',
nodebalancer: 'NodeBalancers',
placementGroups: 'Placement Groups',
stackscript: 'StackScripts',
volume: 'Volumes',
vpc: 'VPCs',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,14 @@ import { Typography } from 'src/components/Typography';
import type { FormikHelpers } from 'formik';

interface Props {
disabledCreateButton: boolean;
handleChange: (e: React.ChangeEvent<HTMLInputElement>) => void;
setFieldValue: FormikHelpers<any>['setFieldValue'];
value: boolean;
}

export const PlacementGroupsAffinityEnforcementRadioGroup = (props: Props) => {
const { handleChange, setFieldValue, value } = props;
const { disabledCreateButton, handleChange, setFieldValue, value } = props;
return (
<Box sx={{ pt: 2 }}>
<Notice
Expand Down Expand Up @@ -45,6 +46,7 @@ export const PlacementGroupsAffinityEnforcementRadioGroup = (props: Props) => {
</Typography>
}
control={<Radio />}
disabled={disabledCreateButton}
value={true}
/>
<FormControlLabel
Expand All @@ -56,6 +58,7 @@ export const PlacementGroupsAffinityEnforcementRadioGroup = (props: Props) => {
</Typography>
}
control={<Radio />}
disabled={disabledCreateButton}
sx={{ mt: 2 }}
value={false}
/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,13 @@ import { affinityTypeOptions } from './utils';
import type { FormikHelpers } from 'formik';

interface Props {
disabledCreateButton: boolean;
error: string | undefined;
setFieldValue: FormikHelpers<any>['setFieldValue'];
}

export const PlacementGroupsAffinityTypeSelect = (props: Props) => {
const { error, setFieldValue } = props;
const { disabledCreateButton, error, setFieldValue } = props;
return (
<Autocomplete
defaultValue={affinityTypeOptions.find(
Expand Down Expand Up @@ -82,6 +83,7 @@ export const PlacementGroupsAffinityTypeSelect = (props: Props) => {
),
}}
disableClearable={true}
disabled={disabledCreateButton}
errorText={error}
label="Affinity Type"
options={affinityTypeOptions}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import { PlacementGroupsCreateDrawer } from './PlacementGroupsCreateDrawer';

const commonProps = {
allPlacementGroups: [],
disabledCreateButton: false,
onClose: vi.fn(),
open: true,
};
Expand Down
mjac0bs marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import { RegionSelect } from 'src/components/RegionSelect/RegionSelect';
import { Stack } from 'src/components/Stack';
import { TextField } from 'src/components/TextField';
import { Typography } from 'src/components/Typography';
import { getRestrictedResourceText } from 'src/features/Account/utils';
import { useFormValidateOnChange } from 'src/hooks/useFormValidateOnChange';
import { useCreatePlacementGroup } from 'src/queries/placementGroups';
import { useRegionsQuery } from 'src/queries/regions';
Expand All @@ -30,6 +31,7 @@ export const PlacementGroupsCreateDrawer = (
) => {
const {
allPlacementGroups,
disabledCreateButton,
onClose,
onPlacementGroupCreate,
open,
Expand Down Expand Up @@ -131,6 +133,17 @@ export const PlacementGroupsCreateDrawer = (
open={open}
title="Create Placement Group"
>
{disabledCreateButton && (
<Notice
text={getRestrictedResourceText({
action: 'edit',
resourceType: 'Placement Groups',
})}
important
spacingTop={16}
variant="error"
/>
)}
<form onSubmit={handleSubmit}>
<Stack spacing={1}>
{generalError && <Notice text={generalError} variant="error" />}
Expand All @@ -146,7 +159,7 @@ export const PlacementGroupsCreateDrawer = (
autoFocus: true,
}}
aria-label="Label for the Placement Group"
disabled={false}
disabled={disabledCreateButton || false}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same here - the naming convention could lead to confusion.disabledCreateButton implies only the button and disabling the form using this variable isn't optimal. The button can also be disabled for other reasons limits) so we could run into logic problems down the line.

How about renaming disabledCreateButton with disablePlacementGroupCreation (or similar) where it applies?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as the other comment.

errorText={errors.label}
label="Label"
name="label"
Expand All @@ -165,25 +178,30 @@ export const PlacementGroupsCreateDrawer = (
handleRegionSelect(selection);
}}
currentCapability="Placement Group"
disabled={Boolean(selectedRegionId)}
disabled={Boolean(selectedRegionId) || disabledCreateButton}
helperText="Only regions supporting Placement Groups are listed."
regions={regions ?? []}
selectedId={selectedRegionId ?? values.region}
/>
)}
<PlacementGroupsAffinityTypeSelect
disabledCreateButton={disabledCreateButton}
error={errors.affinity_type}
setFieldValue={setFieldValue}
/>
<PlacementGroupsAffinityEnforcementRadioGroup
disabledCreateButton={disabledCreateButton}
handleChange={handleChange}
setFieldValue={setFieldValue}
value={values.is_strict}
/>
<ActionsPanel
primaryButtonProps={{
'data-testid': 'submit',
disabled: isSubmitting || hasRegionReachedPGCapacity,
disabled:
isSubmitting ||
hasRegionReachedPGCapacity ||
disabledCreateButton,
label: 'Create Placement Group',
loading: isSubmitting,
onClick: () => setHasFormBeenSubmitted(true),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,14 @@ 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 { getRestrictedResourceText } from 'src/features/Account/utils';
import { useFlags } from 'src/hooks/useFlags';
import { useRestrictedGlobalGrantCheck } from 'src/hooks/useRestrictedGlobalGrantCheck';
import {
useMutatePlacementGroup,
usePlacementGroupQuery,
Expand All @@ -37,6 +40,10 @@ export const PlacementGroupsDetail = () => {
Boolean(flags.placementGroups?.enabled)
);

const isLinodeReadOnly = useRestrictedGlobalGrantCheck({
globalGrantType: 'add_linodes',
});

const {
error: updatePlacementGroupError,
mutateAsync: updatePlacementGroup,
Expand Down Expand Up @@ -105,10 +112,22 @@ export const PlacementGroupsDetail = () => {
},
pathname: `/placement-groups/${label}`,
}}
disableEditButton={isLinodeReadOnly}
docsLabel="Docs"
docsLink="TODO VM_Placement: add doc link"
title="Placement Group Detail"
/>
{isLinodeReadOnly && (
<Notice
text={getRestrictedResourceText({
action: 'edit',
resourceType: 'Placement Groups',
})}
important
spacingTop={16}
variant="warning"
/>
)}
<Tabs
index={tabIndex === -1 ? 0 : tabIndex}
onChange={(i: number) => history.push(tabs[i].routeName)}
Expand All @@ -119,7 +138,10 @@ export const PlacementGroupsDetail = () => {
<PlacementGroupsSummary placementGroup={placementGroup} />
</SafeTabPanel>
<SafeTabPanel index={1}>
<PlacementGroupsLinodes placementGroup={placementGroup} />
<PlacementGroupsLinodes
isLinodeReadOnly={isLinodeReadOnly}
placementGroup={placementGroup}
/>
</SafeTabPanel>
</TabPanels>
</Tabs>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,10 @@ import { PlacementGroupsLinodes } from './PlacementGroupsLinodes';
describe('PlacementGroupsLinodes', () => {
it('renders an error state if placement groups are undefined', () => {
const { getByText } = renderWithTheme(
<PlacementGroupsLinodes placementGroup={undefined} />
<PlacementGroupsLinodes
isLinodeReadOnly={false}
placementGroup={undefined}
/>
);

expect(
Expand All @@ -28,7 +31,10 @@ describe('PlacementGroupsLinodes', () => {
});

const { getByPlaceholderText, getByRole } = renderWithTheme(
<PlacementGroupsLinodes placementGroup={placementGroup} />
<PlacementGroupsLinodes
isLinodeReadOnly={false}
placementGroup={placementGroup}
/>
);

expect(getByPlaceholderText('Search Linodes')).toBeInTheDocument();
Expand Down
Loading
Loading