Skip to content
Open
Show file tree
Hide file tree
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
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 @@ -5,6 +5,7 @@ export const CUSTOMER_SUPPORT = 'customer support';
export const grantTypeMap = {
account: 'Account',
bucket: 'Buckets',
key: 'Access Keys',
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure if we should add grant mapping for keys if there are no key grants at the moment. I understand that this is for getRestrictedResourceText method, but maybe it would be better to provide an explicit tooltip text for the Access Key button for now, e.g. "You must have full account access to perform this operation.".
We will need some input from the Cloud Manager core team on this one.

Copy link
Contributor

Choose a reason for hiding this comment

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

I just discussed it with Jason - we should use a dedicated text for both Buckets and Access Key. Jason sugested the following text:

You don't have administrator privileges for Object Storage.Β Your administrator must change your account to full access to provide access to Object Storage.

We can improve the text later after discussing it with TW.

database: 'Databases',
domain: 'Domains',
firewall: 'Firewalls',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,17 +4,9 @@ import { renderWithTheme } from 'src/utilities/testHelpers';

import { AccessKeyLanding } from './AccessKeyLanding';

const props = {
accessDrawerOpen: false,
closeAccessDrawer: vi.fn(),
isRestrictedUser: false,
mode: 'creating' as any,
openAccessDrawer: vi.fn(),
};

describe('AccessKeyLanding', () => {
it('should render a table of access keys', async () => {
const { getByTestId } = renderWithTheme(<AccessKeyLanding {...props} />);
const { getByTestId } = renderWithTheme(<AccessKeyLanding />);
expect(getByTestId('data-qa-access-key-table')).toBeInTheDocument();
});
});
Original file line number Diff line number Diff line change
@@ -1,59 +1,22 @@
import {
createObjectStorageKeys,
revokeObjectStorageKey,
updateObjectStorageKey,
} from '@linode/api-v4/lib/object-storage';
import { useAccountSettings } from '@linode/queries';
import { revokeObjectStorageKey } from '@linode/api-v4/lib/object-storage';
import { useErrors, useOpenClose } from '@linode/utilities';
import { useNavigate } from '@tanstack/react-router';
import * as React from 'react';

import { DocumentTitleSegment } from 'src/components/DocumentTitle';
import { PaginationFooter } from 'src/components/PaginationFooter/PaginationFooter';
import { SecretTokenDialog } from 'src/features/Profile/SecretTokenDialog/SecretTokenDialog';
import { usePaginationV2 } from 'src/hooks/usePaginationV2';
import { useObjectStorageAccessKeys } from 'src/queries/object-storage/queries';
import {
sendCreateAccessKeyEvent,
sendEditAccessKeyEvent,
sendRevokeAccessKeyEvent,
} from 'src/utilities/analytics/customEventAnalytics';
import { getAPIErrorOrDefault, getErrorMap } from 'src/utilities/errorUtils';
import { sendRevokeAccessKeyEvent } from 'src/utilities/analytics/customEventAnalytics';
import { getAPIErrorOrDefault } from 'src/utilities/errorUtils';

import { useIsObjMultiClusterEnabled } from '../hooks/useIsObjectStorageGen2Enabled';
import { AccessKeyDrawer } from './AccessKeyDrawer';
import { AccessKeyTable } from './AccessKeyTable/AccessKeyTable';
import { OMC_AccessKeyDrawer } from './OMC_AccessKeyDrawer';
import { RevokeAccessKeyDialog } from './RevokeAccessKeyDialog';
import { ViewPermissionsDrawer } from './ViewPermissionsDrawer';

import type { MODE, OpenAccessDrawer } from './types';
import type {
CreateObjectStorageKeyPayload,
ObjectStorageKey,
UpdateObjectStorageKeyPayload,
} from '@linode/api-v4/lib/object-storage';
import type { FormikBag, FormikHelpers } from 'formik';

interface Props {
accessDrawerOpen: boolean;
closeAccessDrawer: () => void;
isRestrictedUser: boolean;
mode: MODE;
openAccessDrawer: (mode: MODE) => void;
}

export type FormikProps = FormikBag<Props, CreateObjectStorageKeyPayload>;

export const AccessKeyLanding = (props: Props) => {
const {
accessDrawerOpen,
closeAccessDrawer,
isRestrictedUser,
mode,
openAccessDrawer,
} = props;
import type { ObjectStorageKey } from '@linode/api-v4/lib/object-storage';

export const AccessKeyLanding = () => {
const navigate = useNavigate();
const pagination = usePaginationV2({
currentRoute: '/object-storage/access-keys',
Expand All @@ -66,30 +29,15 @@ export const AccessKeyLanding = (props: Props) => {
page_size: pagination.pageSize,
});

const { data: accountSettings, refetch: requestAccountSettings } =
useAccountSettings();

// Key to display in Confirmation Modal upon creation
const [keyToDisplay, setKeyToDisplay] =
React.useState<null | ObjectStorageKey>(null);

// Key to rename (by clicking on a key's kebab menu )
const [keyToEdit, setKeyToEdit] = React.useState<null | ObjectStorageKey>(
null
);

// Key to revoke (by clicking on a key's kebab menu )
const [keyToRevoke, setKeyToRevoke] = React.useState<null | ObjectStorageKey>(
null
);
const [isRevoking, setIsRevoking] = React.useState<boolean>(false);
const [revokeErrors, setRevokeErrors] = useErrors();

const displayKeysDialog = useOpenClose();
const revokeKeysDialog = useOpenClose();

const { isObjMultiClusterEnabled } = useIsObjMultiClusterEnabled();

// Redirect to base access keys route if current page has no data
// TODO: Remove this implementation and replace `usePagination` with `usePaginate` hook. See [M3-10442]
React.useEffect(() => {
Expand All @@ -109,123 +57,6 @@ export const AccessKeyLanding = (props: Props) => {
}
}, [data, isLoading, pagination.page, navigate]);

const handleCreateKey = (
values: CreateObjectStorageKeyPayload,
{
setErrors,
setStatus,
setSubmitting,
}: FormikHelpers<CreateObjectStorageKeyPayload>
) => {
// Clear out status (used for general errors)
setStatus(null);
setSubmitting(true);

createObjectStorageKeys(values)
.then((data) => {
setSubmitting(false);

setKeyToDisplay(data);

// "Refresh" keys to include the newly created key
refetch();

props.closeAccessDrawer();
displayKeysDialog.open();

// If our Redux Store says that the user doesn't have OBJ enabled,
// it probably means they have just enabled it with the creation
// of this key. In that case, update the Redux Store so that
// subsequently created keys don't need to go through the
// confirmation flow.
if (accountSettings?.object_storage === 'disabled') {
requestAccountSettings();
}

// @analytics
sendCreateAccessKeyEvent();
})
.catch((errorResponse) => {
// We also need to refresh account settings on failure, since, depending
// on the error, Object Storage service might have actually been enabled.
if (accountSettings?.object_storage === 'disabled') {
requestAccountSettings();
}

setSubmitting(false);

const errors = getAPIErrorOrDefault(
errorResponse,
'There was an issue creating your Access Key.'
);
const mappedErrors = getErrorMap(['label'], errors);

// `status` holds general errors
if (mappedErrors.none) {
setStatus(mappedErrors.none);
}

setErrors(mappedErrors);
});
};

const handleEditKey = (
values: UpdateObjectStorageKeyPayload,
{
setErrors,
setStatus,
setSubmitting,
}: FormikHelpers<UpdateObjectStorageKeyPayload>
) => {
// This shouldn't happen, but just in case.
if (!keyToEdit) {
return;
}

// Clear out status (used for general errors)
setStatus(null);

// If the new label is the same as the old one, no need to make an API
// request. Just close the drawer and return early.
if (values.label === keyToEdit.label) {
return closeAccessDrawer();
}

setSubmitting(true);

updateObjectStorageKey(
keyToEdit.id,
isObjMultiClusterEnabled ? values : { label: values.label }
)
.then((_) => {
setSubmitting(false);

// "Refresh" keys to display the newly updated key
refetch();

closeAccessDrawer();

// @analytics
sendEditAccessKeyEvent();
})
.catch((errorResponse) => {
setSubmitting(false);

const errors = getAPIErrorOrDefault(
errorResponse,
'There was an issue updating your Access Key.'
);
const mappedErrors = getErrorMap(['label'], errors);

// `status` holds general errors
if (mappedErrors.none) {
setStatus(mappedErrors.none);
}

setErrors(mappedErrors);
});
};

const handleRevokeKeys = () => {
// This shouldn't happen, but just in case.
if (!keyToRevoke) {
Expand Down Expand Up @@ -260,12 +91,19 @@ export const AccessKeyLanding = (props: Props) => {

const openDrawer: OpenAccessDrawer = (
mode: MODE,
objectStorageKey: null | ObjectStorageKey = null
objectStorageKey: ObjectStorageKey
) => {
setKeyToEdit(objectStorageKey);
if (mode !== 'creating') {
openAccessDrawer(mode);
let drawerUrl = `/object-storage/access-keys/${objectStorageKey.id}`;

if (mode === 'editing') {
drawerUrl += `/update`;
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we really need this? To make it consistent, we would need to have URL for each action. Looks like it's out of the scope for the current ticket.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

"we would need to have URL for each action", yes, this is the pattern we are using for Volumes, and I made the first step to migrate OBJ towards that same pattern.

"out of the scope for the current ticket.", refactoring yes, but for the landing page to work correctly I had to make AccessKeys and CreateBucket drawers to be standalone. It is a relatively small change in my opinion.

}

if (mode === 'viewing') {
drawerUrl += `/details`;
}

navigate({ to: drawerUrl });
};

const openRevokeDialog = (objectStorageKey: ObjectStorageKey) => {
Expand All @@ -279,19 +117,18 @@ export const AccessKeyLanding = (props: Props) => {
};

return (
<div>
<DocumentTitleSegment
segment={`${accessDrawerOpen ? `Create an Access Key` : `Access Keys`}`}
/>
<>
<DocumentTitleSegment segment="Access Keys" />
Copy link
Contributor

Choose a reason for hiding this comment

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

This change create inconsistency. In all other pages, the title says "Create a ..." when on the create page.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

To be honest, I don't like the way it was initially implemented. I did the simplification and added a TODO comment to create a proper component for displaying those titles.

Do you think we can leave it like this for now (there is no Create a title) and create a refactoring ticket?


<AccessKeyTable
data={data?.data}
data-qa-access-key-table
error={error}
isLoading={isLoading}
isRestrictedUser={isRestrictedUser}
openDrawer={openDrawer}
openRevokeDialog={openRevokeDialog}
/>

<PaginationFooter
count={data?.results || 0}
eventCategory="object storage keys table"
Expand All @@ -301,37 +138,6 @@ export const AccessKeyLanding = (props: Props) => {
pageSize={pagination.pageSize}
/>

{isObjMultiClusterEnabled ? (
<OMC_AccessKeyDrawer
isRestrictedUser={props.isRestrictedUser}
mode={mode}
objectStorageKey={keyToEdit ? keyToEdit : undefined}
onClose={closeAccessDrawer}
onSubmit={mode === 'creating' ? handleCreateKey : handleEditKey}
open={accessDrawerOpen}
/>
) : (
<AccessKeyDrawer
isRestrictedUser={props.isRestrictedUser}
mode={mode}
objectStorageKey={keyToEdit ? keyToEdit : undefined}
onClose={closeAccessDrawer}
onSubmit={mode === 'creating' ? handleCreateKey : handleEditKey}
open={accessDrawerOpen}
/>
)}

<ViewPermissionsDrawer
objectStorageKey={keyToEdit}
onClose={closeAccessDrawer}
open={mode === 'viewing' && accessDrawerOpen}
/>
<SecretTokenDialog
objectStorageKey={keyToDisplay}
onClose={displayKeysDialog.close}
open={displayKeysDialog.isOpen}
title="Access Keys"
/>
<RevokeAccessKeyDialog
errors={revokeErrors}
handleClose={closeRevokeDialog}
Expand All @@ -341,6 +147,6 @@ export const AccessKeyLanding = (props: Props) => {
label={keyToRevoke?.label || ''}
numAccessKeys={data?.results || 0}
/>
</div>
</>
);
};
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ describe('ObjectStorageKeyTable', () => {
data: [],
error: undefined,
isLoading: false,
isRestrictedUser: false,
openDrawer: vi.fn(),
openRevokeDialog: vi.fn(),
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,20 +24,12 @@ export interface AccessKeyTableProps {
data: ObjectStorageKey[] | undefined;
error: APIError[] | null | undefined;
isLoading: boolean;
isRestrictedUser: boolean;
openDrawer: OpenAccessDrawer;
openRevokeDialog: (objectStorageKey: ObjectStorageKey) => void;
}

export const AccessKeyTable = (props: AccessKeyTableProps) => {
const {
data,
error,
isLoading,
isRestrictedUser,
openDrawer,
openRevokeDialog,
} = props;
const { data, error, isLoading, openDrawer, openRevokeDialog } = props;

const [showHostNamesDrawer, setShowHostNamesDrawers] =
useState<boolean>(false);
Expand Down Expand Up @@ -85,7 +77,6 @@ export const AccessKeyTable = (props: AccessKeyTableProps) => {
error={error}
isLoading={isLoading}
isObjMultiClusterEnabled={isObjMultiClusterEnabled}
isRestrictedUser={isRestrictedUser}
openDrawer={openDrawer}
openRevokeDialog={openRevokeDialog}
setHostNames={setHostNames}
Expand Down
Loading
Loading