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

change: [M3-7740] - Use "default" for non-parent/proxy/child user_type instead of null #10176

Merged
merged 7 commits into from
Feb 13, 2024
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@linode/api-v4": Upcoming Features
---

Update /account and /profile UserType from `null` to `"default"` ([#10176](https://github.com/linode/manager/pull/10176))
4 changes: 2 additions & 2 deletions packages/api-v4/src/account/types.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import type { APIWarning, RequestOptions } from '../types';
import type { Capabilities, Region } from '../regions';

export type UserType = 'child' | 'parent' | 'proxy';
export type UserType = 'child' | 'parent' | 'proxy' | 'default';

export interface User {
email: string;
@@ -30,7 +30,7 @@ export interface User {
ssh_keys: string[];
tfa_enabled: boolean;
username: string;
user_type: UserType | null;
user_type: UserType;
verified_phone_number: string | null;
}

2 changes: 1 addition & 1 deletion packages/api-v4/src/profile/types.ts
Original file line number Diff line number Diff line change
@@ -24,7 +24,7 @@ export interface Profile {
two_factor_auth: boolean;
restricted: boolean;
verified_phone_number: string | null;
user_type: UserType | null;
user_type: UserType;
}

export interface TokenRequest {
5 changes: 5 additions & 0 deletions packages/manager/.changeset/pr-10176-tests-1707761742808.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@linode/manager": Tests
---

Update Cypress tests to use `"default"` `user_type` for non-parent/child/proxy users ([#10176](https://github.com/linode/manager/pull/10176))
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@linode/manager": Upcoming Features
---

Update components and unit tests to use `"default"` `user_type` for non-parent/child/proxy users ([#10176](https://github.com/linode/manager/pull/10176))
Original file line number Diff line number Diff line change
@@ -2,13 +2,10 @@
* @file Integration tests for Cloud Manager account login history flows.
*/

import { accountFactory, profileFactory } from 'src/factories';
import { profileFactory } from 'src/factories';
import { accountLoginFactory } from 'src/factories/accountLogin';
import { formatDate } from 'src/utilities/formatDate';
import {
mockGetAccount,
mockGetAccountLogins,
} from 'support/intercepts/account';
import { mockGetAccountLogins } from 'support/intercepts/account';
import {
mockAppendFeatureFlags,
mockGetFeatureFlagClientstream,
@@ -24,11 +21,10 @@ describe('Account login history', () => {
* - Confirm that the login table displays a mocked successful unrestricted user login.
*/
it('users can view the login history table', () => {
const mockAccount = accountFactory.build();
const mockProfile = profileFactory.build({
username: 'mock-user',
restricted: false,
user_type: null,
user_type: 'default',
});
const mockFailedLogin = accountLoginFactory.build({
status: 'failed',
@@ -40,7 +36,6 @@ describe('Account login history', () => {
restricted: false,
});

mockGetAccount(mockAccount).as('getAccount');
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Unrelated to the user_type change: we didn't actually need to mock GET account in these login history specs, so it's been removed.

mockGetProfile(mockProfile).as('getProfile');
mockGetAccountLogins([mockFailedLogin, mockSuccessfulLogin]).as(
'getAccountLogins'
@@ -54,12 +49,7 @@ describe('Account login history', () => {

// Navigate to Account Login History page.
cy.visitWithLogin('/account/login-history');
cy.wait([
'@getAccount',
'@getClientStream',
'@getFeatureFlags',
'@getProfile',
]);
cy.wait(['@getClientStream', '@getFeatureFlags', '@getProfile']);

// Confirm helper text above table is visible.
cy.findByText(
@@ -115,14 +105,12 @@ describe('Account login history', () => {
* - Confirms that a child user sees a notice instead.
*/
it('child users cannot view login history', () => {
const mockAccount = accountFactory.build();
const mockProfile = profileFactory.build({
username: 'mock-child-user',
restricted: false,
user_type: 'child',
});

mockGetAccount(mockAccount).as('getAccount');
mockGetProfile(mockProfile).as('getProfile');

// TODO: Parent/Child - M3-7559 clean up when feature is live in prod and feature flag is removed.
@@ -133,12 +121,7 @@ describe('Account login history', () => {

// Navigate to Account Login History page.
cy.visitWithLogin('/account/login-history');
cy.wait([
'@getAccount',
'@getClientStream',
'@getFeatureFlags',
'@getProfile',
]);
cy.wait(['@getClientStream', '@getFeatureFlags', '@getProfile']);

// Confirm helper text above table and table are not visible.
cy.findByText(
@@ -157,15 +140,13 @@ describe('Account login history', () => {
* - Confirms that a restricted user sees a notice instead.
*/
it('restricted users cannot view login history', () => {
const mockAccount = accountFactory.build();
const mockProfile = profileFactory.build({
username: 'mock-restricted-user',
restricted: true,
user_type: null,
user_type: 'default',
});

mockGetProfile(mockProfile).as('getProfile');
mockGetAccount(mockAccount).as('getAccount');

// TODO: Parent/Child - M3-7559 clean up when feature is live in prod and feature flag is removed.
mockAppendFeatureFlags({
@@ -175,12 +156,7 @@ describe('Account login history', () => {

// Navigate to Account Login History page.
cy.visitWithLogin('/account/login-history');
cy.wait([
'@getAccount',
'@getClientStream',
'@getFeatureFlags',
'@getProfile',
]);
cy.wait(['@getClientStream', '@getFeatureFlags', '@getProfile']);

// Confirm helper text above table and table are not visible.
cy.findByText(
Original file line number Diff line number Diff line change
@@ -179,7 +179,7 @@ describe('username', () => {
it('disables username/email fields for regular restricted user', () => {
const mockRegularRestrictedProfile = profileFactory.build({
username: 'regular-restricted-user',
user_type: null,
user_type: 'default',
restricted: true,
});

Original file line number Diff line number Diff line change
@@ -208,7 +208,7 @@ describe('restricted user billing flows', () => {

const mockUser = accountUserFactory.build({
username: mockProfile.username,
user_type: null,
user_type: 'default',
restricted: false,
});

@@ -248,7 +248,7 @@ describe('restricted user billing flows', () => {
const mockUser = accountUserFactory.build({
username: mockProfile.username,
restricted: true,
user_type: null,
user_type: 'default',
});

const mockGrants = grantsFactory.build({
@@ -304,7 +304,7 @@ describe('restricted user billing flows', () => {

const mockUserRegular = accountUserFactory.build({
username: mockProfileRegular.username,
user_type: null,
user_type: 'default',
restricted: false,
});

2 changes: 1 addition & 1 deletion packages/manager/src/factories/accountUsers.ts
Original file line number Diff line number Diff line change
@@ -8,7 +8,7 @@ export const accountUserFactory = Factory.Sync.makeFactory<User>({
restricted: true,
ssh_keys: [],
tfa_enabled: false,
user_type: null,
user_type: 'default',
username: Factory.each((i) => `user-${i}`),
verified_phone_number: null,
});
2 changes: 1 addition & 1 deletion packages/manager/src/factories/profile.ts
Original file line number Diff line number Diff line change
@@ -25,7 +25,7 @@ export const profileFactory = Factory.Sync.makeFactory<Profile>({
timezone: 'Asia/Shanghai',
two_factor_auth: false,
uid: 9999,
user_type: null,
user_type: 'default',
username: 'mock-user',
verified_phone_number: '+15555555555',
});
5 changes: 4 additions & 1 deletion packages/manager/src/features/Account/AccountLogins.tsx
Original file line number Diff line number Diff line change
@@ -166,7 +166,10 @@ const AccountLogins = () => {
</>
) : (
<Notice important variant="warning">
{getAccessRestrictedText(profile?.user_type ?? null)}
{getAccessRestrictedText(
profile?.user_type,
flags.parentChildAccountAccess
)}
</Notice>
);
};
Original file line number Diff line number Diff line change
@@ -20,7 +20,7 @@ const CloseAccountSetting = () => {

// Disable the Close Account button for users with a Parent/Proxy/Child user type.
const isCloseAccountDisabled = Boolean(
flags.parentChildAccountAccess && profile?.user_type !== null
flags.parentChildAccountAccess && profile?.user_type !== 'default'
);
const closeAccountButtonTooltipText =
isCloseAccountDisabled && profile?.user_type === 'child'
9 changes: 7 additions & 2 deletions packages/manager/src/features/Account/utils.ts
Original file line number Diff line number Diff line change
@@ -35,9 +35,14 @@ export const getRestrictedResourceText = ({
/**
* Get an 'access restricted' message based on user type.
*/
export const getAccessRestrictedText = (userType: UserType | null) => {
export const getAccessRestrictedText = (
userType: UserType | undefined,
isParentChildFeatureEnabled?: boolean
) => {
return `Access restricted. Please contact your ${
userType === 'child' ? 'business partner' : 'account administrator'
isParentChildFeatureEnabled && userType === 'child'
? 'business partner'
: 'account administrator'
} to request the necessary permission.`;
};

Original file line number Diff line number Diff line change
@@ -101,7 +101,7 @@ describe('Create API Token Drawer', () => {

it('Should not show the Child Account Access scope for a non-parent user account with the parent/child feature flag on', () => {
queryMocks.useProfile.mockReturnValue({
data: profileFactory.build({ user_type: null }),
data: profileFactory.build({ user_type: 'default' }),
});

const { queryByText } = renderWithTheme(
Original file line number Diff line number Diff line change
@@ -167,10 +167,7 @@ describe('View API Token Drawer', () => {
});

describe('Parent/Child: User Roles', () => {
const setupAndRender = (
userType: UserType | null,
enableFeatureFlag = true
) => {
const setupAndRender = (userType: UserType, enableFeatureFlag = true) => {
queryMocks.useProfile.mockReturnValue({
data: profileFactory.build({ user_type: userType }),
});
@@ -181,7 +178,7 @@ describe('View API Token Drawer', () => {
};

const testChildScopeNotDisplayed = (
userType: UserType | null,
userType: UserType,
enableFeatureFlag = true
) => {
const { queryByText } = setupAndRender(userType, enableFeatureFlag);
@@ -193,8 +190,8 @@ describe('View API Token Drawer', () => {
testChildScopeNotDisplayed('parent', false);
});

it('should not display the Child Account Access scope for a user account without a parent uer type', () => {
testChildScopeNotDisplayed(null);
it('should not display the Child Account Access scope for a user account without a parent user type', () => {
testChildScopeNotDisplayed('default');
});

it('should not display the Child Account Access scope for "proxy" user type', () => {
Original file line number Diff line number Diff line change
@@ -102,7 +102,10 @@ describe('UserMenu', () => {
rest.get('*/profile', (req, res, ctx) => {
return res(
ctx.json(
profileFactory.build({ user_type: null, username: 'regular-user' })
profileFactory.build({
user_type: 'default',
username: 'regular-user',
})
)
);
})
Original file line number Diff line number Diff line change
@@ -71,7 +71,11 @@ export const UserMenu = React.memo(() => {
hasParentChildAccountAccess && (isParentUser || isProxyUser);
const open = Boolean(anchorEl);
const id = open ? 'user-menu-popover' : undefined;
const companyName = (profile?.user_type && account?.company) ?? '';
const companyName =
(hasParentChildAccountAccess &&
profile?.user_type !== 'default' &&
account?.company) ??
'';
Comment on lines +74 to +78
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was the fix for the user menu that would have displayed the company name for regular users, without explicitly checking now that the user_type isn't default.

const showCompanyName = hasParentChildAccountAccess && companyName;

// Used for fetching parent profile and account data by making a request with the parent's token.
2 changes: 1 addition & 1 deletion packages/manager/src/features/Users/UserRow.test.tsx
Original file line number Diff line number Diff line change
@@ -117,7 +117,7 @@ describe('UserRow', () => {
}),
// Mock the active profile, which must NOT be of `parent` user type to hide the Child Account Access column.
rest.get('*/profile', (req, res, ctx) => {
return res(ctx.json(profileFactory.build({ user_type: null })));
return res(ctx.json(profileFactory.build({ user_type: 'default' })));
})
);

Loading