Skip to content

Commit

Permalink
upcoming: [M3-7696] - Edit Access key Drawer - Fix Save button is dis…
Browse files Browse the repository at this point in the history
…abled (linode#10118)

* upcoming: [M3-7696] - Edit Access key Drawer - Save Edit Access key Drawer - Fix save button is disabled.

* Remove unused imports

* Added changeset: Edit Access key Drawer - Fix Save button is disabled.

* code cleanup

* Update utils.ts

* PR - feedback

* PR feedback - @abailly-akamai

* Update packages/manager/.changeset/pr-10118-upcoming-features-1706544516803.md

Co-authored-by: Dajahi Wiley <[email protected]>

* PR - feedback - @DevDW

* Remove error when regions are selected.

---------

Co-authored-by: Dajahi Wiley <[email protected]>
  • Loading branch information
cpathipa and dwiley-akamai authored Feb 7, 2024
1 parent e33c125 commit 165b5bf
Show file tree
Hide file tree
Showing 11 changed files with 285 additions and 39 deletions.
7 changes: 5 additions & 2 deletions packages/api-v4/src/object-storage/objectStorageKeys.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
import { createObjectStorageKeysSchema } from '@linode/validation/lib/objectStorageKeys.schema';
import {
createObjectStorageKeysSchema,
updateObjectStorageKeysSchema,
} from '@linode/validation/lib/objectStorageKeys.schema';
import { API_ROOT } from '../constants';
import Request, {
setData,
Expand Down Expand Up @@ -51,7 +54,7 @@ export const updateObjectStorageKey = (
Request<ObjectStorageKey>(
setMethod('PUT'),
setURL(`${API_ROOT}/object-storage/keys/${encodeURIComponent(id)}`),
setData(data, createObjectStorageKeysSchema)
setData(data, updateObjectStorageKeysSchema)
);

/**
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@linode/manager": Upcoming Features
---

"Save" button in Edit Access Key drawer disabled unless field values are changed ([#10118](https://github.com/linode/manager/pull/10118))
Original file line number Diff line number Diff line change
Expand Up @@ -698,10 +698,10 @@ class LinodeCreateContainer extends React.PureComponent<CombinedProps, State> {
selectedTypeID: this.params.typeID,
showGDPRCheckbox: Boolean(
!this.props.profile.data?.restricted &&
isEURegion(
getSelectedRegionGroup(this.props.regionsData, this.params.regionID)
) &&
this.props.agreements?.data?.eu_model
isEURegion(
getSelectedRegionGroup(this.props.regionsData, this.params.regionID)
) &&
this.props.agreements?.data?.eu_model
),
signedAgreement: false,
};
Expand Down Expand Up @@ -833,10 +833,10 @@ class LinodeCreateContainer extends React.PureComponent<CombinedProps, State> {
const request =
createType === 'fromLinode'
? () =>
this.props.linodeActions.cloneLinode({
sourceLinodeId: linodeID!,
...payload,
})
this.props.linodeActions.cloneLinode({
sourceLinodeId: linodeID!,
...payload,
})
: () => this.props.linodeActions.createLinode(payload);

this.setState({ formIsSubmitting: true });
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import {
Scope,
} from '@linode/api-v4/lib/object-storage';
import { createObjectStorageKeysSchema } from '@linode/validation/lib/objectStorageKeys.schema';
import { Formik } from 'formik';
import { Formik, FormikProps } from 'formik';
import * as React from 'react';

import { ActionsPanel } from 'src/components/ActionsPanel/ActionsPanel';
Expand All @@ -33,7 +33,10 @@ export interface AccessKeyDrawerProps {
// If the mode is 'editing', we should have an ObjectStorageKey to edit
objectStorageKey?: ObjectStorageKey;
onClose: () => void;
onSubmit: (values: ObjectStorageKeyRequest, formikProps: any) => void;
onSubmit: (
values: ObjectStorageKeyRequest,
formikProps: FormikProps<ObjectStorageKeyRequest>
) => void;
open: boolean;
}

Expand Down Expand Up @@ -120,7 +123,10 @@ export const AccessKeyDrawer = (props: AccessKeyDrawerProps) => {
label: initialLabelValue,
};

const handleSubmit = (values: ObjectStorageKeyRequest, formikProps: any) => {
const handleSubmit = (
values: ObjectStorageKeyRequest,
formikProps: FormikProps<ObjectStorageKeyRequest>
) => {
// If the user hasn't toggled the Limited Access button,
// don't include any bucket_access information in the payload.

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import {
revokeObjectStorageKey,
updateObjectStorageKey,
} from '@linode/api-v4/lib/object-storage';
import { FormikBag } from 'formik';
import { FormikBag, FormikHelpers } from 'formik';
import * as React from 'react';

import { DocumentTitleSegment } from 'src/components/DocumentTitle';
Expand Down Expand Up @@ -96,7 +96,11 @@ export const AccessKeyLanding = (props: Props) => {

const handleCreateKey = (
values: ObjectStorageKeyRequest,
{ setErrors, setStatus, setSubmitting }: FormikProps
{
setErrors,
setStatus,
setSubmitting,
}: FormikHelpers<ObjectStorageKeyRequest>
) => {
// Clear out status (used for general errors)
setStatus(null);
Expand Down Expand Up @@ -152,7 +156,11 @@ export const AccessKeyLanding = (props: Props) => {

const handleEditKey = (
values: ObjectStorageKeyRequest,
{ setErrors, setStatus, setSubmitting }: FormikProps
{
setErrors,
setStatus,
setSubmitting,
}: FormikHelpers<ObjectStorageKeyRequest>
) => {
// This shouldn't happen, but just in case.
if (!keyToEdit) {
Expand All @@ -170,7 +178,7 @@ export const AccessKeyLanding = (props: Props) => {

setSubmitting(true);

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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ export const AccessKeyActionMenu = ({
onClick: () => {
openDrawer('editing', objectStorageKey);
},
title: 'Edit Label',
title: isObjMultiClusterEnabled ? 'Edit' : 'Edit Label',
},
{
onClick: () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,10 @@ import {
ObjectStorageKey,
ObjectStorageKeyRequest,
Scope,
UpdateObjectStorageKeyRequest,
} from '@linode/api-v4/lib/object-storage';
import { createObjectStorageKeysSchema } from '@linode/validation/lib/objectStorageKeys.schema';
import { useFormik } from 'formik';
import { useFormik, FormikProps } from 'formik';
import React, { useEffect, useState } from 'react';

import { ActionsPanel } from 'src/components/ActionsPanel/ActionsPanel';
Expand All @@ -28,14 +29,20 @@ import { confirmObjectStorage } from '../utilities';
import { AccessKeyRegions } from './AccessKeyRegions/AccessKeyRegions';
import { LimitedAccessControls } from './LimitedAccessControls';
import { MODE } from './types';
import { generateUpdatePayload, hasLabelOrRegionsChanged } from './utils';

export interface AccessKeyDrawerProps {
isRestrictedUser: boolean;
mode: MODE;
// If the mode is 'editing', we should have an ObjectStorageKey to edit
objectStorageKey?: ObjectStorageKey;
onClose: () => void;
onSubmit: (values: ObjectStorageKeyRequest, formikProps: any) => void;
onSubmit: (
values: ObjectStorageKeyRequest | UpdateObjectStorageKeyRequest,
formikProps: FormikProps<
ObjectStorageKeyRequest | UpdateObjectStorageKeyRequest
>
) => void;
open: boolean;
}

Expand Down Expand Up @@ -116,10 +123,11 @@ export const OMC_AccessKeyDrawer = (props: AccessKeyDrawerProps) => {
// and so not included in Formik's types
const [limitedAccessChecked, setLimitedAccessChecked] = useState(false);

const title = createMode ? 'Create Access Key' : 'Edit Access Key Label';
const title = createMode ? 'Create Access Key' : 'Edit Access Key';

const initialLabelValue =
!createMode && objectStorageKey ? objectStorageKey.label : '';

const initialRegions =
!createMode && objectStorageKey
? objectStorageKey.regions?.map((region) => region.id)
Expand Down Expand Up @@ -148,13 +156,25 @@ export const OMC_AccessKeyDrawer = (props: AccessKeyDrawerProps) => {
}
: { ...values, bucket_access: null };

onSubmit(payload, formik);
const updatePayload = generateUpdatePayload(values, initialValues);

if (mode !== 'creating') {
onSubmit(updatePayload, formik);
} else {
onSubmit(payload, formik);
}
},
validateOnBlur: true,
validateOnChange: false,
validationSchema: createObjectStorageKeysSchema,
});

const isSaveDisabled =
isRestrictedUser ||
(mode !== 'creating' &&
objectStorageKey &&
!hasLabelOrRegionsChanged(formik.values, objectStorageKey));

const beforeSubmit = () => {
confirmObjectStorage<FormState>(
accountSettings?.object_storage || 'active',
Expand Down Expand Up @@ -257,6 +277,7 @@ export const OMC_AccessKeyDrawer = (props: AccessKeyDrawerProps) => {
'bucket_access',
getDefaultScopes(bucketsInRegions, regionsLookup)
);
formik.validateField('regions');
}}
onChange={(values) => {
const bucketsInRegions = buckets?.filter(
Expand Down Expand Up @@ -287,10 +308,7 @@ export const OMC_AccessKeyDrawer = (props: AccessKeyDrawerProps) => {
<ActionsPanel
primaryButtonProps={{
'data-testid': 'submit',
disabled:
isRestrictedUser ||
(mode !== 'creating' &&
formik.values.label === initialLabelValue),
disabled: isSaveDisabled,
label: createMode ? 'Create Access Key' : 'Save Changes',
loading: formik.isSubmitting,
onClick: beforeSubmit,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,98 @@
import { ObjectStorageKey } from '@linode/api-v4/lib/object-storage';

import { FormState } from './OMC_AccessKeyDrawer';
import { generateUpdatePayload, hasLabelOrRegionsChanged } from './utils';

describe('generateUpdatePayload', () => {
const initialValues: FormState = {
bucket_access: [],
label: 'initialLabel',
regions: ['region1', 'region2'],
};

it('should return empty object if no changes', () => {
const updatedValues = { ...initialValues };
expect(generateUpdatePayload(updatedValues, initialValues)).toEqual({});
});

it('should return updated label if only label changed', () => {
const updatedValues = { ...initialValues, label: 'newLabel' };
expect(generateUpdatePayload(updatedValues, initialValues)).toEqual({
label: 'newLabel',
});
});

it('should return updated regions if only regions changed', () => {
const updatedValues = { ...initialValues, regions: ['region3', 'region4'] };
expect(generateUpdatePayload(updatedValues, initialValues)).toEqual({
regions: ['region3', 'region4'],
});
});

it('should return updated label and regions if both changed', () => {
const updatedValues = {
bucket_access: [],
label: 'newLabel',
regions: ['region3', 'region4'],
};
expect(generateUpdatePayload(updatedValues, initialValues)).toEqual({
label: 'newLabel',
regions: ['region3', 'region4'],
});
});
});

describe('hasLabelOrRegionsChanged', () => {
const updatedValues: FormState = {
bucket_access: [],
label: 'initialLabel',
regions: ['region3', 'region4'],
};
const initialValues: ObjectStorageKey = {
access_key: '',
bucket_access: null,
id: 0,
label: updatedValues.label,
limited: false,
regions: [
{ id: 'region3', s3_endpoint: '' },
{ id: 'region4', s3_endpoint: '' },
],

secret_key: '',
};

it('returns false when both label and regions are unchanged', () => {
expect(hasLabelOrRegionsChanged(updatedValues, initialValues)).toBe(false);
});

it('returns true when only the label has changed', () => {
expect(
hasLabelOrRegionsChanged(
{ ...updatedValues, label: 'newLabel' },
initialValues
)
).toBe(true);
});

it('returns true when only the regions have changed', () => {
expect(
hasLabelOrRegionsChanged(
{
...updatedValues,
regions: ['region5'],
},
initialValues
)
).toBe(true);
});

it('returns true when both label and regions have changed', () => {
expect(
hasLabelOrRegionsChanged(
{ ...updatedValues, label: 'newLabel', regions: ['region5'] },
initialValues
)
).toBe(true);
});
});
Loading

0 comments on commit 165b5bf

Please sign in to comment.