fix: Profile Query Keys, User Permissions loading state, and read_onl…

…y PAT creation (#10040) * fix profile query keys * add extra bug fix * Fix loading state for User Permissions page * fix two factor invalidation * use `null` insted of empty object * Fix error for RO PAT creation --------- Co-authored-by: Banks Nussman <banks@nussman.us> Co-authored-by: mjac0bs <mjacobs@akamai.com>
linode · Jan 8, 2024 · e66fcf6 · e66fcf6
1 parent c1ce3ec
commit e66fcf6
Show file tree

Hide file tree

Showing 7 changed files with 94 additions and 45 deletions.
diff --git a/packages/manager/src/features/Profile/APITokens/CreateAPITokenDrawer.tsx b/packages/manager/src/features/Profile/APITokens/CreateAPITokenDrawer.tsx
@@ -168,6 +168,18 @@ export const CreateAPITokenDrawer = (props: Props) => {
   const filteredPermissions = allPermissions.filter(
     (scopeTup) => basePermNameMap[scopeTup[0]] !== 'Child Account Access'
   );
+  // TODO: Parent/Child - remove this conditional once code is in prod.
+  // Note: We couldn't include 'child_account' in our list of permissions in utils
+  // because it needs to be feature-flagged. Therefore, we're manually adding it here.
+  if (flags.parentChildAccountAccess && user?.user_type !== null) {
+    const childAccountIndex = allPermissions.findIndex(
+      ([scope]) => scope === 'child_account'
+    );
+    if (childAccountIndex === -1) {
+      allPermissions.push(['child_account', 0]);
+    }
+    basePermNameMap.child_account = 'Child Account Access';
+  }
 
   return (
     <Drawer onClose={onClose} open={open} title="Add Personal Access Token">

diff --git a/packages/manager/src/features/Profile/APITokens/ViewAPITokenDrawer.test.tsx b/packages/manager/src/features/Profile/APITokens/ViewAPITokenDrawer.test.tsx
@@ -20,7 +20,8 @@ vi.mock('src/queries/accountUsers', async () => {
   };
 });
 
-const nonParentPerms = basePerms.filter((value) => value !== 'child_account');
+// TODO: Parent/Child - add back after API code is in prod. Replace basePerms with nonParentPerms.
+// const nonParentPerms = basePerms.filter((value) => value !== 'child_account');
 
 const token = appTokenFactory.build({ label: 'my-token', scopes: '*' });
 const limitedToken = appTokenFactory.build({
@@ -43,7 +44,7 @@ describe('View API Token Drawer', () => {
 
   it('should show all permissions as read/write with wildcard scopes', () => {
     const { getByTestId } = renderWithTheme(<ViewAPITokenDrawer {...props} />);
-    for (const permissionName of nonParentPerms) {
+    for (const permissionName of basePerms) {
       expect(getByTestId(`perm-${permissionName}`)).toHaveAttribute(
         'aria-label',
         `This token has 2 access for ${permissionName}`
@@ -56,7 +57,7 @@ describe('View API Token Drawer', () => {
       <ViewAPITokenDrawer {...props} token={limitedToken} />,
       { flags: { parentChildAccountAccess: false } }
     );
-    for (const permissionName of nonParentPerms) {
+    for (const permissionName of basePerms) {
       expect(getByTestId(`perm-${permissionName}`)).toHaveAttribute(
         'aria-label',
         `This token has 0 access for ${permissionName}`
@@ -71,7 +72,7 @@ describe('View API Token Drawer', () => {
         token={appTokenFactory.build({ scopes: 'account:read_write' })}
       />
     );
-    for (const permissionName of nonParentPerms) {
+    for (const permissionName of basePerms) {
       // We only expect account to have read/write for this test
       const expectedScopeLevel = permissionName === 'account' ? 2 : 0;
       expect(getByTestId(`perm-${permissionName}`)).toHaveAttribute(
@@ -109,7 +110,7 @@ describe('View API Token Drawer', () => {
       volumes: 1,
     } as const;
 
-    for (const permissionName of nonParentPerms) {
+    for (const permissionName of basePerms) {
       const expectedScopeLevel = expectedScopeLevels[permissionName];
       expect(getByTestId(`perm-${permissionName}`)).toHaveAttribute(
         'aria-label',
@@ -136,8 +137,9 @@ describe('View API Token Drawer', () => {
     );
 
     const childScope = getByText('Child Account Access');
+    // TODO: Parent/Child - confirm that this scope level shouldn't be 2
     const expectedScopeLevels = {
-      child_account: 2,
+      child_account: 0,
     } as const;
     const childPermissionName = 'child_account';
 

diff --git a/packages/manager/src/features/Profile/APITokens/ViewAPITokenDrawer.tsx b/packages/manager/src/features/Profile/APITokens/ViewAPITokenDrawer.tsx
@@ -41,6 +41,18 @@ export const ViewAPITokenDrawer = (props: Props) => {
   const filteredPermissions = allPermissions.filter(
     (scopeTup) => basePermNameMap[scopeTup[0]] !== 'Child Account Access'
   );
+  // TODO: Parent/Child - remove this conditional once code is in prod.
+  // Note: We couldn't include 'child_account' in our list of permissions in utils
+  // because it needs to be feature-flagged. Therefore, we're manually adding it here.
+  if (flags.parentChildAccountAccess && user?.user_type !== null) {
+    const childAccountIndex = allPermissions.findIndex(
+      ([scope]) => scope === 'child_account'
+    );
+    if (childAccountIndex === -1) {
+      allPermissions.push(['child_account', 0]);
+    }
+    basePermNameMap.child_account = 'Child Account Access';
+  }
 
   return (
     <Drawer onClose={onClose} open={open} title={token?.label ?? 'Token'}>

diff --git a/packages/manager/src/features/Profile/APITokens/utils.test.ts b/packages/manager/src/features/Profile/APITokens/utils.test.ts
@@ -26,7 +26,8 @@ describe('APIToken utils', () => {
       const result = scopeStringToPermTuples('*');
       const expected = [
         ['account', 2],
-        ['child_account', 2],
+        // TODO: Parent/Child - add this scope once code is in prod.
+        // ['child_account', 2],
         ['databases', 2],
         ['domains', 2],
         ['events', 2],
@@ -50,7 +51,8 @@ describe('APIToken utils', () => {
       const result = scopeStringToPermTuples('');
       const expected = [
         ['account', 0],
-        ['child_account', 0],
+        // TODO: Parent/Child - add this scope once code is in prod.
+        // ['child_account', 0],
         ['databases', 0],
         ['domains', 0],
         ['events', 0],
@@ -75,7 +77,8 @@ describe('APIToken utils', () => {
       const result = scopeStringToPermTuples('account:none');
       const expected = [
         ['account', 0],
-        ['child_account', 0],
+        // TODO: Parent/Child - add this scope once code is in prod.
+        // ['child_account', 0],
         ['databases', 0],
         ['domains', 0],
         ['events', 0],
@@ -100,7 +103,8 @@ describe('APIToken utils', () => {
       const result = scopeStringToPermTuples('account:read_only');
       const expected = [
         ['account', 1],
-        ['child_account', 0],
+        // TODO: Parent/Child - add this scope once code is in prod.
+        // ['child_account', 0],
         ['databases', 0],
         ['domains', 0],
         ['events', 0],
@@ -125,7 +129,8 @@ describe('APIToken utils', () => {
       const result = scopeStringToPermTuples('account:read_write');
       const expected = [
         ['account', 2],
-        ['child_account', 0],
+        // TODO: Parent/Child - add this scope once code is in prod.
+        // ['child_account', 0],
         ['databases', 0],
         ['domains', 0],
         ['events', 0],
@@ -152,7 +157,8 @@ describe('APIToken utils', () => {
       );
       const expected = [
         ['account', 0],
-        ['child_account', 0],
+        // TODO: Parent/Child - add this scope once code is in prod.
+        // ['child_account', 0],
         ['databases', 0],
         ['domains', 1],
         ['events', 0],
@@ -181,7 +187,8 @@ describe('APIToken utils', () => {
       const result = scopeStringToPermTuples('account:none,tokens:read_write');
       const expected = [
         ['account', 2],
-        ['child_account', 0],
+        // TODO: Parent/Child - add this scope once code is in prod.
+        // ['child_account', 0],
         ['databases', 0],
         ['domains', 0],
         ['events', 0],
@@ -210,7 +217,8 @@ describe('APIToken utils', () => {
       const result = scopeStringToPermTuples('account:read_only,tokens:none');
       const expected = [
         ['account', 1],
-        ['child_account', 0],
+        // TODO: Parent/Child - add this scope once code is in prod.
+        // ['child_account', 0],
         ['databases', 0],
         ['domains', 0],
         ['events', 0],
@@ -235,7 +243,8 @@ describe('APIToken utils', () => {
       it('should return 0 if all scopes are 0', () => {
         const scopes: Permission[] = [
           ['account', 0],
-          ['child_account', 0],
+          // TODO: Parent/Child - add this scope once code is in prod.
+          // ['child_account', 0],
           ['databases', 0],
           ['domains', 0],
           ['events', 0],
@@ -255,7 +264,8 @@ describe('APIToken utils', () => {
       it('should return 1 if all scopes are 1', () => {
         const scopes: Permission[] = [
           ['account', 1],
-          ['child_account', 1],
+          // TODO: Parent/Child - add this scope once code is in prod.
+          // ['child_account', 1],
           ['databases', 1],
           ['domains', 1],
           ['events', 1],
@@ -275,7 +285,8 @@ describe('APIToken utils', () => {
       it('should return 2 if all scopes are 2', () => {
         const scopes: Permission[] = [
           ['account', 2],
-          ['child_account', 2],
+          // TODO: Parent/Child - add this scope once code is in prod.
+          // ['child_account', 2],
           ['databases', 2],
           ['domains', 2],
           ['events', 2],
@@ -295,7 +306,8 @@ describe('APIToken utils', () => {
       it('should return null if all scopes are different', () => {
         const scopes: Permission[] = [
           ['account', 1],
-          ['child_account', 0],
+          // TODO: Parent/Child - add this scope once code is in prod.
+          // ['child_account', 0],
           ['databases', 0],
           ['domains', 2],
           ['events', 0],

diff --git a/packages/manager/src/features/Profile/APITokens/utils.ts b/packages/manager/src/features/Profile/APITokens/utils.ts
@@ -6,7 +6,8 @@ export type Permission = [string, number];
 
 export const basePerms = [
   'account',
-  'child_account',
+  // TODO: Parent/Child - add this scope once API code is in prod.
+  // 'child_account',
   'databases',
   'domains',
   'events',
@@ -24,7 +25,8 @@ export const basePerms = [
 
 export const basePermNameMap: Record<string, string> = {
   account: 'Account',
-  child_account: 'Child Account Access',
+  // TODO: Parent/Child - add this scope once API code is in prod.
+  // child_account: 'Child Account Access',
   databases: 'Databases',
   domains: 'Domains',
   events: 'Events',

diff --git a/packages/manager/src/features/Users/UserPermissions.tsx b/packages/manager/src/features/Users/UserPermissions.tsx
@@ -19,6 +19,7 @@ import { compose as recompose } from 'recompose';
 
 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';
@@ -113,12 +114,13 @@ class UserPermissions extends React.Component<CombinedProps, State> {
   }
 
   render() {
+    const { loading } = this.state;
     const { username } = this.props;
 
     return (
       <React.Fragment>
         <DocumentTitleSegment segment={`${username} - Permissions`} />
-        {this.renderBody()}
+        {loading ? <CircleProgress /> : this.renderBody()}
       </React.Fragment>
     );
   }

diff --git a/packages/manager/src/queries/profile.ts b/packages/manager/src/queries/profile.ts
@@ -41,11 +41,19 @@ import type { RequestOptions } from '@linode/api-v4';
 
 export const queryKey = 'profile';
 
-export const useProfile = ({ headers }: RequestOptions = {}) => {
-  const key = [queryKey, headers];
-  return useQuery<Profile, APIError[]>(key, () => getProfile({ headers }), {
-    ...queryPresets.oneTimeFetch,
-  });
+export const useProfile = (options?: RequestOptions) => {
+  const key = [
+    queryKey,
+    options?.headers ? { headers: options.headers } : null,
+  ];
+
+  return useQuery<Profile, APIError[]>(
+    key,
+    () => getProfile({ headers: options?.headers }),
+    {
+      ...queryPresets.oneTimeFetch,
+    }
+  );
 };
 
 export const useMutateProfile = () => {
@@ -60,28 +68,25 @@ export const updateProfileData = (
   newData: Partial<Profile>,
   queryClient: QueryClient
 ): void => {
-  queryClient.setQueryData([queryKey, undefined], (oldData: Profile) => ({
+  queryClient.setQueryData([queryKey, null], (oldData: Profile) => ({
     ...oldData,
     ...newData,
   }));
 };
 
 export const useGrants = () => {
   const { data: profile } = useProfile();
-  return useQuery<Grants, APIError[]>(
-    [queryKey, undefined, 'grants'],
-    listGrants,
-    {
-      ...queryPresets.oneTimeFetch,
-      enabled: Boolean(profile?.restricted),
-    }
-  );
+  return useQuery<Grants, APIError[]>([queryKey, 'grants'], listGrants, {
+    ...queryPresets.oneTimeFetch,
+    enabled: Boolean(profile?.restricted),
+  });
 };
 
 export const getProfileData = (queryClient: QueryClient) =>
-  queryClient.getQueryData<Profile>([queryKey, undefined]);
+  queryClient.getQueryData<Profile>([queryKey, null]);
+
 export const getGrantData = (queryClient: QueryClient) =>
-  queryClient.getQueryData<Grants>([queryKey, undefined, 'grants']);
+  queryClient.getQueryData<Grants>([queryKey, 'grants']);
 
 export const useSMSOptOutMutation = () => {
   const queryClient = useQueryClient();
@@ -108,7 +113,7 @@ export const useSSHKeysQuery = (
   enabled = true
 ) =>
   useQuery(
-    [queryKey, {}, 'ssh-keys', 'paginated', params, filter],
+    [queryKey, 'ssh-keys', 'paginated', params, filter],
     () => getSSHKeys(params, filter),
     {
       enabled,
@@ -122,7 +127,7 @@ export const useCreateSSHKeyMutation = () => {
     createSSHKey,
     {
       onSuccess() {
-        queryClient.invalidateQueries([queryKey, undefined, 'ssh-keys']);
+        queryClient.invalidateQueries([queryKey, 'ssh-keys']);
         // also invalidate the /account/users data because that endpoint returns some SSH key data
         queryClient.invalidateQueries([accountQueryKey, 'users']);
       },
@@ -136,7 +141,7 @@ export const useUpdateSSHKeyMutation = (id: number) => {
     (data) => updateSSHKey(id, data),
     {
       onSuccess() {
-        queryClient.invalidateQueries([queryKey, undefined, 'ssh-keys']);
+        queryClient.invalidateQueries([queryKey, 'ssh-keys']);
         // also invalidate the /account/users data because that endpoint returns some SSH key data
         queryClient.invalidateQueries([accountQueryKey, 'users']);
       },
@@ -148,7 +153,7 @@ export const useDeleteSSHKeyMutation = (id: number) => {
   const queryClient = useQueryClient();
   return useMutation<{}, APIError[]>(() => deleteSSHKey(id), {
     onSuccess() {
-      queryClient.invalidateQueries([queryKey, undefined, 'ssh-keys']);
+      queryClient.invalidateQueries([queryKey, 'ssh-keys']);
       // also invalidate the /account/users data because that endpoint returns some SSH key data
       queryClient.invalidateQueries([accountQueryKey, 'users']);
     },
@@ -159,14 +164,14 @@ export const sshKeyEventHandler = (event: EventWithStore) => {
   // This event handler is a bit agressive and will over-fetch, but UX will
   // be great because this will ensure Cloud has up to date data all the time.
 
-  event.queryClient.invalidateQueries([queryKey, undefined, 'ssh-keys']);
+  event.queryClient.invalidateQueries([queryKey, 'ssh-keys']);
   // also invalidate the /account/users data because that endpoint returns some SSH key data
   event.queryClient.invalidateQueries([accountQueryKey, 'users']);
 };
 
 export const useTrustedDevicesQuery = (params?: Params, filter?: Filter) =>
   useQuery<ResourcePage<TrustedDevice>, APIError[]>(
-    [queryKey, {}, 'trusted-devices', 'paginated', params, filter],
+    [queryKey, 'trusted-devices', 'paginated', params, filter],
     () => getTrustedDevices(params, filter),
     {
       keepPreviousData: true,
@@ -177,7 +182,7 @@ export const useRevokeTrustedDeviceMutation = (id: number) => {
   const queryClient = useQueryClient();
   return useMutation<{}, APIError[]>(() => deleteTrustedDevice(id), {
     onSuccess() {
-      queryClient.invalidateQueries([queryKey, undefined, 'trusted-devices']);
+      queryClient.invalidateQueries([queryKey, 'trusted-devices']);
     },
   });
 };
@@ -186,7 +191,9 @@ export const useDisableTwoFactorMutation = () => {
   const queryClient = useQueryClient();
   return useMutation<{}, APIError[]>(disableTwoFactor, {
     onSuccess() {
-      queryClient.invalidateQueries([queryKey, undefined]);
+      queryClient.invalidateQueries([queryKey, null]);
+      // also invalidate the /account/users data because that endpoint returns 2FA status for each user
+      queryClient.invalidateQueries([accountQueryKey, 'users']);
     },
   });
 };