-
Notifications
You must be signed in to change notification settings - Fork 400
new: STORIF-320 - Object storage creation flow updated. #13508
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
base: develop
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| 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', | ||
|
|
@@ -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(() => { | ||
|
|
@@ -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) { | ||
|
|
@@ -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`; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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) => { | ||
|
|
@@ -279,19 +117,18 @@ export const AccessKeyLanding = (props: Props) => { | |
| }; | ||
|
|
||
| return ( | ||
| <div> | ||
| <DocumentTitleSegment | ||
| segment={`${accessDrawerOpen ? `Create an Access Key` : `Access Keys`}`} | ||
| /> | ||
| <> | ||
| <DocumentTitleSegment segment="Access Keys" /> | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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" | ||
|
|
@@ -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} | ||
|
|
@@ -341,6 +147,6 @@ export const AccessKeyLanding = (props: Props) => { | |
| label={keyToRevoke?.label || ''} | ||
| numAccessKeys={data?.results || 0} | ||
| /> | ||
| </div> | ||
| </> | ||
| ); | ||
| }; | ||
There was a problem hiding this comment.
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
getRestrictedResourceTextmethod, 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.
There was a problem hiding this comment.
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:
We can improve the text later after discussing it with TW.