From 16011c3ae8c8bef5408b13cc212e5fc9ab9f3468 Mon Sep 17 00:00:00 2001 From: ecarrill Date: Thu, 12 Sep 2024 12:57:21 -0700 Subject: [PATCH 1/4] fix: [M3-7571] - Missing label for Full Account Access Toggle --- .../e2e/core/account/user-permissions.spec.ts | 8 +- .../features/Users/UserPermissions.styles.ts | 14 +- .../src/features/Users/UserPermissions.tsx | 143 +++++++++--------- .../Users/UserPermissionsEntitySection.tsx | 2 +- 4 files changed, 83 insertions(+), 84 deletions(-) diff --git a/packages/manager/cypress/e2e/core/account/user-permissions.spec.ts b/packages/manager/cypress/e2e/core/account/user-permissions.spec.ts index 53be928aded..a89094ddcc2 100644 --- a/packages/manager/cypress/e2e/core/account/user-permissions.spec.ts +++ b/packages/manager/cypress/e2e/core/account/user-permissions.spec.ts @@ -216,9 +216,7 @@ describe('User permission management', () => { // Restrict account access, confirm page updates to reflect change. mockUpdateUser(mockUser.username, mockUserUpdated); mockGetUserGrants(mockUser.username, mockUserGrantsUpdated); - cy.findByLabelText('Toggle Full Account Access') - .should('be.visible') - .click(); + cy.findByText('Full Account Access').should('be.visible').click(); ui.toast.assertMessage('User permissions successfully saved.'); @@ -250,9 +248,7 @@ describe('User permission management', () => { // Re-enable unrestricted account access, confirm page updates to reflect change. mockUpdateUser(mockUser.username, mockUser); mockGetUserGrantsUnrestrictedAccess(mockUser.username); - cy.findByLabelText('Toggle Full Account Access') - .should('be.visible') - .click(); + cy.findByText('Full Account Access').should('be.visible').click(); cy.findByText('General Permissions').should('be.visible'); cy.findByText(unrestrictedAccessMessage).should('be.visible'); diff --git a/packages/manager/src/features/Users/UserPermissions.styles.ts b/packages/manager/src/features/Users/UserPermissions.styles.ts index a5bc3241b98..fc2a46a9b90 100644 --- a/packages/manager/src/features/Users/UserPermissions.styles.ts +++ b/packages/manager/src/features/Users/UserPermissions.styles.ts @@ -1,5 +1,5 @@ -import Grid from '@mui/material/Unstable_Grid2'; import { styled } from '@mui/material/styles'; +import Grid from '@mui/material/Unstable_Grid2'; import { CircleProgress } from 'src/components/CircleProgress'; import Select from 'src/components/EnhancedSelect/Select'; @@ -41,16 +41,17 @@ export const StyledHeaderGrid = styled(Grid, { [theme.breakpoints.down('sm')]: { marginLeft: theme.spacing(2), marginTop: theme.spacing(1), - width: '100%', }, + width: '100%', })); -export const StyledSubHeaderGrid = styled(Grid, { - label: 'StyledSubHeaderGrid', +export const StyledFullAccountAccessToggleGrid = styled(Grid, { + label: 'StyledFullAccountAccessToggleGrid', })(({ theme }) => ({ + marginTop: theme.spacing(2), + padding: 0, [theme.breakpoints.down('sm')]: { - margin: theme.spacing(0.5), - padding: 0, + paddingLeft: theme.spacing(2), }, })); @@ -68,7 +69,6 @@ export const StyledPaper = styled(Paper, { label: 'StyledPaper', })(({ theme }) => ({ paddingBottom: 0, - paddingTop: 0, [theme.breakpoints.down('sm')]: { padding: 0, }, diff --git a/packages/manager/src/features/Users/UserPermissions.tsx b/packages/manager/src/features/Users/UserPermissions.tsx index ab193e1d5ca..167d7bddb8f 100644 --- a/packages/manager/src/features/Users/UserPermissions.tsx +++ b/packages/manager/src/features/Users/UserPermissions.tsx @@ -1,19 +1,11 @@ import { - GlobalGrantTypes, - Grant, - GrantLevel, - GrantType, - Grants, - User, getGrants, getUser, updateGrants, updateUser, } from '@linode/api-v4/lib/account'; -import { APIError } from '@linode/api-v4/lib/types'; import { Paper } from '@mui/material'; import Grid from '@mui/material/Unstable_Grid2'; -import { QueryClient } from '@tanstack/react-query'; import { enqueueSnackbar } from 'notistack'; import { compose, flatten, lensPath, omit, set } from 'ramda'; import * as React from 'react'; @@ -21,9 +13,7 @@ import * as React from 'react'; import { ActionsPanel } from 'src/components/ActionsPanel/ActionsPanel'; import { Box } from 'src/components/Box'; import { CircleProgress } from 'src/components/CircleProgress'; -// import { Button } from 'src/components/Button/Button'; import { DocumentTitleSegment } from 'src/components/DocumentTitle'; -import { Item } from 'src/components/EnhancedSelect/Select'; import { FormControlLabel } from 'src/components/FormControlLabel'; import { Notice } from 'src/components/Notice/Notice'; import { SelectionCard } from 'src/components/SelectionCard/SelectionCard'; @@ -34,14 +24,8 @@ import { TabPanels } from 'src/components/Tabs/TabPanels'; import { Tabs } from 'src/components/Tabs/Tabs'; import { Toggle } from 'src/components/Toggle/Toggle'; import { Typography } from 'src/components/Typography'; -import { - WithFeatureFlagProps, - withFeatureFlags, -} from 'src/containers/flags.container'; -import { - WithQueryClientProps, - withQueryClient, -} from 'src/containers/withQueryClient.container'; +import { withFeatureFlags } from 'src/containers/flags.container'; +import { withQueryClient } from 'src/containers/withQueryClient.container'; import { PARENT_USER, grantTypeMap } from 'src/features/Account/constants'; import { accountQueries } from 'src/queries/account/queries'; import { getAPIErrorOrDefault } from 'src/utilities/errorUtils'; @@ -51,14 +35,28 @@ import { scrollErrorIntoViewV2 } from 'src/utilities/scrollErrorIntoViewV2'; import { StyledCircleProgress, StyledDivWrapper, + StyledFullAccountAccessToggleGrid, StyledHeaderGrid, StyledPaper, StyledPermPaper, StyledSelect, - StyledSubHeaderGrid, StyledUnrestrictedGrid, } from './UserPermissions.styles'; import { UserPermissionsEntitySection } from './UserPermissionsEntitySection'; + +import type { + GlobalGrantTypes, + Grant, + GrantLevel, + GrantType, + Grants, + User, +} from '@linode/api-v4/lib/account'; +import type { APIError } from '@linode/api-v4/lib/types'; +import type { QueryClient } from '@tanstack/react-query'; +import type { Item } from 'src/components/EnhancedSelect/Select'; +import type { WithFeatureFlagProps } from 'src/containers/flags.container'; +import type { WithQueryClientProps } from 'src/containers/withQueryClient.container'; interface Props { accountUsername?: string; currentUsername?: string; @@ -91,30 +89,6 @@ interface State { type CombinedProps = Props & WithQueryClientProps & WithFeatureFlagProps; class UserPermissions extends React.Component { - componentDidMount() { - this.getUserGrants(); - this.getUserType(); - } - - componentDidUpdate(prevProps: CombinedProps) { - if (prevProps.currentUsername !== this.props.currentUsername) { - this.getUserGrants(); - this.getUserType(); - } - } - - render() { - const { loading } = this.state; - const { currentUsername } = this.props; - - return ( -
- - {loading ? : this.renderBody()} -
- ); - } - billingPermOnClick = (value: null | string) => () => { const lp = lensPath(['grants', 'global', 'account_access']); this.setState(set(lp, value)); @@ -371,7 +345,7 @@ class UserPermissions extends React.Component { spacing={2} > - + Billing Access @@ -445,32 +419,41 @@ class UserPermissions extends React.Component { textTransform: 'capitalize', }} data-qa-restrict-access={restricted} - variant="h2" + variant="h1" > {isProxyUser ? PARENT_USER : 'General'} Permissions - - + } - aria-label="Toggle Full Account Access" - checked={!restricted} - disabled={currentUsername === accountUsername} - onChange={this.onChangeRestricted} + slotProps={{ + typography: { + sx: (theme) => ({ + fontFamily: theme.font.bold, + fontSize: '16px', + }), + }, + }} + label="Full Account Access" + labelPlacement="end" + value={restricted} /> - - - theme.font.bold }} - variant="subtitle2" - > - Full Account Access - - + {restricted ? this.renderPermissions() : this.renderUnrestricted()} @@ -531,7 +514,7 @@ class UserPermissions extends React.Component { Configure the specific rights and privileges this user has within the account.{
}Remember that permissions related to actions with the @@ -606,12 +589,11 @@ class UserPermissions extends React.Component { Specific Permissions - { This user has unrestricted access to the account. - {/* */} ); @@ -823,6 +802,30 @@ class UserPermissions extends React.Component { setAllPerm: 'null', userType: null, }; + + componentDidMount() { + this.getUserGrants(); + this.getUserType(); + } + + componentDidUpdate(prevProps: CombinedProps) { + if (prevProps.currentUsername !== this.props.currentUsername) { + this.getUserGrants(); + this.getUserType(); + } + } + + render() { + const { loading } = this.state; + const { currentUsername } = this.props; + + return ( +
+ + {loading ? : this.renderBody()} +
+ ); + } } export default withQueryClient(withFeatureFlags(UserPermissions)); diff --git a/packages/manager/src/features/Users/UserPermissionsEntitySection.tsx b/packages/manager/src/features/Users/UserPermissionsEntitySection.tsx index 78d66dc3c3e..0b85f4af866 100644 --- a/packages/manager/src/features/Users/UserPermissionsEntitySection.tsx +++ b/packages/manager/src/features/Users/UserPermissionsEntitySection.tsx @@ -57,7 +57,7 @@ export const UserPermissionsEntitySection = React.memo( {showHeading && ( Date: Thu, 12 Sep 2024 13:16:35 -0700 Subject: [PATCH 2/4] Add changeset --- packages/manager/.changeset/pr-10931-fixed-1726172174760.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 packages/manager/.changeset/pr-10931-fixed-1726172174760.md diff --git a/packages/manager/.changeset/pr-10931-fixed-1726172174760.md b/packages/manager/.changeset/pr-10931-fixed-1726172174760.md new file mode 100644 index 00000000000..8cdf45b748a --- /dev/null +++ b/packages/manager/.changeset/pr-10931-fixed-1726172174760.md @@ -0,0 +1,5 @@ +--- +"@linode/manager": Fixed +--- + +Missing label for Full Account Toggle ([#10931](https://github.com/linode/manager/pull/10931)) From 362e60c0df47aace2ca35d5ab91da0ae97cf41e8 Mon Sep 17 00:00:00 2001 From: ecarrill Date: Tue, 17 Sep 2024 11:26:29 -0700 Subject: [PATCH 3/4] Update e2e tests --- .../cypress/e2e/core/account/user-permissions.spec.ts | 8 ++++++-- packages/manager/src/features/Users/UserPermissions.tsx | 1 + 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/packages/manager/cypress/e2e/core/account/user-permissions.spec.ts b/packages/manager/cypress/e2e/core/account/user-permissions.spec.ts index a89094ddcc2..7d97c8251f5 100644 --- a/packages/manager/cypress/e2e/core/account/user-permissions.spec.ts +++ b/packages/manager/cypress/e2e/core/account/user-permissions.spec.ts @@ -216,7 +216,9 @@ describe('User permission management', () => { // Restrict account access, confirm page updates to reflect change. mockUpdateUser(mockUser.username, mockUserUpdated); mockGetUserGrants(mockUser.username, mockUserGrantsUpdated); - cy.findByText('Full Account Access').should('be.visible').click(); + cy.get('[data-qa="toggle-full-account-access"]') + .should('be.visible') + .click(); ui.toast.assertMessage('User permissions successfully saved.'); @@ -248,7 +250,9 @@ describe('User permission management', () => { // Re-enable unrestricted account access, confirm page updates to reflect change. mockUpdateUser(mockUser.username, mockUser); mockGetUserGrantsUnrestrictedAccess(mockUser.username); - cy.findByText('Full Account Access').should('be.visible').click(); + cy.get('[data-qa="toggle-full-account-access"]') + .should('be.visible') + .click(); cy.findByText('General Permissions').should('be.visible'); cy.findByText(unrestrictedAccessMessage).should('be.visible'); diff --git a/packages/manager/src/features/Users/UserPermissions.tsx b/packages/manager/src/features/Users/UserPermissions.tsx index 167d7bddb8f..2e8f4b0798f 100644 --- a/packages/manager/src/features/Users/UserPermissions.tsx +++ b/packages/manager/src/features/Users/UserPermissions.tsx @@ -449,6 +449,7 @@ class UserPermissions extends React.Component { }), }, }} + data-qa="toggle-full-account-access" label="Full Account Access" labelPlacement="end" value={restricted} From 1a84ef362dba02d3ce057bade639172f35e78727 Mon Sep 17 00:00:00 2001 From: ecarrill Date: Tue, 17 Sep 2024 12:01:32 -0700 Subject: [PATCH 4/4] Update headers based on PR feedback and lighthouse results --- packages/manager/src/features/Users/UserPermissions.tsx | 6 +++--- .../src/features/Users/UserPermissionsEntitySection.tsx | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/manager/src/features/Users/UserPermissions.tsx b/packages/manager/src/features/Users/UserPermissions.tsx index 2e8f4b0798f..ec85b41df06 100644 --- a/packages/manager/src/features/Users/UserPermissions.tsx +++ b/packages/manager/src/features/Users/UserPermissions.tsx @@ -345,7 +345,7 @@ class UserPermissions extends React.Component { spacing={2} > - + Billing Access @@ -419,7 +419,7 @@ class UserPermissions extends React.Component { textTransform: 'capitalize', }} data-qa-restrict-access={restricted} - variant="h1" + variant="h2" > {isProxyUser ? PARENT_USER : 'General'} Permissions @@ -590,7 +590,7 @@ class UserPermissions extends React.Component { Specific Permissions diff --git a/packages/manager/src/features/Users/UserPermissionsEntitySection.tsx b/packages/manager/src/features/Users/UserPermissionsEntitySection.tsx index 0b85f4af866..78d66dc3c3e 100644 --- a/packages/manager/src/features/Users/UserPermissionsEntitySection.tsx +++ b/packages/manager/src/features/Users/UserPermissionsEntitySection.tsx @@ -57,7 +57,7 @@ export const UserPermissionsEntitySection = React.memo( {showHeading && (